[PATCH 0/4] smpi: msm: fix version 5 and add version 7 support
First, fix version 5 support by using the right ch_offset in then msm_spmi_write() reg accesses.
Then: - properly format command by importing helpers from Linux driver and use a switch/case to handle all versions in msm_spmi_write/read() command. - handle peripheral ownership by poking into the cnfg registers and mark periperal as read-only when the owner id doesn't match - finally add version 7 defines
SPMI Arbiter Version 7 is present on SM8450, SM8550 and SM8650 SoC.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- Neil Armstrong (4): spmi: msm: fix version 5 support spmi: msm: properly format command spmi: msm: handle peripheral ownership spmi: msm: support controller version 7
drivers/spmi/spmi-msm.c | 148 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 116 insertions(+), 32 deletions(-) --- base-commit: f0e6aba1218bca578605697eed8aa94582bf57bb change-id: 20240404-topic-sm8x50-spmi-fixes-aec9b392813b
Best regards,
Properly use ch_offset in msm_spmi_write() reg access.
Fixes: f5a2d6b4b03 ("spmi: msm: add arbiter version 5 support") Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- drivers/spmi/spmi-msm.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c index 5fe8a70abca..97383d8c7b8 100644 --- a/drivers/spmi/spmi-msm.c +++ b/drivers/spmi/spmi-msm.c @@ -93,12 +93,16 @@ static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off,
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_chnls + SPMI_CH_OFFSET(channel) + SPMI_REG_CONFIG); + writel(0x0, priv->spmi_chnls + ch_offset + SPMI_REG_CONFIG);
/* Write single byte */ - writel(val, priv->spmi_chnls + SPMI_CH_OFFSET(channel) + SPMI_REG_WDATA); + writel(val, priv->spmi_chnls + ch_offset + SPMI_REG_WDATA);
/* Prepare write command */ reg |= SPMI_CMD_EXT_REG_WRITE_LONG << SPMI_CMD_OPCODE_SHIFT; @@ -107,18 +111,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_chnls + SPMI_CH_OFFSET(channel) + SPMI_REG_CMD0); + writel(reg, priv->spmi_chnls + ch_offset + SPMI_REG_CMD0);
/* Wait till CMD DONE status */ reg = 0; while (!reg) { - reg = readl(priv->spmi_chnls + SPMI_CH_OFFSET(channel) + + reg = readl(priv->spmi_chnls + ch_offset + SPMI_REG_STATUS); }
Since version 2, the cmd format has changed, takes helpers from Linux driver and use a switch/case to handle all versions in msm_spmi_write/read() command.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- drivers/spmi/spmi-msm.c | 75 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 20 deletions(-)
diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c index 97383d8c7b8..68bb8a38c3c 100644 --- a/drivers/spmi/spmi-msm.c +++ b/drivers/spmi/spmi-msm.c @@ -78,6 +78,16 @@ struct msm_spmi_priv { u32 arb_ver; };
+static u32 pmic_arb_fmt_cmd_v1(u8 opc, u8 sid, u8 pid, u8 off) +{ + return (opc << 27) | (sid << 20) | (pid << 12) | (off << 4) | 1; +} + +static u32 pmic_arb_fmt_cmd_v2(u8 opc, u8 off) +{ + return (opc << 27) | (off << 4) | 1; +} + static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off, uint8_t val) { @@ -93,24 +103,35 @@ static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off,
channel = priv->channel_map[usid][pid];
- if (priv->arb_ver == V5) - ch_offset = SPMI_V5_RW_CH_OFFSET(channel); - else + dev_dbg(dev, "[%d:%d] %s: channel %d\n", usid, pid, __func__, channel); + + switch (priv->arb_ver) { + case V1: + ch_offset = SPMI_CH_OFFSET(channel); + + reg = pmic_arb_fmt_cmd_v1(SPMI_CMD_EXT_REG_WRITE_LONG, + usid, pid, off); + break; + + case V2: ch_offset = SPMI_CH_OFFSET(channel);
+ reg = pmic_arb_fmt_cmd_v2(SPMI_CMD_EXT_REG_WRITE_LONG, off); + break; + + case V5: + ch_offset = SPMI_V5_RW_CH_OFFSET(channel); + + reg = pmic_arb_fmt_cmd_v2(SPMI_CMD_EXT_REG_WRITE_LONG, off); + break; + } + /* Disable IRQ mode for the current channel*/ writel(0x0, priv->spmi_chnls + ch_offset + SPMI_REG_CONFIG);
/* Write single byte */ writel(val, priv->spmi_chnls + ch_offset + SPMI_REG_WDATA);
- /* Prepare write command */ - reg |= SPMI_CMD_EXT_REG_WRITE_LONG << SPMI_CMD_OPCODE_SHIFT; - reg |= (usid << SPMI_CMD_SLAVE_ID_SHIFT); - reg |= (pid << SPMI_CMD_ADDR_SHIFT); - reg |= (off << SPMI_CMD_ADDR_OFFSET_SHIFT); - reg |= 1; /* byte count */ - /* Send write command */ writel(reg, priv->spmi_chnls + ch_offset + SPMI_REG_CMD0);
@@ -143,21 +164,35 @@ static int msm_spmi_read(struct udevice *dev, int usid, int pid, int off)
channel = priv->channel_map[usid][pid];
- if (priv->arb_ver == V5) - ch_offset = SPMI_V5_OBS_CH_OFFSET(channel); - else + dev_dbg(dev, "[%d:%d] %s: channel %d\n", usid, pid, __func__, channel); + + switch (priv->arb_ver) { + case V1: + ch_offset = SPMI_CH_OFFSET(channel); + + /* Prepare read command */ + reg = pmic_arb_fmt_cmd_v1(SPMI_CMD_EXT_REG_READ_LONG, + usid, pid, off); + break; + + case V2: ch_offset = SPMI_CH_OFFSET(channel);
+ /* Prepare read command */ + reg = pmic_arb_fmt_cmd_v2(SPMI_CMD_EXT_REG_READ_LONG, off); + break; + + case V5: + ch_offset = SPMI_V5_OBS_CH_OFFSET(channel); + + /* Prepare read command */ + reg = pmic_arb_fmt_cmd_v2(SPMI_CMD_EXT_REG_READ_LONG, off); + break; + } + /* Disable IRQ mode for the current channel*/ writel(0x0, priv->spmi_obs + ch_offset + SPMI_REG_CONFIG);
- /* Prepare read command */ - reg |= SPMI_CMD_EXT_REG_READ_LONG << SPMI_CMD_OPCODE_SHIFT; - reg |= (usid << SPMI_CMD_SLAVE_ID_SHIFT); - reg |= (pid << SPMI_CMD_ADDR_SHIFT); - reg |= (off << SPMI_CMD_ADDR_OFFSET_SHIFT); - reg |= 1; /* byte count */ - /* Request read */ writel(reg, priv->spmi_obs + ch_offset + SPMI_REG_CMD0);
The cnfg registers provides the owner id for each peripheral, so we can use this id to check if we're allowed to write register to each peripherals.
Since the v5 can handle more peripherals, add the max_channels to scan more starting from version 5, make the channel_map store 32bit values and introduce the SPMI_CHANNEL_READ_ONLY flag to mark a peripheral as read-only.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- drivers/spmi/spmi-msm.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c index 68bb8a38c3c..46e2e09dc26 100644 --- a/drivers/spmi/spmi-msm.c +++ b/drivers/spmi/spmi-msm.c @@ -31,6 +31,8 @@ DECLARE_GLOBAL_DATA_PTR; #define SPMI_V5_OBS_CH_OFFSET(chnl) ((chnl) * 0x80) #define SPMI_V5_RW_CH_OFFSET(chnl) ((chnl) * 0x10000)
+#define SPMI_OWNERSHIP_PERIPH2OWNER(x) ((x) & 0x7) + #define SPMI_REG_CMD0 0x0 #define SPMI_REG_CONFIG 0x4 #define SPMI_REG_STATUS 0x8 @@ -49,9 +51,13 @@ DECLARE_GLOBAL_DATA_PTR; #define SPMI_STATUS_DONE 0x1
#define SPMI_MAX_CHANNELS 128 +#define SPMI_MAX_CHANNELS_V5 512 #define SPMI_MAX_SLAVES 16 #define SPMI_MAX_PERIPH 256
+#define SPMI_CHANNEL_READ_ONLY BIT(31) +#define SPMI_CHANNEL_MASK 0xffff + enum arb_ver { V1 = 1, V2, @@ -72,8 +78,11 @@ struct msm_spmi_priv { phys_addr_t arb_chnl; /* ARB channel mapping base */ phys_addr_t spmi_chnls; /* SPMI channels */ phys_addr_t spmi_obs; /* SPMI observer */ + phys_addr_t spmi_cnfg; /* SPMI config */ + u32 owner; /* Current owner */ + unsigned int max_channels; /* Max channels */ /* SPMI channel map */ - uint8_t channel_map[SPMI_MAX_SLAVES][SPMI_MAX_PERIPH]; + uint32_t channel_map[SPMI_MAX_SLAVES][SPMI_MAX_PERIPH]; /* SPMI bus arbiter version */ u32 arb_ver; }; @@ -100,8 +109,10 @@ static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off, return -EIO; if (pid >= SPMI_MAX_PERIPH) return -EIO; + if (priv->channel_map[usid][pid] & SPMI_CHANNEL_READ_ONLY) + return -EPERM;
- channel = priv->channel_map[usid][pid]; + channel = priv->channel_map[usid][pid] & SPMI_CHANNEL_MASK;
dev_dbg(dev, "[%d:%d] %s: channel %d\n", usid, pid, __func__, channel);
@@ -162,7 +173,7 @@ static int msm_spmi_read(struct udevice *dev, int usid, int pid, int off) if (pid >= SPMI_MAX_PERIPH) return -EIO;
- channel = priv->channel_map[usid][pid]; + channel = priv->channel_map[usid][pid] & SPMI_CHANNEL_MASK;
dev_dbg(dev, "[%d:%d] %s: channel %d\n", usid, pid, __func__, channel);
@@ -227,18 +238,23 @@ static int msm_spmi_probe(struct udevice *dev) 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"); + dev_read_u32(dev, "qcom,ee", &priv->owner);
hw_ver = readl(core_addr + PMIC_ARB_VERSION);
if (hw_ver < PMIC_ARB_VERSION_V3_MIN) { priv->arb_ver = V2; priv->arb_chnl = core_addr + APID_MAP_OFFSET_V1_V2_V3; + priv->max_channels = SPMI_MAX_CHANNELS; } else if (hw_ver < PMIC_ARB_VERSION_V5_MIN) { priv->arb_ver = V3; priv->arb_chnl = core_addr + APID_MAP_OFFSET_V1_V2_V3; + priv->max_channels = SPMI_MAX_CHANNELS; } else { priv->arb_ver = V5; priv->arb_chnl = core_addr + APID_MAP_OFFSET_V5; + priv->max_channels = SPMI_MAX_CHANNELS_V5; + priv->spmi_cnfg = dev_read_addr_name(dev, "cnfg"); }
dev_dbg(dev, "PMIC Arb Version-%d (%#x)\n", hw_ver >> 28, hw_ver); @@ -252,12 +268,21 @@ static int msm_spmi_probe(struct udevice *dev) 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++) { + for (i = 0; i < priv->max_channels; i++) { uint32_t periph = readl(priv->arb_chnl + ARB_CHANNEL_OFFSET(i)); uint8_t slave_id = (periph & 0xf0000) >> 16; uint8_t pid = (periph & 0xff00) >> 8;
priv->channel_map[slave_id][pid] = i; + + /* Mark channels read-only when from different owner */ + if (priv->arb_ver == V5) { + uint32_t cnfg = readl(priv->spmi_cnfg + ARB_CHANNEL_OFFSET(i)); + uint8_t owner = SPMI_OWNERSHIP_PERIPH2OWNER(cnfg); + + if (owner != priv->owner) + priv->channel_map[slave_id][pid] |= SPMI_CHANNEL_READ_ONLY; + } } return 0; }
Add the defines and support for SPMI arbiters version 7, which can handle up to 1024 peripherals, and can also drive a secondary bus which is not implemented yet.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org --- drivers/spmi/spmi-msm.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/drivers/spmi/spmi-msm.c b/drivers/spmi/spmi-msm.c index 46e2e09dc26..244de69b359 100644 --- a/drivers/spmi/spmi-msm.c +++ b/drivers/spmi/spmi-msm.c @@ -23,13 +23,17 @@ DECLARE_GLOBAL_DATA_PTR; #define PMIC_ARB_VERSION_V2_MIN 0x20010000 #define PMIC_ARB_VERSION_V3_MIN 0x30000000 #define PMIC_ARB_VERSION_V5_MIN 0x50000000 +#define PMIC_ARB_VERSION_V7_MIN 0x70000000
#define APID_MAP_OFFSET_V1_V2_V3 (0x800) #define APID_MAP_OFFSET_V5 (0x900) +#define APID_MAP_OFFSET_V7 (0x2000) #define ARB_CHANNEL_OFFSET(n) (0x4 * (n)) #define SPMI_CH_OFFSET(chnl) ((chnl) * 0x8000) #define SPMI_V5_OBS_CH_OFFSET(chnl) ((chnl) * 0x80) +#define SPMI_V7_OBS_CH_OFFSET(chnl) ((chnl) * 0x20) #define SPMI_V5_RW_CH_OFFSET(chnl) ((chnl) * 0x10000) +#define SPMI_V7_RW_CH_OFFSET(chnl) ((chnl) * 0x1000)
#define SPMI_OWNERSHIP_PERIPH2OWNER(x) ((x) & 0x7)
@@ -52,6 +56,7 @@ DECLARE_GLOBAL_DATA_PTR;
#define SPMI_MAX_CHANNELS 128 #define SPMI_MAX_CHANNELS_V5 512 +#define SPMI_MAX_CHANNELS_V7 1024 #define SPMI_MAX_SLAVES 16 #define SPMI_MAX_PERIPH 256
@@ -62,7 +67,8 @@ enum arb_ver { V1 = 1, V2, V3, - V5 = 5 + V5 = 5, + V7 = 7 };
/* @@ -133,6 +139,12 @@ static int msm_spmi_write(struct udevice *dev, int usid, int pid, int off, case V5: ch_offset = SPMI_V5_RW_CH_OFFSET(channel);
+ reg = pmic_arb_fmt_cmd_v2(SPMI_CMD_EXT_REG_WRITE_LONG, off); + break; + + case V7: + ch_offset = SPMI_V7_RW_CH_OFFSET(channel); + reg = pmic_arb_fmt_cmd_v2(SPMI_CMD_EXT_REG_WRITE_LONG, off); break; } @@ -196,6 +208,13 @@ static int msm_spmi_read(struct udevice *dev, int usid, int pid, int off) case V5: ch_offset = SPMI_V5_OBS_CH_OFFSET(channel);
+ /* Prepare read command */ + reg = pmic_arb_fmt_cmd_v2(SPMI_CMD_EXT_REG_READ_LONG, off); + break; + + case V7: + ch_offset = SPMI_V7_OBS_CH_OFFSET(channel); + /* Prepare read command */ reg = pmic_arb_fmt_cmd_v2(SPMI_CMD_EXT_REG_READ_LONG, off); break; @@ -250,10 +269,16 @@ static int msm_spmi_probe(struct udevice *dev) priv->arb_ver = V3; priv->arb_chnl = core_addr + APID_MAP_OFFSET_V1_V2_V3; priv->max_channels = SPMI_MAX_CHANNELS; - } else { + } else if (hw_ver < PMIC_ARB_VERSION_V7_MIN) { priv->arb_ver = V5; priv->arb_chnl = core_addr + APID_MAP_OFFSET_V5; - priv->max_channels = SPMI_MAX_CHANNELS_V5; + priv->max_channels = SPMI_MAX_CHANNELS; + priv->spmi_cnfg = dev_read_addr_name(dev, "cnfg"); + } else { + /* TOFIX: handle second bus */ + priv->arb_ver = V7; + priv->arb_chnl = core_addr + APID_MAP_OFFSET_V7; + priv->max_channels = SPMI_MAX_CHANNELS_V7; priv->spmi_cnfg = dev_read_addr_name(dev, "cnfg"); }
@@ -276,7 +301,7 @@ static int msm_spmi_probe(struct udevice *dev) priv->channel_map[slave_id][pid] = i;
/* Mark channels read-only when from different owner */ - if (priv->arb_ver == V5) { + if (priv->arb_ver == V5 || priv->arb_ver == V7) { uint32_t cnfg = readl(priv->spmi_cnfg + ARB_CHANNEL_OFFSET(i)); uint8_t owner = SPMI_OWNERSHIP_PERIPH2OWNER(cnfg);
On 05/04/2024 10:21, Neil Armstrong wrote:
First, fix version 5 support by using the right ch_offset in then msm_spmi_write() reg accesses.
Then:
- properly format command by importing helpers from Linux driver and use a switch/case to handle all versions in msm_spmi_write/read() command.
- handle peripheral ownership by poking into the cnfg registers and mark periperal as read-only when the owner id doesn't match
- finally add version 7 defines
SPMI Arbiter Version 7 is present on SM8450, SM8550 and SM8650 SoC.
Signed-off-by: Neil Armstrong neil.armstrong@linaro.org
Acked-by: Caleb Connolly caleb.connolly@linaro.org
Mateusz: do you prefer to take these? I'm just as happy to take then through the qcom tree.
Kind regards,
Neil Armstrong (4): spmi: msm: fix version 5 support spmi: msm: properly format command spmi: msm: handle peripheral ownership spmi: msm: support controller version 7
drivers/spmi/spmi-msm.c | 148 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 116 insertions(+), 32 deletions(-)
base-commit: f0e6aba1218bca578605697eed8aa94582bf57bb change-id: 20240404-topic-sm8x50-spmi-fixes-aec9b392813b
Best regards,
On Fri, 05 Apr 2024 10:21:52 +0200, Neil Armstrong wrote:
First, fix version 5 support by using the right ch_offset in then msm_spmi_write() reg accesses.
Then:
- properly format command by importing helpers from Linux driver and use a switch/case to handle all versions in msm_spmi_write/read() command.
- handle peripheral ownership by poking into the cnfg registers and mark periperal as read-only when the owner id doesn't match
- finally add version 7 defines
[...]
Applied, thanks!
[1/4] spmi: msm: fix version 5 support commit: c2de620d64d462c1404ca383ad55aecf3f5a972e [2/4] spmi: msm: properly format command commit: f0b604d9491dc92c9cb092a9e4c0fdf2036cdb8d [3/4] spmi: msm: handle peripheral ownership commit: 59e0482b5edc1fceded3967306561a57f60ba4b3 [4/4] spmi: msm: support controller version 7 commit: ee1d8aa5ecf77991d3bbcfea2d5b37d88bb19473
Best regards,
participants (2)
-
Caleb Connolly -
Neil Armstrong