[PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Vinod Koul]

Hello,

The is version 3 of support for PMIC v7. I have added a new property
qcom,bus-id for supporting v7 and then add driver changes for v7

This depends on yaml conversion patch:
https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/

Changes since v2:
 - Drop yaml conversion patch
 - Fix author for spmi patch
Changes since v1:
 - Add yaml conversion patch and new binding
 - fix driver bug report by Jonathan

David Collins (1):
  spmi: pmic-arb: Add support for PMIC v7

Vinod Koul (1):
  dt-bindings: spmi: Add qcom,bus-id

 .../bindings/spmi/qcom,spmi-pmic-arb.yaml     |  11 +
 drivers/spmi/spmi-pmic-arb.c                  | 233 ++++++++++++++++--
 2 files changed, 225 insertions(+), 19 deletions(-)

-- 
2.31.1

[PATCH v3 1/2] dt-bindings: spmi: Add qcom,bus-id

[Vinod Koul]

PMIC arbiter version 7 and beyond we need to define if we are using
primary or secondary bus, so add a new property of qcom,bus-id

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 .../devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml  | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
index 55d379c85fd9..cf17301fbb62 100644
--- a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
+++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml
@@ -85,6 +85,14 @@ properties:
     description: >
       which of the PMIC Arb provided channels to use for accesses
 
+  qcom,bus-id:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 1
+    description: >
+      SPMI bus instance. only applicable to PMIC arbiter version 7 and beyond.
+      Supported values, 0 = primary bus, 1 = secondary bus
+
 required:
   - compatible
   - reg-names
@@ -116,5 +124,8 @@ examples:
 
         interrupt-controller;
         #interrupt-cells = <4>;
+
+        qcom,bus-id = <0>;
+
     };
 
-- 
2.31.1

Re: [PATCH v3 1/2] dt-bindings: spmi: Add qcom,bus-id

[Rob Herring]

On Tue, 01 Feb 2022 19:11:07 +0530, Vinod Koul wrote:
> PMIC arbiter version 7 and beyond we need to define if we are using
> primary or secondary bus, so add a new property of qcom,bus-id
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>  .../devicetree/bindings/spmi/qcom,spmi-pmic-arb.yaml  | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v3 2/2] spmi: pmic-arb: Add support for PMIC v7

[Vinod Koul]

From: David Collins <collinsd@codeaurora.org>

PMIC v7 has different offset values and seqeunces, so add support for
this new version of PMIC

Signed-off-by: David Collins <collinsd@codeaurora.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/spmi/spmi-pmic-arb.c | 233 ++++++++++++++++++++++++++++++++---
 1 file changed, 214 insertions(+), 19 deletions(-)

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 2113be40b5a9..f4d54e7785a8 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -22,8 +22,14 @@
 #define PMIC_ARB_VERSION_V2_MIN		0x20010000
 #define PMIC_ARB_VERSION_V3_MIN		0x30000000
 #define PMIC_ARB_VERSION_V5_MIN		0x50000000
+#define PMIC_ARB_VERSION_V7_MIN		0x70000000
 #define PMIC_ARB_INT_EN			0x0004
 
+#define PMIC_ARB_FEATURES		0x0004
+#define PMIC_ARB_FEATURES_PERIPH_MASK	GENMASK(10, 0)
+
+#define PMIC_ARB_FEATURES1		0x0008
+
 /* PMIC Arbiter channel registers offsets */
 #define PMIC_ARB_CMD			0x00
 #define PMIC_ARB_CONFIG			0x04
@@ -48,7 +54,6 @@
 #define INVALID_EE				0xFF
 
 /* Ownership Table */
-#define SPMI_OWNERSHIP_TABLE_REG(N)	(0x0700 + (4 * (N)))
 #define SPMI_OWNERSHIP_PERIPH2OWNER(X)	((X) & 0x7)
 
 /* Channel Status fields */
