I think I seem to agree now that overloading the meaning of
max-frequency for the sdio_freq clock might not be a good idea. If
Uffe is Ok I think I will revert back to using the 'clock-frequency'
field in the brcmstb,sdhci-brcmstb dt node and implement other
suggestions as well
Kamal
On Thu, May 19, 2022 at 10:16 PM Alan Cooper <alcooperx@xxxxxxxxx> wrote:
>
> This seems confusing and seems to overload the meaning of
> "max-frequency" which is typically used to limit the clock rate to
> something slower than what's in the CAPs register as the base clock.
> Instead we're trying to overclock the controller because the hardware
> team has verified that it can be run faster than 100MHz which is how
> the system is configured and is in the CAPs register as the base
> clock.
>
> Al
>
> On Tue, May 17, 2022 at 8:50 AM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> >
> > On Fri, 13 May 2022 at 22:19, Kamal Dasu <kdasu.kdev@xxxxxxxxx> wrote:
> > >
> > > From: Al Cooper <alcooperx@xxxxxxxxx>
> > >
> > > The 72116B0 has improved SDIO controllers that allow the max clock
> > > rate to be increased from a max of 100MHz to a max of 150MHz. The
> > > driver will need to get the clock and increase it's default rate
> > > and override the caps register, that still indicates a max of 100MHz.
> > > The new clock will be named "sdio_freq" in the DT node's "clock-names"
> > > list. The driver will use a DT property, "clock-frequency", to
> >
> > /s/clock-frequency/max-frequency
> >
> >
> > > enable this functionality and will get the actual rate in MHz
> > > from the property to allow various speeds to be requested.
> > >
> > > Signed-off-by: Al Cooper <alcooperx@xxxxxxxxx>
> > > Signed-off-by: Kamal Dasu <kdasu.kdev@xxxxxxxxx>
> > > ---
> > > drivers/mmc/host/sdhci-brcmstb.c | 30 ++++++++++++++++++++++++++++++
> > > 1 file changed, 30 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
> > > index 8eb57de48e0c..a1ffdd3f1640 100644
> > > --- a/drivers/mmc/host/sdhci-brcmstb.c
> > > +++ b/drivers/mmc/host/sdhci-brcmstb.c
> > > @@ -250,6 +250,9 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> > > struct sdhci_pltfm_host *pltfm_host;
> > > const struct of_device_id *match;
> > > struct sdhci_brcmstb_priv *priv;
> > > + struct clk *master_clk;
> > > + u32 base_clock_hz = 0;
> > > + u32 actual_clock_mhz;
> > > struct sdhci_host *host;
> > > struct resource *iomem;
> > > struct clk *clk;
> > > @@ -330,6 +333,33 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
> > > if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
> > > host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
> > >
> > > + /* Change the base clock frequency if the DT property exists */
> > > + if (device_property_read_u32(&pdev->dev, "max-frequency",
> > > + &base_clock_hz) != 0)
> > > + goto add_host;
> >
> > The max-frequency DT property is already being parsed by
> > mmc_of_parse() and the value is put in host->mmc->f_max. You could
> > probably use that instead, right?
> >
> > > +
> > > + master_clk = devm_clk_get(&pdev->dev, "sdio_freq");
> > > + if (IS_ERR(master_clk)) {
> > > + dev_warn(&pdev->dev, "Clock for \"sdio_freq\" not found\n");
> > > + goto add_host;
> > > + } else {
> > > + res = clk_prepare_enable(master_clk);
> > > + if (res)
> > > + goto err;
> > > + }
> > > +
> > > + /* set improved clock rate */
> > > + clk_set_rate(master_clk, base_clock_hz);
> > > + actual_clock_mhz = clk_get_rate(master_clk) / 1000000;
> > > +
> > > + host->caps &= ~SDHCI_CLOCK_V3_BASE_MASK;
> > > + host->caps |= (actual_clock_mhz << SDHCI_CLOCK_BASE_SHIFT);
> > > + /* Disable presets because they are now incorrect */
> > > + host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> > > + dev_dbg(&pdev->dev, "Base Clock Frequency changed to %dMHz\n",
> > > + actual_clock_mhz);
> > > +
> > > +add_host:
> > > res = sdhci_brcmstb_add_host(host, priv);
> > > if (res)
> > > goto err;
> > > --
> > > 2.17.1
> > >
> >
> > Kind regards
> > Uffe
