Re: [PATCH v5 8/9] spmi: msm: fix register range names

2023-12-01 Thread Neil Armstrong

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 
---
  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@200f000 {

compatible = "qcom,spmi-pmic-arb";
-   reg = <0x200f000 0x1000
-  0x240 0x40
-  0x2c0 0x40>;
+   reg = <0x200f000 0x001000>,
+ <0x240 0x80>,
+ <0x2c0 0x80>;
+   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 = <0xc44 0x1100>,
  <0xc60 0x200>,
  <0xe60 0x10>;
-   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..
--- 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@200f000 {
-   compatible = "qcom,spmi-pmic-arb";
-   reg = <0x200f800 0x200 0x240 0x40 0x2c0 0x40>;
-   #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 = 

[PATCH v5 8/9] spmi: msm: fix register range names

2023-11-30 Thread Caleb Connolly
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 
---
 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@200f000 {
compatible = "qcom,spmi-pmic-arb";
-   reg = <0x200f000 0x1000
-  0x240 0x40
-  0x2c0 0x40>;
+   reg = <0x200f000 0x001000>,
+ <0x240 0x80>,
+ <0x2c0 0x80>;
+   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 = <0xc44 0x1100>,
  <0xc60 0x200>,
  <0xe60 0x10>;
-   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..
--- 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@200f000 {
-   compatible = "qcom,spmi-pmic-arb";
-   reg = <0x200f800 0x200 0x240 0x40 0x2c0 0x40>;
-   #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