linux-kernel - RE: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

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] [day] [month] [year] [list]
Date: Thu, 27 Jun 2024 09:56:24 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Prabhakar <prabhakar.csengg@...il.com>, Ulf Hansson
	<ulf.hansson@...aro.org>, Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
	Magnus Damm <magnus.damm@...il.com>, "linux-mmc@...r.kernel.org"
	<linux-mmc@...r.kernel.org>
CC: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, Prabhakar Mahadev Lad
	<prabhakar.mahadev-lad.rj@...renesas.com>, Claudiu Beznea
	<claudiu.beznea.uj@...renesas.com>
Subject: RE: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

Hi Wolfram and Prabhakar,

> -----Original Message-----
> From: Prabhakar <prabhakar.csengg@...il.com>
> Sent: Wednesday, June 26, 2024 2:24 PM
> Subject: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC
> 
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> 
> The SDHI/eMMC IPs found in the RZ/V2H(P) (a.k.a. r9a09g057) are very similar to those found in R-
> Car Gen3. However, they are not identical, necessitating an SoC-specific compatible string for
> fine-tuning driver support.
> 
> Key features of the RZ/V2H(P) SDHI/eMMC IPs include:
> - Voltage level control via the IOVS bit.
> - PWEN pin support via SD_STATUS register.
> - Lack of HS400 support.
> - Fixed address mode operation.
> 
> internal regulator support is added to control the voltage levels of SD pins via sd_iovs/sd_pwen
> bits in SD_STATUS register.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> Tested-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com> # on RZ/G3S
> ---
> v3->v4
> - Dropped using 'renesas,sdhi-use-internal-regulator' property
> - Now using of_device_is_available() to check if regulator is available and enabled
> - Dropped extra spaces during operations
> - Included tested by tag from Claudiu
> - Rebased patch on top of https://patchwork.kernel.org/project/linux-renesas-
> soc/patch/20240626085015.32171-2-wsa+renesas@...g-engineering.com/
> 
> v2->v3
> - Moved regulator info to renesas_sdhi_of_data instead of quirks
> - Added support to configure the init state of regulator
> - Added function pointers to configure regulator
> - Added REGULATOR_CHANGE_VOLTAGE mask
> 
> v1->v2
> - Now controlling PWEN bit get/set_voltage
> ---
>  drivers/mmc/host/renesas_sdhi.h               |  13 ++
>  drivers/mmc/host/renesas_sdhi_core.c          |  98 ++++++++++++
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 147 ++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h                   |   5 +
>  4 files changed, 263 insertions(+)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h index
> f12a87442338..cd509e7142ba 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -11,6 +11,8 @@
> 
>  #include <linux/dmaengine.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
>  #include <linux/workqueue.h>
>  #include "tmio_mmc.h"
> 
> @@ -36,6 +38,12 @@ struct renesas_sdhi_of_data {
>  	unsigned int max_blk_count;
>  	unsigned short max_segs;
>  	unsigned long sdhi_flags;
> +	struct regulator_desc *rdesc;
> +	struct regulator_init_data *reg_init_data;
> +	bool regulator_init_state;
> +	unsigned int regulator_init_voltage;
> +	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> +	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned
> +int voltage);
>  };
> 
>  #define SDHI_CALIB_TABLE_MAX 32
> @@ -95,6 +103,11 @@ struct renesas_sdhi {
> 
>  	struct reset_control *rstc;
>  	struct tmio_mmc_host *host;
> +
> +	bool use_internal_regulator;
> +	struct regulator_dev *internal_regulator;
> +	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> +	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned
> +int voltage);
>  };
> 
>  #define host_to_priv(host) \
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 0fc159b52fa9..6f8e745477b5 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -581,12 +581,50 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
> 
>  	if (!preserve) {
>  		if (priv->rstc) {
> +			bool regulator_enabled;
> +
> +			/* to restore back the internal regulator after reset make a copy of it */
> +			if (priv->use_internal_regulator)
> +				regulator_enabled = regulator_is_enabled(host->mmc->supply.vqmmc);

On all other SoCs, SD power enable is GPIO and we define this as GPIO hog, so that
It always supply Vmmc(3.3V) to the card for Card detection.

In this case, the SoC dedicated pin(PWEN) provides VMMC(3.3V). By reset this
value is always 1 (3.3V).

So should we implement handling of PWEN bit as part of vqmmc regulator
as it serves different functionality?

If I understand correctly,
VMMC(Always on 3.3V) for card detection -->PWEN pin
Vqmmc(1.8v-3.3V) for UHS operation for voltage switch-->IOVS pin

So according to me to implementing both the stuff in single regulator
is not correct? Like GPIO hog, PWEN bit should always on,
so that it always supply VMMC. What is your thoughts on this?

Please correct me if I am wrong.

Cheers,
Biju