On Tue, 14 Nov 2023 at 19:18, Caleb Connolly <caleb.conno...@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.conno...@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
>
