RE: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 28 January 2025 13:32
> Subject: Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> 
> Hi Biju,
> 
> On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
> > However, the RZ/G3E SD0 channel has Voltage level control and PWEN pin
> > support via SD_STATUS register.
> >
> > internal regulator support is added to control the voltage levels of
> > the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by
> > populating vqmmc-regulator child node.
> >
> > SD1 and SD2 channels have gpio regulator support and internal
> > regulator support. Selection of the regulator is based on the regulator phandle.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> 
> > @@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> >         if (ret)
> >                 goto efree;
> >
> > +       rcfg.of_node = of_get_child_by_name(dev->of_node,
> > + "vqmmc-regulator");
> 
> If this node becomes required on RZ/V2H and RZ/G3E, and controlled through status, you also need:
> 
>     if (!of_device_is_available(rcfg.of_node)) {
>             of_node_put(rcfg.of_node);
>             rcfg.of_node = NULL;
>     }

OK.

> 
> Or introduce of_get_available_child_by_name()...

OK, will send a separate patch for optimizing the above 2 calls.

> 
> > +       if (rcfg.of_node) {
> > +               rcfg.driver_data = priv->host;
> > +
> > +               renesas_sdhi_vqmmc_regulator.name =
> > + "sdhi-vqmmc-regulator";
> 
> Name conflict in case of multiple instances?

This regulator name is Overriden by of_regulator() and it will pick the name from there.
See below. Am I missing anything?

root@smarc-rzg3e:/cip-test-scripts# cat /sys/class/regulator/regulator.*/name
regulator-dummy
fixed-3.3V
fixed-3.3V
SDHI1 VccQ
SDHI0-VQMMC
SDHI2-VQMMC
SDHI1-VQMMC

Cheers,
Biju




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux