[PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

AngeloGioacchino Del Regno posted 10 patches 3 weeks, 5 days ago
There is a newer version of this series
[PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by AngeloGioacchino Del Regno 3 weeks, 5 days ago
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.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-QRD
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/nvmem/Kconfig          |  1 +
 drivers/nvmem/qcom-spmi-sdam.c | 36 +++++++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index bf47a982cf62..5e924c869e77 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -368,6 +368,7 @@ config NVMEM_SNVS_LPGPR
 config NVMEM_SPMI_SDAM
 	tristate "SPMI SDAM Support"
 	depends on SPMI
+	select REGMAP_SPMI
 	help
 	  This driver supports the Shared Direct Access Memory Module on
 	  Qualcomm Technologies, Inc. PMICs. It provides the clients
diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
index 4f1cca6eab71..2bb7b3bc497e 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,8 +110,17 @@ 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 = 8,
+		.max_register = 0x100,
+		.fast_io = true,
+	};
 	struct sdam_chip *sdam;
 	struct nvmem_device *nvmem;
+	struct spmi_device *sparent;
+	struct spmi_subdevice *sub_sdev;
+	struct device *dev = &pdev->dev;
 	unsigned int val;
 	int rc;
 
@@ -119,19 +128,23 @@ static int sdam_probe(struct platform_device *pdev)
 	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(dev->parent);
+	sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
+	if (IS_ERR(sub_sdev))
+		return PTR_ERR(sub_sdev);
 
-	rc = of_property_read_u32(pdev->dev.of_node, "reg", &sdam->base);
+	rc = of_property_read_u32(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))
+		return dev_err_probe(&pdev->dev, PTR_ERR(sdam->regmap),
+				     "Failed to get regmap handle\n");
+
+	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 +172,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;
 }
@@ -181,3 +194,4 @@ module_platform_driver(sdam_driver);
 
 MODULE_DESCRIPTION("QCOM SPMI SDAM driver");
 MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS("SPMI");
-- 
2.52.0
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Andy Shevchenko 3 weeks, 5 days ago
On Wed, Jan 14, 2026 at 09:39:52AM +0100, 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.

...

> +	struct regmap_config sdam_regmap_config = {
> +		.reg_bits = 16,
> +		.val_bits = 8,

> +		.max_register = 0x100,

Are you sure? This might be a bad naming, but here max == the last accessible.
I bet it has to be 0xff (but since the address is 16-bit it might be actually
257 registers, but sounds very weird).

> +		.fast_io = true,
> +	};

...

> +	rc = of_property_read_u32(dev->of_node, "reg", &sdam_regmap_config.reg_base);

Why not device_property_read_u32(dev, ...) ?

...

> +	sdam->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &sdam_regmap_config);
> +	if (IS_ERR(sdam->regmap))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(sdam->regmap),

You have "dev".

> +				     "Failed to get regmap handle\n");

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by AngeloGioacchino Del Regno 3 weeks, 5 days ago
Il 14/01/26 09:56, Andy Shevchenko ha scritto:
> On Wed, Jan 14, 2026 at 09:39:52AM +0100, 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.
> 
> ...
> 
>> +	struct regmap_config sdam_regmap_config = {
>> +		.reg_bits = 16,
>> +		.val_bits = 8,
> 
>> +		.max_register = 0x100,
> 
> Are you sure? This might be a bad naming, but here max == the last accessible.
> I bet it has to be 0xff (but since the address is 16-bit it might be actually
> 257 registers, but sounds very weird).
> 

Yes, I'm sure.

>> +		.fast_io = true,
>> +	};
> 
> ...
> 
>> +	rc = of_property_read_u32(dev->of_node, "reg", &sdam_regmap_config.reg_base);
> 
> Why not device_property_read_u32(dev, ...) ?
> 
> ...
> 
>> +	sdam->regmap = devm_regmap_init_spmi_ext(&sub_sdev->sdev, &sdam_regmap_config);
>> +	if (IS_ERR(sdam->regmap))
>> +		return dev_err_probe(&pdev->dev, PTR_ERR(sdam->regmap),
> 
> You have "dev".
> 
>> +				     "Failed to get regmap handle\n");
> 

For the other comments: Done in v8.

Cheers,
Angelo
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Andy Shevchenko 3 weeks, 5 days ago
On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
> Il 14/01/26 09:56, Andy Shevchenko ha scritto:
> > On Wed, Jan 14, 2026 at 09:39:52AM +0100, 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.

...

> > > +	struct regmap_config sdam_regmap_config = {
> > > +		.reg_bits = 16,
> > > +		.val_bits = 8,
> > 
> > > +		.max_register = 0x100,
> > 
> > Are you sure? This might be a bad naming, but here max == the last accessible.
> > I bet it has to be 0xff (but since the address is 16-bit it might be actually
> > 257 registers, but sounds very weird).
> 
> Yes, I'm sure.

So, what is resided on address 0x100 ?

> > > +		.fast_io = true,
> > > +	};


-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by AngeloGioacchino Del Regno 3 weeks, 5 days ago
Il 14/01/26 10:00, Andy Shevchenko ha scritto:
> On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
>> Il 14/01/26 09:56, Andy Shevchenko ha scritto:
>>> On Wed, Jan 14, 2026 at 09:39:52AM +0100, 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.
> 
> ...
> 
>>>> +	struct regmap_config sdam_regmap_config = {
>>>> +		.reg_bits = 16,
>>>> +		.val_bits = 8,
>>>
>>>> +		.max_register = 0x100,
>>>
>>> Are you sure? This might be a bad naming, but here max == the last accessible.
>>> I bet it has to be 0xff (but since the address is 16-bit it might be actually
>>> 257 registers, but sounds very weird).
>>
>> Yes, I'm sure.
> 
> So, what is resided on address 0x100 ?
> 

I don't remember, this is research from around 5 months ago, when I've sent
the v1 of this.

If you really want though, I can incorrectly set max_register to 0xff.

Cheers,
Angelo

>>>> +		.fast_io = true,
>>>> +	};
> 
>
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Andy Shevchenko 3 weeks, 5 days ago
On Wed, Jan 14, 2026 at 10:03:57AM +0100, AngeloGioacchino Del Regno wrote:
> Il 14/01/26 10:00, Andy Shevchenko ha scritto:
> > On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
> > > Il 14/01/26 09:56, Andy Shevchenko ha scritto:
> > > > On Wed, Jan 14, 2026 at 09:39:52AM +0100, AngeloGioacchino Del Regno wrote:

...

> > > > > +	struct regmap_config sdam_regmap_config = {
> > > > > +		.reg_bits = 16,
> > > > > +		.val_bits = 8,
> > > > 
> > > > > +		.max_register = 0x100,
> > > > 
> > > > Are you sure? This might be a bad naming, but here max == the last accessible.
> > > > I bet it has to be 0xff (but since the address is 16-bit it might be actually
> > > > 257 registers, but sounds very weird).
> > > 
> > > Yes, I'm sure.
> > 
> > So, what is resided on address 0x100 ?
> 
> I don't remember, this is research from around 5 months ago, when I've sent
> the v1 of this.
> 
> If you really want though, I can incorrectly set max_register to 0xff.

Why incorrectly? Can you dig into the datasheet and check, please? We don't
know what is the 0x100 address means.

> > > > > +		.fast_io = true,
> > > > > +	};

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by AngeloGioacchino Del Regno 3 weeks, 5 days ago
Il 14/01/26 10:07, Andy Shevchenko ha scritto:
> On Wed, Jan 14, 2026 at 10:03:57AM +0100, AngeloGioacchino Del Regno wrote:
>> Il 14/01/26 10:00, Andy Shevchenko ha scritto:
>>> On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
>>>> Il 14/01/26 09:56, Andy Shevchenko ha scritto:
>>>>> On Wed, Jan 14, 2026 at 09:39:52AM +0100, AngeloGioacchino Del Regno wrote:
> 
> ...
> 
>>>>>> +	struct regmap_config sdam_regmap_config = {
>>>>>> +		.reg_bits = 16,
>>>>>> +		.val_bits = 8,
>>>>>
>>>>>> +		.max_register = 0x100,
>>>>>
>>>>> Are you sure? This might be a bad naming, but here max == the last accessible.
>>>>> I bet it has to be 0xff (but since the address is 16-bit it might be actually
>>>>> 257 registers, but sounds very weird).
>>>>
>>>> Yes, I'm sure.
>>>
>>> So, what is resided on address 0x100 ?
>>
>> I don't remember, this is research from around 5 months ago, when I've sent
>> the v1 of this.
>>
>> If you really want though, I can incorrectly set max_register to 0xff.
> 
> Why incorrectly? Can you dig into the datasheet and check, please? We don't
> know what is the 0x100 address means.
> 

I don't have any datasheets for Qualcomm IPs.

Cheers,
Angelo

>>>>>> +		.fast_io = true,
>>>>>> +	};
>
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Andy Shevchenko 3 weeks, 5 days ago
On Wed, Jan 14, 2026 at 10:09:45AM +0100, AngeloGioacchino Del Regno wrote:
> Il 14/01/26 10:07, Andy Shevchenko ha scritto:
> > On Wed, Jan 14, 2026 at 10:03:57AM +0100, AngeloGioacchino Del Regno wrote:
> > > Il 14/01/26 10:00, Andy Shevchenko ha scritto:
> > > > On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
> > > > > Il 14/01/26 09:56, Andy Shevchenko ha scritto:
> > > > > > On Wed, Jan 14, 2026 at 09:39:52AM +0100, AngeloGioacchino Del Regno wrote:

...

> > > > > > > +	struct regmap_config sdam_regmap_config = {
> > > > > > > +		.reg_bits = 16,
> > > > > > > +		.val_bits = 8,
> > > > > > 
> > > > > > > +		.max_register = 0x100,
> > > > > > 
> > > > > > Are you sure? This might be a bad naming, but here max == the last accessible.
> > > > > > I bet it has to be 0xff (but since the address is 16-bit it might be actually
> > > > > > 257 registers, but sounds very weird).
> > > > > 
> > > > > Yes, I'm sure.
> > > > 
> > > > So, what is resided on address 0x100 ?
> > > 
> > > I don't remember, this is research from around 5 months ago, when I've sent
> > > the v1 of this.
> > > 
> > > If you really want though, I can incorrectly set max_register to 0xff.
> > 
> > Why incorrectly? Can you dig into the datasheet and check, please? We don't
> > know what is the 0x100 address means.
> 
> I don't have any datasheets for Qualcomm IPs.

Hmm... Can we have somebody from QC to check on this?
Perhaps Dmitry?

> > > > > > > +		.fast_io = true,
> > > > > > > +	};

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Konrad Dybcio 3 weeks, 5 days ago
On 1/14/26 10:42 AM, Andy Shevchenko wrote:
> On Wed, Jan 14, 2026 at 10:09:45AM +0100, AngeloGioacchino Del Regno wrote:
>> Il 14/01/26 10:07, Andy Shevchenko ha scritto:
>>> On Wed, Jan 14, 2026 at 10:03:57AM +0100, AngeloGioacchino Del Regno wrote:
>>>> Il 14/01/26 10:00, Andy Shevchenko ha scritto:
>>>>> On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
>>>>>> Il 14/01/26 09:56, Andy Shevchenko ha scritto:
>>>>>>> On Wed, Jan 14, 2026 at 09:39:52AM +0100, AngeloGioacchino Del Regno wrote:
> 
> ...
> 
>>>>>>>> +	struct regmap_config sdam_regmap_config = {
>>>>>>>> +		.reg_bits = 16,
>>>>>>>> +		.val_bits = 8,
>>>>>>>
>>>>>>>> +		.max_register = 0x100,
>>>>>>>
>>>>>>> Are you sure? This might be a bad naming, but here max == the last accessible.
>>>>>>> I bet it has to be 0xff (but since the address is 16-bit it might be actually
>>>>>>> 257 registers, but sounds very weird).
>>>>>>
>>>>>> Yes, I'm sure.
>>>>>
>>>>> So, what is resided on address 0x100 ?
>>>>
>>>> I don't remember, this is research from around 5 months ago, when I've sent
>>>> the v1 of this.
>>>>
>>>> If you really want though, I can incorrectly set max_register to 0xff.
>>>
>>> Why incorrectly? Can you dig into the datasheet and check, please? We don't
>>> know what is the 0x100 address means.
>>
>> I don't have any datasheets for Qualcomm IPs.
> 
> Hmm... Can we have somebody from QC to check on this?
> Perhaps Dmitry?

0xe6 is the last usable register today

But I wouldn't mind either 0xff or 0x100 because I don't want
anyone to pull their hair out if a regmap access is dropped some day..

Konrad
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Andy Shevchenko 3 weeks, 5 days ago
On Wed, Jan 14, 2026 at 10:47:20AM +0100, Konrad Dybcio wrote:
> On 1/14/26 10:42 AM, Andy Shevchenko wrote:
> > On Wed, Jan 14, 2026 at 10:09:45AM +0100, AngeloGioacchino Del Regno wrote:
> >> Il 14/01/26 10:07, Andy Shevchenko ha scritto:
> >>> On Wed, Jan 14, 2026 at 10:03:57AM +0100, AngeloGioacchino Del Regno wrote:
> >>>> Il 14/01/26 10:00, Andy Shevchenko ha scritto:
> >>>>> On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
> >>>>>> Il 14/01/26 09:56, Andy Shevchenko ha scritto:
> >>>>>>> On Wed, Jan 14, 2026 at 09:39:52AM +0100, AngeloGioacchino Del Regno wrote:

...

> >>>>>>>> +	struct regmap_config sdam_regmap_config = {
> >>>>>>>> +		.reg_bits = 16,
> >>>>>>>> +		.val_bits = 8,
> >>>>>>>
> >>>>>>>> +		.max_register = 0x100,
> >>>>>>>
> >>>>>>> Are you sure? This might be a bad naming, but here max == the last accessible.
> >>>>>>> I bet it has to be 0xff (but since the address is 16-bit it might be actually
> >>>>>>> 257 registers, but sounds very weird).
> >>>>>>
> >>>>>> Yes, I'm sure.
> >>>>>
> >>>>> So, what is resided on address 0x100 ?
> >>>>
> >>>> I don't remember, this is research from around 5 months ago, when I've sent
> >>>> the v1 of this.
> >>>>
> >>>> If you really want though, I can incorrectly set max_register to 0xff.
> >>>
> >>> Why incorrectly? Can you dig into the datasheet and check, please? We don't
> >>> know what is the 0x100 address means.
> >>
> >> I don't have any datasheets for Qualcomm IPs.
> > 
> > Hmm... Can we have somebody from QC to check on this?
> > Perhaps Dmitry?
> 
> 0xe6 is the last usable register today

Thanks for checking!

> But I wouldn't mind either 0xff or 0x100 because I don't want
> anyone to pull their hair out if a regmap access is dropped some day..

There is actually about the exact window size where registers are belong to the
same entity (subdevice). As in the HW world most of the things are stuck with
power-of-two numbers, and taking into account the naming of the field, I do not
believe one provides a 257 (256 + 1 = 2⁸ + 1) register _windows_ ("s" is also
important here, as it points out to the pattern) for the subdevices. I bet the
0xff, i.e. 256, is the *correct* window from the HW perspective.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Konrad Dybcio 3 weeks, 5 days ago
On 1/14/26 10:55 AM, Andy Shevchenko wrote:
> On Wed, Jan 14, 2026 at 10:47:20AM +0100, Konrad Dybcio wrote:
>> On 1/14/26 10:42 AM, Andy Shevchenko wrote:
>>> On Wed, Jan 14, 2026 at 10:09:45AM +0100, AngeloGioacchino Del Regno wrote:
>>>> Il 14/01/26 10:07, Andy Shevchenko ha scritto:
>>>>> On Wed, Jan 14, 2026 at 10:03:57AM +0100, AngeloGioacchino Del Regno wrote:
>>>>>> Il 14/01/26 10:00, Andy Shevchenko ha scritto:
>>>>>>> On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
>>>>>>>> Il 14/01/26 09:56, Andy Shevchenko ha scritto:
>>>>>>>>> On Wed, Jan 14, 2026 at 09:39:52AM +0100, AngeloGioacchino Del Regno wrote:
> 
> ...
> 
>>>>>>>>>> +	struct regmap_config sdam_regmap_config = {
>>>>>>>>>> +		.reg_bits = 16,
>>>>>>>>>> +		.val_bits = 8,
>>>>>>>>>
>>>>>>>>>> +		.max_register = 0x100,
>>>>>>>>>
>>>>>>>>> Are you sure? This might be a bad naming, but here max == the last accessible.
>>>>>>>>> I bet it has to be 0xff (but since the address is 16-bit it might be actually
>>>>>>>>> 257 registers, but sounds very weird).
>>>>>>>>
>>>>>>>> Yes, I'm sure.
>>>>>>>
>>>>>>> So, what is resided on address 0x100 ?
>>>>>>
>>>>>> I don't remember, this is research from around 5 months ago, when I've sent
>>>>>> the v1 of this.
>>>>>>
>>>>>> If you really want though, I can incorrectly set max_register to 0xff.
>>>>>
>>>>> Why incorrectly? Can you dig into the datasheet and check, please? We don't
>>>>> know what is the 0x100 address means.
>>>>
>>>> I don't have any datasheets for Qualcomm IPs.
>>>
>>> Hmm... Can we have somebody from QC to check on this?
>>> Perhaps Dmitry?
>>
>> 0xe6 is the last usable register today
> 
> Thanks for checking!
> 
>> But I wouldn't mind either 0xff or 0x100 because I don't want
>> anyone to pull their hair out if a regmap access is dropped some day..
> 
> There is actually about the exact window size where registers are belong to the
> same entity (subdevice). As in the HW world most of the things are stuck with
> power-of-two numbers, and taking into account the naming of the field, I do not
> believe one provides a 257 (256 + 1 = 2⁸ + 1) register _windows_ ("s" is also
> important here, as it points out to the pattern) for the subdevices. I bet the
> 0xff, i.e. 256, is the *correct* window from the HW perspective.

Right, [0x100n, 0x100n + 0xff] inclusive is the reserved register window
for all Qualcomm PMIC peripherals, so I guess 0xff is the correct choice
here

If a peripheral is more complex, it's split into a couple of these
same-sized blocks

Konrad
Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Andy Shevchenko 3 weeks, 5 days ago
On Wed, Jan 14, 2026 at 11:04:30AM +0100, Konrad Dybcio wrote:
> On 1/14/26 10:55 AM, Andy Shevchenko wrote:
> > On Wed, Jan 14, 2026 at 10:47:20AM +0100, Konrad Dybcio wrote:
> >> On 1/14/26 10:42 AM, Andy Shevchenko wrote:
> >>> On Wed, Jan 14, 2026 at 10:09:45AM +0100, AngeloGioacchino Del Regno wrote:
> >>>> Il 14/01/26 10:07, Andy Shevchenko ha scritto:
> >>>>> On Wed, Jan 14, 2026 at 10:03:57AM +0100, AngeloGioacchino Del Regno wrote:
> >>>>>> Il 14/01/26 10:00, Andy Shevchenko ha scritto:
> >>>>>>> On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
> >>>>>>>> Il 14/01/26 09:56, Andy Shevchenko ha scritto:
> >>>>>>>>> On Wed, Jan 14, 2026 at 09:39:52AM +0100, AngeloGioacchino Del Regno wrote:

...

> >>>>>>>>>> +	struct regmap_config sdam_regmap_config = {
> >>>>>>>>>> +		.reg_bits = 16,
> >>>>>>>>>> +		.val_bits = 8,
> >>>>>>>>>
> >>>>>>>>>> +		.max_register = 0x100,
> >>>>>>>>>
> >>>>>>>>> Are you sure? This might be a bad naming, but here max == the last accessible.
> >>>>>>>>> I bet it has to be 0xff (but since the address is 16-bit it might be actually
> >>>>>>>>> 257 registers, but sounds very weird).
> >>>>>>>>
> >>>>>>>> Yes, I'm sure.
> >>>>>>>
> >>>>>>> So, what is resided on address 0x100 ?
> >>>>>>
> >>>>>> I don't remember, this is research from around 5 months ago, when I've sent
> >>>>>> the v1 of this.
> >>>>>>
> >>>>>> If you really want though, I can incorrectly set max_register to 0xff.
> >>>>>
> >>>>> Why incorrectly? Can you dig into the datasheet and check, please? We don't
> >>>>> know what is the 0x100 address means.
> >>>>
> >>>> I don't have any datasheets for Qualcomm IPs.
> >>>
> >>> Hmm... Can we have somebody from QC to check on this?
> >>> Perhaps Dmitry?
> >>
> >> 0xe6 is the last usable register today
> > 
> > Thanks for checking!
> > 
> >> But I wouldn't mind either 0xff or 0x100 because I don't want
> >> anyone to pull their hair out if a regmap access is dropped some day..
> > 
> > There is actually about the exact window size where registers are belong to the
> > same entity (subdevice). As in the HW world most of the things are stuck with
> > power-of-two numbers, and taking into account the naming of the field, I do not
> > believe one provides a 257 (256 + 1 = 2⁸ + 1) register _windows_ ("s" is also
> > important here, as it points out to the pattern) for the subdevices. I bet the
> > 0xff, i.e. 256, is the *correct* window from the HW perspective.
> 
> Right, [0x100n, 0x100n + 0xff] inclusive is the reserved register window
> for all Qualcomm PMIC peripherals, so I guess 0xff is the correct choice
> here

Thanks for the clarification, v8 addresses that, so seems to me good to go.

> If a peripheral is more complex, it's split into a couple of these
> same-sized blocks

Right.

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v7 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
Posted by Andy Shevchenko 3 weeks, 5 days ago
On Wed, Jan 14, 2026 at 11:55:18AM +0200, Andy Shevchenko wrote:
> On Wed, Jan 14, 2026 at 10:47:20AM +0100, Konrad Dybcio wrote:
> > On 1/14/26 10:42 AM, Andy Shevchenko wrote:
> > > On Wed, Jan 14, 2026 at 10:09:45AM +0100, AngeloGioacchino Del Regno wrote:
> > >> Il 14/01/26 10:07, Andy Shevchenko ha scritto:
> > >>> On Wed, Jan 14, 2026 at 10:03:57AM +0100, AngeloGioacchino Del Regno wrote:
> > >>>> Il 14/01/26 10:00, Andy Shevchenko ha scritto:
> > >>>>> On Wed, Jan 14, 2026 at 09:59:40AM +0100, AngeloGioacchino Del Regno wrote:
> > >>>>>> Il 14/01/26 09:56, Andy Shevchenko ha scritto:
> > >>>>>>> On Wed, Jan 14, 2026 at 09:39:52AM +0100, AngeloGioacchino Del Regno wrote:

...

> > >>>>>>>> +	struct regmap_config sdam_regmap_config = {
> > >>>>>>>> +		.reg_bits = 16,
> > >>>>>>>> +		.val_bits = 8,
> > >>>>>>>
> > >>>>>>>> +		.max_register = 0x100,
> > >>>>>>>
> > >>>>>>> Are you sure? This might be a bad naming, but here max == the last accessible.
> > >>>>>>> I bet it has to be 0xff (but since the address is 16-bit it might be actually
> > >>>>>>> 257 registers, but sounds very weird).
> > >>>>>>
> > >>>>>> Yes, I'm sure.
> > >>>>>
> > >>>>> So, what is resided on address 0x100 ?
> > >>>>
> > >>>> I don't remember, this is research from around 5 months ago, when I've sent
> > >>>> the v1 of this.
> > >>>>
> > >>>> If you really want though, I can incorrectly set max_register to 0xff.
> > >>>
> > >>> Why incorrectly? Can you dig into the datasheet and check, please? We don't
> > >>> know what is the 0x100 address means.
> > >>
> > >> I don't have any datasheets for Qualcomm IPs.
> > > 
> > > Hmm... Can we have somebody from QC to check on this?
> > > Perhaps Dmitry?
> > 
> > 0xe6 is the last usable register today
> 
> Thanks for checking!
> 
> > But I wouldn't mind either 0xff or 0x100 because I don't want
> > anyone to pull their hair out if a regmap access is dropped some day..
> 
> There is actually about the exact window size where registers are belong to the
> same entity (subdevice). As in the HW world most of the things are stuck with
> power-of-two numbers, and taking into account the naming of the field, I do not
> believe one provides a 257 (256 + 1 = 2⁸ + 1) register _windows_ ("s" is also
> important here, as it points out to the pattern) for the subdevices. I bet the
> 0xff, i.e. 256, is the *correct* window from the HW perspective.

Even more, since these devices are described in DT, the presence of any two
sequential windows will support my version for 100% correctness, as otherwise
it will mean overlapping in the addresses for two devices, i.e. 0x100 in one
is actually 0x00 in the other.

-- 
With Best Regards,
Andy Shevchenko