@@ -91,6 +96,7 @@ enum pmic_arb_channel {
 
 /* Maximum number of support PMIC peripherals */
 #define PMIC_ARB_MAX_PERIPHS		512
+#define PMIC_ARB_MAX_PERIPHS_V7		1024
 #define PMIC_ARB_TIMEOUT_US		100
 #define PMIC_ARB_MAX_TRANS_BYTES	(8)
 
@@ -104,12 +110,12 @@ enum pmic_arb_channel {
 	((((slave_id) & 0xF)   << 28) | \
 	(((periph_id) & 0xFF)  << 20) | \
 	(((irq_id)    & 0x7)   << 16) | \
-	(((apid)      & 0x1FF) << 0))
+	(((apid)      & 0x3FF) << 0))
 
 #define hwirq_to_sid(hwirq)  (((hwirq) >> 28) & 0xF)
 #define hwirq_to_per(hwirq)  (((hwirq) >> 20) & 0xFF)
 #define hwirq_to_irq(hwirq)  (((hwirq) >> 16) & 0x7)
-#define hwirq_to_apid(hwirq) (((hwirq) >> 0)  & 0x1FF)
+#define hwirq_to_apid(hwirq) (((hwirq) >> 0)  & 0x3FF)
 
 struct pmic_arb_ver_ops;
 
@@ -137,6 +143,8 @@ struct apid_data {
  * @spmic:		SPMI controller object
  * @ver_ops:		version dependent operations.
  * @ppid_to_apid	in-memory copy of PPID -> APID mapping table.
+ * @apid_data:		Table of data for all APIDs
+ * @max_periphs:	Number of elements in apid_data[]
  */
 struct spmi_pmic_arb {
 	void __iomem		*rd_base;
@@ -149,8 +157,11 @@ struct spmi_pmic_arb {
 	u8			channel;
 	int			irq;
 	u8			ee;
+	u32			bus_instance;
 	u16			min_apid;
 	u16			max_apid;
+	u16			base_apid;
+	int			apid_count;
 	u32			*mapping_table;
 	DECLARE_BITMAP(mapping_table_valid, PMIC_ARB_MAX_PERIPHS);
 	struct irq_domain	*domain;
@@ -158,7 +169,8 @@ struct spmi_pmic_arb {
 	const struct pmic_arb_ver_ops *ver_ops;
 	u16			*ppid_to_apid;
 	u16			last_apid;
-	struct apid_data	apid_data[PMIC_ARB_MAX_PERIPHS];
+	struct apid_data	*apid_data;
+	int			max_periphs;
 };
 
 /**
@@ -180,6 +192,7 @@ struct spmi_pmic_arb {
  * @irq_clear:		on v1 address of PMIC_ARB_SPMI_PIC_IRQ_CLEARn
  *			on v2 address of SPMI_PIC_IRQ_CLEARn.
  * @apid_map_offset:	offset of PMIC_ARB_REG_CHNLn
+ * @apid_owner:		on v2 and later address of SPMI_PERIPHn_2OWNER_TABLE_REG
  */
 struct pmic_arb_ver_ops {
 	const char *ver_str;
@@ -196,6 +209,7 @@ struct pmic_arb_ver_ops {
 	void __iomem *(*irq_status)(struct spmi_pmic_arb *pmic_arb, u16 n);
 	void __iomem *(*irq_clear)(struct spmi_pmic_arb *pmic_arb, u16 n);
 	u32 (*apid_map_offset)(u16 n);
+	void __iomem *(*apid_owner)(struct spmi_pmic_arb *pmic_arb, u16 n);
 };
 
 static inline void pmic_arb_base_write(struct spmi_pmic_arb *pmic_arb,
@@ -631,6 +645,11 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	int first = pmic_arb->min_apid >> 5;
 	int last = pmic_arb->max_apid >> 5;
+	/*
+	 * acc_offset will be non-zero for the secondary SPMI bus instance on
+	 * v7 controllers.
+	 */
+	int acc_offset = pmic_arb->base_apid >> 5;
 	u8 ee = pmic_arb->ee;
 	u32 status, enable;
 	int i, id, apid;
@@ -638,8 +657,7 @@ static void pmic_arb_chained_irq(struct irq_desc *desc)
 	chained_irq_enter(chip, desc);
 
 	for (i = first; i <= last; ++i) {
-		status = readl_relaxed(
-				ver_ops->owner_acc_status(pmic_arb, ee, i));
+		status = readl_relaxed(ver_ops->owner_acc_status(pmic_arb, ee, i - acc_offset));
 		while (status) {
 			id = ffs(status) - 1;
 			status &= ~BIT(id);
@@ -944,8 +962,8 @@ static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pmic_arb, u16 ppid)
 		if (offset >= pmic_arb->core_size)
 			break;
 
-		regval = readl_relaxed(pmic_arb->cnfg +
-				      SPMI_OWNERSHIP_TABLE_REG(apid));
+		regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(pmic_arb,
+								     apid));
 		apidd->irq_ee = SPMI_OWNERSHIP_PERIPH2OWNER(regval);
 		apidd->write_ee = apidd->irq_ee;
 
@@ -981,20 +999,29 @@ static int pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb *pmic_arb, u16 ppid)
 
 static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
 {
-	struct apid_data *apidd = pmic_arb->apid_data;
+	struct apid_data *apidd;
 	struct apid_data *prev_apidd;
-	u16 i, apid, ppid;
+	u16 i, apid, ppid, apid_max;
 	bool valid, is_irq_ee;
 	u32 regval, offset;
 
 	/*
 	 * In order to allow multiple EEs to write to a single PPID in arbiter
-	 * version 5, there is more than one APID mapped to each PPID.
+	 * version 5 and 7, there is more than one APID mapped to each PPID.
 	 * The owner field for each of these mappings specifies the EE which is
 	 * allowed to write to the APID.  The owner of the last (highest) APID
 	 * for a given PPID will receive interrupts from the PPID.
+	 *
+	 * In arbiter version 7, the APID numbering space is divided between
+	 * the primary bus (0) and secondary bus (1) such that:
+	 * APID = 0 to N-1 are assigned to the primary bus
+	 * APID = N to N+M-1 are assigned to the secondary bus
+	 * where N = number of APIDs supported by the primary bus and
+	 *       M = number of APIDs supported by the secondary bus
 	 */
-	for (i = 0; ; i++, apidd++) {
+	apidd = &pmic_arb->apid_data[pmic_arb->base_apid];
+	apid_max = pmic_arb->base_apid + pmic_arb->apid_count;
+	for (i = pmic_arb->base_apid; i < apid_max; i++, apidd++) {
 		offset = pmic_arb->ver_ops->apid_map_offset(i);
 		if (offset >= pmic_arb->core_size)
 			break;
@@ -1005,8 +1032,8 @@ static int pmic_arb_read_apid_map_v5(struct spmi_pmic_arb *pmic_arb)
 		ppid = (regval >> 8) & PMIC_ARB_PPID_MASK;
 		is_irq_ee = PMIC_ARB_CHAN_IS_IRQ_OWNER(regval);
 
-		regval = readl_relaxed(pmic_arb->cnfg +
-				      SPMI_OWNERSHIP_TABLE_REG(i));
+		regval = readl_relaxed(pmic_arb->ver_ops->apid_owner(pmic_arb,
+								     i));
 		apidd->write_ee = SPMI_OWNERSHIP_PERIPH2OWNER(regval);
 
 		apidd->irq_ee = is_irq_ee ? apidd->write_ee : INVALID_EE;
@@ -1100,6 +1127,40 @@ static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
 	return offset;
 }
 
+/*
+ * v7 offset per ee and per apid for observer channels and per apid for
+ * read/write channels.
+ */
+static int pmic_arb_offset_v7(struct spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
+			   enum pmic_arb_channel ch_type)
+{
+	u16 apid;
+	int rc;
+	u32 offset = 0;
+	u16 ppid = (sid << 8) | (addr >> 8);
+
+	rc = pmic_arb->ver_ops->ppid_to_apid(pmic_arb, ppid);
+	if (rc < 0)
+		return rc;
+
+	apid = rc;
+	switch (ch_type) {
+	case PMIC_ARB_CHANNEL_OBS:
+		offset = 0x8000 * pmic_arb->ee + 0x20 * apid;
+		break;
+	case PMIC_ARB_CHANNEL_RW:
+		if (pmic_arb->apid_data[apid].write_ee != pmic_arb->ee) {
+			dev_err(&pmic_arb->spmic->dev, "disallowed SPMI write to sid=%u, addr=0x%04X\n",
+				sid, addr);
+			return -EPERM;
+		}
+		offset = 0x1000 * apid;
+		break;
+	}
+
+	return offset;
+}
+
 static u32 pmic_arb_fmt_cmd_v1(u8 opc, u8 sid, u16 addr, u8 bc)
 {
 	return (opc << 27) | ((sid & 0xf) << 20) | (addr << 4) | (bc & 0x7);
@@ -1134,6 +1195,12 @@ pmic_arb_owner_acc_status_v5(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
 	return pmic_arb->intr + 0x10000 * m + 0x4 * n;
 }
 
+static void __iomem *
+pmic_arb_owner_acc_status_v7(struct spmi_pmic_arb *pmic_arb, u8 m, u16 n)
+{
+	return pmic_arb->intr + 0x1000 * m + 0x4 * n;
+}
+
 static void __iomem *
 pmic_arb_acc_enable_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
 {
@@ -1152,6 +1219,12 @@ pmic_arb_acc_enable_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
 	return pmic_arb->wr_base + 0x100 + 0x10000 * n;
 }
 
+static void __iomem *
+pmic_arb_acc_enable_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
+{
+	return pmic_arb->wr_base + 0x100 + 0x1000 * n;
+}
+
 static void __iomem *
 pmic_arb_irq_status_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
 {
@@ -1170,6 +1243,12 @@ pmic_arb_irq_status_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
 	return pmic_arb->wr_base + 0x104 + 0x10000 * n;
 }
 
+static void __iomem *
+pmic_arb_irq_status_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
+{
+	return pmic_arb->wr_base + 0x104 + 0x1000 * n;
+}
+
 static void __iomem *
 pmic_arb_irq_clear_v1(struct spmi_pmic_arb *pmic_arb, u16 n)
 {
@@ -1188,6 +1267,12 @@ pmic_arb_irq_clear_v5(struct spmi_pmic_arb *pmic_arb, u16 n)
 	return pmic_arb->wr_base + 0x108 + 0x10000 * n;
 }
 
+static void __iomem *
+pmic_arb_irq_clear_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
+{
+	return pmic_arb->wr_base + 0x108 + 0x1000 * n;
+}
+
 static u32 pmic_arb_apid_map_offset_v2(u16 n)
 {
 	return 0x800 + 0x4 * n;
@@ -1198,6 +1283,28 @@ static u32 pmic_arb_apid_map_offset_v5(u16 n)
 	return 0x900 + 0x4 * n;
 }
 
+static u32 pmic_arb_apid_map_offset_v7(u16 n)
+{
+	return 0x2000 + 0x4 * n;
+}
+
+static void __iomem *
+pmic_arb_apid_owner_v2(struct spmi_pmic_arb *pmic_arb, u16 n)
+{
+	return pmic_arb->cnfg + 0x700 + 0x4 * n;
+}
+
+/*
+ * For arbiter version 7, APID ownership table registers have independent
+ * numbering space for each SPMI bus instance, so each is indexed starting from
+ * 0.
+ */
+static void __iomem *
+pmic_arb_apid_owner_v7(struct spmi_pmic_arb *pmic_arb, u16 n)
+{
+	return pmic_arb->cnfg + 0x4 * (n - pmic_arb->base_apid);
+}
+
 static const struct pmic_arb_ver_ops pmic_arb_v1 = {
 	.ver_str		= "v1",
 	.ppid_to_apid		= pmic_arb_ppid_to_apid_v1,
@@ -1209,6 +1316,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v1 = {
 	.irq_status		= pmic_arb_irq_status_v1,
 	.irq_clear		= pmic_arb_irq_clear_v1,
 	.apid_map_offset	= pmic_arb_apid_map_offset_v2,
+	.apid_owner		= pmic_arb_apid_owner_v2,
 };
 
 static const struct pmic_arb_ver_ops pmic_arb_v2 = {
@@ -1222,6 +1330,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v2 = {
 	.irq_status		= pmic_arb_irq_status_v2,
 	.irq_clear		= pmic_arb_irq_clear_v2,
 	.apid_map_offset	= pmic_arb_apid_map_offset_v2,
+	.apid_owner		= pmic_arb_apid_owner_v2,
 };
 
 static const struct pmic_arb_ver_ops pmic_arb_v3 = {
@@ -1235,6 +1344,7 @@ static const struct pmic_arb_ver_ops pmic_arb_v3 = {
 	.irq_status		= pmic_arb_irq_status_v2,
 	.irq_clear		= pmic_arb_irq_clear_v2,
 	.apid_map_offset	= pmic_arb_apid_map_offset_v2,
+	.apid_owner		= pmic_arb_apid_owner_v2,
 };
 
 static const struct pmic_arb_ver_ops pmic_arb_v5 = {
@@ -1248,6 +1358,21 @@ static const struct pmic_arb_ver_ops pmic_arb_v5 = {
 	.irq_status		= pmic_arb_irq_status_v5,
 	.irq_clear		= pmic_arb_irq_clear_v5,
 	.apid_map_offset	= pmic_arb_apid_map_offset_v5,
+	.apid_owner		= pmic_arb_apid_owner_v2,
+};
+
+static const struct pmic_arb_ver_ops pmic_arb_v7 = {
+	.ver_str		= "v7",
+	.ppid_to_apid		= pmic_arb_ppid_to_apid_v5,
+	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
+	.offset			= pmic_arb_offset_v7,
+	.fmt_cmd		= pmic_arb_fmt_cmd_v2,
+	.owner_acc_status	= pmic_arb_owner_acc_status_v7,
+	.acc_enable		= pmic_arb_acc_enable_v7,
+	.irq_status		= pmic_arb_irq_status_v7,
+	.irq_clear		= pmic_arb_irq_clear_v7,
+	.apid_map_offset	= pmic_arb_apid_map_offset_v7,
+	.apid_owner		= pmic_arb_apid_owner_v7,
 };
 
 static const struct irq_domain_ops pmic_arb_irq_domain_ops = {
@@ -1274,8 +1399,18 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 	pmic_arb = spmi_controller_get_drvdata(ctrl);
 	pmic_arb->spmic = ctrl;
 
+	/*
+	 * Please don't replace this with devm_platform_ioremap_resource() or
+	 * devm_ioremap_resource().  These both result in a call to
+	 * devm_request_mem_region() which prevents multiple mappings of this
+	 * register address range.  SoCs with PMIC arbiter v7 may define two
+	 * arbiter devices, for the two physical SPMI interfaces, which  share
+	 * some register address ranges (i.e. "core", "obsrvr", and "chnls").
+	 * Ensure that both devices probe successfully by calling devm_ioremap()
+	 * which does not result in a devm_request_mem_region() call.
+	 */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "core");
-	core = devm_ioremap_resource(&ctrl->dev, res);
+	core = devm_ioremap(&ctrl->dev, res->start, resource_size(res));
 	if (IS_ERR(core)) {
 		err = PTR_ERR(core);
 		goto err_put_ctrl;
@@ -1304,12 +1439,15 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 			pmic_arb->ver_ops = &pmic_arb_v2;
 		else if (hw_ver < PMIC_ARB_VERSION_V5_MIN)
 			pmic_arb->ver_ops = &pmic_arb_v3;
-		else
+		else if (hw_ver < PMIC_ARB_VERSION_V7_MIN)
 			pmic_arb->ver_ops = &pmic_arb_v5;
+		else
+			pmic_arb->ver_ops = &pmic_arb_v7;
 
 		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 						   "obsrvr");
-		pmic_arb->rd_base = devm_ioremap_resource(&ctrl->dev, res);
+		pmic_arb->rd_base = devm_ioremap(&ctrl->dev, res->start,
+						 resource_size(res));
 		if (IS_ERR(pmic_arb->rd_base)) {
 			err = PTR_ERR(pmic_arb->rd_base);
 			goto err_put_ctrl;
@@ -1317,13 +1455,70 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 
 		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
 						   "chnls");
-		pmic_arb->wr_base = devm_ioremap_resource(&ctrl->dev, res);
+		pmic_arb->wr_base = devm_ioremap(&ctrl->dev, res->start,
+						 resource_size(res));
 		if (IS_ERR(pmic_arb->wr_base)) {
 			err = PTR_ERR(pmic_arb->wr_base);
 			goto err_put_ctrl;
 		}
 	}
 
+	pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS;
+
+	if (hw_ver >= PMIC_ARB_VERSION_V7_MIN) {
+		pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS_V7;
+		/* Optional property for v7: */
+		of_property_read_u32(pdev->dev.of_node, "qcom,bus-id",
+					&pmic_arb->bus_instance);
+		if (pmic_arb->bus_instance > 1) {
+			err = -EINVAL;
+			dev_err(&pdev->dev, "invalid bus instance (%u) specified\n",
+				pmic_arb->bus_instance);
+			goto err_put_ctrl;
+		}
+
+		if (pmic_arb->bus_instance == 0) {
+			pmic_arb->base_apid = 0;
+			pmic_arb->apid_count =
+				readl_relaxed(core + PMIC_ARB_FEATURES) &
+				PMIC_ARB_FEATURES_PERIPH_MASK;
+		} else {
+			pmic_arb->base_apid =
+				readl_relaxed(core + PMIC_ARB_FEATURES) &
+				PMIC_ARB_FEATURES_PERIPH_MASK;
+			pmic_arb->apid_count =
+				readl_relaxed(core + PMIC_ARB_FEATURES1) &
+				PMIC_ARB_FEATURES_PERIPH_MASK;
+		}
+
+		if (pmic_arb->base_apid + pmic_arb->apid_count >
+		    pmic_arb->max_periphs) {
+			err = -EINVAL;
+			dev_err(&pdev->dev, "Unsupported APID count %d detected\n",
+				pmic_arb->base_apid + pmic_arb->apid_count);
+			goto err_put_ctrl;
+		}
+	} else if (hw_ver >= PMIC_ARB_VERSION_V5_MIN) {
+		pmic_arb->base_apid = 0;
+		pmic_arb->apid_count = readl_relaxed(core + PMIC_ARB_FEATURES) &
+					PMIC_ARB_FEATURES_PERIPH_MASK;
+
+		if (pmic_arb->apid_count > pmic_arb->max_periphs) {
+			err = -EINVAL;
+			dev_err(&pdev->dev, "Unsupported APID count %d detected\n",
+				pmic_arb->apid_count);
+			goto err_put_ctrl;
+		}
+	}
+
+	pmic_arb->apid_data = devm_kcalloc(&ctrl->dev, pmic_arb->max_periphs,
+					   sizeof(*pmic_arb->apid_data),
+					   GFP_KERNEL);
+	if (!pmic_arb->apid_data) {
+		err = -ENOMEM;
+		goto err_put_ctrl;
+	}
+
 	dev_info(&ctrl->dev, "PMIC arbiter version %s (0x%x)\n",
 		 pmic_arb->ver_ops->ver_str, hw_ver);
 
@@ -1386,7 +1581,7 @@ static int spmi_pmic_arb_probe(struct platform_device *pdev)
 	/* Initialize max_apid/min_apid to the opposite bounds, during
 	 * the irq domain translation, we are sure to update these */
 	pmic_arb->max_apid = 0;
-	pmic_arb->min_apid = PMIC_ARB_MAX_PERIPHS - 1;
+	pmic_arb->min_apid = pmic_arb->max_periphs - 1;
 
 	platform_set_drvdata(pdev, ctrl);
 	raw_spin_lock_init(&pmic_arb->lock);
-- 
2.31.1

Re: [PATCH v3 2/2] spmi: pmic-arb: Add support for PMIC v7

[Dmitry Baryshkov]

On 01/02/2022 16:41, Vinod Koul wrote:
> From: David Collins <collinsd@codeaurora.org>
> 
> PMIC v7 has different offset values and seqeunces, so add support for
> this new version of PMIC
> 
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>

Tested-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> # SM8450 HDK

> ---
>   drivers/spmi/spmi-pmic-arb.c | 233 ++++++++++++++++++++++++++++++++---
>   1 file changed, 214 insertions(+), 19 deletions(-)


-- 
With best wishes
Dmitry

Re: [PATCH v3 2/2] spmi: pmic-arb: Add support for PMIC v7

[Dmitry Baryshkov]

On 01/02/2022 16:41, Vinod Koul wrote:
> From: David Collins <collinsd@codeaurora.org>
> 
> PMIC v7 has different offset values and seqeunces, so add support for
> this new version of PMIC
> 
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>   drivers/spmi/spmi-pmic-arb.c | 233 ++++++++++++++++++++++++++++++++---
>   1 file changed, 214 insertions(+), 19 deletions(-)


As I was asking Stephen about the fate of this patch series I could not 
stop my self from noticing that one of his comments ([1]) from v1 was 
ignored.
Let me quote it here:

 > The driver is already pretty hard to read because it combines so many
 > generations of spmi arbiter hardware from qcom into one file. It would
 > probably be better to start over and simplify the new version of the
 > driver, possibly sharing code between the two files if possible, but
 > otherwise dropping lots of cruft along the way and simplifying review
 > burden.

After taking a glance, I thought maybe we should really follow this 
approach. And it also allows us to start with the new bindings:

spmi@c400000 {
    compatible = "qcom,spmi-pmic-arb-v7";

    reg = <..... both arb registers as following....>;
    reg-names = "core", "chnls", "observer", "cnfg0", "intr0", "cnfg1", 
"intr1";

    interrupts = <&pdc 1 HIGH>, <&pdc 3 HIGH>;
    interrupt-names = "arb0", "arb1"; /* are the names necessary at all? */

    #address-cells = <1>;
    #size-cells = <0>;

    spmi_bus: bus@0 {
       reg = <0>;
       #address-cells = <2>;
       #size-cells = <0>;
       #interrupt-cells = <4>;
       interrupt-controller;

       pmic@.... {
           // etc.
       };

    };

    spmi1_bus: bus@1 {
       reg = <1>;
       #address-cells = <2>;
       #size-cells = <0>;
       #interrupt-cells = <4>;
       interrupt-controller;
    };
};

Note, this drops the qcom,ee (which is always 0 for all devices I see in 
mainline)) and qcom,channel (which if I understood correctly is used 
only for pmic-arb-v1, ugh). It uses common reg = <N> property instead of
cooked qcom,bus-id. And last, but not least, it save us from huuge 
comments in the source code telling why devm_platform_ioremap_resource 
can not be used.

[1] 
https://lore.kernel.org/linux-arm-msm/20211210020148.B2EA6C004DD@smtp.kernel.org/


-- 
With best wishes
Dmitry

Re: [PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Vinod Koul]

On 01-02-22, 19:11, Vinod Koul wrote:
> Hello,
> 
> The is version 3 of support for PMIC v7. I have added a new property
> qcom,bus-id for supporting v7 and then add driver changes for v7
> 
> This depends on yaml conversion patch:
> https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/

Any feedback on this...

> 
> Changes since v2:
>  - Drop yaml conversion patch
>  - Fix author for spmi patch
> Changes since v1:
>  - Add yaml conversion patch and new binding
>  - fix driver bug report by Jonathan
> 
> David Collins (1):
>   spmi: pmic-arb: Add support for PMIC v7
> 
> Vinod Koul (1):
>   dt-bindings: spmi: Add qcom,bus-id
> 
>  .../bindings/spmi/qcom,spmi-pmic-arb.yaml     |  11 +
>  drivers/spmi/spmi-pmic-arb.c                  | 233 ++++++++++++++++--
>  2 files changed, 225 insertions(+), 19 deletions(-)
> 
> -- 
> 2.31.1

-- 
~Vinod

Re: [PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Dmitry Baryshkov]

On 22/02/2022 19:53, Vinod Koul wrote:
> On 01-02-22, 19:11, Vinod Koul wrote:
>> Hello,
>>
>> The is version 3 of support for PMIC v7. I have added a new property
>> qcom,bus-id for supporting v7 and then add driver changes for v7
>>
>> This depends on yaml conversion patch:
>> https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/
> 
> Any feedback on this...

Another gracious reminder about these patches. At this moment this is 
one of the important pieces lacking for the full SM8450 support in the 
upstream kernel.

> 
>>
>> Changes since v2:
>>   - Drop yaml conversion patch
>>   - Fix author for spmi patch
>> Changes since v1:
>>   - Add yaml conversion patch and new binding
>>   - fix driver bug report by Jonathan
>>
>> David Collins (1):
>>    spmi: pmic-arb: Add support for PMIC v7
>>
>> Vinod Koul (1):
>>    dt-bindings: spmi: Add qcom,bus-id
>>
>>   .../bindings/spmi/qcom,spmi-pmic-arb.yaml     |  11 +
>>   drivers/spmi/spmi-pmic-arb.c                  | 233 ++++++++++++++++--
>>   2 files changed, 225 insertions(+), 19 deletions(-)
>>
>> -- 
>> 2.31.1
> 


-- 
With best wishes
Dmitry

Re: [PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Dmitry Baryshkov]

On 01/05/2022 22:41, Dmitry Baryshkov wrote:
> On 22/02/2022 19:53, Vinod Koul wrote:
>> On 01-02-22, 19:11, Vinod Koul wrote:
>>> Hello,
>>>
>>> The is version 3 of support for PMIC v7. I have added a new property
>>> qcom,bus-id for supporting v7 and then add driver changes for v7
>>>
>>> This depends on yaml conversion patch:
>>> https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/ 
>>>
>>
>> Any feedback on this...
> 
> Another gracious reminder about these patches. At this moment this is 
> one of the important pieces lacking for the full SM8450 support in the 
> upstream kernel.

Stephen, yet another ping. This is the blocking point for the further 
SM8450 progress.

>>> Changes since v2:
>>>   - Drop yaml conversion patch
>>>   - Fix author for spmi patch
>>> Changes since v1:
>>>   - Add yaml conversion patch and new binding
>>>   - fix driver bug report by Jonathan
>>>
>>> David Collins (1):
>>>    spmi: pmic-arb: Add support for PMIC v7
>>>
>>> Vinod Koul (1):
>>>    dt-bindings: spmi: Add qcom,bus-id
>>>
>>>   .../bindings/spmi/qcom,spmi-pmic-arb.yaml     |  11 +
>>>   drivers/spmi/spmi-pmic-arb.c                  | 233 ++++++++++++++++--
>>>   2 files changed, 225 insertions(+), 19 deletions(-)
>>>
>>> -- 
>>> 2.31.1
>>
> 
> 


-- 
With best wishes
Dmitry

Re: [PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Linus Walleij]

On Sat, Jun 18, 2022 at 5:30 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
> On 01/05/2022 22:41, Dmitry Baryshkov wrote:
> > On 22/02/2022 19:53, Vinod Koul wrote:
> >> On 01-02-22, 19:11, Vinod Koul wrote:
> >>> Hello,
> >>>
> >>> The is version 3 of support for PMIC v7. I have added a new property
> >>> qcom,bus-id for supporting v7 and then add driver changes for v7
> >>>
> >>> This depends on yaml conversion patch:
> >>> https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/
> >>>
> >>
> >> Any feedback on this...
> >
> > Another gracious reminder about these patches. At this moment this is
> > one of the important pieces lacking for the full SM8450 support in the
> > upstream kernel.
>
> Stephen, yet another ping. This is the blocking point for the further
> SM8450 progress.

Pending since february!

I would rather suspect something is wrong and it somehow goes
into the spam or wrong inbox on Stephen's end.

Sounds like something Bjorn could solve if Stephen don't have time
to manage the SPMI subsystem right now? If it's OK with Stephen
or he simply don't respond we can just queue the stuff in the SoC
tree I assume?

Yours,
Linus Walleij

Re: [PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-06-18 08:29:58)
> On 01/05/2022 22:41, Dmitry Baryshkov wrote:
> > On 22/02/2022 19:53, Vinod Koul wrote:
> >> On 01-02-22, 19:11, Vinod Koul wrote:
> >>> Hello,
> >>>
> >>> The is version 3 of support for PMIC v7. I have added a new property
> >>> qcom,bus-id for supporting v7 and then add driver changes for v7
> >>>
> >>> This depends on yaml conversion patch:
> >>> https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/ 
> >>>
> >>
> >> Any feedback on this...
> > 
> > Another gracious reminder about these patches. At this moment this is 
> > one of the important pieces lacking for the full SM8450 support in the 
> > upstream kernel.
> 
> Stephen, yet another ping. This is the blocking point for the further 
> SM8450 progress.
> 

Sorry I completely missed this one as it fell off the end of my inbox
into the abyss.

Re: [PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Vinod Koul]

On 29-08-22, 15:25, Stephen Boyd wrote:
> Quoting Dmitry Baryshkov (2022-06-18 08:29:58)
> > On 01/05/2022 22:41, Dmitry Baryshkov wrote:
> > > On 22/02/2022 19:53, Vinod Koul wrote:
> > >> On 01-02-22, 19:11, Vinod Koul wrote:
> > >>> Hello,
> > >>>
> > >>> The is version 3 of support for PMIC v7. I have added a new property
> > >>> qcom,bus-id for supporting v7 and then add driver changes for v7
> > >>>
> > >>> This depends on yaml conversion patch:
> > >>> https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/ 
> > >>>
> > >>
> > >> Any feedback on this...
> > > 
> > > Another gracious reminder about these patches. At this moment this is 
> > > one of the important pieces lacking for the full SM8450 support in the 
> > > upstream kernel.
> > 
> > Stephen, yet another ping. This is the blocking point for the further 
> > SM8450 progress.
> > 
> 
> Sorry I completely missed this one as it fell off the end of my inbox
> into the abyss.

Thanks for the reply. Is this applied now or you have some feedback for
me to address..

Thanks
-- 
~Vinod

Re: [PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Stephen Boyd]

Quoting Vinod Koul (2022-08-29 21:36:00)
> On 29-08-22, 15:25, Stephen Boyd wrote:
> > Quoting Dmitry Baryshkov (2022-06-18 08:29:58)
> > > On 01/05/2022 22:41, Dmitry Baryshkov wrote:
> > > > On 22/02/2022 19:53, Vinod Koul wrote:
> > > >> On 01-02-22, 19:11, Vinod Koul wrote:
> > > >>> Hello,
> > > >>>
> > > >>> The is version 3 of support for PMIC v7. I have added a new property
> > > >>> qcom,bus-id for supporting v7 and then add driver changes for v7
> > > >>>
> > > >>> This depends on yaml conversion patch:
> > > >>> https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/ 
> > > >>>
> > > >>
> > > >> Any feedback on this...
> > > > 
> > > > Another gracious reminder about these patches. At this moment this is 
> > > > one of the important pieces lacking for the full SM8450 support in the 
> > > > upstream kernel.
> > > 
> > > Stephen, yet another ping. This is the blocking point for the further 
> > > SM8450 progress.
> > > 
> > 
> > Sorry I completely missed this one as it fell off the end of my inbox
> > into the abyss.
> 
> Thanks for the reply. Is this applied now or you have some feedback for
> me to address..
> 

Does it apply along with the series from qcom[1]? I have to check and
make sure they both work together.

[1] https://lore.kernel.org/r/1651108369-11059-1-git-send-email-quic_fenglinw@quicinc.com

Re: [PATCH v3 0/2] spmi: pmic-arb: Add support for PMIC v7

[Vinod Koul]

On 30-08-22, 14:11, Stephen Boyd wrote:
> Quoting Vinod Koul (2022-08-29 21:36:00)
> > On 29-08-22, 15:25, Stephen Boyd wrote:
> > > Quoting Dmitry Baryshkov (2022-06-18 08:29:58)
> > > > On 01/05/2022 22:41, Dmitry Baryshkov wrote:
> > > > > On 22/02/2022 19:53, Vinod Koul wrote:
> > > > >> On 01-02-22, 19:11, Vinod Koul wrote:
> > > > >>> Hello,
> > > > >>>
> > > > >>> The is version 3 of support for PMIC v7. I have added a new property
> > > > >>> qcom,bus-id for supporting v7 and then add driver changes for v7
> > > > >>>
> > > > >>> This depends on yaml conversion patch:
> > > > >>> https://lore.kernel.org/linux-arm-msm/20211227170151.73116-1-david@ixit.cz/ 
> > > > >>>
> > > > >>
> > > > >> Any feedback on this...
> > > > > 
> > > > > Another gracious reminder about these patches. At this moment this is 
> > > > > one of the important pieces lacking for the full SM8450 support in the 
> > > > > upstream kernel.
> > > > 
> > > > Stephen, yet another ping. This is the blocking point for the further 
> > > > SM8450 progress.
> > > > 
> > > 
> > > Sorry I completely missed this one as it fell off the end of my inbox
> > > into the abyss.
> > 
> > Thanks for the reply. Is this applied now or you have some feedback for
> > me to address..
> > 
> 
> Does it apply along with the series from qcom[1]? I have to check and
> make sure they both work together.

There were conflicts, I have resolved, tested on top of spmi/next again
and posted the v4 now.. Hope this would get in.

-- 
~Vinod