[PATCH 0/2] Add PMICs support for UMS9621 SoC

[PATCH 0/2] Add PMICs support for UMS9621 SoC

[Jiansheng Wu]

Patch 1, bindings: Convert sprd spi bindings to yaml and add UMP962x PMICs.
Patch 2, mfd: Add PMICs support for UMS9621 SoC.

Jiansheng Wu (2):
  dt-bindings: spi: Convert sprd spi bindings to yaml
  mfd: sprd-sc27xx-spi: Add PMICs support for UMS9621 SoC

 .../bindings/mfd/sprd,sc27xx-pmic.txt         |  40 -------
 .../bindings/mfd/sprd,sc27xx-pmic.yaml        |  84 ++++++++++++++
 drivers/mfd/sprd-sc27xx-spi.c                 | 104 +++++++++++++-----
 3 files changed, 161 insertions(+), 67 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml

-- 
2.17.1

[PATCH 1/2] dt-bindings: spi: Convert sprd spi bindings to yaml

[Jiansheng Wu]

Convert sprd,sc27xx-pmic.txt to yaml, and add UMP962x series PMICs.

Signed-off-by: Jiansheng Wu <jiansheng.wu@unisoc.com>
---
 .../bindings/mfd/sprd,sc27xx-pmic.txt         | 40 ---------
 .../bindings/mfd/sprd,sc27xx-pmic.yaml        | 84 +++++++++++++++++++
 2 files changed, 84 insertions(+), 40 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt
 create mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml

diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt
deleted file mode 100644
index 21b9a897fca5..000000000000
--- a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-Spreadtrum SC27xx Power Management Integrated Circuit (PMIC)
-
-The Spreadtrum SC27xx series PMICs contain SC2720, SC2721, SC2723, SC2730
-and SC2731. The Spreadtrum PMIC belonging to SC27xx series integrates all
-mobile handset power management, audio codec, battery management and user
-interface support function in a single chip. It has 6 major functional
-blocks:
-- DCDCs to support CPU, memory.
-- LDOs to support both internal and external requirement.
-- Battery management system, such as charger, fuel gauge.
-- Audio codec.
-- User interface function, such as indicator, flash LED and so on.
-- IC level interface, such as power on/off control, RTC and typec and so on.
-
-Required properties:
-- compatible: Should be one of the following:
-	"sprd,sc2720"
-	"sprd,sc2721"
-	"sprd,sc2723"
-	"sprd,sc2730"
-	"sprd,sc2731"
-- reg: The address of the device chip select, should be 0.
-- spi-max-frequency: Typically set to 26000000.
-- interrupts: The interrupt line the device is connected to.
-- interrupt-controller: Marks the device node as an interrupt controller.
-- #interrupt-cells: The number of cells to describe an PMIC IRQ, must be 2.
-- #address-cells: Child device offset number of cells, must be 1.
-- #size-cells: Child device size number of cells, must be 0.
-
-Example:
-pmic@0 {
-	compatible = "sprd,sc2731";
-	reg = <0>;
-	spi-max-frequency = <26000000>;
-	interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
-	interrupt-controller;
-	#interrupt-cells = <2>;
-	#address-cells = <1>;
-	#size-cells = <0>;
-};
diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml
new file mode 100644
index 000000000000..590970a17143
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: |
+  Spreadtrum SC27xx and UMP96xx Power Management Integrated Circuit (PMIC)
+
+maintainers:
+  - Orson Zhai <orsonzhai@gmail.com>
+  - Baolin Wang <baolin.wang7@gmail.com>
+  - Chunyan Zhang <zhang.lyra@gmail.com>
+
+description: |
+  The Spreadtrum SC27xx series PMICs contain SC2720, SC2721, SC2723, SC2730, SC2731
+  and UMP96xx series PMICs contain ump9620, ump962 and ump9622. The Spreadtrum PMIC
+  belonging to SC27xx series and ump962x series integrates all mobile handset power
+  management, audio codec, battery management and user interface support function in
+  a single chip. It has 6 major functional.
+
+blocks:
+  - DCDCs to support CPU, memory.
+  - LDOs to support both internal and external requirement.
+  - Battery management system, such as charger, fuel gauge.
+  - Audio codec.
+  - User interface function, such as indicator, flash LED and so on.
+  - IC level interface, such as power on/off control, RTC and typec and so on.
+
+allOf:
+  - $ref: /schemas/spi/sprd,spi-adi.yaml#
+
+properties:
+  compatible:
+    enum:
+      - sprd,sc2720
+      - sprd,sc2721
+      - sprd,sc2723
+      - sprd,sc2730
+      - sprd,sc2731
+      - sprd,ump9620
+      - sprd,ump9621
+      - sprd,ump9622
+
+  reg:
+    maxItems: 7
+    description: The address of the device chip select
+
+  spi-max-frequency:
+    default: 26000000
+
+  interrupts: true
+
+  interrupt-controller:
+    description: Marks the device node as an interrupt controller.
+
+  interrupt-cells:
+    const: 2
+    description: The number of cells to describe an PMIC IRQ, must be 2.
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+  - '#address-cells' # Child device offset number of cells, must be 1.
+  - '#size-cells' # Child device size number of cells, must be 0.
+
+unevaluatedProperties: false
+
+Example:
+  - |
+    adi_bus {
+        pmic@0 {
+            compatible = "sprd,sc2731";
+            reg = <0>;
+            spi-max-frequency = <26000000>;
+            interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+        };
+    };
+...
-- 
2.17.1

