lkml.org 
[lkml]   [2020]   [Nov]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v1 17/30] mmc: sdhci-tegra: Support OPP and core voltage scaling
    From
    Date
    05.11.2020 12:58, Viresh Kumar пишет:
    >> +static void sdhci_tegra_deinit_opp_table(void *data)
    >> +{
    >> + struct device *dev = data;
    >> + struct opp_table *opp_table;
    >> +
    >> + opp_table = dev_pm_opp_get_opp_table(dev);
    > So you need to get an OPP table to put one :)
    > You need to save the pointer returned by dev_pm_opp_set_regulators() instead.

    This is intentional because why do we need to save the pointer if we're
    not using it and we know that we could get this pointer using OPP API?
    This is exactly the same what I did for the CPUFreq driver [1] :)

    [1]
    https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/cpufreq/tegra20-cpufreq.c#L97

    >> + dev_pm_opp_of_remove_table(dev);
    >> + dev_pm_opp_put_regulators(opp_table);
    >> + dev_pm_opp_put_opp_table(opp_table);
    >> +}
    >> +
    >> +static int devm_sdhci_tegra_init_opp_table(struct device *dev)
    >> +{
    >> + struct opp_table *opp_table;
    >> + const char *rname = "core";
    >> + int err;
    >> +
    >> + /* voltage scaling is optional */
    >> + if (device_property_present(dev, "core-supply"))
    >> + opp_table = dev_pm_opp_set_regulators(dev, &rname, 1);
    >> + else
    >
    >> + opp_table = dev_pm_opp_get_opp_table(dev);
    > Nice. I didn't think that someone will end up abusing this API and so made it
    > available for all, but someone just did that. I will fix that in the OPP core.

    The dev_pm_opp_put_regulators() handles the case where regulator is
    missing by acting as dev_pm_opp_get_opp_table(), but the
    dev_pm_opp_set_regulators() doesn't do it. Hence I don't think this is
    an abuse, but the OPP API drawback.

    > Any idea why you are doing what you are doing here ?

    Two reasons:

    1. Voltage regulator is optional, but dev_pm_opp_set_regulators()
    doesn't support optional regulators.

    2. We need to balance the opp_table refcount in order to use OPP API
    without polluting code with if(have_regulator), hence the
    dev_pm_opp_get_opp_table() is needed for taking the opp_table reference
    to have the same refcount as in the case of the dev_pm_opp_set_regulators().

    I guess we could make dev_pm_opp_set_regulators(dev, count) to accept
    regulators count=0 and then act as dev_pm_opp_get_opp_table(dev), will
    it be acceptable?

    \
     
     \ /
      Last update: 2020-11-05 15:18    [W:4.357 / U:0.212 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site