diff mbox series

[v7,6/6] spmi: pmic-arb: Add multi bus support

Message ID 20240329-spmi-multi-master-support-v7-6-7b902824246c@linaro.org (mailing list archive)
State New, archived
Headers show
Series spmi: pmic-arb: Add support for multiple buses | expand

Commit Message

Abel Vesa March 29, 2024, 6:54 p.m. UTC
Starting with HW version 7, there are actually two separate buses
(with two separate sets of wires). So add support for the second bus.
The first platform that needs this support for the second bus is the
Qualcomm X1 Elite, so add the compatible for it as well.

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/spmi/spmi-pmic-arb.c | 138 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 120 insertions(+), 18 deletions(-)

Comments

Neil Armstrong April 2, 2024, 8:25 a.m. UTC | #1
Hi Abel,

On 29/03/2024 19:54, Abel Vesa wrote:
> Starting with HW version 7, there are actually two separate buses
> (with two separate sets of wires). So add support for the second bus.
> The first platform that needs this support for the second bus is the
> Qualcomm X1 Elite, so add the compatible for it as well.
> 
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>   drivers/spmi/spmi-pmic-arb.c | 138 +++++++++++++++++++++++++++++++++++++------
>   1 file changed, 120 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 19ff8665f3d9..56f2b3190d82 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -13,6 +13,7 @@
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/of.h>
> +#include <linux/of_address.h>
>   #include <linux/of_irq.h>
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
> @@ -95,6 +96,8 @@ enum pmic_arb_channel {
>   	PMIC_ARB_CHANNEL_OBS,
>   };
>   
> +#define PMIC_ARB_MAX_BUSES		2
> +
>   /* Maximum number of support PMIC peripherals */
>   #define PMIC_ARB_MAX_PERIPHS		512
>   #define PMIC_ARB_MAX_PERIPHS_V7		1024
> @@ -148,6 +151,7 @@ struct spmi_pmic_arb;
>    * @min_apid:		minimum APID (used for bounding IRQ search)
>    * @max_apid:		maximum APID
>    * @irq:		PMIC ARB interrupt.
> + * @id:			unique ID of the bus
>    */
>   struct spmi_pmic_arb_bus {
>   	struct spmi_pmic_arb	*pmic_arb;
> @@ -165,6 +169,7 @@ struct spmi_pmic_arb_bus {
>   	u16			min_apid;
>   	u16			max_apid;
>   	int			irq;
> +	u8			id;
>   };
>   
>   /**
> @@ -179,7 +184,8 @@ struct spmi_pmic_arb_bus {
>    * @ee:			the current Execution Environment
>    * @ver_ops:		version dependent operations.
>    * @max_periphs:	Number of elements in apid_data[]
> - * @bus:		per arbiter bus instance
> + * @buses:		per arbiter buses instances
> + * @buses_available:	number of buses registered
>    */
>   struct spmi_pmic_arb {
>   	void __iomem		*rd_base;
> @@ -191,7 +197,8 @@ struct spmi_pmic_arb {
>   	u8			ee;
>   	const struct pmic_arb_ver_ops *ver_ops;
>   	int			max_periphs;
> -	struct spmi_pmic_arb_bus *bus;
> +	struct spmi_pmic_arb_bus *buses[PMIC_ARB_MAX_BUSES];
> +	int			buses_available;
>   };
>   
>   /**
> @@ -219,7 +226,7 @@ struct spmi_pmic_arb {
>   struct pmic_arb_ver_ops {
>   	const char *ver_str;
>   	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
> -	int (*init_apid)(struct spmi_pmic_arb_bus *bus);
> +	int (*init_apid)(struct spmi_pmic_arb_bus *bus, int index);
>   	int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
>   	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
>   	int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> @@ -308,8 +315,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
>   			}
>   
>   			if (status & PMIC_ARB_STATUS_FAILURE) {
> -				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x)\n",
> -					__func__, sid, addr, status);
> +				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x) reg: 0x%x\n",
> +					__func__, sid, addr, status, offset);
>   				WARN_ON(1);
>   				return -EIO;
>   			}
> @@ -325,8 +332,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
>   		udelay(1);
>   	}
>   
> -	dev_err(&ctrl->dev, "%s: %#x %#x: timeout, status %#x\n",
> -		__func__, sid, addr, status);
> +	dev_err(&ctrl->dev, "%s: %#x %#x %#x: timeout, status %#x\n",
> +		__func__, bus->id, sid, addr, status);
>   	return -ETIMEDOUT;
>   }
>   
> @@ -1005,11 +1012,17 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
>   	return 0;
>   }
>   
> -static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
> +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus, int index)
>   {
>   	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
>   	u32 *mapping_table;
>   
> +	if (index) {
> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> +			index);
> +		return -EINVAL;
> +	}

Shouldn't be here

> +
>   	mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
>   				     sizeof(*mapping_table), GFP_KERNEL);
>   	if (!mapping_table)
> @@ -1252,11 +1265,17 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
>   	return 0x1000 * pmic_arb->ee + 0x8000 * apid;
>   }
>   
> -static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
> +static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus, int index)
>   {
>   	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
>   	int ret;
>   
> +	if (index) {
> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> +			index);
> +		return -EINVAL;
> +	}

Shouldn't be here

> +
>   	bus->base_apid = 0;
>   	bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
>   					   PMIC_ARB_FEATURES_PERIPH_MASK;
> @@ -1328,6 +1347,50 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
>   	return pmic_arb_get_obsrvr_chnls_v2(pdev);
>   }
>   
> +/*
> + * Only v7 supports 2 buses. Each bus will get a different apid count, read
> + * from different registers.
> + */
> +static int pmic_arb_init_apid_v7(struct spmi_pmic_arb_bus *bus, int index)
> +{
> +	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> +	int ret;
> +
> +	if (index == 0) {
> +		bus->base_apid = 0;
> +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> +						   PMIC_ARB_FEATURES_PERIPH_MASK;
> +	} else if (index == 1) {
> +		bus->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> +						  PMIC_ARB_FEATURES_PERIPH_MASK;
> +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
> +						   PMIC_ARB_FEATURES_PERIPH_MASK;
> +	} else {
> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> +			bus->id);
> +		return -EINVAL;
> +	}
> +
> +	if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
> +		dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
> +			bus->base_apid + bus->apid_count);
> +		return -EINVAL;
> +	}
> +
> +	ret = pmic_arb_init_apid_min_max(bus);
> +	if (ret)
> +		return ret;
> +
> +	ret = pmic_arb_read_apid_map_v5(bus);
> +	if (ret) {
> +		dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}

