[PATCH] PCI: aspeed: Switch to irq_domain_create_linear()

[PATCH] PCI: aspeed: Switch to irq_domain_create_linear()

[Jiri Slaby (SUSE)]

irq_domain_add_linear() is going away as being obsolete now. Switch to
the preferred irq_domain_create_linear(). That differs in the first
parameter: It takes more generic struct fwnode_handle instead of struct
device_node. Therefore, of_fwnode_handle() is added around the
parameter.

Note some of the users can likely use dev->fwnode directly instead of
indirect of_fwnode_handle(dev->of_node). But dev->fwnode is not
guaranteed to be set for all, so this has to be investigated on case to
case basis (by people who can actually test with the HW).

Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Jacky Chou <jacky_chou@aspeedtech.com>
---
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Joel Stanley <joel@jms.id.au>
Cc: Andrew Jeffery <andrew@codeconstruct.com.au>
Cc: linux-aspeed@lists.ozlabs.org
Cc: linux-pci@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/pci/controller/pcie-aspeed.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pcie-aspeed.c b/drivers/pci/controller/pcie-aspeed.c
index 9aa9e14c6148..74b03a55643f 100644
--- a/drivers/pci/controller/pcie-aspeed.c
+++ b/drivers/pci/controller/pcie-aspeed.c
@@ -725,10 +725,10 @@ static int aspeed_pcie_init_irq_domain(struct aspeed_pcie *pcie)
 {
 	int ret;
 
-	pcie->intx_domain = irq_domain_add_linear(pcie->dev->of_node,
-						  PCI_NUM_INTX,
-						  &aspeed_intx_domain_ops,
-						  pcie);
+	pcie->intx_domain = irq_domain_create_linear(dev_fwnode(pcie->dev),
+						     PCI_NUM_INTX,
+						     &aspeed_intx_domain_ops,
+						     pcie);
 	if (!pcie->intx_domain) {
 		ret = dev_err_probe(pcie->dev, -ENOMEM,
 				    "failed to get INTx IRQ domain\n");
-- 
2.55.0

Re: [PATCH] PCI: aspeed: Switch to irq_domain_create_linear()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims to use `of_fwnode_handle()`, but the code actually uses `dev_fwnode()`.
--

--- Patch [1]: [PATCH] PCI: aspeed: Switch to irq_domain_create_linear() ---
commit aee46b37ad2da511a238368770cbd10115b36171
Author: Jiri Slaby (SUSE) <jirislaby@kernel.org>

PCI: aspeed: Switch to irq_domain_create_linear()

This commit updates the driver to use the modern irq_domain_create_linear()
function instead of the obsolete irq_domain_add_linear(). It adjusts the
first parameter to pass a fwnode_handle rather than a device_node.

> irq_domain_add_linear() is going away as being obsolete now. Switch to
> the preferred irq_domain_create_linear(). That differs in the first
> parameter: It takes more generic struct fwnode_handle instead of struct
> device_node. Therefore, of_fwnode_handle() is added around the
> parameter.
> 
> Note some of the users can likely use dev->fwnode directly instead of
> indirect of_fwnode_handle(dev->of_node). But dev->fwnode is not
> guaranteed to be set for all, so this has to be investigated on case to
> case basis (by people who can actually test with the HW).

> diff --git a/drivers/pci/controller/pcie-aspeed.c b/drivers/pci/controller/pcie-aspeed.c
> index 9aa9e14c61484..74b03a55643fa 100644
> --- a/drivers/pci/controller/pcie-aspeed.c
> +++ b/drivers/pci/controller/pcie-aspeed.c
> @@ -725,10 +725,10 @@ static int aspeed_pcie_init_irq_domain(struct aspeed_pcie *pcie)
>  {
>  	int ret;
>  
> -	pcie->intx_domain = irq_domain_add_linear(pcie->dev->of_node,
> -						  PCI_NUM_INTX,
> -						  &aspeed_intx_domain_ops,
> -						  pcie);
> +	pcie->intx_domain = irq_domain_create_linear(dev_fwnode(pcie->dev),
> +						     PCI_NUM_INTX,
> +						     &aspeed_intx_domain_ops,
> +						     pcie);

[Severity: Medium]
Does this code change contradict the commit message? 

The commit message explicitly mentions adding of_fwnode_handle() around the
parameter and warns against using dev->fwnode directly since it is not
guaranteed to be set.

The patch uses dev_fwnode(pcie->dev) here, which fetches dev->fwnode, rather
than using of_fwnode_handle(pcie->dev->of_node) as described. Should the
implementation be updated to use of_fwnode_handle(), or does the commit
message need to be revised to match this dev_fwnode() approach?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708095814.385480-1-jirislaby@kernel.org?part=1

Re: [PATCH] PCI: aspeed: Switch to irq_domain_create_linear()

[Jiri Slaby]

On 08. 07. 26, 12:04, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The commit message claims to use `of_fwnode_handle()`, but the code actually uses `dev_fwnode()`.
> --
> 
> --- Patch [1]: [PATCH] PCI: aspeed: Switch to irq_domain_create_linear() ---
> commit aee46b37ad2da511a238368770cbd10115b36171
> Author: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> 
> PCI: aspeed: Switch to irq_domain_create_linear()
> 
> This commit updates the driver to use the modern irq_domain_create_linear()
> function instead of the obsolete irq_domain_add_linear(). It adjusts the
> first parameter to pass a fwnode_handle rather than a device_node.
> 
>> irq_domain_add_linear() is going away as being obsolete now. Switch to
>> the preferred irq_domain_create_linear(). That differs in the first
>> parameter: It takes more generic struct fwnode_handle instead of struct
>> device_node. Therefore, of_fwnode_handle() is added around the
>> parameter.
>>
>> Note some of the users can likely use dev->fwnode directly instead of
>> indirect of_fwnode_handle(dev->of_node). But dev->fwnode is not
>> guaranteed to be set for all, so this has to be investigated on case to
>> case basis (by people who can actually test with the HW).
> 
>> diff --git a/drivers/pci/controller/pcie-aspeed.c b/drivers/pci/controller/pcie-aspeed.c
>> index 9aa9e14c61484..74b03a55643fa 100644
>> --- a/drivers/pci/controller/pcie-aspeed.c
>> +++ b/drivers/pci/controller/pcie-aspeed.c
>> @@ -725,10 +725,10 @@ static int aspeed_pcie_init_irq_domain(struct aspeed_pcie *pcie)
>>   {
>>   	int ret;
>>   
>> -	pcie->intx_domain = irq_domain_add_linear(pcie->dev->of_node,
>> -						  PCI_NUM_INTX,
>> -						  &aspeed_intx_domain_ops,
>> -						  pcie);
>> +	pcie->intx_domain = irq_domain_create_linear(dev_fwnode(pcie->dev),
>> +						     PCI_NUM_INTX,
>> +						     &aspeed_intx_domain_ops,
>> +						     pcie);
> 
> [Severity: Medium]
> Does this code change contradict the commit message?
> 
> The commit message explicitly mentions adding of_fwnode_handle() around the
> parameter and warns against using dev->fwnode directly since it is not
> guaranteed to be set.

Sashiko is right, I used a template for more patches...

> The patch uses dev_fwnode(pcie->dev) here, which fetches dev->fwnode, rather
> than using of_fwnode_handle(pcie->dev->of_node) as described. Should the
> implementation be updated to use of_fwnode_handle(), or does the commit
> message need to be revised to match this dev_fwnode() approach?

In this very case, dev_fwnode(pcie->dev) is used in 
aspeed_pcie_msi_init(), called few lines below, so this is all right. 
The commit log is obsolete in this case though. Do you want me to resubmit?

thanks,
-- 
js
suse labs