| Message ID | 20211126011039.32-1-tangbin@cmss.chinamobile.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ptp: ixp46x: Fix error handling in ptp_ixp_probe() | expand |
On Fri, Nov 26, 2021 at 2:10 AM Tang Bin <tangbin@cmss.chinamobile.com> wrote: > In the function ptp_ixp_probe(), when get irq failed > after executing platform_get_irq(), the negative value > returned will not be detected here. So fix error handling > in this place. > > Fixes: 9055a2f591629 ("ixp4xx_eth: make ptp support a platform driver") > Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> OK the intention is right but: > - !ixp_clock.master_irq || !ixp_clock.slave_irq) > + (ixp_clock.master_irq < 0) || (ixp_clock.slave_irq < 0)) Keep disallowing 0. Because that is not a valid IRQ. ... <= 0 ... Yours, Linus Walleij
Hi On 2021/11/26 9:26, Linus Walleij wrote: > On Fri, Nov 26, 2021 at 2:10 AM Tang Bin <tangbin@cmss.chinamobile.com> wrote: > >> In the function ptp_ixp_probe(), when get irq failed >> after executing platform_get_irq(), the negative value >> returned will not be detected here. So fix error handling >> in this place. >> >> Fixes: 9055a2f591629 ("ixp4xx_eth: make ptp support a platform driver") >> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> > OK the intention is right but: > >> - !ixp_clock.master_irq || !ixp_clock.slave_irq) >> + (ixp_clock.master_irq < 0) || (ixp_clock.slave_irq < 0)) > Keep disallowing 0. Because that is not a valid IRQ. > > ... <= 0 ... Please look at the function platform_get_irq() in the file drivers/base/platform.c, the example is : * int irq = platform_get_irq(pdev, 0); * if (irq < 0) * return irq; Thanks Tang Bin
On Fri, Nov 26, 2021 at 2:40 AM tangbin <tangbin@cmss.chinamobile.com> wrote: > On 2021/11/26 9:26, Linus Walleij wrote: > > On Fri, Nov 26, 2021 at 2:10 AM Tang Bin <tangbin@cmss.chinamobile.com> wrote: > > > >> In the function ptp_ixp_probe(), when get irq failed > >> after executing platform_get_irq(), the negative value > >> returned will not be detected here. So fix error handling > >> in this place. > >> > >> Fixes: 9055a2f591629 ("ixp4xx_eth: make ptp support a platform driver") > >> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> > > OK the intention is right but: > > > >> - !ixp_clock.master_irq || !ixp_clock.slave_irq) > >> + (ixp_clock.master_irq < 0) || (ixp_clock.slave_irq < 0)) > > Keep disallowing 0. Because that is not a valid IRQ. > > > > ... <= 0 ... > > Please look at the function platform_get_irq() in the file > drivers/base/platform.c, > > the example is : > > * int irq = platform_get_irq(pdev, 0); > > * if (irq < 0) > > * return irq; In this case, reading the code is a bad idea. IRQ 0 is not valid, and the fact that platform_get_irq() can return 0 does not make it valid. See: https://lwn.net/Articles/470820/ Yours, Linus Walleij
diff --git a/drivers/net/ethernet/xscale/ptp_ixp46x.c b/drivers/net/ethernet/xscale/ptp_ixp46x.c index 39234852e01b..c52e01d89e47 100644 --- a/drivers/net/ethernet/xscale/ptp_ixp46x.c +++ b/drivers/net/ethernet/xscale/ptp_ixp46x.c @@ -272,7 +272,7 @@ static int ptp_ixp_probe(struct platform_device *pdev) ixp_clock.master_irq = platform_get_irq(pdev, 0); ixp_clock.slave_irq = platform_get_irq(pdev, 1); if (IS_ERR(ixp_clock.regs) || - !ixp_clock.master_irq || !ixp_clock.slave_irq) + (ixp_clock.master_irq < 0) || (ixp_clock.slave_irq < 0)) return -ENXIO; ixp_clock.caps = ptp_ixp_caps;
In the function ptp_ixp_probe(), when get irq failed after executing platform_get_irq(), the negative value returned will not be detected here. So fix error handling in this place. Fixes: 9055a2f591629 ("ixp4xx_eth: make ptp support a platform driver") Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com> --- drivers/net/ethernet/xscale/ptp_ixp46x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)