[PATCH v2 4/7] spmi: apple: Implement remaining commands
Janne Grunau
j at jannau.net
Sun Aug 2 04:18:53 PDT 2026
On Tue, Jul 28, 2026 at 11:28:10AM +0200, Sasha Finkelstein wrote:
> From: Alba Mendez <me at alba.sh>
>
> Add support for zero write and power management commands
>
> Signed-off-by: Alba Mendez <me at alba.sh>
> Signed-off-by: Sasha Finkelstein <k at chaosmail.tech>
> ---
> drivers/spmi/spmi-apple-controller.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------
> 1 file changed, 68 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/spmi/spmi-apple-controller.c b/drivers/spmi/spmi-apple-controller.c
> index 4678a9e5270a..aed18b7df15c 100644
> --- a/drivers/spmi/spmi-apple-controller.c
> +++ b/drivers/spmi/spmi-apple-controller.c
> @@ -41,9 +41,9 @@ struct apple_spmi {
> readl_poll_timeout((spmi)->regs + (reg), (val), (cond), \
> REG_POLL_INTERVAL_US, REG_POLL_TIMEOUT_US)
>
> -static inline u32 apple_spmi_pack_cmd(u8 opc, u8 sid, u16 saddr, size_t len)
> +static inline u32 apple_spmi_pack_cmd(u8 opc, u8 sid, u16 param)
> {
> - return opc | sid << 8 | saddr << 16 | (len - 1) | (1 << 15);
> + return opc | sid << 8 | (u32)param << 16 | (1 << 15);
> }
>
> /* Wait for Rx FIFO to have something */
> @@ -63,18 +63,27 @@ static int apple_spmi_wait_rx_not_empty(struct spmi_controller *ctrl)
> return 0;
> }
>
> -static int spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> - u16 saddr, u8 *buf, size_t len)
> +static int spmi_raw_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> + u16 param, const u8 *buf, size_t len, u8 *ibuf, size_t ilen)
> {
> struct apple_spmi *spmi = spmi_controller_get_drvdata(ctrl);
> - u32 spmi_cmd = apple_spmi_pack_cmd(opc, sid, saddr, len);
> + u32 spmi_cmd = apple_spmi_pack_cmd(opc, sid, param);
> u32 reply, rsp;
> size_t len_read = 0;
> - u8 i;
> + size_t i = 0, j;
> int ret;
>
> writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);
>
> + while (i < len) {
> + j = 0;
> + spmi_cmd = 0;
> + while ((j < 4) & (i < len))
> + spmi_cmd |= buf[i++] << (j++ * 8);
spmi_cmd = 0;
size_t bytes = min_t(size_t, 4, len - i);
memcpy(&spmi_cmd, buf + i, bytes);
i += bytes;
Is easier to read and it's extremely unlikely that this driver will ever
be used on big-endian system.
> +
> + writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);
> + }
> +
> ret = apple_spmi_wait_rx_not_empty(ctrl);
> if (ret)
> return ret;
> @@ -82,15 +91,15 @@ static int spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> reply = readl(spmi->regs + SPMI_RSP_REG);
>
> /* Read SPMI data reply */
> - while (len_read < len) {
> + while (len_read < ilen) {
> if (readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY) {
> dev_err(&ctrl->dev, "FIFO lacks reply data, controller stuck?\n");
> return -EIO;
> }
> rsp = readl(spmi->regs + SPMI_RSP_REG);
> i = 0;
> - while ((len_read < len) && (i < 4)) {
> - buf[len_read++] = ((0xff << (8 * i)) & rsp) >> (8 * i);
> + while ((len_read < ilen) && (i < 4)) {
> + ibuf[len_read++] = ((0xffU << (8 * i)) & rsp) >> (8 * i);
This could be rewritten with memcpy as well
> i += 1;
> }
> }
> @@ -98,47 +107,67 @@ static int spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> if (!(readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY))
> dev_warn(&ctrl->dev, "FIFO has extra data\n");
>
> - if ((~reply >> SPMI_REPLY_FRAME_PARITY_OFFSET) & ((1 << len) - 1)) {
> + if (!ilen && !(reply & SPMI_REPLY_ACK)) {
> + dev_err(&ctrl->dev, "command not acknowledged\n");
> + return -EIO;
> + }
> + if ((~reply >> SPMI_REPLY_FRAME_PARITY_OFFSET) & ((1 << ilen) - 1)) {
Is the parity check useful for write commands? I'm not sure if packing
all command processing into a single function is a good idea for code
readability. The diff is annoying to read.
> dev_err(&ctrl->dev, "some frames failed parity check\n");
> return -EIO;
> }
> return 0;
> }
>
> -static int spmi_write_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> - u16 saddr, const u8 *buf, size_t len)
> +/* Send a raw command with 1..16 input data frames */
> +static int spmi_raw_cmd_input(struct spmi_controller *ctrl, u8 opc, u8 sid,
> + u16 param, u8 *buf, size_t len)
> {
> - struct apple_spmi *spmi = spmi_controller_get_drvdata(ctrl);
> - u32 spmi_cmd = apple_spmi_pack_cmd(opc, sid, saddr, len);
> - u32 reply;
> - size_t i = 0, j;
> - int ret;
> -
> - writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);
> + return spmi_raw_cmd(ctrl, opc, sid, param, NULL, 0, buf, len);
> +}
>
> - while (i < len) {
> - j = 0;
> - spmi_cmd = 0;
> - while ((j < 4) & (i < len))
> - spmi_cmd |= buf[i++] << (j++ * 8);
> +/* Send a raw command with (optional) body and an input ACK */
> +static int spmi_raw_cmd_ack(struct spmi_controller *ctrl, u8 opc, u8 sid,
> + u16 param, const u8 *buf, size_t len)
> +{
> + return spmi_raw_cmd(ctrl, opc, sid, param, buf, len, NULL, 0);
> +}
>
> - writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);
> +static int spmi_read_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> + u16 saddr, u8 *buf, size_t len)
> +{
> + switch (opc) {
> + case SPMI_CMD_EXT_READ:
> + case SPMI_CMD_EXT_READL:
> + return spmi_raw_cmd_input(ctrl, opc | (len - 1), sid, saddr, buf, len);
> + case SPMI_CMD_READ:
> + return spmi_raw_cmd_input(ctrl, opc | saddr, sid, saddr, buf, len);
> }
> + return -EINVAL;
> +}
>
> - ret = apple_spmi_wait_rx_not_empty(ctrl);
> - if (ret)
> - return ret;
> -
> - reply = readl(spmi->regs + SPMI_RSP_REG);
> -
> - if (!(readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY))
> - dev_warn(&ctrl->dev, "FIFO has extra data\n");
> -
> - if (!(reply & SPMI_REPLY_ACK)) {
> - dev_err(&ctrl->dev, "command not acknowledged\n");
> - return -EIO;
> +static int spmi_write_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
> + u16 saddr, const u8 *buf, size_t len)
> +{
> + switch (opc) {
> + case SPMI_CMD_WRITE:
> + return spmi_raw_cmd_ack(ctrl, opc | saddr, sid, buf[0] << 8 | saddr, NULL, 0);
> + case SPMI_CMD_ZERO_WRITE:
> + return spmi_raw_cmd_ack(ctrl, opc | buf[0], sid, buf[0] << 8 | saddr, NULL, 0);
> + case SPMI_CMD_EXT_WRITE:
> + case SPMI_CMD_EXT_WRITEL:
> + return spmi_raw_cmd_ack(ctrl, opc | (len - 1), sid, saddr, buf, len);
> }
> - return 0;
> + return -EINVAL;
> +}
> +
> +static int spmi_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
> +{
> + if (
> + opc == SPMI_CMD_RESET || opc == SPMI_CMD_SLEEP ||
> + opc == SPMI_CMD_SHUTDOWN || opc == SPMI_CMD_WAKEUP
> + )
no! a switch like for read/write would be nicer to read but this
formatting is just no.
> + return spmi_raw_cmd_ack(ctrl, opc, sid, 0, NULL, 0);
> + return -EINVAL;
> }
>
> static int apple_spmi_probe(struct platform_device *pdev)
> @@ -161,6 +190,7 @@ static int apple_spmi_probe(struct platform_device *pdev)
>
> ctrl->read_cmd = spmi_read_cmd;
> ctrl->write_cmd = spmi_write_cmd;
> + ctrl->cmd = spmi_cmd;
>
> ret = devm_spmi_controller_add(&pdev->dev, ctrl);
> if (ret)
Janne
More information about the linux-arm-kernel
mailing list