| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <acvqiaaptw4s54zekvbkdpfzam4x7juiajcxyohzpfcuf5xpxs@addpa5x73efo>
Date: Wed, 7 May 2025 20:09:25 +0800
From: Inochi Amaoto <inochiama@...il.com>
To: Yixun Lan <dlan@...too.org>, Inochi Amaoto <inochiama@...il.com>
Cc: Ulf Hansson <ulf.hansson@...aro.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Alex Elder <elder@...cstar.com>, linux-mmc@...r.kernel.org,
devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org, spacemit@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] mmc: sdhci-of-k1: add support for SpacemiT K1 SoC
On Tue, May 06, 2025 at 10:57:20PM +0000, Yixun Lan wrote:
> Hi Inochi,
>
> On 06:37 Wed 07 May , Inochi Amaoto wrote:
> > On Thu, May 01, 2025 at 04:50:22PM +0800, Yixun Lan wrote:
> > > The SDHCI controller found in SpacemiT K1 SoC features SD,
> > > SDIO, eMMC support, such as:
> > >
> > > - Compatible for 4-bit SDIO 3.0 UHS-I protocol, up to SDR104
> > > - Compatible for 4-bit SD 3.0 UHS-I protocol, up to SDR104
> > > - Compatible for 8bit eMMC5.1, up to HS400
> > >
> > > Signed-off-by: Yixun Lan <dlan@...too.org>
> > > ---
> > > drivers/mmc/host/Kconfig | 14 ++
> > > drivers/mmc/host/Makefile | 1 +
> > > drivers/mmc/host/sdhci-of-k1.c | 306 +++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 321 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> > > index 6824131b69b188cae58c8f48076715ca647ca28c..0ce78f22c33cfff916a2d4d680c79e9d19637e0e 100644
> > > --- a/drivers/mmc/host/Kconfig
> > > +++ b/drivers/mmc/host/Kconfig
> > > @@ -250,6 +250,20 @@ config MMC_SDHCI_OF_DWCMSHC
> > > If you have a controller with this interface, say Y or M here.
> > > If unsure, say N.
> > >
> > > +config MMC_SDHCI_OF_K1
> > > + tristate "SDHCI OF support for the SpacemiT K1 SoC"
> > > + depends on ARCH_SPACEMIT || COMPILE_TEST
> > > + depends on MMC_SDHCI_PLTFM
> > > + depends on OF
> > > + depends on COMMON_CLK
> > > + help
> > > + This selects the Secure Digital Host Controller Interface (SDHCI)
> > > + found in the SpacemiT K1 SoC.
> > > +
> > > + If you have a controller with this interface, say Y or M here.
> > > +
> > > + If unsure, say N.
> > > +
> > > config MMC_SDHCI_OF_SPARX5
> > > tristate "SDHCI OF support for the MCHP Sparx5 SoC"
> > > depends on MMC_SDHCI_PLTFM
> > > diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> > > index 5147467ec825ffaef3a7bd812fad80545e52b180..75bafc7b162b9e1d4c6c050f5d28b9d7cb582447 100644
> > > --- a/drivers/mmc/host/Makefile
> > > +++ b/drivers/mmc/host/Makefile
> > > @@ -88,6 +88,7 @@ obj-$(CONFIG_MMC_SDHCI_OF_AT91) += sdhci-of-at91.o
> > > obj-$(CONFIG_MMC_SDHCI_OF_ESDHC) += sdhci-of-esdhc.o
> > > obj-$(CONFIG_MMC_SDHCI_OF_HLWD) += sdhci-of-hlwd.o
> > > obj-$(CONFIG_MMC_SDHCI_OF_DWCMSHC) += sdhci-of-dwcmshc.o
> > > +obj-$(CONFIG_MMC_SDHCI_OF_K1) += sdhci-of-k1.o
> > > obj-$(CONFIG_MMC_SDHCI_OF_SPARX5) += sdhci-of-sparx5.o
> > > obj-$(CONFIG_MMC_SDHCI_OF_MA35D1) += sdhci-of-ma35d1.o
> > > obj-$(CONFIG_MMC_SDHCI_BCM_KONA) += sdhci-bcm-kona.o
> > > diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..8988053eeb33a476fa484d145579db6214b2d0b7
> > > --- /dev/null
> > > +++ b/drivers/mmc/host/sdhci-of-k1.c
> > > @@ -0,0 +1,306 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2023-2025 SpacemiT (Hangzhou) Technology Co. Ltd
> > > + * Copyright (c) 2025 Yixun Lan <dlan@...too.org>
> > > + */
> > > +
> > > +#include <linux/bitfield.h>
> > > +#include <linux/clk.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/init.h>
> > > +#include <linux/mmc/card.h>
> > > +#include <linux/mmc/host.h>
> > > +#include <linux/mmc/mmc.h>
> > > +#include <linux/module.h>
> > > +#include <linux/of.h>
> > > +#include <linux/of_device.h>
> > > +#include <linux/platform_device.h>
> > > +
> > > +#include "sdhci.h"
> > > +#include "sdhci-pltfm.h"
> > > +
> > > +#define SDHC_MMC_CTRL_REG 0x114
> > > +#define MISC_INT_EN BIT(1)
> > > +#define MISC_INT BIT(2)
> > > +#define ENHANCE_STROBE_EN BIT(8)
> > > +#define MMC_HS400 BIT(9)
> > > +#define MMC_HS200 BIT(10)
> > > +#define MMC_CARD_MODE BIT(12)
> > > +
> > > +#define SDHC_TX_CFG_REG 0x11C
> > > +#define TX_INT_CLK_SEL BIT(30)
> > > +#define TX_MUX_SEL BIT(31)
> > > +
> > > +#define SDHC_PHY_CTRL_REG 0x160
> > > +#define PHY_FUNC_EN BIT(0)
> > > +#define PHY_PLL_LOCK BIT(1)
> > > +#define HOST_LEGACY_MODE BIT(31)
> > > +
> > > +#define SDHC_PHY_FUNC_REG 0x164
> > > +#define PHY_TEST_EN BIT(7)
> > > +#define HS200_USE_RFIFO BIT(15)
> > > +
> > > +#define SDHC_PHY_DLLCFG 0x168
> > > +#define DLL_PREDLY_NUM GENMASK(3, 2)
> > > +#define DLL_FULLDLY_RANGE GENMASK(5, 4)
> > > +#define DLL_VREG_CTRL GENMASK(7, 6)
> > > +#define DLL_ENABLE BIT(31)
> > > +
> > > +#define SDHC_PHY_DLLCFG1 0x16C
> > > +#define DLL_REG1_CTRL GENMASK(7, 0)
> > > +#define DLL_REG2_CTRL GENMASK(15, 8)
> > > +#define DLL_REG3_CTRL GENMASK(23, 16)
> > > +#define DLL_REG4_CTRL GENMASK(31, 24)
> > > +
> > > +#define SDHC_PHY_DLLSTS 0x170
> > > +#define DLL_LOCK_STATE BIT(0)
> > > +
> > > +#define SDHC_PHY_PADCFG_REG 0x178
> > > +#define PHY_DRIVE_SEL GENMASK(2, 0)
> > > +#define RX_BIAS_CTRL BIT(5)
> > > +
> > > +struct spacemit_sdhci_host {
> > > + struct clk *clk_core;
> > > + struct clk *clk_io;
> > > +};
> > > +
> > > +static inline void spacemit_sdhci_setbits(struct sdhci_host *host, u32 val, int reg)
> > > +{
> > > + sdhci_writel(host, sdhci_readl(host, reg) | val, reg);
> > > +}
> > > +
> > > +static inline void spacemit_sdhci_clrbits(struct sdhci_host *host, u32 val, int reg)
> > > +{
> > > + sdhci_writel(host, sdhci_readl(host, reg) & ~val, reg);
> > > +}
> > > +
> >
> > > +static inline void spacemit_sdhci_clrsetbits(struct sdhci_host *host, u32 clr, u32 set, int reg)
> >
> > updatebits?
> >
> IMO, it's more ambiguous to use updatebits(), so I will just keep it
>
> besides, these above helper functions should really be carefully used,
> setbits() only set bits of 'val' while preserve other bits,
> clrsetbits() will first clear bits, then set, while still preserve others
>
> I will try to put a comment for them while updating next version
This is like something "regmap_update_bits". I think "update_bits" may
be better? I agree that it is good to put a comment here, whatever the
function name is. So this is fine for me, do it in the way you prefer.
Regards,
Inochi
Powered by blists - more mailing lists