Re: [PATCH 1/2] dt-bindings: spi: Convert sprd spi bindings to yaml

[Krzysztof Kozlowski]

On 22/08/2023 09:51, Jiansheng Wu wrote:
> Convert sprd,sc27xx-pmic.txt to yaml, and add UMP962x series PMICs.
> 
> Signed-off-by: Jiansheng Wu <jiansheng.wu@unisoc.com>

Subject is bogus. This is not SPI.

> ---
>  .../bindings/mfd/sprd,sc27xx-pmic.txt         | 40 ---------
>  .../bindings/mfd/sprd,sc27xx-pmic.yaml        | 84 +++++++++++++++++++
>  2 files changed, 84 insertions(+), 40 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt
>  create mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt
> deleted file mode 100644
> index 21b9a897fca5..000000000000
> --- a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -Spreadtrum SC27xx Power Management Integrated Circuit (PMIC)
> -
> -The Spreadtrum SC27xx series PMICs contain SC2720, SC2721, SC2723, SC2730
> -and SC2731. The Spreadtrum PMIC belonging to SC27xx series integrates all
> -mobile handset power management, audio codec, battery management and user
> -interface support function in a single chip. It has 6 major functional
> -blocks:
> -- DCDCs to support CPU, memory.
> -- LDOs to support both internal and external requirement.
> -- Battery management system, such as charger, fuel gauge.
> -- Audio codec.
> -- User interface function, such as indicator, flash LED and so on.
> -- IC level interface, such as power on/off control, RTC and typec and so on.
> -
> -Required properties:
> -- compatible: Should be one of the following:
> -	"sprd,sc2720"
> -	"sprd,sc2721"
> -	"sprd,sc2723"
> -	"sprd,sc2730"
> -	"sprd,sc2731"
> -- reg: The address of the device chip select, should be 0.
> -- spi-max-frequency: Typically set to 26000000.
> -- interrupts: The interrupt line the device is connected to.
> -- interrupt-controller: Marks the device node as an interrupt controller.
> -- #interrupt-cells: The number of cells to describe an PMIC IRQ, must be 2.
> -- #address-cells: Child device offset number of cells, must be 1.
> -- #size-cells: Child device size number of cells, must be 0.
> -
> -Example:
> -pmic@0 {
> -	compatible = "sprd,sc2731";
> -	reg = <0>;
> -	spi-max-frequency = <26000000>;
> -	interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> -	interrupt-controller;
> -	#interrupt-cells = <2>;
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> -};
> diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml
> new file mode 100644
> index 000000000000..590970a17143
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: |
> +  Spreadtrum SC27xx and UMP96xx Power Management Integrated Circuit (PMIC)
> +
> +maintainers:
> +  - Orson Zhai <orsonzhai@gmail.com>
> +  - Baolin Wang <baolin.wang7@gmail.com>
> +  - Chunyan Zhang <zhang.lyra@gmail.com>
> +
> +description: |
> +  The Spreadtrum SC27xx series PMICs contain SC2720, SC2721, SC2723, SC2730, SC2731
> +  and UMP96xx series PMICs contain ump9620, ump962 and ump9622. The Spreadtrum PMIC
> +  belonging to SC27xx series and ump962x series integrates all mobile handset power
> +  management, audio codec, battery management and user interface support function in
> +  a single chip. It has 6 major functional.
> +
> +blocks:

