Received-SPF: pass (google.com: domain of linux-kernel+bounces-204053-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240605074936.578687-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <20240605074936.578687-5-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <TY3PR01MB113464449FACE8364BF667CBB86FA2@TY3PR01MB11346.jpnprd01.prod.outlook.com>
 <CA+V-a8tXLsfTvuCWDqiFkWbe=C2Coo8KF4GchbHPzOG+RTiChw@mail.gmail.com> <TY3PR01MB113468DD4C215E58B4499BAAA86FA2@TY3PR01MB11346.jpnprd01.prod.outlook.com>
In-Reply-To: <TY3PR01MB113468DD4C215E58B4499BAAA86FA2@TY3PR01MB11346.jpnprd01.prod.outlook.com>
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Date: Thu, 6 Jun 2024 10:49:36 +0100
Message-ID: <CA+V-a8tXp0sa_FiVzAkz2Uax1iv3XK90ug2KuKxuHRSP5Vfx1Q@mail.gmail.com>
Subject: Re: [RFC PATCH 4/4] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC
To: Biju Das <biju.das.jz@bp.renesas.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>, Ulf Hansson <ulf.hansson@linaro.org>, 
	Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org>, 
	Wolfram Sang <wsa+renesas@sang-engineering.com>, Liam Girdwood <lgirdwood@gmail.com>, 
	Mark Brown <broonie@kernel.org>, Magnus Damm <magnus.damm@gmail.com>, 
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>, 
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	"linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>, 
	Fabrizio Castro <fabrizio.castro.jz@renesas.com>, 
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Biju,

On Thu, Jun 6, 2024 at 10:43=E2=80=AFAM Biju Das <biju.das.jz@bp.renesas.co=
m> wrote:
>
> Hi Prabhakar,
>
>
> > -----Original Message-----
> > From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> > Sent: Thursday, June 6, 2024 10:38 AM
> > Subject: Re: [RFC PATCH 4/4] mmc: renesas_sdhi: Add support for RZ/V2H(=
P) SoC
> >
> > Hi Biju,
> >
> > Thank you for the review.
> >
> > On Thu, Jun 6, 2024 at 10:32=E2=80=AFAM Biju Das <biju.das.jz@bp.renesa=
s.com> wrote:
> > >
> > > Hi Prabhakar,
> > >
<snip>
> > > >
> > > > +static void renesas_sdhi_sd_status_pwen(struct tmio_mmc_host *host=
,
> > > > +bool on) {
> > > > +     u32 sd_status;
> > > > +
> > > > +     sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
> > > > +     if (on)
> > > > +             sd_status |=3D  SD_STATUS_PWEN;
> > > > +     else
> > > > +             sd_status &=3D  ~SD_STATUS_PWEN;
> > > > +
> > > > +     sd_ctrl_write32(host, CTL_SD_STATUS, sd_status); }
> > > > +
> > >
> > > May be use regulator_set_voltage() to set this??
> > >
> > This is the PWEN bit which is not modelled as a regulator, we cannot us=
e regulator_set_voltage() to
> > set this bit.
>
> So, there may be a race between regulator driver and this bit??
>
No, there won't be any race between the regulator driver and this bit
as the regulator driver only controls the IOVS bit and not the PWEN
bit.

> >
> > > >  static int renesas_sdhi_start_signal_voltage_switch(struct mmc_hos=
t *mmc,
> > > >                                                   struct mmc_ios
> > > > *ios)  { @@ -587,6 +600,9 @@ static void renesas_sdhi_reset(struct
> > > > tmio_mmc_host *host, bool preserve)
> > > >                                         false, priv->rstc);
> > > >                       /* At least SDHI_VER_GEN2_SDR50 needs manual =
release of reset */
> > > >                       sd_ctrl_write16(host, CTL_RESET_SD, 0x0001);
> > > > +                     if (sdhi_has_quirk(priv, sd_pwen))
> > > > +                             renesas_sdhi_sd_status_pwen(host,
> > > > + true);
> > > > +
> > > >                       priv->needs_adjust_hs400 =3D false;
> > > >                       renesas_sdhi_set_clock(host, host->clk_cache)=
;
> > > >
> > > > @@ -904,6 +920,34 @@ static void renesas_sdhi_enable_dma(struct tmi=
o_mmc_host *host, bool
> > enable)
> > > >       renesas_sdhi_sdbuf_width(host, enable ? width : 16);  }
> > > >
> > > > +static int renesas_sdhi_internal_dmac_register_regulator(struct pl=
atform_device *pdev,
> > > > +                                                      const struct=
 renesas_sdhi_quirks
> > *quirks) {
> > > > +     struct tmio_mmc_host *host =3D platform_get_drvdata(pdev);
> > > > +     struct renesas_sdhi *priv =3D host_to_priv(host);
> > > > +     struct regulator_config rcfg =3D {
> > > > +             .dev =3D &pdev->dev,
> > > > +             .driver_data =3D priv,
> > > > +     };
> > > > +     struct regulator_dev *rdev;
> > > > +     const char *devname;
> > > > +
> > > > +     devname =3D devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-vqmmc-=
regulator",
> > > > +                              dev_name(&pdev->dev));
> > > > +     if (!devname)
> > > > +             return -ENOMEM;
> > > > +
> > > > +     quirks->rdesc->name =3D devname;
> > > > +     rcfg.regmap =3D devm_regmap_init_mmio(&pdev->dev, host->ctl +
> > > > + quirks->rdesc_offset,
> > >
> > > This is (CTL_SD_STATUS << 2) , so the variable can be dropped from qu=
irks.
> > >
> > rdesc_offset is added to make code generic, that is in future if there =
is a new chip with a
> > different offset which supports IOVS we can just pass the offset for it=
.
>
> Currently there is no consumer for it, so it can save memory. When a futu=
re chip comes
> we can bring back this variable??
>
OK.

Cheers,
Prabhakar