Hi Prabhakar, On Wed, Jun 26, 2024 at 02:23:41PM +0100, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxx> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> # 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@xxxxxxxxxxxxxxxxxxxx/ > > 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); I am open for discussing this but maybe here only + struct renesas_sdhi_regulator *internal_regulator or something and create the new struct with the additions above? > + int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable); > + int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage); Do we need these functions because the regulator framework cannot force these actions because it caches the old state? I wonder if we can avoid these functions... And the questions from the other threads need further discussions as well. But in general, I still like this approach. Thank you, Wolfram
Attachment:
signature.asc
Description: PGP signature
