[PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Rosen Penev]

Replace irq_of_parse_and_map() with platform_get_irq() in both
sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
way to obtain IRQs for platform devices and provides better error
reporting.  Remove the now-unnecessary #include <linux/of_irq.h>.

irq_of_parse_and_map() requires irq_dispose_mapping(), which is missing.

Assisted-by: opencode:big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/ata/sata_dwc_460ex.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 4fc22ce4bd9a..35aa7f9acdf7 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -19,7 +19,6 @@
 #include <linux/device.h>
 #include <linux/dmaengine.h>
 #include <linux/of.h>
-#include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
 #include <linux/libata.h>
@@ -226,7 +225,6 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
 				 struct sata_dwc_device *hsdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *np = dev->of_node;
 
 	hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
 	if (!hsdev->dma)
@@ -236,11 +234,9 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
 	hsdev->dma->id = pdev->id;
 
 	/* Get SATA DMA interrupt number */
-	hsdev->dma->irq = irq_of_parse_and_map(np, 1);
-	if (!hsdev->dma->irq) {
-		dev_err(dev, "no SATA DMA irq\n");
-		return -ENODEV;
-	}
+	hsdev->dma->irq = platform_get_irq(pdev, 1);
+	if (hsdev->dma->irq < 0)
+		return hsdev->dma->irq;
 
 	/* Get physical SATA DMA register base address */
 	hsdev->dma->regs = devm_platform_ioremap_resource(pdev, 1);
@@ -1173,11 +1169,9 @@ static int sata_dwc_probe(struct platform_device *ofdev)
 	sata_dwc_enable_interrupts(hsdev);
 
 	/* Get SATA interrupt number */
-	irq = irq_of_parse_and_map(np, 0);
-	if (!irq) {
-		dev_err(dev, "no SATA DMA irq\n");
-		return -ENODEV;
-	}
+	irq = platform_get_irq(ofdev, 0);
+	if (irq < 0)
+		return irq;
 
 #ifdef CONFIG_SATA_DWC_OLD_DMA
 	if (!of_property_present(np, "dmas")) {
-- 
2.54.0

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Niklas Cassel]

On Sun, Jun 28, 2026 at 04:03:10PM -0700, Rosen Penev wrote:
> Replace irq_of_parse_and_map() with platform_get_irq() in both
> sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
> way to obtain IRQs for platform devices and provides better error
> reporting.  Remove the now-unnecessary #include <linux/of_irq.h>.
> 
> irq_of_parse_and_map() requires irq_dispose_mapping(), which is missing.
> 
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---

Reviewed-by: Niklas Cassel <cassel@kernel.org>

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Damien Le Moal]

On 6/29/26 08:03, Rosen Penev wrote:
> Replace irq_of_parse_and_map() with platform_get_irq() in both
> sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
> way to obtain IRQs for platform devices and provides better error
> reporting.  Remove the now-unnecessary #include <linux/of_irq.h>.
> 
> irq_of_parse_and_map() requires irq_dispose_mapping(), which is missing.
> 
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>

Applied to for-7.2-fixes. Thanks!

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Niklas Cassel]

On Sun, Jun 28, 2026 at 04:03:10PM -0700, Rosen Penev wrote:
> Replace irq_of_parse_and_map() with platform_get_irq() in both
> sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
> way to obtain IRQs for platform devices and provides better error
> reporting.  Remove the now-unnecessary #include <linux/of_irq.h>.
> 
> irq_of_parse_and_map() requires irq_dispose_mapping(), which is missing.
> 
> Assisted-by: opencode:big-pickle
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/ata/sata_dwc_460ex.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 4fc22ce4bd9a..35aa7f9acdf7 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
> @@ -19,7 +19,6 @@
>  #include <linux/device.h>
>  #include <linux/dmaengine.h>
>  #include <linux/of.h>
> -#include <linux/of_irq.h>
>  #include <linux/platform_device.h>
>  #include <linux/phy/phy.h>
>  #include <linux/libata.h>
> @@ -226,7 +225,6 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
>  				 struct sata_dwc_device *hsdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct device_node *np = dev->of_node;

