| Message ID | 20220309072834.2081944-1-chi.minghao@zte.com.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [V3] ata: pata_pxa: Use platform_get_irq() to get the interrupt | expand |
On 3/9/22 16:28, cgel.zte@gmail.com wrote: > From: Minghao Chi <chi.minghao@zte.com.cn> > > platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static > allocation of IRQ resources in DT core code, this causes an issue > when using hierarchical interrupt domains using "interrupts" property > in the node as this bypasses the hierarchical setup and messes up the > irq chaining. > > In preparation for removal of static setup of IRQ resource from DT core > code use platform_get_irq(). > > v1->v2: > - Use more specific in the subject: ata: pata_pxa: > - Switch to returning 'irq' > v2->v3: > - drop the unlikely() Looks good. FYI, the changelog above should be placed under the "---" after your Signed-off-by so that it does not stay as part of the commit message. No need to resend, I will remove this when applying. Sergey, Review OK ? > > Reported-by: Zeal Robot <zealci@zte.com.cn> > Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn> > --- > drivers/ata/pata_pxa.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/ata/pata_pxa.c b/drivers/ata/pata_pxa.c > index 41430f79663c..985f42c4fd70 100644 > --- a/drivers/ata/pata_pxa.c > +++ b/drivers/ata/pata_pxa.c > @@ -164,10 +164,10 @@ static int pxa_ata_probe(struct platform_device *pdev) > struct resource *cmd_res; > struct resource *ctl_res; > struct resource *dma_res; > - struct resource *irq_res; > struct pata_pxa_pdata *pdata = dev_get_platdata(&pdev->dev); > struct dma_slave_config config; > int ret = 0; > + int irq; > > /* > * Resource validation, three resources are needed: > @@ -205,9 +205,9 @@ static int pxa_ata_probe(struct platform_device *pdev) > /* > * IRQ pin > */ > - irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > - if (unlikely(irq_res == NULL)) > - return -EINVAL; > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) > + return irq; > > /* > * Allocate the host > @@ -287,7 +287,7 @@ static int pxa_ata_probe(struct platform_device *pdev) > /* > * Activate the ATA host > */ > - ret = ata_host_activate(host, irq_res->start, ata_sff_interrupt, > + ret = ata_host_activate(host, irq, ata_sff_interrupt, > pdata->irq_flags, &pxa_ata_sht); > if (ret) > dma_release_channel(data->dma_chan);
On 09.03.2022 10:28, cgel.zte@gmail.com wrote: > From: Minghao Chi <chi.minghao@zte.com.cn> > > platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static > allocation of IRQ resources in DT core code, this causes an issue > when using hierarchical interrupt domains using "interrupts" property > in the node as this bypasses the hierarchical setup and messes up the > irq chaining. > > In preparation for removal of static setup of IRQ resource from DT core > code use platform_get_irq(). > > v1->v2: > - Use more specific in the subject: ata: pata_pxa: > - Switch to returning 'irq' > v2->v3: > - drop the unlikely() > > Reported-by: Zeal Robot <zealci@zte.com.cn> > Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] MBR, Sergey
On 09.03.2022 10:31, Damien Le Moal wrote: >> From: Minghao Chi <chi.minghao@zte.com.cn> >> >> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static >> allocation of IRQ resources in DT core code, this causes an issue >> when using hierarchical interrupt domains using "interrupts" property >> in the node as this bypasses the hierarchical setup and messes up the >> irq chaining. >> >> In preparation for removal of static setup of IRQ resource from DT core >> code use platform_get_irq(). >> >> v1->v2: >> - Use more specific in the subject: ata: pata_pxa: >> - Switch to returning 'irq' >> v2->v3: >> - drop the unlikely() > > Looks good. FYI, the changelog above should be placed under the "---" > after your Signed-off-by so that it does not stay as part of the commit > message. No need to resend, I will remove this when applying. > > Sergey, > > Review OK ? Yes. Although, strictly speaking, we still need to check for IRQ0 as well... MBR, Sergey
On 09.03.2022 11:55, Sergei Shtylyov wrote: >>> From: Minghao Chi <chi.minghao@zte.com.cn> >>> >>> platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static >>> allocation of IRQ resources in DT core code, this causes an issue >>> when using hierarchical interrupt domains using "interrupts" property >>> in the node as this bypasses the hierarchical setup and messes up the >>> irq chaining. >>> >>> In preparation for removal of static setup of IRQ resource from DT core >>> code use platform_get_irq(). >>> >>> v1->v2: >>> - Use more specific in the subject: ata: pata_pxa: >>> - Switch to returning 'irq' >>> v2->v3: >>> - drop the unlikely() >> >> Looks good. FYI, the changelog above should be placed under the "---" >> after your Signed-off-by so that it does not stay as part of the commit >> message. No need to resend, I will remove this when applying. >> >> Sergey, >> >> Review OK ? > > Yes. Ugh, replied from the wrong account. I had provided my Reviewed-by: tag already... > Although, strictly speaking, we still need to check for IRQ0 as well... The platform_get_irq() patch preventing IRQ0 hasn't landed still... MBR, Sergey
On 3/9/22 16:28, cgel.zte@gmail.com wrote: > From: Minghao Chi <chi.minghao@zte.com.cn> > > platform_get_resource(pdev, IORESOURCE_IRQ, ..) relies on static > allocation of IRQ resources in DT core code, this causes an issue > when using hierarchical interrupt domains using "interrupts" property > in the node as this bypasses the hierarchical setup and messes up the > irq chaining. > > In preparation for removal of static setup of IRQ resource from DT core > code use platform_get_irq(). > > v1->v2: > - Use more specific in the subject: ata: pata_pxa: > - Switch to returning 'irq' > v2->v3: > - drop the unlikely() > > Reported-by: Zeal Robot <zealci@zte.com.cn> > Signed-off-by: Minghao Chi <chi.minghao@zte.com.cn> Applied to for-5.18 (without commit message fix). Thanks !
diff --git a/drivers/ata/pata_pxa.c b/drivers/ata/pata_pxa.c index 41430f79663c..985f42c4fd70 100644 --- a/drivers/ata/pata_pxa.c +++ b/drivers/ata/pata_pxa.c @@ -164,10 +164,10 @@ static int pxa_ata_probe(struct platform_device *pdev) struct resource *cmd_res; struct resource *ctl_res; struct resource *dma_res; - struct resource *irq_res; struct pata_pxa_pdata *pdata = dev_get_platdata(&pdev->dev); struct dma_slave_config config; int ret = 0; + int irq; /* * Resource validation, three resources are needed: @@ -205,9 +205,9 @@ static int pxa_ata_probe(struct platform_device *pdev) /* * IRQ pin */ - irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0); - if (unlikely(irq_res == NULL)) - return -EINVAL; + irq = platform_get_irq(pdev, 0); + if (irq < 0) + return irq; /* * Allocate the host @@ -287,7 +287,7 @@ static int pxa_ata_probe(struct platform_device *pdev) /* * Activate the ATA host */ - ret = ata_host_activate(host, irq_res->start, ata_sff_interrupt, + ret = ata_host_activate(host, irq, ata_sff_interrupt, pdata->irq_flags, &pxa_ata_sht); if (ret) dma_release_channel(data->dma_chan);