On Wed, 2022-11-30 at 18:09 +0100, lorenzo@xxxxxxxxxx wrote: > > Hi Lore, > > > > On Wed, 2022-11-30 at 15:46 +0100, Lorenzo Bianconi wrote: > > > > SDIO may need addtional 512 bytes to align bus operation. If > > > > the > > > > tailroom > > > > of this skb is not big enough, we would access invalid memory > > > > region. > > > > For low level operation, take xmit_buf instead of skb to keep > > > > valid > > > > memory > > > > access in SDIO. > > > > Note: xmit_buf is big enough for single skb size > > > > > > > > Fixes: 764dee47e2c1 ("mt76: sdio: move common code in mt76_sdio > > > > module") > > > > Tested-by: YN Chen <YN.Chen@xxxxxxxxxxxx> > > > > Signed-off-by: Deren Wu <deren.wu@xxxxxxxxxxxx> > > > > --- > > > > drivers/net/wireless/mediatek/mt76/sdio_txrx.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c > > > > b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c > > > > index bfc4de50a4d2..ebea5c4e8da5 100644 > > > > --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c > > > > +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c > > > > @@ -254,7 +254,8 @@ static int mt76s_tx_run_queue(struct > > > > mt76_dev > > > > *dev, struct mt76_queue *q) > > > > > > > > if (!test_bit(MT76_STATE_MCU_RUNNING, &dev- > > > > >phy.state)) > > > > { > > > > __skb_put_zero(e->skb, 4); > > > > - err = __mt76s_xmit_queue(dev, e->skb- > > > > >data, > > > > + memcpy(sdio->xmit_buf, e->skb->data, e- > > > > >skb- > > > > > len); > > > > > > (even if it is not critical for performance) iirc the skb from > > > the > > > mcu is > > > always linear, I guess we can use __skb_grow() instead. What do > > > you > > > think? > > > > > > Regards, > > > Lorenzo > > > > > > > _skb_grow() looks good for me. It's a balance solution for this > > case. > > If you have no concern about the patch below, I will post v2 after > > UT. > > :) > > > > if (!test_bit(MT76_STATE_MCU_RUNNING, &dev->phy.state)) { > > __skb_put_zero(e->skb, 4); > > + err = __skb_grow(e->skb, roundup(e->skb->len, > > + sdio->func- > > > cur_blksize)); > > can we merge __skb_put_zero() and __skb_grow()? Does sdio chip > require the 4 > last bytes to be 0? > > Regards, > Lorenzo The chip need padding 4 zero to align bus operation and we cannnot ignore the process here. I think we still need both of them. Regards, Deren > > > + if (err) > > + return err; > > err = __mt76s_xmit_queue(dev, e->skb->data, > > e->skb->len); > > > > > > Regards, > > Deren > > > > > > + err = __mt76s_xmit_queue(dev, sdio- > > > > >xmit_buf, > > > > e->skb->len); > > > > if (err) > > > > return err; > > > > -- > > > > 2.18.0 > > > >