[PATCH 2/5] spmi: msm: Fix parsing FDT and reading ARB version

[Sumit Garg]

On Mon, 16 Jan 2023 at 06:03, Alexey Minnekhanov
<alexeymin at postmarketos.org> wrote:
>
> First of all, use dev_read_addr_name() instead of
> dev_read_addr_index() to avoid confusion: most dts files
> have their regs specified in the wrong order, so driver
> is reading config reg and using it instead of core reg.
> Using names instead of indexes helps to avoid such errors.
>
> Second, same as Linux driver, use core reg to read version
> from [1]. This fixes reading incorrect arbiter version.
>
> Third, print addresses in hex, so it can be visually
> compared to values in DTS more easily.
>
> [1]: https://elixir.bootlin.com/linux/v6.1.6/source/drivers/spmi/spmi-pmic-arb.c#L1339
>
> Signed-off-by: Alexey Minnekhanov <alexeymin at postmarketos.org>
> ---
>  drivers/spmi/spmi-msm.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)

Apart from nit below, feel free to add:

Reviewed-by: Sumit Garg <sumit.garg at linaro.org>

>
> diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c
> index a9dcf5ab7f91..3df0f12c8b86 100644
> --- a/drivers/spmi/spmi-msm.c
> +++ b/drivers/spmi/spmi-msm.c
> @@ -191,11 +191,12 @@ static int msm_spmi_probe(struct udevice *dev)
>         u32 version;
>         int i;
>
> -       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);
> +       /* DTS bindings: reg-names = "cnfg", "core", "obsrvr"; */

nit: this comment is redundant as there is already DT bindings
documentation to look at.

-Sumit

> +       config_addr = dev_read_addr_name(dev, "cnfg");
> +       priv->spmi_core = dev_read_addr_name(dev, "core");
> +       priv->spmi_obs = dev_read_addr_name(dev, "obsrvr");
>
> -       hw_ver = readl(config_addr + PMIC_ARB_VERSION);
> +       hw_ver = readl(priv->spmi_core + PMIC_ARB_VERSION);
>
>         if (hw_ver < PMIC_ARB_VERSION_V3_MIN) {
>                 priv->arb_ver = V2;
> @@ -218,9 +219,10 @@ static int msm_spmi_probe(struct udevice *dev)
>             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 (%llx)\n", priv->arb_chnl);
> +       dev_dbg(dev, "priv->spmi_core address (%llx)\n", priv->spmi_core);
> +       dev_dbg(dev, "priv->spmi_obs address (%llx)\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.38.2
>