2020-12-21 05:46:41

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH] net: lantiq_etop: check the result of request_irq()

The declaration of request_irq() in <linux/interrupt.h> is marked as
__must_check.

Without the return value check, I see the following warnings:

drivers/net/ethernet/lantiq_etop.c: In function 'ltq_etop_hw_init':
drivers/net/ethernet/lantiq_etop.c:273:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
  273 |    request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/lantiq_etop.c:281:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
  281 |    request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
      |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Miguel Ojeda <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
---

drivers/net/ethernet/lantiq_etop.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
index 2d0c52f7106b..960494f9752b 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -264,13 +264,18 @@ ltq_etop_hw_init(struct net_device *dev)
for (i = 0; i < MAX_DMA_CHAN; i++) {
int irq = LTQ_DMA_CH0_INT + i;
struct ltq_etop_chan *ch = &priv->ch[i];
+ int ret;

ch->idx = ch->dma.nr = i;
ch->dma.dev = &priv->pdev->dev;

if (IS_TX(i)) {
ltq_dma_alloc_tx(&ch->dma);
- request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
+ ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
+ if (ret) {
+ netdev_err(dev, "failed to request irq\n");
+ return ret;
+ }
} else if (IS_RX(i)) {
ltq_dma_alloc_rx(&ch->dma);
for (ch->dma.desc = 0; ch->dma.desc < LTQ_DESC_NUM;
@@ -278,7 +283,11 @@ ltq_etop_hw_init(struct net_device *dev)
if (ltq_etop_alloc_skb(ch))
return -ENOMEM;
ch->dma.desc = 0;
- request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
+ ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
+ if (ret) {
+ netdev_err(dev, "failed to request irq\n");
+ return ret;
+ }
}
ch->dma.irq = irq;
}
--
2.27.0


2020-12-21 15:31:15

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] net: lantiq_etop: check the result of request_irq()