Please don't use confusing YAML syntax.

> +  - DCDCs to support CPU, memory.
> +  - LDOs to support both internal and external requirement.
> +  - Battery management system, such as charger, fuel gauge.
> +  - Audio codec.
> +  - User interface function, such as indicator, flash LED and so on.
> +  - IC level interface, such as power on/off control, RTC and typec and so on.
> +
> +allOf:
> +  - $ref: /schemas/spi/sprd,spi-adi.yaml#

This is confusing. How is this device a SPI controller? You have
entirely different compatibles, so no, it's wrong. Missing ref to
spi-peripheral properties. See other bindings.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - sprd,sc2720
> +      - sprd,sc2721
> +      - sprd,sc2723
> +      - sprd,sc2730
> +      - sprd,sc2731
> +      - sprd,ump9620
> +      - sprd,ump9621
> +      - sprd,ump9622

This does not match your previous bindings and nothing is explained in
commit msg.

> +
> +  reg:
> +    maxItems: 7
> +    description: The address of the device chip select

7 items? No way.

> +
> +  spi-max-frequency:
> +    default: 26000000
> +
> +  interrupts: true

You need constraints.

> +
> +  interrupt-controller:
> +    description: Marks the device node as an interrupt controller.

Drop description. ": true" instead

> +
> +  interrupt-cells:
> +    const: 2
> +    description: The number of cells to describe an PMIC IRQ, must be 2.

You did not test it.

> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - '#address-cells' # Child device offset number of cells, must be 1.

??

> +  - '#size-cells' # Child device size number of cells, must be 0.

?? No way. You do not have any children. Drop bogus properties and
explain it in commit msg.

> +
> +unevaluatedProperties: false
> +
> +Example:
> +  - |
> +    adi_bus {

No underscores in node names, generic node names, whatever this supposed
to be. I guess spi...

> +        pmic@0 {
> +            compatible = "sprd,sc2731";
> +            reg = <0>;
> +            spi-max-frequency = <26000000>;
> +            interrupts = <GIC_SPI 31 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-controller;
> +            #interrupt-cells = <2>;

Test your bindings...

> +...

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: spi: Convert sprd spi bindings to yaml

[Rob Herring]

On Tue, 22 Aug 2023 15:51:12 +0800, Jiansheng Wu wrote:
> Convert sprd,sc27xx-pmic.txt to yaml, and add UMP962x series PMICs.
> 
> Signed-off-by: Jiansheng Wu <jiansheng.wu@unisoc.com>
> ---
>  .../bindings/mfd/sprd,sc27xx-pmic.txt         | 40 ---------
>  .../bindings/mfd/sprd,sc27xx-pmic.yaml        | 84 +++++++++++++++++++
>  2 files changed, 84 insertions(+), 40 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.txt
>  create mode 100644 Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml: 'blocks' is not one of ['$id', '$schema', 'title', 'description', 'examples', 'required', 'allOf', 'anyOf', 'oneOf', 'definitions', '$defs', 'additionalProperties', 'dependencies', 'dependentRequired', 'dependentSchemas', 'patternProperties', 'properties', 'not', 'if', 'then', 'else', 'unevaluatedProperties', 'deprecated', 'maintainers', 'select', '$ref']
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/sprd,sc27xx-pmic.yaml: 'Example' is not one of ['$id', '$schema', 'title', 'description', 'examples', 'required', 'allOf', 'anyOf', 'oneOf', 'definitions', '$defs', 'additionalProperties', 'dependencies', 'dependentRequired', 'dependentSchemas', 'patternProperties', 'properties', 'not', 'if', 'then', 'else', 'unevaluatedProperties', 'deprecated', 'maintainers', 'select', '$ref']
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/sprd,sc27xx-vibrator.example.dtb: pmic@0: $nodename:0: 'pmic@0' does not match '^spi(@.*|-([0-9]|[1-9][0-9]+))?$'
	from schema $id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/sprd,sc27xx-vibrator.example.dtb: pmic@0: vibrator@eb4:reg:0:0: 3764 is greater than the maximum of 256
	from schema $id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/sprd,sc27xx-vibrator.example.dtb: pmic@0: compatible:0: 'sprd,sc2731' is not one of ['sprd,sc9860-adi', 'sprd,sc9863-adi', 'sprd,ums512-adi']
	from schema $id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/sprd,sc27xx-vibrator.example.dtb: pmic@0: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'compatible', 'interrupt-controller', 'interrupts', 'spi-max-frequency', 'vibrator@eb4' were unexpected)
	from schema $id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/sprd,sc27xx-vibrator.example.dtb: pmic@0: reg: [[0, 0]] is too short
	from schema $id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/sprd,sc27xx-vibrator.example.dtb: pmic@0: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'reg', 'vibrator@eb4' were unexpected)
	from schema $id: http://devicetree.org/schemas/mfd/sprd,sc27xx-pmic.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230822075113.25506-2-jiansheng.wu@unisoc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

