Re: [PATCH 1/2] mmc: rockchip_dw_mmc: Enable by default for all supported SoCs

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Chen-Yu,

On 4/29/25 5:44 PM, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The rockchip_dw_mmc driver supports the MMC controller found in Rockchip
> SoCs. This controller is used for the SD card on all SoCs and eMMC on
> older SoCs. Almost all defconfigs for Rockchip platforms have this
> enabled.
> 
> Enable it by default for all supported Rockchip SoCs. Disable it
> explicitly in defconfigs that previously didn't have it enabled.
> 

I'll let Tom confirm (or not), but I think it'd make sense to make this 
patch lighter by not including the defconfig changes that would be 
simply done automatically when doing a defconfig sync, e.g. like 
bdf41fb7b386bdf60303b7a92431467c12779c86 did? This would be nice because 
it would make your patch much easier to apply if they take long to be 
applied (e.g. someone else changes the defconfig and now we have 
conflicts all over the place). This also would remove unrelated changes 
from the diff, specifically the ones for the PX30 boards which are just 
noise at this point.

[...]

> diff --git a/configs/coolpi-cm5-genbook-rk3588_defconfig b/configs/coolpi-cm5-genbook-rk3588_defconfig
> index 3eb5dc968af6..92676ebb984a 100644
> --- a/configs/coolpi-cm5-genbook-rk3588_defconfig
> +++ b/configs/coolpi-cm5-genbook-rk3588_defconfig
> @@ -65,6 +65,7 @@ CONFIG_MMC_HS400_ES_SUPPORT=y
>   CONFIG_SPL_MMC_HS400_ES_SUPPORT=y
>   CONFIG_MMC_HS400_SUPPORT=y
>   CONFIG_SPL_MMC_HS400_SUPPORT=y
> +# CONFIG_MMC_DW is not set
>   CONFIG_MMC_SDHCI=y
>   CONFIG_MMC_SDHCI_SDMA=y
>   CONFIG_MMC_SDHCI_ROCKCHIP=y

Checked that and sdmmc and sio both seems to be disabled in the DTS, so 
makes sense to have the driver disabled too.

[...]

> diff --git a/configs/evb-rv1108_defconfig b/configs/evb-rv1108_defconfig
> index 46b949531017..d23f90a6a606 100644
> --- a/configs/evb-rv1108_defconfig
> +++ b/configs/evb-rv1108_defconfig
> @@ -31,6 +31,7 @@ CONFIG_FASTBOOT_BUF_SIZE=0x08000000
>   CONFIG_FASTBOOT_FLASH_MMC_DEV=1
>   CONFIG_ROCKCHIP_GPIO=y
>   CONFIG_SYS_I2C_ROCKCHIP=y
> +# CONFIG_MMC_DW is not set
>   CONFIG_SPI_FLASH_GIGADEVICE=y
>   CONFIG_SPI_FLASH_WINBOND=y
>   CONFIG_SPI_FLASH_MTD=y

This one seems to be a mistake, sdmmc is enabled and is supported by 
this driver as far as I know. A separate patch to enable it 
(before/after this one, would be nice).

[...]

> diff --git a/configs/geekbox_defconfig b/configs/geekbox_defconfig
> index 80f91de7a11d..9dc642dc4ad9 100644
> --- a/configs/geekbox_defconfig
> +++ b/configs/geekbox_defconfig
> @@ -22,6 +22,7 @@ CONFIG_REGMAP=y
>   CONFIG_SYSCON=y
>   CONFIG_BOUNCE_BUFFER=y
>   CONFIG_CLK=y
> +# CONFIG_MMC_DW is not set
>   CONFIG_PINCTRL=y
>   CONFIG_RAM=y
>   CONFIG_DEBUG_UART_SHIFT=2

This one seems to be a mistake, emmc is enabled and is supported by this 
driver as far as I know. A separate patch to enable it (before/after 
this one, would be nice).

[...]

> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 6740591a6533..f2f0e7dbe601 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -251,6 +251,7 @@ config MMC_DAVINCI
>   config MMC_DW
>   	bool "Synopsys DesignWare Memory Card Interface"
>   	select BOUNCE_BUFFER
> +	default y if ARCH_ROCKCHIP
>   	help
>   	  This selects support for the Synopsys DesignWare Mobile Storage IP
>   	  block, this provides host support for SD and MMC interfaces, in both
> @@ -286,6 +287,7 @@ config MMC_DW_ROCKCHIP
>   	bool "Rockchip SD/MMC controller support"
>   	depends on OF_CONTROL
>   	depends on MMC_DW
> +	default y

default y if ARCH_ROCKCHIP

maybe? To avoid boards from other vendors to have to disable it?

Cheers,
Quentin