Shouldn't be here

> +
>   /*
>    * v7 offset per ee and per apid for observer channels and per apid for
>    * read/write channels.
> @@ -1580,7 +1643,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
>   static const struct pmic_arb_ver_ops pmic_arb_v7 = {
>   	.ver_str		= "v7",
>   	.get_core_resources	= pmic_arb_get_core_resources_v7,
> -	.init_apid		= pmic_arb_init_apid_v5,
> +	.init_apid		= pmic_arb_init_apid_v7,

Shouldn't be here

>   	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
>   	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
>   	.offset			= pmic_arb_offset_v7,
> @@ -1604,6 +1667,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>   				  struct device_node *node,
>   				  struct spmi_pmic_arb *pmic_arb)
>   {
> +	int bus_index = pmic_arb->buses_available;
>   	struct spmi_pmic_arb_bus *bus;
>   	struct device *dev = &pdev->dev;
>   	struct spmi_controller *ctrl;
> @@ -1622,7 +1686,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>   
>   	bus = spmi_controller_get_drvdata(ctrl);
>   
> -	pmic_arb->bus = bus;
> +	pmic_arb->buses[bus_index] = bus;
>   
>   	bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
>   					 sizeof(*bus->ppid_to_apid),
> @@ -1665,12 +1729,13 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>   	bus->cnfg = cnfg;
>   	bus->irq = irq;
>   	bus->spmic = ctrl;
> +	bus->id = bus_index;
>   
> -	ret = pmic_arb->ver_ops->init_apid(bus);
> +	ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
>   	if (ret)
>   		return ret;
>   
> -	dev_dbg(&pdev->dev, "adding irq domain\n");
> +	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);
> @@ -1683,14 +1748,53 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>   					 pmic_arb_chained_irq, bus);
>   
>   	ctrl->dev.of_node = node;
> +	dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
>   
>   	ret = devm_spmi_controller_add(dev, ctrl);
>   	if (ret)
>   		return ret;
>   
> +	pmic_arb->buses_available++;
> +
>   	return 0;
>   }
>   
> +static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
> +					struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node = dev->of_node;
> +	struct device_node *child;
> +	int ret;
> +
> +	/* legacy mode doesn't provide child node for the bus */
> +	if (of_device_is_compatible(node, "qcom,spmi-pmic-arb"))
> +		return spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
> +
> +	for_each_available_child_of_node(node, child) {
> +		if (of_node_name_eq(child, "spmi")) {
> +			ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
> +{
> +	int i;
> +
> +	for (i = 0; i < PMIC_ARB_MAX_BUSES; i++) {
> +		struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
> +
> +		irq_set_chained_handler_and_data(bus->irq,
> +						 NULL, NULL);
> +		irq_domain_remove(bus->domain);
> +	}
> +}
> +
>   static int spmi_pmic_arb_probe(struct platform_device *pdev)
>   {
>   	struct spmi_pmic_arb *pmic_arb;
> @@ -1761,21 +1865,19 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
>   
>   	pmic_arb->ee = ee;
>   
> -	return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
> +	return spmi_pmic_arb_register_buses(pmic_arb, pdev);
>   }
>   
>   static void spmi_pmic_arb_remove(struct platform_device *pdev)
>   {
>   	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> -	struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
>   
> -	irq_set_chained_handler_and_data(bus->irq,
> -					 NULL, NULL);
> -	irq_domain_remove(bus->domain);
> +	spmi_pmic_arb_deregister_buses(pmic_arb);
>   }
>   
>   static const struct of_device_id spmi_pmic_arb_match_table[] = {
>   	{ .compatible = "qcom,spmi-pmic-arb", },
> +	{ .compatible = "qcom,x1e80100-spmi-pmic-arb", },
>   	{},
>   };
>   MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
> 

With issues fixed, it looks fine.

Thanks,
Neil
Abel Vesa April 2, 2024, 8:50 a.m. UTC | #2
On 24-04-02 10:25:52, Neil Armstrong wrote:
> Hi Abel,
> 
> On 29/03/2024 19:54, Abel Vesa wrote:
> > Starting with HW version 7, there are actually two separate buses
> > (with two separate sets of wires). So add support for the second bus.
> > The first platform that needs this support for the second bus is the
> > Qualcomm X1 Elite, so add the compatible for it as well.
> > 
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> >   drivers/spmi/spmi-pmic-arb.c | 138 +++++++++++++++++++++++++++++++++++++------
> >   1 file changed, 120 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> > index 19ff8665f3d9..56f2b3190d82 100644
> > --- a/drivers/spmi/spmi-pmic-arb.c
> > +++ b/drivers/spmi/spmi-pmic-arb.c
> > @@ -13,6 +13,7 @@
> >   #include <linux/kernel.h>
> >   #include <linux/module.h>
> >   #include <linux/of.h>
> > +#include <linux/of_address.h>
> >   #include <linux/of_irq.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/slab.h>
> > @@ -95,6 +96,8 @@ enum pmic_arb_channel {
> >   	PMIC_ARB_CHANNEL_OBS,
> >   };
> > +#define PMIC_ARB_MAX_BUSES		2
> > +
> >   /* Maximum number of support PMIC peripherals */
> >   #define PMIC_ARB_MAX_PERIPHS		512
> >   #define PMIC_ARB_MAX_PERIPHS_V7		1024
> > @@ -148,6 +151,7 @@ struct spmi_pmic_arb;
> >    * @min_apid:		minimum APID (used for bounding IRQ search)
> >    * @max_apid:		maximum APID
> >    * @irq:		PMIC ARB interrupt.
> > + * @id:			unique ID of the bus
> >    */
> >   struct spmi_pmic_arb_bus {
> >   	struct spmi_pmic_arb	*pmic_arb;
> > @@ -165,6 +169,7 @@ struct spmi_pmic_arb_bus {
> >   	u16			min_apid;
> >   	u16			max_apid;
> >   	int			irq;
> > +	u8			id;
> >   };
> >   /**
> > @@ -179,7 +184,8 @@ struct spmi_pmic_arb_bus {
> >    * @ee:			the current Execution Environment
> >    * @ver_ops:		version dependent operations.
> >    * @max_periphs:	Number of elements in apid_data[]
> > - * @bus:		per arbiter bus instance
> > + * @buses:		per arbiter buses instances
> > + * @buses_available:	number of buses registered
> >    */
> >   struct spmi_pmic_arb {
> >   	void __iomem		*rd_base;
> > @@ -191,7 +197,8 @@ struct spmi_pmic_arb {
> >   	u8			ee;
> >   	const struct pmic_arb_ver_ops *ver_ops;
> >   	int			max_periphs;
> > -	struct spmi_pmic_arb_bus *bus;
> > +	struct spmi_pmic_arb_bus *buses[PMIC_ARB_MAX_BUSES];
> > +	int			buses_available;
> >   };
> >   /**
> > @@ -219,7 +226,7 @@ struct spmi_pmic_arb {
> >   struct pmic_arb_ver_ops {
> >   	const char *ver_str;
> >   	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
> > -	int (*init_apid)(struct spmi_pmic_arb_bus *bus);
> > +	int (*init_apid)(struct spmi_pmic_arb_bus *bus, int index);
> >   	int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
> >   	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
> >   	int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> > @@ -308,8 +315,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> >   			}
> >   			if (status & PMIC_ARB_STATUS_FAILURE) {
> > -				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x)\n",
> > -					__func__, sid, addr, status);
> > +				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x) reg: 0x%x\n",
> > +					__func__, sid, addr, status, offset);
> >   				WARN_ON(1);
> >   				return -EIO;
> >   			}
> > @@ -325,8 +332,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
> >   		udelay(1);
> >   	}
> > -	dev_err(&ctrl->dev, "%s: %#x %#x: timeout, status %#x\n",
> > -		__func__, sid, addr, status);
> > +	dev_err(&ctrl->dev, "%s: %#x %#x %#x: timeout, status %#x\n",
> > +		__func__, bus->id, sid, addr, status);
> >   	return -ETIMEDOUT;
> >   }
> > @@ -1005,11 +1012,17 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
> >   	return 0;
> >   }
> > -static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
> > +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus, int index)
> >   {
> >   	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> >   	u32 *mapping_table;
> > +	if (index) {
> > +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> > +			index);
> > +		return -EINVAL;
> > +	}
> 
> Shouldn't be here
> 

You're right. Since the DT bindings for HW < v7 doesn't allow multi bus
support, this check is not needed. Will drop.

> > +
> >   	mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
> >   				     sizeof(*mapping_table), GFP_KERNEL);
> >   	if (!mapping_table)
> > @@ -1252,11 +1265,17 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
> >   	return 0x1000 * pmic_arb->ee + 0x8000 * apid;
> >   }
> > -static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
> > +static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus, int index)
> >   {
> >   	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> >   	int ret;
> > +	if (index) {
> > +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> > +			index);
> > +		return -EINVAL;
> > +	}
> 
> Shouldn't be here
> 

Ditto.

> > +
> >   	bus->base_apid = 0;
> >   	bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> >   					   PMIC_ARB_FEATURES_PERIPH_MASK;
> > @@ -1328,6 +1347,50 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
> >   	return pmic_arb_get_obsrvr_chnls_v2(pdev);
> >   }
> > +/*
> > + * Only v7 supports 2 buses. Each bus will get a different apid count, read
> > + * from different registers.
> > + */
> > +static int pmic_arb_init_apid_v7(struct spmi_pmic_arb_bus *bus, int index)
> > +{
> > +	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> > +	int ret;
> > +
> > +	if (index == 0) {
> > +		bus->base_apid = 0;
> > +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> > +						   PMIC_ARB_FEATURES_PERIPH_MASK;
> > +	} else if (index == 1) {
> > +		bus->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
> > +						  PMIC_ARB_FEATURES_PERIPH_MASK;
> > +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
> > +						   PMIC_ARB_FEATURES_PERIPH_MASK;
> > +	} else {
> > +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
> > +			bus->id);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
> > +		dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
> > +			bus->base_apid + bus->apid_count);
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = pmic_arb_init_apid_min_max(bus);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = pmic_arb_read_apid_map_v5(bus);
> > +	if (ret) {
> > +		dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
> > +			ret);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> 
> Shouldn't be here
> 

Since the apid base and count are different between buses and since v7
supports 2 buses, we need a v7 specific init_apid. So this one is
needed.

> > +
> >   /*
> >    * v7 offset per ee and per apid for observer channels and per apid for
> >    * read/write channels.
> > @@ -1580,7 +1643,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
> >   static const struct pmic_arb_ver_ops pmic_arb_v7 = {
> >   	.ver_str		= "v7",
> >   	.get_core_resources	= pmic_arb_get_core_resources_v7,
> > -	.init_apid		= pmic_arb_init_apid_v5,
> > +	.init_apid		= pmic_arb_init_apid_v7,
> 
> Shouldn't be here
> 

See above.

> >   	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
> >   	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
> >   	.offset			= pmic_arb_offset_v7,
> > @@ -1604,6 +1667,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> >   				  struct device_node *node,
> >   				  struct spmi_pmic_arb *pmic_arb)
> >   {
> > +	int bus_index = pmic_arb->buses_available;
> >   	struct spmi_pmic_arb_bus *bus;
> >   	struct device *dev = &pdev->dev;
> >   	struct spmi_controller *ctrl;
> > @@ -1622,7 +1686,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> >   	bus = spmi_controller_get_drvdata(ctrl);
> > -	pmic_arb->bus = bus;
> > +	pmic_arb->buses[bus_index] = bus;
> >   	bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
> >   					 sizeof(*bus->ppid_to_apid),
> > @@ -1665,12 +1729,13 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> >   	bus->cnfg = cnfg;
> >   	bus->irq = irq;
> >   	bus->spmic = ctrl;
> > +	bus->id = bus_index;
> > -	ret = pmic_arb->ver_ops->init_apid(bus);
> > +	ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
> >   	if (ret)
> >   		return ret;
> > -	dev_dbg(&pdev->dev, "adding irq domain\n");
> > +	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);
> > @@ -1683,14 +1748,53 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
> >   					 pmic_arb_chained_irq, bus);
> >   	ctrl->dev.of_node = node;
> > +	dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
> >   	ret = devm_spmi_controller_add(dev, ctrl);
> >   	if (ret)
> >   		return ret;
> > +	pmic_arb->buses_available++;
> > +
> >   	return 0;
> >   }
> > +static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
> > +					struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *node = dev->of_node;
> > +	struct device_node *child;
> > +	int ret;
> > +
> > +	/* legacy mode doesn't provide child node for the bus */
> > +	if (of_device_is_compatible(node, "qcom,spmi-pmic-arb"))
> > +		return spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
> > +
> > +	for_each_available_child_of_node(node, child) {
> > +		if (of_node_name_eq(child, "spmi")) {
> > +			ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
> > +			if (ret)
> > +				return ret;
> > +		}
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < PMIC_ARB_MAX_BUSES; i++) {
> > +		struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
> > +
> > +		irq_set_chained_handler_and_data(bus->irq,
> > +						 NULL, NULL);
> > +		irq_domain_remove(bus->domain);
> > +	}
> > +}
> > +
> >   static int spmi_pmic_arb_probe(struct platform_device *pdev)
> >   {
> >   	struct spmi_pmic_arb *pmic_arb;
> > @@ -1761,21 +1865,19 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
> >   	pmic_arb->ee = ee;
> > -	return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
> > +	return spmi_pmic_arb_register_buses(pmic_arb, pdev);
> >   }
> >   static void spmi_pmic_arb_remove(struct platform_device *pdev)
> >   {
> >   	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> > -	struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
> > -	irq_set_chained_handler_and_data(bus->irq,
> > -					 NULL, NULL);
> > -	irq_domain_remove(bus->domain);
> > +	spmi_pmic_arb_deregister_buses(pmic_arb);
> >   }
> >   static const struct of_device_id spmi_pmic_arb_match_table[] = {
> >   	{ .compatible = "qcom,spmi-pmic-arb", },
> > +	{ .compatible = "qcom,x1e80100-spmi-pmic-arb", },
> >   	{},
> >   };
> >   MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
> > 
> 
> With issues fixed, it looks fine.
> 
> Thanks,
> Neil
Neil Armstrong April 2, 2024, 8:55 a.m. UTC | #3
On 02/04/2024 10:50, Abel Vesa wrote:
> On 24-04-02 10:25:52, Neil Armstrong wrote:
>> Hi Abel,
>>
>> On 29/03/2024 19:54, Abel Vesa wrote:
>>> Starting with HW version 7, there are actually two separate buses
>>> (with two separate sets of wires). So add support for the second bus.
>>> The first platform that needs this support for the second bus is the
>>> Qualcomm X1 Elite, so add the compatible for it as well.
>>>
>>> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
>>> ---
>>>    drivers/spmi/spmi-pmic-arb.c | 138 +++++++++++++++++++++++++++++++++++++------
>>>    1 file changed, 120 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
>>> index 19ff8665f3d9..56f2b3190d82 100644
>>> --- a/drivers/spmi/spmi-pmic-arb.c
>>> +++ b/drivers/spmi/spmi-pmic-arb.c
>>> @@ -13,6 +13,7 @@
>>>    #include <linux/kernel.h>
>>>    #include <linux/module.h>
>>>    #include <linux/of.h>
>>> +#include <linux/of_address.h>
>>>    #include <linux/of_irq.h>
>>>    #include <linux/platform_device.h>
>>>    #include <linux/slab.h>
>>> @@ -95,6 +96,8 @@ enum pmic_arb_channel {
>>>    	PMIC_ARB_CHANNEL_OBS,
>>>    };
>>> +#define PMIC_ARB_MAX_BUSES		2
>>> +
>>>    /* Maximum number of support PMIC peripherals */
>>>    #define PMIC_ARB_MAX_PERIPHS		512
>>>    #define PMIC_ARB_MAX_PERIPHS_V7		1024
>>> @@ -148,6 +151,7 @@ struct spmi_pmic_arb;
>>>     * @min_apid:		minimum APID (used for bounding IRQ search)
>>>     * @max_apid:		maximum APID
>>>     * @irq:		PMIC ARB interrupt.
>>> + * @id:			unique ID of the bus
>>>     */
>>>    struct spmi_pmic_arb_bus {
>>>    	struct spmi_pmic_arb	*pmic_arb;
>>> @@ -165,6 +169,7 @@ struct spmi_pmic_arb_bus {
>>>    	u16			min_apid;
>>>    	u16			max_apid;
>>>    	int			irq;
>>> +	u8			id;
>>>    };
>>>    /**
>>> @@ -179,7 +184,8 @@ struct spmi_pmic_arb_bus {
>>>     * @ee:			the current Execution Environment
>>>     * @ver_ops:		version dependent operations.
>>>     * @max_periphs:	Number of elements in apid_data[]
>>> - * @bus:		per arbiter bus instance
>>> + * @buses:		per arbiter buses instances
>>> + * @buses_available:	number of buses registered
>>>     */
>>>    struct spmi_pmic_arb {
>>>    	void __iomem		*rd_base;
>>> @@ -191,7 +197,8 @@ struct spmi_pmic_arb {
>>>    	u8			ee;
>>>    	const struct pmic_arb_ver_ops *ver_ops;
>>>    	int			max_periphs;
>>> -	struct spmi_pmic_arb_bus *bus;
>>> +	struct spmi_pmic_arb_bus *buses[PMIC_ARB_MAX_BUSES];
>>> +	int			buses_available;
>>>    };
>>>    /**
>>> @@ -219,7 +226,7 @@ struct spmi_pmic_arb {
>>>    struct pmic_arb_ver_ops {
>>>    	const char *ver_str;
>>>    	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
>>> -	int (*init_apid)(struct spmi_pmic_arb_bus *bus);
>>> +	int (*init_apid)(struct spmi_pmic_arb_bus *bus, int index);
>>>    	int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
>>>    	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
>>>    	int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
>>> @@ -308,8 +315,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
>>>    			}
>>>    			if (status & PMIC_ARB_STATUS_FAILURE) {
>>> -				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x)\n",
>>> -					__func__, sid, addr, status);
>>> +				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x) reg: 0x%x\n",
>>> +					__func__, sid, addr, status, offset);
>>>    				WARN_ON(1);
>>>    				return -EIO;
>>>    			}
>>> @@ -325,8 +332,8 @@ static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
>>>    		udelay(1);
>>>    	}
>>> -	dev_err(&ctrl->dev, "%s: %#x %#x: timeout, status %#x\n",
>>> -		__func__, sid, addr, status);
>>> +	dev_err(&ctrl->dev, "%s: %#x %#x %#x: timeout, status %#x\n",
>>> +		__func__, bus->id, sid, addr, status);
>>>    	return -ETIMEDOUT;
>>>    }
>>> @@ -1005,11 +1012,17 @@ static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
>>>    	return 0;
>>>    }
>>> -static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
>>> +static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus, int index)
>>>    {
>>>    	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
>>>    	u32 *mapping_table;
>>> +	if (index) {
>>> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
>>> +			index);
>>> +		return -EINVAL;
>>> +	}
>>
>> Shouldn't be here
>>
> 
> You're right. Since the DT bindings for HW < v7 doesn't allow multi bus
> support, this check is not needed. Will drop.
> 
>>> +
>>>    	mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
>>>    				     sizeof(*mapping_table), GFP_KERNEL);
>>>    	if (!mapping_table)
>>> @@ -1252,11 +1265,17 @@ static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
>>>    	return 0x1000 * pmic_arb->ee + 0x8000 * apid;
>>>    }
>>> -static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
>>> +static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus, int index)
>>>    {
>>>    	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
>>>    	int ret;
>>> +	if (index) {
>>> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
>>> +			index);
>>> +		return -EINVAL;
>>> +	}
>>
>> Shouldn't be here
>>
> 
> Ditto.
> 
>>> +
>>>    	bus->base_apid = 0;
>>>    	bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
>>>    					   PMIC_ARB_FEATURES_PERIPH_MASK;
>>> @@ -1328,6 +1347,50 @@ static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
>>>    	return pmic_arb_get_obsrvr_chnls_v2(pdev);
>>>    }
>>> +/*
>>> + * Only v7 supports 2 buses. Each bus will get a different apid count, read
>>> + * from different registers.
>>> + */
>>> +static int pmic_arb_init_apid_v7(struct spmi_pmic_arb_bus *bus, int index)
>>> +{
>>> +	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
>>> +	int ret;
>>> +
>>> +	if (index == 0) {
>>> +		bus->base_apid = 0;
>>> +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
>>> +						   PMIC_ARB_FEATURES_PERIPH_MASK;
>>> +	} else if (index == 1) {
>>> +		bus->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
>>> +						  PMIC_ARB_FEATURES_PERIPH_MASK;
>>> +		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
>>> +						   PMIC_ARB_FEATURES_PERIPH_MASK;
>>> +	} else {
>>> +		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
>>> +			bus->id);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
>>> +		dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
>>> +			bus->base_apid + bus->apid_count);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	ret = pmic_arb_init_apid_min_max(bus);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = pmic_arb_read_apid_map_v5(bus);
>>> +	if (ret) {
>>> +		dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
>>> +			ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>
>> Shouldn't be here
>>
> 
> Since the apid base and count are different between buses and since v7
> supports 2 buses, we need a v7 specific init_apid. So this one is
> needed.


I know, all those were wrongly removed in patch 5, just let them in place.

Neil

> 
>>> +
>>>    /*
>>>     * v7 offset per ee and per apid for observer channels and per apid for
>>>     * read/write channels.
>>> @@ -1580,7 +1643,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
>>>    static const struct pmic_arb_ver_ops pmic_arb_v7 = {
>>>    	.ver_str		= "v7",
>>>    	.get_core_resources	= pmic_arb_get_core_resources_v7,
>>> -	.init_apid		= pmic_arb_init_apid_v5,
>>> +	.init_apid		= pmic_arb_init_apid_v7,
>>
>> Shouldn't be here
>>
> 
> See above.
> 
>>>    	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
>>>    	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
>>>    	.offset			= pmic_arb_offset_v7,
>>> @@ -1604,6 +1667,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>>    				  struct device_node *node,
>>>    				  struct spmi_pmic_arb *pmic_arb)
>>>    {
>>> +	int bus_index = pmic_arb->buses_available;
>>>    	struct spmi_pmic_arb_bus *bus;
>>>    	struct device *dev = &pdev->dev;
>>>    	struct spmi_controller *ctrl;
>>> @@ -1622,7 +1686,7 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>>    	bus = spmi_controller_get_drvdata(ctrl);
>>> -	pmic_arb->bus = bus;
>>> +	pmic_arb->buses[bus_index] = bus;
>>>    	bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
>>>    					 sizeof(*bus->ppid_to_apid),
>>> @@ -1665,12 +1729,13 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>>    	bus->cnfg = cnfg;
>>>    	bus->irq = irq;
>>>    	bus->spmic = ctrl;
>>> +	bus->id = bus_index;
>>> -	ret = pmic_arb->ver_ops->init_apid(bus);
>>> +	ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
>>>    	if (ret)
>>>    		return ret;
>>> -	dev_dbg(&pdev->dev, "adding irq domain\n");
>>> +	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);
>>> @@ -1683,14 +1748,53 @@ static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
>>>    					 pmic_arb_chained_irq, bus);
>>>    	ctrl->dev.of_node = node;
>>> +	dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
>>>    	ret = devm_spmi_controller_add(dev, ctrl);
>>>    	if (ret)
>>>    		return ret;
>>> +	pmic_arb->buses_available++;
>>> +
>>>    	return 0;
>>>    }
>>> +static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
>>> +					struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct device_node *node = dev->of_node;
>>> +	struct device_node *child;
>>> +	int ret;
>>> +
>>> +	/* legacy mode doesn't provide child node for the bus */
>>> +	if (of_device_is_compatible(node, "qcom,spmi-pmic-arb"))
>>> +		return spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
>>> +
>>> +	for_each_available_child_of_node(node, child) {
>>> +		if (of_node_name_eq(child, "spmi")) {
>>> +			ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
>>> +			if (ret)
>>> +				return ret;
>>> +		}
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; i < PMIC_ARB_MAX_BUSES; i++) {
>>> +		struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
>>> +
>>> +		irq_set_chained_handler_and_data(bus->irq,
>>> +						 NULL, NULL);
>>> +		irq_domain_remove(bus->domain);
>>> +	}
>>> +}
>>> +
>>>    static int spmi_pmic_arb_probe(struct platform_device *pdev)
>>>    {
>>>    	struct spmi_pmic_arb *pmic_arb;
>>> @@ -1761,21 +1865,19 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
>>>    	pmic_arb->ee = ee;
>>> -	return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
>>> +	return spmi_pmic_arb_register_buses(pmic_arb, pdev);
>>>    }
>>>    static void spmi_pmic_arb_remove(struct platform_device *pdev)
>>>    {
>>>    	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
>>> -	struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
>>> -	irq_set_chained_handler_and_data(bus->irq,
>>> -					 NULL, NULL);
>>> -	irq_domain_remove(bus->domain);
>>> +	spmi_pmic_arb_deregister_buses(pmic_arb);
>>>    }
>>>    static const struct of_device_id spmi_pmic_arb_match_table[] = {
>>>    	{ .compatible = "qcom,spmi-pmic-arb", },
>>> +	{ .compatible = "qcom,x1e80100-spmi-pmic-arb", },
>>>    	{},
>>>    };
>>>    MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);
>>>
>>
>> With issues fixed, it looks fine.
>>
>> Thanks,
>> Neil
diff mbox series

Patch

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 19ff8665f3d9..56f2b3190d82 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -13,6 +13,7 @@ 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -95,6 +96,8 @@  enum pmic_arb_channel {
 	PMIC_ARB_CHANNEL_OBS,
 };
 
+#define PMIC_ARB_MAX_BUSES		2
+
 /* Maximum number of support PMIC peripherals */
 #define PMIC_ARB_MAX_PERIPHS		512
 #define PMIC_ARB_MAX_PERIPHS_V7		1024
@@ -148,6 +151,7 @@  struct spmi_pmic_arb;
  * @min_apid:		minimum APID (used for bounding IRQ search)
  * @max_apid:		maximum APID
  * @irq:		PMIC ARB interrupt.
+ * @id:			unique ID of the bus
  */
 struct spmi_pmic_arb_bus {
 	struct spmi_pmic_arb	*pmic_arb;
@@ -165,6 +169,7 @@  struct spmi_pmic_arb_bus {
 	u16			min_apid;
 	u16			max_apid;
 	int			irq;
+	u8			id;
 };
 
 /**
@@ -179,7 +184,8 @@  struct spmi_pmic_arb_bus {
  * @ee:			the current Execution Environment
  * @ver_ops:		version dependent operations.
  * @max_periphs:	Number of elements in apid_data[]
- * @bus:		per arbiter bus instance
+ * @buses:		per arbiter buses instances
+ * @buses_available:	number of buses registered
  */
 struct spmi_pmic_arb {
 	void __iomem		*rd_base;
@@ -191,7 +197,8 @@  struct spmi_pmic_arb {
 	u8			ee;
 	const struct pmic_arb_ver_ops *ver_ops;
 	int			max_periphs;
-	struct spmi_pmic_arb_bus *bus;
+	struct spmi_pmic_arb_bus *buses[PMIC_ARB_MAX_BUSES];
+	int			buses_available;
 };
 
 /**
@@ -219,7 +226,7 @@  struct spmi_pmic_arb {
 struct pmic_arb_ver_ops {
 	const char *ver_str;
 	int (*get_core_resources)(struct platform_device *pdev, void __iomem *core);
-	int (*init_apid)(struct spmi_pmic_arb_bus *bus);
+	int (*init_apid)(struct spmi_pmic_arb_bus *bus, int index);
 	int (*ppid_to_apid)(struct spmi_pmic_arb_bus *bus, u16 ppid);
 	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
 	int (*offset)(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
@@ -308,8 +315,8 @@  static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
 			}
 
 			if (status & PMIC_ARB_STATUS_FAILURE) {
-				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x)\n",
-					__func__, sid, addr, status);
+				dev_err(&ctrl->dev, "%s: %#x %#x: transaction failed (%#x) reg: 0x%x\n",
+					__func__, sid, addr, status, offset);
 				WARN_ON(1);
 				return -EIO;
 			}
@@ -325,8 +332,8 @@  static int pmic_arb_wait_for_done(struct spmi_controller *ctrl,
 		udelay(1);
 	}
 
-	dev_err(&ctrl->dev, "%s: %#x %#x: timeout, status %#x\n",
-		__func__, sid, addr, status);
+	dev_err(&ctrl->dev, "%s: %#x %#x %#x: timeout, status %#x\n",
+		__func__, bus->id, sid, addr, status);
 	return -ETIMEDOUT;
 }
 
@@ -1005,11 +1012,17 @@  static int pmic_arb_get_core_resources_v1(struct platform_device *pdev,
 	return 0;
 }
 
-static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus)
+static int pmic_arb_init_apid_v1(struct spmi_pmic_arb_bus *bus, int index)
 {
 	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	u32 *mapping_table;
 
+	if (index) {
+		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
+			index);
+		return -EINVAL;
+	}
+
 	mapping_table = devm_kcalloc(&bus->spmic->dev, pmic_arb->max_periphs,
 				     sizeof(*mapping_table), GFP_KERNEL);
 	if (!mapping_table)
@@ -1252,11 +1265,17 @@  static int pmic_arb_offset_v2(struct spmi_pmic_arb_bus *bus, u8 sid, u16 addr,
 	return 0x1000 * pmic_arb->ee + 0x8000 * apid;
 }
 
-static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus)
+static int pmic_arb_init_apid_v5(struct spmi_pmic_arb_bus *bus, int index)
 {
 	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
 	int ret;
 
+	if (index) {
+		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
+			index);
+		return -EINVAL;
+	}
+
 	bus->base_apid = 0;
 	bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
 					   PMIC_ARB_FEATURES_PERIPH_MASK;
@@ -1328,6 +1347,50 @@  static int pmic_arb_get_core_resources_v7(struct platform_device *pdev,
 	return pmic_arb_get_obsrvr_chnls_v2(pdev);
 }
 
+/*
+ * Only v7 supports 2 buses. Each bus will get a different apid count, read
+ * from different registers.
+ */
+static int pmic_arb_init_apid_v7(struct spmi_pmic_arb_bus *bus, int index)
+{
+	struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
+	int ret;
+
+	if (index == 0) {
+		bus->base_apid = 0;
+		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
+						   PMIC_ARB_FEATURES_PERIPH_MASK;
+	} else if (index == 1) {
+		bus->base_apid = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES) &
+						  PMIC_ARB_FEATURES_PERIPH_MASK;
+		bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES1) &
+						   PMIC_ARB_FEATURES_PERIPH_MASK;
+	} else {
+		dev_err(&bus->spmic->dev, "Unsupported buses count %d detected\n",
+			bus->id);
+		return -EINVAL;
+	}
+
+	if (bus->base_apid + bus->apid_count > pmic_arb->max_periphs) {
+		dev_err(&bus->spmic->dev, "Unsupported APID count %d detected\n",
+			bus->base_apid + bus->apid_count);
+		return -EINVAL;
+	}
+
+	ret = pmic_arb_init_apid_min_max(bus);
+	if (ret)
+		return ret;
+
+	ret = pmic_arb_read_apid_map_v5(bus);
+	if (ret) {
+		dev_err(&bus->spmic->dev, "could not read APID->PPID mapping table, rc= %d\n",
+			ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 /*
  * v7 offset per ee and per apid for observer channels and per apid for
  * read/write channels.
@@ -1580,7 +1643,7 @@  static const struct pmic_arb_ver_ops pmic_arb_v5 = {
 static const struct pmic_arb_ver_ops pmic_arb_v7 = {
 	.ver_str		= "v7",
 	.get_core_resources	= pmic_arb_get_core_resources_v7,
-	.init_apid		= pmic_arb_init_apid_v5,
+	.init_apid		= pmic_arb_init_apid_v7,
 	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
 	.offset			= pmic_arb_offset_v7,
@@ -1604,6 +1667,7 @@  static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 				  struct device_node *node,
 				  struct spmi_pmic_arb *pmic_arb)
 {
+	int bus_index = pmic_arb->buses_available;
 	struct spmi_pmic_arb_bus *bus;
 	struct device *dev = &pdev->dev;
 	struct spmi_controller *ctrl;
@@ -1622,7 +1686,7 @@  static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 
 	bus = spmi_controller_get_drvdata(ctrl);
 
-	pmic_arb->bus = bus;
+	pmic_arb->buses[bus_index] = bus;
 
 	bus->ppid_to_apid = devm_kcalloc(dev, PMIC_ARB_MAX_PPID,
 					 sizeof(*bus->ppid_to_apid),
@@ -1665,12 +1729,13 @@  static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 	bus->cnfg = cnfg;
 	bus->irq = irq;
 	bus->spmic = ctrl;
+	bus->id = bus_index;
 
-	ret = pmic_arb->ver_ops->init_apid(bus);
+	ret = pmic_arb->ver_ops->init_apid(bus, bus_index);
 	if (ret)
 		return ret;
 
-	dev_dbg(&pdev->dev, "adding irq domain\n");
+	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);
@@ -1683,14 +1748,53 @@  static int spmi_pmic_arb_bus_init(struct platform_device *pdev,
 					 pmic_arb_chained_irq, bus);
 
 	ctrl->dev.of_node = node;
+	dev_set_name(&ctrl->dev, "spmi-%d", bus_index);
 
 	ret = devm_spmi_controller_add(dev, ctrl);
 	if (ret)
 		return ret;
 
+	pmic_arb->buses_available++;
+
 	return 0;
 }
 
+static int spmi_pmic_arb_register_buses(struct spmi_pmic_arb *pmic_arb,
+					struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *node = dev->of_node;
+	struct device_node *child;
+	int ret;
+
+	/* legacy mode doesn't provide child node for the bus */
+	if (of_device_is_compatible(node, "qcom,spmi-pmic-arb"))
+		return spmi_pmic_arb_bus_init(pdev, node, pmic_arb);
+
+	for_each_available_child_of_node(node, child) {
+		if (of_node_name_eq(child, "spmi")) {
+			ret = spmi_pmic_arb_bus_init(pdev, child, pmic_arb);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return ret;
+}
+
+static void spmi_pmic_arb_deregister_buses(struct spmi_pmic_arb *pmic_arb)
+{
+	int i;
+
+	for (i = 0; i < PMIC_ARB_MAX_BUSES; i++) {
+		struct spmi_pmic_arb_bus *bus = pmic_arb->buses[i];
+
+		irq_set_chained_handler_and_data(bus->irq,
+						 NULL, NULL);
+		irq_domain_remove(bus->domain);
+	}
+}
+
 static int spmi_pmic_arb_probe(struct platform_device *pdev)
 {
 	struct spmi_pmic_arb *pmic_arb;
@@ -1761,21 +1865,19 @@  static int spmi_pmic_arb_probe(struct platform_device *pdev)
 
 	pmic_arb->ee = ee;
 
-	return spmi_pmic_arb_bus_init(pdev, dev->of_node, pmic_arb);
+	return spmi_pmic_arb_register_buses(pmic_arb, pdev);
 }
 
 static void spmi_pmic_arb_remove(struct platform_device *pdev)
 {
 	struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
-	struct spmi_pmic_arb_bus *bus = pmic_arb->bus;
 
-	irq_set_chained_handler_and_data(bus->irq,
-					 NULL, NULL);
-	irq_domain_remove(bus->domain);
+	spmi_pmic_arb_deregister_buses(pmic_arb);
 }
 
 static const struct of_device_id spmi_pmic_arb_match_table[] = {
 	{ .compatible = "qcom,spmi-pmic-arb", },
+	{ .compatible = "qcom,x1e80100-spmi-pmic-arb", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, spmi_pmic_arb_match_table);