On Mon, 16 Jan 2023 at 06:04, Alexey Minnekhanov alexeymin@postmarketos.org wrote:

Update spmi-msm documentation and example to reflect the current state of the driver.

Signed-off-by: Alexey Minnekhanov alexeymin@postmarketos.org

doc/device-tree-bindings/spmi/spmi-msm.txt | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)

Since our long term goal is to align with Linux DT bindings and use the DTS imported from Linux as is. So we should rather get rid of these redundant bindings in u-boot unless there isn't any u-boot specific separate bindings requirement which isn't the case here.

-Sumit

diff --git a/doc/device-tree-bindings/spmi/spmi-msm.txt b/doc/device-tree-bindings/spmi/spmi-msm.txt index ae47673b768b..cd0380b723b9 100644 --- a/doc/device-tree-bindings/spmi/spmi-msm.txt +++ b/doc/device-tree-bindings/spmi/spmi-msm.txt @@ -1,13 +1,17 @@ Qualcomm SPMI arbiter/bus driver

This is bus driver for Qualcomm chips that use SPMI to communicate with PMICs. +The SPMI PMIC Arbiter is found on Snapdragon chipsets. It is an SPMI +controller with wrapping arbitration logic to allow for multiple on-chip +devices to control a single SPMI master.

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)

• 1. PMIC arbiter core registers
• 2. SPMI configuration registers
• 3. SPMI read command (observer) registers

+- reg-names: "core", "cnfg", "obsrvr" for corresponding reg values

Optional properties (if not set by parent):

• #address-cells: 0x1 - childs slave ID address

@@ -18,9 +22,12 @@ Automatic detection of childs is currently not supported.

Example:

-spmi@200f000 { +spmi@fc4cf000 { compatible = "qcom,spmi-pmic-arb";

•   reg = <0x200f800 0x200 0x2400000 0x400000 0x2c00000 0x400000>;
  

•   #address-cells = <0x1>;
  

•   #size-cells = <0x1>;
  

•   reg = <0xfc4cf000 0x1000>,
  

•         <0xfc4cb000 0x1000>,
  

•         <0xfc4ca000 0x1000>;
  

•   reg-names = "core", "cnfg", "obsrvr";
  

•   #address-cells = <1>;
  

•   #size-cells = <1>;
  

};

2.38.2

On Mon, 16 Jan 2023 at 06:04, Alexey Minnekhanov alexeymin@postmarketos.org wrote:

Now that reg-names is required, specify them, and use correct addresses for SPMI arbiter regs, taken from Linux dts [1] [2].

[1] https://elixir.bootlin.com/linux/v6.1.5/source/arch/arm64/boot/dts/qcom/qcs4... [2] https://elixir.bootlin.com/linux/v6.1.5/source/arch/arm64/boot/dts/qcom/sdm8...

Since you have already aligned the u-boot driver to work with Linux DT binding, we should rather exactly match spmi DT nodes as in Linux dts files.

-Sumit

Signed-off-by: Alexey Minnekhanov alexeymin@postmarketos.org

arch/arm/dts/qcs404-evb.dts | 7 ++++--- arch/arm/dts/sdm845.dtsi | 8 ++++---- 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/arm/dts/qcs404-evb.dts b/arch/arm/dts/qcs404-evb.dts index 0639af8fe336..6bf6736139ed 100644 --- a/arch/arm/dts/qcs404-evb.dts +++ b/arch/arm/dts/qcs404-evb.dts @@ -171,9 +171,10 @@

            spmi@200f000 {
                    compatible = "qcom,spmi-pmic-arb";

•                   reg = <0x200f000 0x1000
  

•                          0x2400000 0x400000
  

•                          0x2c00000 0x400000>;
  

•                   reg = <0x0200f000 0x1000>,
  

•                         <0x0200a000 0x2100>,
  

•                         <0x02c00000 0x800000>;
  

•                   reg-names = "core", "cnfg", "obsrvr";
                    #address-cells = <0x1>;
                    #size-cells = <0x1>;
  
  

diff --git a/arch/arm/dts/sdm845.dtsi b/arch/arm/dts/sdm845.dtsi index 607af277f8be..2bb0954133c5 100644 --- a/arch/arm/dts/sdm845.dtsi +++ b/arch/arm/dts/sdm845.dtsi @@ -65,10 +65,10 @@

            spmi@c440000 {
                    compatible = "qcom,spmi-pmic-arb";

•                   reg = <0xc440000 0x1100>,
  

•                         <0xc600000 0x2000000>,
  

•                         <0xe600000 0x100000>;
  

•                   reg-names = "cnfg", "core", "obsrvr";
  

•                   reg = <0x0c440000 0x1100>,
  

•                         <0x0c40a000 0x26000>,
  

•                         <0x0e600000 0x100000>;
  

•                   reg-names = "core", "cnfg", "obsrvr";
                    #address-cells = <0x1>;
                    #size-cells = <0x1>;
  
  

-- 2.38.2

On Mon, 16 Jan 2023 at 06:04, Alexey Minnekhanov alexeymin@postmarketos.org wrote:

In commit f5a2d6b4b03a ("spmi: msm: add arbiter version 5 support") support for arbiter V5 was introduced, and msm_spmi_read() was correctly converted to use varying channel offset depending on ARB version. But msm_spmi_write() was not fully converted.

Even though ch_offset variable was introduced, it was not used in read/write operations in that function. Fix this inconsistency, use calculated ch_offset instead of SPMI_CH_OFFSET(..).

Fixes: f5a2d6b4b03a ("spmi: msm: add arbiter version 5 support")

Signed-off-by: Alexey Minnekhanov alexeymin@postmarketos.org

drivers/spmi/spmi-msm.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)

Reviewed-by: Sumit Garg sumit.garg@linaro.org

-Sumit

diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c index 3df0f12c8b86..2174c10c920a 100644 --- a/drivers/spmi/spmi-msm.c +++ b/drivers/spmi/spmi-msm.c @@ -92,13 +92,16 @@ static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off, return -EIO;

    channel = priv->channel_map[usid][pid];

•   if (priv->arb_ver == V5)
  

•           ch_offset = SPMI_V5_RW_CH_OFFSET(channel);
  

•   else
  

•           ch_offset = SPMI_CH_OFFSET(channel);
  
    /* Disable IRQ mode for the current channel*/
  

•   writel(0x0,
  

•          priv->spmi_core + SPMI_CH_OFFSET(channel) + SPMI_REG_CONFIG);
  

•   writel(0x0, priv->spmi_core + ch_offset + SPMI_REG_CONFIG);
  
    /* Write single byte */
  

•   writel(val, priv->spmi_core + SPMI_CH_OFFSET(channel) + SPMI_REG_WDATA);
  

•   writel(val, priv->spmi_core + ch_offset + SPMI_REG_WDATA);
  
    /* Prepare write command */
    reg |= SPMI_CMD_EXT_REG_WRITE_LONG << SPMI_CMD_OPCODE_SHIFT;
  

@@ -107,19 +110,13 @@ static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off, reg |= (off << SPMI_CMD_ADDR_OFFSET_SHIFT); reg |= 1; /* byte count */

•   if (priv->arb_ver == V5)
  

•           ch_offset = SPMI_V5_RW_CH_OFFSET(channel);
  

•   else
  

•           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_core + ch_offset + SPMI_REG_CMD0);
  
    /* Wait till CMD DONE status */
    reg = 0;
    while (!reg) {
  

•           reg = readl(priv->spmi_core + SPMI_CH_OFFSET(channel) +
  

•                       SPMI_REG_STATUS);
  

•           reg = readl(priv->spmi_core + ch_offset + SPMI_REG_STATUS);
    }
  
    if (reg ^ SPMI_STATUS_DONE) {
  

-- 2.38.2