[PATCH v3 4/5] spmi: msm: fix register range names

[Sumit Garg]

On Tue, 14 Nov 2023 at 19:18, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
> The core and chnl register ranges were swapped on SDM845. Fix it, and
> fetch the register ranges by name instead of by index.
>

You haven't updated qcs404-evb.dts to provide register names, so this
change alone will break that platform.

> Drop the cosmetic "version" variable and clean up the debug logging.
>
> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
> ---
>  arch/arm/dts/sdm845.dtsi |  2 +-
>  drivers/spmi/spmi-msm.c  | 46 ++++++++++++++++++----------------------------
>  2 files changed, 19 insertions(+), 29 deletions(-)
>
> 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>;

You should again drop custom u-boot bindings now:
doc/device-tree-bindings/spmi/spmi-msm.txt

>
> diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c
> index 27a035c0a595..f8834e60c266 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 core */

s/SPMI core/SPMI channels/

-Sumit

>         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));
>
> --
> 2.42.1
>