From: neil.armstrong@linaro.org
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
linux-amlogic@lists.infradead.org, linux-mmc@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, ulf.hansson@linaro.org
Subject: Re: [PATCH v1 1/7] mmc: meson-mx-sdio: Switch to regmap for register access
Date: Mon, 17 Nov 2025 10:08:58 +0100 [thread overview]
Message-ID: <2aef88d5-b9cb-4b2a-866e-f856e49c6b85@linaro.org> (raw)
In-Reply-To: <20251108231253.1641927-2-martin.blumenstingl@googlemail.com>
On 11/9/25 00:12, Martin Blumenstingl wrote:
> Switch the driver over to use regmap to access the registers. This makes
> it consistent with the other Amlogic MMC drivers. No functional changes
> intended.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/mmc/host/Kconfig | 1 +
> drivers/mmc/host/meson-mx-sdio.c | 125 ++++++++++++++++---------------
> 2 files changed, 64 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 2c963cb6724b..027ef2540913 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -504,6 +504,7 @@ config MMC_MESON_MX_SDIO
> depends on ARCH_MESON || COMPILE_TEST
> depends on COMMON_CLK
> depends on OF_ADDRESS
> + select REGMAP_MMIO
> help
> This selects support for the SD/MMC Host Controller on
> Amlogic Meson6, Meson8 and Meson8b SoCs.
> diff --git a/drivers/mmc/host/meson-mx-sdio.c b/drivers/mmc/host/meson-mx-sdio.c
> index 8a49c32fd3f9..2448f21bd683 100644
> --- a/drivers/mmc/host/meson-mx-sdio.c
> +++ b/drivers/mmc/host/meson-mx-sdio.c
> @@ -19,6 +19,7 @@
> #include <linux/ioport.h>
> #include <linux/platform_device.h>
> #include <linux/of_platform.h>
> +#include <linux/regmap.h>
> #include <linux/timer.h>
> #include <linux/types.h>
>
> @@ -108,7 +109,7 @@ struct meson_mx_mmc_host {
> struct clk_fixed_factor fixed_factor;
> struct clk *fixed_factor_clk;
>
> - void __iomem *base;
> + struct regmap *regmap;
> int irq;
> spinlock_t irq_lock;
>
> @@ -122,22 +123,10 @@ struct meson_mx_mmc_host {
> int error;
> };
>
> -static void meson_mx_mmc_mask_bits(struct mmc_host *mmc, char reg, u32 mask,
> - u32 val)
> -{
> - struct meson_mx_mmc_host *host = mmc_priv(mmc);
> - u32 regval;
> -
> - regval = readl(host->base + reg);
> - regval &= ~mask;
> - regval |= (val & mask);
> -
> - writel(regval, host->base + reg);
> -}
> -
> static void meson_mx_mmc_soft_reset(struct meson_mx_mmc_host *host)
> {
> - writel(MESON_MX_SDIO_IRQC_SOFT_RESET, host->base + MESON_MX_SDIO_IRQC);
> + regmap_write(host->regmap, MESON_MX_SDIO_IRQC,
> + MESON_MX_SDIO_IRQC_SOFT_RESET);
> udelay(2);
> }
>
> @@ -158,7 +147,7 @@ static void meson_mx_mmc_start_cmd(struct mmc_host *mmc,
> struct meson_mx_mmc_host *host = mmc_priv(mmc);
> unsigned int pack_size;
> unsigned long irqflags, timeout;
> - u32 mult, send = 0, ext = 0;
> + u32 send = 0, ext = 0;
>
> host->cmd = cmd;
>
> @@ -215,25 +204,22 @@ static void meson_mx_mmc_start_cmd(struct mmc_host *mmc,
>
> spin_lock_irqsave(&host->irq_lock, irqflags);
>
> - mult = readl(host->base + MESON_MX_SDIO_MULT);
> - mult &= ~MESON_MX_SDIO_MULT_PORT_SEL_MASK;
> - mult |= FIELD_PREP(MESON_MX_SDIO_MULT_PORT_SEL_MASK, host->slot_id);
> - mult |= BIT(31);
> - writel(mult, host->base + MESON_MX_SDIO_MULT);
> + regmap_update_bits(host->regmap, MESON_MX_SDIO_MULT,
> + MESON_MX_SDIO_MULT_PORT_SEL_MASK | BIT(31),
> + FIELD_PREP(MESON_MX_SDIO_MULT_PORT_SEL_MASK,
> + host->slot_id) | BIT(31));
>
> /* enable the CMD done interrupt */
> - meson_mx_mmc_mask_bits(mmc, MESON_MX_SDIO_IRQC,
> - MESON_MX_SDIO_IRQC_ARC_CMD_INT_EN,
> - MESON_MX_SDIO_IRQC_ARC_CMD_INT_EN);
> + regmap_set_bits(host->regmap, MESON_MX_SDIO_IRQC,
> + MESON_MX_SDIO_IRQC_ARC_CMD_INT_EN);
>
> /* clear pending interrupts */
> - meson_mx_mmc_mask_bits(mmc, MESON_MX_SDIO_IRQS,
> - MESON_MX_SDIO_IRQS_CMD_INT,
> - MESON_MX_SDIO_IRQS_CMD_INT);
> + regmap_set_bits(host->regmap, MESON_MX_SDIO_IRQS,
> + MESON_MX_SDIO_IRQS_CMD_INT);
>
> - writel(cmd->arg, host->base + MESON_MX_SDIO_ARGU);
> - writel(ext, host->base + MESON_MX_SDIO_EXT);
> - writel(send, host->base + MESON_MX_SDIO_SEND);
> + regmap_write(host->regmap, MESON_MX_SDIO_ARGU, cmd->arg);
> + regmap_write(host->regmap, MESON_MX_SDIO_EXT, ext);
> + regmap_write(host->regmap, MESON_MX_SDIO_SEND, send);
>
> spin_unlock_irqrestore(&host->irq_lock, irqflags);
>
> @@ -263,14 +249,13 @@ static void meson_mx_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>
> switch (ios->bus_width) {
> case MMC_BUS_WIDTH_1:
> - meson_mx_mmc_mask_bits(mmc, MESON_MX_SDIO_CONF,
> - MESON_MX_SDIO_CONF_BUS_WIDTH, 0);
> + regmap_clear_bits(host->regmap, MESON_MX_SDIO_CONF,
> + MESON_MX_SDIO_CONF_BUS_WIDTH);
> break;
>
> case MMC_BUS_WIDTH_4:
> - meson_mx_mmc_mask_bits(mmc, MESON_MX_SDIO_CONF,
> - MESON_MX_SDIO_CONF_BUS_WIDTH,
> - MESON_MX_SDIO_CONF_BUS_WIDTH);
> + regmap_set_bits(host->regmap, MESON_MX_SDIO_CONF,
> + MESON_MX_SDIO_CONF_BUS_WIDTH);
> break;
>
> case MMC_BUS_WIDTH_8:
> @@ -351,8 +336,8 @@ static void meson_mx_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
> host->mrq = mrq;
>
> if (mrq->data)
> - writel(sg_dma_address(mrq->data->sg),
> - host->base + MESON_MX_SDIO_ADDR);
> + regmap_write(host->regmap, MESON_MX_SDIO_ADDR,
> + sg_dma_address(mrq->data->sg));
>
> if (mrq->sbc)
> meson_mx_mmc_start_cmd(mmc, mrq->sbc);
> @@ -364,24 +349,26 @@ static void meson_mx_mmc_read_response(struct mmc_host *mmc,
> struct mmc_command *cmd)
> {
> struct meson_mx_mmc_host *host = mmc_priv(mmc);
> - u32 mult;
> - int i, resp[4];
> + unsigned int i, resp[4];
>
> - mult = readl(host->base + MESON_MX_SDIO_MULT);
> - mult |= MESON_MX_SDIO_MULT_WR_RD_OUT_INDEX;
> - mult &= ~MESON_MX_SDIO_MULT_RESP_READ_INDEX_MASK;
> - mult |= FIELD_PREP(MESON_MX_SDIO_MULT_RESP_READ_INDEX_MASK, 0);
> - writel(mult, host->base + MESON_MX_SDIO_MULT);
> + regmap_update_bits(host->regmap, MESON_MX_SDIO_MULT,
> + MESON_MX_SDIO_MULT_WR_RD_OUT_INDEX |
> + MESON_MX_SDIO_MULT_RESP_READ_INDEX_MASK,
> + MESON_MX_SDIO_MULT_WR_RD_OUT_INDEX |
> + FIELD_PREP(MESON_MX_SDIO_MULT_RESP_READ_INDEX_MASK,
> + 0));
>
> if (cmd->flags & MMC_RSP_136) {
> for (i = 0; i <= 3; i++)
> - resp[3 - i] = readl(host->base + MESON_MX_SDIO_ARGU);
> + regmap_read(host->regmap, MESON_MX_SDIO_ARGU,
> + &resp[3 - i]);
> +
> cmd->resp[0] = (resp[0] << 8) | ((resp[1] >> 24) & 0xff);
> cmd->resp[1] = (resp[1] << 8) | ((resp[2] >> 24) & 0xff);
> cmd->resp[2] = (resp[2] << 8) | ((resp[3] >> 24) & 0xff);
> cmd->resp[3] = (resp[3] << 8);
> } else if (cmd->flags & MMC_RSP_PRESENT) {
> - cmd->resp[0] = readl(host->base + MESON_MX_SDIO_ARGU);
> + regmap_read(host->regmap, MESON_MX_SDIO_ARGU, &cmd->resp[0]);
> }
> }
>
> @@ -422,8 +409,8 @@ static irqreturn_t meson_mx_mmc_irq(int irq, void *data)
>
> spin_lock(&host->irq_lock);
>
> - irqs = readl(host->base + MESON_MX_SDIO_IRQS);
> - send = readl(host->base + MESON_MX_SDIO_SEND);
> + regmap_read(host->regmap, MESON_MX_SDIO_IRQS, &irqs);
> + regmap_read(host->regmap, MESON_MX_SDIO_SEND, &send);
>
> if (irqs & MESON_MX_SDIO_IRQS_CMD_INT)
> ret = meson_mx_mmc_process_cmd_irq(host, irqs, send);
> @@ -431,7 +418,7 @@ static irqreturn_t meson_mx_mmc_irq(int irq, void *data)
> ret = IRQ_HANDLED;
>
> /* finally ACK all pending interrupts */
> - writel(irqs, host->base + MESON_MX_SDIO_IRQS);
> + regmap_write(host->regmap, MESON_MX_SDIO_IRQS, irqs);
>
> spin_unlock(&host->irq_lock);
>
> @@ -470,14 +457,13 @@ static void meson_mx_mmc_timeout(struct timer_list *t)
> struct meson_mx_mmc_host *host = timer_container_of(host, t,
> cmd_timeout);
> unsigned long irqflags;
> - u32 irqc;
> + u32 irqs, argu;
>
> spin_lock_irqsave(&host->irq_lock, irqflags);
>
> /* disable the CMD interrupt */
> - irqc = readl(host->base + MESON_MX_SDIO_IRQC);
> - irqc &= ~MESON_MX_SDIO_IRQC_ARC_CMD_INT_EN;
> - writel(irqc, host->base + MESON_MX_SDIO_IRQC);
> + regmap_clear_bits(host->regmap, MESON_MX_SDIO_IRQC,
> + MESON_MX_SDIO_IRQC_ARC_CMD_INT_EN);
>
> spin_unlock_irqrestore(&host->irq_lock, irqflags);
>
> @@ -488,10 +474,12 @@ static void meson_mx_mmc_timeout(struct timer_list *t)
> if (!host->cmd)
> return;
>
> + regmap_read(host->regmap, MESON_MX_SDIO_IRQS, &irqs);
> + regmap_read(host->regmap, MESON_MX_SDIO_ARGU, &argu);
> +
> dev_dbg(mmc_dev(host->mmc),
> "Timeout on CMD%u (IRQS = 0x%08x, ARGU = 0x%08x)\n",
> - host->cmd->opcode, readl(host->base + MESON_MX_SDIO_IRQS),
> - readl(host->base + MESON_MX_SDIO_ARGU));
> + host->cmd->opcode, irqs, argu);
>
> host->cmd->error = -ETIMEDOUT;
>
> @@ -578,7 +566,8 @@ static int meson_mx_mmc_add_host(struct meson_mx_mmc_host *host)
> return 0;
> }
>
> -static int meson_mx_mmc_register_clks(struct meson_mx_mmc_host *host)
> +static int meson_mx_mmc_register_clks(struct meson_mx_mmc_host *host,
> + void __iomem *base)
> {
> struct clk_init_data init;
> const char *clk_div_parent, *clk_fixed_factor_parent;
> @@ -613,7 +602,7 @@ static int meson_mx_mmc_register_clks(struct meson_mx_mmc_host *host)
> init.flags = CLK_SET_RATE_PARENT;
> init.parent_names = &clk_div_parent;
> init.num_parents = 1;
> - host->cfg_div.reg = host->base + MESON_MX_SDIO_CONF;
> + host->cfg_div.reg = base + MESON_MX_SDIO_CONF;
> host->cfg_div.shift = MESON_MX_SDIO_CONF_CMD_CLK_DIV_SHIFT;
> host->cfg_div.width = MESON_MX_SDIO_CONF_CMD_CLK_DIV_WIDTH;
> host->cfg_div.hw.init = &init;
> @@ -629,12 +618,23 @@ static int meson_mx_mmc_register_clks(struct meson_mx_mmc_host *host)
>
> static int meson_mx_mmc_probe(struct platform_device *pdev)
> {
> + const struct regmap_config meson_mx_sdio_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .max_register = MESON_MX_SDIO_EXT,
> + };
> struct platform_device *slot_pdev;
> struct mmc_host *mmc;
> struct meson_mx_mmc_host *host;
> + void __iomem *base;
> int ret, irq;
> u32 conf;
>
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> slot_pdev = meson_mx_mmc_slot_pdev(&pdev->dev);
> if (!slot_pdev)
> return -ENODEV;
> @@ -656,9 +656,10 @@ static int meson_mx_mmc_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, host);
>
> - host->base = devm_platform_ioremap_resource(pdev, 0);
> - if (IS_ERR(host->base)) {
> - ret = PTR_ERR(host->base);
> + host->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &meson_mx_sdio_regmap_config);
> + if (IS_ERR(host->regmap)) {
> + ret = PTR_ERR(host->regmap);
> goto error_free_mmc;
> }
>
> @@ -687,7 +688,7 @@ static int meson_mx_mmc_probe(struct platform_device *pdev)
> goto error_free_mmc;
> }
>
> - ret = meson_mx_mmc_register_clks(host);
> + ret = meson_mx_mmc_register_clks(host, base);
> if (ret)
> goto error_free_mmc;
>
> @@ -708,7 +709,7 @@ static int meson_mx_mmc_probe(struct platform_device *pdev)
> conf |= FIELD_PREP(MESON_MX_SDIO_CONF_M_ENDIAN_MASK, 0x3);
> conf |= FIELD_PREP(MESON_MX_SDIO_CONF_WRITE_NWR_MASK, 0x2);
> conf |= FIELD_PREP(MESON_MX_SDIO_CONF_WRITE_CRC_OK_STATUS_MASK, 0x2);
> - writel(conf, host->base + MESON_MX_SDIO_CONF);
> + regmap_write(host->regmap, MESON_MX_SDIO_CONF, conf);
>
> meson_mx_mmc_soft_reset(host);
>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
next prev parent reply other threads:[~2025-11-17 9:09 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-08 23:12 [PATCH v1 0/7] mmc: meson-mx-sdio: Various driver improvements Martin Blumenstingl
2025-11-08 23:12 ` [PATCH v1 1/7] mmc: meson-mx-sdio: Switch to regmap for register access Martin Blumenstingl
2025-11-17 9:08 ` neil.armstrong [this message]
2025-11-08 23:12 ` [PATCH v1 2/7] mmc: meson-mx-sdio: Use devm_clk_get_enabled() Martin Blumenstingl
2025-11-17 9:09 ` neil.armstrong
2025-11-08 23:12 ` [PATCH v1 3/7] mmc: meson-mx-sdio: Refactor internal clock initialization Martin Blumenstingl
2025-11-17 9:10 ` neil.armstrong
2025-11-08 23:12 ` [PATCH v1 4/7] mmc: meson-mx-sdio: Use devm_mmc_alloc_host() helper Martin Blumenstingl
2025-11-17 9:10 ` neil.armstrong
2025-11-08 23:12 ` [PATCH v1 5/7] mmc: meson-mx-sdio: Use dev_err_probe() where appropriate Martin Blumenstingl
2025-11-17 9:11 ` neil.armstrong
2025-11-08 23:12 ` [PATCH v1 6/7] mmc: meson-mx-sdio: Fix indentation in meson_mx_mmc_irq_thread() Martin Blumenstingl
2025-11-17 9:11 ` neil.armstrong
2025-11-08 23:12 ` [PATCH v1 7/7] mmc: meson-mx-sdio: Ignore disabled "mmc-slot" child-nodes Martin Blumenstingl
2025-11-18 15:31 ` [PATCH v1 0/7] mmc: meson-mx-sdio: Various driver improvements Ulf Hansson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2aef88d5-b9cb-4b2a-866e-f856e49c6b85@linaro.org \
--to=neil.armstrong@linaro.org \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=ulf.hansson@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®