Re: [RFC PATCH 4/4] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 1425 bytes --]

Hi Prabhakar,

thanks for this series!

On Wed, Jun 05, 2024 at 08:49:36AM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.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.
> 
> sd_iovs and sd_pwen quirks are introduced for SoCs supporting this bit
> to handle voltage level control and power enable via SD_STATUS register.

Two high-level questions:

- can't we use .enable/.disable in regulator_ops for handling pwen?
  Then we could simply use regulator_en/disable in the code and be future
  proof when other SDHI instances have other kinds of regulators (unless
  I am mising something)

- what about not using regmap and use set/get_voltage and friends? My
  concern is that other "new" registers might appear in the future and
  it will be cumbersome to handle the scattered IO regions.

That said, having a regulator is not a quirk in my book. I'd think
'struct renesas_sdhi' is the proper place. Or?

Looking forward to your comments,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]