Unaligned access in drivers/mmc/host/sdhci-of-k1.c (arch:ariscv soc:spacemit k1)

Unaligned access in drivers/mmc/host/sdhci-of-k1.c (arch:ariscv soc:spacemit k1)

[Mohamed via Bugspray Bot]

Mohamed writes via Kernel.org Bugzilla:

There is an unaligned access in spacemit_sdhci_set_uhs_signaling that calls spacemit_sdhci_setbits(host, SDHCI_CTRL_VDD_180, SDHCI_HOST_CONTROL2);

As SDHCI_HOST_CONTROL2	0x3E the helper function spacemit_sdhci_setbits uses readl and writel functions, 
ie.
/* All helper functions will update clr/set while preserve rest bits */
static inline void spacemit_sdhci_setbits(struct sdhci_host *host, u32 val, int reg)
{
	sdhci_writel(host, sdhci_readl(host, reg) | val, reg);
}

So you get an unaligned access exception/panic/oops with cause 5 on the read.

To reproduce this you need to enable the emmc in the dtb as none of the boards have this enabled. It seems a bit strange that the commit message says that the emmc is working, as it is disabled in the emmc, yet there is working version based on 6.6 which does work. Obviously, it must be my mistake as the commit message cannot possibly lie and say that a driver is complete and working.

View: https://bugzilla.kernel.org/show_bug.cgi?id=220841#c0
You can reply to this message to join the discussion.
-- 
Deet-doot-dot, I am a bot.
Kernel.org Bugzilla (bugspray 0.1-dev)

Re: Unaligned access in drivers/mmc/host/sdhci-of-k1.c (arch:ariscv soc:spacemit k1)

[Adrian Hunter]

On 05/12/2025 23:00, Mohamed via Bugspray Bot wrote:
> Mohamed writes via Kernel.org Bugzilla:
> 
> There is an unaligned access in spacemit_sdhci_set_uhs_signaling that calls spacemit_sdhci_setbits(host, SDHCI_CTRL_VDD_180, SDHCI_HOST_CONTROL2);
> 
> As SDHCI_HOST_CONTROL2	0x3E the helper function spacemit_sdhci_setbits uses readl and writel functions, 
> ie.
> /* All helper functions will update clr/set while preserve rest bits */
> static inline void spacemit_sdhci_setbits(struct sdhci_host *host, u32 val, int reg)
> {
> 	sdhci_writel(host, sdhci_readl(host, reg) | val, reg);
> }
> 
> So you get an unaligned access exception/panic/oops with cause 5 on the read.
> 
> To reproduce this you need to enable the emmc in the dtb as none of the boards have this enabled. It seems a bit strange that the commit message says that the emmc is working, as it is disabled in the emmc, yet there is working version based on 6.6 which does work. Obviously, it must be my mistake as the commit message cannot possibly lie and say that a driver is complete and working.
> 
> View: https://bugzilla.kernel.org/show_bug.cgi?id=220841#c0
> You can reply to this message to join the discussion.

Adding Yixun Lan <dlan@gentoo.org>

Needs something like:

diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
index 0cc97e23a2f9..634a362fd66a 100644
--- a/drivers/mmc/host/sdhci-of-k1.c
+++ b/drivers/mmc/host/sdhci-of-k1.c
@@ -112,8 +112,12 @@ static void spacemit_sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned i
 
 	sdhci_set_uhs_signaling(host, timing);
 
-	if (!(host->mmc->caps2 & MMC_CAP2_NO_SDIO))
-		spacemit_sdhci_setbits(host, SDHCI_CTRL_VDD_180, SDHCI_HOST_CONTROL2);
+	if (!(host->mmc->caps2 & MMC_CAP2_NO_SDIO)) {
+		u16 ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
+
+		ctrl_2 |= SDHCI_CTRL_VDD_180;
+		sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
+	}
 }
 
 static void spacemit_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)

Re: Unaligned access in drivers/mmc/host/sdhci-of-k1.c (arch:ariscv soc:spacemit k1)

[Yixun Lan]

Hi Mohamed, Adrian:

On 09:35 Tue 09 Dec     , Adrian Hunter wrote:
> On 05/12/2025 23:00, Mohamed via Bugspray Bot wrote:
> > Mohamed writes via Kernel.org Bugzilla:
> > 
> > There is an unaligned access in spacemit_sdhci_set_uhs_signaling that calls spacemit_sdhci_setbits(host, SDHCI_CTRL_VDD_180, SDHCI_HOST_CONTROL2);
> > 
> > As SDHCI_HOST_CONTROL2	0x3E the helper function spacemit_sdhci_setbits uses readl and writel functions, 
> > ie.
> > /* All helper functions will update clr/set while preserve rest bits */
> > static inline void spacemit_sdhci_setbits(struct sdhci_host *host, u32 val, int reg)
> > {
> > 	sdhci_writel(host, sdhci_readl(host, reg) | val, reg);
> > }
> > 
> > So you get an unaligned access exception/panic/oops with cause 5 on the read.
> > 
> > To reproduce this you need to enable the emmc in the dtb as none of the boards have this enabled. It seems a bit strange that the commit message says that the emmc is working, as it is disabled in the emmc, yet there is working version based on 6.6 which does work. Obviously, it must be my mistake as the commit message cannot possibly lie and say that a driver is complete and working.
> > 
> > View: https://bugzilla.kernel.org/show_bug.cgi?id=220841#c0
> > You can reply to this message to join the discussion.
> 
> Adding Yixun Lan <dlan@gentoo.org>
> 
> Needs something like:
> 
> diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
> index 0cc97e23a2f9..634a362fd66a 100644
> --- a/drivers/mmc/host/sdhci-of-k1.c
> +++ b/drivers/mmc/host/sdhci-of-k1.c
> @@ -112,8 +112,12 @@ static void spacemit_sdhci_set_uhs_signaling(struct sdhci_host *host, unsigned i
>  
>  	sdhci_set_uhs_signaling(host, timing);
>  
> -	if (!(host->mmc->caps2 & MMC_CAP2_NO_SDIO))
the code path is targeting for sdio, which we haven't fully implemented for now
in the mainline code, and it's even skipped with "no-sdio" property in emmc DT node

> -		spacemit_sdhci_setbits(host, SDHCI_CTRL_VDD_180, SDHCI_HOST_CONTROL2);
> +	if (!(host->mmc->caps2 & MMC_CAP2_NO_SDIO)) {
> +		u16 ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
> +		ctrl_2 |= SDHCI_CTRL_VDD_180;
> +		sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
It's might not enough to get sd/sdio work with above change.. 
or did you test the patch with sdhci1 controller using sdio(wifi, for exampe)?
if it works, then worth the effort to push the fix..

btw, we have plan to work on follow-up patches to support sd/sdio, stay tuned

-- 
Yixun Lan (dlan)