While this patch drops 'struct device_node *np = dev->of_node;' from
sata_dwc_dma_init_old() ...

>  
>  	hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
>  	if (!hsdev->dma)
> @@ -236,11 +234,9 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
>  	hsdev->dma->id = pdev->id;
>  
>  	/* Get SATA DMA interrupt number */
> -	hsdev->dma->irq = irq_of_parse_and_map(np, 1);
> -	if (!hsdev->dma->irq) {
> -		dev_err(dev, "no SATA DMA irq\n");
> -		return -ENODEV;
> -	}
> +	hsdev->dma->irq = platform_get_irq(pdev, 1);
> +	if (hsdev->dma->irq < 0)
> +		return hsdev->dma->irq;
>  
>  	/* Get physical SATA DMA register base address */
>  	hsdev->dma->regs = devm_platform_ioremap_resource(pdev, 1);
> @@ -1173,11 +1169,9 @@ static int sata_dwc_probe(struct platform_device *ofdev)

it seems like you forgot to do the same for sata_dwc_probe().

Which results in a new warning:
https://lore.kernel.org/oe-kbuild-all/202606301243.9MrXM4WY-lkp@intel.com/T/#u

Damien, perhaps drop this patch and wait for a v2,
or fixup the patch to remove 'struct device_node *np = dev->of_node;'
also from sata_dwc_probe().


>  	sata_dwc_enable_interrupts(hsdev);
>  
>  	/* Get SATA interrupt number */
> -	irq = irq_of_parse_and_map(np, 0);
> -	if (!irq) {
> -		dev_err(dev, "no SATA DMA irq\n");
> -		return -ENODEV;
> -	}
> +	irq = platform_get_irq(ofdev, 0);
> +	if (irq < 0)
> +		return irq;
>  
>  #ifdef CONFIG_SATA_DWC_OLD_DMA
>  	if (!of_property_present(np, "dmas")) {
> -- 
> 2.54.0
> 

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Damien Le Moal]

On 6/30/26 16:41, Niklas Cassel wrote:
> On Sun, Jun 28, 2026 at 04:03:10PM -0700, Rosen Penev wrote:
>> Replace irq_of_parse_and_map() with platform_get_irq() in both
>> sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
>> way to obtain IRQs for platform devices and provides better error
>> reporting.  Remove the now-unnecessary #include <linux/of_irq.h>.
>>
>> irq_of_parse_and_map() requires irq_dispose_mapping(), which is missing.
>>
>> Assisted-by: opencode:big-pickle
>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> ---
>>  drivers/ata/sata_dwc_460ex.c | 18 ++++++------------
>>  1 file changed, 6 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
>> index 4fc22ce4bd9a..35aa7f9acdf7 100644
>> --- a/drivers/ata/sata_dwc_460ex.c
>> +++ b/drivers/ata/sata_dwc_460ex.c
>> @@ -19,7 +19,6 @@
>>  #include <linux/device.h>
>>  #include <linux/dmaengine.h>
>>  #include <linux/of.h>
>> -#include <linux/of_irq.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/phy/phy.h>
>>  #include <linux/libata.h>
>> @@ -226,7 +225,6 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
>>  				 struct sata_dwc_device *hsdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> -	struct device_node *np = dev->of_node;
> 
> While this patch drops 'struct device_node *np = dev->of_node;' from
> sata_dwc_dma_init_old() ...
> 
>>  
>>  	hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
>>  	if (!hsdev->dma)
>> @@ -236,11 +234,9 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
>>  	hsdev->dma->id = pdev->id;
>>  
>>  	/* Get SATA DMA interrupt number */
>> -	hsdev->dma->irq = irq_of_parse_and_map(np, 1);
>> -	if (!hsdev->dma->irq) {
>> -		dev_err(dev, "no SATA DMA irq\n");
>> -		return -ENODEV;
>> -	}
>> +	hsdev->dma->irq = platform_get_irq(pdev, 1);
>> +	if (hsdev->dma->irq < 0)
>> +		return hsdev->dma->irq;
>>  
>>  	/* Get physical SATA DMA register base address */
>>  	hsdev->dma->regs = devm_platform_ioremap_resource(pdev, 1);
>> @@ -1173,11 +1169,9 @@ static int sata_dwc_probe(struct platform_device *ofdev)
> 
> it seems like you forgot to do the same for sata_dwc_probe().
> 
> Which results in a new warning:
> https://lore.kernel.org/oe-kbuild-all/202606301243.9MrXM4WY-lkp@intel.com/T/#u
> 
> Damien, perhaps drop this patch and wait for a v2,
> or fixup the patch to remove 'struct device_node *np = dev->of_node;'
> also from sata_dwc_probe().
> 
> 
>>  	sata_dwc_enable_interrupts(hsdev);
>>  
>>  	/* Get SATA interrupt number */
>> -	irq = irq_of_parse_and_map(np, 0);
>> -	if (!irq) {
>> -		dev_err(dev, "no SATA DMA irq\n");
>> -		return -ENODEV;
>> -	}
>> +	irq = platform_get_irq(ofdev, 0);
>> +	if (irq < 0)
>> +		return irq;
>>  
>>  #ifdef CONFIG_SATA_DWC_OLD_DMA
>>  	if (!of_property_present(np, "dmas")) {

Hmmm... np is used here though...

I will drop the patch for now and wait for a v2.

>> -- 
>> 2.54.0
>>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Niklas Cassel]

