[PATCH v5 8/9] spmi: msm: fix register range names
Neil Armstrong
neil.armstrong at linaro.org
Fri Dec 1 10:30:36 CET 2023
On 30/11/2023 21:22, Caleb Connolly wrote:
> The core and chnl register ranges were swapped on SDM845. Fix it, and
> fetch the register ranges by name instead of by index.
>
> Drop the cosmetic "version" variable and clean up the debug logging.
>
> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
> ---
> arch/arm/dts/qcs404-evb.dts | 7 +++--
> arch/arm/dts/sdm845.dtsi | 2 +-
> doc/device-tree-bindings/spmi/spmi-msm.txt | 26 -----------------
> drivers/spmi/spmi-msm.c | 46 ++++++++++++------------------
> 4 files changed, 23 insertions(+), 58 deletions(-)
>
> diff --git a/arch/arm/dts/qcs404-evb.dts b/arch/arm/dts/qcs404-evb.dts
> index 3bb580ba4e17..cf41e5a33dbe 100644
> --- a/arch/arm/dts/qcs404-evb.dts
> +++ b/arch/arm/dts/qcs404-evb.dts
> @@ -362,9 +362,10 @@
>
> spmi at 200f000 {
> compatible = "qcom,spmi-pmic-arb";
> - reg = <0x200f000 0x1000
> - 0x2400000 0x400000
> - 0x2c00000 0x400000>;
> + reg = <0x200f000 0x001000>,
> + <0x2400000 0x800000>,
> + <0x2c00000 0x800000>;
> + reg-names = "core", "chnls", "obsrvr";
> #address-cells = <0x1>;
> #size-cells = <0x1>;
>
> diff --git a/arch/arm/dts/sdm845.dtsi b/arch/arm/dts/sdm845.dtsi
> index a26e9f411ee0..96c9749a52c0 100644
> --- a/arch/arm/dts/sdm845.dtsi
> +++ b/arch/arm/dts/sdm845.dtsi
> @@ -63,7 +63,7 @@
> reg = <0xc440000 0x1100>,
> <0xc600000 0x2000000>,
> <0xe600000 0x100000>;
> - reg-names = "cnfg", "core", "obsrvr";
> + reg-names = "core", "chnls", "obsrvr";
> #address-cells = <0x1>;
> #size-cells = <0x1>;
>
> diff --git a/doc/device-tree-bindings/spmi/spmi-msm.txt b/doc/device-tree-bindings/spmi/spmi-msm.txt
> deleted file mode 100644
> index ae47673b768b..000000000000
> --- a/doc/device-tree-bindings/spmi/spmi-msm.txt
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -Qualcomm SPMI arbiter/bus driver
> -
> -This is bus driver for Qualcomm chips that use SPMI to communicate with PMICs.
> -
> -Required properties:
> -- compatible: "qcom,spmi-pmic-arb"
> -- reg: Register block adresses and sizes for various parts of device:
> - 1) PMIC arbiter channel mapping base (PMIC_ARB_REG_CHNLn)
> - 2) SPMI write command (master) registers (PMIC_ARB_CORE_SW_DEC_CHANNELS)
> - 3) SPMI read command (observer) registers (PMIC_ARB_CORE_REGISTERS_OBS)
> -
> -Optional properties (if not set by parent):
> -- #address-cells: 0x1 - childs slave ID address
> -- #size-cells: 0x1
> -
> -All PMICs should be placed as a child nodes of bus arbiter.
> -Automatic detection of childs is currently not supported.
> -
> -Example:
> -
> -spmi at 200f000 {
> - compatible = "qcom,spmi-pmic-arb";
> - reg = <0x200f800 0x200 0x2400000 0x400000 0x2c00000 0x400000>;
> - #address-cells = <0x1>;
> - #size-cells = <0x1>;
> -};
> diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c
> index 27a035c0a595..5fe8a70abca7 100644
> --- a/drivers/spmi/spmi-msm.c
> +++ b/drivers/spmi/spmi-msm.c
> @@ -70,7 +70,7 @@ enum pmic_arb_channel {
>
> struct msm_spmi_priv {
> phys_addr_t arb_chnl; /* ARB channel mapping base */
> - phys_addr_t spmi_core; /* SPMI core */
> + phys_addr_t spmi_chnls; /* SPMI channels */
> phys_addr_t spmi_obs; /* SPMI observer */
> /* SPMI channel map */
> uint8_t channel_map[SPMI_MAX_SLAVES][SPMI_MAX_PERIPH];
> @@ -95,10 +95,10 @@ static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off,
>
> /* Disable IRQ mode for the current channel*/
> writel(0x0,
> - priv->spmi_core + SPMI_CH_OFFSET(channel) + SPMI_REG_CONFIG);
> + priv->spmi_chnls + SPMI_CH_OFFSET(channel) + SPMI_REG_CONFIG);
>
> /* Write single byte */
> - writel(val, priv->spmi_core + SPMI_CH_OFFSET(channel) + SPMI_REG_WDATA);
> + writel(val, priv->spmi_chnls + SPMI_CH_OFFSET(channel) + SPMI_REG_WDATA);
>
> /* Prepare write command */
> reg |= SPMI_CMD_EXT_REG_WRITE_LONG << SPMI_CMD_OPCODE_SHIFT;
> @@ -113,12 +113,12 @@ static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off,
> ch_offset = SPMI_CH_OFFSET(channel);
>
> /* Send write command */
> - writel(reg, priv->spmi_core + SPMI_CH_OFFSET(channel) + SPMI_REG_CMD0);
> + writel(reg, priv->spmi_chnls + SPMI_CH_OFFSET(channel) + SPMI_REG_CMD0);
>
> /* Wait till CMD DONE status */
> reg = 0;
> while (!reg) {
> - reg = readl(priv->spmi_core + SPMI_CH_OFFSET(channel) +
> + reg = readl(priv->spmi_chnls + SPMI_CH_OFFSET(channel) +
> SPMI_REG_STATUS);
> }
>
> @@ -186,47 +186,37 @@ static struct dm_spmi_ops msm_spmi_ops = {
> static int msm_spmi_probe(struct udevice *dev)
> {
> struct msm_spmi_priv *priv = dev_get_priv(dev);
> - phys_addr_t config_addr;
> + phys_addr_t core_addr;
> u32 hw_ver;
> - u32 version;
> int i;
> - int err;
>
> - config_addr = dev_read_addr_index(dev, 0);
> - priv->spmi_core = dev_read_addr_index(dev, 1);
> - priv->spmi_obs = dev_read_addr_index(dev, 2);
> + core_addr = dev_read_addr_name(dev, "core");
> + priv->spmi_chnls = dev_read_addr_name(dev, "chnls");
> + priv->spmi_obs = dev_read_addr_name(dev, "obsrvr");
>
> - hw_ver = readl(config_addr + PMIC_ARB_VERSION);
> + hw_ver = readl(core_addr + PMIC_ARB_VERSION);
>
> if (hw_ver < PMIC_ARB_VERSION_V3_MIN) {
> priv->arb_ver = V2;
> - version = 2;
> - priv->arb_chnl = config_addr + APID_MAP_OFFSET_V1_V2_V3;
> + priv->arb_chnl = core_addr + APID_MAP_OFFSET_V1_V2_V3;
> } else if (hw_ver < PMIC_ARB_VERSION_V5_MIN) {
> priv->arb_ver = V3;
> - version = 3;
> - priv->arb_chnl = config_addr + APID_MAP_OFFSET_V1_V2_V3;
> + priv->arb_chnl = core_addr + APID_MAP_OFFSET_V1_V2_V3;
> } else {
> priv->arb_ver = V5;
> - version = 5;
> - priv->arb_chnl = config_addr + APID_MAP_OFFSET_V5;
> -
> - if (err) {
> - dev_err(dev, "could not read APID->PPID mapping table, rc= %d\n", err);
> - return -1;
> - }
> + priv->arb_chnl = core_addr + APID_MAP_OFFSET_V5;
> }
>
> - dev_dbg(dev, "PMIC Arb Version-%d (0x%x)\n", version, hw_ver);
> + dev_dbg(dev, "PMIC Arb Version-%d (%#x)\n", hw_ver >> 28, hw_ver);
>
> if (priv->arb_chnl == FDT_ADDR_T_NONE ||
> - priv->spmi_core == FDT_ADDR_T_NONE ||
> + priv->spmi_chnls == FDT_ADDR_T_NONE ||
> priv->spmi_obs == FDT_ADDR_T_NONE)
> return -EINVAL;
>
> - dev_dbg(dev, "priv->arb_chnl address (%llu)\n", priv->arb_chnl);
> - dev_dbg(dev, "priv->spmi_core address (%llu)\n", priv->spmi_core);
> - dev_dbg(dev, "priv->spmi_obs address (%llu)\n", priv->spmi_obs);
> + dev_dbg(dev, "priv->arb_chnl address (%#08llx)\n", priv->arb_chnl);
> + dev_dbg(dev, "priv->spmi_chnls address (%#08llx)\n", priv->spmi_chnls);
> + dev_dbg(dev, "priv->spmi_obs address (%#08llx)\n", priv->spmi_obs);
> /* Scan peripherals connected to each SPMI channel */
> for (i = 0; i < SPMI_MAX_PERIPH; i++) {
> uint32_t periph = readl(priv->arb_chnl + ARB_CHANNEL_OFFSET(i));
>
Looks good:
Reviewed-by: Neil Armstrong <neil.armstrong at linaro.org>
More information about the U-Boot
mailing list