[PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree

Konrad Dybcio posted 1 patch 1 year, 8 months ago
There is a newer version of this series
drivers/spmi/spmi-pmic-arb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree
Posted by Konrad Dybcio 1 year, 8 months ago
Currently, irqchips for all of the subnodes (which represent a given
bus master) point to the parent wrapper node. This is no bueno, as
no interrupts arrive, ever (because nothing references that node).

Fix that by passing a reference to the respective master's of_node.

Worth noting, this is a NOP for devices with only a single master
described.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
 drivers/spmi/spmi-pmic-arb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 791cdc160c51..46ea93f78dcd 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -1737,8 +1737,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 
 	dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
 
-	bus->domain = irq_domain_add_tree(dev->of_node,
-					  &pmic_arb_irq_domain_ops, bus);
+	bus->domain = irq_domain_add_tree(node, pmic_arb_irq_domain_ops, bus);
 	if (!bus->domain) {
 		dev_err(&pdev->dev, "unable to create irq_domain\n");
 		return -ENOMEM;

---
base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b
change-id: 20240522-topic-spmi_multi_master_irqfix-f63ebf47b02c

Best regards,
-- 
Konrad Dybcio <konrad.dybcio@linaro.org>
Re: [PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree
Posted by kernel test robot 1 year, 8 months ago
Hi Konrad,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b]

url:    https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/spmi-pmic-arb-Pass-the-correct-of_node-to-irq_domain_add_tree/20240522-194023
base:   8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b
patch link:    https://lore.kernel.org/r/20240522-topic-spmi_multi_master_irqfix-v1-1-f7098b9c8804%40linaro.org
patch subject: [PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree
config: arm64-randconfig-001-20240523 (https://download.01.org/0day-ci/archive/20240523/202405231902.xK5xVGdb-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 7aa382fd7257d9bd4f7fc50bb7078a3c26a1628c)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240523/202405231902.xK5xVGdb-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405231902.xK5xVGdb-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/spmi/spmi-pmic-arb.c:1740:42: error: passing 'const struct irq_domain_ops' to parameter of incompatible type 'const struct irq_domain_ops *'; take the address with &
    1740 |         bus->domain = irq_domain_add_tree(node, pmic_arb_irq_domain_ops, bus);
         |                                                 ^~~~~~~~~~~~~~~~~~~~~~~
         |                                                 &
   include/linux/irqdomain.h:369:36: note: passing argument to parameter 'ops' here
     369 |                                          const struct irq_domain_ops *ops,
         |                                                                       ^
   1 error generated.


vim +1740 drivers/spmi/spmi-pmic-arb.c

  1663	
  1664	static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
  1665					  struct device_node *node,
  1666					  struct spmi_pmic_arb *pmic_arb)
  1667	{
  1668		int bus_index = pmic_arb->buses_available;
  1669		struct spmi_pmic_arb_bus *bus;
  1670		struct device *dev = &pdev->dev;
  1671		struct spmi_controller *ctrl;
  1672		void __iomem *intr;
  1673		void __iomem *cnfg;
  1674		int index, ret;
  1675		int irq;
  1676	
  1677		ctrl = devm_spmi_controller_alloc(dev, sizeof(*bus));
  1678		if (IS_ERR(ctrl))
  1679			return PTR_ERR(ctrl);
  1680	
  1681		ctrl->cmd = pmic_arb_cmd;
  1682		ctrl->read_cmd = pmic_arb_read_cmd;
  1683		ctrl->write_cmd = pmic_arb_write_cmd;
  1684	
  1685		bus = spmi_controller_get_drvdata(ctrl);
  1686	
  1687		pmic_arb->buses[bus_index] = bus;
  1688	
  1689		raw_spin_lock_init(&bus->lock);
  1690	
  1691		bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
  1692						 sizeof(*bus->ppid_to_apid),
  1693						 GFP_KERNEL);
  1694		if (!bus->ppid_to_apid)
  1695			return -ENOMEM;
  1696	
  1697		bus->apid_data = devm_kcalloc(dev, pmic_arb->max_periphs,
  1698					      sizeof(*bus->apid_data),
  1699					      GFP_KERNEL);
  1700		if (!bus->apid_data)
  1701			return -ENOMEM;
  1702	
  1703		index = of_property_match_string(node, "reg-names", "cnfg");
  1704		if (index < 0) {
  1705			dev_err(dev, "cnfg reg region missing");
  1706			return -EINVAL;
  1707		}
  1708	
  1709		cnfg = devm_of_iomap(dev, node, index, NULL);
  1710		if (IS_ERR(cnfg))
  1711			return PTR_ERR(cnfg);
  1712	
  1713		index = of_property_match_string(node, "reg-names", "intr");
  1714		if (index < 0) {
  1715			dev_err(dev, "intr reg region missing");
  1716			return -EINVAL;
  1717		}
  1718	
  1719		intr = devm_of_iomap(dev, node, index, NULL);
  1720		if (IS_ERR(intr))
  1721			return PTR_ERR(intr);
  1722	
  1723		irq = of_irq_get_byname(node, "periph_irq");
  1724		if (irq <= 0)
  1725			return irq ?: -ENXIO;
  1726	
  1727		bus->pmic_arb = pmic_arb;
  1728		bus->intr = intr;
  1729		bus->cnfg = cnfg;
  1730		bus->irq = irq;
  1731		bus->spmic = ctrl;
  1732		bus->id = bus_index;
  1733	
  1734		ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
  1735		if (ret)
  1736			return ret;
  1737	
  1738		dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
  1739	
> 1740		bus->domain = irq_domain_add_tree(node, pmic_arb_irq_domain_ops, bus);
  1741		if (!bus->domain) {
  1742			dev_err(&pdev->dev, "unable to create irq_domain\n");
  1743			return -ENOMEM;
  1744		}
  1745	
  1746		irq_set_chained_handler_and_data(bus->irq,
  1747						 pmic_arb_chained_irq, bus);
  1748	
  1749		ctrl->dev.of_node = node;
  1750		dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
  1751	
  1752		ret = devm_spmi_controller_add(dev, ctrl);
  1753		if (ret)
  1754			return ret;
  1755	
  1756		pmic_arb->buses_available++;
  1757	
  1758		return 0;
  1759	}
  1760	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree
Posted by kernel test robot 1 year, 8 months ago
Hi Konrad,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b]

url:    https://github.com/intel-lab-lkp/linux/commits/Konrad-Dybcio/spmi-pmic-arb-Pass-the-correct-of_node-to-irq_domain_add_tree/20240522-194023
base:   8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b
patch link:    https://lore.kernel.org/r/20240522-topic-spmi_multi_master_irqfix-v1-1-f7098b9c8804%40linaro.org
patch subject: [PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree
config: arm-randconfig-002-20240523 (https://download.01.org/0day-ci/archive/20240523/202405231834.Xi3hn3rT-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240523/202405231834.Xi3hn3rT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202405231834.Xi3hn3rT-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/spmi/spmi-pmic-arb.c: In function 'spmi_pmic_arb_bus_init':
>> drivers/spmi/spmi-pmic-arb.c:1740:49: error: incompatible type for argument 2 of 'irq_domain_add_tree'
    1740 |         bus->domain = irq_domain_add_tree(node, pmic_arb_irq_domain_ops, bus);
         |                                                 ^~~~~~~~~~~~~~~~~~~~~~~
         |                                                 |
         |                                                 struct irq_domain_ops
   In file included from drivers/spmi/spmi-pmic-arb.c:11:
   include/linux/irqdomain.h:369:71: note: expected 'const struct irq_domain_ops *' but argument is of type 'struct irq_domain_ops'
     369 |                                          const struct irq_domain_ops *ops,
         |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~


vim +/irq_domain_add_tree +1740 drivers/spmi/spmi-pmic-arb.c

  1663	
  1664	static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
  1665					  struct device_node *node,
  1666					  struct spmi_pmic_arb *pmic_arb)
  1667	{
  1668		int bus_index = pmic_arb->buses_available;
  1669		struct spmi_pmic_arb_bus *bus;
  1670		struct device *dev = &pdev->dev;
  1671		struct spmi_controller *ctrl;
  1672		void __iomem *intr;
  1673		void __iomem *cnfg;
  1674		int index, ret;
  1675		int irq;
  1676	
  1677		ctrl = devm_spmi_controller_alloc(dev, sizeof(*bus));
  1678		if (IS_ERR(ctrl))
  1679			return PTR_ERR(ctrl);
  1680	
  1681		ctrl->cmd = pmic_arb_cmd;
  1682		ctrl->read_cmd = pmic_arb_read_cmd;
  1683		ctrl->write_cmd = pmic_arb_write_cmd;
  1684	
  1685		bus = spmi_controller_get_drvdata(ctrl);
  1686	
  1687		pmic_arb->buses[bus_index] = bus;
  1688	
  1689		raw_spin_lock_init(&bus->lock);
  1690	
  1691		bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
  1692						 sizeof(*bus->ppid_to_apid),
  1693						 GFP_KERNEL);
  1694		if (!bus->ppid_to_apid)
  1695			return -ENOMEM;
  1696	
  1697		bus->apid_data = devm_kcalloc(dev, pmic_arb->max_periphs,
  1698					      sizeof(*bus->apid_data),
  1699					      GFP_KERNEL);
  1700		if (!bus->apid_data)
  1701			return -ENOMEM;
  1702	
  1703		index = of_property_match_string(node, "reg-names", "cnfg");
  1704		if (index < 0) {
  1705			dev_err(dev, "cnfg reg region missing");
  1706			return -EINVAL;
  1707		}
  1708	
  1709		cnfg = devm_of_iomap(dev, node, index, NULL);
  1710		if (IS_ERR(cnfg))
  1711			return PTR_ERR(cnfg);
  1712	
  1713		index = of_property_match_string(node, "reg-names", "intr");
  1714		if (index < 0) {
  1715			dev_err(dev, "intr reg region missing");
  1716			return -EINVAL;
  1717		}
  1718	
  1719		intr = devm_of_iomap(dev, node, index, NULL);
  1720		if (IS_ERR(intr))
  1721			return PTR_ERR(intr);
  1722	
  1723		irq = of_irq_get_byname(node, "periph_irq");
  1724		if (irq <= 0)
  1725			return irq ?: -ENXIO;
  1726	
  1727		bus->pmic_arb = pmic_arb;
  1728		bus->intr = intr;
  1729		bus->cnfg = cnfg;
  1730		bus->irq = irq;
  1731		bus->spmic = ctrl;
  1732		bus->id = bus_index;
  1733	
  1734		ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
  1735		if (ret)
  1736			return ret;
  1737	
  1738		dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
  1739	
> 1740		bus->domain = irq_domain_add_tree(node, pmic_arb_irq_domain_ops, bus);
  1741		if (!bus->domain) {
  1742			dev_err(&pdev->dev, "unable to create irq_domain\n");
  1743			return -ENOMEM;
  1744		}
  1745	
  1746		irq_set_chained_handler_and_data(bus->irq,
  1747						 pmic_arb_chained_irq, bus);
  1748	
  1749		ctrl->dev.of_node = node;
  1750		dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
  1751	
  1752		ret = devm_spmi_controller_add(dev, ctrl);
  1753		if (ret)
  1754			return ret;
  1755	
  1756		pmic_arb->buses_available++;
  1757	
  1758		return 0;
  1759	}
  1760	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree
Posted by Dmitry Baryshkov 1 year, 8 months ago
On Wed, 22 May 2024 at 14:38, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> Currently, irqchips for all of the subnodes (which represent a given
> bus master) point to the parent wrapper node. This is no bueno, as
> no interrupts arrive, ever (because nothing references that node).
>
> Fix that by passing a reference to the respective master's of_node.
>
> Worth noting, this is a NOP for devices with only a single master
> described.
>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
>  drivers/spmi/spmi-pmic-arb.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 791cdc160c51..46ea93f78dcd 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -1737,8 +1737,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>
>         dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
>
> -       bus->domain = irq_domain_add_tree(dev->of_node,
> -                                         &pmic_arb_irq_domain_ops, bus);
> +       bus->domain = irq_domain_add_tree(node, pmic_arb_irq_domain_ops, bus);

Shouldn't it be &pmic_arb_irq_domain_ops ?

>         if (!bus->domain) {
>                 dev_err(&pdev->dev, "unable to create irq_domain\n");
>                 return -ENOMEM;
>
> ---
> base-commit: 8314289a8d50a4e05d8ece1ae0445a3b57bb4d3b
> change-id: 20240522-topic-spmi_multi_master_irqfix-f63ebf47b02c
>
> Best regards,
> --
> Konrad Dybcio <konrad.dybcio@linaro.org>
>


-- 
With best wishes
Dmitry
Re: [PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree
Posted by Konrad Dybcio 1 year, 8 months ago

On 5/22/24 14:06, Dmitry Baryshkov wrote:
> On Wed, 22 May 2024 at 14:38, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> Currently, irqchips for all of the subnodes (which represent a given
>> bus master) point to the parent wrapper node. This is no bueno, as
>> no interrupts arrive, ever (because nothing references that node).
>>
>> Fix that by passing a reference to the respective master's of_node.
>>
>> Worth noting, this is a NOP for devices with only a single master
>> described.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>>   drivers/spmi/spmi-pmic-arb.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
>> index 791cdc160c51..46ea93f78dcd 100644
>> --- a/drivers/spmi/spmi-pmic-arb.c
>> +++ b/drivers/spmi/spmi-pmic-arb.c
>> @@ -1737,8 +1737,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>
>>          dev_dbg(&pdev->dev, "adding irq domain for bus %d\n", bus_index);
>>
>> -       bus->domain = irq_domain_add_tree(dev->of_node,
>> -                                         &pmic_arb_irq_domain_ops, bus);
>> +       bus->domain = irq_domain_add_tree(node, pmic_arb_irq_domain_ops, bus);
> 
> Shouldn't it be &pmic_arb_irq_domain_ops ?

Yes. Between testing and editing the patch to un-break the line I managed
to.. break the patch..

Konrad
Re: [PATCH] spmi: pmic-arb: Pass the correct of_node to irq_domain_add_tree
Posted by Konrad Dybcio 1 year, 8 months ago

On 5/22/24 13:38, Konrad Dybcio wrote:
> Currently, irqchips for all of the subnodes (which represent a given
> bus master) point to the parent wrapper node. This is no bueno, as
> no interrupts arrive, ever (because nothing references that node).
> 
> Fix that by passing a reference to the respective master's of_node.
> 
> Worth noting, this is a NOP for devices with only a single master
> described.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---

Fixes: 979987371739 ("spmi: pmic-arb: Add multi bus support")

Konrad