* Re: [PATCH 0/3] ARM: dts: aspeed: Deprecate g[45]-style compatibles
From: Joel Stanley @ 2019-08-02 6:15 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Linus Walleij, linux-aspeed, Lee Jones, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Linux ARM, linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM
In-Reply-To: <3691f6cb-2451-43f7-9f00-d5693071ba59@www.fastmail.com>
On Thu, 1 Aug 2019 at 05:45, Andrew Jeffery <andrew@aj.id.au> wrote:
>
>
>
> On Tue, 30 Jul 2019, at 10:27, Andrew Jeffery wrote:
> >
> >
> > On Tue, 30 Jul 2019, at 07:23, Linus Walleij wrote:
> > > On Wed, Jul 24, 2019 at 10:13 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> > >
> > > > It's probably best if we push the three patches all through one tree rather
> > > > than fragmenting. Is everyone happy if Joel applies them to the aspeed tree?
> > >
> > > If you are sure it will not collide with parallell work in the
> > > pinctrl tree, yes.
> > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > >
> > > (If it does collide I'd prefer to take the pinctrl patches and fix the
> > > conflicts in my tree.)
> >
> > Fair enough, I don't know the answer so I'll poke around. I don't
> > really mind
> > where the series goes in, I just want to avoid landing only part of it
> > if I split it up.
>
> Okay, it currently conflicts with my cleanup-devicetree-warnings series.
>
> Joel, do you mind if Linus takes this series through the pinctrl tree, given
> the fix to the devicetrees is patch 1/3?
It depends if you plan more changes to that part of the device tree
this merge window :)
Linus, perhaps the safer option is for me to take 1/3 through my tree
and you can take the rest through yours?
Cheers,
Joel
^ permalink raw reply
* Re: [RFC-ish PATCH 00/17] Clean up ASPEED devicetree warnings
From: Joel Stanley @ 2019-08-02 5:51 UTC (permalink / raw)
To: Andrew Jeffery
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: <fd8e57f0-aee2-403e-b6fb-76d0c18fe306@www.fastmail.com>
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.
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>
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.
Cheers,
Joel
^ permalink raw reply
* [PATCH v2 2/2] pinctrl: qcom: Add SC7180 pinctrl driver
From: Rajendra Nayak @ 2019-08-02 4:15 UTC (permalink / raw)
To: linus.walleij, bjorn.andersson
Cc: linux-arm-msm, agross, robh+dt, linux-gpio, devicetree,
linux-kernel, Jitendra Sharma, Vivek Gautam, Rajendra Nayak
In-Reply-To: <20190802041507.12365-1-rnayak@codeaurora.org>
From: Jitendra Sharma <shajit@codeaurora.org>
Add initial pinctrl driver to support pin configuration with
pinctrl framework for SC7180
Signed-off-by: Jitendra Sharma <shajit@codeaurora.org>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
[rnayak: modify to use upstream tile support
sort and squash some functions]
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/pinctrl/qcom/Kconfig | 9 +
drivers/pinctrl/qcom/Makefile | 1 +
drivers/pinctrl/qcom/pinctrl-sc7180.c | 1146 +++++++++++++++++++++++++
3 files changed, 1156 insertions(+)
create mode 100644 drivers/pinctrl/qcom/pinctrl-sc7180.c
diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index 8e14a5f2e970..af44dafc35e7 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -158,6 +158,15 @@ config PINCTRL_QCOM_SSBI_PMIC
which are using SSBI for communication with SoC. Example PMIC's
devices are pm8058 and pm8921.
+config PINCTRL_SC7180
+ tristate "Qualcomm Technologies Inc SC7180 pin controller driver"
+ depends on GPIOLIB && OF
+ select PINCTRL_MSM
+ help
+ This is the pinctrl, pinmux, pinconf and gpiolib driver for the
+ Qualcomm Technologies Inc TLMM block found on the Qualcomm
+ Technologies Inc SC7180 platform.
+
config PINCTRL_SDM660
tristate "Qualcomm Technologies Inc SDM660 pin controller driver"
depends on GPIOLIB && OF
diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
index ebe906872272..f8bb0c265381 100644
--- a/drivers/pinctrl/qcom/Makefile
+++ b/drivers/pinctrl/qcom/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_PINCTRL_QCOM_SPMI_PMIC) += pinctrl-spmi-gpio.o
obj-$(CONFIG_PINCTRL_QCOM_SPMI_PMIC) += pinctrl-spmi-mpp.o
obj-$(CONFIG_PINCTRL_QCOM_SSBI_PMIC) += pinctrl-ssbi-gpio.o
obj-$(CONFIG_PINCTRL_QCOM_SSBI_PMIC) += pinctrl-ssbi-mpp.o
+obj-$(CONFIG_PINCTRL_SC7180) += pinctrl-sc7180.o
obj-$(CONFIG_PINCTRL_SDM660) += pinctrl-sdm660.o
obj-$(CONFIG_PINCTRL_SDM845) += pinctrl-sdm845.o
obj-$(CONFIG_PINCTRL_SM8150) += pinctrl-sm8150.o
diff --git a/drivers/pinctrl/qcom/pinctrl-sc7180.c b/drivers/pinctrl/qcom/pinctrl-sc7180.c
new file mode 100644
index 000000000000..6399c8a2bc22
--- /dev/null
+++ b/drivers/pinctrl/qcom/pinctrl-sc7180.c
@@ -0,0 +1,1146 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2019, The Linux Foundation. All rights reserved.
+
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-msm.h"
+
+static const char * const sc7180_tiles[] = {
+ "north",
+ "south",
+ "west",
+};
+
+enum {
+ NORTH,
+ SOUTH,
+ WEST
+};
+
+#define FUNCTION(fname) \
+ [msm_mux_##fname] = { \
+ .name = #fname, \
+ .groups = fname##_groups, \
+ .ngroups = ARRAY_SIZE(fname##_groups), \
+ }
+
+#define PINGROUP(id, _tile, f1, f2, f3, f4, f5, f6, f7, f8, f9) \
+ { \
+ .name = "gpio" #id, \
+ .pins = gpio##id##_pins, \
+ .npins = ARRAY_SIZE(gpio##id##_pins), \
+ .funcs = (int[]){ \
+ msm_mux_gpio, /* gpio mode */ \
+ msm_mux_##f1, \
+ msm_mux_##f2, \
+ msm_mux_##f3, \
+ msm_mux_##f4, \
+ msm_mux_##f5, \
+ msm_mux_##f6, \
+ msm_mux_##f7, \
+ msm_mux_##f8, \
+ msm_mux_##f9 \
+ }, \
+ .nfuncs = 10, \
+ .ctl_reg = 0x1000 * id, \
+ .io_reg = 0x1000 * id + 0x4, \
+ .intr_cfg_reg = 0x1000 * id + 0x8, \
+ .intr_status_reg = 0x1000 * id + 0xc, \
+ .intr_target_reg = 0x1000 * id + 0x8, \
+ .tile = _tile, \
+ .mux_bit = 2, \
+ .pull_bit = 0, \
+ .drv_bit = 6, \
+ .oe_bit = 9, \
+ .in_bit = 0, \
+ .out_bit = 1, \
+ .intr_enable_bit = 0, \
+ .intr_status_bit = 0, \
+ .intr_target_bit = 5, \
+ .intr_target_kpss_val = 3, \
+ .intr_raw_status_bit = 4, \
+ .intr_polarity_bit = 1, \
+ .intr_detection_bit = 2, \
+ .intr_detection_width = 2, \
+ }
+
+#define SDC_QDSD_PINGROUP(pg_name, ctl, pull, drv) \
+ { \
+ .name = #pg_name, \
+ .pins = pg_name##_pins, \
+ .npins = ARRAY_SIZE(pg_name##_pins), \
+ .ctl_reg = ctl, \
+ .io_reg = 0, \
+ .intr_cfg_reg = 0, \
+ .intr_status_reg = 0, \
+ .intr_target_reg = 0, \
+ .mux_bit = -1, \
+ .pull_bit = pull, \
+ .drv_bit = drv, \
+ .oe_bit = -1, \
+ .in_bit = -1, \
+ .out_bit = -1, \
+ .intr_enable_bit = -1, \
+ .intr_status_bit = -1, \
+ .intr_target_bit = -1, \
+ .intr_raw_status_bit = -1, \
+ .intr_polarity_bit = -1, \
+ .intr_detection_bit = -1, \
+ .intr_detection_width = -1, \
+ }
+
+#define UFS_RESET(pg_name, offset) \
+ { \
+ .name = #pg_name, \
+ .pins = pg_name##_pins, \
+ .npins = ARRAY_SIZE(pg_name##_pins), \
+ .ctl_reg = offset, \
+ .io_reg = offset + 0x4, \
+ .intr_cfg_reg = 0, \
+ .intr_status_reg = 0, \
+ .intr_target_reg = 0, \
+ .mux_bit = -1, \
+ .pull_bit = 3, \
+ .drv_bit = 0, \
+ .oe_bit = -1, \
+ .in_bit = -1, \
+ .out_bit = 0, \
+ .intr_enable_bit = -1, \
+ .intr_status_bit = -1, \
+ .intr_target_bit = -1, \
+ .intr_raw_status_bit = -1, \
+ .intr_polarity_bit = -1, \
+ .intr_detection_bit = -1, \
+ .intr_detection_width = -1, \
+ }
+static const struct pinctrl_pin_desc sc7180_pins[] = {
+ PINCTRL_PIN(0, "GPIO_0"),
+ PINCTRL_PIN(1, "GPIO_1"),
+ PINCTRL_PIN(2, "GPIO_2"),
+ PINCTRL_PIN(3, "GPIO_3"),
+ PINCTRL_PIN(4, "GPIO_4"),
+ PINCTRL_PIN(5, "GPIO_5"),
+ PINCTRL_PIN(6, "GPIO_6"),
+ PINCTRL_PIN(7, "GPIO_7"),
+ PINCTRL_PIN(8, "GPIO_8"),
+ PINCTRL_PIN(9, "GPIO_9"),
+ PINCTRL_PIN(10, "GPIO_10"),
+ PINCTRL_PIN(11, "GPIO_11"),
+ PINCTRL_PIN(12, "GPIO_12"),
+ PINCTRL_PIN(13, "GPIO_13"),
+ PINCTRL_PIN(14, "GPIO_14"),
+ PINCTRL_PIN(15, "GPIO_15"),
+ PINCTRL_PIN(16, "GPIO_16"),
+ PINCTRL_PIN(17, "GPIO_17"),
+ PINCTRL_PIN(18, "GPIO_18"),
+ PINCTRL_PIN(19, "GPIO_19"),
+ PINCTRL_PIN(20, "GPIO_20"),
+ PINCTRL_PIN(21, "GPIO_21"),
+ PINCTRL_PIN(22, "GPIO_22"),
+ PINCTRL_PIN(23, "GPIO_23"),
+ PINCTRL_PIN(24, "GPIO_24"),
+ PINCTRL_PIN(25, "GPIO_25"),
+ PINCTRL_PIN(26, "GPIO_26"),
+ PINCTRL_PIN(27, "GPIO_27"),
+ PINCTRL_PIN(28, "GPIO_28"),
+ PINCTRL_PIN(29, "GPIO_29"),
+ PINCTRL_PIN(30, "GPIO_30"),
+ PINCTRL_PIN(31, "GPIO_31"),
+ PINCTRL_PIN(32, "GPIO_32"),
+ PINCTRL_PIN(33, "GPIO_33"),
+ PINCTRL_PIN(34, "GPIO_34"),
+ PINCTRL_PIN(35, "GPIO_35"),
+ PINCTRL_PIN(36, "GPIO_36"),
+ PINCTRL_PIN(37, "GPIO_37"),
+ PINCTRL_PIN(38, "GPIO_38"),
+ PINCTRL_PIN(39, "GPIO_39"),
+ PINCTRL_PIN(40, "GPIO_40"),
+ PINCTRL_PIN(41, "GPIO_41"),
+ PINCTRL_PIN(42, "GPIO_42"),
+ PINCTRL_PIN(43, "GPIO_43"),
+ PINCTRL_PIN(44, "GPIO_44"),
+ PINCTRL_PIN(45, "GPIO_45"),
+ PINCTRL_PIN(46, "GPIO_46"),
+ PINCTRL_PIN(47, "GPIO_47"),
+ PINCTRL_PIN(48, "GPIO_48"),
+ PINCTRL_PIN(49, "GPIO_49"),
+ PINCTRL_PIN(50, "GPIO_50"),
+ PINCTRL_PIN(51, "GPIO_51"),
+ PINCTRL_PIN(52, "GPIO_52"),
+ PINCTRL_PIN(53, "GPIO_53"),
+ PINCTRL_PIN(54, "GPIO_54"),
+ PINCTRL_PIN(55, "GPIO_55"),
+ PINCTRL_PIN(56, "GPIO_56"),
+ PINCTRL_PIN(57, "GPIO_57"),
+ PINCTRL_PIN(58, "GPIO_58"),
+ PINCTRL_PIN(59, "GPIO_59"),
+ PINCTRL_PIN(60, "GPIO_60"),
+ PINCTRL_PIN(61, "GPIO_61"),
+ PINCTRL_PIN(62, "GPIO_62"),
+ PINCTRL_PIN(63, "GPIO_63"),
+ PINCTRL_PIN(64, "GPIO_64"),
+ PINCTRL_PIN(65, "GPIO_65"),
+ PINCTRL_PIN(66, "GPIO_66"),
+ PINCTRL_PIN(67, "GPIO_67"),
+ PINCTRL_PIN(68, "GPIO_68"),
+ PINCTRL_PIN(69, "GPIO_69"),
+ PINCTRL_PIN(70, "GPIO_70"),
+ PINCTRL_PIN(71, "GPIO_71"),
+ PINCTRL_PIN(72, "GPIO_72"),
+ PINCTRL_PIN(73, "GPIO_73"),
+ PINCTRL_PIN(74, "GPIO_74"),
+ PINCTRL_PIN(75, "GPIO_75"),
+ PINCTRL_PIN(76, "GPIO_76"),
+ PINCTRL_PIN(77, "GPIO_77"),
+ PINCTRL_PIN(78, "GPIO_78"),
+ PINCTRL_PIN(79, "GPIO_79"),
+ PINCTRL_PIN(80, "GPIO_80"),
+ PINCTRL_PIN(81, "GPIO_81"),
+ PINCTRL_PIN(82, "GPIO_82"),
+ PINCTRL_PIN(83, "GPIO_83"),
+ PINCTRL_PIN(84, "GPIO_84"),
+ PINCTRL_PIN(85, "GPIO_85"),
+ PINCTRL_PIN(86, "GPIO_86"),
+ PINCTRL_PIN(87, "GPIO_87"),
+ PINCTRL_PIN(88, "GPIO_88"),
+ PINCTRL_PIN(89, "GPIO_89"),
+ PINCTRL_PIN(90, "GPIO_90"),
+ PINCTRL_PIN(91, "GPIO_91"),
+ PINCTRL_PIN(92, "GPIO_92"),
+ PINCTRL_PIN(93, "GPIO_93"),
+ PINCTRL_PIN(94, "GPIO_94"),
+ PINCTRL_PIN(95, "GPIO_95"),
+ PINCTRL_PIN(96, "GPIO_96"),
+ PINCTRL_PIN(97, "GPIO_97"),
+ PINCTRL_PIN(98, "GPIO_98"),
+ PINCTRL_PIN(99, "GPIO_99"),
+ PINCTRL_PIN(100, "GPIO_100"),
+ PINCTRL_PIN(101, "GPIO_101"),
+ PINCTRL_PIN(102, "GPIO_102"),
+ PINCTRL_PIN(103, "GPIO_103"),
+ PINCTRL_PIN(104, "GPIO_104"),
+ PINCTRL_PIN(105, "GPIO_105"),
+ PINCTRL_PIN(106, "GPIO_106"),
+ PINCTRL_PIN(107, "GPIO_107"),
+ PINCTRL_PIN(108, "GPIO_108"),
+ PINCTRL_PIN(109, "GPIO_109"),
+ PINCTRL_PIN(110, "GPIO_110"),
+ PINCTRL_PIN(111, "GPIO_111"),
+ PINCTRL_PIN(112, "GPIO_112"),
+ PINCTRL_PIN(113, "GPIO_113"),
+ PINCTRL_PIN(114, "GPIO_114"),
+ PINCTRL_PIN(115, "GPIO_115"),
+ PINCTRL_PIN(116, "GPIO_116"),
+ PINCTRL_PIN(117, "GPIO_117"),
+ PINCTRL_PIN(118, "GPIO_118"),
+ PINCTRL_PIN(119, "UFS_RESET"),
+ PINCTRL_PIN(120, "SDC1_RCLK"),
+ PINCTRL_PIN(121, "SDC1_CLK"),
+ PINCTRL_PIN(122, "SDC1_CMD"),
+ PINCTRL_PIN(123, "SDC1_DATA"),
+ PINCTRL_PIN(124, "SDC2_CLK"),
+ PINCTRL_PIN(125, "SDC2_CMD"),
+ PINCTRL_PIN(126, "SDC2_DATA"),
+};
+
+#define DECLARE_MSM_GPIO_PINS(pin) \
+ static const unsigned int gpio##pin##_pins[] = { pin }
+DECLARE_MSM_GPIO_PINS(0);
+DECLARE_MSM_GPIO_PINS(1);
+DECLARE_MSM_GPIO_PINS(2);
+DECLARE_MSM_GPIO_PINS(3);
+DECLARE_MSM_GPIO_PINS(4);
+DECLARE_MSM_GPIO_PINS(5);
+DECLARE_MSM_GPIO_PINS(6);
+DECLARE_MSM_GPIO_PINS(7);
+DECLARE_MSM_GPIO_PINS(8);
+DECLARE_MSM_GPIO_PINS(9);
+DECLARE_MSM_GPIO_PINS(10);
+DECLARE_MSM_GPIO_PINS(11);
+DECLARE_MSM_GPIO_PINS(12);
+DECLARE_MSM_GPIO_PINS(13);
+DECLARE_MSM_GPIO_PINS(14);
+DECLARE_MSM_GPIO_PINS(15);
+DECLARE_MSM_GPIO_PINS(16);
+DECLARE_MSM_GPIO_PINS(17);
+DECLARE_MSM_GPIO_PINS(18);
+DECLARE_MSM_GPIO_PINS(19);
+DECLARE_MSM_GPIO_PINS(20);
+DECLARE_MSM_GPIO_PINS(21);
+DECLARE_MSM_GPIO_PINS(22);
+DECLARE_MSM_GPIO_PINS(23);
+DECLARE_MSM_GPIO_PINS(24);
+DECLARE_MSM_GPIO_PINS(25);
+DECLARE_MSM_GPIO_PINS(26);
+DECLARE_MSM_GPIO_PINS(27);
+DECLARE_MSM_GPIO_PINS(28);
+DECLARE_MSM_GPIO_PINS(29);
+DECLARE_MSM_GPIO_PINS(30);
+DECLARE_MSM_GPIO_PINS(31);
+DECLARE_MSM_GPIO_PINS(32);
+DECLARE_MSM_GPIO_PINS(33);
+DECLARE_MSM_GPIO_PINS(34);
+DECLARE_MSM_GPIO_PINS(35);
+DECLARE_MSM_GPIO_PINS(36);
+DECLARE_MSM_GPIO_PINS(37);
+DECLARE_MSM_GPIO_PINS(38);
+DECLARE_MSM_GPIO_PINS(39);
+DECLARE_MSM_GPIO_PINS(40);
+DECLARE_MSM_GPIO_PINS(41);
+DECLARE_MSM_GPIO_PINS(42);
+DECLARE_MSM_GPIO_PINS(43);
+DECLARE_MSM_GPIO_PINS(44);
+DECLARE_MSM_GPIO_PINS(45);
+DECLARE_MSM_GPIO_PINS(46);
+DECLARE_MSM_GPIO_PINS(47);
+DECLARE_MSM_GPIO_PINS(48);
+DECLARE_MSM_GPIO_PINS(49);
+DECLARE_MSM_GPIO_PINS(50);
+DECLARE_MSM_GPIO_PINS(51);
+DECLARE_MSM_GPIO_PINS(52);
+DECLARE_MSM_GPIO_PINS(53);
+DECLARE_MSM_GPIO_PINS(54);
+DECLARE_MSM_GPIO_PINS(55);
+DECLARE_MSM_GPIO_PINS(56);
+DECLARE_MSM_GPIO_PINS(57);
+DECLARE_MSM_GPIO_PINS(58);
+DECLARE_MSM_GPIO_PINS(59);
+DECLARE_MSM_GPIO_PINS(60);
+DECLARE_MSM_GPIO_PINS(61);
+DECLARE_MSM_GPIO_PINS(62);
+DECLARE_MSM_GPIO_PINS(63);
+DECLARE_MSM_GPIO_PINS(64);
+DECLARE_MSM_GPIO_PINS(65);
+DECLARE_MSM_GPIO_PINS(66);
+DECLARE_MSM_GPIO_PINS(67);
+DECLARE_MSM_GPIO_PINS(68);
+DECLARE_MSM_GPIO_PINS(69);
+DECLARE_MSM_GPIO_PINS(70);
+DECLARE_MSM_GPIO_PINS(71);
+DECLARE_MSM_GPIO_PINS(72);
+DECLARE_MSM_GPIO_PINS(73);
+DECLARE_MSM_GPIO_PINS(74);
+DECLARE_MSM_GPIO_PINS(75);
+DECLARE_MSM_GPIO_PINS(76);
+DECLARE_MSM_GPIO_PINS(77);
+DECLARE_MSM_GPIO_PINS(78);
+DECLARE_MSM_GPIO_PINS(79);
+DECLARE_MSM_GPIO_PINS(80);
+DECLARE_MSM_GPIO_PINS(81);
+DECLARE_MSM_GPIO_PINS(82);
+DECLARE_MSM_GPIO_PINS(83);
+DECLARE_MSM_GPIO_PINS(84);
+DECLARE_MSM_GPIO_PINS(85);
+DECLARE_MSM_GPIO_PINS(86);
+DECLARE_MSM_GPIO_PINS(87);
+DECLARE_MSM_GPIO_PINS(88);
+DECLARE_MSM_GPIO_PINS(89);
+DECLARE_MSM_GPIO_PINS(90);
+DECLARE_MSM_GPIO_PINS(91);
+DECLARE_MSM_GPIO_PINS(92);
+DECLARE_MSM_GPIO_PINS(93);
+DECLARE_MSM_GPIO_PINS(94);
+DECLARE_MSM_GPIO_PINS(95);
+DECLARE_MSM_GPIO_PINS(96);
+DECLARE_MSM_GPIO_PINS(97);
+DECLARE_MSM_GPIO_PINS(98);
+DECLARE_MSM_GPIO_PINS(99);
+DECLARE_MSM_GPIO_PINS(100);
+DECLARE_MSM_GPIO_PINS(101);
+DECLARE_MSM_GPIO_PINS(102);
+DECLARE_MSM_GPIO_PINS(103);
+DECLARE_MSM_GPIO_PINS(104);
+DECLARE_MSM_GPIO_PINS(105);
+DECLARE_MSM_GPIO_PINS(106);
+DECLARE_MSM_GPIO_PINS(107);
+DECLARE_MSM_GPIO_PINS(108);
+DECLARE_MSM_GPIO_PINS(109);
+DECLARE_MSM_GPIO_PINS(110);
+DECLARE_MSM_GPIO_PINS(111);
+DECLARE_MSM_GPIO_PINS(112);
+DECLARE_MSM_GPIO_PINS(113);
+DECLARE_MSM_GPIO_PINS(114);
+DECLARE_MSM_GPIO_PINS(115);
+DECLARE_MSM_GPIO_PINS(116);
+DECLARE_MSM_GPIO_PINS(117);
+DECLARE_MSM_GPIO_PINS(118);
+
+static const unsigned int ufs_reset_pins[] = { 119 };
+static const unsigned int sdc1_rclk_pins[] = { 120 };
+static const unsigned int sdc1_clk_pins[] = { 121 };
+static const unsigned int sdc1_cmd_pins[] = { 122 };
+static const unsigned int sdc1_data_pins[] = { 123 };
+static const unsigned int sdc2_clk_pins[] = { 124 };
+static const unsigned int sdc2_cmd_pins[] = { 125 };
+static const unsigned int sdc2_data_pins[] = { 126 };
+
+enum sc7180_functions {
+ msm_mux_adsp_ext,
+ msm_mux_agera_pll,
+ msm_mux_aoss_cti,
+ msm_mux_atest_char,
+ msm_mux_atest_char0,
+ msm_mux_atest_char1,
+ msm_mux_atest_char2,
+ msm_mux_atest_char3,
+ msm_mux_atest_tsens,
+ msm_mux_atest_tsens2,
+ msm_mux_atest_usb1,
+ msm_mux_atest_usb2,
+ msm_mux_atest_usb10,
+ msm_mux_atest_usb11,
+ msm_mux_atest_usb12,
+ msm_mux_atest_usb13,
+ msm_mux_atest_usb20,
+ msm_mux_atest_usb21,
+ msm_mux_atest_usb22,
+ msm_mux_atest_usb23,
+ msm_mux_audio_ref,
+ msm_mux_btfm_slimbus,
+ msm_mux_cam_mclk,
+ msm_mux_cci_async,
+ msm_mux_cci_i2c,
+ msm_mux_cci_timer0,
+ msm_mux_cci_timer1,
+ msm_mux_cci_timer2,
+ msm_mux_cci_timer3,
+ msm_mux_cci_timer4,
+ msm_mux_cri_trng,
+ msm_mux_dbg_out,
+ msm_mux_ddr_bist,
+ msm_mux_ddr_pxi0,
+ msm_mux_ddr_pxi1,
+ msm_mux_ddr_pxi2,
+ msm_mux_ddr_pxi3,
+ msm_mux_dp_hot,
+ msm_mux_edp_lcd,
+ msm_mux_gcc_gp1,
+ msm_mux_gcc_gp2,
+ msm_mux_gcc_gp3,
+ msm_mux_gpio,
+ msm_mux_gp_pdm0,
+ msm_mux_gp_pdm1,
+ msm_mux_gp_pdm2,
+ msm_mux_gps_tx,
+ msm_mux_jitter_bist,
+ msm_mux_ldo_en,
+ msm_mux_ldo_update,
+ msm_mux_lpass_ext,
+ msm_mux_mdp_vsync,
+ msm_mux_mdp_vsync0,
+ msm_mux_mdp_vsync1,
+ msm_mux_mdp_vsync2,
+ msm_mux_mdp_vsync3,
+ msm_mux_mi2s_1,
+ msm_mux_mi2s_0,
+ msm_mux_mi2s_2,
+ msm_mux_mss_lte,
+ msm_mux_m_voc,
+ msm_mux_pa_indicator,
+ msm_mux_phase_flag,
+ msm_mux_PLL_BIST,
+ msm_mux_pll_bypassnl,
+ msm_mux_pll_reset,
+ msm_mux_prng_rosc,
+ msm_mux_qdss,
+ msm_mux_qdss_cti,
+ msm_mux_qlink_enable,
+ msm_mux_qlink_request,
+ msm_mux_qspi_clk,
+ msm_mux_qspi_cs,
+ msm_mux_qspi_data,
+ msm_mux_qup00,
+ msm_mux_qup01,
+ msm_mux_qup02,
+ msm_mux_qup03,
+ msm_mux_qup04,
+ msm_mux_qup05,
+ msm_mux_qup10,
+ msm_mux_qup11,
+ msm_mux_qup12,
+ msm_mux_qup13,
+ msm_mux_qup14,
+ msm_mux_qup15,
+ msm_mux_sdc1_tb,
+ msm_mux_sdc2_tb,
+ msm_mux_sd_write,
+ msm_mux_sp_cmu,
+ msm_mux_tgu_ch0,
+ msm_mux_tgu_ch1,
+ msm_mux_tgu_ch2,
+ msm_mux_tgu_ch3,
+ msm_mux_tsense_pwm1,
+ msm_mux_tsense_pwm2,
+ msm_mux_uim1,
+ msm_mux_uim2,
+ msm_mux_uim_batt,
+ msm_mux_usb_phy,
+ msm_mux_vfr_1,
+ msm_mux__V_GPIO,
+ msm_mux__V_PPS_IN,
+ msm_mux__V_PPS_OUT,
+ msm_mux_vsense_trigger,
+ msm_mux_wlan1_adc0,
+ msm_mux_wlan1_adc1,
+ msm_mux_wlan2_adc0,
+ msm_mux_wlan2_adc1,
+ msm_mux__,
+};
+
+static const char * const qup01_groups[] = {
+ "gpio0", "gpio1", "gpio2", "gpio3", "gpio12", "gpio94",
+};
+static const char * const gpio_groups[] = {
+ "gpio0", "gpio1", "gpio2", "gpio3", "gpio4", "gpio5", "gpio6", "gpio7",
+ "gpio8", "gpio9", "gpio10", "gpio11", "gpio12", "gpio13", "gpio14",
+ "gpio15", "gpio16", "gpio17", "gpio18", "gpio19", "gpio20", "gpio21",
+ "gpio22", "gpio23", "gpio24", "gpio25", "gpio26", "gpio27", "gpio28",
+ "gpio29", "gpio30", "gpio31", "gpio32", "gpio33", "gpio34", "gpio35",
+ "gpio36", "gpio37", "gpio38", "gpio39", "gpio40", "gpio41", "gpio42",
+ "gpio43", "gpio44", "gpio45", "gpio46", "gpio47", "gpio48", "gpio49",
+ "gpio50", "gpio51", "gpio52", "gpio53", "gpio54", "gpio55", "gpio56",
+ "gpio57", "gpio58", "gpio59", "gpio60", "gpio61", "gpio62", "gpio63",
+ "gpio64", "gpio65", "gpio66", "gpio67", "gpio68", "gpio69", "gpio70",
+ "gpio71", "gpio72", "gpio73", "gpio74", "gpio75", "gpio76", "gpio77",
+ "gpio78", "gpio79", "gpio80", "gpio81", "gpio82", "gpio83", "gpio84",
+ "gpio85", "gpio86", "gpio87", "gpio88", "gpio89", "gpio90", "gpio91",
+ "gpio92", "gpio93", "gpio94", "gpio95", "gpio96", "gpio97", "gpio98",
+ "gpio99", "gpio100", "gpio101", "gpio102", "gpio103", "gpio104",
+ "gpio105", "gpio106", "gpio107", "gpio108", "gpio109", "gpio110",
+ "gpio111", "gpio112", "gpio113", "gpio114", "gpio115", "gpio116",
+ "gpio117", "gpio118",
+};
+static const char * const phase_flag_groups[] = {
+ "gpio0", "gpio1", "gpio2", "gpio8", "gpio9",
+ "gpio11", "gpio12", "gpio17", "gpio18", "gpio19",
+ "gpio20", "gpio25", "gpio26", "gpio27", "gpio28",
+ "gpio32", "gpio33", "gpio34", "gpio35", "gpio36",
+ "gpio37", "gpio38", "gpio39", "gpio42", "gpio44",
+ "gpio56", "gpio57", "gpio58", "gpio63", "gpio64",
+ "gpio108", "gpio109",
+};
+static const char * const cri_trng_groups[] = {
+ "gpio0", "gpio1", "gpio2",
+};
+static const char * const sp_cmu_groups[] = {
+ "gpio3",
+};
+static const char * const dbg_out_groups[] = {
+ "gpio3",
+};
+static const char * const qdss_cti_groups[] = {
+ "gpio3", "gpio4", "gpio8", "gpio9", "gpio33", "gpio44", "gpio45",
+ "gpio72",
+};
+static const char * const sdc1_tb_groups[] = {
+ "gpio4",
+};
+static const char * const sdc2_tb_groups[] = {
+ "gpio5",
+};
+static const char * const qup11_groups[] = {
+ "gpio6", "gpio7",
+};
+static const char * const ddr_bist_groups[] = {
+ "gpio7", "gpio8", "gpio9", "gpio10",
+};
+static const char * const gp_pdm1_groups[] = {
+ "gpio8", "gpio50",
+};
+static const char * const mdp_vsync_groups[] = {
+ "gpio10", "gpio11", "gpio12", "gpio70", "gpio71",
+};
+static const char * const edp_lcd_groups[] = {
+ "gpio11",
+};
+static const char * const ddr_pxi2_groups[] = {
+ "gpio11", "gpio26",
+};
+static const char * const m_voc_groups[] = {
+ "gpio12",
+};
+static const char * const wlan2_adc0_groups[] = {
+ "gpio12",
+};
+static const char * const atest_usb10_groups[] = {
+ "gpio12",
+};
+static const char * const ddr_pxi3_groups[] = {
+ "gpio12", "gpio108",
+};
+static const char * const cam_mclk_groups[] = {
+ "gpio13", "gpio14", "gpio15", "gpio16", "gpio23",
+};
+static const char * const pll_bypassnl_groups[] = {
+ "gpio13",
+};
+static const char * const qdss_groups[] = {
+ "gpio13", "gpio86", "gpio14", "gpio87",
+ "gpio15", "gpio88", "gpio16", "gpio89",
+ "gpio17", "gpio90", "gpio18", "gpio91",
+ "gpio19", "gpio21", "gpio20", "gpio22",
+ "gpio23", "gpio54", "gpio24", "gpio36",
+ "gpio25", "gpio57", "gpio26", "gpio31",
+ "gpio27", "gpio56", "gpio28", "gpio29",
+ "gpio30", "gpio35", "gpio93", "gpio104",
+ "gpio34", "gpio53", "gpio37", "gpio55",
+};
+static const char * const pll_reset_groups[] = {
+ "gpio14",
+};
+static const char * const qup02_groups[] = {
+ "gpio15", "gpio16",
+};
+static const char * const cci_i2c_groups[] = {
+ "gpio17", "gpio18", "gpio19", "gpio20", "gpio27", "gpio28",
+};
+static const char * const wlan1_adc0_groups[] = {
+ "gpio17",
+};
+static const char * const atest_usb12_groups[] = {
+ "gpio17",
+};
+static const char * const ddr_pxi1_groups[] = {
+ "gpio17", "gpio44",
+};
+static const char * const atest_char_groups[] = {
+ "gpio17",
+};
+static const char * const agera_pll_groups[] = {
+ "gpio18",
+};
+static const char * const vsense_trigger_groups[] = {
+ "gpio18",
+};
+static const char * const ddr_pxi0_groups[] = {
+ "gpio18", "gpio27",
+};
+static const char * const atest_char3_groups[] = {
+ "gpio18",
+};
+static const char * const atest_char2_groups[] = {
+ "gpio19",
+};
+static const char * const atest_char1_groups[] = {
+ "gpio20",
+};
+static const char * const cci_timer0_groups[] = {
+ "gpio21",
+};
+static const char * const gcc_gp2_groups[] = {
+ "gpio21",
+};
+static const char * const atest_char0_groups[] = {
+ "gpio21",
+};
+static const char * const cci_timer1_groups[] = {
+ "gpio22",
+};
+static const char * const gcc_gp3_groups[] = {
+ "gpio22",
+};
+static const char * const cci_timer2_groups[] = {
+ "gpio23",
+};
+static const char * const cci_timer3_groups[] = {
+ "gpio24",
+};
+static const char * const cci_async_groups[] = {
+ "gpio24", "gpio25", "gpio26",
+};
+static const char * const cci_timer4_groups[] = {
+ "gpio25",
+};
+static const char * const qup05_groups[] = {
+ "gpio25", "gpio26", "gpio27", "gpio28",
+};
+static const char * const atest_tsens_groups[] = {
+ "gpio26",
+};
+static const char * const atest_usb11_groups[] = {
+ "gpio26",
+};
+static const char * const PLL_BIST_groups[] = {
+ "gpio27",
+};
+static const char * const sd_write_groups[] = {
+ "gpio33",
+};
+static const char * const qup00_groups[] = {
+ "gpio34", "gpio35", "gpio36", "gpio37",
+};
+static const char * const gp_pdm0_groups[] = {
+ "gpio37", "gpio68",
+};
+static const char * const qup03_groups[] = {
+ "gpio38", "gpio39", "gpio40", "gpio41",
+};
+static const char * const atest_tsens2_groups[] = {
+ "gpio39",
+};
+static const char * const wlan2_adc1_groups[] = {
+ "gpio39",
+};
+static const char * const atest_usb1_groups[] = {
+ "gpio39",
+};
+static const char * const qup12_groups[] = {
+ "gpio42", "gpio43", "gpio44", "gpio45",
+};
+static const char * const wlan1_adc1_groups[] = {
+ "gpio44",
+};
+static const char * const atest_usb13_groups[] = {
+ "gpio44",
+};
+static const char * const qup13_groups[] = {
+ "gpio46", "gpio47",
+};
+static const char * const gcc_gp1_groups[] = {
+ "gpio48", "gpio56",
+};
+static const char * const mi2s_1_groups[] = {
+ "gpio49", "gpio50", "gpio51", "gpio52",
+};
+static const char * const btfm_slimbus_groups[] = {
+ "gpio49", "gpio50", "gpio51", "gpio52",
+};
+static const char * const atest_usb2_groups[] = {
+ "gpio51",
+};
+static const char * const atest_usb23_groups[] = {
+ "gpio52",
+};
+static const char * const mi2s_0_groups[] = {
+ "gpio53", "gpio54", "gpio55", "gpio56",
+};
+static const char * const qup15_groups[] = {
+ "gpio53", "gpio54", "gpio55", "gpio56",
+};
+static const char * const atest_usb22_groups[] = {
+ "gpio53",
+};
+static const char * const atest_usb21_groups[] = {
+ "gpio54",
+};
+static const char * const atest_usb20_groups[] = {
+ "gpio55",
+};
+static const char * const lpass_ext_groups[] = {
+ "gpio57", "gpio58",
+};
+static const char * const audio_ref_groups[] = {
+ "gpio57",
+};
+static const char * const jitter_bist_groups[] = {
+ "gpio57",
+};
+static const char * const gp_pdm2_groups[] = {
+ "gpio57",
+};
+static const char * const qup10_groups[] = {
+ "gpio59", "gpio60", "gpio61", "gpio62", "gpio68", "gpio72",
+};
+static const char * const tgu_ch3_groups[] = {
+ "gpio62",
+};
+static const char * const qspi_clk_groups[] = {
+ "gpio63",
+};
+static const char * const mdp_vsync0_groups[] = {
+ "gpio63",
+};
+static const char * const mi2s_2_groups[] = {
+ "gpio63", "gpio64", "gpio65", "gpio66",
+};
+static const char * const mdp_vsync1_groups[] = {
+ "gpio63",
+};
+static const char * const mdp_vsync2_groups[] = {
+ "gpio63",
+};
+static const char * const mdp_vsync3_groups[] = {
+ "gpio63",
+};
+static const char * const tgu_ch0_groups[] = {
+ "gpio63",
+};
+static const char * const qspi_data_groups[] = {
+ "gpio64", "gpio65", "gpio66", "gpio67",
+};
+static const char * const tgu_ch1_groups[] = {
+ "gpio64",
+};
+static const char * const vfr_1_groups[] = {
+ "gpio65",
+};
+static const char * const tgu_ch2_groups[] = {
+ "gpio65",
+};
+static const char * const qspi_cs_groups[] = {
+ "gpio68", "gpio72",
+};
+static const char * const ldo_en_groups[] = {
+ "gpio70",
+};
+static const char * const ldo_update_groups[] = {
+ "gpio71",
+};
+static const char * const prng_rosc_groups[] = {
+ "gpio72",
+};
+static const char * const uim2_groups[] = {
+ "gpio75", "gpio76", "gpio77", "gpio78",
+};
+static const char * const uim1_groups[] = {
+ "gpio79", "gpio80", "gpio81", "gpio82",
+};
+static const char * const _V_GPIO_groups[] = {
+ "gpio83", "gpio84", "gpio107",
+};
+static const char * const _V_PPS_IN_groups[] = {
+ "gpio83", "gpio84", "gpio107",
+};
+static const char * const _V_PPS_OUT_groups[] = {
+ "gpio83", "gpio84", "gpio107",
+};
+static const char * const gps_tx_groups[] = {
+ "gpio83", "gpio84", "gpio107", "gpio109",
+};
+static const char * const uim_batt_groups[] = {
+ "gpio85",
+};
+static const char * const dp_hot_groups[] = {
+ "gpio85", "gpio117",
+};
+static const char * const aoss_cti_groups[] = {
+ "gpio85",
+};
+static const char * const qup14_groups[] = {
+ "gpio86", "gpio87", "gpio88", "gpio89", "gpio90", "gpio91",
+};
+static const char * const adsp_ext_groups[] = {
+ "gpio87",
+};
+static const char * const tsense_pwm1_groups[] = {
+ "gpio88",
+};
+static const char * const tsense_pwm2_groups[] = {
+ "gpio88",
+};
+static const char * const qlink_request_groups[] = {
+ "gpio96",
+};
+static const char * const qlink_enable_groups[] = {
+ "gpio97",
+};
+static const char * const pa_indicator_groups[] = {
+ "gpio99",
+};
+static const char * const usb_phy_groups[] = {
+ "gpio104",
+};
+static const char * const mss_lte_groups[] = {
+ "gpio108", "gpio109",
+};
+static const char * const qup04_groups[] = {
+ "gpio115", "gpio116",
+};
+
+static const struct msm_function sc7180_functions[] = {
+ FUNCTION(adsp_ext),
+ FUNCTION(agera_pll),
+ FUNCTION(aoss_cti),
+ FUNCTION(atest_char),
+ FUNCTION(atest_char0),
+ FUNCTION(atest_char1),
+ FUNCTION(atest_char2),
+ FUNCTION(atest_char3),
+ FUNCTION(atest_tsens),
+ FUNCTION(atest_tsens2),
+ FUNCTION(atest_usb1),
+ FUNCTION(atest_usb2),
+ FUNCTION(atest_usb10),
+ FUNCTION(atest_usb11),
+ FUNCTION(atest_usb12),
+ FUNCTION(atest_usb13),
+ FUNCTION(atest_usb20),
+ FUNCTION(atest_usb21),
+ FUNCTION(atest_usb22),
+ FUNCTION(atest_usb23),
+ FUNCTION(audio_ref),
+ FUNCTION(btfm_slimbus),
+ FUNCTION(cam_mclk),
+ FUNCTION(cci_async),
+ FUNCTION(cci_i2c),
+ FUNCTION(cci_timer0),
+ FUNCTION(cci_timer1),
+ FUNCTION(cci_timer2),
+ FUNCTION(cci_timer3),
+ FUNCTION(cci_timer4),
+ FUNCTION(cri_trng),
+ FUNCTION(dbg_out),
+ FUNCTION(ddr_bist),
+ FUNCTION(ddr_pxi0),
+ FUNCTION(ddr_pxi1),
+ FUNCTION(ddr_pxi2),
+ FUNCTION(ddr_pxi3),
+ FUNCTION(dp_hot),
+ FUNCTION(edp_lcd),
+ FUNCTION(gcc_gp1),
+ FUNCTION(gcc_gp2),
+ FUNCTION(gcc_gp3),
+ FUNCTION(gpio),
+ FUNCTION(gp_pdm0),
+ FUNCTION(gp_pdm1),
+ FUNCTION(gp_pdm2),
+ FUNCTION(gps_tx),
+ FUNCTION(jitter_bist),
+ FUNCTION(ldo_en),
+ FUNCTION(ldo_update),
+ FUNCTION(lpass_ext),
+ FUNCTION(mdp_vsync),
+ FUNCTION(mdp_vsync0),
+ FUNCTION(mdp_vsync1),
+ FUNCTION(mdp_vsync2),
+ FUNCTION(mdp_vsync3),
+ FUNCTION(mi2s_0),
+ FUNCTION(mi2s_1),
+ FUNCTION(mi2s_2),
+ FUNCTION(mss_lte),
+ FUNCTION(m_voc),
+ FUNCTION(pa_indicator),
+ FUNCTION(phase_flag),
+ FUNCTION(PLL_BIST),
+ FUNCTION(pll_bypassnl),
+ FUNCTION(pll_reset),
+ FUNCTION(prng_rosc),
+ FUNCTION(qdss),
+ FUNCTION(qdss_cti),
+ FUNCTION(qlink_enable),
+ FUNCTION(qlink_request),
+ FUNCTION(qspi_clk),
+ FUNCTION(qspi_cs),
+ FUNCTION(qspi_data),
+ FUNCTION(qup00),
+ FUNCTION(qup01),
+ FUNCTION(qup02),
+ FUNCTION(qup03),
+ FUNCTION(qup04),
+ FUNCTION(qup05),
+ FUNCTION(qup10),
+ FUNCTION(qup11),
+ FUNCTION(qup12),
+ FUNCTION(qup13),
+ FUNCTION(qup14),
+ FUNCTION(qup15),
+ FUNCTION(sdc1_tb),
+ FUNCTION(sdc2_tb),
+ FUNCTION(sd_write),
+ FUNCTION(sp_cmu),
+ FUNCTION(tgu_ch0),
+ FUNCTION(tgu_ch1),
+ FUNCTION(tgu_ch2),
+ FUNCTION(tgu_ch3),
+ FUNCTION(tsense_pwm1),
+ FUNCTION(tsense_pwm2),
+ FUNCTION(uim1),
+ FUNCTION(uim2),
+ FUNCTION(uim_batt),
+ FUNCTION(usb_phy),
+ FUNCTION(vfr_1),
+ FUNCTION(_V_GPIO),
+ FUNCTION(_V_PPS_IN),
+ FUNCTION(_V_PPS_OUT),
+ FUNCTION(vsense_trigger),
+ FUNCTION(wlan1_adc0),
+ FUNCTION(wlan1_adc1),
+ FUNCTION(wlan2_adc0),
+ FUNCTION(wlan2_adc1),
+};
+
+/* Every pin is maintained as a single group, and missing or non-existing pin
+ * would be maintained as dummy group to synchronize pin group index with
+ * pin descriptor registered with pinctrl core.
+ * Clients would not be able to request these dummy pin groups.
+ */
+static const struct msm_pingroup sc7180_groups[] = {
+ [0] = PINGROUP(0, SOUTH, qup01, cri_trng, _, phase_flag, _, _, _, _, _),
+ [1] = PINGROUP(1, SOUTH, qup01, cri_trng, _, phase_flag, _, _, _, _, _),
+ [2] = PINGROUP(2, SOUTH, qup01, cri_trng, _, phase_flag, _, _, _, _, _),
+ [3] = PINGROUP(3, SOUTH, qup01, sp_cmu, dbg_out, qdss_cti, _, _, _, _, _),
+ [4] = PINGROUP(4, NORTH, sdc1_tb, _, qdss_cti, _, _, _, _, _, _),
+ [5] = PINGROUP(5, NORTH, sdc2_tb, _, _, _, _, _, _, _, _),
+ [6] = PINGROUP(6, NORTH, qup11, qup11, _, _, _, _, _, _, _),
+ [7] = PINGROUP(7, NORTH, qup11, qup11, ddr_bist, _, _, _, _, _, _),
+ [8] = PINGROUP(8, NORTH, gp_pdm1, ddr_bist, _, phase_flag, qdss_cti, _, _, _, _),
+ [9] = PINGROUP(9, NORTH, ddr_bist, _, phase_flag, qdss_cti, _, _, _, _, _),
+ [10] = PINGROUP(10, NORTH, mdp_vsync, ddr_bist, _, _, _, _, _, _, _),
+ [11] = PINGROUP(11, NORTH, mdp_vsync, edp_lcd, _, phase_flag, ddr_pxi2, _, _, _, _),
+ [12] = PINGROUP(12, SOUTH, mdp_vsync, m_voc, qup01, _, phase_flag, wlan2_adc0, atest_usb10, ddr_pxi3, _),
+ [13] = PINGROUP(13, SOUTH, cam_mclk, pll_bypassnl, qdss, _, _, _, _, _, _),
+ [14] = PINGROUP(14, SOUTH, cam_mclk, pll_reset, qdss, _, _, _, _, _, _),
+ [15] = PINGROUP(15, SOUTH, cam_mclk, qup02, qup02, qdss, _, _, _, _, _),
+ [16] = PINGROUP(16, SOUTH, cam_mclk, qup02, qup02, qdss, _, _, _, _, _),
+ [17] = PINGROUP(17, SOUTH, cci_i2c, _, phase_flag, qdss, _, wlan1_adc0, atest_usb12, ddr_pxi1, atest_char),
+ [18] = PINGROUP(18, SOUTH, cci_i2c, agera_pll, _, phase_flag, qdss, vsense_trigger, ddr_pxi0, atest_char3, _),
+ [19] = PINGROUP(19, SOUTH, cci_i2c, _, phase_flag, qdss, atest_char2, _, _, _, _),
+ [20] = PINGROUP(20, SOUTH, cci_i2c, _, phase_flag, qdss, atest_char1, _, _, _, _),
+ [21] = PINGROUP(21, NORTH, cci_timer0, gcc_gp2, _, qdss, atest_char0, _, _, _, _),
+ [22] = PINGROUP(22, NORTH, cci_timer1, gcc_gp3, _, qdss, _, _, _, _, _),
+ [23] = PINGROUP(23, SOUTH, cci_timer2, cam_mclk, qdss, _, _, _, _, _, _),
+ [24] = PINGROUP(24, SOUTH, cci_timer3, cci_async, qdss, _, _, _, _, _, _),
+ [25] = PINGROUP(25, SOUTH, cci_timer4, cci_async, qup05, _, phase_flag, qdss, _, _, _),
+ [26] = PINGROUP(26, SOUTH, cci_async, qup05, _, phase_flag, qdss, atest_tsens, atest_usb11, ddr_pxi2, _),
+ [27] = PINGROUP(27, SOUTH, cci_i2c, qup05, PLL_BIST, _, phase_flag, qdss, ddr_pxi0, _, _),
+ [28] = PINGROUP(28, SOUTH, cci_i2c, qup05, _, phase_flag, qdss, _, _, _, _),
+ [29] = PINGROUP(29, NORTH, _, qdss, _, _, _, _, _, _, _),
+ [30] = PINGROUP(30, SOUTH, qdss, _, _, _, _, _, _, _, _),
+ [31] = PINGROUP(31, NORTH, _, qdss, _, _, _, _, _, _, _),
+ [32] = PINGROUP(32, NORTH, _, phase_flag, _, _, _, _, _, _, _),
+ [33] = PINGROUP(33, NORTH, sd_write, _, phase_flag, qdss_cti, _, _, _, _, _),
+ [34] = PINGROUP(34, SOUTH, qup00, _, phase_flag, qdss, _, _, _, _, _),
+ [35] = PINGROUP(35, SOUTH, qup00, _, phase_flag, qdss, _, _, _, _, _),
+ [36] = PINGROUP(36, SOUTH, qup00, _, phase_flag, qdss, _, _, _, _, _),
+ [37] = PINGROUP(37, SOUTH, qup00, gp_pdm0, _, phase_flag, qdss, _, _, _, _),
+ [38] = PINGROUP(38, SOUTH, qup03, _, phase_flag, _, _, _, _, _, _),
+ [39] = PINGROUP(39, SOUTH, qup03, _, phase_flag, atest_tsens2, wlan2_adc1, atest_usb1, _, _, _),
+ [40] = PINGROUP(40, SOUTH, qup03, _, _, _, _, _, _, _, _),
+ [41] = PINGROUP(41, SOUTH, qup03, _, _, _, _, _, _, _, _),
+ [42] = PINGROUP(42, NORTH, qup12, _, phase_flag, _, _, _, _, _, _),
+ [43] = PINGROUP(43, NORTH, qup12, _, _, _, _, _, _, _, _),
+ [44] = PINGROUP(44, NORTH, qup12, _, phase_flag, qdss_cti, wlan1_adc1, atest_usb13, ddr_pxi1, _, _),
+ [45] = PINGROUP(45, NORTH, qup12, qdss_cti, _, _, _, _, _, _, _),
+ [46] = PINGROUP(46, NORTH, qup13, qup13, _, _, _, _, _, _, _),
+ [47] = PINGROUP(47, NORTH, qup13, qup13, _, _, _, _, _, _, _),
+ [48] = PINGROUP(48, NORTH, gcc_gp1, _, _, _, _, _, _, _, _),
+ [49] = PINGROUP(49, WEST, mi2s_1, btfm_slimbus, _, _, _, _, _, _, _),
+ [50] = PINGROUP(50, WEST, mi2s_1, btfm_slimbus, gp_pdm1, _, _, _, _, _, _),
+ [51] = PINGROUP(51, WEST, mi2s_1, btfm_slimbus, atest_usb2, _, _, _, _, _, _),
+ [52] = PINGROUP(52, WEST, mi2s_1, btfm_slimbus, atest_usb23, _, _, _, _, _, _),
+ [53] = PINGROUP(53, WEST, mi2s_0, qup15, qdss, atest_usb22, _, _, _, _, _),
+ [54] = PINGROUP(54, WEST, mi2s_0, qup15, qdss, atest_usb21, _, _, _, _, _),
+ [55] = PINGROUP(55, WEST, mi2s_0, qup15, qdss, atest_usb20, _, _, _, _, _),
+ [56] = PINGROUP(56, WEST, mi2s_0, qup15, gcc_gp1, _, phase_flag, qdss, _, _, _),
+ [57] = PINGROUP(57, WEST, lpass_ext, audio_ref, jitter_bist, gp_pdm2, _, phase_flag, qdss, _, _),
+ [58] = PINGROUP(58, WEST, lpass_ext, _, phase_flag, _, _, _, _, _, _),
+ [59] = PINGROUP(59, NORTH, qup10, _, _, _, _, _, _, _, _),
+ [60] = PINGROUP(60, NORTH, qup10, _, _, _, _, _, _, _, _),
+ [61] = PINGROUP(61, NORTH, qup10, _, _, _, _, _, _, _, _),
+ [62] = PINGROUP(62, NORTH, qup10, tgu_ch3, _, _, _, _, _, _, _),
+ [63] = PINGROUP(63, NORTH, qspi_clk, mdp_vsync0, mi2s_2, mdp_vsync1, mdp_vsync2, mdp_vsync3, tgu_ch0, _, phase_flag),
+ [64] = PINGROUP(64, NORTH, qspi_data, mi2s_2, tgu_ch1, _, phase_flag, _, _, _, _),
+ [65] = PINGROUP(65, NORTH, qspi_data, mi2s_2, vfr_1, tgu_ch2, _, _, _, _, _),
+ [66] = PINGROUP(66, NORTH, qspi_data, mi2s_2, _, _, _, _, _, _, _),
+ [67] = PINGROUP(67, NORTH, qspi_data, _, _, _, _, _, _, _, _),
+ [68] = PINGROUP(68, NORTH, qspi_cs, qup10, gp_pdm0, _, _, _, _, _, _),
+ [69] = PINGROUP(69, WEST, _, _, _, _, _, _, _, _, _),
+ [70] = PINGROUP(70, NORTH, _, _, mdp_vsync, ldo_en, _, _, _, _, _),
+ [71] = PINGROUP(71, NORTH, _, mdp_vsync, ldo_update, _, _, _, _, _, _),
+ [72] = PINGROUP(72, NORTH, qspi_cs, qup10, prng_rosc, _, qdss_cti, _, _, _, _),
+ [73] = PINGROUP(73, WEST, _, _, _, _, _, _, _, _, _),
+ [74] = PINGROUP(74, WEST, _, _, _, _, _, _, _, _, _),
+ [75] = PINGROUP(75, WEST, uim2, _, _, _, _, _, _, _, _),
+ [76] = PINGROUP(76, WEST, uim2, _, _, _, _, _, _, _, _),
+ [77] = PINGROUP(77, WEST, uim2, _, _, _, _, _, _, _, _),
+ [78] = PINGROUP(78, WEST, uim2, _, _, _, _, _, _, _, _),
+ [79] = PINGROUP(79, WEST, uim1, _, _, _, _, _, _, _, _),
+ [80] = PINGROUP(80, WEST, uim1, _, _, _, _, _, _, _, _),
+ [81] = PINGROUP(81, WEST, uim1, _, _, _, _, _, _, _, _),
+ [82] = PINGROUP(82, WEST, uim1, _, _, _, _, _, _, _, _),
+ [83] = PINGROUP(83, WEST, _, _V_GPIO, _V_PPS_IN, _V_PPS_OUT, gps_tx, _, _, _, _),
+ [84] = PINGROUP(84, WEST, _, _V_GPIO, _V_PPS_IN, _V_PPS_OUT, gps_tx, _, _, _, _),
+ [85] = PINGROUP(85, WEST, uim_batt, dp_hot, aoss_cti, _, _, _, _, _, _),
+ [86] = PINGROUP(86, NORTH, qup14, qdss, _, _, _, _, _, _, _),
+ [87] = PINGROUP(87, NORTH, qup14, adsp_ext, qdss, _, _, _, _, _, _),
+ [88] = PINGROUP(88, NORTH, qup14, qdss, tsense_pwm1, tsense_pwm2, _, _, _, _, _),
+ [89] = PINGROUP(89, NORTH, qup14, qdss, _, _, _, _, _, _, _),
+ [90] = PINGROUP(90, NORTH, qup14, qdss, _, _, _, _, _, _, _),
+ [91] = PINGROUP(91, NORTH, qup14, qdss, _, _, _, _, _, _, _),
+ [92] = PINGROUP(92, NORTH, _, _, _, _, _, _, _, _, _),
+ [93] = PINGROUP(93, NORTH, qdss, _, _, _, _, _, _, _, _),
+ [94] = PINGROUP(94, SOUTH, qup01, _, _, _, _, _, _, _, _),
+ [95] = PINGROUP(95, WEST, _, _, _, _, _, _, _, _, _),
+ [96] = PINGROUP(96, WEST, qlink_request, _, _, _, _, _, _, _, _),
+ [97] = PINGROUP(97, WEST, qlink_enable, _, _, _, _, _, _, _, _),
+ [98] = PINGROUP(98, WEST, _, _, _, _, _, _, _, _, _),
+ [99] = PINGROUP(99, WEST, _, pa_indicator, _, _, _, _, _, _, _),
+ [100] = PINGROUP(100, WEST, _, _, _, _, _, _, _, _, _),
+ [101] = PINGROUP(101, NORTH, _, _, _, _, _, _, _, _, _),
+ [102] = PINGROUP(102, NORTH, _, _, _, _, _, _, _, _, _),
+ [103] = PINGROUP(103, NORTH, _, _, _, _, _, _, _, _, _),
+ [104] = PINGROUP(104, WEST, usb_phy, _, qdss, _, _, _, _, _, _),
+ [105] = PINGROUP(105, NORTH, _, _, _, _, _, _, _, _, _),
+ [106] = PINGROUP(106, NORTH, _, _, _, _, _, _, _, _, _),
+ [107] = PINGROUP(107, WEST, _, _V_GPIO, _V_PPS_IN, _V_PPS_OUT, gps_tx, _, _, _, _),
+ [108] = PINGROUP(108, SOUTH, mss_lte, _, phase_flag, ddr_pxi3, _, _, _, _, _),
+ [109] = PINGROUP(109, SOUTH, mss_lte, gps_tx, _, phase_flag, _, _, _, _, _),
+ [110] = PINGROUP(110, NORTH, _, _, _, _, _, _, _, _, _),
+ [111] = PINGROUP(111, NORTH, _, _, _, _, _, _, _, _, _),
+ [112] = PINGROUP(112, NORTH, _, _, _, _, _, _, _, _, _),
+ [113] = PINGROUP(113, NORTH, _, _, _, _, _, _, _, _, _),
+ [114] = PINGROUP(114, NORTH, _, _, _, _, _, _, _, _, _),
+ [115] = PINGROUP(115, WEST, qup04, qup04, _, _, _, _, _, _, _),
+ [116] = PINGROUP(116, WEST, qup04, qup04, _, _, _, _, _, _, _),
+ [117] = PINGROUP(117, WEST, dp_hot, _, _, _, _, _, _, _, _),
+ [118] = PINGROUP(118, WEST, _, _, _, _, _, _, _, _, _),
+ [119] = UFS_RESET(ufs_reset, 0x97f000),
+ [120] = SDC_QDSD_PINGROUP(sdc1_rclk, 0x97a000, 15, 0),
+ [121] = SDC_QDSD_PINGROUP(sdc1_clk, 0x97a000, 13, 6),
+ [122] = SDC_QDSD_PINGROUP(sdc1_cmd, 0x97a000, 11, 3),
+ [123] = SDC_QDSD_PINGROUP(sdc1_data, 0x97a000, 9, 0),
+ [124] = SDC_QDSD_PINGROUP(sdc2_clk, 0x97b000, 14, 6),
+ [125] = SDC_QDSD_PINGROUP(sdc2_cmd, 0x97b000, 11, 3),
+ [126] = SDC_QDSD_PINGROUP(sdc2_data, 0x97b000, 9, 0),
+};
+
+static const struct msm_pinctrl_soc_data sc7180_pinctrl = {
+ .pins = sc7180_pins,
+ .npins = ARRAY_SIZE(sc7180_pins),
+ .functions = sc7180_functions,
+ .nfunctions = ARRAY_SIZE(sc7180_functions),
+ .groups = sc7180_groups,
+ .ngroups = ARRAY_SIZE(sc7180_groups),
+ .ngpios = 120,
+ .tiles = sc7180_tiles,
+ .ntiles = ARRAY_SIZE(sc7180_tiles),
+};
+
+static int sc7180_pinctrl_probe(struct platform_device *pdev)
+{
+ return msm_pinctrl_probe(pdev, &sc7180_pinctrl);
+}
+
+static const struct of_device_id sc7180_pinctrl_of_match[] = {
+ { .compatible = "qcom,sc7180-pinctrl", },
+ { },
+};
+
+static struct platform_driver sc7180_pinctrl_driver = {
+ .driver = {
+ .name = "sc7180-pinctrl",
+ .pm = &msm_pinctrl_dev_pm_ops,
+ .of_match_table = sc7180_pinctrl_of_match,
+ },
+ .probe = sc7180_pinctrl_probe,
+ .remove = msm_pinctrl_remove,
+};
+
+static int __init sc7180_pinctrl_init(void)
+{
+ return platform_driver_register(&sc7180_pinctrl_driver);
+}
+arch_initcall(sc7180_pinctrl_init);
+
+static void __exit sc7180_pinctrl_exit(void)
+{
+ platform_driver_unregister(&sc7180_pinctrl_driver);
+}
+module_exit(sc7180_pinctrl_exit);
+
+MODULE_DESCRIPTION("QTI sc7180 pinctrl driver");
+MODULE_LICENSE("GPL v2");
+MODULE_DEVICE_TABLE(of, sc7180_pinctrl_of_match);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: pinctrl: qcom: Add SC7180 pinctrl binding
From: Rajendra Nayak @ 2019-08-02 4:15 UTC (permalink / raw)
To: linus.walleij, bjorn.andersson
Cc: linux-arm-msm, agross, robh+dt, linux-gpio, devicetree,
linux-kernel, Jitendra Sharma, Vivek Gautam, Rajendra Nayak
From: Jitendra Sharma <shajit@codeaurora.org>
Add the binding for the TLMM pinctrl block found in the SC7180 platform
Signed-off-by: Jitendra Sharma <shajit@codeaurora.org>
Signed-off-by: Vivek Gautam <vivek.gautam@codeaurora.org>
[rnayak: Fix some copy-paste issues, sort and fix functions]
Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
.../bindings/pinctrl/qcom,sc7180-pinctrl.txt | 186 ++++++++++++++++++
1 file changed, 186 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,sc7180-pinctrl.txt
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,sc7180-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/qcom,sc7180-pinctrl.txt
new file mode 100644
index 000000000000..948cd56cfab7
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,sc7180-pinctrl.txt
@@ -0,0 +1,186 @@
+Qualcomm Technologies, Inc. SC7180 TLMM block
+
+This binding describes the Top Level Mode Multiplexer block found in the
+SC7180 platform.
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: must be "qcom,sc7180-pinctrl"
+
+- reg:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: the base address and size of the north, south and west
+ TLMM tiles
+
+- reg-names:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Defintiion: names for the cells of reg, must contain "north", "south"
+ and "west".
+
+- interrupts:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: should specify the TLMM summary IRQ.
+
+- interrupt-controller:
+ Usage: required
+ Value type: <none>
+ Definition: identifies this node as an interrupt controller
+
+- #interrupt-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: must be 2. Specifying the pin number and flags, as defined
+ in <dt-bindings/interrupt-controller/irq.h>
+
+- gpio-controller:
+ Usage: required
+ Value type: <none>
+ Definition: identifies this node as a gpio controller
+
+- #gpio-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: must be 2. Specifying the pin number and flags, as defined
+ in <dt-bindings/gpio/gpio.h>
+
+- gpio-ranges:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: see ../gpio/gpio.txt
+
+- gpio-reserved-ranges:
+ Usage: optional
+ Value type: <prop-encoded-array>
+ Definition: see ../gpio/gpio.txt
+
+Please refer to ../gpio/gpio.txt and ../interrupt-controller/interrupts.txt for
+a general description of GPIO and interrupt bindings.
+
+Please refer to pinctrl-bindings.txt in this directory for details of the
+common pinctrl bindings used by client devices, including the meaning of the
+phrase "pin configuration node".
+
+The pin configuration nodes act as a container for an arbitrary number of
+subnodes. Each of these subnodes represents some desired configuration for a
+pin, a group, or a list of pins or groups. This configuration can include the
+mux function to select on those pin(s)/group(s), and various pin configuration
+parameters, such as pull-up, drive strength, etc.
+
+
+PIN CONFIGURATION NODES:
+
+The name of each subnode is not important; all subnodes should be enumerated
+and processed purely based on their content.
+
+Each subnode only affects those parameters that are explicitly listed. In
+other words, a subnode that lists a mux function but no pin configuration
+parameters implies no information about any pin configuration parameters.
+Similarly, a pin subnode that describes a pullup parameter implies no
+information about e.g. the mux function.
+
+
+The following generic properties as defined in pinctrl-bindings.txt are valid
+to specify in a pin configuration subnode:
+
+- pins:
+ Usage: required
+ Value type: <string-array>
+ Definition: List of gpio pins affected by the properties specified in
+ this subnode.
+
+ Valid pins are:
+ gpio0-gpio118
+ Supports mux, bias and drive-strength
+
+ sdc1_clk, sdc1_cmd, sdc1_data sdc2_clk, sdc2_cmd,
+ sdc2_data sdc1_rclk
+ Supports bias and drive-strength
+
+ ufs_reset
+ Supports bias and drive-strength
+
+- function:
+ Usage: required
+ Value type: <string>
+ Definition: Specify the alternative function to be configured for the
+ specified pins. Functions are only valid for gpio pins.
+ Valid values are:
+
+ adsp_ext, agera_pll, aoss_cti, atest_char, atest_char0,
+ atest_char1, atest_char2, atest_char3, atest_tsens,
+ atest_tsens2, atest_usb1, atest_usb10, atest_usb11,
+ atest_usb12, atest_usb13, atest_usb2, atest_usb20,
+ atest_usb21, atest_usb22, atest_usb23, audio_ref,
+ btfm_slimbus, cam_mclk, cci_async, cci_i2c, cci_timer0,
+ cci_timer1, cci_timer2, cci_timer3, cci_timer4,
+ cri_trng, dbg_out, ddr_bist, ddr_pxi0, ddr_pxi1,
+ ddr_pxi2, ddr_pxi3, dp_hot, edp_lcd, gcc_gp1, gcc_gp2,
+ gcc_gp3, gpio, gp_pdm0, gp_pdm1, gp_pdm2, gps_tx,
+ jitter_bist, ldo_en, ldo_update, lpass_ext, mdp_vsync,
+ mdp_vsync0, mdp_vsync1, mdp_vsync2, mdp_vsync3, mi2s_0,
+ mi2s_1, mi2s_2, mss_lte, m_voc, pa_indicator, phase_flag,
+ PLL_BIST, pll_bypassnl, pll_reset, prng_rosc, qdss,
+ qdss_cti, qlink_enable, qlink_request, qspi_clk, qspi_cs,
+ qspi_data, qup00, qup01, qup02, qup03, qup04, qup05,
+ qup10, qup11, qup12, qup13, qup14, qup15, sdc1_tb,
+ sdc2_tb, sd_write, sp_cmu, tgu_ch0, tgu_ch1, tgu_ch2,
+ tgu_ch3, tsense_pwm1, tsense_pwm2, uim1, uim2, uim_batt,
+ usb_phy, vfr_1, _V_GPIO, _V_PPS_IN, _V_PPS_OUT,
+ vsense_trigger, wlan1_adc0, wlan1_adc1, wlan2_adc0,
+ wlan2_adc1,
+
+- bias-disable:
+ Usage: optional
+ Value type: <none>
+ Definition: The specified pins should be configured as no pull.
+
+- bias-pull-down:
+ Usage: optional
+ Value type: <none>
+ Definition: The specified pins should be configured as pull down.
+
+- bias-pull-up:
+ Usage: optional
+ Value type: <none>
+ Definition: The specified pins should be configured as pull up.
+
+- output-high:
+ Usage: optional
+ Value type: <none>
+ Definition: The specified pins are configured in output mode, driven
+ high.
+ Not valid for sdc pins.
+
+- output-low:
+ Usage: optional
+ Value type: <none>
+ Definition: The specified pins are configured in output mode, driven
+ low.
+ Not valid for sdc pins.
+
+- drive-strength:
+ Usage: optional
+ Value type: <u32>
+ Definition: Selects the drive strength for the specified pins, in mA.
+ Valid values are: 2, 4, 6, 8, 10, 12, 14 and 16
+
+Example:
+
+ tlmm: pinctrl@3000000 {
+ compatible = "qcom,sc7180-pinctrl";
+ reg = <0x3500000 0x300000>,
+ <0x3900000 0x300000>,
+ <0x3D00000 0x300000>;
+ reg-names = "west", "north", "south";
+ interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&tlmm 0 0 119>;
+ gpio-reserved-ranges = <0 4>, <106 4>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related
* Re: [PATCH v7 11/20] cpufreq: tegra124: Add suspend and resume support
From: Viresh Kumar @ 2019-08-02 3:41 UTC (permalink / raw)
To: Sowjanya Komatineni
Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, digetx, devicetree,
rjw, linux-pm
In-Reply-To: <1564607463-28802-12-git-send-email-skomatineni@nvidia.com>
On 31-07-19, 14:10, Sowjanya Komatineni wrote:
> This patch adds suspend and resume pm ops for cpufreq driver.
>
> PLLP is the safe clock source for CPU during system suspend and
> resume as PLLP rate is below the CPU Fmax at Vmin.
>
> CPUFreq driver suspend switches the CPU clock source to PLLP and
> disables the DFLL clock.
>
> During system resume, warmboot code powers up the CPU with PLLP
> clock source. So CPUFreq driver resume enabled DFLL clock and
> switches CPU back to DFLL clock source.
>
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
> drivers/cpufreq/tegra124-cpufreq.c | 60 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 60 insertions(+)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* Re: [PATCH 2/2] pinctrl: qcom: Add SC7180 pinctrl driver
From: Rajendra Nayak @ 2019-08-02 2:30 UTC (permalink / raw)
To: Bjorn Andersson
Cc: linus.walleij, linux-arm-msm, agross, robh+dt, linux-gpio,
devicetree, linux-kernel, Jitendra Sharma, Vivek Gautam
In-Reply-To: <20190801143637.GY7234@tuxbook-pro>
On 8/1/2019 8:06 PM, Bjorn Andersson wrote:
> On Thu 01 Aug 03:07 PDT 2019, Rajendra Nayak wrote:
>
> [..]
>> +static const struct msm_pingroup sc7180_groups[] = {
>> + [0] = PINGROUP(0, SOUTH, qup01, cri_trng, _, phase_flag, _, _, _, _, _),
>> + [1] = PINGROUP(1, SOUTH, qup01, cri_trng, _, phase_flag, _, _, _, _, _),
>> + [2] = PINGROUP(2, SOUTH, qup01, cri_trng, _, phase_flag, _, _, _, _, _),
>> + [3] = PINGROUP(3, SOUTH, qup01, sp_cmu, dbg_out, qdss_cti, _, _, _, _, _),
>> + [4] = PINGROUP(4, NORTH, sdc1_tb, _, qdss_cti, _, _, _, _, _, _), [5] = PINGROUP(5, NORTH, sdc2_tb, _, _, _, _, _, _, _, _),
>> + [6] = PINGROUP(6, NORTH, qup11, qup11, _, _, _, _, _, _, _), [7] = PINGROUP(7, NORTH, qup11, qup11, ddr_bist, _, _, _, _, _, _),
>
> 5 and 7 deserve to be on their own line :)
Oops, looks like some formatting mess, I'll fix and resend.
>
> Apart from that:
>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
thanks for the review.
>
> Regards,
> Bjorn
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 23:49 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: <61652889-2e77-8f1e-9ed4-b7e525a40b10@nvidia.com>
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.
>
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 23:19 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: <85cd5100-467e-d08e-0ae5-ae57a6de5312@nvidia.com>
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.
>>>
>>>
>>> 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.
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 21:30 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: <62a5c6ed-21b1-8403-6fac-9c5d99b5a255@gmail.com>
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.
>>
>>
>> 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?
>
>> 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
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-01 20:54 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: <50bad1d3-df41-d1e5-a7c7-4be9c661ed14@nvidia.com>
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.
>
>
> 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?
>
> 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?
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 20:31 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: <ef9e865f-359b-0873-a414-3d548bd4e590@gmail.com>
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.
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.
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
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-01 20:17 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: <a81b85a2-5634-cfa2-77c5-94c23c4847bd@nvidia.com>
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?
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 19:42 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding, jonathanh, tglx, jason,
marc.zyngier, linus.walleij, stefan, mark.rutland
Cc: pdeschrijver, pgaikwad, sboyd, linux-clk, linux-gpio, jckuo,
josephl, talho, linux-tegra, linux-kernel, mperttunen, spatra,
robh+dt, devicetree
In-Reply-To: <c85ba067-af68-0b4a-d347-501ed7ed0ef9@gmail.com>
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?
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.
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Dmitry Osipenko @ 2019-08-01 19:00 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: <550de191-f982-4544-6fbc-bf16dfeae2c6@nvidia.com>
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.
^ permalink raw reply
* Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore
From: Dmitry Osipenko @ 2019-08-01 18:42 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: <cd9c5116-4235-758c-7a09-d7185d52b22b@nvidia.com>
01.08.2019 21:06, Sowjanya Komatineni пишет:
>
> On 8/1/19 3:53 AM, Dmitry Osipenko wrote:
>> 01.08.2019 0:04, Sowjanya Komatineni пишет:
>>> On 7/31/19 4:11 AM, Dmitry Osipenko wrote:
>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>> This patch adds support for saving OSC clock frequency and the
>>>>> drive-strength during OSC clock init and creates an API to restore
>>>>> OSC control register value from the saved context.
>>>>>
>>>>> This API is invoked by Tegra210 clock driver during system resume
>>>>> to restore the OSC clock settings.
>>>>>
>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
>>>>> drivers/clk/tegra/clk.h | 1 +
>>>>> 2 files changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> b/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> index 8d91b2b191cf..7c6c8abfcde6 100644
>>>>> --- a/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
>>>>> @@ -17,6 +17,10 @@
>>>>> #define OSC_CTRL 0x50
>>>>> #define OSC_CTRL_OSC_FREQ_SHIFT 28
>>>>> #define OSC_CTRL_PLL_REF_DIV_SHIFT 26
>>>>> +#define OSC_CTRL_MASK (0x3f2 | \
>>>>> + (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
>>>>> +
>>>>> +static u32 osc_ctrl_ctx;
>>>>> int __init tegra_osc_clk_init(void __iomem *clk_base, struct
>>>>> tegra_clk *clks,
>>>>> unsigned long *input_freqs, unsigned int num,
>>>>> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem
>>>>> *clk_base, struct tegra_clk *clks,
>>>>> unsigned osc_idx;
>>>>> val = readl_relaxed(clk_base + OSC_CTRL);
>>>>> + osc_ctrl_ctx = val & OSC_CTRL_MASK;
>>>>> osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>>>>> if (osc_idx < num)
>>>>> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk
>>>>> *tegra_clks)
>>>>> *dt_clk = clk;
>>>>> }
>>>>> }
>>>>> +
>>>>> +void tegra_clk_osc_resume(void __iomem *clk_base)
>>>>> +{
>>>>> + u32 val;
>>>>> +
>>>>> + val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
>>>>> + val |= osc_ctrl_ctx;
>>>>> + writel_relaxed(val, clk_base + OSC_CTRL);
>>>> Why a full raw u32 OSC_CTRL value couldn't be simply saved and
>>>> restored?
>>> Storing and restoring only required fields to avoid accidental
>>> misconfiguration.
>>>
>>> OSC_CTRL register has other bits (PLL_REF_DIV) which are configured by
>>> BR depending on OSC_FREQ and also setting PLL_REF_DIV while PLLS are in
>>> use is not safe.
>> I'm looking at the clk-driver sources and see that none of the Tegra
>> drivers ever change the OSC_CTRL configuration, T30/114 even have
>> #defines for the OSC_CTRL that are unused.
>>
>> So, this leads to a question.. does any bootloader really ever change
>> the OSC_CTRL such that it differs after resume from suspend in
>> comparison to the value at the time of kernel's booting up?
>
> For Tegra210, bootloader programs OSC_CTRL register for drivestrength
> programming.
>
> These settings need to be restored to the same on SC7 exit as they gets
> reset during SC7 entry.
Okay, thank you for the clarification.
^ permalink raw reply
* Re: [PATCH v7 16/20] arm64: tegra: Enable wake from deep sleep on RTC alarm
From: Dmitry Osipenko @ 2019-08-01 18:39 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: <1e2e6282-9d94-e322-c4ba-8a1f3b902860@nvidia.com>
01.08.2019 20:56, Sowjanya Komatineni пишет:
>
> On 8/1/19 3:43 AM, Dmitry Osipenko wrote:
>> 01.08.2019 0:08, Sowjanya Komatineni пишет:
>>> On 7/31/19 4:04 AM, Dmitry Osipenko wrote:
>>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>>> This patch updates device tree for RTC and PMC to allow system wake
>>>>> from deep sleep on RTC alarm.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 ++++-
>>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>>> b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>>> index 659753118e96..30a7c48385a2 100644
>>>>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>>> @@ -768,7 +768,8 @@
>>>>> rtc@7000e000 {
>>>>> compatible = "nvidia,tegra210-rtc", "nvidia,tegra20-rtc";
>>>>> reg = <0x0 0x7000e000 0x0 0x100>;
>>>>> - interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
>>>>> + interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
>>>>> + interrupt-parent = <&pmc>;
>>>>> clocks = <&tegra_car TEGRA210_CLK_RTC>;
>>>>> clock-names = "rtc";
>>>>> };
>>>>> @@ -778,6 +779,8 @@
>>>>> reg = <0x0 0x7000e400 0x0 0x400>;
>>>>> clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
>>>>> clock-names = "pclk", "clk32k_in";
>>>>> + #interrupt-cells = <2>;
>>>>> + interrupt-controller;
>>>>> powergates {
>>>>> pd_audio: aud {
>>>>>
>>>> Is this a backwards-compatible change? Or it's not really worth to care
>>>> about the compatibility with older kernel versions, I'm not sure about
>>>> overall state of T210 in the upstream kernel.
>>> I don't think its required to be backwards-compatible as SC7 entry/exit
>>> implementation for T210 is with this patch series onwards..
>> The new device tree binary should work with older kernel versions, AFAIK
>> this is the upstream rule. But if kernel support isn't in a very good
>> shape and not much people are using it, then obviously it is not very
>> important.
>
> Yes, my response to backwards-compatible was with respect to interrupt
> parent change as this will not be backward compatible and also there is
> no Tegra210 suspend/resume earlier. Other functionality wise, it is
> backward compatible.
Should be good then.
^ permalink raw reply
* Re: [PATCH v7 06/20] clk: tegra: Support for OSC context save and restore
From: Sowjanya Komatineni @ 2019-08-01 18:06 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: <3b55a112-42a6-212b-beea-10b64d5341d9@gmail.com>
On 8/1/19 3:53 AM, Dmitry Osipenko wrote:
> 01.08.2019 0:04, Sowjanya Komatineni пишет:
>> On 7/31/19 4:11 AM, Dmitry Osipenko wrote:
>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>> This patch adds support for saving OSC clock frequency and the
>>>> drive-strength during OSC clock init and creates an API to restore
>>>> OSC control register value from the saved context.
>>>>
>>>> This API is invoked by Tegra210 clock driver during system resume
>>>> to restore the OSC clock settings.
>>>>
>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> drivers/clk/tegra/clk-tegra-fixed.c | 15 +++++++++++++++
>>>> drivers/clk/tegra/clk.h | 1 +
>>>> 2 files changed, 16 insertions(+)
>>>>
>>>> diff --git a/drivers/clk/tegra/clk-tegra-fixed.c
>>>> b/drivers/clk/tegra/clk-tegra-fixed.c
>>>> index 8d91b2b191cf..7c6c8abfcde6 100644
>>>> --- a/drivers/clk/tegra/clk-tegra-fixed.c
>>>> +++ b/drivers/clk/tegra/clk-tegra-fixed.c
>>>> @@ -17,6 +17,10 @@
>>>> #define OSC_CTRL 0x50
>>>> #define OSC_CTRL_OSC_FREQ_SHIFT 28
>>>> #define OSC_CTRL_PLL_REF_DIV_SHIFT 26
>>>> +#define OSC_CTRL_MASK (0x3f2 | \
>>>> + (0xf << OSC_CTRL_OSC_FREQ_SHIFT))
>>>> +
>>>> +static u32 osc_ctrl_ctx;
>>>> int __init tegra_osc_clk_init(void __iomem *clk_base, struct
>>>> tegra_clk *clks,
>>>> unsigned long *input_freqs, unsigned int num,
>>>> @@ -29,6 +33,7 @@ int __init tegra_osc_clk_init(void __iomem
>>>> *clk_base, struct tegra_clk *clks,
>>>> unsigned osc_idx;
>>>> val = readl_relaxed(clk_base + OSC_CTRL);
>>>> + osc_ctrl_ctx = val & OSC_CTRL_MASK;
>>>> osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
>>>> if (osc_idx < num)
>>>> @@ -96,3 +101,13 @@ void __init tegra_fixed_clk_init(struct tegra_clk
>>>> *tegra_clks)
>>>> *dt_clk = clk;
>>>> }
>>>> }
>>>> +
>>>> +void tegra_clk_osc_resume(void __iomem *clk_base)
>>>> +{
>>>> + u32 val;
>>>> +
>>>> + val = readl_relaxed(clk_base + OSC_CTRL) & ~OSC_CTRL_MASK;
>>>> + val |= osc_ctrl_ctx;
>>>> + writel_relaxed(val, clk_base + OSC_CTRL);
>>> Why a full raw u32 OSC_CTRL value couldn't be simply saved and restored?
>> Storing and restoring only required fields to avoid accidental
>> misconfiguration.
>>
>> OSC_CTRL register has other bits (PLL_REF_DIV) which are configured by
>> BR depending on OSC_FREQ and also setting PLL_REF_DIV while PLLS are in
>> use is not safe.
> I'm looking at the clk-driver sources and see that none of the Tegra
> drivers ever change the OSC_CTRL configuration, T30/114 even have
> #defines for the OSC_CTRL that are unused.
>
> So, this leads to a question.. does any bootloader really ever change
> the OSC_CTRL such that it differs after resume from suspend in
> comparison to the value at the time of kernel's booting up?
For Tegra210, bootloader programs OSC_CTRL register for drivestrength
programming.
These settings need to be restored to the same on SC7 exit as they gets
reset during SC7 entry.
^ permalink raw reply
* Re: [PATCH v7 07/20] clk: tegra: clk-periph: Add save and restore support
From: Sowjanya Komatineni @ 2019-08-01 17:58 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: <67cf6c13-688d-0305-61e2-c63c8e8b4729@nvidia.com>
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.
^ permalink raw reply
* Re: [PATCH v7 16/20] arm64: tegra: Enable wake from deep sleep on RTC alarm
From: Sowjanya Komatineni @ 2019-08-01 17:56 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: <4f8b9ee7-5402-6463-d6d2-7b00e274a185@gmail.com>
On 8/1/19 3:43 AM, Dmitry Osipenko wrote:
> 01.08.2019 0:08, Sowjanya Komatineni пишет:
>> On 7/31/19 4:04 AM, Dmitry Osipenko wrote:
>>> 31.07.2019 3:20, Sowjanya Komatineni пишет:
>>>> This patch updates device tree for RTC and PMC to allow system wake
>>>> from deep sleep on RTC alarm.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>> ---
>>>> arch/arm64/boot/dts/nvidia/tegra210.dtsi | 5 ++++-
>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>> b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>> index 659753118e96..30a7c48385a2 100644
>>>> --- a/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>> +++ b/arch/arm64/boot/dts/nvidia/tegra210.dtsi
>>>> @@ -768,7 +768,8 @@
>>>> rtc@7000e000 {
>>>> compatible = "nvidia,tegra210-rtc", "nvidia,tegra20-rtc";
>>>> reg = <0x0 0x7000e000 0x0 0x100>;
>>>> - interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
>>>> + interrupts = <16 IRQ_TYPE_LEVEL_HIGH>;
>>>> + interrupt-parent = <&pmc>;
>>>> clocks = <&tegra_car TEGRA210_CLK_RTC>;
>>>> clock-names = "rtc";
>>>> };
>>>> @@ -778,6 +779,8 @@
>>>> reg = <0x0 0x7000e400 0x0 0x400>;
>>>> clocks = <&tegra_car TEGRA210_CLK_PCLK>, <&clk32k_in>;
>>>> clock-names = "pclk", "clk32k_in";
>>>> + #interrupt-cells = <2>;
>>>> + interrupt-controller;
>>>> powergates {
>>>> pd_audio: aud {
>>>>
>>> Is this a backwards-compatible change? Or it's not really worth to care
>>> about the compatibility with older kernel versions, I'm not sure about
>>> overall state of T210 in the upstream kernel.
>> I don't think its required to be backwards-compatible as SC7 entry/exit
>> implementation for T210 is with this patch series onwards..
> The new device tree binary should work with older kernel versions, AFAIK
> this is the upstream rule. But if kernel support isn't in a very good
> shape and not much people are using it, then obviously it is not very
> important.
Yes, my response to backwards-compatible was with respect to interrupt
parent change as this will not be backward compatible and also there is
no Tegra210 suspend/resume earlier. Other functionality wise, it is
backward compatible.
^ permalink raw reply
* Re: [PATCH v7 10/20] clk: tegra: clk-dfll: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-01 17:53 UTC (permalink / raw)
To: Dmitry Osipenko, thierry.reding@gmail.com, Jonathan Hunter,
tglx@linutronix.de, jason@lakedaemon.net, marc.zyngier@arm.com,
linus.walleij@linaro.org, stefan@agner.ch, mark.rutland@arm.com
Cc: Peter De Schrijver, Prashant Gaikwad, sboyd@kernel.org,
linux-clk@vger.kernel.org, linux-gpio@vger.kernel.org,
Jui Chang Kuo, Joseph Lo, Timo Alho, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, Mikko Perttunen, Sandipan Patra,
robh+dt@kernel.org, devicetree@vger.kernel.org, rjw@rjwysocki.net,
viresh.kumar@linaro.org, linux-pm@vger.kernel.org
In-Reply-To: <31990250-e237-ddb9-ce71-29b7c2302fc3@gmail.com>
On 8/1/19 10:10 AM, Dmitry Osipenko wrote:
> 01.08.2019 19:10, Sowjanya Komatineni пишет:
>> I didn’t updated any patches. This is still same v7 just resent with
>> CPUFreq maintainers in CC as I missed to add them earlier.
> There are now two different threads for the same patches, which is not
> very good. When I said that CPUFreq maintainers should be CC'ed, I
> didn't mean to resend it all, sorry for not being clear about it. You
> should've wait for more comments to the original patches and then make a
> v8. I suggest to do the same in the current situation as well, please
> address all the current comments and wait for 1-2 days, then make a v8.
Thought version bump is only when changes were made. Got it now.
Thanks Dmitry. Sure, will give some time for all feedbacks before
sending V8.
^ permalink raw reply
* Re: [PATCH v7 11/20] cpufreq: tegra124: Add suspend and resume support
From: Sowjanya Komatineni @ 2019-08-01 17:51 UTC (permalink / raw)
To: Viresh Kumar
Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, josephl, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, digetx, devicetree,
rjw, linux-pm
In-Reply-To: <20190801054055.trmabmcaj3cpe4pc@vireshk-i7>
On 7/31/19 10:40 PM, Viresh Kumar wrote:
> On 31-07-19, 14:10, Sowjanya Komatineni wrote:
>> This patch adds suspend and resume pm ops for cpufreq driver.
>>
>> PLLP is the safe clock source for CPU during system suspend and
>> resume as PLLP rate is below the CPU Fmax at Vmin.
>>
>> CPUFreq driver suspend switches the CPU clock source to PLLP and
>> disables the DFLL clock.
>>
>> During system resume, warmboot code powers up the CPU with PLLP
>> clock source. So CPUFreq driver resume enabled DFLL clock and
>> switches CPU back to DFLL clock source.
>>
>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>> ---
>> drivers/cpufreq/tegra124-cpufreq.c | 60 ++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 60 insertions(+)
> Is there any hard dependency of this patch on the rest of the patches?
> Can I apply it alone to cpufreq tree ?
>
This patch cannot be applied alone as it has dependency on dfll clock
suspend and resume sequence.
^ permalink raw reply
* [PATCH v1 4/4] gpio: pca953x: Drop %s for constant string literals
From: Andy Shevchenko @ 2019-08-01 17:39 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, linux-gpio, Marek Vasut
Cc: Andy Shevchenko
In-Reply-To: <20190801173938.36676-1-andriy.shevchenko@linux.intel.com>
There is no need to use %s for constant string literals
w/o special characters inside.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/gpio/gpio-pca953x.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 454bbe2fb41f..64d02ca60f53 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -1038,8 +1038,7 @@ static int pca953x_remove(struct i2c_client *client)
ret = pdata->teardown(client, chip->gpio_chip.base,
chip->gpio_chip.ngpio, pdata->context);
if (ret < 0)
- dev_err(&client->dev, "%s failed, %d\n",
- "teardown", ret);
+ dev_err(&client->dev, "teardown failed, %d\n", ret);
} else {
ret = 0;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v1 1/4] gpio: pca953x: Switch to use device_get_match_data()
From: Andy Shevchenko @ 2019-08-01 17:39 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, linux-gpio, Marek Vasut
Cc: Andy Shevchenko
Instead of open coded variants, switch to direct use of
device_get_match_data().
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/gpio/gpio-pca953x.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 378b206d2dc9..54cf01901320 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -949,19 +949,15 @@ static int pca953x_probe(struct i2c_client *client,
if (i2c_id) {
chip->driver_data = i2c_id->driver_data;
} else {
- const struct acpi_device_id *acpi_id;
- struct device *dev = &client->dev;
-
- chip->driver_data = (uintptr_t)of_device_get_match_data(dev);
- if (!chip->driver_data) {
- acpi_id = acpi_match_device(pca953x_acpi_ids, dev);
- if (!acpi_id) {
- ret = -ENODEV;
- goto err_exit;
- }
-
- chip->driver_data = acpi_id->driver_data;
+ const void *match;
+
+ match = device_get_match_data(&client->dev);
+ if (!match) {
+ ret = -ENODEV;
+ goto err_exit;
}
+
+ chip->driver_data = (uintptr_t)match;
}
i2c_set_clientdata(client, chip);
--
2.20.1
^ permalink raw reply related
* [PATCH v1 2/4] gpio: pca953x: Use GENMASK() consistently
From: Andy Shevchenko @ 2019-08-01 17:39 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, linux-gpio, Marek Vasut
Cc: Andy Shevchenko
In-Reply-To: <20190801173938.36676-1-andriy.shevchenko@linux.intel.com>
Use GENMASK() macro for all definitions where it's appropriate.
No functional change intended.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/gpio/gpio-pca953x.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 54cf01901320..aaba0b394d2f 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -9,6 +9,7 @@
*/
#include <linux/acpi.h>
+#include <linux/bits.h>
#include <linux/gpio/driver.h>
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
@@ -28,9 +29,9 @@
#define PCA953X_INVERT 0x02
#define PCA953X_DIRECTION 0x03
-#define REG_ADDR_MASK 0x3f
-#define REG_ADDR_EXT 0x40
-#define REG_ADDR_AI 0x80
+#define REG_ADDR_MASK GENMASK(5, 0)
+#define REG_ADDR_EXT BIT(6)
+#define REG_ADDR_AI BIT(7)
#define PCA957X_IN 0x00
#define PCA957X_INVRT 0x01
@@ -55,17 +56,17 @@
#define PCAL6524_OUT_INDCONF 0x2c
#define PCAL6524_DEBOUNCE 0x2d
-#define PCA_GPIO_MASK 0x00FF
+#define PCA_GPIO_MASK GENMASK(7, 0)
-#define PCAL_GPIO_MASK 0x1f
-#define PCAL_PINCTRL_MASK 0x60
+#define PCAL_GPIO_MASK GENMASK(4, 0)
+#define PCAL_PINCTRL_MASK GENMASK(6, 5)
-#define PCA_INT 0x0100
-#define PCA_PCAL 0x0200
+#define PCA_INT BIT(8)
+#define PCA_PCAL BIT(9)
#define PCA_LATCH_INT (PCA_PCAL | PCA_INT)
-#define PCA953X_TYPE 0x1000
-#define PCA957X_TYPE 0x2000
-#define PCA_TYPE_MASK 0xF000
+#define PCA953X_TYPE BIT(12)
+#define PCA957X_TYPE BIT(13)
+#define PCA_TYPE_MASK GENMASK(15, 12)
#define PCA_CHIP_TYPE(x) ((x) & PCA_TYPE_MASK)
@@ -565,7 +566,7 @@ static void pca953x_irq_mask(struct irq_data *d)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct pca953x_chip *chip = gpiochip_get_data(gc);
- chip->irq_mask[d->hwirq / BANK_SZ] &= ~(1 << (d->hwirq % BANK_SZ));
+ chip->irq_mask[d->hwirq / BANK_SZ] &= ~BIT(d->hwirq % BANK_SZ);
}
static void pca953x_irq_unmask(struct irq_data *d)
@@ -573,7 +574,7 @@ static void pca953x_irq_unmask(struct irq_data *d)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct pca953x_chip *chip = gpiochip_get_data(gc);
- chip->irq_mask[d->hwirq / BANK_SZ] |= 1 << (d->hwirq % BANK_SZ);
+ chip->irq_mask[d->hwirq / BANK_SZ] |= BIT(d->hwirq % BANK_SZ);
}
static int pca953x_irq_set_wake(struct irq_data *d, unsigned int on)
@@ -641,7 +642,7 @@ static int pca953x_irq_set_type(struct irq_data *d, unsigned int type)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct pca953x_chip *chip = gpiochip_get_data(gc);
int bank_nb = d->hwirq / BANK_SZ;
- u8 mask = 1 << (d->hwirq % BANK_SZ);
+ u8 mask = BIT(d->hwirq % BANK_SZ);
if (!(type & IRQ_TYPE_EDGE_BOTH)) {
dev_err(&chip->client->dev, "irq %d: unsupported type %d\n",
@@ -666,7 +667,7 @@ static void pca953x_irq_shutdown(struct irq_data *d)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct pca953x_chip *chip = gpiochip_get_data(gc);
- u8 mask = 1 << (d->hwirq % BANK_SZ);
+ u8 mask = BIT(d->hwirq % BANK_SZ);
chip->irq_trig_raise[d->hwirq / BANK_SZ] &= ~mask;
chip->irq_trig_fall[d->hwirq / BANK_SZ] &= ~mask;
--
2.20.1
^ permalink raw reply related
* [PATCH v1 3/4] gpio: pca953x: Remove explicit comparison with 0
From: Andy Shevchenko @ 2019-08-01 17:39 UTC (permalink / raw)
To: Linus Walleij, Bartosz Golaszewski, linux-gpio, Marek Vasut
Cc: Andy Shevchenko
In-Reply-To: <20190801173938.36676-1-andriy.shevchenko@linux.intel.com>
There is no need to explicitly compare return code with 0.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/gpio/gpio-pca953x.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index aaba0b394d2f..454bbe2fb41f 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -850,12 +850,12 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert)
ret = regcache_sync_region(chip->regmap, chip->regs->output,
chip->regs->output + NBANK(chip));
- if (ret != 0)
+ if (ret)
goto out;
ret = regcache_sync_region(chip->regmap, chip->regs->direction,
chip->regs->direction + NBANK(chip));
- if (ret != 0)
+ if (ret)
goto out;
/* set platform specific polarity inversion */
@@ -1061,14 +1061,14 @@ static int pca953x_regcache_sync(struct device *dev)
*/
ret = regcache_sync_region(chip->regmap, chip->regs->direction,
chip->regs->direction + NBANK(chip));
- if (ret != 0) {
+ if (ret) {
dev_err(dev, "Failed to sync GPIO dir registers: %d\n", ret);
return ret;
}
ret = regcache_sync_region(chip->regmap, chip->regs->output,
chip->regs->output + NBANK(chip));
- if (ret != 0) {
+ if (ret) {
dev_err(dev, "Failed to sync GPIO out registers: %d\n", ret);
return ret;
}
@@ -1077,7 +1077,7 @@ static int pca953x_regcache_sync(struct device *dev)
if (chip->driver_data & PCA_PCAL) {
ret = regcache_sync_region(chip->regmap, PCAL953X_IN_LATCH,
PCAL953X_IN_LATCH + NBANK(chip));
- if (ret != 0) {
+ if (ret) {
dev_err(dev, "Failed to sync INT latch registers: %d\n",
ret);
return ret;
@@ -1085,7 +1085,7 @@ static int pca953x_regcache_sync(struct device *dev)
ret = regcache_sync_region(chip->regmap, PCAL953X_INT_MASK,
PCAL953X_INT_MASK + NBANK(chip));
- if (ret != 0) {
+ if (ret) {
dev_err(dev, "Failed to sync INT mask registers: %d\n",
ret);
return ret;
@@ -1117,7 +1117,7 @@ static int pca953x_resume(struct device *dev)
if (!atomic_read(&chip->wakeup_path)) {
ret = regulator_enable(chip->regulator);
- if (ret != 0) {
+ if (ret) {
dev_err(dev, "Failed to enable regulator: %d\n", ret);
return 0;
}
@@ -1130,7 +1130,7 @@ static int pca953x_resume(struct device *dev)
return ret;
ret = regcache_sync(chip->regmap);
- if (ret != 0) {
+ if (ret) {
dev_err(dev, "Failed to restore register map: %d\n", ret);
return ret;
}
--
2.20.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox