| Message ID | 20231114-b4-qcom-dt-compat-v3-4-88a92f8f00ba@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Qualcomm PMIC fixes | expand |
On Tue, 14 Nov 2023 at 19:18, Caleb Connolly <caleb.connolly@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@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 >
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/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 */ 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));
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@linaro.org> --- arch/arm/dts/sdm845.dtsi | 2 +- drivers/spmi/spmi-msm.c | 46 ++++++++++++++++++---------------------------- 2 files changed, 19 insertions(+), 29 deletions(-)