RE: [PATCH net-next v1 1/5] net: lan743x: reset DMA channels that fail to stop

[Woojung.Huh]

Hi Chris & Nicolai,

...

> > @@ -2271,11 +2300,11 @@ static int lan743x_tx_open(struct lan743x_tx
> > *tx)
> >                         FCT_TX_CTL_EN_(tx->channel_number));
> >
> >       /* reset tx channel */
> > -     lan743x_csr_write(adapter, DMAC_CMD,
> > -                       DMAC_CMD_TX_SWR_(tx->channel_number));
> > -     lan743x_csr_wait_for_bit(adapter, DMAC_CMD,
> > -                              DMAC_CMD_TX_SWR_(tx->channel_number),
> > -                              0, 1000, 20000, 100);
> > +     ret = lan743x_dmac_tx_reset(adapter, tx->channel_number);
> > +     if (ret)
> > +             netdev_err(adapter->netdev,
> > +                        "TX channel %d reset timed out\n",
> > +                        tx->channel_number);
> 
> Not a bug, but what's the benefit of keeping going here?
> 
> If the channel never came out of reset, lan743x_tx_open() and
> lan743x_rx_open() could return the error instead of starting it anyway.
> 
> With the reset work from 4/5 a failed reopen then ends in dev_close(),
> which seems like the better place to end up. It would also settle the
> open question from the cover letter.
> 
Agree with Nicolai's comment that returning error at lan743x_tx/rx_open() to
make lan743x_netdev_open() returns with error.

Per Jakub's email, this patch could be a part of net and more chance to be
in stable tree without patch 4 & 5, it would be better to handle eroor case
along with error message.

Best regards,
Woojung