[PATCH v1 0/7] SPMI: Implement sub-devices and migrate drivers

[PATCH v1 0/7] SPMI: Implement sub-devices and migrate drivers

[AngeloGioacchino Del Regno]

While adding support for newer MediaTek platforms, featuring complex
SPMI PMICs, I've seen that those SPMI-connected chips are internally
divided in various IP blocks, reachable in specific contiguous address
ranges... more or less like a MMIO, but over a slow SPMI bus instead.

I recalled that Qualcomm had something similar... and upon checking a
couple of devicetrees, yeah - indeed it's the same over there.

What I've seen then is a common pattern of reading the "reg" property
from devicetree in a struct member and then either
 A. Wrapping regmap_{read/write/etc}() calls in a function that adds
    the register base with "base + ..register", like it's done with
    writel()/readl() calls; or
 B. Doing the same as A. but without wrapper functions.

Even though that works just fine, in my opinion it's wrong.

The regmap API is way more complex than MMIO-only readl()/writel()
functions for multiple reasons (including supporting multiple busses
like SPMI, of course) - but everyone seemed to forget that regmap
can manage register base offsets transparently and automatically in
its API functions by simply adding a `reg_base` to the regmap_config
structure, which is used for initializing a `struct regmap`.

So, here we go: this series implements the software concept of an SPMI
Sub-Device (which, well, also reflects how Qualcomm and MediaTek's
actual hardware is laid out anyway).

               SPMI Controller
                     |                ______
                     |               /       Sub-Device 1
                     V              /
              SPMI Device (PMIC) ----------- Sub-Device 2
                                    \
                                     \______ Sub-Device 3

As per this implementation, an SPMI Sub-Device can be allocated/created
and added in any driver that implements a... well.. subdevice (!) with
an SPMI "main" device as its parent: this allows to create and finally
to correctly configure a regmap that is specific to the sub-device,
operating on its specific address range and reading, and writing, to
its registers with the regmap API taking care of adding the base address
of a sub-device's registers as per regmap API design.

All of the SPMI Sub-Devices are therefore added as children of the SPMI
Device (usually a PMIC), as communication depends on the PMIC's SPMI bus
to be available (and the PMIC to be up and running, of course).

Summarizing the dependency chain (which is obvious to whoever knows what
is going on with Qualcomm and/or MediaTek SPMI PMICs):
    "SPMI Sub-Device x...N" are children "SPMI Device"
    "SPMI Device" is a child of "SPMI Controller"

(that was just another way to say the same thing as the graph above anyway).

Along with the new SPMI Sub-Device registration functions, I have also
performed a conversion of some Qualcomm SPMI drivers and only where the
actual conversion was trivial.

I haven't included any conversion of more complex Qualcomm SPMI drivers
because I don't have the required bandwidth to do so (and besides, I think,
but haven't exactly verified, that some of those require SoCs that I don't
have for testing anyway).

AngeloGioacchino Del Regno (7):
  spmi: Implement spmi_subdevice_alloc_and_add() and devm variant
  nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
  power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()
  phy: qualcomm: eusb2-repeater: Migrate to
    devm_spmi_subdevice_alloc_and_add()
  misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
  iio: adc: qcom-spmi-iadc: Migrate to
    devm_spmi_subdevice_alloc_and_add()
  iio: adc: qcom-spmi-iadc: Remove regmap R/W wrapper functions

 drivers/iio/adc/qcom-spmi-iadc.c              | 109 ++++++++----------
 drivers/misc/qcom-coincell.c                  |  37 ++++--
 drivers/nvmem/qcom-spmi-sdam.c                |  41 +++++--
 .../phy/qualcomm/phy-qcom-eusb2-repeater.c    |  45 +++++---
 drivers/power/reset/qcom-pon.c                |  33 ++++--
 drivers/spmi/spmi-devres.c                    |  23 ++++
 drivers/spmi/spmi.c                           |  75 ++++++++++++
 include/linux/spmi.h                          |  16 +++
 8 files changed, 272 insertions(+), 107 deletions(-)

-- 
2.50.1

[PATCH v1 1/7] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant

[AngeloGioacchino Del Regno]

Some devices connected over the SPMI bus may be big, in the sense
that those may be a complex of devices managed by a single chip
over the SPMI bus, reachable through a single SID.

Add new functions aimed at managing sub-devices of a SPMI device
spmi_subdevice_alloc_and_add() and a spmi_subdevice_put_and_remove()
for adding a new subdevice and removing it respectively, and also
add their devm_* variants.

The need for such functions comes from the existance of	those
complex Power Management ICs (PMICs), which feature one or many
sub-devices, in some cases with these being even addressable on
the chip in form of SPMI register ranges.

Examples of those devices can be found in both Qualcomm platforms
with their PMICs having PON, RTC, SDAM, GPIO controller, and other
sub-devices, and in newer MediaTek platforms showing similar HW
features and a similar layout with those also having many subdevs.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/spmi/spmi-devres.c | 23 ++++++++++++
 drivers/spmi/spmi.c        | 75 ++++++++++++++++++++++++++++++++++++++
 include/linux/spmi.h       | 16 ++++++++
 3 files changed, 114 insertions(+)

diff --git a/drivers/spmi/spmi-devres.c b/drivers/spmi/spmi-devres.c
index 62c4b3f24d06..9b850f651526 100644
--- a/drivers/spmi/spmi-devres.c
+++ b/drivers/spmi/spmi-devres.c
@@ -60,5 +60,28 @@ int devm_spmi_controller_add(struct device *parent, struct spmi_controller *ctrl
 }
 EXPORT_SYMBOL_GPL(devm_spmi_controller_add);
 
+static void devm_spmi_subdevice_remove(void *res)
+{
+	spmi_subdevice_remove((struct spmi_subdevice *)res);
+}
+
+struct spmi_subdevice *devm_spmi_subdevice_alloc_and_add(struct device *dev,
+							 struct spmi_device *sparent)
+{
+	struct spmi_subdevice *sub_sdev;
+	int ret;
+
+	sub_sdev = spmi_subdevice_alloc_and_add(sparent);
+	if (IS_ERR(sub_sdev))
+		return sub_sdev;
+
+	ret = devm_add_action_or_reset(dev, devm_spmi_subdevice_remove, sub_sdev);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return sub_sdev;
+}
+EXPORT_SYMBOL_GPL(devm_spmi_subdevice_alloc_and_add);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("SPMI devres helpers");
diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
index 3cf8d9bd4566..ce20d4fa702c 100644
--- a/drivers/spmi/spmi.c
+++ b/drivers/spmi/spmi.c
@@ -19,6 +19,7 @@
 
 static bool is_registered;
 static DEFINE_IDA(ctrl_ida);
+static DEFINE_IDA(spmi_subdevice_ida);
 
 static void spmi_dev_release(struct device *dev)
 {
@@ -31,6 +32,18 @@ static const struct device_type spmi_dev_type = {
 	.release	= spmi_dev_release,
 };
 
+static void spmi_subdev_release(struct device *dev)
+{
+	struct spmi_device *sdev = to_spmi_device(dev);
+	struct spmi_subdevice *sub_sdev = container_of(sdev, struct spmi_subdevice, sdev);
+
+	kfree(sub_sdev);
+}
+
+static const struct device_type spmi_subdev_type = {
+	.release	= spmi_subdev_release,
+};
+
 static void spmi_ctrl_release(struct device *dev)
 {
 	struct spmi_controller *ctrl = to_spmi_controller(dev);
@@ -90,6 +103,19 @@ void spmi_device_remove(struct spmi_device *sdev)
 }
 EXPORT_SYMBOL_GPL(spmi_device_remove);
 
+/**
+ * spmi_subdevice_remove() - Remove an SPMI subdevice
+ * @sub_sdev:	spmi_device to be removed
+ */
+void spmi_subdevice_remove(struct spmi_subdevice *sub_sdev)
+{
+	struct spmi_device *sdev = &sub_sdev->sdev;
+
+	device_unregister(&sdev->dev);
+	ida_free(&spmi_subdevice_ida, sub_sdev->devid);
+}
+EXPORT_SYMBOL_GPL(spmi_subdevice_remove);
+
 static inline int
 spmi_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid)
 {
@@ -431,6 +457,55 @@ struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl)
 }
 EXPORT_SYMBOL_GPL(spmi_device_alloc);
 
