* Re: [v5 1/2] dt-bindings: gpio: aspeed: Add SGPIO support
From: Andrew Jeffery @ 2019-07-22 1:42 UTC (permalink / raw)
To: Linus Walleij, Hongwei Zhang
Cc: open list:GPIO SUBSYSTEM, Joel Stanley,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-aspeed, Bartosz Golaszewski, Rob Herring, Mark Rutland,
linux-kernel@vger.kernel.org, Linux ARM
In-Reply-To: <CACRpkdYhVoP75ZDfASW+DH5yf-a5diitiXsh7eLsJx5hsTC9sQ@mail.gmail.com>
On Sat, 20 Jul 2019, at 17:43, Linus Walleij wrote:
> Hi Hongwei,
>
> after looking close at the driver and bindings I have this feeback:
>
> On Fri, Jul 19, 2019 at 9:25 PM Hongwei Zhang <hongweiz@ami.com> wrote:
>
> +- reg : Address and length of the register set for the device
>
> This 0x100 range may look simple but in the driver it looks like
> this:
>
> +static const struct aspeed_sgpio_bank aspeed_sgpio_banks[] = {
> + {
> + .val_regs = 0x0000,
> + .rdata_reg = 0x0070,
> + .irq_regs = 0x0004,
> + .names = { "A", "B", "C", "D" },
> + },
> + {
> + .val_regs = 0x001C,
> + .rdata_reg = 0x0074,
> + .irq_regs = 0x0020,
> + .names = { "E", "F", "G", "H" },
> + },
> + {
> + .val_regs = 0x0038,
> + .rdata_reg = 0x0078,
> + .irq_regs = 0x003C,
> + .names = { "I", "J" },
> + },
> +};
>
> So first break this into up to 10 different instances with one device
> per bank instead.
>
> For each device:
>
> reg = <0x1e780200 4>, <x1e780204 ..>, <0x1e780270 ..>;
> reg-names = "val", "irq", "rdata";
>
> This way you use the device tree features for locating the
> register offsets instead of hard-coding it into the driver,
> which will make the driver simpler.
>
> > +- ngpios : number of GPIO pins to serialise.
> > + (should be multiple of 8, up to 80 pins)
>
> This is wrong. This should be split into 10 different devices providing
> 8 GPIO lines each. The "ngpios" is not intended to subdivide
> banks, but for the case where you use say bits 0..11 of 32 bits in
> a register by setting ngpios to 12.
>
> I see that they use the same clock divider and the same interrupt,
> but this is better to partition into a separate clock divider driver
> and up to 10 instances of GPIO devices with 8 GPIOs each.
>
> I also see that they use the same interrupt. This is fine, in your
> driver just grab a shared IRQ and all the IRQ handlers will be
> traversed. This is a well known pattern for all operating systems.
>
> > +- clocks : A phandle to the APB clock for SGPM clock division
> > +
> > +- bus-frequency : SGPM CLK frequency
>
> I see that there is a common clock control register in offset 0x54.
>
> Split this out to a separate clock driver that provides a divided clock
> that all GPIO blocks can get, no matter if you use 1, 2 .. 10 of these
> blocks.
>
> The fact that these GPIO banks and the clock register is in the same
> memory range does not really matter. Split up the memory range in
> on reg = per GPIO chip and one reg for the clock.
The issue that I see with this is that the SGPIO bus clock configuration
is not the only field in the SGPIO control register - it also contains the
number of GPIOs the bus should output along with a bus enable bit.
The clock, and particularly the number of GPIOs (due to the nature
of the SGPIO bus) both need to be configured before we set the
enable bit (or obviously all can be done simultaneously).
I'll explore some of the options below (it's a bit train-of-thought,
hopefully it makes sense):
If the clock driver owns the control register, it also needs to know how
many GPIOs we want to emit on the bus. This seems like an awkward
configuration parameter for a clock driver.
To avoid the weird parameter we could protect the control register
with a lock shared between the clock driver and the SGPIO driver. This
way the SGPIO driver could have the ngpios parameter, and request
the clock after its written the ngpios value to the control register. A
regmap would be useful here to avoid the resource clash and it also
provides the required lock.
However, you're also going down the path of splitting the driver such
that there's one instance per bank. With this approach we need to
solve two problems: Accounting for the total number of GPIOs, and
only enabling the bus after the last bank has had its driver probed.
The accounting might be handled by accumulating the number of
GPIOs in each bank in the control register itself, e.g. the driver
implementation does something like:
spin_lock(...)
ctrl = ioread32(...)
ngpios = FIELD_GET(ASPEED_SGPIO_CTRL_NGPIOS, ctrl);
ngpios += 8;
ctrl &= ~ASPEED_SGPIO_CTRL_NGPIOS;
ctrl |= FIELD_PREP(ASPEED_SGPIO_CTRL_NGPIOS, ngpios);
iowrite32(ctrl, ...);
spin_unlock(...);
This works on cold boot of the device when the ngpios field is set to
zero due to reset, however will fail on subsequent warm reboots if
the GPIO IP block is protected from reset by the SoC's watchdog
configuration: the field will not be zeroed in this case, and the
value of the field is represented by `NR_BOOTS * NR_GPIOS`,
which is incorrect. To do this correctly I guess we would need some
other global state held in the driver implementation (zeroed when
the kernel is loaded), and write the incremented value to the control
register on each probe() invocation.
However, that aside, we can't simply enable the bus in the clock
enable callback if enable is called per-bank, as it is called once on
the first request with further requests simply refcounted as you
mentioned. This is exactly the behaviour we can't tolerate with the
bus: it must only be enabled after the last GPIO bank is registered,
when we know the total number of GPIOs to emit. So we can only
call clk_prepare_enable() after the last bank is probed, or in the
probe() of the last bank if the driver has some way to tell it is the
last instance. In my mind per-bank clock management makes the
implementation more involved than it should be, and isn't really
reflective of what the clk subsystem is trying to achieve (violates
clk_prepare()/clk_enable() post conditions, or boils down to just
calling devm_clk_get() in probe() with no further action, which
seems kinda pointless).
Having a separate clk implementation whose instance is requested
once by a monolithic SGPIO driver matches how the hardware is
designed. However, given the issue of sharing the register with the
GPIO count, in my opinion it seems to make more sense to leave the
driver design as is and minimise the accidental complexity
introduced by the behavioural mismatch of the clk abstraction
and the addition of locking where it wasn't previously necessary.
Interested in your thoughts.
Andrew
^ permalink raw reply
* Re: [PATCH V6 02/21] pinctrl: tegra: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-07-21 22:48 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <9c844d55-0bc0-471c-6c49-1febe799ebf2@gmail.com>
On 7/21/19 3:09 PM, Dmitry Osipenko wrote:
> 22.07.2019 1:03, Dmitry Osipenko пишет:
>> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>>> This patch adds support for Tegra pinctrl driver suspend and resume.
>>>
>>> During suspend, context of all pinctrl registers are stored and
>>> on resume they are all restored to have all the pinmux and pad
>>> configuration for normal operation.
>>>
>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>> ---
>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 59 +++++++++++++++++++++++++++++++++++
>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
>>> 2 files changed, 62 insertions(+)
>>>
>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> index 186ef98e7b2b..e3a237534281 100644
>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> @@ -631,6 +631,58 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>> }
>>> }
>>>
>>> +static size_t tegra_pinctrl_get_bank_size(struct device *dev,
>>> + unsigned int bank_id)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct resource *res;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, bank_id);
>>> +
>>> + return resource_size(res) / 4;
>>> +}
>>> +
>>> +static int tegra_pinctrl_suspend(struct device *dev)
>>> +{
>>> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
>>> + u32 *backup_regs = pmx->backup_regs;
>>> + u32 *regs;
>>> + size_t bank_size;
>>> + unsigned int i, k;
>>> +
>>> + for (i = 0; i < pmx->nbanks; i++) {
>>> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
>>> + regs = pmx->regs[i];
>>> + for (k = 0; k < bank_size; k++)
>>> + *backup_regs++ = readl_relaxed(regs++);
>>> + }
>>> +
>>> + return pinctrl_force_sleep(pmx->pctl);
>>> +}
>>> +
>>> +static int tegra_pinctrl_resume(struct device *dev)
>>> +{
>>> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
>>> + u32 *backup_regs = pmx->backup_regs;
>>> + u32 *regs;
>>> + size_t bank_size;
>>> + unsigned int i, k;
>>> +
>>> + for (i = 0; i < pmx->nbanks; i++) {
>>> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
>>> + regs = pmx->regs[i];
>>> + for (k = 0; k < bank_size; k++)
>>> + writel_relaxed(*backup_regs++, regs++);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +const struct dev_pm_ops tegra_pinctrl_pm = {
>>> + .suspend = &tegra_pinctrl_suspend,
>>> + .resume = &tegra_pinctrl_resume
>>> +};
>>> +
>>> static bool gpio_node_has_range(const char *compatible)
>>> {
>>> struct device_node *np;
>>> @@ -655,6 +707,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
>>> int i;
>>> const char **group_pins;
>>> int fn, gn, gfn;
>>> + unsigned long backup_regs_size = 0;
>>>
>>> pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
>>> if (!pmx)
>>> @@ -707,6 +760,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
>>> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
>>> if (!res)
>>> break;
>>> + backup_regs_size += resource_size(res);
>>> }
>>> pmx->nbanks = i;
>>>
>>> @@ -715,6 +769,11 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
>>> if (!pmx->regs)
>>> return -ENOMEM;
>>>
>>> + pmx->backup_regs = devm_kzalloc(&pdev->dev, backup_regs_size,
>>> + GFP_KERNEL);
>>> + if (!pmx->backup_regs)
>>> + return -ENOMEM;
>>> +
>>> for (i = 0; i < pmx->nbanks; i++) {
>>> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
>>> pmx->regs[i] = devm_ioremap_resource(&pdev->dev, res);
>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
>>> index 105309774079..0fc82eea9cf1 100644
>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.h
>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
>>> @@ -17,6 +17,7 @@ struct tegra_pmx {
>>>
>>> int nbanks;
>>> void __iomem **regs;
>>> + u32 *backup_regs;
>>> };
>>>
>>> enum tegra_pinconf_param {
>>> @@ -193,6 +194,8 @@ struct tegra_pinctrl_soc_data {
>>> bool drvtype_in_mux;
>>> };
>>>
>>> +extern const struct dev_pm_ops tegra_pinctrl_pm;
>>> +
>>> int tegra_pinctrl_probe(struct platform_device *pdev,
>>> const struct tegra_pinctrl_soc_data *soc_data);
>>> #endif
>>>
>> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
>>
> BTW, you should remove Thierry's ACK from all patches that were modified
> if he didn't re-ACK the new version.
>
> Also, it looks to me that you're manually adding versioning to the
> patches because git usually uses lowercase for 'v'. You could use "git
> format-patch -v6 ..".
ok, Reg. version I am using git format-patch --subject-prefix='PATCH V6'
will change to lower case v
^ permalink raw reply
* Re: [PATCH V6 06/21] clk: tegra: pll: Save and restore pll context
From: Sowjanya Komatineni @ 2019-07-21 22:47 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <ba402ed3-0e6f-26ce-ac0a-8048babb50ca@gmail.com>
On 7/21/19 2:44 PM, Dmitry Osipenko wrote:
> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>> This patch implements save and restore of PLL context.
>>
>> During system suspend, core power goes off and looses the settings
>> of the Tegra CAR controller registers.
>>
>> So during suspend entry pll rate is stored and on resume it is
>> restored back along with its state.
>>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/clk/tegra/clk-pll.c | 121 ++++++++++++++++++++++++++++-----------
>> drivers/clk/tegra/clk-tegra210.c | 2 +-
>> drivers/clk/tegra/clk.h | 10 +++-
>> 3 files changed, 99 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
>> index 1583f5fc992f..f136964e6c44 100644
>> --- a/drivers/clk/tegra/clk-pll.c
>> +++ b/drivers/clk/tegra/clk-pll.c
>> @@ -1008,6 +1008,59 @@ static unsigned long clk_plle_recalc_rate(struct clk_hw *hw,
>> return rate;
>> }
>>
>> +void tegra_clk_sync_state_pll(struct clk_hw *hw)
>> +{
>> + if (!__clk_get_enable_count(hw->clk))
>> + clk_pll_disable(hw);
>> + else
>> + clk_pll_enable(hw);
>> +}
>> +
>> +static int tegra_clk_pll_save_context(struct clk_hw *hw)
>> +{
>> + struct tegra_clk_pll *pll = to_clk_pll(hw);
>> + u32 val = 0;
>> +
>> + pll->rate = clk_hw_get_rate(hw);
>> +
>> + if (pll->params->flags & TEGRA_PLLMB)
>> + val = pll_readl_base(pll);
>> + else if (pll->params->flags & TEGRA_PLLRE)
>> + val = pll_readl_base(pll) & divp_mask_shifted(pll);
>> +
>> + pll->pllbase_ctx = val;
>> +
>> + return 0;
>> +}
>> +
>> +static void tegra_clk_pll_restore_context(struct clk_hw *hw)
>> +{
>> + struct tegra_clk_pll *pll = to_clk_pll(hw);
>> + struct clk_hw *parent = clk_hw_get_parent(hw);
>> + unsigned long parent_rate = clk_hw_get_rate(parent);
>> + u32 val;
>> +
>> + if (clk_pll_is_enabled(hw))
>> + return;
>> +
>> + if (pll->params->flags & TEGRA_PLLMB) {
>> + pll_writel_base(pll->pllbase_ctx, pll);
>> + } else if (pll->params->flags & TEGRA_PLLRE) {
>> + val = pll_readl_base(pll);
>> + val &= ~(divp_mask_shifted(pll));
>> + pll_writel_base(pll->pllbase_ctx | val, pll);
>> + }
>> +
>> + if (pll->params->set_defaults)
>> + pll->params->set_defaults(pll);
>> +
>> + clk_pll_set_rate(hw, pll->rate, parent_rate);
>> +
>> + /* do not sync pllx state here. pllx is sync'd after dfll resume */
>> + if (!(pll->params->flags & TEGRA_PLLX))
>> + tegra_clk_sync_state_pll(hw);
>> +}
>> +
>> const struct clk_ops tegra_clk_pll_ops = {
>> .is_enabled = clk_pll_is_enabled,
>> .enable = clk_pll_enable,
>> @@ -1015,6 +1068,8 @@ const struct clk_ops tegra_clk_pll_ops = {
>> .recalc_rate = clk_pll_recalc_rate,
>> .round_rate = clk_pll_round_rate,
>> .set_rate = clk_pll_set_rate,
>> + .save_context = tegra_clk_pll_save_context,
>> + .restore_context = tegra_clk_pll_restore_context,
>> };
>>
>> const struct clk_ops tegra_clk_plle_ops = {
>> @@ -1802,6 +1857,27 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw)
>>
>> return ret;
>> }
>> +
>> +static void _clk_plle_tegra_init_parent(struct tegra_clk_pll *pll)
>> +{
>> + u32 val, val_aux;
>> +
>> + /* ensure parent is set to pll_ref */
>> + val = pll_readl_base(pll);
>> + val_aux = pll_readl(pll->params->aux_reg, pll);
>> +
>> + if (val & PLL_BASE_ENABLE) {
>> + if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
>> + (val_aux & PLLE_AUX_PLLP_SEL))
>> + WARN(1, "pll_e enabled with unsupported parent %s\n",
>> + (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
>> + "pll_re_vco");
>> + } else {
>> + val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
>> + pll_writel(val_aux, pll->params->aux_reg, pll);
>> + fence_udelay(1, pll->clk_base);
>> + }
>> +}
>> #endif
>>
>> static struct tegra_clk_pll *_tegra_init_pll(void __iomem *clk_base,
>> @@ -2214,27 +2290,12 @@ struct clk *tegra_clk_register_plle_tegra114(const char *name,
>> {
>> struct tegra_clk_pll *pll;
>> struct clk *clk;
>> - u32 val, val_aux;
>>
>> pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
>> if (IS_ERR(pll))
>> return ERR_CAST(pll);
>>
>> - /* ensure parent is set to pll_re_vco */
>> -
>> - val = pll_readl_base(pll);
>> - val_aux = pll_readl(pll_params->aux_reg, pll);
>> -
>> - if (val & PLL_BASE_ENABLE) {
>> - if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
>> - (val_aux & PLLE_AUX_PLLP_SEL))
>> - WARN(1, "pll_e enabled with unsupported parent %s\n",
>> - (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
>> - "pll_re_vco");
>> - } else {
>> - val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
>> - pll_writel(val_aux, pll_params->aux_reg, pll);
>> - }
>> + _clk_plle_tegra_init_parent(pll);
>>
>> clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
>> &tegra_clk_plle_tegra114_ops);
>> @@ -2276,6 +2337,8 @@ static const struct clk_ops tegra_clk_pllss_ops = {
>> .recalc_rate = clk_pll_recalc_rate,
>> .round_rate = clk_pll_ramp_round_rate,
>> .set_rate = clk_pllxc_set_rate,
>> + .save_context = tegra_clk_pll_save_context,
>> + .restore_context = tegra_clk_pll_restore_context,
>> };
>>
>> struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,
>> @@ -2375,6 +2438,7 @@ struct clk *tegra_clk_register_pllre_tegra210(const char *name,
>> pll_params->vco_min = pll_params->adjust_vco(pll_params,
>> parent_rate);
>>
>> + pll_params->flags |= TEGRA_PLLRE;
>> pll = _tegra_init_pll(clk_base, pmc, pll_params, lock);
>> if (IS_ERR(pll))
>> return ERR_CAST(pll);
>> @@ -2520,11 +2584,19 @@ static void clk_plle_tegra210_disable(struct clk_hw *hw)
>> spin_unlock_irqrestore(pll->lock, flags);
>> }
>>
>> +static void tegra_clk_plle_t210_restore_context(struct clk_hw *hw)
>> +{
>> + struct tegra_clk_pll *pll = to_clk_pll(hw);
>> +
>> + _clk_plle_tegra_init_parent(pll);
>> +}
>> +
>> static const struct clk_ops tegra_clk_plle_tegra210_ops = {
>> .is_enabled = clk_plle_tegra210_is_enabled,
>> .enable = clk_plle_tegra210_enable,
>> .disable = clk_plle_tegra210_disable,
>> .recalc_rate = clk_pll_recalc_rate,
>> + .restore_context = tegra_clk_plle_t210_restore_context,
>> };
>>
>> struct clk *tegra_clk_register_plle_tegra210(const char *name,
>> @@ -2535,27 +2607,12 @@ struct clk *tegra_clk_register_plle_tegra210(const char *name,
>> {
>> struct tegra_clk_pll *pll;
>> struct clk *clk;
>> - u32 val, val_aux;
>>
>> pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
>> if (IS_ERR(pll))
>> return ERR_CAST(pll);
>>
>> - /* ensure parent is set to pll_re_vco */
>> -
>> - val = pll_readl_base(pll);
>> - val_aux = pll_readl(pll_params->aux_reg, pll);
>> -
>> - if (val & PLLE_BASE_ENABLE) {
>> - if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
>> - (val_aux & PLLE_AUX_PLLP_SEL))
>> - WARN(1, "pll_e enabled with unsupported parent %s\n",
>> - (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
>> - "pll_re_vco");
>> - } else {
>> - val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
>> - pll_writel(val_aux, pll_params->aux_reg, pll);
>> - }
>> + _clk_plle_tegra_init_parent(pll);
>>
>> clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
>> &tegra_clk_plle_tegra210_ops);
>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>> index 4721ee030d1c..58397f93166c 100644
>> --- a/drivers/clk/tegra/clk-tegra210.c
>> +++ b/drivers/clk/tegra/clk-tegra210.c
>> @@ -1602,7 +1602,7 @@ static struct tegra_clk_pll_params pll_x_params = {
>> .pdiv_tohw = pll_qlin_pdiv_to_hw,
>> .div_nmp = &pllx_nmp,
>> .freq_table = pll_x_freq_table,
>> - .flags = TEGRA_PLL_USE_LOCK | TEGRA_PLL_HAS_LOCK_ENABLE,
>> + .flags = TEGRA_PLL_USE_LOCK | TEGRA_PLL_HAS_LOCK_ENABLE | TEGRA_PLLX,
>> .dyn_ramp = tegra210_pllx_dyn_ramp,
>> .set_defaults = tegra210_pllx_set_defaults,
>> .calc_rate = tegra210_pll_fixed_mdiv_cfg,
>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>> index fb29a8c27873..8532f5150091 100644
>> --- a/drivers/clk/tegra/clk.h
>> +++ b/drivers/clk/tegra/clk.h
>> @@ -235,6 +235,8 @@ struct tegra_clk_pll;
>> * TEGRA_PLLMB - PLLMB has should be treated similar to PLLM. This
>> * flag indicated that it is PLLMB.
>> * TEGRA_PLL_VCO_OUT - Used to indicate that the PLL has a VCO output
>> + * TEGRA_PLLRE - Used to indicate that it is PLLRE.
>> + * TEGRA_PLLX - Used to indicate that it is PLLX.
>> */
>> struct tegra_clk_pll_params {
>> unsigned long input_min;
>> @@ -301,6 +303,8 @@ struct tegra_clk_pll_params {
>> #define TEGRA_MDIV_NEW BIT(11)
>> #define TEGRA_PLLMB BIT(12)
>> #define TEGRA_PLL_VCO_OUT BIT(13)
>> +#define TEGRA_PLLRE BIT(14)
>> +#define TEGRA_PLLX BIT(15)
>>
>> /**
>> * struct tegra_clk_pll - Tegra PLL clock
>> @@ -310,6 +314,8 @@ struct tegra_clk_pll_params {
>> * @pmc: address of PMC, required to read override bits
>> * @lock: register lock
>> * @params: PLL parameters
>> + * @rate: rate during system suspend and resume
>> + * @pllbase_ctx: pll base register value during suspend and resume
>> */
>> struct tegra_clk_pll {
>> struct clk_hw hw;
>> @@ -317,6 +323,8 @@ struct tegra_clk_pll {
>> void __iomem *pmc;
>> spinlock_t *lock;
>> struct tegra_clk_pll_params *params;
>> + unsigned long rate;
>> + u32 pllbase_ctx;
>> };
>>
>> #define to_clk_pll(_hw) container_of(_hw, struct tegra_clk_pll, hw)
>> @@ -840,7 +848,7 @@ u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
>> int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
>> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
>> u8 frac_width, u8 flags);
>> -
>> +void tegra_clk_sync_state_pll(struct clk_hw *hw);
> Looks like this function isn't used anywhere other than this patch. Bug?
With all dfll sequence moved to right places in this patch, yes this can
be collapsed within restore context itself.
Will change in next version
>> /* Combined read fence with delay */
>> #define fence_udelay(delay, reg) \
>>
^ permalink raw reply
* Re: [PATCH V6 14/21] clk: tegra210: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-07-21 22:45 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <e683b417-66fb-38dc-c16b-dab616583a88@gmail.com>
On 7/21/19 2:38 PM, Dmitry Osipenko wrote:
> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>> This patch adds support for clk: tegra210: suspend-resume.
>>
>> All the CAR controller settings are lost on suspend when core
>> power goes off.
>>
>> This patch has implementation for saving and restoring all PLLs
>> and clocks context during system suspend and resume to have the
>> clocks back to same state for normal operation.
>>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/clk/tegra/clk-tegra210.c | 68 ++++++++++++++++++++++++++++++++++++++--
>> drivers/clk/tegra/clk.c | 14 +++++++++
>> drivers/clk/tegra/clk.h | 1 +
>> 3 files changed, 80 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
>> index 55a88c0824a5..68271873acc1 100644
>> --- a/drivers/clk/tegra/clk-tegra210.c
>> +++ b/drivers/clk/tegra/clk-tegra210.c
>> @@ -9,6 +9,7 @@
>> #include <linux/clkdev.h>
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> +#include <linux/syscore_ops.h>
>> #include <linux/delay.h>
>> #include <linux/export.h>
>> #include <linux/mutex.h>
>> @@ -220,11 +221,15 @@
>> #define CLK_M_DIVISOR_SHIFT 2
>> #define CLK_M_DIVISOR_MASK 0x3
>>
>> +#define CLK_MASK_ARM 0x44
>> +#define MISC_CLK_ENB 0x48
>> +
>> #define RST_DFLL_DVCO 0x2f4
>> #define DVFS_DFLL_RESET_SHIFT 0
>>
>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>> +#define CPU_SOFTRST_CTRL 0x380
>>
>> #define LVL2_CLK_GATE_OVRA 0xf8
>> #define LVL2_CLK_GATE_OVRC 0x3a0
>> @@ -2825,6 +2830,7 @@ static int tegra210_enable_pllu(void)
>> struct tegra_clk_pll_freq_table *fentry;
>> struct tegra_clk_pll pllu;
>> u32 reg;
>> + int ret;
>>
>> for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
>> if (fentry->input_rate == pll_ref_freq)
>> @@ -2853,9 +2859,8 @@ static int tegra210_enable_pllu(void)
>> reg |= PLL_ENABLE;
>> writel(reg, clk_base + PLLU_BASE);
>>
>> - readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>> - reg & PLL_BASE_LOCK, 2, 1000);
>> - if (!(reg & PLL_BASE_LOCK)) {
>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>> + if (ret) {
> Why this is needed? Was there a bug?
>
during resume pllu init is needed and to use same terga210_init_pllu,
poll_timeout_atomic can't be used as its ony for atomic context.
So changed to use wait_for_mask which should work in both cases.
>> pr_err("Timed out waiting for PLL_U to lock\n");
>> return -ETIMEDOUT;
>> }
>> @@ -3288,6 +3293,56 @@ static void tegra210_disable_cpu_clock(u32 cpu)
>> }
>>
>> #ifdef CONFIG_PM_SLEEP
>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) + ((_off) * 4))
>> +#define car_writel(_val, _base, _off) \
>> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>> +
>> +static u32 spare_reg_ctx, misc_clk_enb_ctx, clk_msk_arm_ctx;
>> +static u32 cpu_softrst_ctx[3];
>> +
>> +static int tegra210_clk_suspend(void)
>> +{
>> + unsigned int i;
>> +
>> + clk_save_context();
>> +
>> + /*
>> + * save the bootloader configured clock registers SPARE_REG0,
>> + * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL
> Nit: Start all multi-line comments with a capital letter and put dot in
> the end of sentence.
>
>> + */
>> + spare_reg_ctx = readl_relaxed(clk_base + SPARE_REG0);
>> + misc_clk_enb_ctx = readl_relaxed(clk_base + MISC_CLK_ENB);
>> + clk_msk_arm_ctx = readl_relaxed(clk_base + CLK_MASK_ARM);
>> +
>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>> +
>> + return 0;
>> +}
>> +
>> +static void tegra210_clk_resume(void)
>> +{
>> + unsigned int i;
>> +
>> + tegra_clk_osc_resume(clk_base);
>> +
>> + /*
>> + * restore the bootloader configured clock registers SPARE_REG0,
>> + * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL from saved context.
> Same here.
>
>> + */
>> + writel_relaxed(spare_reg_ctx, clk_base + SPARE_REG0);
>> + writel_relaxed(misc_clk_enb_ctx, clk_base + MISC_CLK_ENB);
>> + writel_relaxed(clk_msk_arm_ctx, clk_base + CLK_MASK_ARM);
>> +
>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>> +
>> + fence_udelay(5, clk_base);
>> +
>> + tegra210_init_pllu();
>> + clk_restore_context();
>> +}
>> +
>> static void tegra210_cpu_clock_suspend(void)
>> {
>> /* switch coresite to clk_m, save off original source */
>> @@ -3303,6 +3358,11 @@ static void tegra210_cpu_clock_resume(void)
>> }
>> #endif
>>
>> +static struct syscore_ops tegra_clk_syscore_ops = {
>> + .suspend = tegra210_clk_suspend,
>> + .resume = tegra210_clk_resume,
>> +};
>> +
>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>> .wait_for_reset = tegra210_wait_cpu_in_reset,
>> .disable_clock = tegra210_disable_cpu_clock,
>> @@ -3587,5 +3647,7 @@ static void __init tegra210_clock_init(struct device_node *np)
>> tegra210_mbist_clk_init();
>>
>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>> +
>> + register_syscore_ops(&tegra_clk_syscore_ops);
>> }
>> CLK_OF_DECLARE(tegra210, "nvidia,tegra210-car", tegra210_clock_init);
>> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
>> index 573e3c967ae1..eb08047fd02f 100644
>> --- a/drivers/clk/tegra/clk.c
>> +++ b/drivers/clk/tegra/clk.c
>> @@ -23,6 +23,7 @@
>> #define CLK_OUT_ENB_W 0x364
>> #define CLK_OUT_ENB_X 0x280
>> #define CLK_OUT_ENB_Y 0x298
>> +#define CLK_ENB_PLLP_OUT_CPU BIT(31)
>> #define CLK_OUT_ENB_SET_L 0x320
>> #define CLK_OUT_ENB_CLR_L 0x324
>> #define CLK_OUT_ENB_SET_H 0x328
>> @@ -199,6 +200,19 @@ const struct tegra_clk_periph_regs *get_reg_bank(int clkid)
>> }
>> }
>>
>> +void tegra_clk_set_pllp_out_cpu(bool enable)
>> +{
>> + u32 val;
>> +
>> + val = readl_relaxed(clk_base + CLK_OUT_ENB_Y);
>> + if (enable)
>> + val |= CLK_ENB_PLLP_OUT_CPU;
>> + else
>> + val &= ~CLK_ENB_PLLP_OUT_CPU;
>> +
>> + writel_relaxed(val, clk_base + CLK_OUT_ENB_Y);
>> +}
>> +
>> struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
>> {
>> clk_base = regs;
>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>> index 562a3ee2d537..0ffa763c755b 100644
>> --- a/drivers/clk/tegra/clk.h
>> +++ b/drivers/clk/tegra/clk.h
>> @@ -863,6 +863,7 @@ int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
>> u8 frac_width, u8 flags);
>> void tegra_clk_sync_state_pll(struct clk_hw *hw);
>> void tegra_clk_osc_resume(void __iomem *clk_base);
>> +void tegra_clk_set_pllp_out_cpu(bool enable);
>>
>> /* Combined read fence with delay */
>> #define fence_udelay(delay, reg) \
>>
^ permalink raw reply
* Re: [PATCH V6 11/21] clk: tegra: clk-dfll: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-07-21 22:42 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <54e3237d-c6c2-2007-99b2-89c26004da47@gmail.com>
On 7/21/19 2:32 PM, Dmitry Osipenko wrote:
> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>> This patch implements DFLL suspend and resume operation.
>>
>> During system suspend entry, CPU clock will switch CPU to safe
>> clock source of PLLP and disables DFLL clock output.
>>
>> DFLL driver suspend confirms DFLL disable state and errors out on
>> being active.
>>
>> DFLL is re-initialized during the DFLL driver resume as it goes
>> through complete reset during suspend entry.
>>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/clk/tegra/clk-dfll.c | 44 ++++++++++++++++++++++++++++++
>> drivers/clk/tegra/clk-dfll.h | 2 ++
>> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 1 +
>> 3 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
>> index f8688c2ddf1a..7dcad4ccd0ae 100644
>> --- a/drivers/clk/tegra/clk-dfll.c
>> +++ b/drivers/clk/tegra/clk-dfll.c
>> @@ -1513,6 +1513,50 @@ static int dfll_init(struct tegra_dfll *td)
>> return ret;
>> }
>>
>> +/**
>> + * tegra_dfll_suspend - check DFLL is disabled
>> + * @dev: DFLL device *
>> + *
>> + * DFLL clock should be disabled by the CPUFreq driver. So, make
>> + * sure it is disabled and disable all clocks needed by the DFLL.
>> + */
>> +int tegra_dfll_suspend(struct device *dev)
>> +{
>> + struct tegra_dfll *td = dev_get_drvdata(dev);
>> +
>> + if (dfll_is_running(td)) {
>> + dev_warn(td->dev, "failed disabling the dfll\n");
> Something like "dfll is enabled while shouldn't be\n" will be more
> informative.
>
> This is a error, hence dev_err().
>
>> + return -EBUSY;
>> + }
>> +
>> + pm_runtime_disable(dev);
>> +
>> + clk_unprepare(td->ref_clk);
>> + clk_unprepare(td->soc_clk);
>> + clk_unprepare(td->i2c_clk);
> Please don't do this, DFLL is already disabled if not running.
during resume dfll re-init sequence is same as dfll init so I am using
existing dfll_init
which includes runtime_enable and clk_prepare.
Will create separate dfll_reinit then...
>> + reset_control_assert(td->dvco_rst);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(tegra_dfll_suspend);
>> +
>> +/**
>> + * tegra_dfll_resume - reinitialize DFLL on resume
>> + * @pdev: DFLL instance
>> + *
>> + * Re-initialize DFLL on resume as it gets disabled and reset during
>> + * suspend entry. DFLL clock is enabled in closed loop mode later
>> + * and CPU frequency will be switched to DFLL output.
>> + */
>> +int tegra_dfll_resume(struct device *dev)
>> +{
>> + struct tegra_dfll *td = dev_get_drvdata(dev);
>> +
>> + return dfll_init(td);
> Just create dfll_reinit() variant.
>
>> +}
>> +EXPORT_SYMBOL(tegra_dfll_resume);
>> +
>> /*
>> * DT data fetch
>> */
>> diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
>> index 1b14ebe7268b..fb209eb5f365 100644
>> --- a/drivers/clk/tegra/clk-dfll.h
>> +++ b/drivers/clk/tegra/clk-dfll.h
>> @@ -42,5 +42,7 @@ int tegra_dfll_register(struct platform_device *pdev,
>> struct tegra_dfll_soc_data *tegra_dfll_unregister(struct platform_device *pdev);
>> int tegra_dfll_runtime_suspend(struct device *dev);
>> int tegra_dfll_runtime_resume(struct device *dev);
>> +int tegra_dfll_suspend(struct device *dev);
>> +int tegra_dfll_resume(struct device *dev);
>>
>> #endif /* __DRIVERS_CLK_TEGRA_CLK_DFLL_H */
>> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
>> index e84b6d52cbbd..2ac2679d696d 100644
>> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
>> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
>> @@ -631,6 +631,7 @@ static int tegra124_dfll_fcpu_remove(struct platform_device *pdev)
>> static const struct dev_pm_ops tegra124_dfll_pm_ops = {
>> SET_RUNTIME_PM_OPS(tegra_dfll_runtime_suspend,
>> tegra_dfll_runtime_resume, NULL)
>> + SET_SYSTEM_SLEEP_PM_OPS(tegra_dfll_suspend, tegra_dfll_resume)
>> };
>>
>> static struct platform_driver tegra124_dfll_fcpu_driver = {
>>
^ permalink raw reply
* Re: [PATCH V6 09/21] clk: tegra: clk-super: Fix to enable PLLP branches to CPU
From: Sowjanya Komatineni @ 2019-07-21 22:39 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <0c86cd7f-81b5-40c5-6f1e-796e8f13b522@gmail.com>
On 7/21/19 2:16 PM, Dmitry Osipenko wrote:
> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>> This patch has a fix to enable PLLP branches to CPU before changing
>> the CPU clusters clock source to PLLP for Gen5 Super clock.
>>
>> During system suspend entry and exit, CPU source will be switched
>> to PLLP and this needs PLLP branches to be enabled to CPU prior to
>> the switch.
>>
>> On system resume, warmboot code enables PLLP branches to CPU and
>> powers up the CPU with PLLP clock source.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/clk/tegra/clk-super.c | 11 +++++++++++
>> drivers/clk/tegra/clk-tegra-super-gen4.c | 4 ++--
>> drivers/clk/tegra/clk.h | 4 ++++
>> 3 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
>> index 39ef31b46df5..d73c587e4853 100644
>> --- a/drivers/clk/tegra/clk-super.c
>> +++ b/drivers/clk/tegra/clk-super.c
>> @@ -28,6 +28,9 @@
>> #define super_state_to_src_shift(m, s) ((m->width * s))
>> #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>>
>> +#define CCLK_SRC_PLLP_OUT0 4
>> +#define CCLK_SRC_PLLP_OUT4 5
>> +
>> static u8 clk_super_get_parent(struct clk_hw *hw)
>> {
>> struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
>> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw *hw, u8 index)
>> if (index == mux->div2_index)
>> index = mux->pllx_index;
>> }
>> +
>> + /*
>> + * Enable PLLP branches to CPU before selecting PLLP source
>> + */
>> + if ((mux->flags & TEGRA_CPU_CLK) &&
>> + ((index == CCLK_SRC_PLLP_OUT0) || (index == CCLK_SRC_PLLP_OUT4)))
>> + tegra_clk_set_pllp_out_cpu(true);
> Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
> switching from PLLP?
PLLP may be used for other CPU clusters.
>> val &= ~((super_state_to_src_mask(mux)) << shift);
>> val |= (index & (super_state_to_src_mask(mux))) << shift;
>>
>> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> index cdfe7c9697e1..cd208d0eca2a 100644
>> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
>> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
>> @@ -180,7 +180,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
>> gen_info->num_cclk_g_parents,
>> CLK_SET_RATE_PARENT,
>> clk_base + CCLKG_BURST_POLICY,
>> - 0, 4, 8, 0, NULL);
>> + TEGRA_CPU_CLK, 4, 8, 0, NULL);
>> } else {
>> clk = tegra_clk_register_super_mux("cclk_g",
>> gen_info->cclk_g_parents,
>> @@ -201,7 +201,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
>> gen_info->num_cclk_lp_parents,
>> CLK_SET_RATE_PARENT,
>> clk_base + CCLKLP_BURST_POLICY,
>> - 0, 4, 8, 0, NULL);
>> + TEGRA_CPU_CLK, 4, 8, 0, NULL);
>> } else {
>> clk = tegra_clk_register_super_mux("cclk_lp",
>> gen_info->cclk_lp_parents,
>> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
>> index ac6de3a0b91f..c357b49e49b0 100644
>> --- a/drivers/clk/tegra/clk.h
>> +++ b/drivers/clk/tegra/clk.h
>> @@ -694,6 +694,9 @@ struct clk *tegra_clk_register_periph_data(void __iomem *clk_base,
>> * Flags:
>> * TEGRA_DIVIDER_2 - LP cluster has additional divider. This flag indicates
>> * that this is LP cluster clock.
>> + * TEGRA_CPU_CLK - This flag indicates this is CPU cluster clock. To use PLLP
>> + * for CPU clock source, need to enable PLLP branches to CPU by setting the
>> + * additional bit PLLP_OUT_CPU for gen5 super clock.
>> */
>> struct tegra_clk_super_mux {
>> struct clk_hw hw;
>> @@ -710,6 +713,7 @@ struct tegra_clk_super_mux {
>> #define to_clk_super_mux(_hw) container_of(_hw, struct tegra_clk_super_mux, hw)
>>
>> #define TEGRA_DIVIDER_2 BIT(0)
>> +#define TEGRA_CPU_CLK BIT(1)
> I'd name this TEGRA210_CPU_CLK for clarity.
>
>> extern const struct clk_ops tegra_clk_super_ops;
>> struct clk *tegra_clk_register_super_mux(const char *name,
>>
> Will be better to move the tegra_clk_set_pllp_out_cpu() definition into
> this patch, otherwise this looks inconsistent for reviewer.
ok, Will move to this patch
^ permalink raw reply
* Re: [PATCH V6 21/21] arm64: dts: tegra210-p3450: Jetson nano SC7 timings
From: Dmitry Osipenko @ 2019-07-21 22:25 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-22-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:41, Sowjanya Komatineni пишет:
> This patch adds Jetson nano platform specific SC7 timing configuration
> in the device tree.
You should stat all names with a capital letter, hence "Jetson Nano" and
"Tegra" in all patches (commit title/message and code comments). When
unsure, just go to google and find how official name looks like.
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts b/arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts
> index 9d17ec707bce..b525e69c172a 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts
> +++ b/arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts
> @@ -382,6 +382,13 @@
>
> pmc@7000e400 {
> nvidia,invert-interrupt;
> + nvidia,suspend-mode = <0>;
> + nvidia,cpu-pwr-good-time = <0>;
> + nvidia,cpu-pwr-off-time = <0>;
> + nvidia,core-pwr-good-time = <4587 3876>;
> + nvidia,core-pwr-off-time = <39065>;
> + nvidia,core-power-req-active-high;
> + nvidia,sys-clock-req-active-high;
> };
>
> hda@70030000 {
>
^ permalink raw reply
* Re: [PATCH V6 06/21] clk: tegra: pll: Save and restore pll context
From: Dmitry Osipenko @ 2019-07-21 22:21 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-7-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch implements save and restore of PLL context.
>
> During system suspend, core power goes off and looses the settings
> of the Tegra CAR controller registers.
>
> So during suspend entry pll rate is stored and on resume it is
> restored back along with its state.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-pll.c | 121 ++++++++++++++++++++++++++++-----------
> drivers/clk/tegra/clk-tegra210.c | 2 +-
> drivers/clk/tegra/clk.h | 10 +++-
> 3 files changed, 99 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
> index 1583f5fc992f..f136964e6c44 100644
> --- a/drivers/clk/tegra/clk-pll.c
> +++ b/drivers/clk/tegra/clk-pll.c
> @@ -1008,6 +1008,59 @@ static unsigned long clk_plle_recalc_rate(struct clk_hw *hw,
> return rate;
> }
>
> +void tegra_clk_sync_state_pll(struct clk_hw *hw)
> +{
> + if (!__clk_get_enable_count(hw->clk))
> + clk_pll_disable(hw);
> + else
> + clk_pll_enable(hw);
> +}
> +
> +static int tegra_clk_pll_save_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_pll *pll = to_clk_pll(hw);
> + u32 val = 0;
> +
> + pll->rate = clk_hw_get_rate(hw);
Again, clk_hw_get_rate() returns cached value. Why do you need to
duplicate it?
> + if (pll->params->flags & TEGRA_PLLMB)
> + val = pll_readl_base(pll);
> + else if (pll->params->flags & TEGRA_PLLRE)
> + val = pll_readl_base(pll) & divp_mask_shifted(pll);
> +
> + pll->pllbase_ctx = val;
> +
> + return 0;
> +}
> +
> +static void tegra_clk_pll_restore_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_pll *pll = to_clk_pll(hw);
> + struct clk_hw *parent = clk_hw_get_parent(hw);
> + unsigned long parent_rate = clk_hw_get_rate(parent);
> + u32 val;
> +
> + if (clk_pll_is_enabled(hw))
> + return;
> +
> + if (pll->params->flags & TEGRA_PLLMB) {
> + pll_writel_base(pll->pllbase_ctx, pll);
> + } else if (pll->params->flags & TEGRA_PLLRE) {
> + val = pll_readl_base(pll);
> + val &= ~(divp_mask_shifted(pll));
> + pll_writel_base(pll->pllbase_ctx | val, pll);
> + }
> +
> + if (pll->params->set_defaults)
> + pll->params->set_defaults(pll);
> +
> + clk_pll_set_rate(hw, pll->rate, parent_rate);
> +
> + /* do not sync pllx state here. pllx is sync'd after dfll resume */
> + if (!(pll->params->flags & TEGRA_PLLX))
> + tegra_clk_sync_state_pll(hw);
> +}
> +
> const struct clk_ops tegra_clk_pll_ops = {
> .is_enabled = clk_pll_is_enabled,
> .enable = clk_pll_enable,
> @@ -1015,6 +1068,8 @@ const struct clk_ops tegra_clk_pll_ops = {
> .recalc_rate = clk_pll_recalc_rate,
> .round_rate = clk_pll_round_rate,
> .set_rate = clk_pll_set_rate,
> + .save_context = tegra_clk_pll_save_context,
> + .restore_context = tegra_clk_pll_restore_context,
> };
>
> const struct clk_ops tegra_clk_plle_ops = {
> @@ -1802,6 +1857,27 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw)
>
> return ret;
> }
> +
> +static void _clk_plle_tegra_init_parent(struct tegra_clk_pll *pll)
> +{
> + u32 val, val_aux;
> +
> + /* ensure parent is set to pll_ref */
> + val = pll_readl_base(pll);
> + val_aux = pll_readl(pll->params->aux_reg, pll);
> +
> + if (val & PLL_BASE_ENABLE) {
> + if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
> + (val_aux & PLLE_AUX_PLLP_SEL))
> + WARN(1, "pll_e enabled with unsupported parent %s\n",
> + (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
> + "pll_re_vco");
> + } else {
> + val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
> + pll_writel(val_aux, pll->params->aux_reg, pll);
> + fence_udelay(1, pll->clk_base);
> + }
> +}
> #endif
>
> static struct tegra_clk_pll *_tegra_init_pll(void __iomem *clk_base,
> @@ -2214,27 +2290,12 @@ struct clk *tegra_clk_register_plle_tegra114(const char *name,
> {
> struct tegra_clk_pll *pll;
> struct clk *clk;
> - u32 val, val_aux;
>
> pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
> if (IS_ERR(pll))
> return ERR_CAST(pll);
>
> - /* ensure parent is set to pll_re_vco */
> -
> - val = pll_readl_base(pll);
> - val_aux = pll_readl(pll_params->aux_reg, pll);
> -
> - if (val & PLL_BASE_ENABLE) {
> - if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
> - (val_aux & PLLE_AUX_PLLP_SEL))
> - WARN(1, "pll_e enabled with unsupported parent %s\n",
> - (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
> - "pll_re_vco");
> - } else {
> - val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
> - pll_writel(val_aux, pll_params->aux_reg, pll);
> - }
> + _clk_plle_tegra_init_parent(pll);
>
> clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
> &tegra_clk_plle_tegra114_ops);
> @@ -2276,6 +2337,8 @@ static const struct clk_ops tegra_clk_pllss_ops = {
> .recalc_rate = clk_pll_recalc_rate,
> .round_rate = clk_pll_ramp_round_rate,
> .set_rate = clk_pllxc_set_rate,
> + .save_context = tegra_clk_pll_save_context,
> + .restore_context = tegra_clk_pll_restore_context,
> };
>
> struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,
> @@ -2375,6 +2438,7 @@ struct clk *tegra_clk_register_pllre_tegra210(const char *name,
> pll_params->vco_min = pll_params->adjust_vco(pll_params,
> parent_rate);
>
> + pll_params->flags |= TEGRA_PLLRE;
> pll = _tegra_init_pll(clk_base, pmc, pll_params, lock);
> if (IS_ERR(pll))
> return ERR_CAST(pll);
> @@ -2520,11 +2584,19 @@ static void clk_plle_tegra210_disable(struct clk_hw *hw)
> spin_unlock_irqrestore(pll->lock, flags);
> }
>
> +static void tegra_clk_plle_t210_restore_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_pll *pll = to_clk_pll(hw);
> +
> + _clk_plle_tegra_init_parent(pll);
> +}
> +
> static const struct clk_ops tegra_clk_plle_tegra210_ops = {
> .is_enabled = clk_plle_tegra210_is_enabled,
> .enable = clk_plle_tegra210_enable,
> .disable = clk_plle_tegra210_disable,
> .recalc_rate = clk_pll_recalc_rate,
> + .restore_context = tegra_clk_plle_t210_restore_context,
> };
>
> struct clk *tegra_clk_register_plle_tegra210(const char *name,
> @@ -2535,27 +2607,12 @@ struct clk *tegra_clk_register_plle_tegra210(const char *name,
> {
> struct tegra_clk_pll *pll;
> struct clk *clk;
> - u32 val, val_aux;
>
> pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
> if (IS_ERR(pll))
> return ERR_CAST(pll);
>
> - /* ensure parent is set to pll_re_vco */
> -
> - val = pll_readl_base(pll);
> - val_aux = pll_readl(pll_params->aux_reg, pll);
> -
> - if (val & PLLE_BASE_ENABLE) {
> - if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
> - (val_aux & PLLE_AUX_PLLP_SEL))
> - WARN(1, "pll_e enabled with unsupported parent %s\n",
> - (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
> - "pll_re_vco");
> - } else {
> - val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
> - pll_writel(val_aux, pll_params->aux_reg, pll);
> - }
> + _clk_plle_tegra_init_parent(pll);
>
> clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
> &tegra_clk_plle_tegra210_ops);
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index 4721ee030d1c..58397f93166c 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -1602,7 +1602,7 @@ static struct tegra_clk_pll_params pll_x_params = {
> .pdiv_tohw = pll_qlin_pdiv_to_hw,
> .div_nmp = &pllx_nmp,
> .freq_table = pll_x_freq_table,
> - .flags = TEGRA_PLL_USE_LOCK | TEGRA_PLL_HAS_LOCK_ENABLE,
> + .flags = TEGRA_PLL_USE_LOCK | TEGRA_PLL_HAS_LOCK_ENABLE | TEGRA_PLLX,
> .dyn_ramp = tegra210_pllx_dyn_ramp,
> .set_defaults = tegra210_pllx_set_defaults,
> .calc_rate = tegra210_pll_fixed_mdiv_cfg,
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index fb29a8c27873..8532f5150091 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -235,6 +235,8 @@ struct tegra_clk_pll;
> * TEGRA_PLLMB - PLLMB has should be treated similar to PLLM. This
> * flag indicated that it is PLLMB.
> * TEGRA_PLL_VCO_OUT - Used to indicate that the PLL has a VCO output
> + * TEGRA_PLLRE - Used to indicate that it is PLLRE.
> + * TEGRA_PLLX - Used to indicate that it is PLLX.
> */
> struct tegra_clk_pll_params {
> unsigned long input_min;
> @@ -301,6 +303,8 @@ struct tegra_clk_pll_params {
> #define TEGRA_MDIV_NEW BIT(11)
> #define TEGRA_PLLMB BIT(12)
> #define TEGRA_PLL_VCO_OUT BIT(13)
> +#define TEGRA_PLLRE BIT(14)
> +#define TEGRA_PLLX BIT(15)
>
> /**
> * struct tegra_clk_pll - Tegra PLL clock
> @@ -310,6 +314,8 @@ struct tegra_clk_pll_params {
> * @pmc: address of PMC, required to read override bits
> * @lock: register lock
> * @params: PLL parameters
> + * @rate: rate during system suspend and resume
> + * @pllbase_ctx: pll base register value during suspend and resume
> */
> struct tegra_clk_pll {
> struct clk_hw hw;
> @@ -317,6 +323,8 @@ struct tegra_clk_pll {
> void __iomem *pmc;
> spinlock_t *lock;
> struct tegra_clk_pll_params *params;
> + unsigned long rate;
> + u32 pllbase_ctx;
> };
>
> #define to_clk_pll(_hw) container_of(_hw, struct tegra_clk_pll, hw)
> @@ -840,7 +848,7 @@ u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
> int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
> u8 frac_width, u8 flags);
> -
> +void tegra_clk_sync_state_pll(struct clk_hw *hw);
>
> /* Combined read fence with delay */
> #define fence_udelay(delay, reg) \
>
^ permalink raw reply
* Re: [PATCH V6 05/21] clk: tegra: pllout: Save and restore pllout context
From: Dmitry Osipenko @ 2019-07-21 22:18 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-6-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch implements save and restore of pllout context.
>
> During system suspend, core power goes off and looses the settings
> of the Tegra CAR controller registers.
>
> So during suspend entry the state of pllout is saved and on resume
> it is restored back to have pllout in same state as before suspend.
>
> pllout rate is saved and restore in clock divider so it will be at
> same rate as before suspend when pllout state is restored.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-pll-out.c | 28 ++++++++++++++++++++++++++++
> drivers/clk/tegra/clk-tegra210.c | 3 ++-
> drivers/clk/tegra/clk.h | 9 +++++++++
> 3 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/tegra/clk-pll-out.c b/drivers/clk/tegra/clk-pll-out.c
> index 35f2bf00e1e6..8f26a7e3e579 100644
> --- a/drivers/clk/tegra/clk-pll-out.c
> +++ b/drivers/clk/tegra/clk-pll-out.c
> @@ -69,10 +69,38 @@ static void clk_pll_out_disable(struct clk_hw *hw)
> spin_unlock_irqrestore(pll_out->lock, flags);
> }
>
> +static int tegra_clk_pll_out_save_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_pll_out *pll_out = to_clk_pll_out(hw);
> +
> + if (pll_out->flags & TEGRA_PLLRE_OUT)
> + pll_out->pllout_ctx = readl_relaxed(pll_out->reg);
> + else
> + pll_out->pllout_ctx = clk_hw_get_rate(hw);
This is unused?
> + return 0;
> +}
> +
> +static void tegra_clk_pll_out_restore_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_pll_out *pll_out = to_clk_pll_out(hw);
> +
> + if (pll_out->flags & TEGRA_PLLRE_OUT) {
> + writel_relaxed(pll_out->pllout_ctx, pll_out->reg);
> + } else {
> + if (!__clk_get_enable_count(hw->clk))
> + clk_pll_out_disable(hw);
> + else
> + clk_pll_out_enable(hw);
> + }
> +}
> +
> const struct clk_ops tegra_clk_pll_out_ops = {
> .is_enabled = clk_pll_out_is_enabled,
> .enable = clk_pll_out_enable,
> .disable = clk_pll_out_disable,
> + .save_context = tegra_clk_pll_out_save_context,
> + .restore_context = tegra_clk_pll_out_restore_context,
> };
>
> struct clk *tegra_clk_register_pll_out(const char *name,
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index df172d5772d7..4721ee030d1c 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -3200,7 +3200,8 @@ static void __init tegra210_pll_init(void __iomem *clk_base,
> 8, 8, 1, NULL);
> clk = tegra_clk_register_pll_out("pll_re_out1", "pll_re_out1_div",
> clk_base + PLLRE_OUT1, 1, 0,
> - CLK_SET_RATE_PARENT, 0, NULL);
> + CLK_SET_RATE_PARENT, TEGRA_PLLRE_OUT,
> + NULL);
> clks[TEGRA210_CLK_PLL_RE_OUT1] = clk;
>
> /* PLLE */
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index 83623f5f55f3..fb29a8c27873 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -439,6 +439,12 @@ struct clk *tegra_clk_register_pllu_tegra210(const char *name,
> * @rst_bit_idx: bit to reset PLL divider
> * @lock: register lock
> * @flags: hardware-specific flags
> + * @pllout_ctx: pllout context to save and restore during suspend
> + * and resume
> + *
> + * Flags:
> + * TEGRA_PLLRE_OUT - This flag indicates that it is PLLRE_OUT and is used to
> + * identify PLLRE_OUT during clk_pll_out save and restore.
> */
> struct tegra_clk_pll_out {
> struct clk_hw hw;
> @@ -447,8 +453,11 @@ struct tegra_clk_pll_out {
> u8 rst_bit_idx;
> spinlock_t *lock;
> u8 flags;
> + unsigned int pllout_ctx;
u32
> };
>
> +#define TEGRA_PLLRE_OUT BIT(0)
> +
> #define to_clk_pll_out(_hw) container_of(_hw, struct tegra_clk_pll_out, hw)
>
> extern const struct clk_ops tegra_clk_pll_out_ops;
>
^ permalink raw reply
* Re: [PATCH V6 04/21] clk: tegra: Save and restore divider rate
From: Dmitry Osipenko @ 2019-07-21 22:14 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-5-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch implements context save and restore for clock divider.
>
> During system suspend, core power goes off and looses the settings
> of the Tegra CAR controller registers.
>
> So during suspend entry the context of clock divider is saved and
> on resume context is restored back for normal operation.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-divider.c | 23 +++++++++++++++++++++++
> drivers/clk/tegra/clk.h | 2 ++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
> index e76731fb7d69..ecb7ff9ce97e 100644
> --- a/drivers/clk/tegra/clk-divider.c
> +++ b/drivers/clk/tegra/clk-divider.c
> @@ -109,10 +109,33 @@ static int clk_frac_div_set_rate(struct clk_hw *hw, unsigned long rate,
> return 0;
> }
>
> +static int clk_divider_save_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_frac_div *divider = to_clk_frac_div(hw);
> + struct clk_hw *parent = clk_hw_get_parent(hw);
> + unsigned long parent_rate = clk_hw_get_rate(parent);
> +
> + divider->rate = clk_frac_div_recalc_rate(hw, parent_rate);
I'm not sure what's the point of this, because clk_hw_get_rate() returns
cached value.
> + return 0;
> +}
> +
> +static void clk_divider_restore_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_frac_div *divider = to_clk_frac_div(hw);
> + struct clk_hw *parent = clk_hw_get_parent(hw);
> + unsigned long parent_rate = clk_hw_get_rate(parent);
> +
> + if (clk_frac_div_set_rate(hw, divider->rate, parent_rate) < 0)
> + WARN_ON(1);
Hence this could be:
unsigned long parent_rate = clk_hw_get_rate(parent);
unsigned long rate = clk_hw_get_rate(hw);
if (clk_frac_div_set_rate(hw, rate, parent_rate) < 0)
WARN_ON(1);
> +}
> +
> const struct clk_ops tegra_clk_frac_div_ops = {
> .recalc_rate = clk_frac_div_recalc_rate,
> .set_rate = clk_frac_div_set_rate,
> .round_rate = clk_frac_div_round_rate,
> + .save_context = clk_divider_save_context,
> + .restore_context = clk_divider_restore_context,
> };
>
> struct clk *tegra_clk_register_divider(const char *name,
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index 905bf1096558..83623f5f55f3 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -42,6 +42,7 @@ struct clk *tegra_clk_register_sync_source(const char *name,
> * @width: width of the divider bit field
> * @frac_width: width of the fractional bit field
> * @lock: register lock
> + * @rate: rate during suspend and resume
> *
> * Flags:
> * TEGRA_DIVIDER_ROUND_UP - This flags indicates to round up the divider value.
> @@ -62,6 +63,7 @@ struct tegra_clk_frac_div {
> u8 width;
> u8 frac_width;
> spinlock_t *lock;
> + unsigned long rate;
> };
>
> #define to_clk_frac_div(_hw) container_of(_hw, struct tegra_clk_frac_div, hw)
>
^ permalink raw reply
* Re: [PATCH V6 02/21] pinctrl: tegra: Add suspend and resume support
From: Dmitry Osipenko @ 2019-07-21 22:09 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <d837e4e0-947b-2b90-c680-ab913e70e587@gmail.com>
22.07.2019 1:03, Dmitry Osipenko пишет:
> 21.07.2019 22:40, Sowjanya Komatineni пишет:
>> This patch adds support for Tegra pinctrl driver suspend and resume.
>>
>> During suspend, context of all pinctrl registers are stored and
>> on resume they are all restored to have all the pinmux and pad
>> configuration for normal operation.
>>
>> Acked-by: Thierry Reding <treding@nvidia.com>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/pinctrl/tegra/pinctrl-tegra.c | 59 +++++++++++++++++++++++++++++++++++
>> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
>> 2 files changed, 62 insertions(+)
>>
>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> index 186ef98e7b2b..e3a237534281 100644
>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> @@ -631,6 +631,58 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>> }
>> }
>>
>> +static size_t tegra_pinctrl_get_bank_size(struct device *dev,
>> + unsigned int bank_id)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct resource *res;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, bank_id);
>> +
>> + return resource_size(res) / 4;
>> +}
>> +
>> +static int tegra_pinctrl_suspend(struct device *dev)
>> +{
>> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
>> + u32 *backup_regs = pmx->backup_regs;
>> + u32 *regs;
>> + size_t bank_size;
>> + unsigned int i, k;
>> +
>> + for (i = 0; i < pmx->nbanks; i++) {
>> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
>> + regs = pmx->regs[i];
>> + for (k = 0; k < bank_size; k++)
>> + *backup_regs++ = readl_relaxed(regs++);
>> + }
>> +
>> + return pinctrl_force_sleep(pmx->pctl);
>> +}
>> +
>> +static int tegra_pinctrl_resume(struct device *dev)
>> +{
>> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
>> + u32 *backup_regs = pmx->backup_regs;
>> + u32 *regs;
>> + size_t bank_size;
>> + unsigned int i, k;
>> +
>> + for (i = 0; i < pmx->nbanks; i++) {
>> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
>> + regs = pmx->regs[i];
>> + for (k = 0; k < bank_size; k++)
>> + writel_relaxed(*backup_regs++, regs++);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +const struct dev_pm_ops tegra_pinctrl_pm = {
>> + .suspend = &tegra_pinctrl_suspend,
>> + .resume = &tegra_pinctrl_resume
>> +};
>> +
>> static bool gpio_node_has_range(const char *compatible)
>> {
>> struct device_node *np;
>> @@ -655,6 +707,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
>> int i;
>> const char **group_pins;
>> int fn, gn, gfn;
>> + unsigned long backup_regs_size = 0;
>>
>> pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
>> if (!pmx)
>> @@ -707,6 +760,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
>> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
>> if (!res)
>> break;
>> + backup_regs_size += resource_size(res);
>> }
>> pmx->nbanks = i;
>>
>> @@ -715,6 +769,11 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
>> if (!pmx->regs)
>> return -ENOMEM;
>>
>> + pmx->backup_regs = devm_kzalloc(&pdev->dev, backup_regs_size,
>> + GFP_KERNEL);
>> + if (!pmx->backup_regs)
>> + return -ENOMEM;
>> +
>> for (i = 0; i < pmx->nbanks; i++) {
>> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
>> pmx->regs[i] = devm_ioremap_resource(&pdev->dev, res);
>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
>> index 105309774079..0fc82eea9cf1 100644
>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.h
>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
>> @@ -17,6 +17,7 @@ struct tegra_pmx {
>>
>> int nbanks;
>> void __iomem **regs;
>> + u32 *backup_regs;
>> };
>>
>> enum tegra_pinconf_param {
>> @@ -193,6 +194,8 @@ struct tegra_pinctrl_soc_data {
>> bool drvtype_in_mux;
>> };
>>
>> +extern const struct dev_pm_ops tegra_pinctrl_pm;
>> +
>> int tegra_pinctrl_probe(struct platform_device *pdev,
>> const struct tegra_pinctrl_soc_data *soc_data);
>> #endif
>>
>
> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
>
BTW, you should remove Thierry's ACK from all patches that were modified
if he didn't re-ACK the new version.
Also, it looks to me that you're manually adding versioning to the
patches because git usually uses lowercase for 'v'. You could use "git
format-patch -v6 ..".
^ permalink raw reply
* Re: [PATCH V6 02/21] pinctrl: tegra: Add suspend and resume support
From: Dmitry Osipenko @ 2019-07-21 22:03 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-3-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch adds support for Tegra pinctrl driver suspend and resume.
>
> During suspend, context of all pinctrl registers are stored and
> on resume they are all restored to have all the pinmux and pad
> configuration for normal operation.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra.c | 59 +++++++++++++++++++++++++++++++++++
> drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
> 2 files changed, 62 insertions(+)
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 186ef98e7b2b..e3a237534281 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -631,6 +631,58 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> }
> }
>
> +static size_t tegra_pinctrl_get_bank_size(struct device *dev,
> + unsigned int bank_id)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct resource *res;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, bank_id);
> +
> + return resource_size(res) / 4;
> +}
> +
> +static int tegra_pinctrl_suspend(struct device *dev)
> +{
> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
> + u32 *backup_regs = pmx->backup_regs;
> + u32 *regs;
> + size_t bank_size;
> + unsigned int i, k;
> +
> + for (i = 0; i < pmx->nbanks; i++) {
> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
> + regs = pmx->regs[i];
> + for (k = 0; k < bank_size; k++)
> + *backup_regs++ = readl_relaxed(regs++);
> + }
> +
> + return pinctrl_force_sleep(pmx->pctl);
> +}
> +
> +static int tegra_pinctrl_resume(struct device *dev)
> +{
> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
> + u32 *backup_regs = pmx->backup_regs;
> + u32 *regs;
> + size_t bank_size;
> + unsigned int i, k;
> +
> + for (i = 0; i < pmx->nbanks; i++) {
> + bank_size = tegra_pinctrl_get_bank_size(dev, i);
> + regs = pmx->regs[i];
> + for (k = 0; k < bank_size; k++)
> + writel_relaxed(*backup_regs++, regs++);
> + }
> +
> + return 0;
> +}
> +
> +const struct dev_pm_ops tegra_pinctrl_pm = {
> + .suspend = &tegra_pinctrl_suspend,
> + .resume = &tegra_pinctrl_resume
> +};
> +
> static bool gpio_node_has_range(const char *compatible)
> {
> struct device_node *np;
> @@ -655,6 +707,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
> int i;
> const char **group_pins;
> int fn, gn, gfn;
> + unsigned long backup_regs_size = 0;
>
> pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
> if (!pmx)
> @@ -707,6 +760,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> if (!res)
> break;
> + backup_regs_size += resource_size(res);
> }
> pmx->nbanks = i;
>
> @@ -715,6 +769,11 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
> if (!pmx->regs)
> return -ENOMEM;
>
> + pmx->backup_regs = devm_kzalloc(&pdev->dev, backup_regs_size,
> + GFP_KERNEL);
> + if (!pmx->backup_regs)
> + return -ENOMEM;
> +
> for (i = 0; i < pmx->nbanks; i++) {
> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> pmx->regs[i] = devm_ioremap_resource(&pdev->dev, res);
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
> index 105309774079..0fc82eea9cf1 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.h
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
> @@ -17,6 +17,7 @@ struct tegra_pmx {
>
> int nbanks;
> void __iomem **regs;
> + u32 *backup_regs;
> };
>
> enum tegra_pinconf_param {
> @@ -193,6 +194,8 @@ struct tegra_pinctrl_soc_data {
> bool drvtype_in_mux;
> };
>
> +extern const struct dev_pm_ops tegra_pinctrl_pm;
> +
> int tegra_pinctrl_probe(struct platform_device *pdev,
> const struct tegra_pinctrl_soc_data *soc_data);
> #endif
>
Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
^ permalink raw reply
* Re: [PATCH V6 06/21] clk: tegra: pll: Save and restore pll context
From: Dmitry Osipenko @ 2019-07-21 21:44 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-7-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch implements save and restore of PLL context.
>
> During system suspend, core power goes off and looses the settings
> of the Tegra CAR controller registers.
>
> So during suspend entry pll rate is stored and on resume it is
> restored back along with its state.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-pll.c | 121 ++++++++++++++++++++++++++++-----------
> drivers/clk/tegra/clk-tegra210.c | 2 +-
> drivers/clk/tegra/clk.h | 10 +++-
> 3 files changed, 99 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
> index 1583f5fc992f..f136964e6c44 100644
> --- a/drivers/clk/tegra/clk-pll.c
> +++ b/drivers/clk/tegra/clk-pll.c
> @@ -1008,6 +1008,59 @@ static unsigned long clk_plle_recalc_rate(struct clk_hw *hw,
> return rate;
> }
>
> +void tegra_clk_sync_state_pll(struct clk_hw *hw)
> +{
> + if (!__clk_get_enable_count(hw->clk))
> + clk_pll_disable(hw);
> + else
> + clk_pll_enable(hw);
> +}
> +
> +static int tegra_clk_pll_save_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_pll *pll = to_clk_pll(hw);
> + u32 val = 0;
> +
> + pll->rate = clk_hw_get_rate(hw);
> +
> + if (pll->params->flags & TEGRA_PLLMB)
> + val = pll_readl_base(pll);
> + else if (pll->params->flags & TEGRA_PLLRE)
> + val = pll_readl_base(pll) & divp_mask_shifted(pll);
> +
> + pll->pllbase_ctx = val;
> +
> + return 0;
> +}
> +
> +static void tegra_clk_pll_restore_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_pll *pll = to_clk_pll(hw);
> + struct clk_hw *parent = clk_hw_get_parent(hw);
> + unsigned long parent_rate = clk_hw_get_rate(parent);
> + u32 val;
> +
> + if (clk_pll_is_enabled(hw))
> + return;
> +
> + if (pll->params->flags & TEGRA_PLLMB) {
> + pll_writel_base(pll->pllbase_ctx, pll);
> + } else if (pll->params->flags & TEGRA_PLLRE) {
> + val = pll_readl_base(pll);
> + val &= ~(divp_mask_shifted(pll));
> + pll_writel_base(pll->pllbase_ctx | val, pll);
> + }
> +
> + if (pll->params->set_defaults)
> + pll->params->set_defaults(pll);
> +
> + clk_pll_set_rate(hw, pll->rate, parent_rate);
> +
> + /* do not sync pllx state here. pllx is sync'd after dfll resume */
> + if (!(pll->params->flags & TEGRA_PLLX))
> + tegra_clk_sync_state_pll(hw);
> +}
> +
> const struct clk_ops tegra_clk_pll_ops = {
> .is_enabled = clk_pll_is_enabled,
> .enable = clk_pll_enable,
> @@ -1015,6 +1068,8 @@ const struct clk_ops tegra_clk_pll_ops = {
> .recalc_rate = clk_pll_recalc_rate,
> .round_rate = clk_pll_round_rate,
> .set_rate = clk_pll_set_rate,
> + .save_context = tegra_clk_pll_save_context,
> + .restore_context = tegra_clk_pll_restore_context,
> };
>
> const struct clk_ops tegra_clk_plle_ops = {
> @@ -1802,6 +1857,27 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw)
>
> return ret;
> }
> +
> +static void _clk_plle_tegra_init_parent(struct tegra_clk_pll *pll)
> +{
> + u32 val, val_aux;
> +
> + /* ensure parent is set to pll_ref */
> + val = pll_readl_base(pll);
> + val_aux = pll_readl(pll->params->aux_reg, pll);
> +
> + if (val & PLL_BASE_ENABLE) {
> + if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
> + (val_aux & PLLE_AUX_PLLP_SEL))
> + WARN(1, "pll_e enabled with unsupported parent %s\n",
> + (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
> + "pll_re_vco");
> + } else {
> + val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
> + pll_writel(val_aux, pll->params->aux_reg, pll);
> + fence_udelay(1, pll->clk_base);
> + }
> +}
> #endif
>
> static struct tegra_clk_pll *_tegra_init_pll(void __iomem *clk_base,
> @@ -2214,27 +2290,12 @@ struct clk *tegra_clk_register_plle_tegra114(const char *name,
> {
> struct tegra_clk_pll *pll;
> struct clk *clk;
> - u32 val, val_aux;
>
> pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
> if (IS_ERR(pll))
> return ERR_CAST(pll);
>
> - /* ensure parent is set to pll_re_vco */
> -
> - val = pll_readl_base(pll);
> - val_aux = pll_readl(pll_params->aux_reg, pll);
> -
> - if (val & PLL_BASE_ENABLE) {
> - if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
> - (val_aux & PLLE_AUX_PLLP_SEL))
> - WARN(1, "pll_e enabled with unsupported parent %s\n",
> - (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
> - "pll_re_vco");
> - } else {
> - val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
> - pll_writel(val_aux, pll_params->aux_reg, pll);
> - }
> + _clk_plle_tegra_init_parent(pll);
>
> clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
> &tegra_clk_plle_tegra114_ops);
> @@ -2276,6 +2337,8 @@ static const struct clk_ops tegra_clk_pllss_ops = {
> .recalc_rate = clk_pll_recalc_rate,
> .round_rate = clk_pll_ramp_round_rate,
> .set_rate = clk_pllxc_set_rate,
> + .save_context = tegra_clk_pll_save_context,
> + .restore_context = tegra_clk_pll_restore_context,
> };
>
> struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,
> @@ -2375,6 +2438,7 @@ struct clk *tegra_clk_register_pllre_tegra210(const char *name,
> pll_params->vco_min = pll_params->adjust_vco(pll_params,
> parent_rate);
>
> + pll_params->flags |= TEGRA_PLLRE;
> pll = _tegra_init_pll(clk_base, pmc, pll_params, lock);
> if (IS_ERR(pll))
> return ERR_CAST(pll);
> @@ -2520,11 +2584,19 @@ static void clk_plle_tegra210_disable(struct clk_hw *hw)
> spin_unlock_irqrestore(pll->lock, flags);
> }
>
> +static void tegra_clk_plle_t210_restore_context(struct clk_hw *hw)
> +{
> + struct tegra_clk_pll *pll = to_clk_pll(hw);
> +
> + _clk_plle_tegra_init_parent(pll);
> +}
> +
> static const struct clk_ops tegra_clk_plle_tegra210_ops = {
> .is_enabled = clk_plle_tegra210_is_enabled,
> .enable = clk_plle_tegra210_enable,
> .disable = clk_plle_tegra210_disable,
> .recalc_rate = clk_pll_recalc_rate,
> + .restore_context = tegra_clk_plle_t210_restore_context,
> };
>
> struct clk *tegra_clk_register_plle_tegra210(const char *name,
> @@ -2535,27 +2607,12 @@ struct clk *tegra_clk_register_plle_tegra210(const char *name,
> {
> struct tegra_clk_pll *pll;
> struct clk *clk;
> - u32 val, val_aux;
>
> pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
> if (IS_ERR(pll))
> return ERR_CAST(pll);
>
> - /* ensure parent is set to pll_re_vco */
> -
> - val = pll_readl_base(pll);
> - val_aux = pll_readl(pll_params->aux_reg, pll);
> -
> - if (val & PLLE_BASE_ENABLE) {
> - if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
> - (val_aux & PLLE_AUX_PLLP_SEL))
> - WARN(1, "pll_e enabled with unsupported parent %s\n",
> - (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
> - "pll_re_vco");
> - } else {
> - val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
> - pll_writel(val_aux, pll_params->aux_reg, pll);
> - }
> + _clk_plle_tegra_init_parent(pll);
>
> clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
> &tegra_clk_plle_tegra210_ops);
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index 4721ee030d1c..58397f93166c 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -1602,7 +1602,7 @@ static struct tegra_clk_pll_params pll_x_params = {
> .pdiv_tohw = pll_qlin_pdiv_to_hw,
> .div_nmp = &pllx_nmp,
> .freq_table = pll_x_freq_table,
> - .flags = TEGRA_PLL_USE_LOCK | TEGRA_PLL_HAS_LOCK_ENABLE,
> + .flags = TEGRA_PLL_USE_LOCK | TEGRA_PLL_HAS_LOCK_ENABLE | TEGRA_PLLX,
> .dyn_ramp = tegra210_pllx_dyn_ramp,
> .set_defaults = tegra210_pllx_set_defaults,
> .calc_rate = tegra210_pll_fixed_mdiv_cfg,
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index fb29a8c27873..8532f5150091 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -235,6 +235,8 @@ struct tegra_clk_pll;
> * TEGRA_PLLMB - PLLMB has should be treated similar to PLLM. This
> * flag indicated that it is PLLMB.
> * TEGRA_PLL_VCO_OUT - Used to indicate that the PLL has a VCO output
> + * TEGRA_PLLRE - Used to indicate that it is PLLRE.
> + * TEGRA_PLLX - Used to indicate that it is PLLX.
> */
> struct tegra_clk_pll_params {
> unsigned long input_min;
> @@ -301,6 +303,8 @@ struct tegra_clk_pll_params {
> #define TEGRA_MDIV_NEW BIT(11)
> #define TEGRA_PLLMB BIT(12)
> #define TEGRA_PLL_VCO_OUT BIT(13)
> +#define TEGRA_PLLRE BIT(14)
> +#define TEGRA_PLLX BIT(15)
>
> /**
> * struct tegra_clk_pll - Tegra PLL clock
> @@ -310,6 +314,8 @@ struct tegra_clk_pll_params {
> * @pmc: address of PMC, required to read override bits
> * @lock: register lock
> * @params: PLL parameters
> + * @rate: rate during system suspend and resume
> + * @pllbase_ctx: pll base register value during suspend and resume
> */
> struct tegra_clk_pll {
> struct clk_hw hw;
> @@ -317,6 +323,8 @@ struct tegra_clk_pll {
> void __iomem *pmc;
> spinlock_t *lock;
> struct tegra_clk_pll_params *params;
> + unsigned long rate;
> + u32 pllbase_ctx;
> };
>
> #define to_clk_pll(_hw) container_of(_hw, struct tegra_clk_pll, hw)
> @@ -840,7 +848,7 @@ u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
> int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
> int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
> u8 frac_width, u8 flags);
> -
> +void tegra_clk_sync_state_pll(struct clk_hw *hw);
Looks like this function isn't used anywhere other than this patch. Bug?
> /* Combined read fence with delay */
> #define fence_udelay(delay, reg) \
>
^ permalink raw reply
* Re: [PATCH V6 14/21] clk: tegra210: Add suspend and resume support
From: Dmitry Osipenko @ 2019-07-21 21:38 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-15-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch adds support for clk: tegra210: suspend-resume.
>
> All the CAR controller settings are lost on suspend when core
> power goes off.
>
> This patch has implementation for saving and restoring all PLLs
> and clocks context during system suspend and resume to have the
> clocks back to same state for normal operation.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-tegra210.c | 68 ++++++++++++++++++++++++++++++++++++++--
> drivers/clk/tegra/clk.c | 14 +++++++++
> drivers/clk/tegra/clk.h | 1 +
> 3 files changed, 80 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
> index 55a88c0824a5..68271873acc1 100644
> --- a/drivers/clk/tegra/clk-tegra210.c
> +++ b/drivers/clk/tegra/clk-tegra210.c
> @@ -9,6 +9,7 @@
> #include <linux/clkdev.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> +#include <linux/syscore_ops.h>
> #include <linux/delay.h>
> #include <linux/export.h>
> #include <linux/mutex.h>
> @@ -220,11 +221,15 @@
> #define CLK_M_DIVISOR_SHIFT 2
> #define CLK_M_DIVISOR_MASK 0x3
>
> +#define CLK_MASK_ARM 0x44
> +#define MISC_CLK_ENB 0x48
> +
> #define RST_DFLL_DVCO 0x2f4
> #define DVFS_DFLL_RESET_SHIFT 0
>
> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
> +#define CPU_SOFTRST_CTRL 0x380
>
> #define LVL2_CLK_GATE_OVRA 0xf8
> #define LVL2_CLK_GATE_OVRC 0x3a0
> @@ -2825,6 +2830,7 @@ static int tegra210_enable_pllu(void)
> struct tegra_clk_pll_freq_table *fentry;
> struct tegra_clk_pll pllu;
> u32 reg;
> + int ret;
>
> for (fentry = pll_u_freq_table; fentry->input_rate; fentry++) {
> if (fentry->input_rate == pll_ref_freq)
> @@ -2853,9 +2859,8 @@ static int tegra210_enable_pllu(void)
> reg |= PLL_ENABLE;
> writel(reg, clk_base + PLLU_BASE);
>
> - readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
> - reg & PLL_BASE_LOCK, 2, 1000);
> - if (!(reg & PLL_BASE_LOCK)) {
> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
> + if (ret) {
Why this is needed? Was there a bug?
> pr_err("Timed out waiting for PLL_U to lock\n");
> return -ETIMEDOUT;
> }
> @@ -3288,6 +3293,56 @@ static void tegra210_disable_cpu_clock(u32 cpu)
> }
>
> #ifdef CONFIG_PM_SLEEP
> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) + ((_off) * 4))
> +#define car_writel(_val, _base, _off) \
> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
> +
> +static u32 spare_reg_ctx, misc_clk_enb_ctx, clk_msk_arm_ctx;
> +static u32 cpu_softrst_ctx[3];
> +
> +static int tegra210_clk_suspend(void)
> +{
> + unsigned int i;
> +
> + clk_save_context();
> +
> + /*
> + * save the bootloader configured clock registers SPARE_REG0,
> + * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL
Nit: Start all multi-line comments with a capital letter and put dot in
the end of sentence.
> + */
> + spare_reg_ctx = readl_relaxed(clk_base + SPARE_REG0);
> + misc_clk_enb_ctx = readl_relaxed(clk_base + MISC_CLK_ENB);
> + clk_msk_arm_ctx = readl_relaxed(clk_base + CLK_MASK_ARM);
> +
> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
> +
> + return 0;
> +}
> +
> +static void tegra210_clk_resume(void)
> +{
> + unsigned int i;
> +
> + tegra_clk_osc_resume(clk_base);
> +
> + /*
> + * restore the bootloader configured clock registers SPARE_REG0,
> + * MISC_CLK_ENB, CLK_MASK_ARM, CPU_SOFTRST_CTRL from saved context.
Same here.
> + */
> + writel_relaxed(spare_reg_ctx, clk_base + SPARE_REG0);
> + writel_relaxed(misc_clk_enb_ctx, clk_base + MISC_CLK_ENB);
> + writel_relaxed(clk_msk_arm_ctx, clk_base + CLK_MASK_ARM);
> +
> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
> +
> + fence_udelay(5, clk_base);
> +
> + tegra210_init_pllu();
> + clk_restore_context();
> +}
> +
> static void tegra210_cpu_clock_suspend(void)
> {
> /* switch coresite to clk_m, save off original source */
> @@ -3303,6 +3358,11 @@ static void tegra210_cpu_clock_resume(void)
> }
> #endif
>
> +static struct syscore_ops tegra_clk_syscore_ops = {
> + .suspend = tegra210_clk_suspend,
> + .resume = tegra210_clk_resume,
> +};
> +
> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
> .wait_for_reset = tegra210_wait_cpu_in_reset,
> .disable_clock = tegra210_disable_cpu_clock,
> @@ -3587,5 +3647,7 @@ static void __init tegra210_clock_init(struct device_node *np)
> tegra210_mbist_clk_init();
>
> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
> +
> + register_syscore_ops(&tegra_clk_syscore_ops);
> }
> CLK_OF_DECLARE(tegra210, "nvidia,tegra210-car", tegra210_clock_init);
> diff --git a/drivers/clk/tegra/clk.c b/drivers/clk/tegra/clk.c
> index 573e3c967ae1..eb08047fd02f 100644
> --- a/drivers/clk/tegra/clk.c
> +++ b/drivers/clk/tegra/clk.c
> @@ -23,6 +23,7 @@
> #define CLK_OUT_ENB_W 0x364
> #define CLK_OUT_ENB_X 0x280
> #define CLK_OUT_ENB_Y 0x298
> +#define CLK_ENB_PLLP_OUT_CPU BIT(31)
> #define CLK_OUT_ENB_SET_L 0x320
> #define CLK_OUT_ENB_CLR_L 0x324
> #define CLK_OUT_ENB_SET_H 0x328
> @@ -199,6 +200,19 @@ const struct tegra_clk_periph_regs *get_reg_bank(int clkid)
> }
> }
>
> +void tegra_clk_set_pllp_out_cpu(bool enable)
> +{
> + u32 val;
> +
> + val = readl_relaxed(clk_base + CLK_OUT_ENB_Y);
> + if (enable)
> + val |= CLK_ENB_PLLP_OUT_CPU;
> + else
> + val &= ~CLK_ENB_PLLP_OUT_CPU;
> +
> + writel_relaxed(val, clk_base + CLK_OUT_ENB_Y);
> +}
> +
> struct clk ** __init tegra_clk_init(void __iomem *regs, int num, int banks)
> {
> clk_base = regs;
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index 562a3ee2d537..0ffa763c755b 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -863,6 +863,7 @@ int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
> u8 frac_width, u8 flags);
> void tegra_clk_sync_state_pll(struct clk_hw *hw);
> void tegra_clk_osc_resume(void __iomem *clk_base);
> +void tegra_clk_set_pllp_out_cpu(bool enable);
>
> /* Combined read fence with delay */
> #define fence_udelay(delay, reg) \
>
^ permalink raw reply
* Re: [PATCH V6 11/21] clk: tegra: clk-dfll: Add suspend and resume support
From: Dmitry Osipenko @ 2019-07-21 21:32 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-12-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch implements DFLL suspend and resume operation.
>
> During system suspend entry, CPU clock will switch CPU to safe
> clock source of PLLP and disables DFLL clock output.
>
> DFLL driver suspend confirms DFLL disable state and errors out on
> being active.
>
> DFLL is re-initialized during the DFLL driver resume as it goes
> through complete reset during suspend entry.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-dfll.c | 44 ++++++++++++++++++++++++++++++
> drivers/clk/tegra/clk-dfll.h | 2 ++
> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 1 +
> 3 files changed, 47 insertions(+)
>
> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> index f8688c2ddf1a..7dcad4ccd0ae 100644
> --- a/drivers/clk/tegra/clk-dfll.c
> +++ b/drivers/clk/tegra/clk-dfll.c
> @@ -1513,6 +1513,50 @@ static int dfll_init(struct tegra_dfll *td)
> return ret;
> }
>
> +/**
> + * tegra_dfll_suspend - check DFLL is disabled
> + * @dev: DFLL device *
> + *
> + * DFLL clock should be disabled by the CPUFreq driver. So, make
> + * sure it is disabled and disable all clocks needed by the DFLL.
> + */
> +int tegra_dfll_suspend(struct device *dev)
> +{
> + struct tegra_dfll *td = dev_get_drvdata(dev);
> +
> + if (dfll_is_running(td)) {
> + dev_warn(td->dev, "failed disabling the dfll\n");
Something like "dfll is enabled while shouldn't be\n" will be more
informative.
This is a error, hence dev_err().
> + return -EBUSY;
> + }
> +
> + pm_runtime_disable(dev);
> +
> + clk_unprepare(td->ref_clk);
> + clk_unprepare(td->soc_clk);
> + clk_unprepare(td->i2c_clk);
Please don't do this, DFLL is already disabled if not running.
> + reset_control_assert(td->dvco_rst);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(tegra_dfll_suspend);
> +
> +/**
> + * tegra_dfll_resume - reinitialize DFLL on resume
> + * @pdev: DFLL instance
> + *
> + * Re-initialize DFLL on resume as it gets disabled and reset during
> + * suspend entry. DFLL clock is enabled in closed loop mode later
> + * and CPU frequency will be switched to DFLL output.
> + */
> +int tegra_dfll_resume(struct device *dev)
> +{
> + struct tegra_dfll *td = dev_get_drvdata(dev);
> +
> + return dfll_init(td);
Just create dfll_reinit() variant.
> +}
> +EXPORT_SYMBOL(tegra_dfll_resume);
> +
> /*
> * DT data fetch
> */
> diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
> index 1b14ebe7268b..fb209eb5f365 100644
> --- a/drivers/clk/tegra/clk-dfll.h
> +++ b/drivers/clk/tegra/clk-dfll.h
> @@ -42,5 +42,7 @@ int tegra_dfll_register(struct platform_device *pdev,
> struct tegra_dfll_soc_data *tegra_dfll_unregister(struct platform_device *pdev);
> int tegra_dfll_runtime_suspend(struct device *dev);
> int tegra_dfll_runtime_resume(struct device *dev);
> +int tegra_dfll_suspend(struct device *dev);
> +int tegra_dfll_resume(struct device *dev);
>
> #endif /* __DRIVERS_CLK_TEGRA_CLK_DFLL_H */
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index e84b6d52cbbd..2ac2679d696d 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -631,6 +631,7 @@ static int tegra124_dfll_fcpu_remove(struct platform_device *pdev)
> static const struct dev_pm_ops tegra124_dfll_pm_ops = {
> SET_RUNTIME_PM_OPS(tegra_dfll_runtime_suspend,
> tegra_dfll_runtime_resume, NULL)
> + SET_SYSTEM_SLEEP_PM_OPS(tegra_dfll_suspend, tegra_dfll_resume)
> };
>
> static struct platform_driver tegra124_dfll_fcpu_driver = {
>
^ permalink raw reply
* Re: [PATCH V6 09/21] clk: tegra: clk-super: Fix to enable PLLP branches to CPU
From: Dmitry Osipenko @ 2019-07-21 21:16 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-10-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch has a fix to enable PLLP branches to CPU before changing
> the CPU clusters clock source to PLLP for Gen5 Super clock.
>
> During system suspend entry and exit, CPU source will be switched
> to PLLP and this needs PLLP branches to be enabled to CPU prior to
> the switch.
>
> On system resume, warmboot code enables PLLP branches to CPU and
> powers up the CPU with PLLP clock source.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/clk/tegra/clk-super.c | 11 +++++++++++
> drivers/clk/tegra/clk-tegra-super-gen4.c | 4 ++--
> drivers/clk/tegra/clk.h | 4 ++++
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-super.c b/drivers/clk/tegra/clk-super.c
> index 39ef31b46df5..d73c587e4853 100644
> --- a/drivers/clk/tegra/clk-super.c
> +++ b/drivers/clk/tegra/clk-super.c
> @@ -28,6 +28,9 @@
> #define super_state_to_src_shift(m, s) ((m->width * s))
> #define super_state_to_src_mask(m) (((1 << m->width) - 1))
>
> +#define CCLK_SRC_PLLP_OUT0 4
> +#define CCLK_SRC_PLLP_OUT4 5
> +
> static u8 clk_super_get_parent(struct clk_hw *hw)
> {
> struct tegra_clk_super_mux *mux = to_clk_super_mux(hw);
> @@ -97,6 +100,14 @@ static int clk_super_set_parent(struct clk_hw *hw, u8 index)
> if (index == mux->div2_index)
> index = mux->pllx_index;
> }
> +
> + /*
> + * Enable PLLP branches to CPU before selecting PLLP source
> + */
> + if ((mux->flags & TEGRA_CPU_CLK) &&
> + ((index == CCLK_SRC_PLLP_OUT0) || (index == CCLK_SRC_PLLP_OUT4)))
> + tegra_clk_set_pllp_out_cpu(true);
Should somewhere here be tegra_clk_set_pllp_out_cpu(false) when
switching from PLLP?
> val &= ~((super_state_to_src_mask(mux)) << shift);
> val |= (index & (super_state_to_src_mask(mux))) << shift;
>
> diff --git a/drivers/clk/tegra/clk-tegra-super-gen4.c b/drivers/clk/tegra/clk-tegra-super-gen4.c
> index cdfe7c9697e1..cd208d0eca2a 100644
> --- a/drivers/clk/tegra/clk-tegra-super-gen4.c
> +++ b/drivers/clk/tegra/clk-tegra-super-gen4.c
> @@ -180,7 +180,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
> gen_info->num_cclk_g_parents,
> CLK_SET_RATE_PARENT,
> clk_base + CCLKG_BURST_POLICY,
> - 0, 4, 8, 0, NULL);
> + TEGRA_CPU_CLK, 4, 8, 0, NULL);
> } else {
> clk = tegra_clk_register_super_mux("cclk_g",
> gen_info->cclk_g_parents,
> @@ -201,7 +201,7 @@ static void __init tegra_super_clk_init(void __iomem *clk_base,
> gen_info->num_cclk_lp_parents,
> CLK_SET_RATE_PARENT,
> clk_base + CCLKLP_BURST_POLICY,
> - 0, 4, 8, 0, NULL);
> + TEGRA_CPU_CLK, 4, 8, 0, NULL);
> } else {
> clk = tegra_clk_register_super_mux("cclk_lp",
> gen_info->cclk_lp_parents,
> diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
> index ac6de3a0b91f..c357b49e49b0 100644
> --- a/drivers/clk/tegra/clk.h
> +++ b/drivers/clk/tegra/clk.h
> @@ -694,6 +694,9 @@ struct clk *tegra_clk_register_periph_data(void __iomem *clk_base,
> * Flags:
> * TEGRA_DIVIDER_2 - LP cluster has additional divider. This flag indicates
> * that this is LP cluster clock.
> + * TEGRA_CPU_CLK - This flag indicates this is CPU cluster clock. To use PLLP
> + * for CPU clock source, need to enable PLLP branches to CPU by setting the
> + * additional bit PLLP_OUT_CPU for gen5 super clock.
> */
> struct tegra_clk_super_mux {
> struct clk_hw hw;
> @@ -710,6 +713,7 @@ struct tegra_clk_super_mux {
> #define to_clk_super_mux(_hw) container_of(_hw, struct tegra_clk_super_mux, hw)
>
> #define TEGRA_DIVIDER_2 BIT(0)
> +#define TEGRA_CPU_CLK BIT(1)
I'd name this TEGRA210_CPU_CLK for clarity.
> extern const struct clk_ops tegra_clk_super_ops;
> struct clk *tegra_clk_register_super_mux(const char *name,
>
Will be better to move the tegra_clk_set_pllp_out_cpu() definition into
this patch, otherwise this looks inconsistent for reviewer.
^ permalink raw reply
* Re: [PATCH V6 12/21] cpufreq: tegra124: Add suspend and resume support
From: Dmitry Osipenko @ 2019-07-21 21:04 UTC (permalink / raw)
To: Sowjanya Komatineni, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <1563738060-30213-13-git-send-email-skomatineni@nvidia.com>
21.07.2019 22:40, Sowjanya Komatineni пишет:
> This patch adds suspend and resume pm ops for cpufreq driver.
>
> PLLP is the safe clock source for CPU during system suspend and
> resume as PLLP rate is below the CPU Fmax at Vmin.
>
> CPUFreq driver suspend switches the CPU clock source to PLLP and
> disables the DFLL clock.
>
> During system resume, warmboot code powers up the CPU with PLLP
> clock source. So CPUFreq driver resume enabled DFLL clock and
> switches CPU back to DFLL clock source.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/cpufreq/tegra124-cpufreq.c | 46 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/drivers/cpufreq/tegra124-cpufreq.c b/drivers/cpufreq/tegra124-cpufreq.c
> index 4f0c637b3b49..4eff63379b0f 100644
> --- a/drivers/cpufreq/tegra124-cpufreq.c
> +++ b/drivers/cpufreq/tegra124-cpufreq.c
> @@ -6,6 +6,7 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/clk.h>
> +#include <linux/cpufreq.h>> #include <linux/err.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> @@ -128,8 +129,53 @@ static int tegra124_cpufreq_probe(struct platform_device *pdev)
> return ret;
> }
>
> +static int tegra124_cpufreq_suspend(struct device *dev)
__maybe_unused
> +{
> + struct tegra124_cpufreq_priv *priv = dev_get_drvdata(dev);
> +
> + /*
> + * PLLP rate 408Mhz is below the CPU Fmax at Vmin and is safe to
> + * use during suspend and resume. So, switch the CPU clock source
> + * to PLLP and disable DFLL.
> + */
> + clk_set_parent(priv->cpu_clk, priv->pllp_clk);
Error check?
> +
> + /* disable DFLL clock */
> + clk_disable_unprepare(priv->dfll_clk);
> +
> + return 0;
> +}
> +
> +static int tegra124_cpufreq_resume(struct device *dev)
__maybe_unused
> +{
> + struct tegra124_cpufreq_priv *priv = dev_get_drvdata(dev);
> + int ret = 0;
> +
> + /*
> + * Warmboot code powers up the CPU with PLLP clock source.
> + * Enable DFLL clock and switch CPU clock source back to DFLL.
> + */
> + ret = clk_prepare_enable(priv->dfll_clk);
> + if (ret) {
> + dev_warn(dev, "failed to enable DFLL clock for CPU\n");
dev_err("..: %d\n", err);
> + clk_set_parent(priv->cpu_clk, priv->pllp_clk);
This is not needed because CPU is already on PLLP.
> + disable_cpufreq();
> + return ret;
> + }
> +
> + clk_set_parent(priv->cpu_clk, priv->dfll_clk);
Error check?
> +
> + return ret;
> +}
> +
> +static const struct dev_pm_ops tegra124_cpufreq_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(tegra124_cpufreq_suspend,
> + tegra124_cpufreq_resume)
> +};
> +
> static struct platform_driver tegra124_cpufreq_platdrv = {
> .driver.name = "cpufreq-tegra124",
> + .driver.pm = &tegra124_cpufreq_pm_ops,
> .probe = tegra124_cpufreq_probe,
> };
>
>
^ permalink raw reply
* Re: [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Marc Zyngier @ 2019-07-21 20:24 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: thierry.reding, jonathanh, tglx, jason, linus.walleij, stefan,
mark.rutland, pdeschrijver, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, josephl, talho, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree
In-Reply-To: <1563738060-30213-2-git-send-email-skomatineni@nvidia.com>
On Sun, 21 Jul 2019 12:40:40 -0700
Sowjanya Komatineni <skomatineni@nvidia.com> wrote:
> Tegra210 platforms use sc7 entry firmware to program Tegra LP0/SC7 entry
> sequence and sc7 entry firmware is run from COP/BPMP-Lite.
>
> So, COP/BPMP-Lite still need IRQ function to finish SC7 suspend sequence
> for Tegra210.
>
> This patch has fix for leaving the COP IRQ enabled for Tegra210 during
> interrupt controller suspend operation.
>
> Acked-by: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
Acked-by: Marc Zyngier <maz@kernel.org>
Please let me know how you want this to be merged (either via the
irqchip tree as a standalone patch, or as part of the series via
another tree).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* [PATCH V6 01/21] irqchip: tegra: Do not disable COP IRQ during suspend
From: Sowjanya Komatineni @ 2019-07-21 19:40 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree
In-Reply-To: <1563738060-30213-1-git-send-email-skomatineni@nvidia.com>
Tegra210 platforms use sc7 entry firmware to program Tegra LP0/SC7 entry
sequence and sc7 entry firmware is run from COP/BPMP-Lite.
So, COP/BPMP-Lite still need IRQ function to finish SC7 suspend sequence
for Tegra210.
This patch has fix for leaving the COP IRQ enabled for Tegra210 during
interrupt controller suspend operation.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/irqchip/irq-tegra.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
index e1f771c72fc4..851f88cef508 100644
--- a/drivers/irqchip/irq-tegra.c
+++ b/drivers/irqchip/irq-tegra.c
@@ -44,6 +44,7 @@ static unsigned int num_ictlrs;
struct tegra_ictlr_soc {
unsigned int num_ictlrs;
+ bool supports_sc7;
};
static const struct tegra_ictlr_soc tegra20_ictlr_soc = {
@@ -56,6 +57,7 @@ static const struct tegra_ictlr_soc tegra30_ictlr_soc = {
static const struct tegra_ictlr_soc tegra210_ictlr_soc = {
.num_ictlrs = 6,
+ .supports_sc7 = true,
};
static const struct of_device_id ictlr_matches[] = {
@@ -67,6 +69,7 @@ static const struct of_device_id ictlr_matches[] = {
struct tegra_ictlr_info {
void __iomem *base[TEGRA_MAX_NUM_ICTLRS];
+ const struct tegra_ictlr_soc *soc;
#ifdef CONFIG_PM_SLEEP
u32 cop_ier[TEGRA_MAX_NUM_ICTLRS];
u32 cop_iep[TEGRA_MAX_NUM_ICTLRS];
@@ -147,8 +150,20 @@ static int tegra_ictlr_suspend(void)
lic->cop_ier[i] = readl_relaxed(ictlr + ICTLR_COP_IER);
lic->cop_iep[i] = readl_relaxed(ictlr + ICTLR_COP_IEP_CLASS);
- /* Disable COP interrupts */
- writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
+ /*
+ * AVP/COP/BPMP-Lite is the Tegra boot processor.
+ *
+ * Tegra210 system suspend flow uses sc7entry firmware which
+ * is executed by COP/BPMP and it includes disabling COP IRQ,
+ * clamping CPU rail, turning off VDD_CPU, and preparing the
+ * system to go to SC7/LP0.
+ *
+ * COP/BPMP wakes up when COP IRQ is triggered and runs
+ * sc7entry-firmware. So need to keep COP interrupt enabled.
+ */
+ if (!lic->soc->supports_sc7)
+ /* Disable COP interrupts if SC7 is not supported */
+ writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
/* Disable CPU interrupts */
writel_relaxed(~0ul, ictlr + ICTLR_CPU_IER_CLR);
@@ -339,6 +354,7 @@ static int __init tegra_ictlr_init(struct device_node *node,
goto out_unmap;
}
+ lic->soc = soc;
tegra_ictlr_syscore_init();
pr_info("%pOF: %d interrupts forwarded to %pOF\n",
--
2.7.4
^ permalink raw reply related
* [PATCH V6 03/21] pinctrl: tegra210: Add Tegra210 pinctrl pm ops
From: Sowjanya Komatineni @ 2019-07-21 19:40 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree
In-Reply-To: <1563738060-30213-1-git-send-email-skomatineni@nvidia.com>
This patch adds suspend and resume functionality to Tegra210 pinctrl.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/pinctrl/tegra/pinctrl-tegra210.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
index 39ab6480a941..fc072a36deb3 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
@@ -1571,6 +1571,7 @@ static struct platform_driver tegra210_pinctrl_driver = {
.driver = {
.name = "tegra210-pinctrl",
.of_match_table = tegra210_pinctrl_of_match,
+ .pm = &tegra_pinctrl_pm,
},
.probe = tegra210_pinctrl_probe,
};
--
2.7.4
^ permalink raw reply related
* [PATCH V6 04/21] clk: tegra: Save and restore divider rate
From: Sowjanya Komatineni @ 2019-07-21 19:40 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree
In-Reply-To: <1563738060-30213-1-git-send-email-skomatineni@nvidia.com>
This patch implements context save and restore for clock divider.
During system suspend, core power goes off and looses the settings
of the Tegra CAR controller registers.
So during suspend entry the context of clock divider is saved and
on resume context is restored back for normal operation.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-divider.c | 23 +++++++++++++++++++++++
drivers/clk/tegra/clk.h | 2 ++
2 files changed, 25 insertions(+)
diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
index e76731fb7d69..ecb7ff9ce97e 100644
--- a/drivers/clk/tegra/clk-divider.c
+++ b/drivers/clk/tegra/clk-divider.c
@@ -109,10 +109,33 @@ static int clk_frac_div_set_rate(struct clk_hw *hw, unsigned long rate,
return 0;
}
+static int clk_divider_save_context(struct clk_hw *hw)
+{
+ struct tegra_clk_frac_div *divider = to_clk_frac_div(hw);
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ unsigned long parent_rate = clk_hw_get_rate(parent);
+
+ divider->rate = clk_frac_div_recalc_rate(hw, parent_rate);
+
+ return 0;
+}
+
+static void clk_divider_restore_context(struct clk_hw *hw)
+{
+ struct tegra_clk_frac_div *divider = to_clk_frac_div(hw);
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ unsigned long parent_rate = clk_hw_get_rate(parent);
+
+ if (clk_frac_div_set_rate(hw, divider->rate, parent_rate) < 0)
+ WARN_ON(1);
+}
+
const struct clk_ops tegra_clk_frac_div_ops = {
.recalc_rate = clk_frac_div_recalc_rate,
.set_rate = clk_frac_div_set_rate,
.round_rate = clk_frac_div_round_rate,
+ .save_context = clk_divider_save_context,
+ .restore_context = clk_divider_restore_context,
};
struct clk *tegra_clk_register_divider(const char *name,
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 905bf1096558..83623f5f55f3 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -42,6 +42,7 @@ struct clk *tegra_clk_register_sync_source(const char *name,
* @width: width of the divider bit field
* @frac_width: width of the fractional bit field
* @lock: register lock
+ * @rate: rate during suspend and resume
*
* Flags:
* TEGRA_DIVIDER_ROUND_UP - This flags indicates to round up the divider value.
@@ -62,6 +63,7 @@ struct tegra_clk_frac_div {
u8 width;
u8 frac_width;
spinlock_t *lock;
+ unsigned long rate;
};
#define to_clk_frac_div(_hw) container_of(_hw, struct tegra_clk_frac_div, hw)
--
2.7.4
^ permalink raw reply related
* [PATCH V6 00/21] SC7 entry and exit support for Tegra210
From: Sowjanya Komatineni @ 2019-07-21 19:40 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree
This patch series includes Tegra210 deepsleep support with RTC alarm
wake event.
This series also includes save and restore of PLLs, clocks, OSC contexts
for deepsleep exit to normal operation.
This patch series doesn't support 100% suspend/resume to allow fully
functional state upon resume and we are working on some more drivers suspend
and resume implementations.
[V6]: Changes between V5 & V6 are
- V5 feedback fixes
- DFLL suspend and resume moved to DFLL clock driver
- Add suspend and resume support for CPUFreq driver to explicitly
switch source to safe source of PLLP and disable DFLL clock.
- Fix to super clock driver to enable PLLP branch to CPU before
source switch to PLLP.
- Added save and restore support for super clock driver.
[V5]: Changes between V4 & V5 are
- V4 feedback fixes
[V4]: Changes between V3 & V4 are
- V3 feedback fixes
- Removed park bits clear for EMMC pads in pinctrl-tegra driver
function tegra_pinctrl_clear_parked_bits as based on V3 feedback
parked_bit is updated to parked_bitmask to use with DRV_PINGROUP
as well and thierry posted patch series for this.
- Implemented all peripheral clocks save and restore through their
corresponding clk_ops save_context and restore_context and removed
all direct registers store and restore in clk-tegra210 driver.
- Created separate patch for fence_delay update during PLLU init based
on V3 feedback.
- Added more comments in tegra210_clk_resume regarding dfll restore
sequence and its dependency on peripheral clocks restore.
[V3]: Changes between V2 & V3 are
- V2 feedback fixes
- GPIO restore should happen prior to Pinctrl restore to prevent
glitch on GPIO lines. So using resume_noirq for gpio tegra to allow
gpio resume prior to pinctrl resume.
- Implemented save_context and restore_context callbacks for clock
plls, pll outs and dividers in corresponding drivers.
Note: Peripheral clocks and clock enable and reset need to be in
Tegra210 clock suspend/resume as they need to be in proper sequence
w.r.t DFLL resume for restoring CPU clock.
- Removed gpio-tegra changes for hierarchical support to have PMC as
parent to GPIOs for GPIO wake event support. Thierry is working on
gpiolib for some cleanup before adding hierarchical support. So
holding on to GPIO wake support for now.
[V2] : V1 feedback fixes
Patch 0002: This version still using syscore. Thierry suggest not to
use syscore and waiting on suggestion from Linux Walleij for any better
way of storing current state of pins before suspend entry and restoring
them on resume at very early stage. So left this the same way as V1 and
will address once I get more feedback on this.
Also need to findout and implement proper way of forcing resume order
between pinctrl and gpio driver.
[V1]: Tegra210 SC7 entry and exit thru RTC wake and Power button GPIO wake
using hierarchical IRQ with PMC as parent to GPIO.
Sowjanya Komatineni (21):
irqchip: tegra: Do not disable COP IRQ during suspend
pinctrl: tegra: Add suspend and resume support
pinctrl: tegra210: Add Tegra210 pinctrl pm ops
clk: tegra: Save and restore divider rate
clk: tegra: pllout: Save and restore pllout context
clk: tegra: pll: Save and restore pll context
clk: tegra: Support for OSC context save and restore
clk: tegra: clk-periph: Add save and restore support
clk: tegra: clk-super: Fix to enable PLLP branches to CPU
clk: tegra: clk-super: Add save and restore support
clk: tegra: clk-dfll: Add suspend and resume support
cpufreq: tegra124: Add suspend and resume support
clk: tegra210: Use fence_udelay during PLLU init
clk: tegra210: Add suspend and resume support
soc/tegra: pmc: Allow to support more tegras wake
soc/tegra: pmc: Add pmc wake support for tegra210
arm64: tegra: Enable wake from deep sleep on RTC alarm.
soc/tegra: pmc: Configure core power request polarity
soc/tegra: pmc: Configure deep sleep control settings
arm64: dts: tegra210-p2180: Jetson TX1 SC7 timings
arm64: dts: tegra210-p3450: Jetson nano SC7 timings
arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi | 7 +
arch/arm64/boot/dts/nvidia/tegra210-p3450-0000.dts | 7 +
arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 +-
drivers/clk/tegra/clk-dfll.c | 44 +++++++
drivers/clk/tegra/clk-dfll.h | 2 +
drivers/clk/tegra/clk-divider.c | 23 ++++
drivers/clk/tegra/clk-periph-fixed.c | 33 +++++
drivers/clk/tegra/clk-periph-gate.c | 34 +++++
drivers/clk/tegra/clk-periph.c | 43 +++++++
drivers/clk/tegra/clk-pll-out.c | 28 ++++
drivers/clk/tegra/clk-pll.c | 121 +++++++++++++-----
drivers/clk/tegra/clk-sdmmc-mux.c | 30 +++++
drivers/clk/tegra/clk-super.c | 51 ++++++++
drivers/clk/tegra/clk-tegra-fixed.c | 15 +++
drivers/clk/tegra/clk-tegra-super-gen4.c | 4 +-
drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 1 +
drivers/clk/tegra/clk-tegra210.c | 81 ++++++++++--
drivers/clk/tegra/clk.c | 14 ++
drivers/clk/tegra/clk.h | 36 +++++-
drivers/cpufreq/tegra124-cpufreq.c | 46 +++++++
drivers/irqchip/irq-tegra.c | 20 ++-
drivers/pinctrl/tegra/pinctrl-tegra.c | 59 +++++++++
drivers/pinctrl/tegra/pinctrl-tegra.h | 3 +
drivers/pinctrl/tegra/pinctrl-tegra210.c | 1 +
drivers/soc/tegra/pmc.c | 142 ++++++++++++++++++++-
25 files changed, 798 insertions(+), 52 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH V6 02/21] pinctrl: tegra: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-07-21 19:40 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree
In-Reply-To: <1563738060-30213-1-git-send-email-skomatineni@nvidia.com>
This patch adds support for Tegra pinctrl driver suspend and resume.
During suspend, context of all pinctrl registers are stored and
on resume they are all restored to have all the pinmux and pad
configuration for normal operation.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/pinctrl/tegra/pinctrl-tegra.c | 59 +++++++++++++++++++++++++++++++++++
drivers/pinctrl/tegra/pinctrl-tegra.h | 3 ++
2 files changed, 62 insertions(+)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 186ef98e7b2b..e3a237534281 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -631,6 +631,58 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
}
}
+static size_t tegra_pinctrl_get_bank_size(struct device *dev,
+ unsigned int bank_id)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct resource *res;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, bank_id);
+
+ return resource_size(res) / 4;
+}
+
+static int tegra_pinctrl_suspend(struct device *dev)
+{
+ struct tegra_pmx *pmx = dev_get_drvdata(dev);
+ u32 *backup_regs = pmx->backup_regs;
+ u32 *regs;
+ size_t bank_size;
+ unsigned int i, k;
+
+ for (i = 0; i < pmx->nbanks; i++) {
+ bank_size = tegra_pinctrl_get_bank_size(dev, i);
+ regs = pmx->regs[i];
+ for (k = 0; k < bank_size; k++)
+ *backup_regs++ = readl_relaxed(regs++);
+ }
+
+ return pinctrl_force_sleep(pmx->pctl);
+}
+
+static int tegra_pinctrl_resume(struct device *dev)
+{
+ struct tegra_pmx *pmx = dev_get_drvdata(dev);
+ u32 *backup_regs = pmx->backup_regs;
+ u32 *regs;
+ size_t bank_size;
+ unsigned int i, k;
+
+ for (i = 0; i < pmx->nbanks; i++) {
+ bank_size = tegra_pinctrl_get_bank_size(dev, i);
+ regs = pmx->regs[i];
+ for (k = 0; k < bank_size; k++)
+ writel_relaxed(*backup_regs++, regs++);
+ }
+
+ return 0;
+}
+
+const struct dev_pm_ops tegra_pinctrl_pm = {
+ .suspend = &tegra_pinctrl_suspend,
+ .resume = &tegra_pinctrl_resume
+};
+
static bool gpio_node_has_range(const char *compatible)
{
struct device_node *np;
@@ -655,6 +707,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
int i;
const char **group_pins;
int fn, gn, gfn;
+ unsigned long backup_regs_size = 0;
pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
if (!pmx)
@@ -707,6 +760,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
if (!res)
break;
+ backup_regs_size += resource_size(res);
}
pmx->nbanks = i;
@@ -715,6 +769,11 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
if (!pmx->regs)
return -ENOMEM;
+ pmx->backup_regs = devm_kzalloc(&pdev->dev, backup_regs_size,
+ GFP_KERNEL);
+ if (!pmx->backup_regs)
+ return -ENOMEM;
+
for (i = 0; i < pmx->nbanks; i++) {
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
pmx->regs[i] = devm_ioremap_resource(&pdev->dev, res);
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
index 105309774079..0fc82eea9cf1 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.h
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
@@ -17,6 +17,7 @@ struct tegra_pmx {
int nbanks;
void __iomem **regs;
+ u32 *backup_regs;
};
enum tegra_pinconf_param {
@@ -193,6 +194,8 @@ struct tegra_pinctrl_soc_data {
bool drvtype_in_mux;
};
+extern const struct dev_pm_ops tegra_pinctrl_pm;
+
int tegra_pinctrl_probe(struct platform_device *pdev,
const struct tegra_pinctrl_soc_data *soc_data);
#endif
--
2.7.4
^ permalink raw reply related
* [PATCH V6 06/21] clk: tegra: pll: Save and restore pll context
From: Sowjanya Komatineni @ 2019-07-21 19:40 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree
In-Reply-To: <1563738060-30213-1-git-send-email-skomatineni@nvidia.com>
This patch implements save and restore of PLL context.
During system suspend, core power goes off and looses the settings
of the Tegra CAR controller registers.
So during suspend entry pll rate is stored and on resume it is
restored back along with its state.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-pll.c | 121 ++++++++++++++++++++++++++++-----------
drivers/clk/tegra/clk-tegra210.c | 2 +-
drivers/clk/tegra/clk.h | 10 +++-
3 files changed, 99 insertions(+), 34 deletions(-)
diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 1583f5fc992f..f136964e6c44 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1008,6 +1008,59 @@ static unsigned long clk_plle_recalc_rate(struct clk_hw *hw,
return rate;
}
+void tegra_clk_sync_state_pll(struct clk_hw *hw)
+{
+ if (!__clk_get_enable_count(hw->clk))
+ clk_pll_disable(hw);
+ else
+ clk_pll_enable(hw);
+}
+
+static int tegra_clk_pll_save_context(struct clk_hw *hw)
+{
+ struct tegra_clk_pll *pll = to_clk_pll(hw);
+ u32 val = 0;
+
+ pll->rate = clk_hw_get_rate(hw);
+
+ if (pll->params->flags & TEGRA_PLLMB)
+ val = pll_readl_base(pll);
+ else if (pll->params->flags & TEGRA_PLLRE)
+ val = pll_readl_base(pll) & divp_mask_shifted(pll);
+
+ pll->pllbase_ctx = val;
+
+ return 0;
+}
+
+static void tegra_clk_pll_restore_context(struct clk_hw *hw)
+{
+ struct tegra_clk_pll *pll = to_clk_pll(hw);
+ struct clk_hw *parent = clk_hw_get_parent(hw);
+ unsigned long parent_rate = clk_hw_get_rate(parent);
+ u32 val;
+
+ if (clk_pll_is_enabled(hw))
+ return;
+
+ if (pll->params->flags & TEGRA_PLLMB) {
+ pll_writel_base(pll->pllbase_ctx, pll);
+ } else if (pll->params->flags & TEGRA_PLLRE) {
+ val = pll_readl_base(pll);
+ val &= ~(divp_mask_shifted(pll));
+ pll_writel_base(pll->pllbase_ctx | val, pll);
+ }
+
+ if (pll->params->set_defaults)
+ pll->params->set_defaults(pll);
+
+ clk_pll_set_rate(hw, pll->rate, parent_rate);
+
+ /* do not sync pllx state here. pllx is sync'd after dfll resume */
+ if (!(pll->params->flags & TEGRA_PLLX))
+ tegra_clk_sync_state_pll(hw);
+}
+
const struct clk_ops tegra_clk_pll_ops = {
.is_enabled = clk_pll_is_enabled,
.enable = clk_pll_enable,
@@ -1015,6 +1068,8 @@ const struct clk_ops tegra_clk_pll_ops = {
.recalc_rate = clk_pll_recalc_rate,
.round_rate = clk_pll_round_rate,
.set_rate = clk_pll_set_rate,
+ .save_context = tegra_clk_pll_save_context,
+ .restore_context = tegra_clk_pll_restore_context,
};
const struct clk_ops tegra_clk_plle_ops = {
@@ -1802,6 +1857,27 @@ static int clk_pllu_tegra114_enable(struct clk_hw *hw)
return ret;
}
+
+static void _clk_plle_tegra_init_parent(struct tegra_clk_pll *pll)
+{
+ u32 val, val_aux;
+
+ /* ensure parent is set to pll_ref */
+ val = pll_readl_base(pll);
+ val_aux = pll_readl(pll->params->aux_reg, pll);
+
+ if (val & PLL_BASE_ENABLE) {
+ if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
+ (val_aux & PLLE_AUX_PLLP_SEL))
+ WARN(1, "pll_e enabled with unsupported parent %s\n",
+ (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
+ "pll_re_vco");
+ } else {
+ val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
+ pll_writel(val_aux, pll->params->aux_reg, pll);
+ fence_udelay(1, pll->clk_base);
+ }
+}
#endif
static struct tegra_clk_pll *_tegra_init_pll(void __iomem *clk_base,
@@ -2214,27 +2290,12 @@ struct clk *tegra_clk_register_plle_tegra114(const char *name,
{
struct tegra_clk_pll *pll;
struct clk *clk;
- u32 val, val_aux;
pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
if (IS_ERR(pll))
return ERR_CAST(pll);
- /* ensure parent is set to pll_re_vco */
-
- val = pll_readl_base(pll);
- val_aux = pll_readl(pll_params->aux_reg, pll);
-
- if (val & PLL_BASE_ENABLE) {
- if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
- (val_aux & PLLE_AUX_PLLP_SEL))
- WARN(1, "pll_e enabled with unsupported parent %s\n",
- (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
- "pll_re_vco");
- } else {
- val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
- pll_writel(val_aux, pll_params->aux_reg, pll);
- }
+ _clk_plle_tegra_init_parent(pll);
clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
&tegra_clk_plle_tegra114_ops);
@@ -2276,6 +2337,8 @@ static const struct clk_ops tegra_clk_pllss_ops = {
.recalc_rate = clk_pll_recalc_rate,
.round_rate = clk_pll_ramp_round_rate,
.set_rate = clk_pllxc_set_rate,
+ .save_context = tegra_clk_pll_save_context,
+ .restore_context = tegra_clk_pll_restore_context,
};
struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,
@@ -2375,6 +2438,7 @@ struct clk *tegra_clk_register_pllre_tegra210(const char *name,
pll_params->vco_min = pll_params->adjust_vco(pll_params,
parent_rate);
+ pll_params->flags |= TEGRA_PLLRE;
pll = _tegra_init_pll(clk_base, pmc, pll_params, lock);
if (IS_ERR(pll))
return ERR_CAST(pll);
@@ -2520,11 +2584,19 @@ static void clk_plle_tegra210_disable(struct clk_hw *hw)
spin_unlock_irqrestore(pll->lock, flags);
}
+static void tegra_clk_plle_t210_restore_context(struct clk_hw *hw)
+{
+ struct tegra_clk_pll *pll = to_clk_pll(hw);
+
+ _clk_plle_tegra_init_parent(pll);
+}
+
static const struct clk_ops tegra_clk_plle_tegra210_ops = {
.is_enabled = clk_plle_tegra210_is_enabled,
.enable = clk_plle_tegra210_enable,
.disable = clk_plle_tegra210_disable,
.recalc_rate = clk_pll_recalc_rate,
+ .restore_context = tegra_clk_plle_t210_restore_context,
};
struct clk *tegra_clk_register_plle_tegra210(const char *name,
@@ -2535,27 +2607,12 @@ struct clk *tegra_clk_register_plle_tegra210(const char *name,
{
struct tegra_clk_pll *pll;
struct clk *clk;
- u32 val, val_aux;
pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
if (IS_ERR(pll))
return ERR_CAST(pll);
- /* ensure parent is set to pll_re_vco */
-
- val = pll_readl_base(pll);
- val_aux = pll_readl(pll_params->aux_reg, pll);
-
- if (val & PLLE_BASE_ENABLE) {
- if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
- (val_aux & PLLE_AUX_PLLP_SEL))
- WARN(1, "pll_e enabled with unsupported parent %s\n",
- (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
- "pll_re_vco");
- } else {
- val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
- pll_writel(val_aux, pll_params->aux_reg, pll);
- }
+ _clk_plle_tegra_init_parent(pll);
clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
&tegra_clk_plle_tegra210_ops);
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index 4721ee030d1c..58397f93166c 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -1602,7 +1602,7 @@ static struct tegra_clk_pll_params pll_x_params = {
.pdiv_tohw = pll_qlin_pdiv_to_hw,
.div_nmp = &pllx_nmp,
.freq_table = pll_x_freq_table,
- .flags = TEGRA_PLL_USE_LOCK | TEGRA_PLL_HAS_LOCK_ENABLE,
+ .flags = TEGRA_PLL_USE_LOCK | TEGRA_PLL_HAS_LOCK_ENABLE | TEGRA_PLLX,
.dyn_ramp = tegra210_pllx_dyn_ramp,
.set_defaults = tegra210_pllx_set_defaults,
.calc_rate = tegra210_pll_fixed_mdiv_cfg,
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index fb29a8c27873..8532f5150091 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -235,6 +235,8 @@ struct tegra_clk_pll;
* TEGRA_PLLMB - PLLMB has should be treated similar to PLLM. This
* flag indicated that it is PLLMB.
* TEGRA_PLL_VCO_OUT - Used to indicate that the PLL has a VCO output
+ * TEGRA_PLLRE - Used to indicate that it is PLLRE.
+ * TEGRA_PLLX - Used to indicate that it is PLLX.
*/
struct tegra_clk_pll_params {
unsigned long input_min;
@@ -301,6 +303,8 @@ struct tegra_clk_pll_params {
#define TEGRA_MDIV_NEW BIT(11)
#define TEGRA_PLLMB BIT(12)
#define TEGRA_PLL_VCO_OUT BIT(13)
+#define TEGRA_PLLRE BIT(14)
+#define TEGRA_PLLX BIT(15)
/**
* struct tegra_clk_pll - Tegra PLL clock
@@ -310,6 +314,8 @@ struct tegra_clk_pll_params {
* @pmc: address of PMC, required to read override bits
* @lock: register lock
* @params: PLL parameters
+ * @rate: rate during system suspend and resume
+ * @pllbase_ctx: pll base register value during suspend and resume
*/
struct tegra_clk_pll {
struct clk_hw hw;
@@ -317,6 +323,8 @@ struct tegra_clk_pll {
void __iomem *pmc;
spinlock_t *lock;
struct tegra_clk_pll_params *params;
+ unsigned long rate;
+ u32 pllbase_ctx;
};
#define to_clk_pll(_hw) container_of(_hw, struct tegra_clk_pll, hw)
@@ -840,7 +848,7 @@ u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
u8 frac_width, u8 flags);
-
+void tegra_clk_sync_state_pll(struct clk_hw *hw);
/* Combined read fence with delay */
#define fence_udelay(delay, reg) \
--
2.7.4
^ permalink raw reply related
* [PATCH V6 05/21] clk: tegra: pllout: Save and restore pllout context
From: Sowjanya Komatineni @ 2019-07-21 19:40 UTC (permalink / raw)
To: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, skomatineni, linux-tegra, linux-kernel,
mperttunen, spatra, robh+dt, digetx, devicetree
In-Reply-To: <1563738060-30213-1-git-send-email-skomatineni@nvidia.com>
This patch implements save and restore of pllout context.
During system suspend, core power goes off and looses the settings
of the Tegra CAR controller registers.
So during suspend entry the state of pllout is saved and on resume
it is restored back to have pllout in same state as before suspend.
pllout rate is saved and restore in clock divider so it will be at
same rate as before suspend when pllout state is restored.
Acked-by: Thierry Reding <treding@nvidia.com>
Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
drivers/clk/tegra/clk-pll-out.c | 28 ++++++++++++++++++++++++++++
drivers/clk/tegra/clk-tegra210.c | 3 ++-
drivers/clk/tegra/clk.h | 9 +++++++++
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-pll-out.c b/drivers/clk/tegra/clk-pll-out.c
index 35f2bf00e1e6..8f26a7e3e579 100644
--- a/drivers/clk/tegra/clk-pll-out.c
+++ b/drivers/clk/tegra/clk-pll-out.c
@@ -69,10 +69,38 @@ static void clk_pll_out_disable(struct clk_hw *hw)
spin_unlock_irqrestore(pll_out->lock, flags);
}
+static int tegra_clk_pll_out_save_context(struct clk_hw *hw)
+{
+ struct tegra_clk_pll_out *pll_out = to_clk_pll_out(hw);
+
+ if (pll_out->flags & TEGRA_PLLRE_OUT)
+ pll_out->pllout_ctx = readl_relaxed(pll_out->reg);
+ else
+ pll_out->pllout_ctx = clk_hw_get_rate(hw);
+
+ return 0;
+}
+
+static void tegra_clk_pll_out_restore_context(struct clk_hw *hw)
+{
+ struct tegra_clk_pll_out *pll_out = to_clk_pll_out(hw);
+
+ if (pll_out->flags & TEGRA_PLLRE_OUT) {
+ writel_relaxed(pll_out->pllout_ctx, pll_out->reg);
+ } else {
+ if (!__clk_get_enable_count(hw->clk))
+ clk_pll_out_disable(hw);
+ else
+ clk_pll_out_enable(hw);
+ }
+}
+
const struct clk_ops tegra_clk_pll_out_ops = {
.is_enabled = clk_pll_out_is_enabled,
.enable = clk_pll_out_enable,
.disable = clk_pll_out_disable,
+ .save_context = tegra_clk_pll_out_save_context,
+ .restore_context = tegra_clk_pll_out_restore_context,
};
struct clk *tegra_clk_register_pll_out(const char *name,
diff --git a/drivers/clk/tegra/clk-tegra210.c b/drivers/clk/tegra/clk-tegra210.c
index df172d5772d7..4721ee030d1c 100644
--- a/drivers/clk/tegra/clk-tegra210.c
+++ b/drivers/clk/tegra/clk-tegra210.c
@@ -3200,7 +3200,8 @@ static void __init tegra210_pll_init(void __iomem *clk_base,
8, 8, 1, NULL);
clk = tegra_clk_register_pll_out("pll_re_out1", "pll_re_out1_div",
clk_base + PLLRE_OUT1, 1, 0,
- CLK_SET_RATE_PARENT, 0, NULL);
+ CLK_SET_RATE_PARENT, TEGRA_PLLRE_OUT,
+ NULL);
clks[TEGRA210_CLK_PLL_RE_OUT1] = clk;
/* PLLE */
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 83623f5f55f3..fb29a8c27873 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -439,6 +439,12 @@ struct clk *tegra_clk_register_pllu_tegra210(const char *name,
* @rst_bit_idx: bit to reset PLL divider
* @lock: register lock
* @flags: hardware-specific flags
+ * @pllout_ctx: pllout context to save and restore during suspend
+ * and resume
+ *
+ * Flags:
+ * TEGRA_PLLRE_OUT - This flag indicates that it is PLLRE_OUT and is used to
+ * identify PLLRE_OUT during clk_pll_out save and restore.
*/
struct tegra_clk_pll_out {
struct clk_hw hw;
@@ -447,8 +453,11 @@ struct tegra_clk_pll_out {
u8 rst_bit_idx;
spinlock_t *lock;
u8 flags;
+ unsigned int pllout_ctx;
};
+#define TEGRA_PLLRE_OUT BIT(0)
+
#define to_clk_pll_out(_hw) container_of(_hw, struct tegra_clk_pll_out, hw)
extern const struct clk_ops tegra_clk_pll_out_ops;
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox