Re: [PATCH v1 2/7] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()
From: AngeloGioacchino Del Regno
Date: Tue Jul 22 2025 - 04:22:58 EST
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@xxxxxxxxxxxxx>
---
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