+/**
+ * spmi_subdevice_alloc_and_add(): Allocate and add a new SPMI sub-device
+ * @sparent:	SPMI parent device with previously registered SPMI controller
+ *
+ * Returns:
+ * Pointer to newly allocated SPMI sub-device for success or negative ERR_PTR.
+ */
+struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent)
+{
+	struct spmi_subdevice *sub_sdev;
+	struct spmi_device *sdev;
+	int ret;
+
+	if (!sparent)
+		return ERR_PTR(-EINVAL);
+
+	sub_sdev = kzalloc(sizeof(*sub_sdev), GFP_KERNEL);
+	if (!sub_sdev)
+		return ERR_PTR(-ENOMEM);
+
+	ret = ida_alloc(&spmi_subdevice_ida, GFP_KERNEL);
+	if (ret < 0) {
+		kfree(sub_sdev);
+		return ERR_PTR(ret);
+	}
+
+	sdev = &sub_sdev->sdev;
+	sdev->ctrl = sparent->ctrl;
+	device_initialize(&sdev->dev);
+	sdev->dev.parent = &sparent->dev;
+	sdev->dev.bus = &spmi_bus_type;
+	sdev->dev.type = &spmi_subdev_type;
+
+	sub_sdev->devid = ret;
+	sdev->usid = sparent->usid;
+	dev_set_name(&sdev->dev,
+		     "%d-%02x.%d.auto", sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
+	ret = device_add(&sdev->dev);
+	if (ret) {
+		dev_err(&sdev->dev, "Can't add %s, status %d\n",
+			dev_name(&sdev->dev), ret);
+		put_device(&sdev->dev);
+		return ERR_PTR(ret);
+	}
+
+	return sub_sdev;
+}
+EXPORT_SYMBOL_GPL(spmi_subdevice_alloc_and_add);
+
 /**
  * spmi_controller_alloc() - Allocate a new SPMI controller
  * @parent:	parent device
diff --git a/include/linux/spmi.h b/include/linux/spmi.h
index 28e8c8bd3944..7cea0a5b034b 100644
--- a/include/linux/spmi.h
+++ b/include/linux/spmi.h
@@ -69,6 +69,22 @@ int spmi_device_add(struct spmi_device *sdev);
 
 void spmi_device_remove(struct spmi_device *sdev);
 
+/**
+ * struct spmi_subdevice - Basic representation of an SPMI sub-device
+ * @sdev:	Sub-device representation of an SPMI device
+ * @devid:	Platform Device ID of an SPMI sub-device
+ */
+struct spmi_subdevice {
+	struct spmi_device	sdev;
+	unsigned int		devid;
+};
+
+struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent);
+void spmi_subdevice_remove(struct spmi_subdevice *sdev);
+
+struct spmi_subdevice *devm_spmi_subdevice_alloc_and_add(struct device *dev,
+							 struct spmi_device *sparent);
+
 /**
  * struct spmi_controller - interface to the SPMI master controller
  * @dev:	Driver model representation of the device.
-- 
2.50.1

[PATCH v1 2/7] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Some Qualcomm PMICs integrate a SDAM device, internally located in
a specific address range reachable through SPMI communication.

Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device for SDAM
and initialize its own regmap with this sub-device's specific base
address, retrieved from the devicetree.

This allows to stop manually adding the register base address to
every R/W call in this driver, as this can be, and is now, handled
by the regmap API instead.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/nvmem/qcom-spmi-sdam.c | 41 +++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
index 4f1cca6eab71..1b80e8563a33 100644
--- a/drivers/nvmem/qcom-spmi-sdam.c
+++ b/drivers/nvmem/qcom-spmi-sdam.c
@@ -9,6 +9,7 @@
 #include <linux/nvmem-provider.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/spmi.h>
 
 #define SDAM_MEM_START			0x40
 #define REGISTER_MAP_ID			0x40
@@ -20,7 +21,6 @@
 struct sdam_chip {
 	struct regmap			*regmap;
 	struct nvmem_config		sdam_config;
-	unsigned int			base;
 	unsigned int			size;
 };
 
@@ -73,7 +73,7 @@ static int sdam_read(void *priv, unsigned int offset, void *val,
 		return -EINVAL;
 	}
 
-	rc = regmap_bulk_read(sdam->regmap, sdam->base + offset, val, bytes);
+	rc = regmap_bulk_read(sdam->regmap, offset, val, bytes);
 	if (rc < 0)
 		dev_err(dev, "Failed to read SDAM offset %#x len=%zd, rc=%d\n",
 						offset, bytes, rc);
@@ -100,7 +100,7 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
 		return -EINVAL;
 	}
 
-	rc = regmap_bulk_write(sdam->regmap, sdam->base + offset, val, bytes);
+	rc = regmap_bulk_write(sdam->regmap, offset, val, bytes);
 	if (rc < 0)
 		dev_err(dev, "Failed to write SDAM offset %#x len=%zd, rc=%d\n",
 						offset, bytes, rc);
@@ -110,28 +110,47 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
 
 static int sdam_probe(struct platform_device *pdev)
 {
+	struct regmap_config sdam_regmap_config = {
+		.reg_bits = 16,
+		.val_bits = 16,
+		.max_register = 0x100,
+		.fast_io = true
+	};
 	struct sdam_chip *sdam;
 	struct nvmem_device *nvmem;
+	struct spmi_device *sparent;
+	struct spmi_subdevice *sub_sdev;
 	unsigned int val;
 	int rc;
 
+	if (!pdev->dev.parent)
+		return -ENODEV;
+
 	sdam = devm_kzalloc(&pdev->dev, sizeof(*sdam), GFP_KERNEL);
 	if (!sdam)
 		return -ENOMEM;
 
-	sdam->regmap = dev_get_regmap(pdev->dev.parent, NULL);
-	if (!sdam->regmap) {
-		dev_err(&pdev->dev, "Failed to get regmap handle\n");
-		return -ENXIO;
-	}
+	sparent = to_spmi_device(pdev->dev.parent);
+	if (!sparent)
+		return -ENODEV;
 
-	rc = of_property_read_u32(pdev->dev.of_node, "reg", &sdam->base);
+	sub_sdev = devm_spmi_subdevice_alloc_and_add(&pdev->dev, sparent);
+	if (IS_ERR(sub_sdev))
+		return PTR_ERR(sub_sdev);
+
+	rc = of_property_read_u32(pdev->dev.of_node, "reg", &sdam_regmap_config.reg_base);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "Failed to get SDAM base, rc=%d\n", rc);
 		return -EINVAL;
 	}
 
-	rc = regmap_read(sdam->regmap, sdam->base + SDAM_SIZE, &val);
+	sdam->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &sdam_regmap_config);
+	if (IS_ERR(sdam->regmap)) {
+		dev_err(&pdev->dev, "Failed to get regmap handle\n");
+		return PTR_ERR(sdam->regmap);
+	}
+
+	rc = regmap_read(sdam->regmap, SDAM_SIZE, &val);
 	if (rc < 0) {
 		dev_err(&pdev->dev, "Failed to read SDAM_SIZE rc=%d\n", rc);
 		return -EINVAL;
@@ -159,7 +178,7 @@ static int sdam_probe(struct platform_device *pdev)
 	}
 	dev_dbg(&pdev->dev,
 		"SDAM base=%#x size=%u registered successfully\n",
-		sdam->base, sdam->size);
+		sdam_regmap_config.reg_base, sdam->size);
 
 	return 0;
 }
-- 
2.50.1

[PATCH v1 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Some Qualcomm PMICs integrates a Power On device supporting pwrkey
and resin along with the Android reboot reason action identifier.

Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device for PON
and initialize its own regmap with this sub-device's specific base
address, retrieved from the devicetree.

This allows to stop manually adding the register base address to
every R/W call in this driver, as this can be, and is now, handled
by the regmap API instead.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/power/reset/qcom-pon.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
index 7e108982a582..bd7e7e04ff02 100644
--- a/drivers/power/reset/qcom-pon.c
+++ b/drivers/power/reset/qcom-pon.c
@@ -11,6 +11,7 @@
 #include <linux/reboot.h>
 #include <linux/reboot-mode.h>
 #include <linux/regmap.h>
+#include <linux/spmi.h>
 
 #define PON_SOFT_RB_SPARE		0x8f
 
@@ -22,7 +23,6 @@
 struct qcom_pon {
 	struct device *dev;
 	struct regmap *regmap;
-	u32 baseaddr;
 	struct reboot_mode_driver reboot_mode;
 	long reason_shift;
 };
@@ -35,7 +35,7 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
 	int ret;
 
 	ret = regmap_update_bits(pon->regmap,
-				 pon->baseaddr + PON_SOFT_RB_SPARE,
+				 PON_SOFT_RB_SPARE,
 				 GENMASK(7, pon->reason_shift),
 				 magic << pon->reason_shift);
 	if (ret < 0)
@@ -46,27 +46,46 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
 
 static int qcom_pon_probe(struct platform_device *pdev)
 {
+	struct regmap_config qcom_pon_regmap_config = {
+		.reg_bits = 16,
+		.val_bits = 16,
+		.max_register = 0x100,
+		.fast_io = true
+	};
+	struct spmi_subdevice *sub_sdev;
+	struct spmi_device *sparent;
 	struct qcom_pon *pon;
 	long reason_shift;
 	int error;
 
+	if (!pdev->dev.parent)
+		return -ENODEV;
+
 	pon = devm_kzalloc(&pdev->dev, sizeof(*pon), GFP_KERNEL);
 	if (!pon)
 		return -ENOMEM;
 
 	pon->dev = &pdev->dev;
 
-	pon->regmap = dev_get_regmap(pdev->dev.parent, NULL);
-	if (!pon->regmap) {
-		dev_err(&pdev->dev, "failed to locate regmap\n");
+	sparent = to_spmi_device(pdev->dev.parent);
+	if (!sparent)
 		return -ENODEV;
-	}
+
+	sub_sdev = devm_spmi_subdevice_alloc_and_add(&pdev->dev, sparent);
+	if (IS_ERR(sub_sdev))
+		return PTR_ERR(sub_sdev);
 
 	error = of_property_read_u32(pdev->dev.of_node, "reg",
-				     &pon->baseaddr);
+				     &qcom_pon_regmap_config.reg_base);
 	if (error)
 		return error;
 
+	pon->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &qcom_pon_regmap_config);
+	if (!pon->regmap) {
+		dev_err(&pdev->dev, "failed to locate regmap\n");
+		return -ENODEV;
+	}
+
 	reason_shift = (long)of_device_get_match_data(&pdev->dev);
 
 	if (reason_shift != NO_REASON_SHIFT) {
-- 
2.50.1

[PATCH v1 4/7] phy: qualcomm: eusb2-repeater: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Some Qualcomm PMICs integrate an USB Repeater device, used to
convert between eUSB2 and USB 2.0 signaling levels, reachable
in a specific address range over SPMI.

Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device for EUSB2
and initialize its own regmap with this sub-device's specific base
address, retrieved from the devicetree.

This allows to stop manually adding the register base address to
every R/W call in this driver, as this can be, and is now, handled
by the regmap API instead.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../phy/qualcomm/phy-qcom-eusb2-repeater.c    | 45 +++++++++++--------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
index e0f2acc8109c..f65437a85924 100644
--- a/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
+++ b/drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c
@@ -9,6 +9,7 @@
 #include <linux/regmap.h>
 #include <linux/of.h>
 #include <linux/phy/phy.h>
+#include <linux/spmi.h>
 
 /* eUSB2 status registers */
 #define EUSB2_RPTR_STATUS		0x08
@@ -55,7 +56,6 @@ struct eusb2_repeater {
 	struct phy *phy;
 	struct regulator_bulk_data *vregs;
 	const struct eusb2_repeater_cfg *cfg;
-	u32 base;
 	enum phy_mode mode;
 };
 
@@ -110,7 +110,6 @@ static int eusb2_repeater_init(struct phy *phy)
 	struct eusb2_repeater *rptr = phy_get_drvdata(phy);
 	struct device_node *np = rptr->dev->of_node;
 	struct regmap *regmap = rptr->regmap;