On 30 June 2026 10:21:58 CEST, Damien Le Moal <dlemoal@kernel.org> wrote:
>On 6/30/26 16:41, Niklas Cassel wrote:
>> On Sun, Jun 28, 2026 at 04:03:10PM -0700, Rosen Penev wrote:
>>> Replace irq_of_parse_and_map() with platform_get_irq() in both
>>> sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
>>> way to obtain IRQs for platform devices and provides better error
>>> reporting.  Remove the now-unnecessary #include <linux/of_irq.h>.
>>>
>>> irq_of_parse_and_map() requires irq_dispose_mapping(), which is missing.
>>>
>>> Assisted-by: opencode:big-pickle
>>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>>> ---
>>>  drivers/ata/sata_dwc_460ex.c | 18 ++++++------------
>>>  1 file changed, 6 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
>>> index 4fc22ce4bd9a..35aa7f9acdf7 100644
>>> --- a/drivers/ata/sata_dwc_460ex.c
>>> +++ b/drivers/ata/sata_dwc_460ex.c
>>> @@ -19,7 +19,6 @@
>>>  #include <linux/device.h>
>>>  #include <linux/dmaengine.h>
>>>  #include <linux/of.h>
>>> -#include <linux/of_irq.h>
>>>  #include <linux/platform_device.h>
>>>  #include <linux/phy/phy.h>
>>>  #include <linux/libata.h>
>>> @@ -226,7 +225,6 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
>>>  				 struct sata_dwc_device *hsdev)
>>>  {
>>>  	struct device *dev = &pdev->dev;
>>> -	struct device_node *np = dev->of_node;
>> 
>> While this patch drops 'struct device_node *np = dev->of_node;' from
>> sata_dwc_dma_init_old() ...
>> 
>>>  
>>>  	hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
>>>  	if (!hsdev->dma)
>>> @@ -236,11 +234,9 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
>>>  	hsdev->dma->id = pdev->id;
>>>  
>>>  	/* Get SATA DMA interrupt number */
>>> -	hsdev->dma->irq = irq_of_parse_and_map(np, 1);
>>> -	if (!hsdev->dma->irq) {
>>> -		dev_err(dev, "no SATA DMA irq\n");
>>> -		return -ENODEV;
>>> -	}
>>> +	hsdev->dma->irq = platform_get_irq(pdev, 1);
>>> +	if (hsdev->dma->irq < 0)
>>> +		return hsdev->dma->irq;
>>>  
>>>  	/* Get physical SATA DMA register base address */
>>>  	hsdev->dma->regs = devm_platform_ioremap_resource(pdev, 1);
>>> @@ -1173,11 +1169,9 @@ static int sata_dwc_probe(struct platform_device *ofdev)
>> 
>> it seems like you forgot to do the same for sata_dwc_probe().
>> 
>> Which results in a new warning:
>> https://lore.kernel.org/oe-kbuild-all/202606301243.9MrXM4WY-lkp@intel.com/T/#u
>> 
>> Damien, perhaps drop this patch and wait for a v2,
>> or fixup the patch to remove 'struct device_node *np = dev->of_node;'
>> also from sata_dwc_probe().
>> 
>> 
>>>  	sata_dwc_enable_interrupts(hsdev);
>>>  
>>>  	/* Get SATA interrupt number */
>>> -	irq = irq_of_parse_and_map(np, 0);
>>> -	if (!irq) {
>>> -		dev_err(dev, "no SATA DMA irq\n");
>>> -		return -ENODEV;
>>> -	}
>>> +	irq = platform_get_irq(ofdev, 0);
>>> +	if (irq < 0)
>>> +		return irq;
>>>  
>>>  #ifdef CONFIG_SATA_DWC_OLD_DMA
>>>  	if (!of_property_present(np, "dmas")) {
>
>Hmmm... np is used here though...

Inside ifdef, that is probably why Sashiko did not catch this new warning.

Rosen, I suggest that you use dev->of_node directly within the ifdef, and drop the local np variable.


Kind regards,
Niklas

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Rosen Penev]

On Tue, Jun 30, 2026 at 1:30 AM Niklas Cassel <cassel@kernel.org> wrote:
>
> On 30 June 2026 10:21:58 CEST, Damien Le Moal <dlemoal@kernel.org> wrote:
> >On 6/30/26 16:41, Niklas Cassel wrote:
> >> On Sun, Jun 28, 2026 at 04:03:10PM -0700, Rosen Penev wrote:
> >>> Replace irq_of_parse_and_map() with platform_get_irq() in both
> >>> sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
> >>> way to obtain IRQs for platform devices and provides better error
> >>> reporting.  Remove the now-unnecessary #include <linux/of_irq.h>.
> >>>
> >>> irq_of_parse_and_map() requires irq_dispose_mapping(), which is missing.
> >>>
> >>> Assisted-by: opencode:big-pickle
> >>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> >>> ---
> >>>  drivers/ata/sata_dwc_460ex.c | 18 ++++++------------
> >>>  1 file changed, 6 insertions(+), 12 deletions(-)
> >>>
> >>> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> >>> index 4fc22ce4bd9a..35aa7f9acdf7 100644
> >>> --- a/drivers/ata/sata_dwc_460ex.c
> >>> +++ b/drivers/ata/sata_dwc_460ex.c
> >>> @@ -19,7 +19,6 @@
> >>>  #include <linux/device.h>
> >>>  #include <linux/dmaengine.h>
> >>>  #include <linux/of.h>
> >>> -#include <linux/of_irq.h>
> >>>  #include <linux/platform_device.h>
> >>>  #include <linux/phy/phy.h>
> >>>  #include <linux/libata.h>
> >>> @@ -226,7 +225,6 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
> >>>                              struct sata_dwc_device *hsdev)
> >>>  {
> >>>     struct device *dev = &pdev->dev;
> >>> -   struct device_node *np = dev->of_node;
> >>
> >> While this patch drops 'struct device_node *np = dev->of_node;' from
> >> sata_dwc_dma_init_old() ...
> >>
> >>>
> >>>     hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
> >>>     if (!hsdev->dma)
> >>> @@ -236,11 +234,9 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
> >>>     hsdev->dma->id = pdev->id;
> >>>
> >>>     /* Get SATA DMA interrupt number */
> >>> -   hsdev->dma->irq = irq_of_parse_and_map(np, 1);
> >>> -   if (!hsdev->dma->irq) {
> >>> -           dev_err(dev, "no SATA DMA irq\n");
> >>> -           return -ENODEV;
> >>> -   }
> >>> +   hsdev->dma->irq = platform_get_irq(pdev, 1);
> >>> +   if (hsdev->dma->irq < 0)
> >>> +           return hsdev->dma->irq;
> >>>
> >>>     /* Get physical SATA DMA register base address */
> >>>     hsdev->dma->regs = devm_platform_ioremap_resource(pdev, 1);
> >>> @@ -1173,11 +1169,9 @@ static int sata_dwc_probe(struct platform_device *ofdev)
> >>
> >> it seems like you forgot to do the same for sata_dwc_probe().
> >>
> >> Which results in a new warning:
> >> https://lore.kernel.org/oe-kbuild-all/202606301243.9MrXM4WY-lkp@intel.com/T/#u
> >>
> >> Damien, perhaps drop this patch and wait for a v2,
> >> or fixup the patch to remove 'struct device_node *np = dev->of_node;'
> >> also from sata_dwc_probe().
> >>
> >>
> >>>     sata_dwc_enable_interrupts(hsdev);
> >>>
> >>>     /* Get SATA interrupt number */
> >>> -   irq = irq_of_parse_and_map(np, 0);
> >>> -   if (!irq) {
> >>> -           dev_err(dev, "no SATA DMA irq\n");
> >>> -           return -ENODEV;
> >>> -   }
> >>> +   irq = platform_get_irq(ofdev, 0);
> >>> +   if (irq < 0)
> >>> +           return irq;
> >>>
> >>>  #ifdef CONFIG_SATA_DWC_OLD_DMA
> >>>     if (!of_property_present(np, "dmas")) {
> >
> >Hmmm... np is used here though...
>
> Inside ifdef, that is probably why Sashiko did not catch this new warning.
>
> Rosen, I suggest that you use dev->of_node directly within the ifdef, and drop the local np variable.
device_property_present is probably better. But yeah. I'll whip up a v2.
>
>
> Kind regards,
> Niklas
>

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Rosen Penev]

On Tue, Jun 30, 2026 at 1:33 AM Rosen Penev <rosenp@gmail.com> wrote:
>
> On Tue, Jun 30, 2026 at 1:30 AM Niklas Cassel <cassel@kernel.org> wrote:
> >
> > On 30 June 2026 10:21:58 CEST, Damien Le Moal <dlemoal@kernel.org> wrote:
> > >On 6/30/26 16:41, Niklas Cassel wrote:
> > >> On Sun, Jun 28, 2026 at 04:03:10PM -0700, Rosen Penev wrote:
> > >>> Replace irq_of_parse_and_map() with platform_get_irq() in both
> > >>> sata_dwc_dma_init_old() and sata_dwc_probe(). This is the preferred
> > >>> way to obtain IRQs for platform devices and provides better error
> > >>> reporting.  Remove the now-unnecessary #include <linux/of_irq.h>.
> > >>>
> > >>> irq_of_parse_and_map() requires irq_dispose_mapping(), which is missing.
> > >>>
> > >>> Assisted-by: opencode:big-pickle
> > >>> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > >>> ---
> > >>>  drivers/ata/sata_dwc_460ex.c | 18 ++++++------------
> > >>>  1 file changed, 6 insertions(+), 12 deletions(-)
> > >>>
> > >>> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> > >>> index 4fc22ce4bd9a..35aa7f9acdf7 100644
> > >>> --- a/drivers/ata/sata_dwc_460ex.c
> > >>> +++ b/drivers/ata/sata_dwc_460ex.c
> > >>> @@ -19,7 +19,6 @@
> > >>>  #include <linux/device.h>
> > >>>  #include <linux/dmaengine.h>
> > >>>  #include <linux/of.h>
> > >>> -#include <linux/of_irq.h>
> > >>>  #include <linux/platform_device.h>
> > >>>  #include <linux/phy/phy.h>
> > >>>  #include <linux/libata.h>
> > >>> @@ -226,7 +225,6 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
> > >>>                              struct sata_dwc_device *hsdev)
> > >>>  {
> > >>>     struct device *dev = &pdev->dev;
> > >>> -   struct device_node *np = dev->of_node;
> > >>
> > >> While this patch drops 'struct device_node *np = dev->of_node;' from
> > >> sata_dwc_dma_init_old() ...
> > >>
> > >>>
> > >>>     hsdev->dma = devm_kzalloc(dev, sizeof(*hsdev->dma), GFP_KERNEL);
> > >>>     if (!hsdev->dma)
> > >>> @@ -236,11 +234,9 @@ static int sata_dwc_dma_init_old(struct platform_device *pdev,
> > >>>     hsdev->dma->id = pdev->id;
> > >>>
> > >>>     /* Get SATA DMA interrupt number */
> > >>> -   hsdev->dma->irq = irq_of_parse_and_map(np, 1);
> > >>> -   if (!hsdev->dma->irq) {
> > >>> -           dev_err(dev, "no SATA DMA irq\n");
> > >>> -           return -ENODEV;
> > >>> -   }
> > >>> +   hsdev->dma->irq = platform_get_irq(pdev, 1);
> > >>> +   if (hsdev->dma->irq < 0)
> > >>> +           return hsdev->dma->irq;
> > >>>
> > >>>     /* Get physical SATA DMA register base address */
> > >>>     hsdev->dma->regs = devm_platform_ioremap_resource(pdev, 1);
> > >>> @@ -1173,11 +1169,9 @@ static int sata_dwc_probe(struct platform_device *ofdev)
> > >>
> > >> it seems like you forgot to do the same for sata_dwc_probe().
> > >>
> > >> Which results in a new warning:
> > >> https://lore.kernel.org/oe-kbuild-all/202606301243.9MrXM4WY-lkp@intel.com/T/#u
> > >>
> > >> Damien, perhaps drop this patch and wait for a v2,
> > >> or fixup the patch to remove 'struct device_node *np = dev->of_node;'
> > >> also from sata_dwc_probe().
> > >>
> > >>
> > >>>     sata_dwc_enable_interrupts(hsdev);
> > >>>
> > >>>     /* Get SATA interrupt number */
> > >>> -   irq = irq_of_parse_and_map(np, 0);
> > >>> -   if (!irq) {
> > >>> -           dev_err(dev, "no SATA DMA irq\n");
> > >>> -           return -ENODEV;
> > >>> -   }
> > >>> +   irq = platform_get_irq(ofdev, 0);
> > >>> +   if (irq < 0)
> > >>> +           return irq;
> > >>>
> > >>>  #ifdef CONFIG_SATA_DWC_OLD_DMA
> > >>>     if (!of_property_present(np, "dmas")) {
> > >
> > >Hmmm... np is used here though...
> >
> > Inside ifdef, that is probably why Sashiko did not catch this new warning.
> >
> > Rosen, I suggest that you use dev->of_node directly within the ifdef, and drop the local np variable.
> device_property_present is probably better. But yeah. I'll whip up a v2.
thinking about this some more. Should use IS_ENABLED to prevent hiding
this code from the compiler.
> >
> >
> > Kind regards,
> > Niklas
> >

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Damien Le Moal]

On 6/30/26 17:52, Rosen Penev wrote:
>>> Inside ifdef, that is probably why Sashiko did not catch this new warning.
>>>
>>> Rosen, I suggest that you use dev->of_node directly within the ifdef, and drop the local np variable.
>> device_property_present is probably better. But yeah. I'll whip up a v2.
> thinking about this some more. Should use IS_ENABLED to prevent hiding
> this code from the compiler.

What code are you talking about ?
What is in the #ifdef CONFIG_SATA_DWC_OLD_DMA ?
CONFIG_SATA_DWC_OLD_DMA is determined automatically by Kconfig. So this is not
hidding anything and why I did not see the warning in my compile tests
(CONFIG_SATA_DWC_OLD_DMA was enabled for me). The report was for Sparc, and
CONFIG_SATA_DWC_OLD_DMA was disabled so the unused variable warning showed up.


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Niklas Cassel]

On Tue, Jun 30, 2026 at 06:00:46PM +0900, Damien Le Moal wrote:
> On 6/30/26 17:52, Rosen Penev wrote:
> >>> Inside ifdef, that is probably why Sashiko did not catch this new warning.
> >>>
> >>> Rosen, I suggest that you use dev->of_node directly within the ifdef, and drop the local np variable.
> >> device_property_present is probably better. But yeah. I'll whip up a v2.
> > thinking about this some more. Should use IS_ENABLED to prevent hiding
> > this code from the compiler.
> 
> What code are you talking about ?
> What is in the #ifdef CONFIG_SATA_DWC_OLD_DMA ?
> CONFIG_SATA_DWC_OLD_DMA is determined automatically by Kconfig. So this is not
> hidding anything and why I did not see the warning in my compile tests
> (CONFIG_SATA_DWC_OLD_DMA was enabled for me). The report was for Sparc, and
> CONFIG_SATA_DWC_OLD_DMA was disabled so the unused variable warning showed up.

I think he means: keep the local np variable,
and replace the #ifdef with
if (IS_ENABLED(CONFIG_SATA_DWC_OLD_DMA) && !of_property_present(np, "dmas")) {


I think it will work for both CONFIG_SATA_DWC_OLD_DMA=y and
CONFIG_SATA_DWC_OLD_DMA=n without giving any warnings.



Kind regards,
Niklas

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Damien Le Moal]

On 6/30/26 18:06, Niklas Cassel wrote:
> On Tue, Jun 30, 2026 at 06:00:46PM +0900, Damien Le Moal wrote:
>> On 6/30/26 17:52, Rosen Penev wrote:
>>>>> Inside ifdef, that is probably why Sashiko did not catch this new warning.
>>>>>
>>>>> Rosen, I suggest that you use dev->of_node directly within the ifdef, and drop the local np variable.
>>>> device_property_present is probably better. But yeah. I'll whip up a v2.
>>> thinking about this some more. Should use IS_ENABLED to prevent hiding
>>> this code from the compiler.
>>
>> What code are you talking about ?
>> What is in the #ifdef CONFIG_SATA_DWC_OLD_DMA ?
>> CONFIG_SATA_DWC_OLD_DMA is determined automatically by Kconfig. So this is not
>> hidding anything and why I did not see the warning in my compile tests
>> (CONFIG_SATA_DWC_OLD_DMA was enabled for me). The report was for Sparc, and
>> CONFIG_SATA_DWC_OLD_DMA was disabled so the unused variable warning showed up.
> 
> I think he means: keep the local np variable,
> and replace the #ifdef with
> if (IS_ENABLED(CONFIG_SATA_DWC_OLD_DMA) && !of_property_present(np, "dmas")) {
> 
> 
> I think it will work for both CONFIG_SATA_DWC_OLD_DMA=y and
> CONFIG_SATA_DWC_OLD_DMA=n without giving any warnings.

I do not think it will. The if() will be compiled out if IS_ENABLED() is false,
but the variable declaration will remain and so will the warning. Unless that
declaration also goes away, IS_ENABLED() will not solve the issue. But if the
variable is removed, I gree is is a lot cleaner !

> 
> 
> 
> Kind regards,
> Niklas


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Niklas Cassel]

On Tue, Jun 30, 2026 at 06:22:52PM +0900, Damien Le Moal wrote:
> On 6/30/26 18:06, Niklas Cassel wrote:
> > On Tue, Jun 30, 2026 at 06:00:46PM +0900, Damien Le Moal wrote:
> >> On 6/30/26 17:52, Rosen Penev wrote:
> >>>>> Inside ifdef, that is probably why Sashiko did not catch this new warning.
> >>>>>
> >>>>> Rosen, I suggest that you use dev->of_node directly within the ifdef, and drop the local np variable.
> >>>> device_property_present is probably better. But yeah. I'll whip up a v2.
> >>> thinking about this some more. Should use IS_ENABLED to prevent hiding
> >>> this code from the compiler.
> >>
> >> What code are you talking about ?
> >> What is in the #ifdef CONFIG_SATA_DWC_OLD_DMA ?
> >> CONFIG_SATA_DWC_OLD_DMA is determined automatically by Kconfig. So this is not
> >> hidding anything and why I did not see the warning in my compile tests
> >> (CONFIG_SATA_DWC_OLD_DMA was enabled for me). The report was for Sparc, and
> >> CONFIG_SATA_DWC_OLD_DMA was disabled so the unused variable warning showed up.
> > 
> > I think he means: keep the local np variable,
> > and replace the #ifdef with
> > if (IS_ENABLED(CONFIG_SATA_DWC_OLD_DMA) && !of_property_present(np, "dmas")) {
> > 
> > 
> > I think it will work for both CONFIG_SATA_DWC_OLD_DMA=y and
> > CONFIG_SATA_DWC_OLD_DMA=n without giving any warnings.
> 
> I do not think it will. The if() will be compiled out if IS_ENABLED() is false,
> but the variable declaration will remain and so will the warning. Unless that
> declaration also goes away, IS_ENABLED() will not solve the issue. But if the
> variable is removed, I gree is is a lot cleaner !

The point of using IS_ENABLED, is that the compiler will see the code
(that is what Rosen meant), and the code that is not used will be
eliminated using dead code elimination from the compiler.

The statement:
if (0 && !of_property_present(np, "dmas")) {

Will not make the compiler consider np as unused.


I even verified it using:

if (IS_ENABLED(CONFIG_SATA_DWC_OLD_DMA) && !of_property_present(np, "dmas")) {

and if did not give my any warning when CONFIG_SATA_DWC_OLD_DMA=n.

However, I had to comment out the call to sata_dwc_dma_init_old()
since sata_dwc_dma_init_old() is only defined inside #ifdef...

Sure, we could create a dummy / stub for that function, but to be honest,
I think my earlier suggestion of just using dev->of_node directly within
the ifdef, and drop the local np variable would be the smallest change.


Kind regards,
Niklas

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Damien Le Moal]

On 6/30/26 18:55, Niklas Cassel wrote:
> Sure, we could create a dummy / stub for that function, but to be honest,
> I think my earlier suggestion of just using dev->of_node directly within
> the ifdef, and drop the local np variable would be the smallest change.

Yes. But I am not against replacing that ifdef with IS_ENABLED().
That's prettier :)

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Niklas Cassel]

On Tue, Jun 30, 2026 at 07:06:55PM +0900, Damien Le Moal wrote:
> On 6/30/26 18:55, Niklas Cassel wrote:
> > Sure, we could create a dummy / stub for that function, but to be honest,
> > I think my earlier suggestion of just using dev->of_node directly within
> > the ifdef, and drop the local np variable would be the smallest change.
> 
> Yes. But I am not against replacing that ifdef with IS_ENABLED().
> That's prettier :)

That is possible if we create a dummy / stub for sata_dwc_dma_init_old()
(since that function is currently defined within #ifdef).


Kind regards,
Niklas

Re: [PATCH] ata: sata_dwc_460ex: use platform_get_irq()

[Rosen Penev]

On Tue, Jun 30, 2026 at 4:16 AM Niklas Cassel <cassel@kernel.org> wrote:
>
> On Tue, Jun 30, 2026 at 07:06:55PM +0900, Damien Le Moal wrote:
> > On 6/30/26 18:55, Niklas Cassel wrote:
> > > Sure, we could create a dummy / stub for that function, but to be honest,
> > > I think my earlier suggestion of just using dev->of_node directly within
> > > the ifdef, and drop the local np variable would be the smallest change.
> >
> > Yes. But I am not against replacing that ifdef with IS_ENABLED().
> > That's prettier :)
>
> That is possible if we create a dummy / stub for sata_dwc_dma_init_old()
> (since that function is currently defined within #ifdef).
Yeah for that reason I'll keep it as it.
>
>
> Kind regards,
> Niklas