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