-	u32 base = rptr->base;
 	u32 poll_val;
 	int ret;
 	u8 val;
@@ -119,25 +118,25 @@ static int eusb2_repeater_init(struct phy *phy)
 	if (ret)
 		return ret;
 
-	regmap_write(regmap, base + EUSB2_EN_CTL1, EUSB2_RPTR_EN);
+	regmap_write(regmap, EUSB2_EN_CTL1, EUSB2_RPTR_EN);
 
 	/* Write registers from init table */
 	for (int i = 0; i < rptr->cfg->init_tbl_num; i++)
-		regmap_write(regmap, base + rptr->cfg->init_tbl[i].reg,
+		regmap_write(regmap, rptr->cfg->init_tbl[i].reg,
 			     rptr->cfg->init_tbl[i].value);
 
 	/* Override registers from devicetree values */
 	if (!of_property_read_u8(np, "qcom,tune-usb2-amplitude", &val))
-		regmap_write(regmap, base + EUSB2_TUNE_USB2_PREEM, val);
+		regmap_write(regmap, EUSB2_TUNE_USB2_PREEM, val);
 
 	if (!of_property_read_u8(np, "qcom,tune-usb2-disc-thres", &val))
-		regmap_write(regmap, base + EUSB2_TUNE_HSDISC, val);
+		regmap_write(regmap, EUSB2_TUNE_HSDISC, val);
 
 	if (!of_property_read_u8(np, "qcom,tune-usb2-preem", &val))
-		regmap_write(regmap, base + EUSB2_TUNE_IUSB2, val);
+		regmap_write(regmap, EUSB2_TUNE_IUSB2, val);
 
 	/* Wait for status OK */
-	ret = regmap_read_poll_timeout(regmap, base + EUSB2_RPTR_STATUS, poll_val,
+	ret = regmap_read_poll_timeout(regmap, EUSB2_RPTR_STATUS, poll_val,
 				       poll_val & RPTR_OK, 10, 5);
 	if (ret)
 		dev_err(rptr->dev, "initialization timed-out\n");
@@ -150,7 +149,6 @@ static int eusb2_repeater_set_mode(struct phy *phy,
 {
 	struct eusb2_repeater *rptr = phy_get_drvdata(phy);
 	struct regmap *regmap = rptr->regmap;
-	u32 base = rptr->base;
 
 	switch (mode) {
 	case PHY_MODE_USB_HOST:
@@ -159,8 +157,8 @@ static int eusb2_repeater_set_mode(struct phy *phy,
 		 * per eUSB 1.2 Spec. Below implement software workaround until
 		 * PHY and controller is fixing seen observation.
 		 */
-		regmap_write(regmap, base + EUSB2_FORCE_EN_5, F_CLK_19P2M_EN);
-		regmap_write(regmap, base + EUSB2_FORCE_VAL_5, V_CLK_19P2M_EN);
+		regmap_write(regmap, EUSB2_FORCE_EN_5, F_CLK_19P2M_EN);
+		regmap_write(regmap, EUSB2_FORCE_VAL_5, V_CLK_19P2M_EN);
 		break;
 	case PHY_MODE_USB_DEVICE:
 		/*
@@ -169,8 +167,8 @@ static int eusb2_repeater_set_mode(struct phy *phy,
 		 * repeater doesn't clear previous value due to shared
 		 * regulators (say host <-> device mode switch).
 		 */
-		regmap_write(regmap, base + EUSB2_FORCE_EN_5, 0);
-		regmap_write(regmap, base + EUSB2_FORCE_VAL_5, 0);
+		regmap_write(regmap, EUSB2_FORCE_EN_5, 0);
+		regmap_write(regmap, EUSB2_FORCE_VAL_5, 0);
 		break;
 	default:
 		return -EINVAL;
@@ -195,11 +193,18 @@ static const struct phy_ops eusb2_repeater_ops = {
 
 static int eusb2_repeater_probe(struct platform_device *pdev)
 {
+	struct regmap_config eusb2_regmap_config = {
+		.reg_bits = 16,
+		.val_bits = 16,
+		.max_register = 0x100,
+		.fast_io = true
+	};
+	struct spmi_device *sparent;
 	struct eusb2_repeater *rptr;
+	struct spmi_subdevice *sub_sdev;
 	struct device *dev = &pdev->dev;
 	struct phy_provider *phy_provider;
 	struct device_node *np = dev->of_node;
-	u32 res;
 	int ret;
 
 	rptr = devm_kzalloc(dev, sizeof(*rptr), GFP_KERNEL);
@@ -213,15 +218,17 @@ static int eusb2_repeater_probe(struct platform_device *pdev)
 	if (!rptr->cfg)
 		return -EINVAL;
 
-	rptr->regmap = dev_get_regmap(dev->parent, NULL);
-	if (!rptr->regmap)
-		return -ENODEV;
+	sub_sdev = devm_spmi_subdevice_alloc_and_add(&pdev->dev, sparent);
+	if (IS_ERR(sub_sdev))
+		return PTR_ERR(sub_sdev);
 
-	ret = of_property_read_u32(np, "reg", &res);
+	ret = of_property_read_u32(np, "reg", &eusb2_regmap_config.reg_base);
 	if (ret < 0)
 		return ret;
 
-	rptr->base = res;
+	rptr->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &eusb2_regmap_config);
+	if (IS_ERR(rptr->regmap))
+		return -ENODEV;
 
 	ret = eusb2_repeater_init_vregs(rptr);
 	if (ret < 0) {
-- 
2.50.1

[PATCH v1 5/7] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Some Qualcomm PMICs integrate a charger for coincells, usually
powering an RTC when external (or main battery) power is missing.

Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device and
initialize its own regmap with this sub-device's specific base
address, retrieved from the devicetree.

This allows to stop manually adding the register base address to
every R/W call in this driver, as this can be, and is now, handled
by the regmap API instead.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/misc/qcom-coincell.c | 37 ++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
index 3c57f7429147..2a4788ed9e4f 100644
--- a/drivers/misc/qcom-coincell.c
+++ b/drivers/misc/qcom-coincell.c
@@ -9,11 +9,11 @@
 #include <linux/of.h>
 #include <linux/regmap.h>
 #include <linux/platform_device.h>
+#include <linux/spmi.h>
 
 struct qcom_coincell {
 	struct device	*dev;
 	struct regmap	*regmap;
-	u32		base_addr;
 };
 
 #define QCOM_COINCELL_REG_RSET		0x44
@@ -35,7 +35,7 @@ static int qcom_coincell_chgr_config(struct qcom_coincell *chgr, int rset,
 	/* if disabling, just do that and skip other operations */
 	if (!enable)
 		return regmap_write(chgr->regmap,
-			  chgr->base_addr + QCOM_COINCELL_REG_ENABLE, 0);
+			  QCOM_COINCELL_REG_ENABLE, 0);
 
 	/* find index for current-limiting resistor */
 	for (i = 0; i < ARRAY_SIZE(qcom_rset_map); i++)
@@ -58,7 +58,7 @@ static int qcom_coincell_chgr_config(struct qcom_coincell *chgr, int rset,
 	}
 
 	rc = regmap_write(chgr->regmap,
-			  chgr->base_addr + QCOM_COINCELL_REG_RSET, i);
+			  QCOM_COINCELL_REG_RSET, i);
 	if (rc) {
 		/*
 		 * This is mainly to flag a bad base_addr (reg) from dts.
@@ -71,19 +71,27 @@ static int qcom_coincell_chgr_config(struct qcom_coincell *chgr, int rset,
 	}
 
 	rc = regmap_write(chgr->regmap,
-		chgr->base_addr + QCOM_COINCELL_REG_VSET, j);
+		QCOM_COINCELL_REG_VSET, j);
 	if (rc)
 		return rc;
 
 	/* set 'enable' register */
 	return regmap_write(chgr->regmap,
-			    chgr->base_addr + QCOM_COINCELL_REG_ENABLE,
+			    QCOM_COINCELL_REG_ENABLE,
 			    QCOM_COINCELL_ENABLE);
 }
 
 static int qcom_coincell_probe(struct platform_device *pdev)
 {
+	struct regmap_config qcom_coincell_regmap_config = {
+		.reg_bits = 16,
+		.val_bits = 16,
+		.max_register = 0x100,
+		.fast_io = true
+	};
 	struct device_node *node = pdev->dev.of_node;
+	struct spmi_subdevice *sub_sdev;
+	struct spmi_device *sparent;
 	struct qcom_coincell chgr;
 	u32 rset = 0;
 	u32 vset = 0;
@@ -92,16 +100,25 @@ static int qcom_coincell_probe(struct platform_device *pdev)
 
 	chgr.dev = &pdev->dev;
 
-	chgr.regmap = dev_get_regmap(pdev->dev.parent, NULL);
+	rc = of_property_read_u32(node, "reg", &qcom_coincell_regmap_config.reg_base);
+	if (rc)
+		return rc;
+
+	sparent = to_spmi_device(pdev->dev.parent);
+	if (!sparent)
+		return -ENODEV;
+
+	sub_sdev = devm_spmi_subdevice_alloc_and_add(&pdev->dev, sparent);
+	if (IS_ERR(sub_sdev))
+		return PTR_ERR(sub_sdev);
+
+	chgr.regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev,
+						&qcom_coincell_regmap_config);
 	if (!chgr.regmap) {
 		dev_err(chgr.dev, "Unable to get regmap\n");
 		return -EINVAL;
 	}
 
-	rc = of_property_read_u32(node, "reg", &chgr.base_addr);
-	if (rc)
-		return rc;
-
 	enable = !of_property_read_bool(node, "qcom,charger-disable");
 
 	if (enable) {
-- 
2.50.1

[PATCH v1 6/7] iio: adc: qcom-spmi-iadc: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Some Qualcomm PMICs integrate an Current ADC device, reachable
in a specific address range over SPMI.

Instead of using the parent SPMI device (the main PMIC) as a kind
of syscon in this driver, register a new SPMI sub-device and
initialize its own regmap with this sub-device's specific base
address, retrieved from the devicetree.

This allows to stop manually adding the register base address to
every R/W call in this driver, as this can be, and is now, handled
by the regmap API instead.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/iio/adc/qcom-spmi-iadc.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
index b64a8a407168..34e31bb49f4f 100644
--- a/drivers/iio/adc/qcom-spmi-iadc.c
+++ b/drivers/iio/adc/qcom-spmi-iadc.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/spmi.h>
 
 /* IADC register and bit definition */
 #define IADC_REVISION2				0x1
@@ -94,7 +95,6 @@
  * struct iadc_chip - IADC Current ADC device structure.
  * @regmap: regmap for register read/write.
  * @dev: This device pointer.
- * @base: base offset for the ADC peripheral.
  * @rsense: Values of the internal and external sense resister in micro Ohms.
  * @poll_eoc: Poll for end of conversion instead of waiting for IRQ.
  * @offset: Raw offset values for the internal and external channels.
@@ -105,7 +105,6 @@
 struct iadc_chip {
 	struct regmap	*regmap;
 	struct device	*dev;
-	u16		base;
 	bool		poll_eoc;
 	u32		rsense[2];
 	u16		offset[2];
@@ -119,7 +118,7 @@ static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data)
 	unsigned int val;
 	int ret;
 
-	ret = regmap_read(iadc->regmap, iadc->base + offset, &val);
+	ret = regmap_read(iadc->regmap, offset, &val);
 	if (ret < 0)
 		return ret;
 
@@ -129,7 +128,7 @@ static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data)
 
 static int iadc_write(struct iadc_chip *iadc, u16 offset, u8 data)
 {
-	return regmap_write(iadc->regmap, iadc->base + offset, data);
+	return regmap_write(iadc->regmap, offset, data);
 }
 
 static int iadc_reset(struct iadc_chip *iadc)
@@ -270,7 +269,7 @@ static int iadc_poll_wait_eoc(struct iadc_chip *iadc, unsigned int interval_us)
 
 static int iadc_read_result(struct iadc_chip *iadc, u16 *data)
 {
-	return regmap_bulk_read(iadc->regmap, iadc->base + IADC_DATA, data, 2);
+	return regmap_bulk_read(iadc->regmap, IADC_DATA, data, 2);
 }
 
 static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data)
@@ -483,12 +482,19 @@ static const struct iio_chan_spec iadc_channels[] = {
 
 static int iadc_probe(struct platform_device *pdev)
 {
+	struct regmap_config iadc_regmap_config = {
+		.reg_bits = 16,
+		.val_bits = 16,
+		.max_register = 0x100,
+		.fast_io = true
+	};
 	struct device_node *node = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
+	struct spmi_subdevice *sub_sdev;
+	struct spmi_device *sparent;
 	struct iio_dev *indio_dev;
 	struct iadc_chip *iadc;
 	int ret, irq_eoc;
-	u32 res;
 
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*iadc));
 	if (!indio_dev)
@@ -497,18 +503,24 @@ static int iadc_probe(struct platform_device *pdev)
 	iadc = iio_priv(indio_dev);
 	iadc->dev = dev;
 
-	iadc->regmap = dev_get_regmap(dev->parent, NULL);
-	if (!iadc->regmap)
+	sparent = to_spmi_device(pdev->dev.parent);
+	if (!sparent)
 		return -ENODEV;
 
+	sub_sdev = devm_spmi_subdevice_alloc_and_add(&pdev->dev, sparent);
+	if (IS_ERR(sub_sdev))
+		return PTR_ERR(sub_sdev);
+
 	init_completion(&iadc->complete);
 	mutex_init(&iadc->lock);
 
-	ret = of_property_read_u32(node, "reg", &res);
+	ret = of_property_read_u32(node, "reg", &iadc_regmap_config.reg_base);
 	if (ret < 0)
 		return -ENODEV;
 
-	iadc->base = res;
+	iadc->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &iadc_regmap_config);
+	if (IS_ERR(iadc->regmap))
+		return PTR_ERR(iadc->regmap);
 
 	ret = iadc_version_check(iadc);
 	if (ret < 0)
-- 
2.50.1

[PATCH v1 7/7] iio: adc: qcom-spmi-iadc: Remove regmap R/W wrapper functions

[AngeloGioacchino Del Regno]

This driver doesn't need to add any register base address to any
regmap call anymore since it was migrated to register as a SPMI
subdevice with its own regmap reg_base, which makes the regmap
API to automatically add such base address internally.

Since the iadc_{read,write,read_result}() functions now only do
call regmap_{read,write,bulk_read}() and nothing else, simplify
the driver by removing them and by calling regmap APIs directly.

Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/iio/adc/qcom-spmi-iadc.c | 83 ++++++++++++--------------------
 1 file changed, 30 insertions(+), 53 deletions(-)

diff --git a/drivers/iio/adc/qcom-spmi-iadc.c b/drivers/iio/adc/qcom-spmi-iadc.c
index 34e31bb49f4f..111027ae033e 100644
--- a/drivers/iio/adc/qcom-spmi-iadc.c
+++ b/drivers/iio/adc/qcom-spmi-iadc.c
@@ -113,77 +113,59 @@ struct iadc_chip {
 	struct completion complete;
 };
 
-static int iadc_read(struct iadc_chip *iadc, u16 offset, u8 *data)
-{
-	unsigned int val;
-	int ret;
-
-	ret = regmap_read(iadc->regmap, offset, &val);
-	if (ret < 0)
-		return ret;
-
-	*data = val;
-	return 0;
-}
-
-static int iadc_write(struct iadc_chip *iadc, u16 offset, u8 data)
-{
-	return regmap_write(iadc->regmap, offset, data);
-}
-
 static int iadc_reset(struct iadc_chip *iadc)
 {
-	u8 data;
+	u32 data;
 	int ret;
 
-	ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA);
+	ret = regmap_write(iadc->regmap, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA);
 	if (ret < 0)
 		return ret;
 
-	ret = iadc_read(iadc, IADC_PERH_RESET_CTL3, &data);
+	ret = regmap_read(iadc->regmap, IADC_PERH_RESET_CTL3, &data);
 	if (ret < 0)
 		return ret;
 
-	ret = iadc_write(iadc, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA);
+	ret = regmap_write(iadc->regmap, IADC_SEC_ACCESS, IADC_SEC_ACCESS_DATA);
 	if (ret < 0)
 		return ret;
 
 	data |= IADC_FOLLOW_WARM_RB;
 
-	return iadc_write(iadc, IADC_PERH_RESET_CTL3, data);
+	return regmap_write(iadc->regmap, IADC_PERH_RESET_CTL3, data);
 }
 
 static int iadc_set_state(struct iadc_chip *iadc, bool state)
 {
-	return iadc_write(iadc, IADC_EN_CTL1, state ? IADC_EN_CTL1_SET : 0);
+	return regmap_write(iadc->regmap, IADC_EN_CTL1, state ? IADC_EN_CTL1_SET : 0);
 }
 
 static void iadc_status_show(struct iadc_chip *iadc)
 {
-	u8 mode, sta1, chan, dig, en, req;
+	u32 mode, sta1, chan, dig, en, req;
 	int ret;
 
-	ret = iadc_read(iadc, IADC_MODE_CTL, &mode);
+	ret = regmap_read(iadc->regmap, IADC_MODE_CTL, &mode);
 	if (ret < 0)
 		return;
 
-	ret = iadc_read(iadc, IADC_DIG_PARAM, &dig);
+	ret = regmap_read(iadc->regmap, IADC_DIG_PARAM, &dig);
 	if (ret < 0)
 		return;
 
-	ret = iadc_read(iadc, IADC_CH_SEL_CTL, &chan);
+	ret = regmap_read(iadc->regmap, IADC_CH_SEL_CTL, &chan);
 	if (ret < 0)
 		return;
 
-	ret = iadc_read(iadc, IADC_CONV_REQ, &req);
+	ret = regmap_read(iadc->regmap, IADC_CONV_REQ, &req);
 	if (ret < 0)
 		return;
 
-	ret = iadc_read(iadc, IADC_STATUS1, &sta1);
+	ret = regmap_read(iadc->regmap, IADC_STATUS1, &sta1);
 	if (ret < 0)
 		return;
 
-	ret = iadc_read(iadc, IADC_EN_CTL1, &en);
+	ret = regmap_read(iadc->regmap, IADC_EN_CTL1, &en);
 	if (ret < 0)
 		return;
 
@@ -199,34 +181,34 @@ static int iadc_configure(struct iadc_chip *iadc, int channel)
 
 	/* Mode selection */
 	mode = (IADC_OP_MODE_NORMAL << IADC_OP_MODE_SHIFT) | IADC_TRIM_EN;
-	ret = iadc_write(iadc, IADC_MODE_CTL, mode);
+	ret = regmap_write(iadc->regmap, IADC_MODE_CTL, mode);
 	if (ret < 0)
 		return ret;
 
 	/* Channel selection */
-	ret = iadc_write(iadc, IADC_CH_SEL_CTL, channel);
+	ret = regmap_write(iadc->regmap, IADC_CH_SEL_CTL, channel);
 	if (ret < 0)
 		return ret;
 
 	/* Digital parameter setup */
 	decim = IADC_DEF_DECIMATION << IADC_DIG_DEC_RATIO_SEL_SHIFT;
-	ret = iadc_write(iadc, IADC_DIG_PARAM, decim);
+	ret = regmap_write(iadc->regmap, IADC_DIG_PARAM, decim);
 	if (ret < 0)
 		return ret;
 
 	/* HW settle time delay */
-	ret = iadc_write(iadc, IADC_HW_SETTLE_DELAY, IADC_DEF_HW_SETTLE_TIME);
+	ret = regmap_write(iadc->regmap, IADC_HW_SETTLE_DELAY, IADC_DEF_HW_SETTLE_TIME);
 	if (ret < 0)
 		return ret;
 
-	ret = iadc_write(iadc, IADC_FAST_AVG_CTL, IADC_DEF_AVG_SAMPLES);
+	ret = regmap_write(iadc->regmap, IADC_FAST_AVG_CTL, IADC_DEF_AVG_SAMPLES);
 	if (ret < 0)
 		return ret;
 
 	if (IADC_DEF_AVG_SAMPLES)
-		ret = iadc_write(iadc, IADC_FAST_AVG_EN, IADC_FAST_AVG_EN_SET);
+		ret = regmap_write(iadc->regmap, IADC_FAST_AVG_EN, IADC_FAST_AVG_EN_SET);
 	else
-		ret = iadc_write(iadc, IADC_FAST_AVG_EN, 0);
+		ret = regmap_write(iadc->regmap, IADC_FAST_AVG_EN, 0);
 
 	if (ret < 0)
 		return ret;
@@ -239,19 +221,19 @@ static int iadc_configure(struct iadc_chip *iadc, int channel)
 		return ret;
 
 	/* Request conversion */
-	return iadc_write(iadc, IADC_CONV_REQ, IADC_CONV_REQ_SET);
+	return regmap_write(iadc->regmap, IADC_CONV_REQ, IADC_CONV_REQ_SET);
 }
 
 static int iadc_poll_wait_eoc(struct iadc_chip *iadc, unsigned int interval_us)
 {
 	unsigned int count, retry;
 	int ret;
-	u8 sta1;
+	u32 sta1;
 
 	retry = interval_us / IADC_CONV_TIME_MIN_US;
 
 	for (count = 0; count < retry; count++) {
-		ret = iadc_read(iadc, IADC_STATUS1, &sta1);
+		ret = regmap_read(iadc->regmap, IADC_STATUS1, &sta1);
 		if (ret < 0)
 			return ret;
 
@@ -267,11 +249,6 @@ static int iadc_poll_wait_eoc(struct iadc_chip *iadc, unsigned int interval_us)
 	return -ETIMEDOUT;
 }
 
-static int iadc_read_result(struct iadc_chip *iadc, u16 *data)
-{
-	return regmap_bulk_read(iadc->regmap, IADC_DATA, data, 2);
-}
-
 static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data)
 {
 	unsigned int wait;
@@ -296,7 +273,7 @@ static int iadc_do_conversion(struct iadc_chip *iadc, int chan, u16 *data)
 	}
 
 	if (!ret)
-		ret = iadc_read_result(iadc, data);
+		ret = regmap_bulk_read(iadc->regmap, IADC_DATA, data, 2);
 exit:
 	iadc_set_state(iadc, false);
 	if (ret < 0)
@@ -392,10 +369,10 @@ static int iadc_update_offset(struct iadc_chip *iadc)
 
 static int iadc_version_check(struct iadc_chip *iadc)
 {
-	u8 val;
+	u32 val;
 	int ret;
 
-	ret = iadc_read(iadc, IADC_PERPH_TYPE, &val);
+	ret = regmap_read(iadc->regmap, IADC_PERPH_TYPE, &val);
 	if (ret < 0)
 		return ret;
 
@@ -404,7 +381,7 @@ static int iadc_version_check(struct iadc_chip *iadc)
 		return -EINVAL;
 	}
 
-	ret = iadc_read(iadc, IADC_PERPH_SUBTYPE, &val);
+	ret = regmap_read(iadc->regmap, IADC_PERPH_SUBTYPE, &val);
 	if (ret < 0)
 		return ret;
 
@@ -413,7 +390,7 @@ static int iadc_version_check(struct iadc_chip *iadc)
 		return -EINVAL;
 	}
 
-	ret = iadc_read(iadc, IADC_REVISION2, &val);
+	ret = regmap_read(iadc->regmap, IADC_REVISION2, &val);
 	if (ret < 0)
 		return ret;
 
@@ -428,7 +405,7 @@ static int iadc_version_check(struct iadc_chip *iadc)
 static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node)
 {
 	int ret, sign, int_sense;
-	u8 deviation;
+	u32 deviation;
 
 	ret = of_property_read_u32(node, "qcom,external-resistor-micro-ohms",
 				   &iadc->rsense[IADC_EXT_RSENSE]);
@@ -440,7 +417,7 @@ static int iadc_rsense_read(struct iadc_chip *iadc, struct device_node *node)
 		return -EINVAL;
 	}
 
-	ret = iadc_read(iadc, IADC_NOMINAL_RSENSE, &deviation);
+	ret = regmap_read(iadc->regmap, IADC_NOMINAL_RSENSE, &deviation);
 	if (ret < 0)
 		return ret;
 
-- 
2.50.1

Re: [PATCH v1 5/7] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()

[Greg KH]

On Mon, Jul 21, 2025 at 09:55:23AM +0200, AngeloGioacchino Del Regno wrote:
> Some Qualcomm PMICs integrate a charger for coincells, usually
> powering an RTC when external (or main battery) power is missing.
> 
> Instead of using the parent SPMI device (the main PMIC) as a kind
> of syscon in this driver, register a new SPMI sub-device and
> initialize its own regmap with this sub-device's specific base
> address, retrieved from the devicetree.
> 
> This allows to stop manually adding the register base address to
> every R/W call in this driver, as this can be, and is now, handled
> by the regmap API instead.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/misc/qcom-coincell.c | 37 ++++++++++++++++++++++++++----------
>  1 file changed, 27 insertions(+), 10 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v1 2/7] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

[Casey Connolly]

Hi Angelo,

On 21/07/2025 09:55, AngeloGioacchino Del Regno wrote:
> Some Qualcomm PMICs integrate a SDAM device, internally located in
> a specific address range reachable through SPMI communication.
> 
> Instead of using the parent SPMI device (the main PMIC) as a kind
> of syscon in this driver, register a new SPMI sub-device for SDAM
> and initialize its own regmap with this sub-device's specific base
> address, retrieved from the devicetree.
> 
> This allows to stop manually adding the register base address to
> every R/W call in this driver, as this can be, and is now, handled
> by the regmap API instead.

This is honestly a really nice improvement :D>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
>  drivers/nvmem/qcom-spmi-sdam.c | 41 +++++++++++++++++++++++++---------
>  1 file changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
> index 4f1cca6eab71..1b80e8563a33 100644
> --- a/drivers/nvmem/qcom-spmi-sdam.c
> +++ b/drivers/nvmem/qcom-spmi-sdam.c
> @@ -9,6 +9,7 @@
>  #include <linux/nvmem-provider.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> +#include <linux/spmi.h>
>  
>  #define SDAM_MEM_START			0x40
>  #define REGISTER_MAP_ID			0x40
> @@ -20,7 +21,6 @@
>  struct sdam_chip {
>  	struct regmap			*regmap;
>  	struct nvmem_config		sdam_config;
> -	unsigned int			base;
>  	unsigned int			size;
>  };
>  
> @@ -73,7 +73,7 @@ static int sdam_read(void *priv, unsigned int offset, void *val,
>  		return -EINVAL;
>  	}
>  
> -	rc = regmap_bulk_read(sdam->regmap, sdam->base + offset, val, bytes);
> +	rc = regmap_bulk_read(sdam->regmap, offset, val, bytes);
>  	if (rc < 0)
>  		dev_err(dev, "Failed to read SDAM offset %#x len=%zd, rc=%d\n",
>  						offset, bytes, rc);
> @@ -100,7 +100,7 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>  		return -EINVAL;
>  	}
>  
> -	rc = regmap_bulk_write(sdam->regmap, sdam->base + offset, val, bytes);
> +	rc = regmap_bulk_write(sdam->regmap, offset, val, bytes);
>  	if (rc < 0)
>  		dev_err(dev, "Failed to write SDAM offset %#x len=%zd, rc=%d\n",
>  						offset, bytes, rc);
> @@ -110,28 +110,47 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>  
>  static int sdam_probe(struct platform_device *pdev)
>  {
> +	struct regmap_config sdam_regmap_config = {
> +		.reg_bits = 16,
> +		.val_bits = 16,

I believe registers are 8 bits wide, at least on Qualcomm platforms.

Kind regards,
-- 
// Casey (she/her)

Re: [PATCH v1 5/7] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()

[kernel test robot]

Hi AngeloGioacchino,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20250718]
[cannot apply to jic23-iio/togreg sre-power-supply/for-next char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.16-rc7 v6.16-rc6 v6.16-rc5 v6.16-rc7]
[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/AngeloGioacchino-Del-Regno/spmi-Implement-spmi_subdevice_alloc_and_add-and-devm-variant/20250721-155809
base:   next-20250718
patch link:    https://lore.kernel.org/r/20250721075525.29636-6-angelogioacchino.delregno%40collabora.com
patch subject: [PATCH v1 5/7] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
config: x86_64-buildonly-randconfig-003-20250721 (https://download.01.org/0day-ci/archive/20250721/202507211953.9ai6l420-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250721/202507211953.9ai6l420-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/202507211953.9ai6l420-lkp@intel.com/

All errors (new ones prefixed by >>):

   ld: vmlinux.o: in function `qcom_coincell_probe':
>> drivers/misc/qcom-coincell.c:111: undefined reference to `devm_spmi_subdevice_alloc_and_add'
>> ld: drivers/misc/qcom-coincell.c:115: undefined reference to `__devm_regmap_init_spmi_ext'


vim +111 drivers/misc/qcom-coincell.c

    83	
    84	static int qcom_coincell_probe(struct platform_device *pdev)
    85	{
    86		struct regmap_config qcom_coincell_regmap_config = {
    87			.reg_bits = 16,
    88			.val_bits = 16,
    89			.max_register = 0x100,
    90			.fast_io = true
    91		};
    92		struct device_node *node = pdev->dev.of_node;
    93		struct spmi_subdevice *sub_sdev;
    94		struct spmi_device *sparent;
    95		struct qcom_coincell chgr;
    96		u32 rset = 0;
    97		u32 vset = 0;
    98		bool enable;
    99		int rc;
   100	
   101		chgr.dev = &pdev->dev;
   102	
   103		rc = of_property_read_u32(node, "reg", &qcom_coincell_regmap_config.reg_base);
   104		if (rc)
   105			return rc;
   106	
   107		sparent = to_spmi_device(pdev->dev.parent);
   108		if (!sparent)
   109			return -ENODEV;
   110	
 > 111		sub_sdev = devm_spmi_subdevice_alloc_and_add(&pdev->dev, sparent);
   112		if (IS_ERR(sub_sdev))
   113			return PTR_ERR(sub_sdev);
   114	
 > 115		chgr.regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev,
   116							&qcom_coincell_regmap_config);
   117		if (!chgr.regmap) {
   118			dev_err(chgr.dev, "Unable to get regmap\n");
   119			return -EINVAL;
   120		}
   121	
   122		enable = !of_property_read_bool(node, "qcom,charger-disable");
   123	
   124		if (enable) {
   125			rc = of_property_read_u32(node, "qcom,rset-ohms", &rset);
   126			if (rc) {
   127				dev_err(chgr.dev,
   128					"can't find 'qcom,rset-ohms' in DT block");
   129				return rc;
   130			}
   131	
   132			rc = of_property_read_u32(node, "qcom,vset-millivolts", &vset);
   133			if (rc) {
   134				dev_err(chgr.dev,
   135				    "can't find 'qcom,vset-millivolts' in DT block");
   136				return rc;
   137			}
   138		}
   139	
   140		return qcom_coincell_chgr_config(&chgr, rset, vset, enable);
   141	}
   142	

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

Re: [PATCH v1 1/7] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant

[Andy Shevchenko]

On Mon, Jul 21, 2025 at 09:55:19AM +0200, AngeloGioacchino Del Regno wrote:
> Some devices connected over the SPMI bus may be big, in the sense
> that those may be a complex of devices managed by a single chip
> over the SPMI bus, reachable through a single SID.
> 
> Add new functions aimed at managing sub-devices of a SPMI device
> spmi_subdevice_alloc_and_add() and a spmi_subdevice_put_and_remove()
> for adding a new subdevice and removing it respectively, and also
> add their devm_* variants.
> 
> The need for such functions comes from the existance of	those
> complex Power Management ICs (PMICs), which feature one or many
> sub-devices, in some cases with these being even addressable on
> the chip in form of SPMI register ranges.
> 
> Examples of those devices can be found in both Qualcomm platforms
> with their PMICs having PON, RTC, SDAM, GPIO controller, and other
> sub-devices, and in newer MediaTek platforms showing similar HW
> features and a similar layout with those also having many subdevs.

...

>  EXPORT_SYMBOL_GPL(devm_spmi_controller_add);

> +EXPORT_SYMBOL_GPL(devm_spmi_subdevice_alloc_and_add);

I am wondering how hard to move these to a dedicated namespace.
Basically you can define a default namespace, and at the same time
add import to all (current) users.

...

> +	dev_set_name(&sdev->dev,
> +		     "%d-%02x.%d.auto", sdev->ctrl->nr, sdev->usid, sub_sdev->devid);

No error check?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()

[Andy Shevchenko]

On Mon, Jul 21, 2025 at 09:55:21AM +0200, AngeloGioacchino Del Regno wrote:
> Some Qualcomm PMICs integrates a Power On device supporting pwrkey
> and resin along with the Android reboot reason action identifier.
> 
> Instead of using the parent SPMI device (the main PMIC) as a kind
> of syscon in this driver, register a new SPMI sub-device for PON
> and initialize its own regmap with this sub-device's specific base
> address, retrieved from the devicetree.
> 
> This allows to stop manually adding the register base address to
> every R/W call in this driver, as this can be, and is now, handled
> by the regmap API instead.

...

> +	struct regmap_config qcom_pon_regmap_config = {
> +		.reg_bits = 16,
> +		.val_bits = 16,
> +		.max_register = 0x100,
> +		.fast_io = true

Please, leave trailing comma in this and other similar cases.

> +	};


>  	struct qcom_pon *pon;
>  	long reason_shift;
>  	int error;

> +	if (!pdev->dev.parent)
> +		return -ENODEV;

You can start using

	struct device *dev = &pdev->dev;

here and perhaps one may convert the rest to it...

...

>  	error = of_property_read_u32(pdev->dev.of_node, "reg",

...including, but not limited to, use of device_property_read_u32(dev, ...) here.

> -				     &pon->baseaddr);
> +				     &qcom_pon_regmap_config.reg_base);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/7] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Il 21/07/25 12:44, Casey Connolly ha scritto:
> Hi Angelo,
> 
> On 21/07/2025 09:55, AngeloGioacchino Del Regno wrote:
>> Some Qualcomm PMICs integrate a SDAM device, internally located in
>> a specific address range reachable through SPMI communication.
>>
>> Instead of using the parent SPMI device (the main PMIC) as a kind
>> of syscon in this driver, register a new SPMI sub-device for SDAM
>> and initialize its own regmap with this sub-device's specific base
>> address, retrieved from the devicetree.
>>
>> This allows to stop manually adding the register base address to
>> every R/W call in this driver, as this can be, and is now, handled
>> by the regmap API instead.
> 
> This is honestly a really nice improvement :D>

Thanks! :-D

>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   drivers/nvmem/qcom-spmi-sdam.c | 41 +++++++++++++++++++++++++---------
>>   1 file changed, 30 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
>> index 4f1cca6eab71..1b80e8563a33 100644
>> --- a/drivers/nvmem/qcom-spmi-sdam.c
>> +++ b/drivers/nvmem/qcom-spmi-sdam.c
>> @@ -9,6 +9,7 @@
>>   #include <linux/nvmem-provider.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/regmap.h>
>> +#include <linux/spmi.h>
>>   
>>   #define SDAM_MEM_START			0x40
>>   #define REGISTER_MAP_ID			0x40
>> @@ -20,7 +21,6 @@
>>   struct sdam_chip {
>>   	struct regmap			*regmap;
>>   	struct nvmem_config		sdam_config;
>> -	unsigned int			base;
>>   	unsigned int			size;
>>   };
>>   
>> @@ -73,7 +73,7 @@ static int sdam_read(void *priv, unsigned int offset, void *val,
>>   		return -EINVAL;
>>   	}
>>   
>> -	rc = regmap_bulk_read(sdam->regmap, sdam->base + offset, val, bytes);
>> +	rc = regmap_bulk_read(sdam->regmap, offset, val, bytes);
>>   	if (rc < 0)
>>   		dev_err(dev, "Failed to read SDAM offset %#x len=%zd, rc=%d\n",
>>   						offset, bytes, rc);
>> @@ -100,7 +100,7 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>>   		return -EINVAL;
>>   	}
>>   
>> -	rc = regmap_bulk_write(sdam->regmap, sdam->base + offset, val, bytes);
>> +	rc = regmap_bulk_write(sdam->regmap, offset, val, bytes);
>>   	if (rc < 0)
>>   		dev_err(dev, "Failed to write SDAM offset %#x len=%zd, rc=%d\n",
>>   						offset, bytes, rc);
>> @@ -110,28 +110,47 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>>   
>>   static int sdam_probe(struct platform_device *pdev)
>>   {
>> +	struct regmap_config sdam_regmap_config = {
>> +		.reg_bits = 16,
>> +		.val_bits = 16,
> 
> I believe registers are 8 bits wide, at least on Qualcomm platforms.
> 

I used 16 because usually that's the usual default for SPMI - but if you're sure
about Qualcomm platforms having 8-bits wide registers and you can confirm that,
I can change both of those to 8 in a jiffy.

I anyway have to send a v2 because I forgot an error check - so changing this is
not a problem at all for me.

But.

Before me changing - can you please please please double check and confirm?

If you can also check the register width of the others that I converted, I'd really
appreciate that (I have no datasheets for qcom so it's a bit of a guessing game for
me here... :-P), just so that we get everything right from the get-go ... even if
a mismatch wouldn't really cause issues in the current state of things.

That's because - I just noticed - in qcom-spmi-pmic.c, reg_bits is 16, but val_bits
is 8 (which basically means "the registers are 16-bits wide, but we always only
care about the lower 8 bits).

Thanks again!

Cheers,
Angelo

Re: [PATCH v1 1/7] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant

[AngeloGioacchino Del Regno]

Il 21/07/25 13:34, Andy Shevchenko ha scritto:
> On Mon, Jul 21, 2025 at 09:55:19AM +0200, AngeloGioacchino Del Regno wrote:
>> Some devices connected over the SPMI bus may be big, in the sense
>> that those may be a complex of devices managed by a single chip
>> over the SPMI bus, reachable through a single SID.
>>
>> Add new functions aimed at managing sub-devices of a SPMI device
>> spmi_subdevice_alloc_and_add() and a spmi_subdevice_put_and_remove()
>> for adding a new subdevice and removing it respectively, and also
>> add their devm_* variants.
>>
>> The need for such functions comes from the existance of	those
>> complex Power Management ICs (PMICs), which feature one or many
>> sub-devices, in some cases with these being even addressable on
>> the chip in form of SPMI register ranges.
>>
>> Examples of those devices can be found in both Qualcomm platforms
>> with their PMICs having PON, RTC, SDAM, GPIO controller, and other
>> sub-devices, and in newer MediaTek platforms showing similar HW
>> features and a similar layout with those also having many subdevs.
> 
> ...
> 
>>   EXPORT_SYMBOL_GPL(devm_spmi_controller_add);
> 
>> +EXPORT_SYMBOL_GPL(devm_spmi_subdevice_alloc_and_add);
> 
> I am wondering how hard to move these to a dedicated namespace.
> Basically you can define a default namespace, and at the same time
> add import to all (current) users.
> 

I can check if this can be done "relatively easily" (as in, if I can do that
without requiring "a hundred" immutable branches for every subsystem using
SPMI).

If turns out being feasible - I do actually like the idea so yes I'd be pleased
to do that... even though honestly it's completely out of scope for this series.

In case I can't - it's still something that can be done later.

> ...
> 
>> +	dev_set_name(&sdev->dev,
>> +		     "%d-%02x.%d.auto", sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
> 
> No error check?
> 

Heh, whoops, forgot about it :-)

Thanks for catching that - adding the check for v2.

Cheers,
Angelo

Re: [PATCH v1 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Il 21/07/25 13:36, Andy Shevchenko ha scritto:
> On Mon, Jul 21, 2025 at 09:55:21AM +0200, AngeloGioacchino Del Regno wrote:
>> Some Qualcomm PMICs integrates a Power On device supporting pwrkey
>> and resin along with the Android reboot reason action identifier.
>>
>> Instead of using the parent SPMI device (the main PMIC) as a kind
>> of syscon in this driver, register a new SPMI sub-device for PON
>> and initialize its own regmap with this sub-device's specific base
>> address, retrieved from the devicetree.
>>
>> This allows to stop manually adding the register base address to
>> every R/W call in this driver, as this can be, and is now, handled
>> by the regmap API instead.
> 
> ...
> 
>> +	struct regmap_config qcom_pon_regmap_config = {
>> +		.reg_bits = 16,
>> +		.val_bits = 16,
>> +		.max_register = 0x100,
>> +		.fast_io = true
> 
> Please, leave trailing comma in this and other similar cases.
> 

Oki, will do!

>> +	};
> 
> 
>>   	struct qcom_pon *pon;
>>   	long reason_shift;
>>   	int error;
> 
>> +	if (!pdev->dev.parent)
>> +		return -ENODEV;
> 
> You can start using
> 
> 	struct device *dev = &pdev->dev;
> 
> here and perhaps one may convert the rest to it...
> 
> ...
> 
>>   	error = of_property_read_u32(pdev->dev.of_node, "reg",
> 
> ...including, but not limited to, use of device_property_read_u32(dev, ...) here.
> 

I didn't do that for one single reason: I did not want to add noise to the commits
and wanted those to exclusively migrate the drivers to the new API, literally
without doing *anything* else unnecessary, even if I have located some almost
effortless improvements that I could've done to those drivers.

Please - I prefer to keep it this way: these are the first commits that add the
usage of the new functions and of the concept of SPMI subdevices, and I really
want those to contain just that and nothing else - because I suspect that these
will be taken as example and will be read by the next person that is implementing
a new SPMI (sub)driver or converting any remaining ones to subdevice.

Cheers,
Angelo

Re: [PATCH v1 4/7] phy: qualcomm: eusb2-repeater: Migrate to devm_spmi_subdevice_alloc_and_add()

[kernel test robot]

Hi AngeloGioacchino,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20250718]
[cannot apply to jic23-iio/togreg sre-power-supply/for-next char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.16-rc7 v6.16-rc6 v6.16-rc5 v6.16-rc7]
[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/AngeloGioacchino-Del-Regno/spmi-Implement-spmi_subdevice_alloc_and_add-and-devm-variant/20250721-155809
base:   next-20250718
patch link:    https://lore.kernel.org/r/20250721075525.29636-5-angelogioacchino.delregno%40collabora.com
patch subject: [PATCH v1 4/7] phy: qualcomm: eusb2-repeater: Migrate to devm_spmi_subdevice_alloc_and_add()
config: i386-buildonly-randconfig-002-20250721 (https://download.01.org/0day-ci/archive/20250721/202507212056.GmiGorFf-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250721/202507212056.GmiGorFf-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/202507212056.GmiGorFf-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c:221:59: warning: variable 'sparent' is uninitialized when used here [-Wuninitialized]
     221 |         sub_sdev = devm_spmi_subdevice_alloc_and_add(&pdev->dev, sparent);
         |                                                                  ^~~~~~~
   drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c:202:29: note: initialize the variable 'sparent' to silence this warning
     202 |         struct spmi_device *sparent;
         |                                    ^
         |                                     = NULL
   1 warning generated.


vim +/sparent +221 drivers/phy/qualcomm/phy-qcom-eusb2-repeater.c

   193	
   194	static int eusb2_repeater_probe(struct platform_device *pdev)
   195	{
   196		struct regmap_config eusb2_regmap_config = {
   197			.reg_bits = 16,
   198			.val_bits = 16,
   199			.max_register = 0x100,
   200			.fast_io = true
   201		};
   202		struct spmi_device *sparent;
   203		struct eusb2_repeater *rptr;
   204		struct spmi_subdevice *sub_sdev;
   205		struct device *dev = &pdev->dev;
   206		struct phy_provider *phy_provider;
   207		struct device_node *np = dev->of_node;
   208		int ret;
   209	
   210		rptr = devm_kzalloc(dev, sizeof(*rptr), GFP_KERNEL);
   211		if (!rptr)
   212			return -ENOMEM;
   213	
   214		rptr->dev = dev;
   215		dev_set_drvdata(dev, rptr);
   216	
   217		rptr->cfg = of_device_get_match_data(dev);
   218		if (!rptr->cfg)
   219			return -EINVAL;
   220	
 > 221		sub_sdev = devm_spmi_subdevice_alloc_and_add(&pdev->dev, sparent);
   222		if (IS_ERR(sub_sdev))
   223			return PTR_ERR(sub_sdev);
   224	
   225		ret = of_property_read_u32(np, "reg", &eusb2_regmap_config.reg_base);
   226		if (ret < 0)
   227			return ret;
   228	
   229		rptr->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &eusb2_regmap_config);
   230		if (IS_ERR(rptr->regmap))
   231			return -ENODEV;
   232	
   233		ret = eusb2_repeater_init_vregs(rptr);
   234		if (ret < 0) {
   235			dev_err(dev, "unable to get supplies\n");
   236			return ret;
   237		}
   238	
   239		rptr->phy = devm_phy_create(dev, np, &eusb2_repeater_ops);
   240		if (IS_ERR(rptr->phy)) {
   241			dev_err(dev, "failed to create PHY: %d\n", ret);
   242			return PTR_ERR(rptr->phy);
   243		}
   244	
   245		phy_set_drvdata(rptr->phy, rptr);
   246	
   247		phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
   248		if (IS_ERR(phy_provider))
   249			return PTR_ERR(phy_provider);
   250	
   251		return 0;
   252	}
   253	

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

Re: [PATCH v1 2/7] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

[Casey Connolly]

On 21/07/2025 14:45, AngeloGioacchino Del Regno wrote:
> Il 21/07/25 12:44, Casey Connolly ha scritto:
>> Hi Angelo,
>>
>> On 21/07/2025 09:55, AngeloGioacchino Del Regno wrote:
>>> Some Qualcomm PMICs integrate a SDAM device, internally located in
>>> a specific address range reachable through SPMI communication.
>>>
>>> Instead of using the parent SPMI device (the main PMIC) as a kind
>>> of syscon in this driver, register a new SPMI sub-device for SDAM
>>> and initialize its own regmap with this sub-device's specific base
>>> address, retrieved from the devicetree.
>>>
>>> This allows to stop manually adding the register base address to
>>> every R/W call in this driver, as this can be, and is now, handled
>>> by the regmap API instead.
>>
>> This is honestly a really nice improvement :D>
> 
> Thanks! :-D
> 
>>> Signed-off-by: AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@collabora.com>
>>> ---
>>>   drivers/nvmem/qcom-spmi-sdam.c | 41 +++++++++++++++++++++++++---------
>>>   1 file changed, 30 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-
>>> spmi-sdam.c
>>> index 4f1cca6eab71..1b80e8563a33 100644
>>> --- a/drivers/nvmem/qcom-spmi-sdam.c
>>> +++ b/drivers/nvmem/qcom-spmi-sdam.c
>>> @@ -9,6 +9,7 @@
>>>   #include <linux/nvmem-provider.h>
>>>   #include <linux/platform_device.h>
>>>   #include <linux/regmap.h>
>>> +#include <linux/spmi.h>
>>>     #define SDAM_MEM_START            0x40
>>>   #define REGISTER_MAP_ID            0x40
>>> @@ -20,7 +21,6 @@
>>>   struct sdam_chip {
>>>       struct regmap            *regmap;
>>>       struct nvmem_config        sdam_config;
>>> -    unsigned int            base;
>>>       unsigned int            size;
>>>   };
>>>   @@ -73,7 +73,7 @@ static int sdam_read(void *priv, unsigned int
>>> offset, void *val,
>>>           return -EINVAL;
>>>       }
>>>   -    rc = regmap_bulk_read(sdam->regmap, sdam->base + offset, val,
>>> bytes);
>>> +    rc = regmap_bulk_read(sdam->regmap, offset, val, bytes);
>>>       if (rc < 0)
>>>           dev_err(dev, "Failed to read SDAM offset %#x len=%zd,
>>> rc=%d\n",
>>>                           offset, bytes, rc);
>>> @@ -100,7 +100,7 @@ static int sdam_write(void *priv, unsigned int
>>> offset, void *val,
>>>           return -EINVAL;
>>>       }
>>>   -    rc = regmap_bulk_write(sdam->regmap, sdam->base + offset, val,
>>> bytes);
>>> +    rc = regmap_bulk_write(sdam->regmap, offset, val, bytes);
>>>       if (rc < 0)
>>>           dev_err(dev, "Failed to write SDAM offset %#x len=%zd,
>>> rc=%d\n",
>>>                           offset, bytes, rc);
>>> @@ -110,28 +110,47 @@ static int sdam_write(void *priv, unsigned int
>>> offset, void *val,
>>>     static int sdam_probe(struct platform_device *pdev)
>>>   {
>>> +    struct regmap_config sdam_regmap_config = {
>>> +        .reg_bits = 16,
>>> +        .val_bits = 16,
>>
>> I believe registers are 8 bits wide, at least on Qualcomm platforms.
>>
> 
> I used 16 because usually that's the usual default for SPMI - but if
> you're sure
> about Qualcomm platforms having 8-bits wide registers and you can
> confirm that,
> I can change both of those to 8 in a jiffy.

reg_bits should be 16, only val_bits needs changing.

> 
> I anyway have to send a v2 because I forgot an error check - so changing
> this is
> not a problem at all for me.
> 
> But.
> 
> Before me changing - can you please please please double check and confirm?
> 
> If you can also check the register width of the others that I converted,
> I'd really
> appreciate that (I have no datasheets for qcom so it's a bit of a
> guessing game for
> me here... :-P), just so that we get everything right from the get-
> go ... even if
> a mismatch wouldn't really cause issues in the current state of things.
> 
> That's because - I just noticed - in qcom-spmi-pmic.c, reg_bits is 16,
> but val_bits
> is 8 (which basically means "the registers are 16-bits wide, but we
> always only
> care about the lower 8 bits).

yeah that's right, so as of today where we don't have the subdevices we
always get 8 bit values from the registers.

I don't have so much access to docs anymore but I only ever remember
seeing 8 bit wide registers on Qualcomm PMICs, I've never seen anything
wider, I can think of a few drivers where changing this to 16 would for
sure cause havoc.

Kind regards,

> 
> Thanks again!
> 
> Cheers,
> Angelo
-- 
// Casey (she/her)

Re: [PATCH v1 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()

[Andy Shevchenko]

On Mon, Jul 21, 2025 at 03:05:46PM +0200, AngeloGioacchino Del Regno wrote:
> Il 21/07/25 13:36, Andy Shevchenko ha scritto:
> > On Mon, Jul 21, 2025 at 09:55:21AM +0200, AngeloGioacchino Del Regno wrote:

...

> > > +	if (!pdev->dev.parent)
> > > +		return -ENODEV;
> > 
> > You can start using
> > 
> > 	struct device *dev = &pdev->dev;
> > 
> > here and perhaps one may convert the rest to it...
> > 
> > ...
> > 
> > >   	error = of_property_read_u32(pdev->dev.of_node, "reg",
> > 
> > ...including, but not limited to, use of device_property_read_u32(dev, ...) here.
> > 
> 
> I didn't do that for one single reason: I did not want to add noise to the commits
> and wanted those to exclusively migrate the drivers to the new API, literally
> without doing *anything* else unnecessary, even if I have located some almost
> effortless improvements that I could've done to those drivers.
> 
> Please - I prefer to keep it this way: these are the first commits that add the
> usage of the new functions and of the concept of SPMI subdevices, and I really
> want those to contain just that and nothing else - because I suspect that these
> will be taken as example and will be read by the next person that is implementing
> a new SPMI (sub)driver or converting any remaining ones to subdevice.

You can introduce a temporary variable in this change and use it only in the
lines you have added/touched. We have similar approach in several drivers.
Then somebody (not specifically should be you) can move it forward.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 4/7] phy: qualcomm: eusb2-repeater: Migrate to devm_spmi_subdevice_alloc_and_add()

[kernel test robot]

Hi AngeloGioacchino,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20250718]
[cannot apply to jic23-iio/togreg sre-power-supply/for-next char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.16-rc7 v6.16-rc6 v6.16-rc5 v6.16-rc7]
[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/AngeloGioacchino-Del-Regno/spmi-Implement-spmi_subdevice_alloc_and_add-and-devm-variant/20250721-155809
base:   next-20250718
patch link:    https://lore.kernel.org/r/20250721075525.29636-5-angelogioacchino.delregno%40collabora.com
patch subject: [PATCH v1 4/7] phy: qualcomm: eusb2-repeater: Migrate to devm_spmi_subdevice_alloc_and_add()
config: sparc-randconfig-002-20250721 (https://download.01.org/0day-ci/archive/20250722/202507220236.c4472GGM-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250722/202507220236.c4472GGM-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/202507220236.c4472GGM-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "devm_spmi_subdevice_alloc_and_add" [drivers/phy/qualcomm/phy-qcom-eusb2-repeater.ko] undefined!
>> ERROR: modpost: "__devm_regmap_init_spmi_ext" [drivers/phy/qualcomm/phy-qcom-eusb2-repeater.ko] undefined!

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

Re: [PATCH v1 2/7] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Il 21/07/25 15:56, Casey Connolly ha scritto:
> 
> 
> On 21/07/2025 14:45, AngeloGioacchino Del Regno wrote:
>> Il 21/07/25 12:44, Casey Connolly ha scritto:
>>> Hi Angelo,
>>>
>>> On 21/07/2025 09:55, AngeloGioacchino Del Regno wrote:
>>>> Some Qualcomm PMICs integrate a SDAM device, internally located in
>>>> a specific address range reachable through SPMI communication.
>>>>
>>>> Instead of using the parent SPMI device (the main PMIC) as a kind
>>>> of syscon in this driver, register a new SPMI sub-device for SDAM
>>>> and initialize its own regmap with this sub-device's specific base
>>>> address, retrieved from the devicetree.
>>>>
>>>> This allows to stop manually adding the register base address to
>>>> every R/W call in this driver, as this can be, and is now, handled
>>>> by the regmap API instead.
>>>
>>> This is honestly a really nice improvement :D>
>>
>> Thanks! :-D
>>
>>>> Signed-off-by: AngeloGioacchino Del Regno
>>>> <angelogioacchino.delregno@collabora.com>
>>>> ---
>>>>    drivers/nvmem/qcom-spmi-sdam.c | 41 +++++++++++++++++++++++++---------
>>>>    1 file changed, 30 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-
>>>> spmi-sdam.c
>>>> index 4f1cca6eab71..1b80e8563a33 100644
>>>> --- a/drivers/nvmem/qcom-spmi-sdam.c
>>>> +++ b/drivers/nvmem/qcom-spmi-sdam.c
>>>> @@ -9,6 +9,7 @@
>>>>    #include <linux/nvmem-provider.h>
>>>>    #include <linux/platform_device.h>
>>>>    #include <linux/regmap.h>
>>>> +#include <linux/spmi.h>
>>>>      #define SDAM_MEM_START            0x40
>>>>    #define REGISTER_MAP_ID            0x40
>>>> @@ -20,7 +21,6 @@
>>>>    struct sdam_chip {
>>>>        struct regmap            *regmap;
>>>>        struct nvmem_config        sdam_config;
>>>> -    unsigned int            base;
>>>>        unsigned int            size;
>>>>    };
>>>>    @@ -73,7 +73,7 @@ static int sdam_read(void *priv, unsigned int
>>>> offset, void *val,
>>>>            return -EINVAL;
>>>>        }
>>>>    -    rc = regmap_bulk_read(sdam->regmap, sdam->base + offset, val,
>>>> bytes);
>>>> +    rc = regmap_bulk_read(sdam->regmap, offset, val, bytes);
>>>>        if (rc < 0)
>>>>            dev_err(dev, "Failed to read SDAM offset %#x len=%zd,
>>>> rc=%d\n",
>>>>                            offset, bytes, rc);
>>>> @@ -100,7 +100,7 @@ static int sdam_write(void *priv, unsigned int
>>>> offset, void *val,
>>>>            return -EINVAL;
>>>>        }
>>>>    -    rc = regmap_bulk_write(sdam->regmap, sdam->base + offset, val,
>>>> bytes);
>>>> +    rc = regmap_bulk_write(sdam->regmap, offset, val, bytes);
>>>>        if (rc < 0)
>>>>            dev_err(dev, "Failed to write SDAM offset %#x len=%zd,
>>>> rc=%d\n",
>>>>                            offset, bytes, rc);
>>>> @@ -110,28 +110,47 @@ static int sdam_write(void *priv, unsigned int
>>>> offset, void *val,
>>>>      static int sdam_probe(struct platform_device *pdev)
>>>>    {
>>>> +    struct regmap_config sdam_regmap_config = {
>>>> +        .reg_bits = 16,
>>>> +        .val_bits = 16,
>>>
>>> I believe registers are 8 bits wide, at least on Qualcomm platforms.
>>>
>>
>> I used 16 because usually that's the usual default for SPMI - but if
>> you're sure
>> about Qualcomm platforms having 8-bits wide registers and you can
>> confirm that,
>> I can change both of those to 8 in a jiffy.
> 
> reg_bits should be 16, only val_bits needs changing.
> 
>>
>> I anyway have to send a v2 because I forgot an error check - so changing
>> this is
>> not a problem at all for me.
>>
>> But.
>>
>> Before me changing - can you please please please double check and confirm?
>>
>> If you can also check the register width of the others that I converted,
>> I'd really
>> appreciate that (I have no datasheets for qcom so it's a bit of a
>> guessing game for
>> me here... :-P), just so that we get everything right from the get-
>> go ... even if
>> a mismatch wouldn't really cause issues in the current state of things.
>>
>> That's because - I just noticed - in qcom-spmi-pmic.c, reg_bits is 16,
>> but val_bits
>> is 8 (which basically means "the registers are 16-bits wide, but we
>> always only
>> care about the lower 8 bits).
> 
> yeah that's right, so as of today where we don't have the subdevices we
> always get 8 bit values from the registers.
> 
> I don't have so much access to docs anymore but I only ever remember
> seeing 8 bit wide registers on Qualcomm PMICs, I've never seen anything
> wider, I can think of a few drivers where changing this to 16 would for
> sure cause havoc.
> 

Yeah, right. I'm changing all val_bits to 8 to ensure compatibility.

Cheers,
Angelo

Re: [PATCH v1 3/7] power: reset: qcom-pon: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

Il 21/07/25 19:14, Andy Shevchenko ha scritto:
> On Mon, Jul 21, 2025 at 03:05:46PM +0200, AngeloGioacchino Del Regno wrote:
>> Il 21/07/25 13:36, Andy Shevchenko ha scritto:
>>> On Mon, Jul 21, 2025 at 09:55:21AM +0200, AngeloGioacchino Del Regno wrote:
> 
> ...
> 
>>>> +	if (!pdev->dev.parent)
>>>> +		return -ENODEV;
>>>
>>> You can start using
>>>
>>> 	struct device *dev = &pdev->dev;
>>>
>>> here and perhaps one may convert the rest to it...
>>>
>>> ...
>>>
>>>>    	error = of_property_read_u32(pdev->dev.of_node, "reg",
>>>
>>> ...including, but not limited to, use of device_property_read_u32(dev, ...) here.
>>>
>>
>> I didn't do that for one single reason: I did not want to add noise to the commits
>> and wanted those to exclusively migrate the drivers to the new API, literally
>> without doing *anything* else unnecessary, even if I have located some almost
>> effortless improvements that I could've done to those drivers.
>>
>> Please - I prefer to keep it this way: these are the first commits that add the
>> usage of the new functions and of the concept of SPMI subdevices, and I really
>> want those to contain just that and nothing else - because I suspect that these
>> will be taken as example and will be read by the next person that is implementing
>> a new SPMI (sub)driver or converting any remaining ones to subdevice.
> 
> You can introduce a temporary variable in this change and use it only in the
> lines you have added/touched. We have similar approach in several drivers.
> Then somebody (not specifically should be you) can move it forward.
> 

Makes sense, okay!

Cheers,
Angelo