On Mon, Dec 21, 2020 at 02:43:23PM +0900, Masahiro Yamada wrote:
> The declaration of request_irq() in <linux/interrupt.h> is marked as
> __must_check.
>
> Without the return value check, I see the following warnings:
>
> drivers/net/ethernet/lantiq_etop.c: In function 'ltq_etop_hw_init':
> drivers/net/ethernet/lantiq_etop.c:273:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
> ? 273 | ? ?request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> ? ? ? | ? ?^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/lantiq_etop.c:281:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
> ? 281 | ? ?request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
> ? ? ? | ? ?^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Reported-by: Miguel Ojeda <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> drivers/net/ethernet/lantiq_etop.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
> index 2d0c52f7106b..960494f9752b 100644
> --- a/drivers/net/ethernet/lantiq_etop.c
> +++ b/drivers/net/ethernet/lantiq_etop.c
> @@ -264,13 +264,18 @@ ltq_etop_hw_init(struct net_device *dev)
> for (i = 0; i < MAX_DMA_CHAN; i++) {
> int irq = LTQ_DMA_CH0_INT + i;
> struct ltq_etop_chan *ch = &priv->ch[i];
> + int ret;
>
> ch->idx = ch->dma.nr = i;
> ch->dma.dev = &priv->pdev->dev;
>
> if (IS_TX(i)) {
> ltq_dma_alloc_tx(&ch->dma);
> - request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> + ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> + if (ret) {
> + netdev_err(dev, "failed to request irq\n");
> + return ret;

You need to cleanup what ltq_dma_alloc_tx() did.

> + }
> } else if (IS_RX(i)) {
> ltq_dma_alloc_rx(&ch->dma);
> for (ch->dma.desc = 0; ch->dma.desc < LTQ_DESC_NUM;
> @@ -278,7 +283,11 @@ ltq_etop_hw_init(struct net_device *dev)
> if (ltq_etop_alloc_skb(ch))
> return -ENOMEM;
> ch->dma.desc = 0;
> - request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
> + ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
> + if (ret) {
> + netdev_err(dev, "failed to request irq\n");
> + return ret;

And here you need to cleanup ltq_dma_alloc_rx().

Andrew

2020-12-21 16:01:44

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] net: lantiq_etop: check the result of request_irq()

On Tue, Dec 22, 2020 at 12:26 AM Andrew Lunn <[email protected]> wrote:
>
> On Mon, Dec 21, 2020 at 02:43:23PM +0900, Masahiro Yamada wrote:
> > The declaration of request_irq() in <linux/interrupt.h> is marked as
> > __must_check.
> >
> > Without the return value check, I see the following warnings:
> >
> > drivers/net/ethernet/lantiq_etop.c: In function 'ltq_etop_hw_init':
> > drivers/net/ethernet/lantiq_etop.c:273:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
> > 273 | request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/net/ethernet/lantiq_etop.c:281:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
> > 281 | request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Reported-by: Miguel Ojeda <[email protected]>
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > ---
> >
> > drivers/net/ethernet/lantiq_etop.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
> > index 2d0c52f7106b..960494f9752b 100644
> > --- a/drivers/net/ethernet/lantiq_etop.c
> > +++ b/drivers/net/ethernet/lantiq_etop.c
> > @@ -264,13 +264,18 @@ ltq_etop_hw_init(struct net_device *dev)
> > for (i = 0; i < MAX_DMA_CHAN; i++) {
> > int irq = LTQ_DMA_CH0_INT + i;
> > struct ltq_etop_chan *ch = &priv->ch[i];
> > + int ret;
> >
> > ch->idx = ch->dma.nr = i;
> > ch->dma.dev = &priv->pdev->dev;
> >
> > if (IS_TX(i)) {
> > ltq_dma_alloc_tx(&ch->dma);
> > - request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > + ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > + if (ret) {
> > + netdev_err(dev, "failed to request irq\n");
> > + return ret;
>
> You need to cleanup what ltq_dma_alloc_tx() did.


Any failure from this function will roll back
in the following paths:

ltq_etop_hw_exit()
-> ltq_etop_free_channel()
-> ltq_dma_free()


So, dma is freed anyway.

One problem I see is,
ltq_etop_hw_exit() frees all DMA channels,
some of which may not have been allocated yet.

If it is a bug, it is an existing bug.


>
> > + }
> > } else if (IS_RX(i)) {
> > ltq_dma_alloc_rx(&ch->dma);
> > for (ch->dma.desc = 0; ch->dma.desc < LTQ_DESC_NUM;
> > @@ -278,7 +283,11 @@ ltq_etop_hw_init(struct net_device *dev)
> > if (ltq_etop_alloc_skb(ch))
> > return -ENOMEM;


This -ENOMEM does not roll back anything here.

As stated above, dma_free_coherent() is called.
The problem is, ltq_etop_hw_exit() rolls back too much.

If your requirement is "this driver is completely wrong. Please rewrite it",
sorry, I cannot (unless I am paid to do so).

I am just following this driver's roll-back model.

Please do not expect more to a person who
volunteers to eliminate build warnings.

Of course, if somebody volunteers to rewrite this driver correctly,
that is appreciated.



> > ch->dma.desc = 0;
> > - request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
> > + ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
> > + if (ret) {
> > + netdev_err(dev, "failed to request irq\n");
> > + return ret;
>
> And here you need to cleanup ltq_dma_alloc_rx().
>
> Andrew



--
Best Regards
Masahiro Yamada

2020-12-21 17:05:19

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] net: lantiq_etop: check the result of request_irq()

On Tue, Dec 22, 2020 at 12:59:08AM +0900, Masahiro Yamada wrote:
> On Tue, Dec 22, 2020 at 12:26 AM Andrew Lunn <[email protected]> wrote:
> >
> > On Mon, Dec 21, 2020 at 02:43:23PM +0900, Masahiro Yamada wrote:
> > > The declaration of request_irq() in <linux/interrupt.h> is marked as
> > > __must_check.
> > >
> > > Without the return value check, I see the following warnings:
> > >
> > > drivers/net/ethernet/lantiq_etop.c: In function 'ltq_etop_hw_init':
> > > drivers/net/ethernet/lantiq_etop.c:273:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
> > > 273 | request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > drivers/net/ethernet/lantiq_etop.c:281:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
> > > 281 | request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > Reported-by: Miguel Ojeda <[email protected]>
> > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > ---
> > >
> > > drivers/net/ethernet/lantiq_etop.c | 13 +++++++++++--
> > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
> > > index 2d0c52f7106b..960494f9752b 100644
> > > --- a/drivers/net/ethernet/lantiq_etop.c
> > > +++ b/drivers/net/ethernet/lantiq_etop.c
> > > @@ -264,13 +264,18 @@ ltq_etop_hw_init(struct net_device *dev)
> > > for (i = 0; i < MAX_DMA_CHAN; i++) {
> > > int irq = LTQ_DMA_CH0_INT + i;
> > > struct ltq_etop_chan *ch = &priv->ch[i];
> > > + int ret;
> > >
> > > ch->idx = ch->dma.nr = i;
> > > ch->dma.dev = &priv->pdev->dev;
> > >
> > > if (IS_TX(i)) {
> > > ltq_dma_alloc_tx(&ch->dma);
> > > - request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > > + ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > > + if (ret) {
> > > + netdev_err(dev, "failed to request irq\n");
> > > + return ret;
> >
> > You need to cleanup what ltq_dma_alloc_tx() did.
>
>
> Any failure from this function will roll back
> in the following paths:
>
> ltq_etop_hw_exit()
> -> ltq_etop_free_channel()
> -> ltq_dma_free()
>
>
> So, dma is freed anyway.

O.K, thanks for the information.

> One problem I see is,
> ltq_etop_hw_exit() frees all DMA channels,
> some of which may not have been allocated yet.
>
> If it is a bug, it is an existing bug.
>
>
> >
> > > + }
> > > } else if (IS_RX(i)) {
> > > ltq_dma_alloc_rx(&ch->dma);
> > > for (ch->dma.desc = 0; ch->dma.desc < LTQ_DESC_NUM;
> > > @@ -278,7 +283,11 @@ ltq_etop_hw_init(struct net_device *dev)
> > > if (ltq_etop_alloc_skb(ch))
> > > return -ENOMEM;
>
>
> This -ENOMEM does not roll back anything here.
>
> As stated above, dma_free_coherent() is called.
> The problem is, ltq_etop_hw_exit() rolls back too much.
>
> If your requirement is "this driver is completely wrong. Please rewrite it",
> sorry, I cannot (unless I am paid to do so).
>
> I am just following this driver's roll-back model.
>
> Please do not expect more to a person who
> volunteers to eliminate build warnings.

There is a balance here. We should not remove a warning unless we
properly fix the warning. Otherwise having the warning is pointless.

So please leave the warning in place, and maybe somebody else will
fully fix it.

Andrew

2020-12-21 18:05:45

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] net: lantiq_etop: check the result of request_irq()

On Mon, Dec 21, 2020 at 6:01 PM Andrew Lunn <[email protected]> wrote:
>
> So please leave the warning in place, and maybe somebody else will
> fully fix it.

For context: the plan is to enable the warning unconditionally
starting with 5.11. After that, the idea is making it an error as soon
as reasonable (e.g. 5.12 if no warnings remain by then).

However, if there is nobody planning to fix a given warning, then I'd
say documenting the problem with a `FIXME` comment (plus a change like
this or simply ignoring the return value) would be the best approach.

Cheers,
Miguel

2020-12-21 18:07:19

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH] net: lantiq_etop: check the result of request_irq()

On Tue, Dec 22, 2020 at 12:59:08AM +0900, Masahiro Yamada wrote:
> On Tue, Dec 22, 2020 at 12:26 AM Andrew Lunn <[email protected]> wrote:
> >
> > On Mon, Dec 21, 2020 at 02:43:23PM +0900, Masahiro Yamada wrote:
> > > The declaration of request_irq() in <linux/interrupt.h> is marked as
> > > __must_check.
> > >
> > > Without the return value check, I see the following warnings:
> > >
> > > drivers/net/ethernet/lantiq_etop.c: In function 'ltq_etop_hw_init':
> > > drivers/net/ethernet/lantiq_etop.c:273:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
> > > 273 | request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > drivers/net/ethernet/lantiq_etop.c:281:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
> > > 281 | request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >
> > > Reported-by: Miguel Ojeda <[email protected]>
> > > Signed-off-by: Masahiro Yamada <[email protected]>
> > > ---
> > >
> > > drivers/net/ethernet/lantiq_etop.c | 13 +++++++++++--
> > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
> > > index 2d0c52f7106b..960494f9752b 100644
> > > --- a/drivers/net/ethernet/lantiq_etop.c
> > > +++ b/drivers/net/ethernet/lantiq_etop.c
> > > @@ -264,13 +264,18 @@ ltq_etop_hw_init(struct net_device *dev)
> > > for (i = 0; i < MAX_DMA_CHAN; i++) {
> > > int irq = LTQ_DMA_CH0_INT + i;
> > > struct ltq_etop_chan *ch = &priv->ch[i];
> > > + int ret;
> > >
> > > ch->idx = ch->dma.nr = i;
> > > ch->dma.dev = &priv->pdev->dev;
> > >
> > > if (IS_TX(i)) {
> > > ltq_dma_alloc_tx(&ch->dma);
> > > - request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > > + ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
> > > + if (ret) {
> > > + netdev_err(dev, "failed to request irq\n");
> > > + return ret;
> >
> > You need to cleanup what ltq_dma_alloc_tx() did.
>
>
> Any failure from this function will roll back
> in the following paths:
>
> ltq_etop_hw_exit()
> -> ltq_etop_free_channel()
> -> ltq_dma_free()
>
>
> So, dma is freed anyway.
>
> One problem I see is,
> ltq_etop_hw_exit() frees all DMA channels,
> some of which may not have been allocated yet.
>
> If it is a bug, it is an existing bug.
>
>
> >
> > > + }
> > > } else if (IS_RX(i)) {
> > > ltq_dma_alloc_rx(&ch->dma);
> > > for (ch->dma.desc = 0; ch->dma.desc < LTQ_DESC_NUM;
> > > @@ -278,7 +283,11 @@ ltq_etop_hw_init(struct net_device *dev)
> > > if (ltq_etop_alloc_skb(ch))
> > > return -ENOMEM;
>
>
> This -ENOMEM does not roll back anything here.
>
> As stated above, dma_free_coherent() is called.
> The problem is, ltq_etop_hw_exit() rolls back too much.
>
> If your requirement is "this driver is completely wrong. Please rewrite it",
> sorry, I cannot (unless I am paid to do so).
>
> I am just following this driver's roll-back model.
>
> Please do not expect more to a person who
> volunteers to eliminate build warnings.
>
> Of course, if somebody volunteers to rewrite this driver correctly,
> that is appreciated.

Hi Hauke

Do you still have this hardware? Do you have time to take a look at
the cleanup code?

Thanks
Andrew

2020-12-21 23:44:11

by Hauke Mehrtens

[permalink] [raw]
Subject: Re: [PATCH] net: lantiq_etop: check the result of request_irq()

On 12/21/20 7:04 PM, Andrew Lunn wrote:
> On Tue, Dec 22, 2020 at 12:59:08AM +0900, Masahiro Yamada wrote:
>> On Tue, Dec 22, 2020 at 12:26 AM Andrew Lunn <[email protected]> wrote:
>>>
>>> On Mon, Dec 21, 2020 at 02:43:23PM +0900, Masahiro Yamada wrote:
>>>> The declaration of request_irq() in <linux/interrupt.h> is marked as
>>>> __must_check.
>>>>
>>>> Without the return value check, I see the following warnings:
>>>>
>>>> drivers/net/ethernet/lantiq_etop.c: In function 'ltq_etop_hw_init':
>>>> drivers/net/ethernet/lantiq_etop.c:273:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
>>>> 273 | request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
>>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>> drivers/net/ethernet/lantiq_etop.c:281:4: warning: ignoring return value of 'request_irq', declared with attribute warn_unused_result [-Wunused-result]
>>>> 281 | request_irq(irq, ltq_etop_dma_irq, 0, "etop_rx", priv);
>>>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>
>>>> Reported-by: Miguel Ojeda <[email protected]>
>>>> Signed-off-by: Masahiro Yamada <[email protected]>
>>>> ---
>>>>
>>>> drivers/net/ethernet/lantiq_etop.c | 13 +++++++++++--
>>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
>>>> index 2d0c52f7106b..960494f9752b 100644
>>>> --- a/drivers/net/ethernet/lantiq_etop.c
>>>> +++ b/drivers/net/ethernet/lantiq_etop.c
>>>> @@ -264,13 +264,18 @@ ltq_etop_hw_init(struct net_device *dev)
>>>> for (i = 0; i < MAX_DMA_CHAN; i++) {
>>>> int irq = LTQ_DMA_CH0_INT + i;
>>>> struct ltq_etop_chan *ch = &priv->ch[i];
>>>> + int ret;
>>>>
>>>> ch->idx = ch->dma.nr = i;
>>>> ch->dma.dev = &priv->pdev->dev;
>>>>
>>>> if (IS_TX(i)) {
>>>> ltq_dma_alloc_tx(&ch->dma);
>>>> - request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
>>>> + ret = request_irq(irq, ltq_etop_dma_irq, 0, "etop_tx", priv);
>>>> + if (ret) {
>>>> + netdev_err(dev, "failed to request irq\n");
>>>> + return ret;
>>>
>>> You need to cleanup what ltq_dma_alloc_tx() did.
>>
>>
>> Any failure from this function will roll back
>> in the following paths:
>>
>> ltq_etop_hw_exit()
>> -> ltq_etop_free_channel()
>> -> ltq_dma_free()
>>
>>
>> So, dma is freed anyway.
>>
>> One problem I see is,
>> ltq_etop_hw_exit() frees all DMA channels,
>> some of which may not have been allocated yet.
>>
>> If it is a bug, it is an existing bug.
>>
>>
>>>
>>>> + }
>>>> } else if (IS_RX(i)) {
>>>> ltq_dma_alloc_rx(&ch->dma);
>>>> for (ch->dma.desc = 0; ch->dma.desc < LTQ_DESC_NUM;
>>>> @@ -278,7 +283,11 @@ ltq_etop_hw_init(struct net_device *dev)
>>>> if (ltq_etop_alloc_skb(ch))
>>>> return -ENOMEM;
>>
>>
>> This -ENOMEM does not roll back anything here.
>>
>> As stated above, dma_free_coherent() is called.
>> The problem is, ltq_etop_hw_exit() rolls back too much.
>>
>> If your requirement is "this driver is completely wrong. Please rewrite it",
>> sorry, I cannot (unless I am paid to do so).
>>
>> I am just following this driver's roll-back model.
>>
>> Please do not expect more to a person who
>> volunteers to eliminate build warnings.
>>
>> Of course, if somebody volunteers to rewrite this driver correctly,
>> that is appreciated.
>
> Hi Hauke
>
> Do you still have this hardware? Do you have time to take a look at
> the cleanup code?
>
> Thanks
> Andrew
>

Hi Andrew,

I have this hardware somewhere at home, but I never made it work.
If I find some time I can have a loom at this problem in the next few weeks.

Hauke