On Wed, Jan 22, 2025 at 02:57:59PM +0530, Sachin Gupta wrote:
>
>
> On 1/7/2025 8:38 PM, Dmitry Baryshkov wrote:
> > On Tue, Jan 07, 2025 at 11:13:32AM +0530, Sachin Gupta wrote:
> > >
> > >
> > > On 12/27/2024 12:23 AM, Dmitry Baryshkov wrote:
> > > > On Thu, Dec 26, 2024 at 11:22:40AM +0530, Sachin Gupta wrote:
> > > > >
> > > > >
> > > > > On 12/19/2024 11:24 AM, Dmitry Baryshkov wrote:
> > > > > > On Wed, Dec 18, 2024 at 02:40:57PM +0530, Sachin Gupta wrote:
> > > > > > > +
> > > > > > > +static unsigned int sdhci_msm_get_clk_rate(struct sdhci_host *host, u32 req_clk)
> > > > > > > +{
> > > > > > > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > > > > > > + struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
> > > > > > > + struct clk *core_clk = msm_host->bulk_clks[0].clk;
> > > > > > > + unsigned int sup_clk;
> > > > > > > +
> > > > > > > + if (req_clk < sdhci_msm_get_min_clock(host))
> > > > > > > + return sdhci_msm_get_min_clock(host);
> > > > > > > +
> > > > > > > + sup_clk = clk_round_rate(core_clk, clk_get_rate(core_clk));
> > > > > > > +
> > > > > > > + if (host->clock != msm_host->clk_rate)
> > > > > > > + sup_clk = sup_clk / 2;
> > > > > > > +
> > > > > > > + return sup_clk;
> > > > > >
> > > > > > Why?
> > > > >
> > > > > Sorry, I did not understand your question. Can you please explain in detail.
> > > >
> > > > Please explain the maths. You get the rate from the clock, then you
> > > > round it, but it is the rate that has just been returned, so there
> > > > should be no need to round it. And after that there a division by two
> > > > for some reason. So I've asked for an explanation for that code.
> > > >
> > >
> > > clk_round_rate is used in case of over clocking issue we can round it to the
> > > usable frequency.
> >
> > If it is a frequency _returned_ by the clock driver, why do you need to
> > round it? It sounds like that freq should be usable anyway.
> >
>
> I agree, rounding will be taken care by clock driver. Will remove in my next
> patch.
>
> > > Divide by 2 is used as for HS400 the tuning happens in
> > > HS200 mode only so to update the frequency to 192 Mhz.
> >
> > Again, is it really 192 MHz? Or 19.2 MHz?
> > Also if it is for HS400, then shouldn't /2 be limited to that mode?
> >
>
> Yes, It is 192 MHz.
Good, thanks for the confirmation.
> As part of eMMC Init, driver will try to init with the best mode supported
> by controller and device. In this case it is HS400 mode, But as part of
> HS400 mode, we perform Tuning in HS200 mode only where we need to configure
> half of the clock.
This isn't an answer to the question. Let me rephrase it for you: if the
/2 is only used for HS400, why should it be attempted in all other
modes? Please limit the /2 just to HS400.
--
With best wishes
Dmitry