[PATCH 2/2] mfd: sprd-sc27xx-spi: Add PMICs support for UMS9621 SoC

[Jiansheng Wu]

There are three PMICs (UMP9620/21/22) on Unisoc's UMS9621 chip.
UMP9620 is a master PMIC, the others are slave ones. Slave PMICs
don't have irq functions, which is different from master device,
such as SC27xx series and UMP9620, etc.

Signed-off-by: Jiansheng Wu <jiansheng.wu@unisoc.com>
---
 drivers/mfd/sprd-sc27xx-spi.c | 104 +++++++++++++++++++++++++---------
 1 file changed, 77 insertions(+), 27 deletions(-)

diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
index d21f32cc784d..aa91301568a9 100644
--- a/drivers/mfd/sprd-sc27xx-spi.c
+++ b/drivers/mfd/sprd-sc27xx-spi.c
@@ -24,6 +24,10 @@
 #define SPRD_SC2731_IRQ_BASE		0x140
 #define SPRD_SC2731_IRQ_NUMS		16
 #define SPRD_SC2731_CHG_DET		0xedc
+#define SPRD_UMP9620_IRQ_BASE           0x80
+#define SPRD_UMP9620_IRQ_NUMS           11
+#define SPRD_UMP9621_SLAVE_ID           0x8000
+#define SPRD_UMP9622_SLAVE_ID           0xc000
 
 /* PMIC charger detection definition */
 #define SPRD_PMIC_CHG_DET_DELAY_US	200000
