* Re: [PATCH] pinctrl: rzn1: Add of_node_put() before return
From: Geert Uytterhoeven @ 2019-08-05 7:18 UTC (permalink / raw)
To: Nishka Dasgupta
Cc: Geert Uytterhoeven, Linus Walleij, Linux-Renesas,
open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
Phil Edworthy
In-Reply-To: <20190804154029.2749-1-nishkadg.linux@gmail.com>
CC Phil
On Sun, Aug 4, 2019 at 5:40 PM Nishka Dasgupta <nishkadg.linux@gmail.com> wrote:
> Each iteration of for_each_child_of_node puts the previous node, but in
> the case of a return from the middle of the loop, there is no put, thus
> causing a memory leak. Hence add an of_node_put before the return in
> three places.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in sh-pfc-for-v5.4.
> ---
> drivers/pinctrl/pinctrl-rzn1.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-rzn1.c b/drivers/pinctrl/pinctrl-rzn1.c
> index cc0e5aa9128a..0f6f8a10a53a 100644
> --- a/drivers/pinctrl/pinctrl-rzn1.c
> +++ b/drivers/pinctrl/pinctrl-rzn1.c
> @@ -412,8 +412,10 @@ static int rzn1_dt_node_to_map(struct pinctrl_dev *pctldev,
>
> for_each_child_of_node(np, child) {
> ret = rzn1_dt_node_to_map_one(pctldev, child, map, num_maps);
> - if (ret < 0)
> + if (ret < 0) {
> + of_node_put(child);
> return ret;
> + }
> }
>
> return 0;
> @@ -792,8 +794,10 @@ static int rzn1_pinctrl_parse_functions(struct device_node *np,
> grp = &ipctl->groups[ipctl->ngroups];
> grp->func = func->name;
> ret = rzn1_pinctrl_parse_groups(child, grp, ipctl);
> - if (ret < 0)
> + if (ret < 0) {
> + of_node_put(child);
> return ret;
> + }
> i++;
> ipctl->ngroups++;
> }
> @@ -838,8 +842,10 @@ static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
>
> for_each_child_of_node(np, child) {
> ret = rzn1_pinctrl_parse_functions(child, ipctl, i++);
> - if (ret < 0)
> + if (ret < 0) {
> + of_node_put(child);
> return ret;
> + }
> }
>
> return 0;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* [PATCH] arm64: dts: ls1028a: fix gpio nodes
From: Hui Song @ 2019-08-05 6:57 UTC (permalink / raw)
To: Shawn Guo, Li Yang, Rob Herring, Mark Rutland, Linus Walleij,
Bartosz Golaszewski
Cc: linux-arm-kernel, devicetree, linux-kernel, linux-gpio, Song Hui
From: Song Hui <hui.song_1@nxp.com>
Update the nodes to include little-endian
property to be consistent with the hardware.
Signed-off-by: Song Hui <hui.song_1@nxp.com>
---
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index aef5b06..7ccbbfc 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -277,33 +277,36 @@
};
gpio1: gpio@2300000 {
- compatible = "fsl,qoriq-gpio";
+ compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
reg = <0x0 0x2300000 0x0 0x10000>;
interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+ little-endian;
};
gpio2: gpio@2310000 {
- compatible = "fsl,qoriq-gpio";
+ compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
reg = <0x0 0x2310000 0x0 0x10000>;
interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+ little-endian;
};
gpio3: gpio@2320000 {
- compatible = "fsl,qoriq-gpio";
+ compatible = "fsl,ls1028a-gpio","fsl,qoriq-gpio";
reg = <0x0 0x2320000 0x0 0x10000>;
interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
gpio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
+ little-endian;
};
usb0: usb@3100000 {
--
2.9.5
^ permalink raw reply related
* Re: [RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings
From: Andrew Jeffery @ 2019-08-05 0:48 UTC (permalink / raw)
To: Joel Stanley
Cc: Rob Herring, linux-aspeed, Mark Rutland, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
linux-kernel@vger.kernel.org, Adriana Kobylak,
Alexander A. Filippov, Arnd Bergmann,
YangBrianC.W 楊嘉偉 TAO, Corey Minyard,
Greg Kroah-Hartman, Haiyue Wang, John Wang, Ken Chen,
Linus Walleij, open list:GPIO SUBSYSTEM, openipmi-developer,
Patrick Venture, Stefan M Schaeckeler, Tao Ren, Xo Wang, yao.yuan
In-Reply-To: <CACPK8Xc4Vigeu1B1Su5392BSCSKfoEDqt_tiDtgKmNH5ucAvAg@mail.gmail.com>
On Fri, 2 Aug 2019, at 15:21, Joel Stanley wrote:
> On Tue, 30 Jul 2019 at 01:09, Andrew Jeffery <andrew@aj.id.au> wrote:
>
> > > > The bang-for-buck is in fixing up the KCS bindings which removes all-but-two of
> > > > the remaining warnings (which we can't feasibly remove), but doing so forces
> > > > code changes (which I'd avoided up until this point).
> > > >
> > > > Reflecting broadly on the fixes, I think I've made a mistake way back by using
> > > > syscon/simple-mfds to expose the innards of the SCU and LPC controllers in the
> > > > devicetree. This series cleans up what's currently there, but I have half a
> > > > mind to rev the SCU and LPC bindings to not use simple-mfd and instead have a
> > > > driver implementation that uses `platform_device_register_full()` or similar to
> > > > deal with the mess.
> > > >
> > > > Rob - I'm looking for your thoughts here and on the series, I've never felt
> > > > entirely comfortable with what I cooked up. Your advice would be appreciated.
> > >
> > > The series generally looks fine to me from a quick scan. As far as
> > > dropping 'simple-mfd', having less fine grained description in DT is
> > > generally my preference. It comes down to whether what you have
> > > defined is maintainable. As most of it is just additions, I think what
> > > you have is fine. Maybe keep all this in mind for the next chip
> > > depending how the SCU and LPC change.
> >
> > Okay, I think the timing of that suggestion is good given where things are with
> > the AST2600. I'll keep that in mind.
> >
> > Consensus so far seems to be that the series is fine. I'll split it up and send out
> > the sub-series to the relevant lists with the acks accumulated here.
>
> The series look good. I suggest posting the KCS bindings and driver
> changes as their own series to go through the IPMI tree.
Yeah, that was the plan.
>
> Please add my tag to all the patches except the OCC one, which I need
> to do some investigation in to.
>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
Thanks, will do.
>
> The others can go via the aspeed tree. Perhaps post them as their own
> series too so I don't get confused and apply the wrong ones. That way
> if Rob wants to send his reviewed-by he can.
SGTM.
Cheers,
Andrew
^ permalink raw reply
* [PATCH] pinctrl: freescale: mxs: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-04 16:04 UTC (permalink / raw)
To: linus.walleij, linux-arm-kernel, linux-gpio, linux-imx, s.hauer,
kernel, stefan, shawnguo, festevam, aisheng.dong
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return in
three places.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/freescale/pinctrl-mxs.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/freescale/pinctrl-mxs.c b/drivers/pinctrl/freescale/pinctrl-mxs.c
index 641b3088876f..735cedd0958a 100644
--- a/drivers/pinctrl/freescale/pinctrl-mxs.c
+++ b/drivers/pinctrl/freescale/pinctrl-mxs.c
@@ -488,8 +488,10 @@ static int mxs_pinctrl_probe_dt(struct platform_device *pdev,
if (of_property_read_u32(child, "reg", &val)) {
ret = mxs_pinctrl_parse_group(pdev, child,
idxg++, NULL);
- if (ret)
+ if (ret) {
+ of_node_put(child);
return ret;
+ }
continue;
}
@@ -499,15 +501,19 @@ static int mxs_pinctrl_probe_dt(struct platform_device *pdev,
f->ngroups,
sizeof(*f->groups),
GFP_KERNEL);
- if (!f->groups)
+ if (!f->groups) {
+ of_node_put(child);
return -ENOMEM;
+ }
fn = child->name;
i = 0;
}
ret = mxs_pinctrl_parse_group(pdev, child, idxg++,
&f->groups[i++]);
- if (ret)
+ if (ret) {
+ of_node_put(child);
return ret;
+ }
}
return 0;
--
2.19.1
^ permalink raw reply related
* [PATCH] pinctrl: samsung: exynos: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-04 16:02 UTC (permalink / raw)
To: linus.walleij, linux-arm-kernel, linux-gpio, tomasz.figa, krzk,
s.nawrocki, kgene, linux-samsung-soc
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/samsung/pinctrl-exynos.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
index ebc27b06718c..e7f4cbad2c92 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -486,8 +486,10 @@ int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)
if (match) {
irq_chip = kmemdup(match->data,
sizeof(*irq_chip), GFP_KERNEL);
- if (!irq_chip)
+ if (!irq_chip) {
+ of_node_put(np);
return -ENOMEM;
+ }
wkup_np = np;
break;
}
--
2.19.1
^ permalink raw reply related
* [PATCH] pinctrl: nomadik: abx500: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-04 15:51 UTC (permalink / raw)
To: linus.walleij, linux-arm-kernel, linux-gpio; +Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/nomadik/pinctrl-abx500.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pinctrl/nomadik/pinctrl-abx500.c b/drivers/pinctrl/nomadik/pinctrl-abx500.c
index c3595200e1e6..7aa534576a45 100644
--- a/drivers/pinctrl/nomadik/pinctrl-abx500.c
+++ b/drivers/pinctrl/nomadik/pinctrl-abx500.c
@@ -815,6 +815,7 @@ static int abx500_dt_node_to_map(struct pinctrl_dev *pctldev,
&reserved_maps, num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map, *num_maps);
+ of_node_put(np);
return ret;
}
}
--
2.19.1
^ permalink raw reply related
* [PATCH] pinctrl: nomadik: nomadik: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-04 15:51 UTC (permalink / raw)
To: linus.walleij, linux-arm-kernel, linux-gpio; +Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/nomadik/pinctrl-nomadik.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index ddd1f466d302..2a8190b11d10 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -1508,6 +1508,7 @@ static int nmk_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
&reserved_maps, num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map, *num_maps);
+ of_node_put(np);
return ret;
}
}
--
2.19.1
^ permalink raw reply related
* [PATCH] pinctrl: spear: spear: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-04 15:49 UTC (permalink / raw)
To: vireshk, linus.walleij, linux-arm-kernel, linux-gpio; +Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return in
two places.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/spear/pinctrl-spear.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/spear/pinctrl-spear.c b/drivers/pinctrl/spear/pinctrl-spear.c
index c4f850345dc4..7ec19c73f870 100644
--- a/drivers/pinctrl/spear/pinctrl-spear.c
+++ b/drivers/pinctrl/spear/pinctrl-spear.c
@@ -157,12 +157,16 @@ static int spear_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
/* calculate number of maps required */
for_each_child_of_node(np_config, np) {
ret = of_property_read_string(np, "st,function", &function);
- if (ret < 0)
+ if (ret < 0) {
+ of_node_put(np);
return ret;
+ }
ret = of_property_count_strings(np, "st,pins");
- if (ret < 0)
+ if (ret < 0) {
+ of_node_put(np);
return ret;
+ }
count += ret;
}
--
2.19.1
^ permalink raw reply related
* [PATCH] pinctrl: rzn1: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-04 15:40 UTC (permalink / raw)
To: geert+renesas, linus.walleij, linux-renesas-soc, linux-gpio,
linux-kernel
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return in
three places.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/pinctrl-rzn1.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-rzn1.c b/drivers/pinctrl/pinctrl-rzn1.c
index cc0e5aa9128a..0f6f8a10a53a 100644
--- a/drivers/pinctrl/pinctrl-rzn1.c
+++ b/drivers/pinctrl/pinctrl-rzn1.c
@@ -412,8 +412,10 @@ static int rzn1_dt_node_to_map(struct pinctrl_dev *pctldev,
for_each_child_of_node(np, child) {
ret = rzn1_dt_node_to_map_one(pctldev, child, map, num_maps);
- if (ret < 0)
+ if (ret < 0) {
+ of_node_put(child);
return ret;
+ }
}
return 0;
@@ -792,8 +794,10 @@ static int rzn1_pinctrl_parse_functions(struct device_node *np,
grp = &ipctl->groups[ipctl->ngroups];
grp->func = func->name;
ret = rzn1_pinctrl_parse_groups(child, grp, ipctl);
- if (ret < 0)
+ if (ret < 0) {
+ of_node_put(child);
return ret;
+ }
i++;
ipctl->ngroups++;
}
@@ -838,8 +842,10 @@ static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
for_each_child_of_node(np, child) {
ret = rzn1_pinctrl_parse_functions(child, ipctl, i++);
- if (ret < 0)
+ if (ret < 0) {
+ of_node_put(child);
return ret;
+ }
}
return 0;
--
2.19.1
^ permalink raw reply related
* [PATCH] pinctrl: falcon: Add of_node_put() before return
From: Nishka Dasgupta @ 2019-08-04 15:27 UTC (permalink / raw)
To: linus.walleij, linux-gpio; +Cc: Nishka Dasgupta
Each iteration of for_each_compatible_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak. Hence add an of_node_put before the return in two
places.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/pinctrl/pinctrl-falcon.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-falcon.c b/drivers/pinctrl/pinctrl-falcon.c
index ef133a82e612..2257803f6f34 100644
--- a/drivers/pinctrl/pinctrl-falcon.c
+++ b/drivers/pinctrl/pinctrl-falcon.c
@@ -455,12 +455,15 @@ static int pinctrl_falcon_probe(struct platform_device *pdev)
falcon_info.clk[*bank] = clk_get(&ppdev->dev, NULL);
if (IS_ERR(falcon_info.clk[*bank])) {
dev_err(&ppdev->dev, "failed to get clock\n");
+ of_node_put(np)
return PTR_ERR(falcon_info.clk[*bank]);
}
falcon_info.membase[*bank] = devm_ioremap_resource(&pdev->dev,
&res);
- if (IS_ERR(falcon_info.membase[*bank]))
+ if (IS_ERR(falcon_info.membase[*bank])) {
+ of_node_put(np);
return PTR_ERR(falcon_info.membase[*bank]);
+ }
avail = pad_r32(falcon_info.membase[*bank],
LTQ_PADC_AVAIL);
--
2.19.1
^ permalink raw reply related
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-04 12:31 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: <2eab4f34-34be-e317-05bb-37749f2ef792@gmail.com>
04.08.2019 15:24, Dmitry Osipenko пишет:
> 04.08.2019 2:44, Sowjanya Komatineni пишет:
>>
>> On 8/3/19 10:01 AM, Sowjanya Komatineni wrote:
>>>
>>> On 8/3/19 3:33 AM, Dmitry Osipenko wrote:
>>>> 03.08.2019 2:51, Sowjanya Komatineni пишет:
>>>>> On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
>>>>>> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>>>>>>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>>>>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>>>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock
>>>>>>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and
>>>>>>>>>>>>>>>>>>>>>>>> looses the
>>>>>>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to
>>>>>>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>> +static int
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + udelay(2);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> static const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>>>>>>> .is_enabled =
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>>>>>>> + .save_context =
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>>>>>>> @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>> +static int clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + udelay(5);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void
>>>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>> +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw
>>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers
>>>>>>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the
>>>>>>>>>>>>>>>>>>>>>> context's
>>>>>>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing
>>>>>>>>>>>>>>>>>>>>> clk_mux_ops
>>>>>>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from
>>>>>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>>>>>>> its better to implement save_context and
>>>>>>>>>>>>>>>>>>>>> restore_context to
>>>>>>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring'
>>>>>>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be done
>>>>>>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the
>>>>>>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>>>>>>> referring to
>>>>>>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>>>>>>> use for
>>>>>>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>>> so I
>>>>>>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then
>>>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>>>>>>>>>>>>>>> I'm not sure whether it will be good for every driver that
>>>>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>>>>> generic
>>>>>>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper
>>>>>>>>>>>>>>>>> function
>>>>>>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>>>>>>> parent.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> The clk-periph restoring also includes case of combining
>>>>>>>>>>>>>>>>> divider
>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>> parent restoring, so generic helper could be useful in that
>>>>>>>>>>>>>>>>> case
>>>>>>>>>>>>>>>>> as well.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't
>>>>>>>>>>>>>>>>> you?
>>>>>>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>>>>>>> rather
>>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>>>>>>> clk_periph
>>>>>>>>>>>>>>>> restore.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>>>>>>> anyway.
>>>>>>>>>>>> Will do this for periph clk mux
>>>>> Just to be clear, clk_mux don't have cached parent. get_parent is by
>>>>> register read. So, cant directly use get_parent and then set during
>>>>> restore.
>>>>>
>>>>> So will create both clk_mux_save/restore_context in generic clk driver
>>>>> and will invoke them during tegra peripheral clock suspend/resume.
>>>> Why MUX clock doesn't have a cached parent? What MUX clock you're
>>>> talking about?
>>>>
>>>> [snip]
>>>
>>> Please ignore got it.
>>>
>>> Will send next version after giving few more days for feedback.
>>>
>> Couple of issues:
>>
>> 1.) I see clk-tegra-periph driver periph_clks init_data entries for some
>> peripherals are not correct for Tegra 114 and later chips.
>>
>> Eg I2C TEGRA_INIT_DATA_TABLE entries in clk-tegra-periph are used for all Tegra
>> chipsets currently in the driver.
>>
>> These entries are using MUX shift of 30 and MUX mask only for 2 bits which is
>> correct for T30 and prior.
>> But for later Tegra chips, it should be MUX shift 29 and MASK(3).
>>
>> Also, I2C parent idx entries in mux_pllp_clkm_idx are different from Tegra114
>> onwards.
>>
>> As we are using only PLLP and CLKM sources only for I2C, their corresponding mux
>> values from register spec by using upper 2 bits for T114 onwards match actual 2
>> bits of MUX value on T30 and prior.
>>
>> Not sure if this something known pending to port actual clock MUX table changes
>> for Tegra114 onwards?
>>
>> Or
>>
>> Are we purposely using upper 2 bits only for clock source for T114 and later as
>> the upper 2 bit values of the limited clock source we are using match with
>> previous Tegra peripheral clock source mux values?
>
> The actual hardware values are compatible on all Tegra SoCs and PLLC2 and PLLC3
> are just unused on T114+, so should be okay.
>
>> Peter/Thierry, Can you please help comment on this?
>>
>>
>> 2.) Other issue is regarding using clk_set_parent directly during clk_peripheral
>> restore is clk_core_set_parent checks new parent with current parent and if its
>> same, it just returns as success which is good in normal operation.
>>
>> But during restore, we can't use clk_set_parent as new parent is from
>> clk_get_parent on restore and this is same as cached parent.
>>
>> So clk_set_parent returns 0 but acutal register value for clk source is different
>> as it gets reset on SC7 entry/exit and to restore need to invoke mux_ops
>> set_parent with parent_index.
>>
>> So this need parent index for cached parent and without using context variable to
>> store this, need an API like you were originally suggesting for get_parent_index
>> to get parent index for the specified parent clk_hw.
>>
>> As we decided not to use save/restore for clk_mux ops as its generic for other
>> drivers, looks like we need get_parent_index API to use for restoring peripheral
>> source and use this with clk_mux_ops set_parent.
>>
>> clk core driver already has clk_fetch_parent_index but is it OK to export this?
>>
>> Otherwise, will create separate API in clk driver which returns parent index from
>> parent clk_hw by using this existing clk_fetch_parent_index so this API can be
>> used by other drivers.
Actually, you probably meant exactly the same what I wrote below. So
yes, it should be good to extend the CCF API since likely that this case
will be useful for other drivers as well.
> The clk_fetch_parent_index can't be used directly as you can see because it uses
> clk_core which is internal to clk.c, you'll need to make a wrapper for getting the
> cached index.
>
> Something like this:
>
> int clk_hw_get_parent_index(struct clk_hw *hw, struct *parent_hw)
> {
> if (!hw || !parent_hw)
> return -EINVAL;
>
> return clk_fetch_parent_index(hw->core, parent_hw->core);
> }
>
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-04 12:24 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: <4dec8efb-bc3b-0275-8dea-7600a8f9e478@nvidia.com>
04.08.2019 2:44, Sowjanya Komatineni пишет:
>
> On 8/3/19 10:01 AM, Sowjanya Komatineni wrote:
>>
>> On 8/3/19 3:33 AM, Dmitry Osipenko wrote:
>>> 03.08.2019 2:51, Sowjanya Komatineni пишет:
>>>> On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
>>>>> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>>>>>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>>>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock
>>>>>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and
>>>>>>>>>>>>>>>>>>>>>>> looses the
>>>>>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to
>>>>>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>> +static int
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + udelay(2);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> static const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>>>>>> .is_enabled =
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>>>>>> + .save_context =
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>>>>>> @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>> +static int clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + udelay(5);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void
>>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>> +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw
>>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers
>>>>>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the
>>>>>>>>>>>>>>>>>>>>> context's
>>>>>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing
>>>>>>>>>>>>>>>>>>>> clk_mux_ops
>>>>>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from
>>>>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>>>>>> its better to implement save_context and
>>>>>>>>>>>>>>>>>>>> restore_context to
>>>>>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring'
>>>>>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be done
>>>>>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the
>>>>>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>>>>>> referring to
>>>>>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>>>>>> use for
>>>>>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>> so I
>>>>>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then
>>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>>>>>>>>>>>>>> I'm not sure whether it will be good for every driver that
>>>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>>>> generic
>>>>>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper
>>>>>>>>>>>>>>>> function
>>>>>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>>>>>> parent.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> The clk-periph restoring also includes case of combining
>>>>>>>>>>>>>>>> divider
>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>> parent restoring, so generic helper could be useful in that
>>>>>>>>>>>>>>>> case
>>>>>>>>>>>>>>>> as well.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't
>>>>>>>>>>>>>>>> you?
>>>>>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>>>>>> rather
>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>>>>>> clk_periph
>>>>>>>>>>>>>>> restore.
>>>>>>>>>>>>>>>
>>>>>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>>>>>> anyway.
>>>>>>>>>>> Will do this for periph clk mux
>>>> Just to be clear, clk_mux don't have cached parent. get_parent is by
>>>> register read. So, cant directly use get_parent and then set during
>>>> restore.
>>>>
>>>> So will create both clk_mux_save/restore_context in generic clk driver
>>>> and will invoke them during tegra peripheral clock suspend/resume.
>>> Why MUX clock doesn't have a cached parent? What MUX clock you're
>>> talking about?
>>>
>>> [snip]
>>
>> Please ignore got it.
>>
>> Will send next version after giving few more days for feedback.
>>
> Couple of issues:
>
> 1.) I see clk-tegra-periph driver periph_clks init_data entries for some
> peripherals are not correct for Tegra 114 and later chips.
>
> Eg I2C TEGRA_INIT_DATA_TABLE entries in clk-tegra-periph are used for all Tegra
> chipsets currently in the driver.
>
> These entries are using MUX shift of 30 and MUX mask only for 2 bits which is
> correct for T30 and prior.
> But for later Tegra chips, it should be MUX shift 29 and MASK(3).
>
> Also, I2C parent idx entries in mux_pllp_clkm_idx are different from Tegra114
> onwards.
>
> As we are using only PLLP and CLKM sources only for I2C, their corresponding mux
> values from register spec by using upper 2 bits for T114 onwards match actual 2
> bits of MUX value on T30 and prior.
>
> Not sure if this something known pending to port actual clock MUX table changes
> for Tegra114 onwards?
>
> Or
>
> Are we purposely using upper 2 bits only for clock source for T114 and later as
> the upper 2 bit values of the limited clock source we are using match with
> previous Tegra peripheral clock source mux values?
The actual hardware values are compatible on all Tegra SoCs and PLLC2 and PLLC3
are just unused on T114+, so should be okay.
> Peter/Thierry, Can you please help comment on this?
>
>
> 2.) Other issue is regarding using clk_set_parent directly during clk_peripheral
> restore is clk_core_set_parent checks new parent with current parent and if its
> same, it just returns as success which is good in normal operation.
>
> But during restore, we can't use clk_set_parent as new parent is from
> clk_get_parent on restore and this is same as cached parent.
>
> So clk_set_parent returns 0 but acutal register value for clk source is different
> as it gets reset on SC7 entry/exit and to restore need to invoke mux_ops
> set_parent with parent_index.
>
> So this need parent index for cached parent and without using context variable to
> store this, need an API like you were originally suggesting for get_parent_index
> to get parent index for the specified parent clk_hw.
>
> As we decided not to use save/restore for clk_mux ops as its generic for other
> drivers, looks like we need get_parent_index API to use for restoring peripheral
> source and use this with clk_mux_ops set_parent.
>
> clk core driver already has clk_fetch_parent_index but is it OK to export this?
>
> Otherwise, will create separate API in clk driver which returns parent index from
> parent clk_hw by using this existing clk_fetch_parent_index so this API can be
> used by other drivers.
The clk_fetch_parent_index can't be used directly as you can see because it uses
clk_core which is internal to clk.c, you'll need to make a wrapper for getting the
cached index.
Something like this:
int clk_hw_get_parent_index(struct clk_hw *hw, struct *parent_hw)
{
if (!hw || !parent_hw)
return -EINVAL;
return clk_fetch_parent_index(hw->core, parent_hw->core);
}
^ permalink raw reply
* [PATCH v4] spi: bcm2835: Convert to use CS GPIO descriptors
From: Linus Walleij @ 2019-08-04 0:38 UTC (permalink / raw)
To: Mark Brown, linux-spi
Cc: linux-gpio, Bartosz Golaszewski, Linus Walleij, Lukas Wunner,
Stefan Wahren, Martin Sperl, Chris Boot
This converts the BCM2835 SPI master driver to use GPIO
descriptors for chip select handling.
The BCM2835 driver was relying on the core to drive the
CS high/low so very small changes were needed for this
part. If it managed to request the CS from the device tree
node, all is pretty straight forward.
However for native GPIOs this driver has a quite unorthodox
loopback to request some GPIOs from the SoC GPIO chip by
looking it up from the device tree using gpiochip_find()
and then offseting hard into its numberspace. This has
been augmented a bit by using gpiochip_request_own_desc()
but this code really needs to be verified. If "native CS"
is actually an SoC GPIO, why is it even done this way?
Should this GPIO not just be defined in the device tree
like any other CS GPIO? I'm confused.
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Martin Sperl <kernel@martin.sperl.org>
Cc: Chris Boot <bootc@bootc.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v3->v4:
- Fix the offset of the chipselect line to be 8 - CS
as in the original code.
- Use the modified gpiochip_request_own_desc() to set up
line inversion semantics if need be. Look at the OF
node of the SPI device for flags.
ChangeLog v2->v3:
- Fix unused variable "err" compile-time message.
ChangeLog RFT->v2:
- Rebased on v5.1-rc1
I would very much appreciate if someone took this for
a ride on top of linux-next (there are some fixes in
the -rcs you need) and see if all still works as expected.
---
drivers/spi/spi-bcm2835.c | 58 +++++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 18 deletions(-)
diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 6f243a90c844..a40c09c553d3 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -25,7 +25,9 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_device.h>
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h> /* FIXME: using chip internals */
+#include <linux/gpio/driver.h> /* FIXME: using chip internals */
#include <linux/of_irq.h>
#include <linux/spi/spi.h>
@@ -930,14 +932,19 @@ static int chip_match_name(struct gpio_chip *chip, void *data)
static int bcm2835_spi_setup(struct spi_device *spi)
{
- int err;
struct gpio_chip *chip;
+ enum gpio_lookup_flags lflags;
+
/*
* sanity checking the native-chipselects
*/
if (spi->mode & SPI_NO_CS)
return 0;
- if (gpio_is_valid(spi->cs_gpio))
+ /*
+ * The SPI core has successfully requested the CS GPIO line from the
+ * device tree, so we are done.
+ */
+ if (spi->cs_gpiod)
return 0;
if (spi->chip_select > 1) {
/* error in the case of native CS requested with CS > 1
@@ -948,29 +955,43 @@ static int bcm2835_spi_setup(struct spi_device *spi)
"setup: only two native chip-selects are supported\n");
return -EINVAL;
}
- /* now translate native cs to GPIO */
+
+ /*
+ * Translate native CS to GPIO
+ *
+ * FIXME: poking around in the gpiolib internals like this is
+ * not very good practice. Find a way to locate the real problem
+ * and fix it. Why is the GPIO descriptor in spi->cs_gpiod
+ * sometimes not assigned correctly? Erroneous device trees?
+ */
/* get the gpio chip for the base */
chip = gpiochip_find("pinctrl-bcm2835", chip_match_name);
if (!chip)
return 0;
- /* and calculate the real CS */
- spi->cs_gpio = chip->base + 8 - spi->chip_select;
+ /*
+ * Retrieve the corresponding GPIO line used for CS.
+ * The inversion semantics will be handled by the GPIO core
+ * code, so we pass GPIOS_OUT_LOW for "unasserted" and
+ * the correct flag for inversion semantics. The SPI_CS_HIGH
+ * on spi->mode cannot be checked for polarity in this case
+ * as the flag use_gpio_descriptors enforces SPI_CS_HIGH.
+ */
+ if (of_property_read_bool(spi->dev.of_node, "spi-cs-high"))
+ lflags = GPIO_ACTIVE_HIGH;
+ else
+ lflags = GPIO_ACTIVE_LOW;
+ spi->cs_gpiod = gpiochip_request_own_desc(chip, 8 - spi->chip_select,
+ DRV_NAME,
+ lflags,
+ GPIOD_OUT_LOW);
+ if (IS_ERR(spi->cs_gpiod))
+ return PTR_ERR(spi->cs_gpiod);
/* and set up the "mode" and level */
- dev_info(&spi->dev, "setting up native-CS%i as GPIO %i\n",
- spi->chip_select, spi->cs_gpio);
-
- /* set up GPIO as output and pull to the correct level */
- err = gpio_direction_output(spi->cs_gpio,
- (spi->mode & SPI_CS_HIGH) ? 0 : 1);
- if (err) {
- dev_err(&spi->dev,
- "could not set CS%i gpio %i as output: %i",
- spi->chip_select, spi->cs_gpio, err);
- return err;
- }
+ dev_info(&spi->dev, "setting up native-CS%i to use GPIO\n",
+ spi->chip_select);
return 0;
}
@@ -988,6 +1009,7 @@ static int bcm2835_spi_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, ctlr);
+ ctlr->use_gpio_descriptors = true;
ctlr->mode_bits = BCM2835_SPI_MODE_BITS;
ctlr->bits_per_word_mask = SPI_BPW_MASK(8);
ctlr->num_chipselect = 3;
--
2.21.0
^ permalink raw reply related
* [PATCH v2] spi: fsl: Convert to use CS GPIO descriptors
From: Linus Walleij @ 2019-08-04 0:35 UTC (permalink / raw)
To: Mark Brown, linux-spi, Fabio Estevam
Cc: linux-gpio, Bartosz Golaszewski, Linus Walleij, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, NXP Linux Team
This converts the Freescale SPI master driver to use GPIO
descriptors for chip select handling.
The Freescale (fsl) driver has a lot of quirks to look up
"gpios" rather than "cs-gpios" from the device tree.
After the prior patch that will make gpiolib return the
GPIO descriptor for "gpios" in response to a request for
"cs-gpios", this code can be cut down quite a bit.
The driver has custom handling of chip select rather
than using the core (which may be possible but not
done in this patch) so it still needs to refer directly
to spi->cs_gpiod to set the chip select.
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Rebased on v5.3-rc1
---
drivers/spi/spi-fsl-lib.h | 3 -
drivers/spi/spi-fsl-spi.c | 193 +++++---------------------------------
2 files changed, 25 insertions(+), 171 deletions(-)
diff --git a/drivers/spi/spi-fsl-lib.h b/drivers/spi/spi-fsl-lib.h
index 3576167283dc..015a1abb6a84 100644
--- a/drivers/spi/spi-fsl-lib.h
+++ b/drivers/spi/spi-fsl-lib.h
@@ -91,9 +91,6 @@ static inline u32 mpc8xxx_spi_read_reg(__be32 __iomem *reg)
struct mpc8xxx_spi_probe_info {
struct fsl_spi_platform_data pdata;
- int ngpios;
- int *gpios;
- bool *alow_flags;
__be32 __iomem *immr_spi_cs;
};
diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index 1d9b33aa1a3b..4b80ace1d137 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -18,7 +18,7 @@
#include <linux/delay.h>
#include <linux/dma-mapping.h>
#include <linux/fsl_devices.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
#include <linux/kernel.h>
@@ -28,7 +28,6 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
-#include <linux/of_gpio.h>
#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/spi/spi.h>
@@ -481,32 +480,6 @@ static int fsl_spi_setup(struct spi_device *spi)
return retval;
}
- if (mpc8xxx_spi->type == TYPE_GRLIB) {
- if (gpio_is_valid(spi->cs_gpio)) {
- int desel;
-
- retval = gpio_request(spi->cs_gpio,
- dev_name(&spi->dev));
- if (retval)
- return retval;
-
- desel = !(spi->mode & SPI_CS_HIGH);
- retval = gpio_direction_output(spi->cs_gpio, desel);
- if (retval) {
- gpio_free(spi->cs_gpio);
- return retval;
- }
- } else if (spi->cs_gpio != -ENOENT) {
- if (spi->cs_gpio < 0)
- return spi->cs_gpio;
- return -EINVAL;
- }
- /* When spi->cs_gpio == -ENOENT, a hole in the phandle list
- * indicates to use native chipselect if present, or allow for
- * an always selected chip
- */
- }
-
/* Initialize chipselect - might be active for SPI_CS_HIGH mode */
fsl_spi_chipselect(spi, BITBANG_CS_INACTIVE);
@@ -515,12 +488,8 @@ static int fsl_spi_setup(struct spi_device *spi)
static void fsl_spi_cleanup(struct spi_device *spi)
{
- struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(spi->master);
struct spi_mpc8xxx_cs *cs = spi_get_ctldata(spi);
- if (mpc8xxx_spi->type == TYPE_GRLIB && gpio_is_valid(spi->cs_gpio))
- gpio_free(spi->cs_gpio);
-
kfree(cs);
spi_set_ctldata(spi, NULL);
}
@@ -586,8 +555,8 @@ static void fsl_spi_grlib_cs_control(struct spi_device *spi, bool on)
u32 slvsel;
u16 cs = spi->chip_select;
- if (gpio_is_valid(spi->cs_gpio)) {
- gpio_set_value(spi->cs_gpio, on);
+ if (spi->cs_gpiod) {
+ gpiod_set_value(spi->cs_gpiod, on);
} else if (cs < mpc8xxx_spi->native_chipselects) {
slvsel = mpc8xxx_spi_read_reg(®_base->slvsel);
slvsel = on ? (slvsel | (1 << cs)) : (slvsel & ~(1 << cs));
@@ -718,139 +687,19 @@ static struct spi_master * fsl_spi_probe(struct device *dev,
static void fsl_spi_cs_control(struct spi_device *spi, bool on)
{
- struct device *dev = spi->dev.parent->parent;
- struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
- struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
- u16 cs = spi->chip_select;
-
- if (cs < pinfo->ngpios) {
- int gpio = pinfo->gpios[cs];
- bool alow = pinfo->alow_flags[cs];
-
- gpio_set_value(gpio, on ^ alow);
+ if (spi->cs_gpiod) {
+ gpiod_set_value(spi->cs_gpiod, on);
} else {
- if (WARN_ON_ONCE(cs > pinfo->ngpios || !pinfo->immr_spi_cs))
+ struct device *dev = spi->dev.parent->parent;
+ struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
+ struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
+
+ if (WARN_ON_ONCE(!pinfo->immr_spi_cs))
return;
iowrite32be(on ? SPI_BOOT_SEL_BIT : 0, pinfo->immr_spi_cs);
}
}
-static int of_fsl_spi_get_chipselects(struct device *dev)
-{
- struct device_node *np = dev->of_node;
- struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
- struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
- bool spisel_boot = IS_ENABLED(CONFIG_FSL_SOC) &&
- of_property_read_bool(np, "fsl,spisel_boot");
- int ngpios;
- int i = 0;
- int ret;
-
- ngpios = of_gpio_count(np);
- ngpios = max(ngpios, 0);
- if (ngpios == 0 && !spisel_boot) {
- /*
- * SPI w/o chip-select line. One SPI device is still permitted
- * though.
- */
- pdata->max_chipselect = 1;
- return 0;
- }
-
- pinfo->ngpios = ngpios;
- pinfo->gpios = kmalloc_array(ngpios, sizeof(*pinfo->gpios),
- GFP_KERNEL);
- if (!pinfo->gpios)
- return -ENOMEM;
- memset(pinfo->gpios, -1, ngpios * sizeof(*pinfo->gpios));
-
- pinfo->alow_flags = kcalloc(ngpios, sizeof(*pinfo->alow_flags),
- GFP_KERNEL);
- if (!pinfo->alow_flags) {
- ret = -ENOMEM;
- goto err_alloc_flags;
- }
-
- for (; i < ngpios; i++) {
- int gpio;
- enum of_gpio_flags flags;
-
- gpio = of_get_gpio_flags(np, i, &flags);
- if (!gpio_is_valid(gpio)) {
- dev_err(dev, "invalid gpio #%d: %d\n", i, gpio);
- ret = gpio;
- goto err_loop;
- }
-
- ret = gpio_request(gpio, dev_name(dev));
- if (ret) {
- dev_err(dev, "can't request gpio #%d: %d\n", i, ret);
- goto err_loop;
- }
-
- pinfo->gpios[i] = gpio;
- pinfo->alow_flags[i] = flags & OF_GPIO_ACTIVE_LOW;
-
- ret = gpio_direction_output(pinfo->gpios[i],
- pinfo->alow_flags[i]);
- if (ret) {
- dev_err(dev,
- "can't set output direction for gpio #%d: %d\n",
- i, ret);
- goto err_loop;
- }
- }
-
-#if IS_ENABLED(CONFIG_FSL_SOC)
- if (spisel_boot) {
- pinfo->immr_spi_cs = ioremap(get_immrbase() + IMMR_SPI_CS_OFFSET, 4);
- if (!pinfo->immr_spi_cs) {
- ret = -ENOMEM;
- i = ngpios - 1;
- goto err_loop;
- }
- }
-#endif
-
- pdata->max_chipselect = ngpios + spisel_boot;
- pdata->cs_control = fsl_spi_cs_control;
-
- return 0;
-
-err_loop:
- while (i >= 0) {
- if (gpio_is_valid(pinfo->gpios[i]))
- gpio_free(pinfo->gpios[i]);
- i--;
- }
-
- kfree(pinfo->alow_flags);
- pinfo->alow_flags = NULL;
-err_alloc_flags:
- kfree(pinfo->gpios);
- pinfo->gpios = NULL;
- return ret;
-}
-
-static int of_fsl_spi_free_chipselects(struct device *dev)
-{
- struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
- struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
- int i;
-
- if (!pinfo->gpios)
- return 0;
-
- for (i = 0; i < pdata->max_chipselect; i++) {
- if (gpio_is_valid(pinfo->gpios[i]))
- gpio_free(pinfo->gpios[i]);
- }
-
- kfree(pinfo->gpios);
- kfree(pinfo->alow_flags);
- return 0;
-}
-
static int of_fsl_spi_probe(struct platform_device *ofdev)
{
struct device *dev = &ofdev->dev;
@@ -866,9 +715,21 @@ static int of_fsl_spi_probe(struct platform_device *ofdev)
type = fsl_spi_get_type(&ofdev->dev);
if (type == TYPE_FSL) {
- ret = of_fsl_spi_get_chipselects(dev);
- if (ret)
- goto err;
+ struct fsl_spi_platform_data *pdata = dev_get_platdata(dev);
+#if IS_ENABLED(CONFIG_FSL_SOC)
+ struct mpc8xxx_spi_probe_info *pinfo = to_of_pinfo(pdata);
+ bool spisel_boot = of_property_read_bool(np, "fsl,spisel_boot");
+
+ if (spisel_boot) {
+ pinfo->immr_spi_cs = ioremap(get_immrbase() + IMMR_SPI_CS_OFFSET, 4);
+ if (!pinfo->immr_spi_cs) {
+ ret = -ENOMEM;
+ goto err;
+ }
+ }
+#endif
+
+ pdata->cs_control = fsl_spi_cs_control;
}
ret = of_address_to_resource(np, 0, &mem);
@@ -891,8 +752,6 @@ static int of_fsl_spi_probe(struct platform_device *ofdev)
err:
irq_dispose_mapping(irq);
- if (type == TYPE_FSL)
- of_fsl_spi_free_chipselects(dev);
return ret;
}
@@ -902,8 +761,6 @@ static int of_fsl_spi_remove(struct platform_device *ofdev)
struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(master);
fsl_spi_cpm_free(mpc8xxx_spi);
- if (mpc8xxx_spi->type == TYPE_FSL)
- of_fsl_spi_free_chipselects(&ofdev->dev);
return 0;
}
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-03 23:44 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: <274e67b1-16b8-2475-d026-68bd89b090ec@nvidia.com>
On 8/3/19 10:01 AM, Sowjanya Komatineni wrote:
>
> On 8/3/19 3:33 AM, Dmitry Osipenko wrote:
>> 03.08.2019 2:51, Sowjanya Komatineni пишет:
>>> On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
>>>> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>>>>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux
>>>>>>>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and
>>>>>>>>>>>>>>>>>>>>>> looses the
>>>>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks
>>>>>>>>>>>>>>>>>>>>>> back to
>>>>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>> +static int
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + udelay(2);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> static const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>>>>> .is_enabled =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>>>>> .enable =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>>>>> .disable =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>>>>> + .save_context =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>>>>> @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>> +static int
>>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + udelay(5);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> const struct clk_ops
>>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void
>>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>> +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops =
>>>>>>>>>>>>>>>>>>>>>> periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags &
>>>>>>>>>>>>>>>>>>>>>> TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct
>>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops =
>>>>>>>>>>>>>>>>>>>>>> periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *div_ops =
>>>>>>>>>>>>>>>>>>>>>> periph->div_ops;
>>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags &
>>>>>>>>>>>>>>>>>>>>>> TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the
>>>>>>>>>>>>>>>>>>>> dividers
>>>>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the
>>>>>>>>>>>>>>>>>>>> context's
>>>>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing
>>>>>>>>>>>>>>>>>>> clk_mux_ops
>>>>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent()
>>>>>>>>>>>>>>>>>>> from
>>>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>>>>> its better to implement save_context and
>>>>>>>>>>>>>>>>>>> restore_context to
>>>>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a
>>>>>>>>>>>>>>>>> 'restoring'
>>>>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be
>>>>>>>>>>>>>>>>> done
>>>>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to
>>>>>>>>>>>>>>>>> associate the
>>>>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>>>>> referring to
>>>>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>>>>> use for
>>>>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>> so I
>>>>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and
>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>> clk-periph restore need to invoke
>>>>>>>>>>>>>>>> mux_ops->restore_context.
>>>>>>>>>>>>>>> I'm not sure whether it will be good for every driver that
>>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>>> generic
>>>>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic
>>>>>>>>>>>>>>> helper
>>>>>>>>>>>>>>> function
>>>>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>>>>> parent.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The clk-periph restoring also includes case of combining
>>>>>>>>>>>>>>> divider
>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>> parent restoring, so generic helper could be useful in that
>>>>>>>>>>>>>>> case
>>>>>>>>>>>>>>> as well.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>>>>> instead of manually saving the clock's enable-state,
>>>>>>>>>>>>>>> couldn't
>>>>>>>>>>>>>>> you?
>>>>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>>>>> rather
>>>>>>>>>>>>>> than
>>>>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>>>>> clk_periph
>>>>>>>>>>>>>> restore.
>>>>>>>>>>>>>>
>>>>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>>>>> anyway.
>>>>>>>>>> Will do this for periph clk mux
>>> Just to be clear, clk_mux don't have cached parent. get_parent is by
>>> register read. So, cant directly use get_parent and then set during
>>> restore.
>>>
>>> So will create both clk_mux_save/restore_context in generic clk driver
>>> and will invoke them during tegra peripheral clock suspend/resume.
>> Why MUX clock doesn't have a cached parent? What MUX clock you're
>> talking about?
>>
>> [snip]
>
> Please ignore got it.
>
> Will send next version after giving few more days for feedback.
>
Couple of issues:
1.) I see clk-tegra-periph driver periph_clks init_data entries for some
peripherals are not correct for Tegra 114 and later chips.
Eg I2C TEGRA_INIT_DATA_TABLE entries in clk-tegra-periph are used for
all Tegra chipsets currently in the driver.
These entries are using MUX shift of 30 and MUX mask only for 2 bits
which is correct for T30 and prior.
But for later Tegra chips, it should be MUX shift 29 and MASK(3).
Also, I2C parent idx entries in mux_pllp_clkm_idx are different from
Tegra114 onwards.
As we are using only PLLP and CLKM sources only for I2C, their
corresponding mux values from register spec by using upper 2 bits for
T114 onwards match actual 2 bits of MUX value on T30 and prior.
Not sure if this something known pending to port actual clock MUX table
changes for Tegra114 onwards?
Or
Are we purposely using upper 2 bits only for clock source for T114 and
later as the upper 2 bit values of the limited clock source we are using
match with previous Tegra peripheral clock source mux values?
Peter/Thierry, Can you please help comment on this?
2.) Other issue is regarding using clk_set_parent directly during
clk_peripheral restore is clk_core_set_parent checks new parent with
current parent and if its same, it just returns as success which is good
in normal operation.
But during restore, we can't use clk_set_parent as new parent is from
clk_get_parent on restore and this is same as cached parent.
So clk_set_parent returns 0 but acutal register value for clk source is
different as it gets reset on SC7 entry/exit and to restore need to
invoke mux_ops set_parent with parent_index.
So this need parent index for cached parent and without using context
variable to store this, need an API like you were originally suggesting
for get_parent_index to get parent index for the specified parent clk_hw.
As we decided not to use save/restore for clk_mux ops as its generic for
other drivers, looks like we need get_parent_index API to use for
restoring peripheral source and use this with clk_mux_ops set_parent.
clk core driver already has clk_fetch_parent_index but is it OK to
export this?
Otherwise, will create separate API in clk driver which returns parent
index from parent clk_hw by using this existing clk_fetch_parent_index
so this API can be used by other drivers.
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-03 17:01 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: <8d30f325-1c63-3802-7c21-f313115f8e55@gmail.com>
On 8/3/19 3:33 AM, Dmitry Osipenko wrote:
> 03.08.2019 2:51, Sowjanya Komatineni пишет:
>> On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
>>> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>>>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock
>>>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and
>>>>>>>>>>>>>>>>>>>>> looses the
>>>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to
>>>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>> +static int
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + udelay(2);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> static const struct clk_ops
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>>>> .is_enabled =
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>>>> + .save_context =
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>>>> @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>> +static int clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + udelay(5);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> const struct clk_ops
>>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>>> clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void
>>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>> +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw
>>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers
>>>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the
>>>>>>>>>>>>>>>>>>> context's
>>>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing
>>>>>>>>>>>>>>>>>> clk_mux_ops
>>>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from
>>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>>>> its better to implement save_context and
>>>>>>>>>>>>>>>>>> restore_context to
>>>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring'
>>>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be done
>>>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the
>>>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>>>> referring to
>>>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>>>> use for
>>>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>> so I
>>>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then
>>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>>>>>>>>>>>> I'm not sure whether it will be good for every driver that
>>>>>>>>>>>>>> uses
>>>>>>>>>>>>>> generic
>>>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper
>>>>>>>>>>>>>> function
>>>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>>>> parent.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The clk-periph restoring also includes case of combining
>>>>>>>>>>>>>> divider
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>> parent restoring, so generic helper could be useful in that
>>>>>>>>>>>>>> case
>>>>>>>>>>>>>> as well.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't
>>>>>>>>>>>>>> you?
>>>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>>>> rather
>>>>>>>>>>>>> than
>>>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>>>> clk_periph
>>>>>>>>>>>>> restore.
>>>>>>>>>>>>>
>>>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>>>> anyway.
>>>>>>>>> Will do this for periph clk mux
>> Just to be clear, clk_mux don't have cached parent. get_parent is by
>> register read. So, cant directly use get_parent and then set during
>> restore.
>>
>> So will create both clk_mux_save/restore_context in generic clk driver
>> and will invoke them during tegra peripheral clock suspend/resume.
> Why MUX clock doesn't have a cached parent? What MUX clock you're
> talking about?
>
> [snip]
Please ignore got it.
Will send next version after giving few more days for feedback.
^ permalink raw reply
* [PATCH] gpiolib: Take MUX usage into account
From: Ramon Fried @ 2019-08-03 13:34 UTC (permalink / raw)
To: linus.walleij, bgolaszewski
Cc: linux-gpio, linux-kernel, Stefan Wahren, Ramon Fried
From: Stefan Wahren <stefan.wahren@i2se.com>
The user space like gpioinfo only see the GPIO usage but not the
MUX usage (e.g. I2C or SPI usage) of a pin. As a user we want to know which
pin is free/safe to use. So take the MUX usage of strict pinmux controllers
into account to get a more realistic view for ioctl GPIO_GET_LINEINFO_IOCTL.
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Tested-By: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Ramon Fried <rfried.dev@gmail.com>
---
Sending Stefan's RFC as patch, as I tested it and it seems to work,
additionally, an accompanying fix was made by me to gpiolibd to fix a
display error of the actual result:
https://patchwork.ozlabs.org/patch/1139923/
drivers/gpio/gpiolib.c | 3 ++-
drivers/pinctrl/core.c | 23 +++++++++++++++++++++++
drivers/pinctrl/pinmux.c | 18 ++++++++++++++++++
drivers/pinctrl/pinmux.h | 7 +++++++
include/linux/pinctrl/consumer.h | 6 ++++++
5 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index e013d417a936..2fd9eee0b98c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1082,7 +1082,8 @@ static long gpio_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
test_bit(FLAG_IS_HOGGED, &desc->flags) ||
test_bit(FLAG_USED_AS_IRQ, &desc->flags) ||
test_bit(FLAG_EXPORT, &desc->flags) ||
- test_bit(FLAG_SYSFS, &desc->flags))
+ test_bit(FLAG_SYSFS, &desc->flags) ||
+ pinctrl_gpio_is_in_use(chip->base + lineinfo.line_offset))
lineinfo.flags |= GPIOLINE_FLAG_KERNEL;
if (test_bit(FLAG_IS_OUT, &desc->flags))
lineinfo.flags |= GPIOLINE_FLAG_IS_OUT;
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index a64849a9d1b0..0dd00c175eed 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -759,6 +759,29 @@ int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
return -EINVAL;
}
+bool pinctrl_gpio_is_in_use(unsigned gpio)
+{
+ struct pinctrl_dev *pctldev;
+ struct pinctrl_gpio_range *range;
+ bool result;
+ int pin;
+
+ if (pinctrl_get_device_gpio_range(gpio, &pctldev, &range))
+ return false;
+
+ mutex_lock(&pctldev->mutex);
+
+ /* Convert to the pin controllers number space */
+ pin = gpio_to_pin(range, gpio);
+
+ result = pinmux_is_in_use(pctldev, pin);
+
+ mutex_unlock(&pctldev->mutex);
+
+ return result;
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_is_in_use);
+
/**
* pinctrl_gpio_request() - request a single pin to be used as GPIO
* @gpio: the GPIO pin number from the GPIO subsystem number space
diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c
index 020e54f843f9..02d2751a4884 100644
--- a/drivers/pinctrl/pinmux.c
+++ b/drivers/pinctrl/pinmux.c
@@ -70,6 +70,24 @@ int pinmux_validate_map(const struct pinctrl_map *map, int i)
return 0;
}
+bool pinmux_is_in_use(struct pinctrl_dev *pctldev, unsigned pin)
+{
+ struct pin_desc *desc = pin_desc_get(pctldev, pin);
+ const struct pinmux_ops *ops = pctldev->desc->pmxops;
+
+ if (!desc) {
+ dev_err(pctldev->dev,
+ "pin %u is not registered so it cannot be requested\n",
+ pin);
+ return false;
+ }
+
+ if (ops->strict && desc->mux_usecount)
+ return true;
+
+ return ops->strict && !!desc->gpio_owner;
+}
+
/**
* pin_request() - request a single pin to be muxed in, typically for GPIO
* @pin: the pin number in the global pin space
diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h
index 794cb3a003ff..24ae61136803 100644
--- a/drivers/pinctrl/pinmux.h
+++ b/drivers/pinctrl/pinmux.h
@@ -15,6 +15,8 @@ int pinmux_check_ops(struct pinctrl_dev *pctldev);
int pinmux_validate_map(const struct pinctrl_map *map, int i);
+bool pinmux_is_in_use(struct pinctrl_dev *pctldev, unsigned pin);
+
int pinmux_request_gpio(struct pinctrl_dev *pctldev,
struct pinctrl_gpio_range *range,
unsigned pin, unsigned gpio);
@@ -42,6 +44,11 @@ static inline int pinmux_validate_map(const struct pinctrl_map *map, int i)
return 0;
}
+static inline bool pinmux_is_in_use(struct pinctrl_dev *pctldev, unsigned pin)
+{
+ return false;
+}
+
static inline int pinmux_request_gpio(struct pinctrl_dev *pctldev,
struct pinctrl_gpio_range *range,
unsigned pin, unsigned gpio)
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 86720a5a384f..d26826b057a1 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -24,6 +24,7 @@ struct device;
#ifdef CONFIG_PINCTRL
/* External interface to pin control */
+extern bool pinctrl_gpio_is_in_use(unsigned gpio);
extern int pinctrl_gpio_request(unsigned gpio);
extern void pinctrl_gpio_free(unsigned gpio);
extern int pinctrl_gpio_direction_input(unsigned gpio);
@@ -61,6 +62,11 @@ static inline int pinctrl_pm_select_idle_state(struct device *dev)
#else /* !CONFIG_PINCTRL */
+static inline bool pinctrl_gpio_is_in_use(unsigned gpio)
+{
+ return 0;
+}
+
static inline int pinctrl_gpio_request(unsigned gpio)
{
return 0;
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-03 10:33 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: <d82de63a-02a3-8fb0-57e7-7fe00d6b86ab@nvidia.com>
03.08.2019 2:51, Sowjanya Komatineni пишет:
>
> On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
>> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock
>>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and
>>>>>>>>>>>>>>>>>>>> looses the
>>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to
>>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>> +static int
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + udelay(2);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> static const struct clk_ops
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>>> .is_enabled =
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>>> + .save_context =
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>>> @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>> +static int clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + udelay(5);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> const struct clk_ops
>>>>>>>>>>>>>>>>>>>> tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>>> clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void
>>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>> +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw
>>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph =
>>>>>>>>>>>>>>>>>>>> to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers
>>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the
>>>>>>>>>>>>>>>>>> context's
>>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing
>>>>>>>>>>>>>>>>> clk_mux_ops
>>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from
>>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>>> its better to implement save_context and
>>>>>>>>>>>>>>>>> restore_context to
>>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring'
>>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be done
>>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the
>>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>>> referring to
>>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>>> Yes.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>>> use for
>>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>> so I
>>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then
>>>>>>>>>>>>>> during
>>>>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>>>>>>>>>>> I'm not sure whether it will be good for every driver that
>>>>>>>>>>>>> uses
>>>>>>>>>>>>> generic
>>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper
>>>>>>>>>>>>> function
>>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>>> parent.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The clk-periph restoring also includes case of combining
>>>>>>>>>>>>> divider
>>>>>>>>>>>>> and
>>>>>>>>>>>>> parent restoring, so generic helper could be useful in that
>>>>>>>>>>>>> case
>>>>>>>>>>>>> as well.
>>>>>>>>>>>>>
>>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't
>>>>>>>>>>>>> you?
>>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>>> rather
>>>>>>>>>>>> than
>>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>>> clk_periph
>>>>>>>>>>>> restore.
>>>>>>>>>>>>
>>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>>> anyway.
>>>>>>>> Will do this for periph clk mux
>
> Just to be clear, clk_mux don't have cached parent. get_parent is by
> register read. So, cant directly use get_parent and then set during
> restore.
>
> So will create both clk_mux_save/restore_context in generic clk driver
> and will invoke them during tegra peripheral clock suspend/resume.
Why MUX clock doesn't have a cached parent? What MUX clock you're
talking about?
[snip]
^ permalink raw reply
* Re: [PATCH 3/3] arm64: dts: ls1088a: Revise gpio registers to little-endian
From: Shawn Guo @ 2019-08-03 7:11 UTC (permalink / raw)
To: Chuanhua Han
Cc: leoyang.li, robh+dt, mark.rutland, linus.walleij, bgolaszewski,
linux-arm-kernel, devicetree, linux-kernel, linux-gpio
In-Reply-To: <20190529083254.39581-3-chuanhua.han@nxp.com>
On Wed, May 29, 2019 at 04:32:54PM +0800, Chuanhua Han wrote:
> Since fsl-ls1088a Soc GPIO registers are used as little endian,
> the patch adds the little-endian attribute to each gpio node.
>
> Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-02 23:51 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: <73cd521b-782c-7fb2-d904-ae8b07927d47@gmail.com>
On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock
>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and looses the
>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to
>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>> +static int
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + udelay(2);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> static const struct clk_ops
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>> .is_enabled =
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>> @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>> +static int clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + udelay(5);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>> struct clk *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>> +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers
>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the context's
>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing clk_mux_ops
>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from
>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>> its better to implement save_context and restore_context to
>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring'
>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be done
>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the
>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>> referring to
>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>> Yes.
>>>>>>>>>>>>
>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>> use for
>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>
>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>> so I
>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then
>>>>>>>>>>>>> during
>>>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>>>>>>>>>> I'm not sure whether it will be good for every driver that uses
>>>>>>>>>>>> generic
>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper
>>>>>>>>>>>> function
>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>> parent.
>>>>>>>>>>>>
>>>>>>>>>>>> The clk-periph restoring also includes case of combining divider
>>>>>>>>>>>> and
>>>>>>>>>>>> parent restoring, so generic helper could be useful in that case
>>>>>>>>>>>> as well.
>>>>>>>>>>>>
>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't
>>>>>>>>>>>> you?
>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>> rather
>>>>>>>>>>> than
>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>> clk_periph
>>>>>>>>>>> restore.
>>>>>>>>>>>
>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>> anyway.
>>>>>>> Will do this for periph clk mux
Just to be clear, clk_mux don't have cached parent. get_parent is by
register read. So, cant directly use get_parent and then set during restore.
So will create both clk_mux_save/restore_context in generic clk driver
and will invoke them during tegra peripheral clock suspend/resume.
>>>>>>>>>>> Reg clk_gate, looks like we cant use generic
>>>>>>>>>>> clk_gate_restore_context
>>>>>>>>>>> for clk-periph as it calls enable/disable callbacks and
>>>>>>>>>>> clk_periph_enable/disable in clk-periph-gate also updated
>>>>>>>>>>> refcnt and
>>>>>>>>>>> depending on that actual enable/disable is set.
>>>>>>>>>>>
>>>>>>>>>>> During suspend, peripherals that are already enabled have their
>>>>>>>>>>> refcnt >
>>>>>>>>>>> 1, so they dont go thru enable/disable on restore if we use same
>>>>>>>>>>> enable/disable callback.
>>>>>>>>>> Looks like you could just decrement the gate's enable_refcnt on
>>>>>>>>>> save_context, wouldn't that work?
>>>>>>>>>>
>>>>>>>> gate->enable_refcnt is within clk-periph-gate which gets updated
>>>>>>>> when
>>>>>>>> enable/disable callbacks get execute thru clk_core_enable/disable.
>>>>>>>> But actual enable_count used in clk_gate_restore_context is the one
>>>>>>>> which gets updated with in the clk core enable/disable functions
>>>>>>>> which
>>>>>>>> invokes these callbacks. Depending on this enable_count in clk
>>>>>>>> core it
>>>>>>>> invokes enable/disable.
>>>>>>>>
>>>>>>>> So, this will cause mismatch if we handle refcnt during save/restore
>>>>>>>> of tegra_clk_periph_gate_ops and also enable/disable thru this
>>>>>>>> clk_gate_restore_context is based on enable_count from clk core.
>>>>>>>>
>>>>>>>>>>> Also to align exact reset state along with CLK (like for case
>>>>>>>>>>> where
>>>>>>>>>>> CLK
>>>>>>>>>>> is enabled but peripheral might be in reset state), implemented
>>>>>>>>>>> save/restore in tegra specific tegra_clk_periph_gate_ops
>>>>>>>>>> I'm wondering whether instead of saving/restoring reset-state of
>>>>>>>>>> every
>>>>>>>>>> clock, you could simply save/restore the whole RST_DEV_x_SET
>>>>>>>>>> register.
>>>>>>>>>> Couldn't you?
>>>>>>>>> Thats what I was doing in first version of patch. But later as we
>>>>>>>>> moved to use clk_save_context and clk_restore_context, peripheral
>>>>>>>>> clk_hw RST & CLK enables happen thru its corresponding save/restore
>>>>>>>>> after source restore
>>>>>>>> Also, to align both CLK & RST to the exact state of register, doing
>>>>>>>> save/restore in tegra_clk_periph_gate_ops and invoking this after
>>>>>>>> source restore for peripheral clock, seems cleaner to avoid any
>>>>>>>> misconfiguration b/w rst & clk settings.
>>>>>>>>
>>>>>> It looks to me that it is very wasteful to store/restore each
>>>>>> individual
>>>>>> gate and reset state, also given that some of them are shared. I think
>>>>>> that the gates and resets should be restored separately for the
>>>>>> peripherals by a custom tegra_clk_save/restore_periph_gates/resets().
>>>>> clk_periph_fixed_disable just disables clock only without deasserting
>>>>> the corresponding peripheral.
>>>>>
>>>>> corresponding peripheral drivers can also issue reset assert/deassert
>>>>> thru reset_control_assert/deassert.
>>>>>
>>>>> So, we will not get the actual state of clk and rst unless we read and
>>>>> save state of reset and clock separately during save_context.
>>>>>
>>>>> Currently patch is already using custom
>>>>> tegra_clk_periph_fixed_save/restore_context for corresponding clk_ops.
>>>>> Are you suggesting to do save and restore of complete CLK_ENB/RST_DEV
>>>>> register settings instead of individual peripheral bits?
>>>> Yes, I'm suggesting to do a complete ungate/reset handling of the
>>>> devices in a separate function. All enabling/deassertion will be done in
>>>> a single hop, hence using 7us delay and four u32 words, which is much
>>>> nicer IMHO.
>>> Actually six words, three for CLKs and three for RSTs.
>> OK, So with separate function doing complete register save/restore for
>> clk & rst, we can't do this thru clk_ops save/restore as clk_ops
>> save_restore happens per peripheral wise. So if we decide to do this,
>> then this should be invoked in clk-tegra210 driver suspend/resume.
> Yes, per-clock save/restore should be used for setting rate and parent.
> The ungating and resetting could be done separately to keep things cleaner.
>
^ permalink raw reply
* Re: [PATCH -next] pinctrl: sprd: Fix platform_no_drv_owner.cocci warnings
From: Linus Walleij @ 2019-08-02 22:33 UTC (permalink / raw)
To: YueHaibing
Cc: Orson Zhai, Baolin Wang, Chunyan Zhang, Greg Kroah-Hartman,
Kate Stewart, Thomas Gleixner, Randy Dunlap, Richard Fontana,
open list:GPIO SUBSYSTEM, kernel-janitors
In-Reply-To: <20190719032414.85369-1-yuehaibing@huawei.com>
On Fri, Jul 19, 2019 at 5:18 AM YueHaibing <yuehaibing@huawei.com> wrote:
> Remove .owner field if calls are used which set it automatically
> Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] dt-bindings: pinctrl: stm32: Fix missing 'clocks' property in examples
From: Linus Walleij @ 2019-08-02 22:31 UTC (permalink / raw)
To: Rob Herring
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, linux-kernel@vger.kernel.org, Maxime Coquelin,
Alexandre Torgue, open list:GPIO SUBSYSTEM, linux-stm32
In-Reply-To: <20190716215618.29757-1-robh@kernel.org>
On Tue, Jul 16, 2019 at 11:56 PM Rob Herring <robh@kernel.org> wrote:
> Now that examples are validated against the DT schema, an error with
> required 'clocks' property missing is exposed:
>
> Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.example.dt.yaml: \
> pinctrl@40020000: gpio@0: 'clocks' is a required property
> Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.example.dt.yaml: \
> pinctrl@50020000: gpio@1000: 'clocks' is a required property
> Documentation/devicetree/bindings/pinctrl/st,stm32-pinctrl.example.dt.yaml: \
> pinctrl@50020000: gpio@2000: 'clocks' is a required property
>
> Add the missing 'clocks' properties to the examples to fix the errors.
>
> Fixes: 2c9239c125f0 ("dt-bindings: pinctrl: Convert stm32 pinctrl bindings to json-schema")
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Signed-off-by: Rob Herring <robh@kernel.org>
This seems to already be upstream, but I have no memory of applying it.
Less work for me :)
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] gpio: refactor gpiochip_allocate_mask() with bitmap_alloc()
From: Linus Walleij @ 2019-08-02 22:26 UTC (permalink / raw)
To: Masahiro Yamada
Cc: open list:GPIO SUBSYSTEM, Stephen Boyd, Bartosz Golaszewski,
linux-kernel@vger.kernel.org
In-Reply-To: <20190718065101.26994-1-yamada.masahiro@socionext.com>
On Thu, Jul 18, 2019 at 8:51 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Refactor gpiochip_allocate_mask() slightly by using bitmap_alloc().
>
> I used bitmap_free() for the corresponding free parts. Actually,
> bitmap_free() is a wrapper of kfree(), but I did this for consistency.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Patch applied with Stephen's ACK.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 2/2] gpio: mpc8xxx: Add ls1028a device specify function.
From: Linus Walleij @ 2019-08-02 22:19 UTC (permalink / raw)
To: Hui Song
Cc: Shawn Guo, Li Yang, Rob Herring, Mark Rutland,
Bartosz Golaszewski, Linux ARM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM
In-Reply-To: <20190718094902.15562-2-hui.song_1@nxp.com>
On Thu, Jul 18, 2019 at 11:58 AM Hui Song <hui.song_1@nxp.com> wrote:
> From: Song Hui <hui.song_1@nxp.com>
>
> There is a device specify register(named GPIO_IBE)
> on ls1028a need to enable in initial stage.
>
> Signed-off-by: Song Hui <hui.song_1@nxp.com>
Patch applied.
As noted on patch 1/2, send a separate patch to add the
device tree bindings in Documentation/devicetree/bindings/gpio/gpio-mpc8xxx.txt
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-02 21:18 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: <73cd521b-782c-7fb2-d904-ae8b07927d47@gmail.com>
On 8/2/19 2:15 PM, Dmitry Osipenko wrote:
> 02.08.2019 23:32, Sowjanya Komatineni пишет:
>> On 8/2/19 1:17 PM, Dmitry Osipenko wrote:
>>> 02.08.2019 23:13, Dmitry Osipenko пишет:
>>>> 02.08.2019 21:33, Sowjanya Komatineni пишет:
>>>>> On 8/2/19 5:38 AM, Dmitry Osipenko wrote:
>>>>>> 02.08.2019 2:49, Sowjanya Komatineni пишет:
>>>>>>> On 8/1/19 4:19 PM, Sowjanya Komatineni wrote:
>>>>>>>> On 8/1/19 2:30 PM, Sowjanya Komatineni wrote:
>>>>>>>>> On 8/1/19 1:54 PM, Dmitry Osipenko wrote:
>>>>>>>>>> 01.08.2019 23:31, Sowjanya Komatineni пишет:
>>>>>>>>>>> On 8/1/19 1:17 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>> 01.08.2019 22:42, Sowjanya Komatineni пишет:
>>>>>>>>>>>>> On 8/1/19 12:00 PM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>> 01.08.2019 20:58, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>> On 7/31/19 4:09 PM, Sowjanya Komatineni wrote:
>>>>>>>>>>>>>>>> On 7/31/19 3:44 AM, Dmitry Osipenko wrote:
>>>>>>>>>>>>>>>>> 31.07.2019 12:50, Dmitry Osipenko пишет:
>>>>>>>>>>>>>>>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>>>>>>>>>>>>>>>> This patch implements save and restore context for
>>>>>>>>>>>>>>>>>>> peripheral
>>>>>>>>>>>>>>>>>>> fixed
>>>>>>>>>>>>>>>>>>> clock ops, peripheral gate clock ops, sdmmc mux clock
>>>>>>>>>>>>>>>>>>> ops, and
>>>>>>>>>>>>>>>>>>> peripheral clock ops.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> During system suspend, core power goes off and looses the
>>>>>>>>>>>>>>>>>>> settings
>>>>>>>>>>>>>>>>>>> of the Tegra CAR controller registers.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> So during suspend entry clock and reset state of
>>>>>>>>>>>>>>>>>>> peripherals is
>>>>>>>>>>>>>>>>>>> saved
>>>>>>>>>>>>>>>>>>> and on resume they are restored to have clocks back to
>>>>>>>>>>>>>>>>>>> same
>>>>>>>>>>>>>>>>>>> rate and
>>>>>>>>>>>>>>>>>>> state as before suspend.
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>>>>>>>>>>> Signed-off-by: Sowjanya Komatineni
>>>>>>>>>>>>>>>>>>> <skomatineni@nvidia.com>
>>>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-fixed.c | 33
>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph-gate.c | 34
>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-periph.c | 37
>>>>>>>>>>>>>>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk-sdmmc-mux.c | 28
>>>>>>>>>>>>>>>>>>> +++++++++++++++++++++++++++
>>>>>>>>>>>>>>>>>>> drivers/clk/tegra/clk.h | 6 ++++++
>>>>>>>>>>>>>>>>>>> 5 files changed, 138 insertions(+)
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>> index c088e7a280df..21b24530fa00 100644
>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-fixed.c
>>>>>>>>>>>>>>>>>>> @@ -60,11 +60,44 @@
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate(struct
>>>>>>>>>>>>>>>>>>> clk_hw *hw,
>>>>>>>>>>>>>>>>>>> return (unsigned long)rate;
>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>> +static int
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_save_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + fixed->enb_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->enb_reg) &
>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>> + fixed->rst_ctx = readl_relaxed(fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg) &
>>>>>>>>>>>>>>>>>>> + mask;
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> +static void
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_fixed *fixed =
>>>>>>>>>>>>>>>>>>> to_tegra_clk_periph_fixed(hw);
>>>>>>>>>>>>>>>>>>> + u32 mask = 1 << (fixed->num % 32);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (fixed->enb_ctx)
>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->enb_set_reg);
>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->enb_clr_reg);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + udelay(2);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (!fixed->rst_ctx) {
>>>>>>>>>>>>>>>>>>> + udelay(5); /* reset propogation delay */
>>>>>>>>>>>>>>>>>>> + writel_relaxed(mask, fixed->base +
>>>>>>>>>>>>>>>>>>> fixed->regs->rst_reg);
>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> static const struct clk_ops
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_ops
>>>>>>>>>>>>>>>>>>> = {
>>>>>>>>>>>>>>>>>>> .is_enabled =
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_is_enabled,
>>>>>>>>>>>>>>>>>>> .enable = tegra_clk_periph_fixed_enable,
>>>>>>>>>>>>>>>>>>> .disable = tegra_clk_periph_fixed_disable,
>>>>>>>>>>>>>>>>>>> .recalc_rate =
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_recalc_rate,
>>>>>>>>>>>>>>>>>>> + .save_context = tegra_clk_periph_fixed_save_context,
>>>>>>>>>>>>>>>>>>> + .restore_context =
>>>>>>>>>>>>>>>>>>> tegra_clk_periph_fixed_restore_context,
>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>> struct clk
>>>>>>>>>>>>>>>>>>> *tegra_clk_register_periph_fixed(const
>>>>>>>>>>>>>>>>>>> char
>>>>>>>>>>>>>>>>>>> *name,
>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>> index 4b31beefc9fc..6ba5b08e0787 100644
>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph-gate.c
>>>>>>>>>>>>>>>>>>> @@ -25,6 +25,8 @@ static
>>>>>>>>>>>>>>>>>>> DEFINE_SPINLOCK(periph_ref_lock);
>>>>>>>>>>>>>>>>>>> #define read_rst(gate) \
>>>>>>>>>>>>>>>>>>> readl_relaxed(gate->clk_base +
>>>>>>>>>>>>>>>>>>> (gate->regs->rst_reg))
>>>>>>>>>>>>>>>>>>> +#define write_rst_set(val, gate) \
>>>>>>>>>>>>>>>>>>> + writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>> (gate->regs->rst_set_reg))
>>>>>>>>>>>>>>>>>>> #define write_rst_clr(val, gate) \
>>>>>>>>>>>>>>>>>>> writel_relaxed(val, gate->clk_base +
>>>>>>>>>>>>>>>>>>> (gate->regs->rst_clr_reg))
>>>>>>>>>>>>>>>>>>> @@ -110,10 +112,42 @@ static void
>>>>>>>>>>>>>>>>>>> clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>> spin_unlock_irqrestore(&periph_ref_lock, flags);
>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>> +static int clk_periph_gate_save_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + gate->clk_state_ctx = read_enb(gate) &
>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>> + gate->rst_state_ctx = read_rst(gate) &
>>>>>>>>>>>>>>>>>>> periph_clk_to_bit(gate);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> +static void clk_periph_gate_restore_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph_gate *gate =
>>>>>>>>>>>>>>>>>>> to_clk_periph_gate(hw);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (gate->clk_state_ctx)
>>>>>>>>>>>>>>>>>>> + write_enb_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>> + write_enb_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + udelay(5);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (!(gate->flags & TEGRA_PERIPH_NO_RESET) &&
>>>>>>>>>>>>>>>>>>> + !(gate->flags & TEGRA_PERIPH_MANUAL_RESET)) {
>>>>>>>>>>>>>>>>>>> + if (gate->rst_state_ctx)
>>>>>>>>>>>>>>>>>>> + write_rst_set(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>> + else
>>>>>>>>>>>>>>>>>>> + write_rst_clr(periph_clk_to_bit(gate), gate);
>>>>>>>>>>>>>>>>>>> + }
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> const struct clk_ops tegra_clk_periph_gate_ops = {
>>>>>>>>>>>>>>>>>>> .is_enabled = clk_periph_is_enabled,
>>>>>>>>>>>>>>>>>>> .enable = clk_periph_enable,
>>>>>>>>>>>>>>>>>>> .disable = clk_periph_disable,
>>>>>>>>>>>>>>>>>>> + .save_context = clk_periph_gate_save_context,
>>>>>>>>>>>>>>>>>>> + .restore_context = clk_periph_gate_restore_context,
>>>>>>>>>>>>>>>>>>> };
>>>>>>>>>>>>>>>>>>> struct clk *tegra_clk_register_periph_gate(const
>>>>>>>>>>>>>>>>>>> char *name,
>>>>>>>>>>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>> b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>> index 58437da25156..06fb62955768 100644
>>>>>>>>>>>>>>>>>>> --- a/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>> +++ b/drivers/clk/tegra/clk-periph.c
>>>>>>>>>>>>>>>>>>> @@ -99,6 +99,37 @@ static void clk_periph_disable(struct
>>>>>>>>>>>>>>>>>>> clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> gate_ops->disable(gate_hw);
>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>> +static int clk_periph_save_context(struct
>>>>>>>>>>>>>>>>>>> clk_hw *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_GATE))
>>>>>>>>>>>>>>>>>>> + gate_ops->save_context(gate_hw);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + periph->parent_ctx = clk_periph_get_parent(hw);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + return 0;
>>>>>>>>>>>>>>>>>>> +}
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> +static void clk_periph_restore_context(struct clk_hw
>>>>>>>>>>>>>>>>>>> *hw)
>>>>>>>>>>>>>>>>>>> +{
>>>>>>>>>>>>>>>>>>> + struct tegra_clk_periph *periph = to_clk_periph(hw);
>>>>>>>>>>>>>>>>>>> + const struct clk_ops *gate_ops = periph->gate_ops;
>>>>>>>>>>>>>>>>>>> + struct clk_hw *gate_hw = &periph->gate.hw;
>>>>>>>>>>>>>>>>>>> + const struct clk_ops *div_ops = periph->div_ops;
>>>>>>>>>>>>>>>>>>> + struct clk_hw *div_hw = &periph->divider.hw;
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + clk_periph_set_parent(hw, periph->parent_ctx);
>>>>>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>>>>>> + if (!(periph->gate.flags & TEGRA_PERIPH_NO_DIV))
>>>>>>>>>>>>>>>>>>> + div_ops->restore_context(div_hw);
>>>>>>>>>>>>>>>>>> Could you please point to where the divider's
>>>>>>>>>>>>>>>>>> save_context()
>>>>>>>>>>>>>>>>>> happens?
>>>>>>>>>>>>>>>>>> Because I can't see it.
>>>>>>>>>>>>>>>>> Ah, I now see that there is no need to save the dividers
>>>>>>>>>>>>>>>>> context
>>>>>>>>>>>>>>>>> because
>>>>>>>>>>>>>>>>> clk itself has enough info that is needed for the context's
>>>>>>>>>>>>>>>>> restoring
>>>>>>>>>>>>>>>>> (like I pointed in the review to v6).
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Looks like you could also implement a new
>>>>>>>>>>>>>>>>> clk_hw_get_parent_index()
>>>>>>>>>>>>>>>>> generic helper to get the index instead of storing it
>>>>>>>>>>>>>>>>> manually.
>>>>>>>>>>>>>>>> clk_periph_get_parent basically invokes existing clk_mux_ops
>>>>>>>>>>>>>>>> get_parent() which is then saved in tegra_clk_periph.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> All existing drivers are using directly get_parent() from
>>>>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>>>>> which actually gets index from the register read.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> To have this more generic w.r.t save/restore context
>>>>>>>>>>>>>>>> point of
>>>>>>>>>>>>>>>> view,
>>>>>>>>>>>>>>>> probably instead of implementing new get_parent_index
>>>>>>>>>>>>>>>> helper,
>>>>>>>>>>>>>>>> I think
>>>>>>>>>>>>>>>> its better to implement save_context and restore_context to
>>>>>>>>>>>>>>>> clk_mux_ops along with creating parent_index field into
>>>>>>>>>>>>>>>> clk_mux to
>>>>>>>>>>>>>>>> cache index during set_parent.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> So we just need to invoke mux_ops save_context and
>>>>>>>>>>>>>>>> restore_context.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I hope its ok to add save/restore context to clk_mux_ops
>>>>>>>>>>>>>>> to be
>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>> generic w.r.t save/restore context rather than
>>>>>>>>>>>>>>> get_parent_index
>>>>>>>>>>>>>>> API.
>>>>>>>>>>>>>>> Please confirm if you agree.
>>>>>>>>>>>>>> Sounds like a good idea. I see that there is a 'restoring'
>>>>>>>>>>>>>> helper for
>>>>>>>>>>>>>> the generic clk_gate, seems something similar could be done
>>>>>>>>>>>>>> for the
>>>>>>>>>>>>>> clk_mux. And looks like anyway you'll need to associate the
>>>>>>>>>>>>>> parent
>>>>>>>>>>>>>> clock
>>>>>>>>>>>>>> with the hw index in order to restore the muxing.
>>>>>>>>>>>>> by 'restoring' helper for generic clk_gate, are you
>>>>>>>>>>>>> referring to
>>>>>>>>>>>>> clk_gate_restore_context API?
>>>>>>>>>>>> Yes.
>>>>>>>>>>>>
>>>>>>>>>>>>> clk_gate_restore_context is API that's any clk drivers can
>>>>>>>>>>>>> use for
>>>>>>>>>>>>> clk_gate operation restore for custom gate clk_ops.
>>>>>>>>>>>>>
>>>>>>>>>>>>> But clk-periph is directly using generic clk_mux ops from
>>>>>>>>>>>>> clk_mux
>>>>>>>>>>>>> so I
>>>>>>>>>>>>> think we should add .restore_context to clk_mux_ops and then
>>>>>>>>>>>>> during
>>>>>>>>>>>>> clk-periph restore need to invoke mux_ops->restore_context.
>>>>>>>>>>>> I'm not sure whether it will be good for every driver that uses
>>>>>>>>>>>> generic
>>>>>>>>>>>> clk_mux ops. Should be more flexible to have a generic helper
>>>>>>>>>>>> function
>>>>>>>>>>>> that any driver could use in order to restore the clock's
>>>>>>>>>>>> parent.
>>>>>>>>>>>>
>>>>>>>>>>>> The clk-periph restoring also includes case of combining divider
>>>>>>>>>>>> and
>>>>>>>>>>>> parent restoring, so generic helper could be useful in that case
>>>>>>>>>>>> as well.
>>>>>>>>>>>>
>>>>>>>>>>>> It also looks like you could actually use the
>>>>>>>>>>>> clk_gate_restore_context()
>>>>>>>>>>>> instead of manually saving the clock's enable-state, couldn't
>>>>>>>>>>>> you?
>>>>>>>>>>> ok for clk_mux, can add generic clk_mux_restore_context API
>>>>>>>>>>> rather
>>>>>>>>>>> than
>>>>>>>>>>> using restore_context in clk_ops and will invoke that during
>>>>>>>>>>> clk_periph
>>>>>>>>>>> restore.
>>>>>>>>>>>
>>>>>>> digging thru looks like for clk_periph source restore instead of
>>>>>>> clk_mux_restore_context, i can directly do clk_hw_get_parent and
>>>>>>> clk_set_parent with mux_hw as they invoke mux_ops get/set parent
>>>>>>> anyway.
>>>>>>> Will do this for periph clk mux
>>>>>>>>>>> Reg clk_gate, looks like we cant use generic
>>>>>>>>>>> clk_gate_restore_context
>>>>>>>>>>> for clk-periph as it calls enable/disable callbacks and
>>>>>>>>>>> clk_periph_enable/disable in clk-periph-gate also updated
>>>>>>>>>>> refcnt and
>>>>>>>>>>> depending on that actual enable/disable is set.
>>>>>>>>>>>
>>>>>>>>>>> During suspend, peripherals that are already enabled have their
>>>>>>>>>>> refcnt >
>>>>>>>>>>> 1, so they dont go thru enable/disable on restore if we use same
>>>>>>>>>>> enable/disable callback.
>>>>>>>>>> Looks like you could just decrement the gate's enable_refcnt on
>>>>>>>>>> save_context, wouldn't that work?
>>>>>>>>>>
>>>>>>>> gate->enable_refcnt is within clk-periph-gate which gets updated
>>>>>>>> when
>>>>>>>> enable/disable callbacks get execute thru clk_core_enable/disable.
>>>>>>>> But actual enable_count used in clk_gate_restore_context is the one
>>>>>>>> which gets updated with in the clk core enable/disable functions
>>>>>>>> which
>>>>>>>> invokes these callbacks. Depending on this enable_count in clk
>>>>>>>> core it
>>>>>>>> invokes enable/disable.
>>>>>>>>
>>>>>>>> So, this will cause mismatch if we handle refcnt during save/restore
>>>>>>>> of tegra_clk_periph_gate_ops and also enable/disable thru this
>>>>>>>> clk_gate_restore_context is based on enable_count from clk core.
>>>>>>>>
>>>>>>>>>>> Also to align exact reset state along with CLK (like for case
>>>>>>>>>>> where
>>>>>>>>>>> CLK
>>>>>>>>>>> is enabled but peripheral might be in reset state), implemented
>>>>>>>>>>> save/restore in tegra specific tegra_clk_periph_gate_ops
>>>>>>>>>> I'm wondering whether instead of saving/restoring reset-state of
>>>>>>>>>> every
>>>>>>>>>> clock, you could simply save/restore the whole RST_DEV_x_SET
>>>>>>>>>> register.
>>>>>>>>>> Couldn't you?
>>>>>>>>> Thats what I was doing in first version of patch. But later as we
>>>>>>>>> moved to use clk_save_context and clk_restore_context, peripheral
>>>>>>>>> clk_hw RST & CLK enables happen thru its corresponding save/restore
>>>>>>>>> after source restore
>>>>>>>> Also, to align both CLK & RST to the exact state of register, doing
>>>>>>>> save/restore in tegra_clk_periph_gate_ops and invoking this after
>>>>>>>> source restore for peripheral clock, seems cleaner to avoid any
>>>>>>>> misconfiguration b/w rst & clk settings.
>>>>>>>>
>>>>>> It looks to me that it is very wasteful to store/restore each
>>>>>> individual
>>>>>> gate and reset state, also given that some of them are shared. I think
>>>>>> that the gates and resets should be restored separately for the
>>>>>> peripherals by a custom tegra_clk_save/restore_periph_gates/resets().
>>>>> clk_periph_fixed_disable just disables clock only without deasserting
>>>>> the corresponding peripheral.
>>>>>
>>>>> corresponding peripheral drivers can also issue reset assert/deassert
>>>>> thru reset_control_assert/deassert.
>>>>>
>>>>> So, we will not get the actual state of clk and rst unless we read and
>>>>> save state of reset and clock separately during save_context.
>>>>>
>>>>> Currently patch is already using custom
>>>>> tegra_clk_periph_fixed_save/restore_context for corresponding clk_ops.
>>>>> Are you suggesting to do save and restore of complete CLK_ENB/RST_DEV
>>>>> register settings instead of individual peripheral bits?
>>>> Yes, I'm suggesting to do a complete ungate/reset handling of the
>>>> devices in a separate function. All enabling/deassertion will be done in
>>>> a single hop, hence using 7us delay and four u32 words, which is much
>>>> nicer IMHO.
>>> Actually six words, three for CLKs and three for RSTs.
>> OK, So with separate function doing complete register save/restore for
>> clk & rst, we can't do this thru clk_ops save/restore as clk_ops
>> save_restore happens per peripheral wise. So if we decide to do this,
>> then this should be invoked in clk-tegra210 driver suspend/resume.
> Yes, per-clock save/restore should be used for setting rate and parent.
> The ungating and resetting could be done separately to keep things cleaner.
>
OK, Will move back to register wise save/restore for clk_enb/rst_dev in
next version.
^ permalink raw reply
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