[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eff0e0f5-a94d-cca5-3558-247d1d90d28d@nvidia.com>
Date: Wed, 20 Nov 2019 11:09:08 -0800
From: Sowjanya Komatineni <skomatineni@...dia.com>
To: Dmitry Osipenko <digetx@...il.com>, <thierry.reding@...il.com>,
<jonathanh@...dia.com>, <mperttunen@...dia.com>,
<gregkh@...uxfoundation.org>, <sboyd@...nel.org>,
<tglx@...utronix.de>, <robh+dt@...nel.org>, <mark.rutland@....com>
CC: <allison@...utok.net>, <pdeschrijver@...dia.com>,
<pgaikwad@...dia.com>, <mturquette@...libre.com>,
<horms+renesas@...ge.net.au>, <Jisheng.Zhang@...aptics.com>,
<krzk@...nel.org>, <arnd@...db.de>, <spujar@...dia.com>,
<josephl@...dia.com>, <vidyas@...dia.com>,
<daniel.lezcano@...aro.org>, <mmaddireddy@...dia.com>,
<markz@...dia.com>, <devicetree@...r.kernel.org>,
<linux-clk@...r.kernel.org>, <linux-tegra@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 04/17] soc: tegra: Add Tegra PMC clock registrations
into PMC driver
On 11/20/19 9:46 AM, Dmitry Osipenko wrote:
> 19.11.2019 23:08, Sowjanya Komatineni пишет:
>> On 11/19/19 11:33 AM, Dmitry Osipenko wrote:
>>> 19.11.2019 09:50, Sowjanya Komatineni пишет:
>>>> Tegra PMC has clk_out_1, clk_out_2, clk_out_3 with mux and gate for
>>>> each of these clocks.
>>>>
>>>> Currently these PMC clocks are registered by Tegra clock driver using
>>>> clk_register_mux and clk_register_gate by passing PMC base address
>>>> and register offsets and PMC programming for these clocks happens
>>>> through direct PMC access by the clock driver.
>>>>
>>>> With this, when PMC is in secure mode any direct PMC access from the
>>>> non-secure world does not go through and these clocks will not be
>>>> functional.
>>>>
>>>> This patch adds these clocks registration with PMC as a clock provider
>>>> for these clocks. clk_ops callback implementations for these clocks
>>>> uses tegra_pmc_readl and tegra_pmc_writel which supports PMC programming
>>>> in secure mode and non-secure mode.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@...dia.com>
>>>> ---
>>>> drivers/soc/tegra/pmc.c | 330
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 330 insertions(+)
>>>>
>>>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>>>> index 7a5aab0b993b..790a6619ba32 100644
>>>> --- a/drivers/soc/tegra/pmc.c
>>>> +++ b/drivers/soc/tegra/pmc.c
>>>> @@ -13,6 +13,9 @@
>>>> #include <linux/arm-smccc.h>
>>>> #include <linux/clk.h>
>>>> +#include <linux/clk-provider.h>
>>>> +#include <linux/clkdev.h>
>>>> +#include <linux/clk/clk-conf.h>
>>>> #include <linux/clk/tegra.h>
>>>> #include <linux/debugfs.h>
>>>> #include <linux/delay.h>
>>>> @@ -48,6 +51,7 @@
>>>> #include <dt-bindings/pinctrl/pinctrl-tegra-io-pad.h>
>>>> #include <dt-bindings/gpio/tegra186-gpio.h>
>>>> #include <dt-bindings/gpio/tegra194-gpio.h>
>>>> +#include <dt-bindings/soc/tegra-pmc.h>
>>>> #define PMC_CNTRL 0x0
>>>> #define PMC_CNTRL_INTR_POLARITY BIT(17) /* inverts INTR
>>>> polarity */
>>>> @@ -108,6 +112,7 @@
>>>> #define PMC_WAKE2_STATUS 0x168
>>>> #define PMC_SW_WAKE2_STATUS 0x16c
>>>> +#define PMC_CLK_OUT_CNTRL 0x1a8
>>>> #define PMC_SATA_PWRGT 0x1ac
>>>> #define PMC_SATA_PWRGT_PLLE_IDDQ_VALUE BIT(5)
>>>> #define PMC_SATA_PWRGT_PLLE_IDDQ_SWCTL BIT(4)
>>>> @@ -170,6 +175,78 @@
>>>> #define TEGRA_SMC_PMC_READ 0xaa
>>>> #define TEGRA_SMC_PMC_WRITE 0xbb
>>>> +struct pmc_clk_mux {
>>>> + struct clk_hw hw;
>>>> + unsigned long offs;
>>>> + u32 mask;
>>>> + u32 shift;
>>>> + /* register lock */
>>>> + spinlock_t *lock;
>>>> +};
>>>> +
>>>> +#define to_pmc_clk_mux(_hw) container_of(_hw, struct pmc_clk_mux, hw)
>>>> +
>>>> +struct pmc_clk_gate {
>>>> + struct clk_hw hw;
>>>> + unsigned long offs;
>>>> + u32 shift;
>>>> + /* register lock */
>>>> + spinlock_t *lock;
> Why clk_out_lock is needed at all? CCLK framework already takes care of
> the clock's locking and then nothing else in PMC code uses that lock to
> avoid races, thus that spinlock doesn't do anything useful and should be
> removed from both mux and gate.
Will remove spinlock in next version.
>>>> +};
>>>> +
>>>> +#define to_pmc_clk_gate(_hw) container_of(_hw, struct pmc_clk_gate, hw)
>>>> +
>>>> +struct pmc_clk_init_data {
>>>> + char *mux_name;
>>>> + char *gate_name;
>>>> + const char **parents;
>>>> + int num_parents;
>>>> + int mux_id;
>>>> + int gate_id;
>>>> + char *dev_name;
>>>> + u8 mux_shift;
>>>> + u8 gate_shift;
>>>> + u8 init_parent;
>>>> + int init_state;
>>>> + struct pmc_clk_mux mux;
>>>> + struct pmc_clk_gate gate;
>>>> +};
>>>> +
>>>> +#define PMC_CLK(_num, _mux_shift, _gate_shift, _init_parent,
>>>> _init_state)\
>>>> + {\
>>>> + .mux_name = "clk_out_" #_num "_mux",\
>>>> + .gate_name = "clk_out_" #_num,\
>>>> + .parents = clk_out ##_num ##_parents,\
>>>> + .num_parents = ARRAY_SIZE(clk_out ##_num ##_parents),\
>>>> + .mux_id = TEGRA_PMC_CLK_OUT_ ##_num ##_MUX,\
>>>> + .gate_id = TEGRA_PMC_CLK_OUT_ ##_num,\
>>>> + .dev_name = "extern" #_num,\
>>>> + .mux_shift = _mux_shift,\
>>>> + .gate_shift = _gate_shift,\
>>>> + .init_parent = _init_parent,\
>>>> + .init_state = _init_state,\
>>>> + }
>>>> +
>>>> +static DEFINE_SPINLOCK(clk_out_lock);
>>>> +
>>>> +static const char *clk_out1_parents[] = { "clk_m", "clk_m_div2",
>>>> + "clk_m_div4", "extern1",
>>>> +};
>>>> +
>>>> +static const char *clk_out2_parents[] = { "clk_m", "clk_m_div2",
>>>> + "clk_m_div4", "extern2",
>>>> +};
>>>> +
>>>> +static const char *clk_out3_parents[] = { "clk_m", "clk_m_div2",
>>>> + "clk_m_div4", "extern3",
>>>> +};
>>> Why these are unused?
>> They are used in PMC_CLK macro
> Looks like it will better to define those three structs directly,
> without the PMC_CLK macro.
>
> [snip]
ok, will define structs directly in next version.
Powered by blists - more mailing lists