Message-ID: <de08d697-4202-499c-9784-2bb8d3d614b6@collabora.com>
Date: Wed, 21 Jan 2026 12:00:35 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: jic23@...nel.org, sboyd@...nel.org
Cc: dlechner@...libre.com, nuno.sa@...log.com, andy@...nel.org,
 arnd@...db.de, gregkh@...uxfoundation.org, srini@...nel.org,
 vkoul@...nel.org, neil.armstrong@...aro.org, sre@...nel.org,
 krzk@...nel.org, dmitry.baryshkov@....qualcomm.com, quic_wcheng@...cinc.com,
 melody.olvera@....qualcomm.com, quic_nsekar@...cinc.com,
 ivo.ivanov.ivanov1@...il.com, abelvesa@...nel.org, luca.weiss@...rphone.com,
 konrad.dybcio@....qualcomm.com, mitltlatltl@...il.com,
 krishna.kurapati@....qualcomm.com, linux-arm-msm@...r.kernel.org,
 linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-phy@...ts.infradead.org, linux-pm@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH v8 00/10] SPMI: Implement sub-devices and migrate drivers

Il 14/01/26 10:27, AngeloGioacchino Del Regno ha scritto:
> Changes in v8:

With this series being finally fully reviewed and having lots of acks, and
since I think that the main part of this is in SPMI subsystem rather than
others....

....Stephen or Jonathan, can you please pick at least the SPMI code so that
it goes in for this merge window and doesn't get forgotten again?

That also makes it easier for the other maintainers who didn't give an ack (and
that don't want their subsystem patches to go through SPMI) to apply the patches
in the next merge window, IMO.

Thanks,
Angelo


>   - Renamed *res to *sub_sdev in devm_spmi_subdevice_remove() (Andy)
>   - Changed kerneldoc wording to "error pointer" for function
>     spmi_subdevice_alloc_and_add() (Andy)
>   - Shuffled around some assignments in spmi_subdevice_alloc_and_add() (Andy)
>   - Used device_property_read_u32() instead of of_property_read_u32()
>     in all of the migrated drivers (Andy)
>   - Changed .max_register field in all of the migrated drivers from
>     0x100 to 0xff (Andy)
>   - Kept `sta1` declaration in reversed xmas tree order in function
>     iadc_poll_wait_eoc() of qcom-spmi-iadc.c (Andy)
> 
> Changes in v7:
>   - Added commit to cleanup redundant dev_name() in the pre-existing
>     spmi_device_add() function
>   - Added commit removing unneeded goto and improving spmi_device_add()
>     readability by returning error in error path, and explicitly zero
>     for success at the end.
> 
> Changes in v6:
>   - Added commit to convert spmi.c to %pe error format and used
>     %pe error format in spmi_subdevice code as wanted by Uwe Kleine-Konig
> 
> Changes in v5:
>   - Changed dev_err to dev_err_probe in qcom-spmi-sdam (and done
>     that even though I disagree - because I wanted this series to
>     *exclusively* introduce the minimum required changes to
>     migrate to the new API, but okay, whatever....!);
>   - Added missing REGMAP dependency in Kconfig for qcom-spmi-sdam,
>     phy-qcom-eusb2-repeater and qcom-coincell to resolve build
>     issues when the already allowed COMPILE_TEST is enabled
>     as pointed out by the test robot's randconfig builds.
> 
> Changes in v4:
>   - Added selection of REGMAP_SPMI in Kconfig for qcom-coincell and
>     for phy-qcom-eusb2-repeater to resolve undefined references when
>     compiled with some randconfig
> 
> Changes in v3:
>   - Fixed importing "SPMI" namespace in spmi-devres.c
>   - Removed all instances of defensive programming, as pointed out by
>     jic23 and Sebastian
>   - Removed explicit casting as pointed out by jic23
>   - Moved ida_free call to spmi_subdev_release() and simplified error
>     handling in spmi_subdevice_alloc_and_add() as pointed out by jic23
> 
> Changes in v2:
>   - Fixed missing `sparent` initialization in phy-qcom-eusb2-repeater
>   - Changed val_bits to 8 in all Qualcomm drivers to ensure
>     compatibility as suggested by Casey
>   - Added struct device pointer in all conversion commits as suggested
>     by Andy
>   - Exported newly introduced functions with a new "SPMI" namespace
>     and imported the same in all converted drivers as suggested by Andy
>   - Added missing error checking for dev_set_name() call in spmi.c
>     as suggested by Andy
>   - Added comma to last entry of regmap_config as suggested by Andy
> 
> 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 (10):
>    spmi: Remove redundant dev_name() print in spmi_device_add()
>    spmi: Print error status with %pe format
>    spmi: Remove unneeded goto in spmi_device_add() error path
>    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/Kconfig                          |   2 +
>   drivers/misc/qcom-coincell.c                  |  38 ++++--
>   drivers/nvmem/Kconfig                         |   1 +
>   drivers/nvmem/qcom-spmi-sdam.c                |  38 ++++--
>   drivers/phy/qualcomm/Kconfig                  |   2 +
>   .../phy/qualcomm/phy-qcom-eusb2-repeater.c    |  55 +++++----
>   drivers/power/reset/qcom-pon.c                |  33 ++++--
>   drivers/spmi/spmi-devres.c                    |  24 ++++
>   drivers/spmi/spmi.c                           |  95 +++++++++++++--
>   include/linux/spmi.h                          |  16 +++
>   11 files changed, 290 insertions(+), 123 deletions(-)
> 

Powered by blists - more mailing lists