@@ -45,6 +49,7 @@ struct sprd_pmic {
 };
 
 struct sprd_pmic_data {
+	u32 slave_id;
 	u32 irq_base;
 	u32 num_irqs;
 	u32 charger_det;
@@ -67,6 +72,19 @@ static const struct sprd_pmic_data sc2731_data = {
 	.charger_det = SPRD_SC2731_CHG_DET,
 };
 
+static const struct sprd_pmic_data ump9620_data = {
+	.irq_base = SPRD_UMP9620_IRQ_BASE,
+	.num_irqs = SPRD_UMP9620_IRQ_NUMS,
+};
+
+static const struct sprd_pmic_data ump9621_data = {
+	.slave_id = SPRD_UMP9621_SLAVE_ID,
+};
+
+static const struct sprd_pmic_data ump9622_data = {
+	.slave_id = SPRD_UMP9622_SLAVE_ID,
+};
+
 enum usb_charger_type sprd_pmic_detect_charger_type(struct device *dev)
 {
 	struct spi_device *spi = to_spi_device(dev);
@@ -108,8 +126,27 @@ static int sprd_pmic_spi_write(void *context, const void *data, size_t count)
 {
 	struct device *dev = context;
 	struct spi_device *spi = to_spi_device(dev);
+	const struct sprd_pmic_data *pdata;
+	int ret;
+	u32 *pmdata;
+
+	if (!pdata->slave_id) {
+		ret = spi_write(spi, data, count);
+	} else {
+		pdata = ((struct sprd_pmic *)spi_get_drvdata(spi))->pdata;
+
+		pmdata = kzalloc(count, GFP_KERNEL);
+		if (!pmdata)
+			return -ENOMEM;
+		memcpy(pmdata, data, count);
+		*pmdata += pdata->slave_id;
+		ret = spi_write(spi, (const void *)pmdata, count);
+		kfree(pmdata);
+	}
+	if (ret)
+		pr_err("pmic mfd write failed!\n");
 
-	return spi_write(spi, data, count);
+	return ret;
 }
 
 static int sprd_pmic_spi_read(void *context,
@@ -118,6 +155,7 @@ static int sprd_pmic_spi_read(void *context,
 {
 	struct device *dev = context;
 	struct spi_device *spi = to_spi_device(dev);
+	const struct sprd_pmic_data *pdata;
 	u32 rx_buf[2] = { 0 };
 	int ret;
 
@@ -125,11 +163,16 @@ static int sprd_pmic_spi_read(void *context,
 	if (reg_size != sizeof(u32) || val_size != sizeof(u32))
 		return -EINVAL;
 
+	pdata = ((struct sprd_pmic *)spi_get_drvdata(spi))->pdata;
 	/* Copy address to read from into first element of SPI buffer. */
 	memcpy(rx_buf, reg, sizeof(u32));
+	if (!pdata->slave_id)
+		rx_buf[0] += pdata->slave_id;
 	ret = spi_read(spi, rx_buf, 1);
-	if (ret < 0)
+	if (ret < 0) {
+		pr_err("pmic mfd read failed!\n");
 		return ret;
+	}
 
 	memcpy(val, rx_buf, val_size);
 	return 0;
@@ -175,33 +218,34 @@ static int sprd_pmic_probe(struct spi_device *spi)
 
 	spi_set_drvdata(spi, ddata);
 	ddata->dev = &spi->dev;
-	ddata->irq = spi->irq;
 	ddata->pdata = pdata;
 
-	ddata->irq_chip.name = dev_name(&spi->dev);
-	ddata->irq_chip.status_base =
-		pdata->irq_base + SPRD_PMIC_INT_MASK_STATUS;
-	ddata->irq_chip.unmask_base = pdata->irq_base + SPRD_PMIC_INT_EN;
-	ddata->irq_chip.ack_base = 0;
-	ddata->irq_chip.num_regs = 1;
-	ddata->irq_chip.num_irqs = pdata->num_irqs;
-
-	ddata->irqs = devm_kcalloc(&spi->dev,
-				   pdata->num_irqs, sizeof(struct regmap_irq),
-				   GFP_KERNEL);
-	if (!ddata->irqs)
-		return -ENOMEM;
-
-	ddata->irq_chip.irqs = ddata->irqs;
-	for (i = 0; i < pdata->num_irqs; i++)
-		ddata->irqs[i].mask = BIT(i);
-
-	ret = devm_regmap_add_irq_chip(&spi->dev, ddata->regmap, ddata->irq,
-				       IRQF_ONESHOT, 0,
-				       &ddata->irq_chip, &ddata->irq_data);
-	if (ret) {
-		dev_err(&spi->dev, "Failed to add PMIC irq chip %d\n", ret);
-		return ret;
+	if (spi->irq) {
+		ddata->irq = spi->irq;
+		ddata->irq_chip.name = dev_name(&spi->dev);
+		ddata->irq_chip.status_base = pdata->irq_base + SPRD_PMIC_INT_MASK_STATUS;
+		ddata->irq_chip.unmask_base = pdata->irq_base + SPRD_PMIC_INT_EN;
+		ddata->irq_chip.ack_base = 0;
+		ddata->irq_chip.num_regs = 1;
+		ddata->irq_chip.num_irqs = pdata->num_irqs;
+
+		ddata->irqs = devm_kcalloc(&spi->dev,
+					   pdata->num_irqs, sizeof(struct regmap_irq),
+					   GFP_KERNEL);
+		if (!ddata->irqs)
+			return -ENOMEM;
+
+		ddata->irq_chip.irqs = ddata->irqs;
+		for (i = 0; i < pdata->num_irqs; i++)
+			ddata->irqs[i].mask = BIT(i);
+
+		ret = devm_regmap_add_irq_chip(&spi->dev, ddata->regmap, ddata->irq,
+					       IRQF_ONESHOT, 0,
+					       &ddata->irq_chip, &ddata->irq_data);
+		if (ret) {
+			dev_err(&spi->dev, "Failed to add PMIC irq chip %d\n", ret);
+			return ret;
+		}
 	}
 
 	ret = devm_of_platform_populate(&spi->dev);
@@ -240,6 +284,9 @@ static DEFINE_SIMPLE_DEV_PM_OPS(sprd_pmic_pm_ops,
 static const struct of_device_id sprd_pmic_match[] = {
 	{ .compatible = "sprd,sc2730", .data = &sc2730_data },
 	{ .compatible = "sprd,sc2731", .data = &sc2731_data },
+	{ .compatible = "sprd,ump9620", .data = &ump9620_data },
+	{ .compatible = "sprd,ump9621", .data = &ump9621_data },
+	{ .compatible = "sprd,ump9622", .data = &ump9622_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, sprd_pmic_match);
@@ -247,6 +294,9 @@ MODULE_DEVICE_TABLE(of, sprd_pmic_match);
 static const struct spi_device_id sprd_pmic_spi_ids[] = {
 	{ .name = "sc2730", .driver_data = (unsigned long)&sc2730_data },
 	{ .name = "sc2731", .driver_data = (unsigned long)&sc2731_data },
+	{ .name = "ump9620", .driver_data = (unsigned long)&ump9620_data },
+	{ .name = "ump9621", .driver_data = (unsigned long)&ump9621_data },
+	{ .name = "ump9622", .driver_data = (unsigned long)&ump9622_data },
 	{},
 };
 MODULE_DEVICE_TABLE(spi, sprd_pmic_spi_ids);
-- 
2.17.1

Re: [PATCH 2/2] mfd: sprd-sc27xx-spi: Add PMICs support for UMS9621 SoC

[Krzysztof Kozlowski]

On 22/08/2023 09:51, Jiansheng Wu wrote:
> There are three PMICs (UMP9620/21/22) on Unisoc's UMS9621 chip.
> UMP9620 is a master PMIC, the others are slave ones. Slave PMICs
> don't have irq functions, which is different from master device,
> such as SC27xx series and UMP9620, etc.
> 
> Signed-off-by: Jiansheng Wu <jiansheng.wu@unisoc.com>
> ---
>  drivers/mfd/sprd-sc27xx-spi.c | 104 +++++++++++++++++++++++++---------
>  1 file changed, 77 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/mfd/sprd-sc27xx-spi.c b/drivers/mfd/sprd-sc27xx-spi.c
> index d21f32cc784d..aa91301568a9 100644
> --- a/drivers/mfd/sprd-sc27xx-spi.c
> +++ b/drivers/mfd/sprd-sc27xx-spi.c
> @@ -24,6 +24,10 @@
>  #define SPRD_SC2731_IRQ_BASE		0x140
>  #define SPRD_SC2731_IRQ_NUMS		16
>  #define SPRD_SC2731_CHG_DET		0xedc
> +#define SPRD_UMP9620_IRQ_BASE           0x80
> +#define SPRD_UMP9620_IRQ_NUMS           11
> +#define SPRD_UMP9621_SLAVE_ID           0x8000
> +#define SPRD_UMP9622_SLAVE_ID           0xc000
>  
>  /* PMIC charger detection definition */
>  #define SPRD_PMIC_CHG_DET_DELAY_US	200000
> @@ -45,6 +49,7 @@ struct sprd_pmic {
>  };
>  
>  struct sprd_pmic_data {
> +	u32 slave_id;

See coding style about such wording. You know, it is not 2010 anymore...

>  	u32 irq_base;
>  	u32 num_irqs;
>  	u32 charger_det;
> @@ -67,6 +72,19 @@ static const struct sprd_pmic_data sc2731_data = {
>  	.charger_det = SPRD_SC2731_CHG_DET,
>  };
>  
> +static const struct sprd_pmic_data ump9620_data = {
> +	.irq_base = SPRD_UMP9620_IRQ_BASE,
> +	.num_irqs = SPRD_UMP9620_IRQ_NUMS,
> +};
> +
> +static const struct sprd_pmic_data ump9621_data = {
> +	.slave_id = SPRD_UMP9621_SLAVE_ID,
> +};
> +
> +static const struct sprd_pmic_data ump9622_data = {
> +	.slave_id = SPRD_UMP9622_SLAVE_ID,
> +};
> +
>  enum usb_charger_type sprd_pmic_detect_charger_type(struct device *dev)
>  {
>  	struct spi_device *spi = to_spi_device(dev);
> @@ -108,8 +126,27 @@ static int sprd_pmic_spi_write(void *context, const void *data, size_t count)
>  {
>  	struct device *dev = context;
>  	struct spi_device *spi = to_spi_device(dev);
> +	const struct sprd_pmic_data *pdata;
> +	int ret;
> +	u32 *pmdata;
> +
> +	if (!pdata->slave_id) {
> +		ret = spi_write(spi, data, count);
> +	} else {
> +		pdata = ((struct sprd_pmic *)spi_get_drvdata(spi))->pdata;
> +
> +		pmdata = kzalloc(count, GFP_KERNEL);
> +		if (!pmdata)
> +			return -ENOMEM;
> +		memcpy(pmdata, data, count);
> +		*pmdata += pdata->slave_id;
> +		ret = spi_write(spi, (const void *)pmdata, count);
> +		kfree(pmdata);
> +	}
> +	if (ret)
> +		pr_err("pmic mfd write failed!\n");
>  
> -	return spi_write(spi, data, count);
> +	return ret;
>  }
>  
>  static int sprd_pmic_spi_read(void *context,
> @@ -118,6 +155,7 @@ static int sprd_pmic_spi_read(void *context,
>  {
>  	struct device *dev = context;
>  	struct spi_device *spi = to_spi_device(dev);
> +	const struct sprd_pmic_data *pdata;
>  	u32 rx_buf[2] = { 0 };
>  	int ret;
>  
> @@ -125,11 +163,16 @@ static int sprd_pmic_spi_read(void *context,
>  	if (reg_size != sizeof(u32) || val_size != sizeof(u32))
>  		return -EINVAL;
>  
> +	pdata = ((struct sprd_pmic *)spi_get_drvdata(spi))->pdata;
>  	/* Copy address to read from into first element of SPI buffer. */
>  	memcpy(rx_buf, reg, sizeof(u32));
> +	if (!pdata->slave_id)
> +		rx_buf[0] += pdata->slave_id;
>  	ret = spi_read(spi, rx_buf, 1);
> -	if (ret < 0)
> +	if (ret < 0) {
> +		pr_err("pmic mfd read failed!\n");

Eh... drivers do not use pr_ but dev_. Which you can easily see within
this driver, so before posting changes please look at the driver and its
style, then learn from it and use similar coding convention.

>  		return ret;
> +	}
>  
>  	memcpy(val, rx_buf, val_size);
>  	return 0;

...


>  
>  	ret = devm_of_platform_populate(&spi->dev);
> @@ -240,6 +284,9 @@ static DEFINE_SIMPLE_DEV_PM_OPS(sprd_pmic_pm_ops,
>  static const struct of_device_id sprd_pmic_match[] = {
>  	{ .compatible = "sprd,sc2730", .data = &sc2730_data },
>  	{ .compatible = "sprd,sc2731", .data = &sc2731_data },
> +	{ .compatible = "sprd,ump9620", .data = &ump9620_data },
> +	{ .compatible = "sprd,ump9621", .data = &ump9621_data },
> +	{ .compatible = "sprd,ump9622", .data = &ump9622_data },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, sprd_pmic_match);
> @@ -247,6 +294,9 @@ MODULE_DEVICE_TABLE(of, sprd_pmic_match);
>  static const struct spi_device_id sprd_pmic_spi_ids[] = {
>  	{ .name = "sc2730", .driver_data = (unsigned long)&sc2730_data },
>  	{ .name = "sc2731", .driver_data = (unsigned long)&sc2731_data },
> +	{ .name = "ump9620", .driver_data = (unsigned long)&ump9620_data },
> +	{ .name = "ump9621", .driver_data = (unsigned long)&ump9621_data },
> +	{ .name = "ump9622", .driver_data = (unsigned long)&ump9622_data },

So here you sneaked new compatibles... Sorry, adding new compatibles is
not the same as converting old ones. Entirely separate patch.

Best regards,
Krzysztof

Re: [PATCH 2/2] mfd: sprd-sc27xx-spi: Add PMICs support for UMS9621 SoC

[kernel test robot]

Hi Jiansheng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on robh/for-next lee-leds/for-leds-next lee-mfd/for-mfd-fixes linus/master v6.5-rc7 next-20230822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiansheng-Wu/dt-bindings-spi-Convert-sprd-spi-bindings-to-yaml/20230822-155400
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:    https://lore.kernel.org/r/20230822075113.25506-3-jiansheng.wu%40unisoc.com
patch subject: [PATCH 2/2] mfd: sprd-sc27xx-spi: Add PMICs support for UMS9621 SoC
config: i386-buildonly-randconfig-004-20230822 (https://download.01.org/0day-ci/archive/20230823/202308230247.XxIH8PLy-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230823/202308230247.XxIH8PLy-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/202308230247.XxIH8PLy-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/mfd/sprd-sc27xx-spi.c: In function 'sprd_pmic_spi_write':
>> drivers/mfd/sprd-sc27xx-spi.c:133:12: warning: 'pdata' is used uninitialized in this function [-Wuninitialized]
     133 |  if (!pdata->slave_id) {
         |       ~~~~~^~~~~~~~~~


vim +/pdata +133 drivers/mfd/sprd-sc27xx-spi.c

   124	
   125	static int sprd_pmic_spi_write(void *context, const void *data, size_t count)
   126	{
   127		struct device *dev = context;
   128		struct spi_device *spi = to_spi_device(dev);
   129		const struct sprd_pmic_data *pdata;
   130		int ret;
   131		u32 *pmdata;
   132	
 > 133		if (!pdata->slave_id) {
   134			ret = spi_write(spi, data, count);
   135		} else {
   136			pdata = ((struct sprd_pmic *)spi_get_drvdata(spi))->pdata;
   137	
   138			pmdata = kzalloc(count, GFP_KERNEL);
   139			if (!pmdata)
   140				return -ENOMEM;
   141			memcpy(pmdata, data, count);
   142			*pmdata += pdata->slave_id;
   143			ret = spi_write(spi, (const void *)pmdata, count);
   144			kfree(pmdata);
   145		}
   146		if (ret)
   147			pr_err("pmic mfd write failed!\n");
   148	
   149		return ret;
   150	}
   151	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] mfd: sprd-sc27xx-spi: Add PMICs support for UMS9621 SoC

[kernel test robot]

Hi Jiansheng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on robh/for-next lee-leds/for-leds-next lee-mfd/for-mfd-fixes linus/master v6.5-rc7 next-20230822]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jiansheng-Wu/dt-bindings-spi-Convert-sprd-spi-bindings-to-yaml/20230822-155400
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link:    https://lore.kernel.org/r/20230822075113.25506-3-jiansheng.wu%40unisoc.com
patch subject: [PATCH 2/2] mfd: sprd-sc27xx-spi: Add PMICs support for UMS9621 SoC
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20230823/202308230504.UYqC6CLk-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230823/202308230504.UYqC6CLk-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/202308230504.UYqC6CLk-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/mfd/sprd-sc27xx-spi.c: In function 'sprd_pmic_spi_write':
>> drivers/mfd/sprd-sc27xx-spi.c:133:19: warning: 'pdata' is used uninitialized [-Wuninitialized]
     133 |         if (!pdata->slave_id) {
         |              ~~~~~^~~~~~~~~~
   drivers/mfd/sprd-sc27xx-spi.c:129:38: note: 'pdata' was declared here
     129 |         const struct sprd_pmic_data *pdata;
         |                                      ^~~~~


vim +/pdata +133 drivers/mfd/sprd-sc27xx-spi.c

   124	
   125	static int sprd_pmic_spi_write(void *context, const void *data, size_t count)
   126	{
   127		struct device *dev = context;
   128		struct spi_device *spi = to_spi_device(dev);
   129		const struct sprd_pmic_data *pdata;
   130		int ret;
   131		u32 *pmdata;
   132	
 > 133		if (!pdata->slave_id) {
   134			ret = spi_write(spi, data, count);
   135		} else {
   136			pdata = ((struct sprd_pmic *)spi_get_drvdata(spi))->pdata;
   137	
   138			pmdata = kzalloc(count, GFP_KERNEL);
   139			if (!pmdata)
   140				return -ENOMEM;
   141			memcpy(pmdata, data, count);
   142			*pmdata += pdata->slave_id;
   143			ret = spi_write(spi, (const void *)pmdata, count);
   144			kfree(pmdata);
   145		}
   146		if (ret)
   147			pr_err("pmic mfd write failed!\n");
   148	
   149		return ret;
   150	}
   151	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki