| Message ID | a8c87c6d8dd6d9e57c515036a333ff89fc56bcbf.1665033366.git.christophe.leroy@csgroup.eu |
|---|---|
| State | New |
| Headers | show |
| Series | ata: pata_mpc52xx: Replace NO_IRQ by 0 | expand |
On 10/6/22 14:17, Christophe Leroy wrote: > NO_IRQ is used to check the return of irq_of_parse_and_map(). > > On some architecture NO_IRQ is 0, on other architectures it is -1. > > irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. > > So use 0 instead of using NO_IRQ. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Doesn't this need fixes and cc:stable tags ? > --- > drivers/ata/pata_mpc52xx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c > index 6559b606736d..3ebd6522a1fd 100644 > --- a/drivers/ata/pata_mpc52xx.c > +++ b/drivers/ata/pata_mpc52xx.c > @@ -731,7 +731,7 @@ static int mpc52xx_ata_probe(struct platform_device *op) > udma_mask = ATA_UDMA2 & ((1 << (*prop + 1)) - 1); > > ata_irq = irq_of_parse_and_map(op->dev.of_node, 0); > - if (ata_irq == NO_IRQ) { > + if (!ata_irq) { > dev_err(&op->dev, "error mapping irq\n"); > return -EINVAL; > }
Le 06/10/2022 à 07:45, Damien Le Moal a écrit : > On 10/6/22 14:17, Christophe Leroy wrote: >> NO_IRQ is used to check the return of irq_of_parse_and_map(). >> >> On some architecture NO_IRQ is 0, on other architectures it is -1. >> >> irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. >> >> So use 0 instead of using NO_IRQ. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > Doesn't this need fixes and cc:stable tags ? I don't think so, because the only user of this driver is powerpc and powerpc has NO_IRQ set to 0. I'm sending this because I was to remove NO_IRQ in powerpc in a second step. > >> --- >> drivers/ata/pata_mpc52xx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c >> index 6559b606736d..3ebd6522a1fd 100644 >> --- a/drivers/ata/pata_mpc52xx.c >> +++ b/drivers/ata/pata_mpc52xx.c >> @@ -731,7 +731,7 @@ static int mpc52xx_ata_probe(struct platform_device *op) >> udma_mask = ATA_UDMA2 & ((1 << (*prop + 1)) - 1); >> >> ata_irq = irq_of_parse_and_map(op->dev.of_node, 0); >> - if (ata_irq == NO_IRQ) { >> + if (!ata_irq) { >> dev_err(&op->dev, "error mapping irq\n"); >> return -EINVAL; >> } >
On 10/6/22 14:49, Christophe Leroy wrote: > > > Le 06/10/2022 à 07:45, Damien Le Moal a écrit : >> On 10/6/22 14:17, Christophe Leroy wrote: >>> NO_IRQ is used to check the return of irq_of_parse_and_map(). >>> >>> On some architecture NO_IRQ is 0, on other architectures it is -1. >>> >>> irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. >>> >>> So use 0 instead of using NO_IRQ. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> >> Doesn't this need fixes and cc:stable tags ? > > I don't think so, because the only user of this driver is powerpc and > powerpc has NO_IRQ set to 0. > > I'm sending this because I was to remove NO_IRQ in powerpc in a second step. OK. Got it. So queuing this for 6.2 is OK ? Or do you prefer seeing this as a "fix" in 6.1 ? > >> >>> --- >>> drivers/ata/pata_mpc52xx.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c >>> index 6559b606736d..3ebd6522a1fd 100644 >>> --- a/drivers/ata/pata_mpc52xx.c >>> +++ b/drivers/ata/pata_mpc52xx.c >>> @@ -731,7 +731,7 @@ static int mpc52xx_ata_probe(struct platform_device *op) >>> udma_mask = ATA_UDMA2 & ((1 << (*prop + 1)) - 1); >>> >>> ata_irq = irq_of_parse_and_map(op->dev.of_node, 0); >>> - if (ata_irq == NO_IRQ) { >>> + if (!ata_irq) { >>> dev_err(&op->dev, "error mapping irq\n"); >>> return -EINVAL; >>> }
Le 06/10/2022 à 08:30, Damien Le Moal a écrit : > On 10/6/22 14:49, Christophe Leroy wrote: >> >> >> Le 06/10/2022 à 07:45, Damien Le Moal a écrit : >>> On 10/6/22 14:17, Christophe Leroy wrote: >>>> NO_IRQ is used to check the return of irq_of_parse_and_map(). >>>> >>>> On some architecture NO_IRQ is 0, on other architectures it is -1. >>>> >>>> irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. >>>> >>>> So use 0 instead of using NO_IRQ. >>>> >>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>> >>> Doesn't this need fixes and cc:stable tags ? >> >> I don't think so, because the only user of this driver is powerpc and >> powerpc has NO_IRQ set to 0. >> >> I'm sending this because I was to remove NO_IRQ in powerpc in a second step. > > OK. Got it. So queuing this for 6.2 is OK ? Or do you prefer seeing this > as a "fix" in 6.1 ? Yes 6.2 is fine. Thanks Christophe > >> >>> >>>> --- >>>> drivers/ata/pata_mpc52xx.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c >>>> index 6559b606736d..3ebd6522a1fd 100644 >>>> --- a/drivers/ata/pata_mpc52xx.c >>>> +++ b/drivers/ata/pata_mpc52xx.c >>>> @@ -731,7 +731,7 @@ static int mpc52xx_ata_probe(struct platform_device *op) >>>> udma_mask = ATA_UDMA2 & ((1 << (*prop + 1)) - 1); >>>> >>>> ata_irq = irq_of_parse_and_map(op->dev.of_node, 0); >>>> - if (ata_irq == NO_IRQ) { >>>> + if (!ata_irq) { >>>> dev_err(&op->dev, "error mapping irq\n"); >>>> return -EINVAL; >>>> } >
Hello! Perhaps s/by/with/ in the subject? On 10/6/22 8:17 AM, Christophe Leroy wrote: > NO_IRQ is used to check the return of irq_of_parse_and_map(). > > On some architecture NO_IRQ is 0, on other architectures it is -1. > > irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. > > So use 0 instead of using NO_IRQ. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] MBR, Sergey
On 10/6/22 17:37, Sergey Shtylyov wrote: > Hello! > > Perhaps s/by/with/ in the subject? I can fix that when applying. > > On 10/6/22 8:17 AM, Christophe Leroy wrote: > >> NO_IRQ is used to check the return of irq_of_parse_and_map(). >> >> On some architecture NO_IRQ is 0, on other architectures it is -1. >> >> irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. >> >> So use 0 instead of using NO_IRQ. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> > > [...] > > MBR, Sergey
On 10/6/22 14:17, Christophe Leroy wrote: > NO_IRQ is used to check the return of irq_of_parse_and_map(). > > On some architecture NO_IRQ is 0, on other architectures it is -1. > > irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. > > So use 0 instead of using NO_IRQ. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > drivers/ata/pata_mpc52xx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c > index 6559b606736d..3ebd6522a1fd 100644 > --- a/drivers/ata/pata_mpc52xx.c > +++ b/drivers/ata/pata_mpc52xx.c > @@ -731,7 +731,7 @@ static int mpc52xx_ata_probe(struct platform_device *op) > udma_mask = ATA_UDMA2 & ((1 << (*prop + 1)) - 1); > > ata_irq = irq_of_parse_and_map(op->dev.of_node, 0); > - if (ata_irq == NO_IRQ) { > + if (!ata_irq) { > dev_err(&op->dev, "error mapping irq\n"); > return -EINVAL; > } Applied to for-6.2. Thanks !
diff --git a/drivers/ata/pata_mpc52xx.c b/drivers/ata/pata_mpc52xx.c index 6559b606736d..3ebd6522a1fd 100644 --- a/drivers/ata/pata_mpc52xx.c +++ b/drivers/ata/pata_mpc52xx.c @@ -731,7 +731,7 @@ static int mpc52xx_ata_probe(struct platform_device *op) udma_mask = ATA_UDMA2 & ((1 << (*prop + 1)) - 1); ata_irq = irq_of_parse_and_map(op->dev.of_node, 0); - if (ata_irq == NO_IRQ) { + if (!ata_irq) { dev_err(&op->dev, "error mapping irq\n"); return -EINVAL; }
NO_IRQ is used to check the return of irq_of_parse_and_map(). On some architecture NO_IRQ is 0, on other architectures it is -1. irq_of_parse_and_map() returns 0 on error, independent of NO_IRQ. So use 0 instead of using NO_IRQ. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- drivers/ata/pata_mpc52xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)