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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPDyKFoDWS6DWdKOaxTDEYeKv3hjVDoR7XGi19nESVssc-RG8g@mail.gmail.com>
Date: Mon, 19 May 2025 13:34:14 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Yixun Lan <dlan@...too.org>
Cc: 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 Thu, 1 May 2025 at 10:51, Yixun Lan <dlan@...too.org> 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)

These define-names look a bit too generic to me. Please add some
additional prefixes so it becomes more clear what they are.

This applies to all the others below too.

> +#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)

[...]

> +
> +static int spacemit_sdhci_pre_select_hs400(struct mmc_host *mmc)
> +{
> +       struct sdhci_host *host = mmc_priv(mmc);
> +
> +       spacemit_sdhci_setbits(host, MMC_HS400, SDHC_MMC_CTRL_REG);
> +       host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;

At least this deserves a comment. Isn't MMC_CAP_WAIT_WHILE_BUSY
working for all cases?

> +
> +       return 0;
> +}
> +
> +static void spacemit_sdhci_post_select_hs400(struct mmc_host *mmc)
> +{
> +       struct sdhci_host *host = mmc_priv(mmc);
> +
> +       spacemit_sdhci_phy_dll_init(host);
> +       host->mmc->caps &= ~MMC_CAP_WAIT_WHILE_BUSY;

Dito.

[...]

Kind regards
Uffe

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