* Re: [PATCH] dt-bindings: pinctrl: aspeed: Fix 'compatible' schema errors
From: Rob Herring @ 2019-07-16 15:04 UTC (permalink / raw)
To: Joel Stanley
Cc: devicetree, Linus Walleij, Andrew Jeffery, linux-aspeed,
open list:GPIO SUBSYSTEM, Linux ARM
In-Reply-To: <CACPK8Xdz98CQzgE2KCjz8eOhPtx=H8jTe1hVT7LvP77U_gGASQ@mail.gmail.com>
On Mon, Jul 15, 2019 at 5:17 PM Joel Stanley <joel@jms.id.au> wrote:
>
> On Mon, 15 Jul 2019 at 22:37, Rob Herring <robh@kernel.org> wrote:
> >
> > The Aspeed pinctl schema have errors in the 'compatible' schema:
> >
> > Documentation/devicetree/bindings/pinctrl/aspeed,ast2400-pinctrl.yaml: \
> > properties:compatible:enum: ['aspeed', 'ast2400-pinctrl', 'aspeed', 'g4-pinctrl'] has non-unique elements
> > Documentation/devicetree/bindings/pinctrl/aspeed,ast2500-pinctrl.yaml: \
> > properties:compatible:enum: ['aspeed', 'ast2500-pinctrl', 'aspeed', 'g5-pinctrl'] has non-unique elements
> >
> > Flow style sequences have to be quoted if the vales contain ','. Fix
> > this by using the more common one line per entry formatting.
>
> >
> > properties:
> > compatible:
> > - enum: [ aspeed,ast2400-pinctrl, aspeed,g4-pinctrl ]
> > + enum:
> > + - aspeed,ast2400-pinctrl
> > + - aspeed,g4-pinctrl
>
> Thanks for the fix. However, we've standardised on the first form for
> all of our device trees, so we can drop the second compatible string
> from the bindings.
Doing that would introduce validation warnings until the dts file is
updated. So we still need this change until that happens.
Rob
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Dmitry Osipenko @ 2019-07-16 15:00 UTC (permalink / raw)
To: Peter De Schrijver, Joseph Lo, Sowjanya Komatineni
Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland, pgaikwad, sboyd, linux-clk,
linux-gpio, jckuo, talho, linux-tegra, linux-kernel, mperttunen,
spatra, robh+dt, devicetree
In-Reply-To: <20190716080610.GE12715@pdeschrijver-desktop.Nvidia.com>
16.07.2019 11:06, Peter De Schrijver пишет:
> On Tue, Jul 16, 2019 at 03:24:26PM +0800, Joseph Lo wrote:
>>> OK, Will add to CPUFreq driver...
>>>>
>>>> The other thing that also need attention is that T124 CPUFreq driver
>>>> implicitly relies on DFLL driver to be probed first, which is icky.
>>>>
>>> Should I add check for successful dfll clk register explicitly in
>>> CPUFreq driver probe and defer till dfll clk registers?
Probably you should use the "device links". See [1][2] for the example.
[1]
https://elixir.bootlin.com/linux/v5.2.1/source/drivers/gpu/drm/tegra/dc.c#L2383
[2] https://www.kernel.org/doc/html/latest/driver-api/device_link.html
Return EPROBE_DEFER instead of EINVAL if device_link_add() fails. And
use of_find_device_by_node() to get the DFLL's device, see [3].
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/devfreq/tegra20-devfreq.c#n100
>> Sorry, I didn't follow the mail thread. Just regarding the DFLL part.
>>
>> As you know it, the DFLL clock is one of the CPU clock sources and
>> integrated with DVFS control logic with the regulator. We will not switch
>> CPU to other clock sources once we switched to DFLL. Because the CPU has
>> been regulated by the DFLL HW with the DVFS table (CVB or OPP table you see
>> in the driver.). We shouldn't reparent it to other sources with unknew
>> freq/volt pair. That's not guaranteed to work. We allow switching to
>> open-loop mode but different sources.
Okay, then the CPUFreq driver will have to enforce DFLL freq to PLLP's
rate before switching to PLLP in order to have a proper CPU voltage.
>> And I don't exactly understand why we need to switch to PLLP in CPU idle
>> driver. Just keep it on CL-DVFS mode all the time.
>>
>> In SC7 entry, the dfll suspend function moves it the open-loop mode. That's
>> all. The sc7-entryfirmware will handle the rest of the sequence to turn off
>> the CPU power.
>>
>> In SC7 resume, the warmboot code will handle the sequence to turn on
>> regulator and power up the CPU cluster. And leave it on PLL_P. After
>> resuming to the kernel, we re-init DFLL, restore the CPU clock policy (CPU
>> runs on DFLL open-loop mode) and then moving to close-loop mode.
The DFLL is re-inited after switching CCLK to DFLL parent during of the
early clocks-state restoring by CaR driver. Hence instead of having odd
hacks in the CaR driver, it is much nicer to have a proper
suspend-resume sequencing of the device drivers. In this case CPUFreq
driver is the driver that enables DFLL and switches CPU to that clock
source, which means that this driver is also should be responsible for
management of the DFLL's state during of suspend/resume process. If
CPUFreq driver disables DFLL during suspend and re-enables it during
resume, then looks like the CaR driver hacks around DFLL are not needed.
>> The DFLL part looks good to me. BTW, change the patch subject to "Add
>> suspend-resume support" seems more appropriate to me.
>>
>
> To clarify this, the sequences for DFLL use are as follows (assuming all
> required DFLL hw configuration has been done)
>
> Switch to DFLL:
> 0) Save current parent and frequency
> 1) Program DFLL to open loop mode
> 2) Enable DFLL
> 3) Change cclk_g parent to DFLL
> For OVR regulator:
> 4) Change PWM output pin from tristate to output
> 5) Enable DFLL PWM output
> For I2C regulator:
> 4) Enable DFLL I2C output
> 6) Program DFLL to closed loop mode
>
> Switch away from DFLL:
> 0) Change cclk_g parent to PLLP so the CPU frequency is ok for any vdd_cpu voltage
> 1) Program DFLL to open loop mode
>
> For OVR regulator:
> 2) Change PWM output pin from output to tristate: vdd_cpu will go back
> to hardwired boot voltage.
> 3) Disable DFLL PWM output
>
> For I2C regulator:
> 2) Program vdd_cpu regulator voltage to the boot voltage
> 3) Disable DFLL I2C output
>
> 4) Reprogram parent saved in step 0 of 'Switch to DFLL' to the saved
> frequency
> 5) Change cclk_g parent to saved parent
> 6) Disable DFLL
Thanks!
^ permalink raw reply
* linusw/for-next boot: 42 boots: 0 failed, 42 passed (v5.2-10813-g88785b7fa74a)
From: kernelci.org bot @ 2019-07-16 13:11 UTC (permalink / raw)
To: linux-gpio, fellows
linusw/for-next boot: 42 boots: 0 failed, 42 passed (v5.2-10813-g88785b7fa74a)
Full Boot Summary: https://kernelci.org/boot/all/job/linusw/branch/for-next/kernel/v5.2-10813-g88785b7fa74a/
Full Build Summary: https://kernelci.org/build/linusw/branch/for-next/kernel/v5.2-10813-g88785b7fa74a/
Tree: linusw
Branch: for-next
Git Describe: v5.2-10813-g88785b7fa74a
Git Commit: 88785b7fa74ae2dc52f879140b976984b5374c79
Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/
Tested: 29 unique boards, 15 SoC families, 3 builds out of 6
---
For more info write to <info@kernelci.org>
^ permalink raw reply
* linusw/devel boot: 41 boots: 0 failed, 41 passed (v5.2-10809-g1507704f1e98)
From: kernelci.org bot @ 2019-07-16 13:07 UTC (permalink / raw)
To: linux-gpio, fellows
linusw/devel boot: 41 boots: 0 failed, 41 passed (v5.2-10809-g1507704f1e98)
Full Boot Summary: https://kernelci.org/boot/all/job/linusw/branch/devel/kernel/v5.2-10809-g1507704f1e98/
Full Build Summary: https://kernelci.org/build/linusw/branch/devel/kernel/v5.2-10809-g1507704f1e98/
Tree: linusw
Branch: devel
Git Describe: v5.2-10809-g1507704f1e98
Git Commit: 1507704f1e980da91f3888ba35e4c6d34e2f9e16
Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/
Tested: 28 unique boards, 15 SoC families, 3 builds out of 6
---
For more info write to <info@kernelci.org>
^ permalink raw reply
* linusw/fixes boot: 43 boots: 0 failed, 43 passed (v5.2-10813-g88785b7fa74a)
From: kernelci.org bot @ 2019-07-16 13:06 UTC (permalink / raw)
To: linux-gpio, fellows
linusw/fixes boot: 43 boots: 0 failed, 43 passed (v5.2-10813-g88785b7fa74a)
Full Boot Summary: https://kernelci.org/boot/all/job/linusw/branch/fixes/kernel/v5.2-10813-g88785b7fa74a/
Full Build Summary: https://kernelci.org/build/linusw/branch/fixes/kernel/v5.2-10813-g88785b7fa74a/
Tree: linusw
Branch: fixes
Git Describe: v5.2-10813-g88785b7fa74a
Git Commit: 88785b7fa74ae2dc52f879140b976984b5374c79
Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/
Tested: 29 unique boards, 15 SoC families, 3 builds out of 6
---
For more info write to <info@kernelci.org>
^ permalink raw reply
* [PATCH] gpio: of: Break out OF-only code
From: Linus Walleij @ 2019-07-16 12:39 UTC (permalink / raw)
To: linux-gpio; +Cc: Bartosz Golaszewski, Linus Walleij, Andy Shevchenko
The core gpiolib should not contain any OF/device tree-only
code. Try to break out the main part of it and push it down
into the optional gpiolib-of.c part of the library.
Create a local gpiolib-of.h header and move stuff around a
bit to get a clean cut.
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib-of.c | 114 ++++++++++++++++++++++++++++++++++++++
drivers/gpio/gpiolib.c | 113 +++----------------------------------
drivers/gpio/gpiolib.h | 27 ---------
3 files changed, 122 insertions(+), 132 deletions(-)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 2bc0bcd7a410..00ed60304cd4 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -21,6 +21,34 @@
#include <linux/gpio/machine.h>
#include "gpiolib.h"
+#include "gpiolib-of.h"
+
+/*
+ * This is used by external users of of_gpio_count() from <linux/of_gpio.h>
+ *
+ * FIXME: get rid of those external users by converting them to GPIO
+ * descriptors and let them all use gpiod_get_count()
+ */
+int of_gpio_get_count(struct device *dev, const char *con_id)
+{
+ int ret;
+ char propname[32];
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
+ if (con_id)
+ snprintf(propname, sizeof(propname), "%s-%s",
+ con_id, gpio_suffixes[i]);
+ else
+ snprintf(propname, sizeof(propname), "%s",
+ gpio_suffixes[i]);
+
+ ret = of_gpio_named_count(dev->of_node, propname);
+ if (ret > 0)
+ break;
+ }
+ return ret ? ret : -ENOENT;
+}
static int of_gpiochip_match_node_and_xlate(struct gpio_chip *chip, void *data)
{
@@ -53,6 +81,23 @@ static struct gpio_desc *of_xlate_and_get_gpiod_flags(struct gpio_chip *chip,
return gpiochip_get_desc(chip, ret);
}
+/**
+ * of_gpio_need_valid_mask() - figure out if the OF GPIO driver needs
+ * to set the .valid_mask
+ * @dev: the device for the GPIO provider
+ * @return: true if the valid mask needs to be set
+ */
+bool of_gpio_need_valid_mask(struct gpio_chip *gc)
+{
+ int size;
+ struct device_node *np = gc->of_node;
+
+ size = of_property_count_u32_elems(np, "gpio-reserved-ranges");
+ if (size > 0 && size % 2 == 0)
+ return true;
+ return false;
+}
+
static void of_gpio_flags_quirks(struct device_node *np,
const char *propname,
enum of_gpio_flags *flags,
@@ -237,6 +282,75 @@ int of_get_named_gpio_flags(struct device_node *np, const char *list_name,
}
EXPORT_SYMBOL(of_get_named_gpio_flags);
+/**
+ * gpiod_get_from_of_node() - obtain a GPIO from an OF node
+ * @node: handle of the OF node
+ * @propname: name of the DT property representing the GPIO
+ * @index: index of the GPIO to obtain for the consumer
+ * @dflags: GPIO initialization flags
+ * @label: label to attach to the requested GPIO
+ *
+ * Returns:
+ * On successful request the GPIO pin is configured in accordance with
+ * provided @dflags.
+ *
+ * In case of error an ERR_PTR() is returned.
+ */
+struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
+ const char *propname, int index,
+ enum gpiod_flags dflags,
+ const char *label)
+{
+ unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
+ struct gpio_desc *desc;
+ enum of_gpio_flags flags;
+ bool active_low = false;
+ bool single_ended = false;
+ bool open_drain = false;
+ bool transitory = false;
+ int ret;
+
+ desc = of_get_named_gpiod_flags(node, propname,
+ index, &flags);
+
+ if (!desc || IS_ERR(desc)) {
+ return desc;
+ }
+
+ active_low = flags & OF_GPIO_ACTIVE_LOW;
+ single_ended = flags & OF_GPIO_SINGLE_ENDED;
+ open_drain = flags & OF_GPIO_OPEN_DRAIN;
+ transitory = flags & OF_GPIO_TRANSITORY;
+
+ ret = gpiod_request(desc, label);
+ if (ret == -EBUSY && (flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE))
+ return desc;
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (active_low)
+ lflags |= GPIO_ACTIVE_LOW;
+
+ if (single_ended) {
+ if (open_drain)
+ lflags |= GPIO_OPEN_DRAIN;
+ else
+ lflags |= GPIO_OPEN_SOURCE;
+ }
+
+ if (transitory)
+ lflags |= GPIO_TRANSITORY;
+
+ ret = gpiod_configure_flags(desc, propname, lflags, dflags);
+ if (ret < 0) {
+ gpiod_put(desc);
+ return ERR_PTR(ret);
+ }
+
+ return desc;
+}
+EXPORT_SYMBOL(gpiod_get_from_of_node);
+
/*
* The SPI GPIO bindings happened before we managed to establish that GPIO
* properties should be named "foo-gpios" so we have this special kludge for
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 24300f401fce..3e262a280f95 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -11,7 +11,6 @@
#include <linux/debugfs.h>
#include <linux/seq_file.h>
#include <linux/gpio.h>
-#include <linux/of_gpio.h>
#include <linux/idr.h>
#include <linux/slab.h>
#include <linux/acpi.h>
@@ -30,6 +29,7 @@
#include <uapi/linux/gpio.h>
#include "gpiolib.h"
+#include "gpiolib-of.h"
#define CREATE_TRACE_POINTS
#include <trace/events/gpio.h>
@@ -360,22 +360,15 @@ static unsigned long *gpiochip_allocate_mask(struct gpio_chip *chip)
return p;
}
-static int gpiochip_alloc_valid_mask(struct gpio_chip *gpiochip)
+static int gpiochip_alloc_valid_mask(struct gpio_chip *gc)
{
-#ifdef CONFIG_OF_GPIO
- int size;
- struct device_node *np = gpiochip->of_node;
-
- size = of_property_count_u32_elems(np, "gpio-reserved-ranges");
- if (size > 0 && size % 2 == 0)
- gpiochip->need_valid_mask = true;
-#endif
-
- if (!gpiochip->need_valid_mask)
+ if (IS_ENABLED(CONFIG_OF_GPIO))
+ gc->need_valid_mask = of_gpio_need_valid_mask(gc);
+ if (!gc->need_valid_mask)
return 0;
- gpiochip->valid_mask = gpiochip_allocate_mask(gpiochip);
- if (!gpiochip->valid_mask)
+ gc->valid_mask = gpiochip_allocate_mask(gc);
+ if (!gc->valid_mask)
return -ENOMEM;
return 0;
@@ -3993,27 +3986,6 @@ static struct gpio_desc *gpiod_find(struct device *dev, const char *con_id,
return desc;
}
-static int dt_gpio_count(struct device *dev, const char *con_id)
-{
- int ret;
- char propname[32];
- unsigned int i;
-
- for (i = 0; i < ARRAY_SIZE(gpio_suffixes); i++) {
- if (con_id)
- snprintf(propname, sizeof(propname), "%s-%s",
- con_id, gpio_suffixes[i]);
- else
- snprintf(propname, sizeof(propname), "%s",
- gpio_suffixes[i]);
-
- ret = of_gpio_named_count(dev->of_node, propname);
- if (ret > 0)
- break;
- }
- return ret ? ret : -ENOENT;
-}
-
static int platform_gpio_count(struct device *dev, const char *con_id)
{
struct gpiod_lookup_table *table;
@@ -4046,7 +4018,7 @@ int gpiod_count(struct device *dev, const char *con_id)
int count = -ENOENT;
if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
- count = dt_gpio_count(dev, con_id);
+ count = of_gpio_get_count(dev, con_id);
else if (IS_ENABLED(CONFIG_ACPI) && dev && ACPI_HANDLE(dev))
count = acpi_gpio_count(dev, con_id);
@@ -4247,75 +4219,6 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
}
EXPORT_SYMBOL_GPL(gpiod_get_index);
-/**
- * gpiod_get_from_of_node() - obtain a GPIO from an OF node
- * @node: handle of the OF node
- * @propname: name of the DT property representing the GPIO
- * @index: index of the GPIO to obtain for the consumer
- * @dflags: GPIO initialization flags
- * @label: label to attach to the requested GPIO
- *
- * Returns:
- * On successful request the GPIO pin is configured in accordance with
- * provided @dflags.
- *
- * In case of error an ERR_PTR() is returned.
- */
-struct gpio_desc *gpiod_get_from_of_node(struct device_node *node,
- const char *propname, int index,
- enum gpiod_flags dflags,
- const char *label)
-{
- unsigned long lflags = GPIO_LOOKUP_FLAGS_DEFAULT;
- struct gpio_desc *desc;
- enum of_gpio_flags flags;
- bool active_low = false;
- bool single_ended = false;
- bool open_drain = false;
- bool transitory = false;
- int ret;
-
- desc = of_get_named_gpiod_flags(node, propname,
- index, &flags);
-
- if (!desc || IS_ERR(desc)) {
- return desc;
- }
-
- active_low = flags & OF_GPIO_ACTIVE_LOW;
- single_ended = flags & OF_GPIO_SINGLE_ENDED;
- open_drain = flags & OF_GPIO_OPEN_DRAIN;
- transitory = flags & OF_GPIO_TRANSITORY;
-
- ret = gpiod_request(desc, label);
- if (ret == -EBUSY && (flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE))
- return desc;
- if (ret)
- return ERR_PTR(ret);
-
- if (active_low)
- lflags |= GPIO_ACTIVE_LOW;
-
- if (single_ended) {
- if (open_drain)
- lflags |= GPIO_OPEN_DRAIN;
- else
- lflags |= GPIO_OPEN_SOURCE;
- }
-
- if (transitory)
- lflags |= GPIO_TRANSITORY;
-
- ret = gpiod_configure_flags(desc, propname, lflags, dflags);
- if (ret < 0) {
- gpiod_put(desc);
- return ERR_PTR(ret);
- }
-
- return desc;
-}
-EXPORT_SYMBOL(gpiod_get_from_of_node);
-
/**
* fwnode_get_named_gpiod - obtain a GPIO from firmware node
* @fwnode: handle of the firmware node
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 7c52c2442173..75d2e909d8c6 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -16,7 +16,6 @@
#include <linux/module.h>
#include <linux/cdev.h>
-enum of_gpio_flags;
struct acpi_device;
/**
@@ -92,32 +91,6 @@ struct acpi_gpio_info {
/* gpio suffixes used for ACPI and device tree lookup */
static __maybe_unused const char * const gpio_suffixes[] = { "gpios", "gpio" };
-#ifdef CONFIG_OF_GPIO
-struct gpio_desc *of_find_gpio(struct device *dev,
- const char *con_id,
- unsigned int idx,
- unsigned long *lookupflags);
-struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
- const char *list_name, int index, enum of_gpio_flags *flags);
-int of_gpiochip_add(struct gpio_chip *gc);
-void of_gpiochip_remove(struct gpio_chip *gc);
-#else
-static inline struct gpio_desc *of_find_gpio(struct device *dev,
- const char *con_id,
- unsigned int idx,
- unsigned long *lookupflags)
-{
- return ERR_PTR(-ENOENT);
-}
-static inline struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,
- const char *list_name, int index, enum of_gpio_flags *flags)
-{
- return ERR_PTR(-ENOENT);
-}
-static inline int of_gpiochip_add(struct gpio_chip *gc) { return 0; }
-static inline void of_gpiochip_remove(struct gpio_chip *gc) { }
-#endif /* CONFIG_OF_GPIO */
-
#ifdef CONFIG_ACPI
void acpi_gpiochip_add(struct gpio_chip *chip);
void acpi_gpiochip_remove(struct gpio_chip *chip);
--
2.21.0
^ permalink raw reply related
* linusw/for-next build: 6 builds: 0 failed, 6 passed, 3 warnings (v5.2-10813-g88785b7fa74a)
From: kernelci.org bot @ 2019-07-16 12:26 UTC (permalink / raw)
To: linux-gpio, fellows
linusw/for-next build: 6 builds: 0 failed, 6 passed, 3 warnings (v5.2-10813-g88785b7fa74a)
Full Build Summary: https://kernelci.org/build/linusw/branch/for-next/kernel/v5.2-10813-g88785b7fa74a/
Tree: linusw
Branch: for-next
Git Describe: v5.2-10813-g88785b7fa74a
Git Commit: 88785b7fa74ae2dc52f879140b976984b5374c79
Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/
Built: 6 unique architectures
Warnings Detected:
arc:
arm64:
arm:
mips:
32r2el_defconfig (gcc-8): 3 warnings
riscv:
x86_64:
Warnings summary:
3 <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
================================================================================
Detailed per-defconfig build reports:
--------------------------------------------------------------------------------
32r2el_defconfig (mips, gcc-8) — PASS, 0 errors, 3 warnings, 0 section mismatches
Warnings:
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
--------------------------------------------------------------------------------
defconfig (riscv, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
defconfig (arm64, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
multi_v7_defconfig (arm, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
nsim_hs_defconfig (arc, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
x86_64_defconfig (x86_64, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
---
For more info write to <info@kernelci.org>
^ permalink raw reply
* linusw/devel build: 6 builds: 0 failed, 6 passed, 3 warnings (v5.2-10809-g1507704f1e98)
From: kernelci.org bot @ 2019-07-16 12:22 UTC (permalink / raw)
To: linux-gpio, fellows
linusw/devel build: 6 builds: 0 failed, 6 passed, 3 warnings (v5.2-10809-g1507704f1e98)
Full Build Summary: https://kernelci.org/build/linusw/branch/devel/kernel/v5.2-10809-g1507704f1e98/
Tree: linusw
Branch: devel
Git Describe: v5.2-10809-g1507704f1e98
Git Commit: 1507704f1e980da91f3888ba35e4c6d34e2f9e16
Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/
Built: 6 unique architectures
Warnings Detected:
arc:
arm64:
arm:
mips:
32r2el_defconfig (gcc-8): 3 warnings
riscv:
x86_64:
Warnings summary:
3 <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
================================================================================
Detailed per-defconfig build reports:
--------------------------------------------------------------------------------
32r2el_defconfig (mips, gcc-8) — PASS, 0 errors, 3 warnings, 0 section mismatches
Warnings:
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
--------------------------------------------------------------------------------
defconfig (riscv, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
defconfig (arm64, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
multi_v7_defconfig (arm, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
nsim_hs_defconfig (arc, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
x86_64_defconfig (x86_64, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
---
For more info write to <info@kernelci.org>
^ permalink raw reply
* linusw/fixes build: 6 builds: 0 failed, 6 passed, 3 warnings (v5.2-10813-g88785b7fa74a)
From: kernelci.org bot @ 2019-07-16 12:22 UTC (permalink / raw)
To: linux-gpio, fellows
linusw/fixes build: 6 builds: 0 failed, 6 passed, 3 warnings (v5.2-10813-g88785b7fa74a)
Full Build Summary: https://kernelci.org/build/linusw/branch/fixes/kernel/v5.2-10813-g88785b7fa74a/
Tree: linusw
Branch: fixes
Git Describe: v5.2-10813-g88785b7fa74a
Git Commit: 88785b7fa74ae2dc52f879140b976984b5374c79
Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/
Built: 6 unique architectures
Warnings Detected:
arc:
arm64:
arm:
mips:
32r2el_defconfig (gcc-8): 3 warnings
riscv:
x86_64:
Warnings summary:
3 <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
================================================================================
Detailed per-defconfig build reports:
--------------------------------------------------------------------------------
32r2el_defconfig (mips, gcc-8) — PASS, 0 errors, 3 warnings, 0 section mismatches
Warnings:
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
<stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
--------------------------------------------------------------------------------
defconfig (riscv, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
defconfig (arm64, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
multi_v7_defconfig (arm, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
nsim_hs_defconfig (arc, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
--------------------------------------------------------------------------------
x86_64_defconfig (x86_64, gcc-8) — PASS, 0 errors, 0 warnings, 0 section mismatches
---
For more info write to <info@kernelci.org>
^ permalink raw reply
* [PATCH] gpio: of: Normalize return code variable name
From: Linus Walleij @ 2019-07-16 11:58 UTC (permalink / raw)
To: linux-gpio; +Cc: Bartosz Golaszewski, Linus Walleij
It is confusing to name return variables mixedly "status",
"err" or "ret". I just changed them all to "ret", by personal
preference, to lower cognitive stress.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib-of.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index f974075ff00e..2bc0bcd7a410 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -740,7 +740,7 @@ static int of_gpiochip_add_pin_range(struct gpio_chip *chip) { return 0; }
int of_gpiochip_add(struct gpio_chip *chip)
{
- int status;
+ int ret;
if (!chip->of_node)
return 0;
@@ -755,9 +755,9 @@ int of_gpiochip_add(struct gpio_chip *chip)
of_gpiochip_init_valid_mask(chip);
- status = of_gpiochip_add_pin_range(chip);
- if (status)
- return status;
+ ret = of_gpiochip_add_pin_range(chip);
+ if (ret)
+ return ret;
/* If the chip defines names itself, these take precedence */
if (!chip->names)
@@ -766,13 +766,13 @@ int of_gpiochip_add(struct gpio_chip *chip)
of_node_get(chip->of_node);
- status = of_gpiochip_scan_gpios(chip);
- if (status) {
+ ret = of_gpiochip_scan_gpios(chip);
+ if (ret) {
of_node_put(chip->of_node);
gpiochip_remove_pin_ranges(chip);
}
- return status;
+ return ret;
}
void of_gpiochip_remove(struct gpio_chip *chip)
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] dt-bindings: Ensure child nodes are of type 'object'
From: Mark Brown @ 2019-07-16 11:09 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
David Woodhouse, Brian Norris, Marek Vasut, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Linus Walleij,
Maxime Coquelin, Alexandre Torgue, linux-mtd, linux-gpio,
linux-stm32, linux-spi
In-Reply-To: <20190715230457.3901-1-robh@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 375 bytes --]
On Mon, Jul 15, 2019 at 05:04:57PM -0600, Rob Herring wrote:
> Properties which are child node definitions need to have an explict
> type. Otherwise, a matching (DT) property can silently match when an
> error is desired. Fix this up tree-wide. Once this is fixed, the
> meta-schema will enforce this on any child node definitions.
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [GIT PULL] gpio fixes for v5.3-rc1
From: Linus Walleij @ 2019-07-16 9:13 UTC (permalink / raw)
To: Bartosz Golaszewski; +Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski
In-Reply-To: <20190715095903.18307-1-brgl@bgdev.pl>
On Mon, Jul 15, 2019 at 11:59 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> please pull the following fixes applied to my tree during this merge window.
>
> The following changes since commit fec88ab0af9706b2201e5daf377c5031c62d11f7:
>
> Merge tag 'for-linus-hmm' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma (2019-07-14 19:42:11 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git tags/gpio-v5.3-rc1-fixes-for-linus
>
> for you to fetch changes up to 19ec11a2233d24a7811836fa735203aaccf95a23:
>
> gpio: em: remove the gpiochip before removing the irq domain (2019-07-15 11:52:42 +0200)
Pulled into my "fixes" branch, excellent, thanks!
Linus Walleij
^ permalink raw reply
* [PATCH] gpio: gpiolib: Normalize return code variable name
From: Linus Walleij @ 2019-07-16 9:11 UTC (permalink / raw)
To: linux-gpio; +Cc: Bartosz Golaszewski, Linus Walleij
It is confusing to name return variables mixedly "status",
"err" or "ret". I just changed them all to "ret", by personal
preference, to lower cognitive stress.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpiolib.c | 220 ++++++++++++++++++++---------------------
1 file changed, 110 insertions(+), 110 deletions(-)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3ee99d070608..24300f401fce 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -213,7 +213,7 @@ int gpiod_get_direction(struct gpio_desc *desc)
{
struct gpio_chip *chip;
unsigned offset;
- int status;
+ int ret;
chip = gpiod_to_chip(desc);
offset = gpio_chip_hwgpio(desc);
@@ -221,17 +221,17 @@ int gpiod_get_direction(struct gpio_desc *desc)
if (!chip->get_direction)
return -ENOTSUPP;
- status = chip->get_direction(chip, offset);
- if (status > 0) {
+ ret = chip->get_direction(chip, offset);
+ if (ret > 0) {
/* GPIOF_DIR_IN, or other positive */
- status = 1;
+ ret = 1;
clear_bit(FLAG_IS_OUT, &desc->flags);
}
- if (status == 0) {
+ if (ret == 0) {
/* GPIOF_DIR_OUT */
set_bit(FLAG_IS_OUT, &desc->flags);
}
- return status;
+ return ret;
}
EXPORT_SYMBOL_GPL(gpiod_get_direction);
@@ -1172,21 +1172,21 @@ static void gpiodevice_release(struct device *dev)
static int gpiochip_setup_dev(struct gpio_device *gdev)
{
- int status;
+ int ret;
cdev_init(&gdev->chrdev, &gpio_fileops);
gdev->chrdev.owner = THIS_MODULE;
gdev->dev.devt = MKDEV(MAJOR(gpio_devt), gdev->id);
- status = cdev_device_add(&gdev->chrdev, &gdev->dev);
- if (status)
- return status;
+ ret = cdev_device_add(&gdev->chrdev, &gdev->dev);
+ if (ret)
+ return ret;
chip_dbg(gdev->chip, "added GPIO chardev (%d:%d)\n",
MAJOR(gpio_devt), gdev->id);
- status = gpiochip_sysfs_register(gdev);
- if (status)
+ ret = gpiochip_sysfs_register(gdev);
+ if (ret)
goto err_remove_device;
/* From this point, the .release() function cleans up gpio_device */
@@ -1199,7 +1199,7 @@ static int gpiochip_setup_dev(struct gpio_device *gdev)
err_remove_device:
cdev_device_del(&gdev->chrdev, &gdev->dev);
- return status;
+ return ret;
}
static void gpiochip_machine_hog(struct gpio_chip *chip, struct gpiod_hog *hog)
@@ -1240,13 +1240,13 @@ static void machine_gpiochip_add(struct gpio_chip *chip)
static void gpiochip_setup_devs(void)
{
struct gpio_device *gdev;
- int err;
+ int ret;
list_for_each_entry(gdev, &gpio_devices, list) {
- err = gpiochip_setup_dev(gdev);
- if (err)
+ ret = gpiochip_setup_dev(gdev);
+ if (ret)
pr_err("%s: Failed to initialize gpio device (%d)\n",
- dev_name(&gdev->dev), err);
+ dev_name(&gdev->dev), ret);
}
}
@@ -1255,7 +1255,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
struct lock_class_key *request_key)
{
unsigned long flags;
- int status = 0;
+ int ret = 0;
unsigned i;
int base = chip->base;
struct gpio_device *gdev;
@@ -1285,7 +1285,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
gdev->id = ida_simple_get(&gpio_ida, 0, 0, GFP_KERNEL);
if (gdev->id < 0) {
- status = gdev->id;
+ ret = gdev->id;
goto err_free_gdev;
}
dev_set_name(&gdev->dev, "gpiochip%d", gdev->id);
@@ -1301,13 +1301,13 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
gdev->descs = kcalloc(chip->ngpio, sizeof(gdev->descs[0]), GFP_KERNEL);
if (!gdev->descs) {
- status = -ENOMEM;
+ ret = -ENOMEM;
goto err_free_ida;
}
if (chip->ngpio == 0) {
chip_err(chip, "tried to insert a GPIO chip with zero lines\n");
- status = -EINVAL;
+ ret = -EINVAL;
goto err_free_descs;
}
@@ -1317,7 +1317,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
gdev->label = kstrdup_const(chip->label ?: "unknown", GFP_KERNEL);
if (!gdev->label) {
- status = -ENOMEM;
+ ret = -ENOMEM;
goto err_free_descs;
}
@@ -1336,7 +1336,7 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
if (base < 0) {
base = gpiochip_find_base(chip->ngpio);
if (base < 0) {
- status = base;
+ ret = base;
spin_unlock_irqrestore(&gpio_lock, flags);
goto err_free_label;
}
@@ -1350,8 +1350,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
}
gdev->base = base;
- status = gpiodev_add_to_list(gdev);
- if (status) {
+ ret = gpiodev_add_to_list(gdev);
+ if (ret) {
spin_unlock_irqrestore(&gpio_lock, flags);
goto err_free_label;
}
@@ -1365,28 +1365,28 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
INIT_LIST_HEAD(&gdev->pin_ranges);
#endif
- status = gpiochip_set_desc_names(chip);
- if (status)
+ ret = gpiochip_set_desc_names(chip);
+ if (ret)
goto err_remove_from_list;
- status = gpiochip_irqchip_init_valid_mask(chip);
- if (status)
+ ret = gpiochip_irqchip_init_valid_mask(chip);
+ if (ret)
goto err_remove_from_list;
- status = gpiochip_alloc_valid_mask(chip);
- if (status)
+ ret = gpiochip_alloc_valid_mask(chip);
+ if (ret)
goto err_remove_irqchip_mask;
- status = gpiochip_add_irqchip(chip, lock_key, request_key);
- if (status)
+ ret = gpiochip_add_irqchip(chip, lock_key, request_key);
+ if (ret)
goto err_free_gpiochip_mask;
- status = of_gpiochip_add(chip);
- if (status)
+ ret = of_gpiochip_add(chip);
+ if (ret)
goto err_remove_chip;
- status = gpiochip_init_valid_mask(chip);
- if (status)
+ ret = gpiochip_init_valid_mask(chip);
+ if (ret)
goto err_remove_of_chip;
for (i = 0; i < chip->ngpio; i++) {
@@ -1413,8 +1413,8 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
* Otherwise, defer until later.
*/
if (gpiolib_initialized) {
- status = gpiochip_setup_dev(gdev);
- if (status)
+ ret = gpiochip_setup_dev(gdev);
+ if (ret)
goto err_remove_acpi_chip;
}
return 0;
@@ -1444,9 +1444,9 @@ int gpiochip_add_data_with_key(struct gpio_chip *chip, void *data,
/* failures here can mean systems won't boot... */
pr_err("%s: GPIOs %d..%d (%s) failed to register, %d\n", __func__,
gdev->base, gdev->base + gdev->ngpio - 1,
- chip->label ? : "generic", status);
+ chip->label ? : "generic", ret);
kfree(gdev);
- return status;
+ return ret;
}
EXPORT_SYMBOL_GPL(gpiochip_add_data_with_key);
@@ -1740,7 +1740,7 @@ int gpiochip_irq_map(struct irq_domain *d, unsigned int irq,
irq_hw_number_t hwirq)
{
struct gpio_chip *chip = d->host_data;
- int err = 0;
+ int ret = 0;
if (!gpiochip_irqchip_irq_valid(chip, hwirq))
return -ENXIO;
@@ -1758,12 +1758,12 @@ int gpiochip_irq_map(struct irq_domain *d, unsigned int irq,
irq_set_noprobe(irq);
if (chip->irq.num_parents == 1)
- err = irq_set_parent(irq, chip->irq.parents[0]);
+ ret = irq_set_parent(irq, chip->irq.parents[0]);
else if (chip->irq.map)
- err = irq_set_parent(irq, chip->irq.map[hwirq]);
+ ret = irq_set_parent(irq, chip->irq.map[hwirq]);
- if (err < 0)
- return err;
+ if (ret < 0)
+ return ret;
/*
* No set-up of the hardware will happen if IRQ_TYPE_NONE
@@ -2321,7 +2321,7 @@ EXPORT_SYMBOL_GPL(gpiochip_remove_pin_ranges);
static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
{
struct gpio_chip *chip = desc->gdev->chip;
- int status;
+ int ret;
unsigned long flags;
unsigned offset;
@@ -2339,10 +2339,10 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
if (test_and_set_bit(FLAG_REQUESTED, &desc->flags) == 0) {
desc_set_label(desc, label ? : "?");
- status = 0;
+ ret = 0;
} else {
kfree_const(label);
- status = -EBUSY;
+ ret = -EBUSY;
goto done;
}
@@ -2351,12 +2351,12 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
spin_unlock_irqrestore(&gpio_lock, flags);
offset = gpio_chip_hwgpio(desc);
if (gpiochip_line_is_valid(chip, offset))
- status = chip->request(chip, offset);
+ ret = chip->request(chip, offset);
else
- status = -EINVAL;
+ ret = -EINVAL;
spin_lock_irqsave(&gpio_lock, flags);
- if (status < 0) {
+ if (ret < 0) {
desc_set_label(desc, NULL);
kfree_const(label);
clear_bit(FLAG_REQUESTED, &desc->flags);
@@ -2371,7 +2371,7 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
}
done:
spin_unlock_irqrestore(&gpio_lock, flags);
- return status;
+ return ret;
}
/*
@@ -2414,24 +2414,24 @@ static int validate_desc(const struct gpio_desc *desc, const char *func)
int gpiod_request(struct gpio_desc *desc, const char *label)
{
- int status = -EPROBE_DEFER;
+ int ret = -EPROBE_DEFER;
struct gpio_device *gdev;
VALIDATE_DESC(desc);
gdev = desc->gdev;
if (try_module_get(gdev->owner)) {
- status = gpiod_request_commit(desc, label);
- if (status < 0)
+ ret = gpiod_request_commit(desc, label);
+ if (ret < 0)
module_put(gdev->owner);
else
get_device(&gdev->dev);
}
- if (status)
- gpiod_dbg(desc, "%s: status %d\n", __func__, status);
+ if (ret)
+ gpiod_dbg(desc, "%s: status %d\n", __func__, ret);
- return status;
+ return ret;
}
static bool gpiod_free_commit(struct gpio_desc *desc)
@@ -2533,22 +2533,22 @@ struct gpio_desc *gpiochip_request_own_desc(struct gpio_chip *chip, u16 hwnum,
enum gpiod_flags dflags)
{
struct gpio_desc *desc = gpiochip_get_desc(chip, hwnum);
- int err;
+ int ret;
if (IS_ERR(desc)) {
chip_err(chip, "failed to get GPIO descriptor\n");
return desc;
}
- err = gpiod_request_commit(desc, label);
- if (err < 0)
- return ERR_PTR(err);
+ ret = gpiod_request_commit(desc, label);
+ if (ret < 0)
+ return ERR_PTR(ret);
- err = gpiod_configure_flags(desc, label, lflags, dflags);
- if (err) {
+ ret = gpiod_configure_flags(desc, label, lflags, dflags);
+ if (ret) {
chip_err(chip, "setup of own GPIO %s failed\n", label);
gpiod_free_commit(desc);
- return ERR_PTR(err);
+ return ERR_PTR(ret);
}
return desc;
@@ -2611,7 +2611,7 @@ static int gpio_set_config(struct gpio_chip *gc, unsigned offset,
int gpiod_direction_input(struct gpio_desc *desc)
{
struct gpio_chip *chip;
- int status = 0;
+ int ret = 0;
VALIDATE_DESC(desc);
chip = desc->gdev->chip;
@@ -2635,7 +2635,7 @@ int gpiod_direction_input(struct gpio_desc *desc)
* assume we are in input mode after this.
*/
if (chip->direction_input) {
- status = chip->direction_input(chip, gpio_chip_hwgpio(desc));
+ ret = chip->direction_input(chip, gpio_chip_hwgpio(desc));
} else if (chip->get_direction &&
(chip->get_direction(chip, gpio_chip_hwgpio(desc)) != 1)) {
gpiod_warn(desc,
@@ -2643,7 +2643,7 @@ int gpiod_direction_input(struct gpio_desc *desc)
__func__);
return -EIO;
}
- if (status == 0)
+ if (ret == 0)
clear_bit(FLAG_IS_OUT, &desc->flags);
if (test_bit(FLAG_PULL_UP, &desc->flags))
@@ -2653,9 +2653,9 @@ int gpiod_direction_input(struct gpio_desc *desc)
gpio_set_config(chip, gpio_chip_hwgpio(desc),
PIN_CONFIG_BIAS_PULL_DOWN);
- trace_gpio_direction(desc_to_gpio(desc), 1, status);
+ trace_gpio_direction(desc_to_gpio(desc), 1, ret);
- return status;
+ return ret;
}
EXPORT_SYMBOL_GPL(gpiod_direction_input);
@@ -2927,7 +2927,7 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
struct gpio_array *array_info,
unsigned long *value_bitmap)
{
- int err, i = 0;
+ int ret, i = 0;
/*
* Validate array_info against desc_array and its size.
@@ -2940,11 +2940,11 @@ int gpiod_get_array_value_complex(bool raw, bool can_sleep,
if (!can_sleep)
WARN_ON(array_info->chip->can_sleep);
- err = gpio_chip_get_multiple(array_info->chip,
+ ret = gpio_chip_get_multiple(array_info->chip,
array_info->get_mask,
value_bitmap);
- if (err)
- return err;
+ if (ret)
+ return ret;
if (!raw && !bitmap_empty(array_info->invert_mask, array_size))
bitmap_xor(value_bitmap, value_bitmap,
@@ -3132,24 +3132,24 @@ EXPORT_SYMBOL_GPL(gpiod_get_array_value);
*/
static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
{
- int err = 0;
+ int ret = 0;
struct gpio_chip *chip = desc->gdev->chip;
int offset = gpio_chip_hwgpio(desc);
if (value) {
- err = chip->direction_input(chip, offset);
- if (!err)
+ ret = chip->direction_input(chip, offset);
+ if (!ret)
clear_bit(FLAG_IS_OUT, &desc->flags);
} else {
- err = chip->direction_output(chip, offset, 0);
- if (!err)
+ ret = chip->direction_output(chip, offset, 0);
+ if (!ret)
set_bit(FLAG_IS_OUT, &desc->flags);
}
- trace_gpio_direction(desc_to_gpio(desc), value, err);
- if (err < 0)
+ trace_gpio_direction(desc_to_gpio(desc), value, ret);
+ if (ret < 0)
gpiod_err(desc,
"%s: Error in set_value for open drain err %d\n",
- __func__, err);
+ __func__, ret);
}
/*
@@ -3159,24 +3159,24 @@ static void gpio_set_open_drain_value_commit(struct gpio_desc *desc, bool value)
*/
static void gpio_set_open_source_value_commit(struct gpio_desc *desc, bool value)
{
- int err = 0;
+ int ret = 0;
struct gpio_chip *chip = desc->gdev->chip;
int offset = gpio_chip_hwgpio(desc);
if (value) {
- err = chip->direction_output(chip, offset, 1);
- if (!err)
+ ret = chip->direction_output(chip, offset, 1);
+ if (!ret)
set_bit(FLAG_IS_OUT, &desc->flags);
} else {
- err = chip->direction_input(chip, offset);
- if (!err)
+ ret = chip->direction_input(chip, offset);
+ if (!ret)
clear_bit(FLAG_IS_OUT, &desc->flags);
}
- trace_gpio_direction(desc_to_gpio(desc), !value, err);
- if (err < 0)
+ trace_gpio_direction(desc_to_gpio(desc), !value, ret);
+ if (ret < 0)
gpiod_err(desc,
"%s: Error in set_value for open source err %d\n",
- __func__, err);
+ __func__, ret);
}
static void gpiod_set_raw_value_commit(struct gpio_desc *desc, bool value)
@@ -4108,7 +4108,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_optional);
int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
unsigned long lflags, enum gpiod_flags dflags)
{
- int status;
+ int ret;
if (lflags & GPIO_ACTIVE_LOW)
set_bit(FLAG_ACTIVE_LOW, &desc->flags);
@@ -4141,9 +4141,9 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
else if (lflags & GPIO_PULL_DOWN)
set_bit(FLAG_PULL_DOWN, &desc->flags);
- status = gpiod_set_transitory(desc, (lflags & GPIO_TRANSITORY));
- if (status < 0)
- return status;
+ ret = gpiod_set_transitory(desc, (lflags & GPIO_TRANSITORY));
+ if (ret < 0)
+ return ret;
/* No particular flag request, return here... */
if (!(dflags & GPIOD_FLAGS_BIT_DIR_SET)) {
@@ -4153,12 +4153,12 @@ int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id,
/* Process flags */
if (dflags & GPIOD_FLAGS_BIT_DIR_OUT)
- status = gpiod_direction_output(desc,
+ ret = gpiod_direction_output(desc,
!!(dflags & GPIOD_FLAGS_BIT_DIR_VAL));
else
- status = gpiod_direction_input(desc);
+ ret = gpiod_direction_input(desc);
- return status;
+ return ret;
}
/**
@@ -4182,7 +4182,7 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
{
unsigned long lookupflags = GPIO_LOOKUP_FLAGS_DEFAULT;
struct gpio_desc *desc = NULL;
- int status;
+ int ret;
/* Maybe we have a device name, maybe not */
const char *devname = dev ? dev_name(dev) : "?";
@@ -4217,9 +4217,9 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
* If a connection label was passed use that, else attempt to use
* the device name as label
*/
- status = gpiod_request(desc, con_id ? con_id : devname);
- if (status < 0) {
- if (status == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE) {
+ ret = gpiod_request(desc, con_id ? con_id : devname);
+ if (ret < 0) {
+ if (ret == -EBUSY && flags & GPIOD_FLAGS_BIT_NONEXCLUSIVE) {
/*
* This happens when there are several consumers for
* the same GPIO line: we just return here without
@@ -4232,15 +4232,15 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
con_id ? con_id : devname);
return desc;
} else {
- return ERR_PTR(status);
+ return ERR_PTR(ret);
}
}
- status = gpiod_configure_flags(desc, con_id, lookupflags, flags);
- if (status < 0) {
+ ret = gpiod_configure_flags(desc, con_id, lookupflags, flags);
+ if (ret < 0) {
dev_dbg(dev, "setup of GPIO %s failed\n", con_id);
gpiod_put(desc);
- return ERR_PTR(status);
+ return ERR_PTR(ret);
}
return desc;
@@ -4424,7 +4424,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
struct gpio_chip *chip;
struct gpio_desc *local_desc;
int hwnum;
- int status;
+ int ret;
chip = gpiod_to_chip(desc);
hwnum = gpio_chip_hwgpio(desc);
@@ -4432,10 +4432,10 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
local_desc = gpiochip_request_own_desc(chip, hwnum, name,
lflags, dflags);
if (IS_ERR(local_desc)) {
- status = PTR_ERR(local_desc);
+ ret = PTR_ERR(local_desc);
pr_err("requesting hog GPIO %s (chip %s, offset %d) failed, %d\n",
- name, chip->label, hwnum, status);
- return status;
+ name, chip->label, hwnum, ret);
+ return ret;
}
/* Mark GPIO as hogged so it can be identified and removed later */
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] Revert "gpio/spi: Fix spi-gpio regression on active high CS"
From: Linus Walleij @ 2019-07-16 8:56 UTC (permalink / raw)
To: Sasha Levin
Cc: open list:GPIO SUBSYSTEM, Bartosz Golaszewski, linux-spi, stable
In-Reply-To: <20190716012712.BC9F22173C@mail.kernel.org>
On Tue, Jul 16, 2019 at 3:27 AM Sasha Levin <sashal@kernel.org> wrote:
> v5.2.1: Build OK!
> v5.1.18: Failed to apply! Possible dependencies:
(...)
> How should we proceed with this patch?
Only apply it to 5.2.y once upstream.
Thanks!
Linus Walleij
^ permalink raw reply
* Re: [PATCH] dt-bindings: Ensure child nodes are of type 'object'
From: Maxime Ripard @ 2019-07-16 8:28 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-kernel, Chen-Yu Tsai, David Woodhouse,
Brian Norris, Marek Vasut, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Linus Walleij, Maxime Coquelin,
Alexandre Torgue, Mark Brown, linux-mtd, linux-gpio, linux-stm32,
linux-spi
In-Reply-To: <20190715230457.3901-1-robh@kernel.org>
On Mon, Jul 15, 2019 at 05:04:57PM -0600, Rob Herring wrote:
> Properties which are child node definitions need to have an explict
> type. Otherwise, a matching (DT) property can silently match when an
> error is desired. Fix this up tree-wide. Once this is fixed, the
> meta-schema will enforce this on any child node definitions.
>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Marek Vasut <marek.vasut@gmail.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-mtd@lists.infradead.org
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-spi@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
Thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Peter De Schrijver @ 2019-07-16 8:06 UTC (permalink / raw)
To: Joseph Lo
Cc: Sowjanya Komatineni, Dmitry Osipenko, thierry.reding, jonathanh,
tglx, jason, marc.zyngier, linus.walleij, stefan, mark.rutland,
pgaikwad, sboyd, linux-clk, linux-gpio, jckuo, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <c5853e1a-d812-2dbd-3bec-0a9b0b0f6f3e@nvidia.com>
On Tue, Jul 16, 2019 at 03:24:26PM +0800, Joseph Lo wrote:
> > OK, Will add to CPUFreq driver...
> > >
> > > The other thing that also need attention is that T124 CPUFreq driver
> > > implicitly relies on DFLL driver to be probed first, which is icky.
> > >
> > Should I add check for successful dfll clk register explicitly in
> > CPUFreq driver probe and defer till dfll clk registers?
>
> Sorry, I didn't follow the mail thread. Just regarding the DFLL part.
>
> As you know it, the DFLL clock is one of the CPU clock sources and
> integrated with DVFS control logic with the regulator. We will not switch
> CPU to other clock sources once we switched to DFLL. Because the CPU has
> been regulated by the DFLL HW with the DVFS table (CVB or OPP table you see
> in the driver.). We shouldn't reparent it to other sources with unknew
> freq/volt pair. That's not guaranteed to work. We allow switching to
> open-loop mode but different sources.
>
> And I don't exactly understand why we need to switch to PLLP in CPU idle
> driver. Just keep it on CL-DVFS mode all the time.
>
> In SC7 entry, the dfll suspend function moves it the open-loop mode. That's
> all. The sc7-entryfirmware will handle the rest of the sequence to turn off
> the CPU power.
>
> In SC7 resume, the warmboot code will handle the sequence to turn on
> regulator and power up the CPU cluster. And leave it on PLL_P. After
> resuming to the kernel, we re-init DFLL, restore the CPU clock policy (CPU
> runs on DFLL open-loop mode) and then moving to close-loop mode.
>
> The DFLL part looks good to me. BTW, change the patch subject to "Add
> suspend-resume support" seems more appropriate to me.
>
To clarify this, the sequences for DFLL use are as follows (assuming all
required DFLL hw configuration has been done)
Switch to DFLL:
0) Save current parent and frequency
1) Program DFLL to open loop mode
2) Enable DFLL
3) Change cclk_g parent to DFLL
For OVR regulator:
4) Change PWM output pin from tristate to output
5) Enable DFLL PWM output
For I2C regulator:
4) Enable DFLL I2C output
6) Program DFLL to closed loop mode
Switch away from DFLL:
0) Change cclk_g parent to PLLP so the CPU frequency is ok for any vdd_cpu voltage
1) Program DFLL to open loop mode
For OVR regulator:
2) Change PWM output pin from output to tristate: vdd_cpu will go back
to hardwired boot voltage.
3) Disable DFLL PWM output
For I2C regulator:
2) Program vdd_cpu regulator voltage to the boot voltage
3) Disable DFLL I2C output
4) Reprogram parent saved in step 0 of 'Switch to DFLL' to the saved
frequency
5) Change cclk_g parent to saved parent
6) Disable DFLL
Peter.
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Joseph Lo @ 2019-07-16 7:24 UTC (permalink / raw)
To: Sowjanya Komatineni, Dmitry Osipenko
Cc: thierry.reding, jonathanh, tglx, jason, marc.zyngier,
linus.walleij, stefan, mark.rutland, pdeschrijver, pgaikwad,
sboyd, linux-clk, linux-gpio, jckuo, talho, linux-tegra,
linux-kernel, mperttunen, spatra, robh+dt, devicetree
In-Reply-To: <21266e4f-16b1-4c87-067a-16c07c803b6e@nvidia.com>
On 7/16/19 2:35 PM, Sowjanya Komatineni wrote:
>
> On 7/15/19 10:37 PM, Dmitry Osipenko wrote:
>> В Mon, 15 Jul 2019 21:37:09 -0700
>> Sowjanya Komatineni <skomatineni@nvidia.com> пишет:
>>
>>> On 7/15/19 8:50 PM, Dmitry Osipenko wrote:
>>>> 16.07.2019 6:00, Sowjanya Komatineni пишет:
>>>>> On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
>>>>>> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
>>>>>>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
>>>>>>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
>>>>>>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
>>>>>>>>>> This patch adds system suspend and resume support for Tegra210
>>>>>>>>>> clocks.
>>>>>>>>>>
>>>>>>>>>> All the CAR controller settings are lost on suspend when core
>>>>>>>>>> power goes off.
>>>>>>>>>>
>>>>>>>>>> This patch has implementation for saving and restoring all
>>>>>>>>>> the PLLs and clocks context during system suspend and resume
>>>>>>>>>> to have the clocks back to same state for normal operation.
>>>>>>>>>>
>>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>>> ---
>>>>>>>>>> drivers/clk/tegra/clk-tegra210.c | 115
>>>>>>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>>>>>> drivers/clk/tegra/clk.c | 14 +++++
>>>>>>>>>> drivers/clk/tegra/clk.h | 1 +
>>>>>>>>>> 3 files changed, 127 insertions(+), 3 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> index 1c08c53482a5..1b839544e086 100644
>>>>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>>> @@ -9,10 +9,12 @@
>>>>>>>>>> #include <linux/clkdev.h>
>>>>>>>>>> #include <linux/of.h>
>>>>>>>>>> #include <linux/of_address.h>
>>>>>>>>>> +#include <linux/of_platform.h>
>>>>>>>>>> #include <linux/delay.h>
>>>>>>>>>> #include <linux/export.h>
>>>>>>>>>> #include <linux/mutex.h>
>>>>>>>>>> #include <linux/clk/tegra.h>
>>>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>>> #include <dt-bindings/clock/tegra210-car.h>
>>>>>>>>>> #include <dt-bindings/reset/tegra210-car.h>
>>>>>>>>>> #include <linux/iopoll.h>
>>>>>>>>>> @@ -20,6 +22,7 @@
>>>>>>>>>> #include <soc/tegra/pmc.h>
>>>>>>>>>> #include "clk.h"
>>>>>>>>>> +#include "clk-dfll.h"
>>>>>>>>>> #include "clk-id.h"
>>>>>>>>>> /*
>>>>>>>>>> @@ -225,6 +228,7 @@
>>>>>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>>>>> #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>>>>> #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>> struct tegra_clk_pll_freq_table *fentry;
>>>>>>>>>> struct tegra_clk_pll pllu;
>>>>>>>>>> u32 reg;
>>>>>>>>>> + int ret;
>>>>>>>>>> for (fentry = pll_u_freq_table; fentry->input_rate;
>>>>>>>>>> fentry++) {
>>>>>>>>>> if (fentry->input_rate == pll_ref_freq)
>>>>>>>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
>>>>>>>>>> fence_udelay(1, clk_base);
>>>>>>>>>> reg |= PLL_ENABLE;
>>>>>>>>>> writel(reg, clk_base + PLLU_BASE);
>>>>>>>>>> + fence_udelay(1, clk_base);
>>>>>>>>>> - readl_relaxed_poll_timeout_atomic(clk_base +
>>>>>>>>>> PLLU_BASE, reg,
>>>>>>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>>>>> - if (!(reg & PLL_BASE_LOCK)) {
>>>>>>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE,
>>>>>>>>>> PLL_BASE_LOCK);
>>>>>>>>>> + if (ret) {
>>>>>>>>>> pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>>>>> return -ETIMEDOUT;
>>>>>>>>>> }
>>>>>>>>>> @@ -3283,6 +3288,103 @@ static void
>>>>>>>>>> tegra210_disable_cpu_clock(u32 cpu)
>>>>>>>>>> }
>>>>>>>>>> #ifdef CONFIG_PM_SLEEP
>>>>>>>>>> +static u32 cpu_softrst_ctx[3];
>>>>>>>>>> +static struct platform_device *dfll_pdev;
>>>>>>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base +
>>>>>>>>>> (_base) + ((_off) * 4))
>>>>>>>>>> +#define car_writel(_val, _base, _off) \
>>>>>>>>>> + writel_relaxed(_val, clk_base + (_base) + ((_off) *
>>>>>>>>>> 4)) +
>>>>>>>>>> +static int tegra210_clk_suspend(void)
>>>>>>>>>> +{
>>>>>>>>>> + unsigned int i;
>>>>>>>>>> + struct device_node *node;
>>>>>>>>>> +
>>>>>>>>>> + tegra_cclkg_burst_policy_save_context();
>>>>>>>>>> +
>>>>>>>>>> + if (!dfll_pdev) {
>>>>>>>>>> + node = of_find_compatible_node(NULL, NULL,
>>>>>>>>>> + "nvidia,tegra210-dfll");
>>>>>>>>>> + if (node)
>>>>>>>>>> + dfll_pdev = of_find_device_by_node(node);
>>>>>>>>>> +
>>>>>>>>>> + of_node_put(node);
>>>>>>>>>> + if (!dfll_pdev)
>>>>>>>>>> + pr_err("dfll node not found. no suspend for
>>>>>>>>>> dfll\n");
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> + if (dfll_pdev)
>>>>>>>>>> + tegra_dfll_suspend(dfll_pdev);
>>>>>>>>>> +
>>>>>>>>>> + /* Enable PLLP_OUT_CPU after dfll suspend */
>>>>>>>>>> + tegra_clk_set_pllp_out_cpu(true);
>>>>>>>>>> +
>>>>>>>>>> + tegra_sclk_cclklp_burst_policy_save_context();
>>>>>>>>>> +
>>>>>>>>>> + clk_save_context();
>>>>>>>>>> +
>>>>>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>>>>>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>>>>>>>>>> +
>>>>>>>>>> + return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +static void tegra210_clk_resume(void)
>>>>>>>>>> +{
>>>>>>>>>> + unsigned int i;
>>>>>>>>>> + struct clk_hw *parent;
>>>>>>>>>> + struct clk *clk;
>>>>>>>>>> +
>>>>>>>>>> + /*
>>>>>>>>>> + * clk_restore_context restores clocks as per the clock
>>>>>>>>>> tree.
>>>>>>>>>> + *
>>>>>>>>>> + * dfllCPU_out is first in the clock tree to get
>>>>>>>>>> restored and it
>>>>>>>>>> + * involves programming DFLL controller along with
>>>>>>>>>> restoring CPUG
>>>>>>>>>> + * clock burst policy.
>>>>>>>>>> + *
>>>>>>>>>> + * DFLL programming needs dfll_ref and dfll_soc
>>>>>>>>>> peripheral clocks
>>>>>>>>>> + * to be restores which are part ofthe peripheral
>>>>>>>>>> clocks.
>>>>>>> ^ white-space
>>>>>>>
>>>>>>> Please use spellchecker to avoid typos.
>>>>>>>>>> + * So, peripheral clocks restore should happen prior to
>>>>>>>>>> dfll clock
>>>>>>>>>> + * restore.
>>>>>>>>>> + */
>>>>>>>>>> +
>>>>>>>>>> + tegra_clk_osc_resume(clk_base);
>>>>>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>>>>>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>>>>>>>>> +
>>>>>>>>>> + /* restore all plls and peripheral clocks */
>>>>>>>>>> + tegra210_init_pllu();
>>>>>>>>>> + clk_restore_context();
>>>>>>>>>> +
>>>>>>>>>> + fence_udelay(5, clk_base);
>>>>>>>>>> +
>>>>>>>>>> + /* resume SCLK and CPULP clocks */
>>>>>>>>>> + tegra_sclk_cpulp_burst_policy_restore_context();
>>>>>>>>>> +
>>>>>>>>>> + /*
>>>>>>>>>> + * restore CPUG clocks:
>>>>>>>>>> + * - enable DFLL in open loop mode
>>>>>>>>>> + * - switch CPUG to DFLL clock source
>>>>>>>>>> + * - close DFLL loop
>>>>>>>>>> + * - sync PLLX state
>>>>>>>>>> + */
>>>>>>>>>> + if (dfll_pdev)
>>>>>>>>>> + tegra_dfll_resume(dfll_pdev, false);
>>>>>>>>>> +
>>>>>>>>>> + tegra_cclkg_burst_policy_restore_context();
>>>>>>>>>> + fence_udelay(2, clk_base);
>>>>>>>>>> +
>>>>>>>>>> + if (dfll_pdev)
>>>>>>>>>> + tegra_dfll_resume(dfll_pdev, true);
>>>>>>>>>> +
>>>>>>>>>> + parent =
>>>>>>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
>>>>>>>>>> + clk = clks[TEGRA210_CLK_PLL_X];
>>>>>>>>>> + if (parent != __clk_get_hw(clk))
>>>>>>>>>> + tegra_clk_sync_state_pll(__clk_get_hw(clk));
>>>>>>>>>> +
>>>>>>>>>> + /* Disable PLL_OUT_CPU after DFLL resume */
>>>>>>>>>> + tegra_clk_set_pllp_out_cpu(false);
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> static void tegra210_cpu_clock_suspend(void)
>>>>>>>>>> {
>>>>>>>>>> /* switch coresite to clk_m, save off original source
>>>>>>>>>> */ @@ -3298,6 +3400,11 @@ static void
>>>>>>>>>> tegra210_cpu_clock_resume(void) }
>>>>>>>>>> #endif
>>>>>>>>>> +static struct syscore_ops tegra_clk_syscore_ops = {
>>>>>>>>>> + .suspend = tegra210_clk_suspend,
>>>>>>>>>> + .resume = tegra210_clk_resume,
>>>>>>>>>> +};
>>>>>>>>>> +
>>>>>>>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>>>>>>>>>> .wait_for_reset = tegra210_wait_cpu_in_reset,
>>>>>>>>>> .disable_clock = tegra210_disable_cpu_clock,
>>>>>>>>>> @@ -3583,5 +3690,7 @@ static void __init
>>>>>>>>>> tegra210_clock_init(struct device_node *np)
>>>>>>>>>> tegra210_mbist_clk_init();
>>>>>>>>>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>>>>>>>>>> +
>>>>>>>>>> + register_syscore_ops(&tegra_clk_syscore_ops);
>>>>>>>>>> }
>>>>>>>>> Is it really worthwhile to use syscore_ops for suspend/resume
>>>>>>>>> given that drivers for
>>>>>>>>> won't resume before the CLK driver anyway? Are there any other
>>>>>>>>> options for CLK
>>>>>>>>> suspend/resume?
>>>>>>>>>
>>>>>>>>> I'm also not sure whether PM runtime API could be used at all
>>>>>>>>> in the context of
>>>>>>>>> syscore_ops ..
>>>>>>>>>
>>>>>>>>> Secondly, what about to use generic clk_save_context() /
>>>>>>>>> clk_restore_context()
>>>>>>>>> helpers for the suspend-resume? It looks to me that some other
>>>>>>>>> essential (and proper)
>>>>>>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the
>>>>>>>>> clocks using the generic
>>>>>>>>> CLK Framework API.
>>>>>>>> Clock resume should happen very early to restore peripheral and
>>>>>>>> cpu clocks very early than peripheral drivers resume happens.
>>>>>>> If all peripheral drivers properly requested all of the
>>>>>>> necessary clocks and CLK driver was a platform driver, then I
>>>>>>> guess the probe should have been naturally ordered. But that's
>>>>>>> not very achievable with the currently available infrastructure
>>>>>>> in the kernel, so I'm not arguing that the clocks should be
>>>>>>> explicitly resumed before the users.
>>>>>>>> this patch series uses clk_save_context and clk_restore_context
>>>>>>>> for corresponding divider, pll, pllout.. save and restore
>>>>>>>> context.
>>>>>>> Now I see that indeed this API is utilized in this patch, thank
>>>>>>> you for the clarification.
>>>>>>>> But as there is dependency on dfll resume and cpu and pllx
>>>>>>>> clocks restore, couldnt use clk_save_context and
>>>>>>>> clk_restore_context for dfll.
>>>>>>>>
>>>>>>>> So implemented recommended dfll resume sequence in main
>>>>>>>> Tegra210 clock driver along with invoking
>>>>>>>> clk_save_context/clk_restore_context where all other clocks
>>>>>>>> save/restore happens as per clock tree traversal.
>>>>>>> Could you please clarify what part of peripherals clocks is
>>>>>>> required for DFLL's restore? Couldn't DFLL driver be changed to
>>>>>>> avoid that quirkness and thus to make DFLL driver suspend/resume
>>>>>>> the clock?
>>>>>> DFLL source ref_clk and soc_clk need to be restored prior to dfll.
>>>>>>
>>>>>> I see dfllCPU_out parent to CCLK_G first in the clock tree and
>>>>>> dfll_ref and dfll_soc peripheral clocks are not resumed by the
>>>>>> time dfll resume happens first.
>>>>>>
>>>>>> ref_clk and soc_clk source is from pll_p and clock tree has these
>>>>>> registered under pll_p which happens later.
>>>>>>
>>>>>> tegra210_clock_init registers in order plls, peripheral clocks,
>>>>>> super_clk init for cclk_g during clock driver probe and dfll
>>>>>> probe and register happens later.
>>>>> One more thing, CLDVFS peripheral clock enable is also needed to be
>>>>> enabled to program DFLL Controller and all peripheral clock
>>>>> context is restored only after their PLL sources are restored.
>>>>>
>>>>> DFLL restore involves dfll source clock resume along with CLDVFS
>>>>> periheral clock enable and reset
>>>> I don't quite see why you can't simply add suspend/resume callbacks
>>>> to the CPUFreq driver to:
>>>>
>>>> On suspend:
>>>> 1. Switch CPU to PLLP (or whatever "safe" parent)
>>>> 2. Disable/teardown DFLL
>>>>
>>>> On resume:
>>>> 1. Enable/restore DFLL
>>>> 2. Switch CPU back to DFLL
>>> dfll runtime suspend/resume are already part of dfll_pm_ops. Don't we
>>> want to use it for suspend/resume as well?
>> Looks like no. Seems runtime PM of that driver is intended solely for
>> the DFLL's clk management.
>>
>>> currently no APIs are shared b/w clk/tegra driver and CPUFreq driver
>>> to invoke dfll suspend/resume in CPUFreq driver
>>>
>> Just add it. Also, please note that CPUFreq driver is optional and thus
>> you may need to switch CPU to a safe parent on clk-core suspend as
>> well in order to resume properly if CPU was running off unsafe parent
>> during boot and CPUFreq driver is disabled in kernel build (or failed
>> to load).
> OK, Will add to CPUFreq driver...
>>
>> The other thing that also need attention is that T124 CPUFreq driver
>> implicitly relies on DFLL driver to be probed first, which is icky.
>>
> Should I add check for successful dfll clk register explicitly in
> CPUFreq driver probe and defer till dfll clk registers?
Sorry, I didn't follow the mail thread. Just regarding the DFLL part.
As you know it, the DFLL clock is one of the CPU clock sources and
integrated with DVFS control logic with the regulator. We will not
switch CPU to other clock sources once we switched to DFLL. Because the
CPU has been regulated by the DFLL HW with the DVFS table (CVB or OPP
table you see in the driver.). We shouldn't reparent it to other sources
with unknew freq/volt pair. That's not guaranteed to work. We allow
switching to open-loop mode but different sources.
And I don't exactly understand why we need to switch to PLLP in CPU idle
driver. Just keep it on CL-DVFS mode all the time.
In SC7 entry, the dfll suspend function moves it the open-loop mode.
That's all. The sc7-entryfirmware will handle the rest of the sequence
to turn off the CPU power.
In SC7 resume, the warmboot code will handle the sequence to turn on
regulator and power up the CPU cluster. And leave it on PLL_P. After
resuming to the kernel, we re-init DFLL, restore the CPU clock policy
(CPU runs on DFLL open-loop mode) and then moving to close-loop mode.
The DFLL part looks good to me. BTW, change the patch subject to "Add
suspend-resume support" seems more appropriate to me.
Thanks.
^ permalink raw reply
* Re: [PATCH] dt-bindings: Ensure child nodes are of type 'object'
From: Miquel Raynal @ 2019-07-16 7:19 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
David Woodhouse, Brian Norris, Marek Vasut, Richard Weinberger,
Vignesh Raghavendra, Linus Walleij, Maxime Coquelin,
Alexandre Torgue, Mark Brown, linux-mtd, linux-gpio, linux-stm32,
linux-spi
In-Reply-To: <20190715230457.3901-1-robh@kernel.org>
Hi Rob,
Rob Herring <robh@kernel.org> wrote on Mon, 15 Jul 2019 17:04:57 -0600:
> Properties which are child node definitions need to have an explict
> type. Otherwise, a matching (DT) property can silently match when an
> error is desired. Fix this up tree-wide. Once this is fixed, the
> meta-schema will enforce this on any child node definitions.
>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Cc: Chen-Yu Tsai <wens@csie.org>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Brian Norris <computersforpeace@gmail.com>
> Cc: Marek Vasut <marek.vasut@gmail.com>
> Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Vignesh Raghavendra <vigneshr@ti.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-mtd@lists.infradead.org
> Cc: linux-gpio@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-spi@vger.kernel.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> Please ack. I will take this via the DT tree.
>
> Rob
>
> .../devicetree/bindings/bus/allwinner,sun8i-a23-rsb.yaml | 1 +
> .../devicetree/bindings/mtd/allwinner,sun4i-a10-nand.yaml | 1 +
> Documentation/devicetree/bindings/mtd/nand-controller.yaml | 1 +
> .../devicetree/bindings/pinctrl/st,stm32-pinctrl.yaml | 3 +++
> .../devicetree/bindings/spi/allwinner,sun4i-a10-spi.yaml | 1 +
> .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 1 +
> 6 files changed, 8 insertions(+)
>
[...]
> diff --git a/Documentation/devicetree/bindings/mtd/nand-controller.yaml b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> index 199ba5ac2a06..d261b7096c69 100644
> --- a/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> +++ b/Documentation/devicetree/bindings/mtd/nand-controller.yaml
> @@ -40,6 +40,7 @@ properties:
>
> patternProperties:
> "^nand@[a-f0-9]$":
> + type: object
> properties:
> reg:
> description:
For the mtd .yaml:
Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Sowjanya Komatineni @ 2019-07-16 6:35 UTC (permalink / raw)
To: Dmitry Osipenko
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, devicetree
In-Reply-To: <20190716083701.225f0fd9@dimatab>
On 7/15/19 10:37 PM, Dmitry Osipenko wrote:
> В Mon, 15 Jul 2019 21:37:09 -0700
> Sowjanya Komatineni <skomatineni@nvidia.com> пишет:
>
>> On 7/15/19 8:50 PM, Dmitry Osipenko wrote:
>>> 16.07.2019 6:00, Sowjanya Komatineni пишет:
>>>> On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
>>>>> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
>>>>>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
>>>>>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
>>>>>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
>>>>>>>>> This patch adds system suspend and resume support for Tegra210
>>>>>>>>> clocks.
>>>>>>>>>
>>>>>>>>> All the CAR controller settings are lost on suspend when core
>>>>>>>>> power goes off.
>>>>>>>>>
>>>>>>>>> This patch has implementation for saving and restoring all
>>>>>>>>> the PLLs and clocks context during system suspend and resume
>>>>>>>>> to have the clocks back to same state for normal operation.
>>>>>>>>>
>>>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>>>> ---
>>>>>>>>> drivers/clk/tegra/clk-tegra210.c | 115
>>>>>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>>>>> drivers/clk/tegra/clk.c | 14 +++++
>>>>>>>>> drivers/clk/tegra/clk.h | 1 +
>>>>>>>>> 3 files changed, 127 insertions(+), 3 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>> b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>> index 1c08c53482a5..1b839544e086 100644
>>>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>>>> @@ -9,10 +9,12 @@
>>>>>>>>> #include <linux/clkdev.h>
>>>>>>>>> #include <linux/of.h>
>>>>>>>>> #include <linux/of_address.h>
>>>>>>>>> +#include <linux/of_platform.h>
>>>>>>>>> #include <linux/delay.h>
>>>>>>>>> #include <linux/export.h>
>>>>>>>>> #include <linux/mutex.h>
>>>>>>>>> #include <linux/clk/tegra.h>
>>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>>> #include <dt-bindings/clock/tegra210-car.h>
>>>>>>>>> #include <dt-bindings/reset/tegra210-car.h>
>>>>>>>>> #include <linux/iopoll.h>
>>>>>>>>> @@ -20,6 +22,7 @@
>>>>>>>>> #include <soc/tegra/pmc.h>
>>>>>>>>> #include "clk.h"
>>>>>>>>> +#include "clk-dfll.h"
>>>>>>>>> #include "clk-id.h"
>>>>>>>>> /*
>>>>>>>>> @@ -225,6 +228,7 @@
>>>>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>>>> #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>>>> #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
>>>>>>>>> struct tegra_clk_pll_freq_table *fentry;
>>>>>>>>> struct tegra_clk_pll pllu;
>>>>>>>>> u32 reg;
>>>>>>>>> + int ret;
>>>>>>>>> for (fentry = pll_u_freq_table; fentry->input_rate;
>>>>>>>>> fentry++) {
>>>>>>>>> if (fentry->input_rate == pll_ref_freq)
>>>>>>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
>>>>>>>>> fence_udelay(1, clk_base);
>>>>>>>>> reg |= PLL_ENABLE;
>>>>>>>>> writel(reg, clk_base + PLLU_BASE);
>>>>>>>>> + fence_udelay(1, clk_base);
>>>>>>>>> - readl_relaxed_poll_timeout_atomic(clk_base +
>>>>>>>>> PLLU_BASE, reg,
>>>>>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>>>> - if (!(reg & PLL_BASE_LOCK)) {
>>>>>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE,
>>>>>>>>> PLL_BASE_LOCK);
>>>>>>>>> + if (ret) {
>>>>>>>>> pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>>>> return -ETIMEDOUT;
>>>>>>>>> }
>>>>>>>>> @@ -3283,6 +3288,103 @@ static void
>>>>>>>>> tegra210_disable_cpu_clock(u32 cpu)
>>>>>>>>> }
>>>>>>>>> #ifdef CONFIG_PM_SLEEP
>>>>>>>>> +static u32 cpu_softrst_ctx[3];
>>>>>>>>> +static struct platform_device *dfll_pdev;
>>>>>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base +
>>>>>>>>> (_base) + ((_off) * 4))
>>>>>>>>> +#define car_writel(_val, _base, _off) \
>>>>>>>>> + writel_relaxed(_val, clk_base + (_base) + ((_off) *
>>>>>>>>> 4)) +
>>>>>>>>> +static int tegra210_clk_suspend(void)
>>>>>>>>> +{
>>>>>>>>> + unsigned int i;
>>>>>>>>> + struct device_node *node;
>>>>>>>>> +
>>>>>>>>> + tegra_cclkg_burst_policy_save_context();
>>>>>>>>> +
>>>>>>>>> + if (!dfll_pdev) {
>>>>>>>>> + node = of_find_compatible_node(NULL, NULL,
>>>>>>>>> + "nvidia,tegra210-dfll");
>>>>>>>>> + if (node)
>>>>>>>>> + dfll_pdev = of_find_device_by_node(node);
>>>>>>>>> +
>>>>>>>>> + of_node_put(node);
>>>>>>>>> + if (!dfll_pdev)
>>>>>>>>> + pr_err("dfll node not found. no suspend for
>>>>>>>>> dfll\n");
>>>>>>>>> + }
>>>>>>>>> +
>>>>>>>>> + if (dfll_pdev)
>>>>>>>>> + tegra_dfll_suspend(dfll_pdev);
>>>>>>>>> +
>>>>>>>>> + /* Enable PLLP_OUT_CPU after dfll suspend */
>>>>>>>>> + tegra_clk_set_pllp_out_cpu(true);
>>>>>>>>> +
>>>>>>>>> + tegra_sclk_cclklp_burst_policy_save_context();
>>>>>>>>> +
>>>>>>>>> + clk_save_context();
>>>>>>>>> +
>>>>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>>>>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>>>>>>>>> +
>>>>>>>>> + return 0;
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> +static void tegra210_clk_resume(void)
>>>>>>>>> +{
>>>>>>>>> + unsigned int i;
>>>>>>>>> + struct clk_hw *parent;
>>>>>>>>> + struct clk *clk;
>>>>>>>>> +
>>>>>>>>> + /*
>>>>>>>>> + * clk_restore_context restores clocks as per the clock
>>>>>>>>> tree.
>>>>>>>>> + *
>>>>>>>>> + * dfllCPU_out is first in the clock tree to get
>>>>>>>>> restored and it
>>>>>>>>> + * involves programming DFLL controller along with
>>>>>>>>> restoring CPUG
>>>>>>>>> + * clock burst policy.
>>>>>>>>> + *
>>>>>>>>> + * DFLL programming needs dfll_ref and dfll_soc
>>>>>>>>> peripheral clocks
>>>>>>>>> + * to be restores which are part ofthe peripheral
>>>>>>>>> clocks.
>>>>>> ^ white-space
>>>>>>
>>>>>> Please use spellchecker to avoid typos.
>>>>>>
>>>>>>>>> + * So, peripheral clocks restore should happen prior to
>>>>>>>>> dfll clock
>>>>>>>>> + * restore.
>>>>>>>>> + */
>>>>>>>>> +
>>>>>>>>> + tegra_clk_osc_resume(clk_base);
>>>>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>>>>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>>>>>>>> +
>>>>>>>>> + /* restore all plls and peripheral clocks */
>>>>>>>>> + tegra210_init_pllu();
>>>>>>>>> + clk_restore_context();
>>>>>>>>> +
>>>>>>>>> + fence_udelay(5, clk_base);
>>>>>>>>> +
>>>>>>>>> + /* resume SCLK and CPULP clocks */
>>>>>>>>> + tegra_sclk_cpulp_burst_policy_restore_context();
>>>>>>>>> +
>>>>>>>>> + /*
>>>>>>>>> + * restore CPUG clocks:
>>>>>>>>> + * - enable DFLL in open loop mode
>>>>>>>>> + * - switch CPUG to DFLL clock source
>>>>>>>>> + * - close DFLL loop
>>>>>>>>> + * - sync PLLX state
>>>>>>>>> + */
>>>>>>>>> + if (dfll_pdev)
>>>>>>>>> + tegra_dfll_resume(dfll_pdev, false);
>>>>>>>>> +
>>>>>>>>> + tegra_cclkg_burst_policy_restore_context();
>>>>>>>>> + fence_udelay(2, clk_base);
>>>>>>>>> +
>>>>>>>>> + if (dfll_pdev)
>>>>>>>>> + tegra_dfll_resume(dfll_pdev, true);
>>>>>>>>> +
>>>>>>>>> + parent =
>>>>>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
>>>>>>>>> + clk = clks[TEGRA210_CLK_PLL_X];
>>>>>>>>> + if (parent != __clk_get_hw(clk))
>>>>>>>>> + tegra_clk_sync_state_pll(__clk_get_hw(clk));
>>>>>>>>> +
>>>>>>>>> + /* Disable PLL_OUT_CPU after DFLL resume */
>>>>>>>>> + tegra_clk_set_pllp_out_cpu(false);
>>>>>>>>> +}
>>>>>>>>> +
>>>>>>>>> static void tegra210_cpu_clock_suspend(void)
>>>>>>>>> {
>>>>>>>>> /* switch coresite to clk_m, save off original source
>>>>>>>>> */ @@ -3298,6 +3400,11 @@ static void
>>>>>>>>> tegra210_cpu_clock_resume(void) }
>>>>>>>>> #endif
>>>>>>>>> +static struct syscore_ops tegra_clk_syscore_ops = {
>>>>>>>>> + .suspend = tegra210_clk_suspend,
>>>>>>>>> + .resume = tegra210_clk_resume,
>>>>>>>>> +};
>>>>>>>>> +
>>>>>>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>>>>>>>>> .wait_for_reset = tegra210_wait_cpu_in_reset,
>>>>>>>>> .disable_clock = tegra210_disable_cpu_clock,
>>>>>>>>> @@ -3583,5 +3690,7 @@ static void __init
>>>>>>>>> tegra210_clock_init(struct device_node *np)
>>>>>>>>> tegra210_mbist_clk_init();
>>>>>>>>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>>>>>>>>> +
>>>>>>>>> + register_syscore_ops(&tegra_clk_syscore_ops);
>>>>>>>>> }
>>>>>>>> Is it really worthwhile to use syscore_ops for suspend/resume
>>>>>>>> given that drivers for
>>>>>>>> won't resume before the CLK driver anyway? Are there any other
>>>>>>>> options for CLK
>>>>>>>> suspend/resume?
>>>>>>>>
>>>>>>>> I'm also not sure whether PM runtime API could be used at all
>>>>>>>> in the context of
>>>>>>>> syscore_ops ..
>>>>>>>>
>>>>>>>> Secondly, what about to use generic clk_save_context() /
>>>>>>>> clk_restore_context()
>>>>>>>> helpers for the suspend-resume? It looks to me that some other
>>>>>>>> essential (and proper)
>>>>>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the
>>>>>>>> clocks using the generic
>>>>>>>> CLK Framework API.
>>>>>>> Clock resume should happen very early to restore peripheral and
>>>>>>> cpu clocks very early than peripheral drivers resume happens.
>>>>>> If all peripheral drivers properly requested all of the
>>>>>> necessary clocks and CLK driver was a platform driver, then I
>>>>>> guess the probe should have been naturally ordered. But that's
>>>>>> not very achievable with the currently available infrastructure
>>>>>> in the kernel, so I'm not arguing that the clocks should be
>>>>>> explicitly resumed before the users.
>>>>>>> this patch series uses clk_save_context and clk_restore_context
>>>>>>> for corresponding divider, pll, pllout.. save and restore
>>>>>>> context.
>>>>>> Now I see that indeed this API is utilized in this patch, thank
>>>>>> you for the clarification.
>>>>>>
>>>>>>> But as there is dependency on dfll resume and cpu and pllx
>>>>>>> clocks restore, couldnt use clk_save_context and
>>>>>>> clk_restore_context for dfll.
>>>>>>>
>>>>>>> So implemented recommended dfll resume sequence in main
>>>>>>> Tegra210 clock driver along with invoking
>>>>>>> clk_save_context/clk_restore_context where all other clocks
>>>>>>> save/restore happens as per clock tree traversal.
>>>>>> Could you please clarify what part of peripherals clocks is
>>>>>> required for DFLL's restore? Couldn't DFLL driver be changed to
>>>>>> avoid that quirkness and thus to make DFLL driver suspend/resume
>>>>>> the clock?
>>>>> DFLL source ref_clk and soc_clk need to be restored prior to dfll.
>>>>>
>>>>> I see dfllCPU_out parent to CCLK_G first in the clock tree and
>>>>> dfll_ref and dfll_soc peripheral clocks are not resumed by the
>>>>> time dfll resume happens first.
>>>>>
>>>>> ref_clk and soc_clk source is from pll_p and clock tree has these
>>>>> registered under pll_p which happens later.
>>>>>
>>>>> tegra210_clock_init registers in order plls, peripheral clocks,
>>>>> super_clk init for cclk_g during clock driver probe and dfll
>>>>> probe and register happens later.
>>>>>
>>>> One more thing, CLDVFS peripheral clock enable is also needed to be
>>>> enabled to program DFLL Controller and all peripheral clock
>>>> context is restored only after their PLL sources are restored.
>>>>
>>>> DFLL restore involves dfll source clock resume along with CLDVFS
>>>> periheral clock enable and reset
>>>>
>>> I don't quite see why you can't simply add suspend/resume callbacks
>>> to the CPUFreq driver to:
>>>
>>> On suspend:
>>> 1. Switch CPU to PLLP (or whatever "safe" parent)
>>> 2. Disable/teardown DFLL
>>>
>>> On resume:
>>> 1. Enable/restore DFLL
>>> 2. Switch CPU back to DFLL
>> dfll runtime suspend/resume are already part of dfll_pm_ops. Don't we
>> want to use it for suspend/resume as well?
> Looks like no. Seems runtime PM of that driver is intended solely for
> the DFLL's clk management.
>
>> currently no APIs are shared b/w clk/tegra driver and CPUFreq driver
>> to invoke dfll suspend/resume in CPUFreq driver
>>
> Just add it. Also, please note that CPUFreq driver is optional and thus
> you may need to switch CPU to a safe parent on clk-core suspend as
> well in order to resume properly if CPU was running off unsafe parent
> during boot and CPUFreq driver is disabled in kernel build (or failed
> to load).
OK, Will add to CPUFreq driver...
>
> The other thing that also need attention is that T124 CPUFreq driver
> implicitly relies on DFLL driver to be probed first, which is icky.
>
Should I add check for successful dfll clk register explicitly in
CPUFreq driver probe and defer till dfll clk registers?
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Dmitry Osipenko @ 2019-07-16 6:20 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, devicetree
In-Reply-To: <20190716083701.225f0fd9@dimatab>
В Tue, 16 Jul 2019 08:37:01 +0300
Dmitry Osipenko <digetx@gmail.com> пишет:
> В Mon, 15 Jul 2019 21:37:09 -0700
> Sowjanya Komatineni <skomatineni@nvidia.com> пишет:
>
> > On 7/15/19 8:50 PM, Dmitry Osipenko wrote:
> > > 16.07.2019 6:00, Sowjanya Komatineni пишет:
> > >> On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
> > >>> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
> > >>>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
> > >>>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
> > >>>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
> > >>>>>>> This patch adds system suspend and resume support for
> > >>>>>>> Tegra210 clocks.
> > >>>>>>>
> > >>>>>>> All the CAR controller settings are lost on suspend when
> > >>>>>>> core power goes off.
> > >>>>>>>
> > >>>>>>> This patch has implementation for saving and restoring all
> > >>>>>>> the PLLs and clocks context during system suspend and resume
> > >>>>>>> to have the clocks back to same state for normal operation.
> > >>>>>>>
> > >>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
> > >>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> > >>>>>>> ---
> > >>>>>>> drivers/clk/tegra/clk-tegra210.c | 115
> > >>>>>>> ++++++++++++++++++++++++++++++++++++++-
> > >>>>>>> drivers/clk/tegra/clk.c | 14 +++++
> > >>>>>>> drivers/clk/tegra/clk.h | 1 +
> > >>>>>>> 3 files changed, 127 insertions(+), 3 deletions(-)
> > >>>>>>>
> > >>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
> > >>>>>>> b/drivers/clk/tegra/clk-tegra210.c
> > >>>>>>> index 1c08c53482a5..1b839544e086 100644
> > >>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
> > >>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
> > >>>>>>> @@ -9,10 +9,12 @@
> > >>>>>>> #include <linux/clkdev.h>
> > >>>>>>> #include <linux/of.h>
> > >>>>>>> #include <linux/of_address.h>
> > >>>>>>> +#include <linux/of_platform.h>
> > >>>>>>> #include <linux/delay.h>
> > >>>>>>> #include <linux/export.h>
> > >>>>>>> #include <linux/mutex.h>
> > >>>>>>> #include <linux/clk/tegra.h>
> > >>>>>>> +#include <linux/syscore_ops.h>
> > >>>>>>> #include <dt-bindings/clock/tegra210-car.h>
> > >>>>>>> #include <dt-bindings/reset/tegra210-car.h>
> > >>>>>>> #include <linux/iopoll.h>
> > >>>>>>> @@ -20,6 +22,7 @@
> > >>>>>>> #include <soc/tegra/pmc.h>
> > >>>>>>> #include "clk.h"
> > >>>>>>> +#include "clk-dfll.h"
> > >>>>>>> #include "clk-id.h"
> > >>>>>>> /*
> > >>>>>>> @@ -225,6 +228,7 @@
> > >>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
> > >>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
> > >>>>>>> +#define CPU_SOFTRST_CTRL 0x380
> > >>>>>>> #define LVL2_CLK_GATE_OVRA 0xf8
> > >>>>>>> #define LVL2_CLK_GATE_OVRC 0x3a0
> > >>>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
> > >>>>>>> struct tegra_clk_pll_freq_table *fentry;
> > >>>>>>> struct tegra_clk_pll pllu;
> > >>>>>>> u32 reg;
> > >>>>>>> + int ret;
> > >>>>>>> for (fentry = pll_u_freq_table;
> > >>>>>>> fentry->input_rate; fentry++) {
> > >>>>>>> if (fentry->input_rate == pll_ref_freq)
> > >>>>>>> @@ -2847,10 +2852,10 @@ static int
> > >>>>>>> tegra210_enable_pllu(void) fence_udelay(1, clk_base);
> > >>>>>>> reg |= PLL_ENABLE;
> > >>>>>>> writel(reg, clk_base + PLLU_BASE);
> > >>>>>>> + fence_udelay(1, clk_base);
> > >>>>>>> - readl_relaxed_poll_timeout_atomic(clk_base +
> > >>>>>>> PLLU_BASE, reg,
> > >>>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
> > >>>>>>> - if (!(reg & PLL_BASE_LOCK)) {
> > >>>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE,
> > >>>>>>> PLL_BASE_LOCK);
> > >>>>>>> + if (ret) {
> > >>>>>>> pr_err("Timed out waiting for PLL_U to lock\n");
> > >>>>>>> return -ETIMEDOUT;
> > >>>>>>> }
> > >>>>>>> @@ -3283,6 +3288,103 @@ static void
> > >>>>>>> tegra210_disable_cpu_clock(u32 cpu)
> > >>>>>>> }
> > >>>>>>> #ifdef CONFIG_PM_SLEEP
> > >>>>>>> +static u32 cpu_softrst_ctx[3];
> > >>>>>>> +static struct platform_device *dfll_pdev;
> > >>>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base +
> > >>>>>>> (_base) + ((_off) * 4))
> > >>>>>>> +#define car_writel(_val, _base, _off) \
> > >>>>>>> + writel_relaxed(_val, clk_base + (_base) + ((_off) *
> > >>>>>>> 4)) +
> > >>>>>>> +static int tegra210_clk_suspend(void)
> > >>>>>>> +{
> > >>>>>>> + unsigned int i;
> > >>>>>>> + struct device_node *node;
> > >>>>>>> +
> > >>>>>>> + tegra_cclkg_burst_policy_save_context();
> > >>>>>>> +
> > >>>>>>> + if (!dfll_pdev) {
> > >>>>>>> + node = of_find_compatible_node(NULL, NULL,
> > >>>>>>> + "nvidia,tegra210-dfll");
> > >>>>>>> + if (node)
> > >>>>>>> + dfll_pdev = of_find_device_by_node(node);
> > >>>>>>> +
> > >>>>>>> + of_node_put(node);
> > >>>>>>> + if (!dfll_pdev)
> > >>>>>>> + pr_err("dfll node not found. no suspend for
> > >>>>>>> dfll\n");
> > >>>>>>> + }
> > >>>>>>> +
> > >>>>>>> + if (dfll_pdev)
> > >>>>>>> + tegra_dfll_suspend(dfll_pdev);
> > >>>>>>> +
> > >>>>>>> + /* Enable PLLP_OUT_CPU after dfll suspend */
> > >>>>>>> + tegra_clk_set_pllp_out_cpu(true);
> > >>>>>>> +
> > >>>>>>> + tegra_sclk_cclklp_burst_policy_save_context();
> > >>>>>>> +
> > >>>>>>> + clk_save_context();
> > >>>>>>> +
> > >>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
> > >>>>>>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL,
> > >>>>>>> i); +
> > >>>>>>> + return 0;
> > >>>>>>> +}
> > >>>>>>> +
> > >>>>>>> +static void tegra210_clk_resume(void)
> > >>>>>>> +{
> > >>>>>>> + unsigned int i;
> > >>>>>>> + struct clk_hw *parent;
> > >>>>>>> + struct clk *clk;
> > >>>>>>> +
> > >>>>>>> + /*
> > >>>>>>> + * clk_restore_context restores clocks as per the clock
> > >>>>>>> tree.
> > >>>>>>> + *
> > >>>>>>> + * dfllCPU_out is first in the clock tree to get
> > >>>>>>> restored and it
> > >>>>>>> + * involves programming DFLL controller along with
> > >>>>>>> restoring CPUG
> > >>>>>>> + * clock burst policy.
> > >>>>>>> + *
> > >>>>>>> + * DFLL programming needs dfll_ref and dfll_soc
> > >>>>>>> peripheral clocks
> > >>>>>>> + * to be restores which are part ofthe peripheral
> > >>>>>>> clocks.
> > >>>> ^ white-space
> > >>>>
> > >>>> Please use spellchecker to avoid typos.
> > >>>>
> > >>>>>>> + * So, peripheral clocks restore should happen prior to
> > >>>>>>> dfll clock
> > >>>>>>> + * restore.
> > >>>>>>> + */
> > >>>>>>> +
> > >>>>>>> + tegra_clk_osc_resume(clk_base);
> > >>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
> > >>>>>>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL,
> > >>>>>>> i); +
> > >>>>>>> + /* restore all plls and peripheral clocks */
> > >>>>>>> + tegra210_init_pllu();
> > >>>>>>> + clk_restore_context();
> > >>>>>>> +
> > >>>>>>> + fence_udelay(5, clk_base);
> > >>>>>>> +
> > >>>>>>> + /* resume SCLK and CPULP clocks */
> > >>>>>>> + tegra_sclk_cpulp_burst_policy_restore_context();
> > >>>>>>> +
> > >>>>>>> + /*
> > >>>>>>> + * restore CPUG clocks:
> > >>>>>>> + * - enable DFLL in open loop mode
> > >>>>>>> + * - switch CPUG to DFLL clock source
> > >>>>>>> + * - close DFLL loop
> > >>>>>>> + * - sync PLLX state
> > >>>>>>> + */
> > >>>>>>> + if (dfll_pdev)
> > >>>>>>> + tegra_dfll_resume(dfll_pdev, false);
> > >>>>>>> +
> > >>>>>>> + tegra_cclkg_burst_policy_restore_context();
> > >>>>>>> + fence_udelay(2, clk_base);
> > >>>>>>> +
> > >>>>>>> + if (dfll_pdev)
> > >>>>>>> + tegra_dfll_resume(dfll_pdev, true);
> > >>>>>>> +
> > >>>>>>> + parent =
> > >>>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
> > >>>>>>> + clk = clks[TEGRA210_CLK_PLL_X];
> > >>>>>>> + if (parent != __clk_get_hw(clk))
> > >>>>>>> + tegra_clk_sync_state_pll(__clk_get_hw(clk));
> > >>>>>>> +
> > >>>>>>> + /* Disable PLL_OUT_CPU after DFLL resume */
> > >>>>>>> + tegra_clk_set_pllp_out_cpu(false);
> > >>>>>>> +}
> > >>>>>>> +
> > >>>>>>> static void tegra210_cpu_clock_suspend(void)
> > >>>>>>> {
> > >>>>>>> /* switch coresite to clk_m, save off original
> > >>>>>>> source */ @@ -3298,6 +3400,11 @@ static void
> > >>>>>>> tegra210_cpu_clock_resume(void) }
> > >>>>>>> #endif
> > >>>>>>> +static struct syscore_ops tegra_clk_syscore_ops = {
> > >>>>>>> + .suspend = tegra210_clk_suspend,
> > >>>>>>> + .resume = tegra210_clk_resume,
> > >>>>>>> +};
> > >>>>>>> +
> > >>>>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
> > >>>>>>> .wait_for_reset = tegra210_wait_cpu_in_reset,
> > >>>>>>> .disable_clock = tegra210_disable_cpu_clock,
> > >>>>>>> @@ -3583,5 +3690,7 @@ static void __init
> > >>>>>>> tegra210_clock_init(struct device_node *np)
> > >>>>>>> tegra210_mbist_clk_init();
> > >>>>>>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
> > >>>>>>> +
> > >>>>>>> + register_syscore_ops(&tegra_clk_syscore_ops);
> > >>>>>>> }
> > >>>>>> Is it really worthwhile to use syscore_ops for suspend/resume
> > >>>>>> given that drivers for
> > >>>>>> won't resume before the CLK driver anyway? Are there any
> > >>>>>> other options for CLK
> > >>>>>> suspend/resume?
> > >>>>>>
> > >>>>>> I'm also not sure whether PM runtime API could be used at all
> > >>>>>> in the context of
> > >>>>>> syscore_ops ..
> > >>>>>>
> > >>>>>> Secondly, what about to use generic clk_save_context() /
> > >>>>>> clk_restore_context()
> > >>>>>> helpers for the suspend-resume? It looks to me that some
> > >>>>>> other essential (and proper)
> > >>>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the
> > >>>>>> clocks using the generic
> > >>>>>> CLK Framework API.
> > >>>>> Clock resume should happen very early to restore peripheral
> > >>>>> and cpu clocks very early than peripheral drivers resume
> > >>>>> happens.
> > >>>> If all peripheral drivers properly requested all of the
> > >>>> necessary clocks and CLK driver was a platform driver, then I
> > >>>> guess the probe should have been naturally ordered. But that's
> > >>>> not very achievable with the currently available infrastructure
> > >>>> in the kernel, so I'm not arguing that the clocks should be
> > >>>> explicitly resumed before the users.
> > >>>>> this patch series uses clk_save_context and
> > >>>>> clk_restore_context for corresponding divider, pll, pllout..
> > >>>>> save and restore context.
> > >>>> Now I see that indeed this API is utilized in this patch, thank
> > >>>> you for the clarification.
> > >>>>
> > >>>>> But as there is dependency on dfll resume and cpu and pllx
> > >>>>> clocks restore, couldnt use clk_save_context and
> > >>>>> clk_restore_context for dfll.
> > >>>>>
> > >>>>> So implemented recommended dfll resume sequence in main
> > >>>>> Tegra210 clock driver along with invoking
> > >>>>> clk_save_context/clk_restore_context where all other clocks
> > >>>>> save/restore happens as per clock tree traversal.
> > >>>> Could you please clarify what part of peripherals clocks is
> > >>>> required for DFLL's restore? Couldn't DFLL driver be changed to
> > >>>> avoid that quirkness and thus to make DFLL driver
> > >>>> suspend/resume the clock?
> > >>> DFLL source ref_clk and soc_clk need to be restored prior to
> > >>> dfll.
> > >>>
> > >>> I see dfllCPU_out parent to CCLK_G first in the clock tree and
> > >>> dfll_ref and dfll_soc peripheral clocks are not resumed by the
> > >>> time dfll resume happens first.
> > >>>
> > >>> ref_clk and soc_clk source is from pll_p and clock tree has
> > >>> these registered under pll_p which happens later.
> > >>>
> > >>> tegra210_clock_init registers in order plls, peripheral clocks,
> > >>> super_clk init for cclk_g during clock driver probe and dfll
> > >>> probe and register happens later.
> > >>>
> > >> One more thing, CLDVFS peripheral clock enable is also needed to
> > >> be enabled to program DFLL Controller and all peripheral clock
> > >> context is restored only after their PLL sources are restored.
> > >>
> > >> DFLL restore involves dfll source clock resume along with CLDVFS
> > >> periheral clock enable and reset
> > >>
> > > I don't quite see why you can't simply add suspend/resume
> > > callbacks to the CPUFreq driver to:
> > >
> > > On suspend:
> > > 1. Switch CPU to PLLP (or whatever "safe" parent)
> > > 2. Disable/teardown DFLL
> > >
> > > On resume:
> > > 1. Enable/restore DFLL
> > > 2. Switch CPU back to DFLL
> >
> > dfll runtime suspend/resume are already part of dfll_pm_ops. Don't
> > we want to use it for suspend/resume as well?
>
> Looks like no. Seems runtime PM of that driver is intended solely for
> the DFLL's clk management.
>
> > currently no APIs are shared b/w clk/tegra driver and CPUFreq driver
> > to invoke dfll suspend/resume in CPUFreq driver
> >
>
> Just add it. Also, please note that CPUFreq driver is optional and
> thus you may need to switch CPU to a safe parent on clk-core suspend
> as well in order to resume properly if CPU was running off unsafe
> parent during boot and CPUFreq driver is disabled in kernel build (or
> failed to load).
Although, if PLLs are restored before CCLK, then it should be fine
as-is.
> The other thing that also need attention is that T124 CPUFreq driver
> implicitly relies on DFLL driver to be probed first, which is icky.
>
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Dmitry Osipenko @ 2019-07-16 5:37 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, devicetree
In-Reply-To: <86fc07d5-ab2e-a52a-a570-b1dfff4c20fe@nvidia.com>
В Mon, 15 Jul 2019 21:37:09 -0700
Sowjanya Komatineni <skomatineni@nvidia.com> пишет:
> On 7/15/19 8:50 PM, Dmitry Osipenko wrote:
> > 16.07.2019 6:00, Sowjanya Komatineni пишет:
> >> On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
> >>> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
> >>>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
> >>>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
> >>>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
> >>>>>>> This patch adds system suspend and resume support for Tegra210
> >>>>>>> clocks.
> >>>>>>>
> >>>>>>> All the CAR controller settings are lost on suspend when core
> >>>>>>> power goes off.
> >>>>>>>
> >>>>>>> This patch has implementation for saving and restoring all
> >>>>>>> the PLLs and clocks context during system suspend and resume
> >>>>>>> to have the clocks back to same state for normal operation.
> >>>>>>>
> >>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
> >>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> >>>>>>> ---
> >>>>>>> drivers/clk/tegra/clk-tegra210.c | 115
> >>>>>>> ++++++++++++++++++++++++++++++++++++++-
> >>>>>>> drivers/clk/tegra/clk.c | 14 +++++
> >>>>>>> drivers/clk/tegra/clk.h | 1 +
> >>>>>>> 3 files changed, 127 insertions(+), 3 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
> >>>>>>> b/drivers/clk/tegra/clk-tegra210.c
> >>>>>>> index 1c08c53482a5..1b839544e086 100644
> >>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
> >>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
> >>>>>>> @@ -9,10 +9,12 @@
> >>>>>>> #include <linux/clkdev.h>
> >>>>>>> #include <linux/of.h>
> >>>>>>> #include <linux/of_address.h>
> >>>>>>> +#include <linux/of_platform.h>
> >>>>>>> #include <linux/delay.h>
> >>>>>>> #include <linux/export.h>
> >>>>>>> #include <linux/mutex.h>
> >>>>>>> #include <linux/clk/tegra.h>
> >>>>>>> +#include <linux/syscore_ops.h>
> >>>>>>> #include <dt-bindings/clock/tegra210-car.h>
> >>>>>>> #include <dt-bindings/reset/tegra210-car.h>
> >>>>>>> #include <linux/iopoll.h>
> >>>>>>> @@ -20,6 +22,7 @@
> >>>>>>> #include <soc/tegra/pmc.h>
> >>>>>>> #include "clk.h"
> >>>>>>> +#include "clk-dfll.h"
> >>>>>>> #include "clk-id.h"
> >>>>>>> /*
> >>>>>>> @@ -225,6 +228,7 @@
> >>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
> >>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
> >>>>>>> +#define CPU_SOFTRST_CTRL 0x380
> >>>>>>> #define LVL2_CLK_GATE_OVRA 0xf8
> >>>>>>> #define LVL2_CLK_GATE_OVRC 0x3a0
> >>>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
> >>>>>>> struct tegra_clk_pll_freq_table *fentry;
> >>>>>>> struct tegra_clk_pll pllu;
> >>>>>>> u32 reg;
> >>>>>>> + int ret;
> >>>>>>> for (fentry = pll_u_freq_table; fentry->input_rate;
> >>>>>>> fentry++) {
> >>>>>>> if (fentry->input_rate == pll_ref_freq)
> >>>>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
> >>>>>>> fence_udelay(1, clk_base);
> >>>>>>> reg |= PLL_ENABLE;
> >>>>>>> writel(reg, clk_base + PLLU_BASE);
> >>>>>>> + fence_udelay(1, clk_base);
> >>>>>>> - readl_relaxed_poll_timeout_atomic(clk_base +
> >>>>>>> PLLU_BASE, reg,
> >>>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
> >>>>>>> - if (!(reg & PLL_BASE_LOCK)) {
> >>>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE,
> >>>>>>> PLL_BASE_LOCK);
> >>>>>>> + if (ret) {
> >>>>>>> pr_err("Timed out waiting for PLL_U to lock\n");
> >>>>>>> return -ETIMEDOUT;
> >>>>>>> }
> >>>>>>> @@ -3283,6 +3288,103 @@ static void
> >>>>>>> tegra210_disable_cpu_clock(u32 cpu)
> >>>>>>> }
> >>>>>>> #ifdef CONFIG_PM_SLEEP
> >>>>>>> +static u32 cpu_softrst_ctx[3];
> >>>>>>> +static struct platform_device *dfll_pdev;
> >>>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base +
> >>>>>>> (_base) + ((_off) * 4))
> >>>>>>> +#define car_writel(_val, _base, _off) \
> >>>>>>> + writel_relaxed(_val, clk_base + (_base) + ((_off) *
> >>>>>>> 4)) +
> >>>>>>> +static int tegra210_clk_suspend(void)
> >>>>>>> +{
> >>>>>>> + unsigned int i;
> >>>>>>> + struct device_node *node;
> >>>>>>> +
> >>>>>>> + tegra_cclkg_burst_policy_save_context();
> >>>>>>> +
> >>>>>>> + if (!dfll_pdev) {
> >>>>>>> + node = of_find_compatible_node(NULL, NULL,
> >>>>>>> + "nvidia,tegra210-dfll");
> >>>>>>> + if (node)
> >>>>>>> + dfll_pdev = of_find_device_by_node(node);
> >>>>>>> +
> >>>>>>> + of_node_put(node);
> >>>>>>> + if (!dfll_pdev)
> >>>>>>> + pr_err("dfll node not found. no suspend for
> >>>>>>> dfll\n");
> >>>>>>> + }
> >>>>>>> +
> >>>>>>> + if (dfll_pdev)
> >>>>>>> + tegra_dfll_suspend(dfll_pdev);
> >>>>>>> +
> >>>>>>> + /* Enable PLLP_OUT_CPU after dfll suspend */
> >>>>>>> + tegra_clk_set_pllp_out_cpu(true);
> >>>>>>> +
> >>>>>>> + tegra_sclk_cclklp_burst_policy_save_context();
> >>>>>>> +
> >>>>>>> + clk_save_context();
> >>>>>>> +
> >>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
> >>>>>>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
> >>>>>>> +
> >>>>>>> + return 0;
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> +static void tegra210_clk_resume(void)
> >>>>>>> +{
> >>>>>>> + unsigned int i;
> >>>>>>> + struct clk_hw *parent;
> >>>>>>> + struct clk *clk;
> >>>>>>> +
> >>>>>>> + /*
> >>>>>>> + * clk_restore_context restores clocks as per the clock
> >>>>>>> tree.
> >>>>>>> + *
> >>>>>>> + * dfllCPU_out is first in the clock tree to get
> >>>>>>> restored and it
> >>>>>>> + * involves programming DFLL controller along with
> >>>>>>> restoring CPUG
> >>>>>>> + * clock burst policy.
> >>>>>>> + *
> >>>>>>> + * DFLL programming needs dfll_ref and dfll_soc
> >>>>>>> peripheral clocks
> >>>>>>> + * to be restores which are part ofthe peripheral
> >>>>>>> clocks.
> >>>> ^ white-space
> >>>>
> >>>> Please use spellchecker to avoid typos.
> >>>>
> >>>>>>> + * So, peripheral clocks restore should happen prior to
> >>>>>>> dfll clock
> >>>>>>> + * restore.
> >>>>>>> + */
> >>>>>>> +
> >>>>>>> + tegra_clk_osc_resume(clk_base);
> >>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
> >>>>>>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
> >>>>>>> +
> >>>>>>> + /* restore all plls and peripheral clocks */
> >>>>>>> + tegra210_init_pllu();
> >>>>>>> + clk_restore_context();
> >>>>>>> +
> >>>>>>> + fence_udelay(5, clk_base);
> >>>>>>> +
> >>>>>>> + /* resume SCLK and CPULP clocks */
> >>>>>>> + tegra_sclk_cpulp_burst_policy_restore_context();
> >>>>>>> +
> >>>>>>> + /*
> >>>>>>> + * restore CPUG clocks:
> >>>>>>> + * - enable DFLL in open loop mode
> >>>>>>> + * - switch CPUG to DFLL clock source
> >>>>>>> + * - close DFLL loop
> >>>>>>> + * - sync PLLX state
> >>>>>>> + */
> >>>>>>> + if (dfll_pdev)
> >>>>>>> + tegra_dfll_resume(dfll_pdev, false);
> >>>>>>> +
> >>>>>>> + tegra_cclkg_burst_policy_restore_context();
> >>>>>>> + fence_udelay(2, clk_base);
> >>>>>>> +
> >>>>>>> + if (dfll_pdev)
> >>>>>>> + tegra_dfll_resume(dfll_pdev, true);
> >>>>>>> +
> >>>>>>> + parent =
> >>>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
> >>>>>>> + clk = clks[TEGRA210_CLK_PLL_X];
> >>>>>>> + if (parent != __clk_get_hw(clk))
> >>>>>>> + tegra_clk_sync_state_pll(__clk_get_hw(clk));
> >>>>>>> +
> >>>>>>> + /* Disable PLL_OUT_CPU after DFLL resume */
> >>>>>>> + tegra_clk_set_pllp_out_cpu(false);
> >>>>>>> +}
> >>>>>>> +
> >>>>>>> static void tegra210_cpu_clock_suspend(void)
> >>>>>>> {
> >>>>>>> /* switch coresite to clk_m, save off original source
> >>>>>>> */ @@ -3298,6 +3400,11 @@ static void
> >>>>>>> tegra210_cpu_clock_resume(void) }
> >>>>>>> #endif
> >>>>>>> +static struct syscore_ops tegra_clk_syscore_ops = {
> >>>>>>> + .suspend = tegra210_clk_suspend,
> >>>>>>> + .resume = tegra210_clk_resume,
> >>>>>>> +};
> >>>>>>> +
> >>>>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
> >>>>>>> .wait_for_reset = tegra210_wait_cpu_in_reset,
> >>>>>>> .disable_clock = tegra210_disable_cpu_clock,
> >>>>>>> @@ -3583,5 +3690,7 @@ static void __init
> >>>>>>> tegra210_clock_init(struct device_node *np)
> >>>>>>> tegra210_mbist_clk_init();
> >>>>>>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
> >>>>>>> +
> >>>>>>> + register_syscore_ops(&tegra_clk_syscore_ops);
> >>>>>>> }
> >>>>>> Is it really worthwhile to use syscore_ops for suspend/resume
> >>>>>> given that drivers for
> >>>>>> won't resume before the CLK driver anyway? Are there any other
> >>>>>> options for CLK
> >>>>>> suspend/resume?
> >>>>>>
> >>>>>> I'm also not sure whether PM runtime API could be used at all
> >>>>>> in the context of
> >>>>>> syscore_ops ..
> >>>>>>
> >>>>>> Secondly, what about to use generic clk_save_context() /
> >>>>>> clk_restore_context()
> >>>>>> helpers for the suspend-resume? It looks to me that some other
> >>>>>> essential (and proper)
> >>>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the
> >>>>>> clocks using the generic
> >>>>>> CLK Framework API.
> >>>>> Clock resume should happen very early to restore peripheral and
> >>>>> cpu clocks very early than peripheral drivers resume happens.
> >>>> If all peripheral drivers properly requested all of the
> >>>> necessary clocks and CLK driver was a platform driver, then I
> >>>> guess the probe should have been naturally ordered. But that's
> >>>> not very achievable with the currently available infrastructure
> >>>> in the kernel, so I'm not arguing that the clocks should be
> >>>> explicitly resumed before the users.
> >>>>> this patch series uses clk_save_context and clk_restore_context
> >>>>> for corresponding divider, pll, pllout.. save and restore
> >>>>> context.
> >>>> Now I see that indeed this API is utilized in this patch, thank
> >>>> you for the clarification.
> >>>>
> >>>>> But as there is dependency on dfll resume and cpu and pllx
> >>>>> clocks restore, couldnt use clk_save_context and
> >>>>> clk_restore_context for dfll.
> >>>>>
> >>>>> So implemented recommended dfll resume sequence in main
> >>>>> Tegra210 clock driver along with invoking
> >>>>> clk_save_context/clk_restore_context where all other clocks
> >>>>> save/restore happens as per clock tree traversal.
> >>>> Could you please clarify what part of peripherals clocks is
> >>>> required for DFLL's restore? Couldn't DFLL driver be changed to
> >>>> avoid that quirkness and thus to make DFLL driver suspend/resume
> >>>> the clock?
> >>> DFLL source ref_clk and soc_clk need to be restored prior to dfll.
> >>>
> >>> I see dfllCPU_out parent to CCLK_G first in the clock tree and
> >>> dfll_ref and dfll_soc peripheral clocks are not resumed by the
> >>> time dfll resume happens first.
> >>>
> >>> ref_clk and soc_clk source is from pll_p and clock tree has these
> >>> registered under pll_p which happens later.
> >>>
> >>> tegra210_clock_init registers in order plls, peripheral clocks,
> >>> super_clk init for cclk_g during clock driver probe and dfll
> >>> probe and register happens later.
> >>>
> >> One more thing, CLDVFS peripheral clock enable is also needed to be
> >> enabled to program DFLL Controller and all peripheral clock
> >> context is restored only after their PLL sources are restored.
> >>
> >> DFLL restore involves dfll source clock resume along with CLDVFS
> >> periheral clock enable and reset
> >>
> > I don't quite see why you can't simply add suspend/resume callbacks
> > to the CPUFreq driver to:
> >
> > On suspend:
> > 1. Switch CPU to PLLP (or whatever "safe" parent)
> > 2. Disable/teardown DFLL
> >
> > On resume:
> > 1. Enable/restore DFLL
> > 2. Switch CPU back to DFLL
>
> dfll runtime suspend/resume are already part of dfll_pm_ops. Don't we
> want to use it for suspend/resume as well?
Looks like no. Seems runtime PM of that driver is intended solely for
the DFLL's clk management.
> currently no APIs are shared b/w clk/tegra driver and CPUFreq driver
> to invoke dfll suspend/resume in CPUFreq driver
>
Just add it. Also, please note that CPUFreq driver is optional and thus
you may need to switch CPU to a safe parent on clk-core suspend as
well in order to resume properly if CPU was running off unsafe parent
during boot and CPUFreq driver is disabled in kernel build (or failed
to load).
The other thing that also need attention is that T124 CPUFreq driver
implicitly relies on DFLL driver to be probed first, which is icky.
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Sowjanya Komatineni @ 2019-07-16 4:37 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: <0ee055ad-d397-32e5-60ee-d62c14c6f77b@gmail.com>
On 7/15/19 8:50 PM, Dmitry Osipenko wrote:
> 16.07.2019 6:00, Sowjanya Komatineni пишет:
>> On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
>>> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
>>>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
>>>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
>>>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
>>>>>>> This patch adds system suspend and resume support for Tegra210
>>>>>>> clocks.
>>>>>>>
>>>>>>> All the CAR controller settings are lost on suspend when core power
>>>>>>> goes off.
>>>>>>>
>>>>>>> This patch has implementation for saving and restoring all the PLLs
>>>>>>> and clocks context during system suspend and resume to have the
>>>>>>> clocks back to same state for normal operation.
>>>>>>>
>>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>>> ---
>>>>>>> drivers/clk/tegra/clk-tegra210.c | 115
>>>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>>> drivers/clk/tegra/clk.c | 14 +++++
>>>>>>> drivers/clk/tegra/clk.h | 1 +
>>>>>>> 3 files changed, 127 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>>>>>> b/drivers/clk/tegra/clk-tegra210.c
>>>>>>> index 1c08c53482a5..1b839544e086 100644
>>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>>> @@ -9,10 +9,12 @@
>>>>>>> #include <linux/clkdev.h>
>>>>>>> #include <linux/of.h>
>>>>>>> #include <linux/of_address.h>
>>>>>>> +#include <linux/of_platform.h>
>>>>>>> #include <linux/delay.h>
>>>>>>> #include <linux/export.h>
>>>>>>> #include <linux/mutex.h>
>>>>>>> #include <linux/clk/tegra.h>
>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>> #include <dt-bindings/clock/tegra210-car.h>
>>>>>>> #include <dt-bindings/reset/tegra210-car.h>
>>>>>>> #include <linux/iopoll.h>
>>>>>>> @@ -20,6 +22,7 @@
>>>>>>> #include <soc/tegra/pmc.h>
>>>>>>> #include "clk.h"
>>>>>>> +#include "clk-dfll.h"
>>>>>>> #include "clk-id.h"
>>>>>>> /*
>>>>>>> @@ -225,6 +228,7 @@
>>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>>> #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>>> #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
>>>>>>> struct tegra_clk_pll_freq_table *fentry;
>>>>>>> struct tegra_clk_pll pllu;
>>>>>>> u32 reg;
>>>>>>> + int ret;
>>>>>>> for (fentry = pll_u_freq_table; fentry->input_rate;
>>>>>>> fentry++) {
>>>>>>> if (fentry->input_rate == pll_ref_freq)
>>>>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
>>>>>>> fence_udelay(1, clk_base);
>>>>>>> reg |= PLL_ENABLE;
>>>>>>> writel(reg, clk_base + PLLU_BASE);
>>>>>>> + fence_udelay(1, clk_base);
>>>>>>> - readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
>>>>>>> - if (!(reg & PLL_BASE_LOCK)) {
>>>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>>> + if (ret) {
>>>>>>> pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>>> return -ETIMEDOUT;
>>>>>>> }
>>>>>>> @@ -3283,6 +3288,103 @@ static void tegra210_disable_cpu_clock(u32
>>>>>>> cpu)
>>>>>>> }
>>>>>>> #ifdef CONFIG_PM_SLEEP
>>>>>>> +static u32 cpu_softrst_ctx[3];
>>>>>>> +static struct platform_device *dfll_pdev;
>>>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) +
>>>>>>> ((_off) * 4))
>>>>>>> +#define car_writel(_val, _base, _off) \
>>>>>>> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>>>>>>> +
>>>>>>> +static int tegra210_clk_suspend(void)
>>>>>>> +{
>>>>>>> + unsigned int i;
>>>>>>> + struct device_node *node;
>>>>>>> +
>>>>>>> + tegra_cclkg_burst_policy_save_context();
>>>>>>> +
>>>>>>> + if (!dfll_pdev) {
>>>>>>> + node = of_find_compatible_node(NULL, NULL,
>>>>>>> + "nvidia,tegra210-dfll");
>>>>>>> + if (node)
>>>>>>> + dfll_pdev = of_find_device_by_node(node);
>>>>>>> +
>>>>>>> + of_node_put(node);
>>>>>>> + if (!dfll_pdev)
>>>>>>> + pr_err("dfll node not found. no suspend for dfll\n");
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (dfll_pdev)
>>>>>>> + tegra_dfll_suspend(dfll_pdev);
>>>>>>> +
>>>>>>> + /* Enable PLLP_OUT_CPU after dfll suspend */
>>>>>>> + tegra_clk_set_pllp_out_cpu(true);
>>>>>>> +
>>>>>>> + tegra_sclk_cclklp_burst_policy_save_context();
>>>>>>> +
>>>>>>> + clk_save_context();
>>>>>>> +
>>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>>>>>>> +
>>>>>>> + return 0;
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void tegra210_clk_resume(void)
>>>>>>> +{
>>>>>>> + unsigned int i;
>>>>>>> + struct clk_hw *parent;
>>>>>>> + struct clk *clk;
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * clk_restore_context restores clocks as per the clock tree.
>>>>>>> + *
>>>>>>> + * dfllCPU_out is first in the clock tree to get restored and it
>>>>>>> + * involves programming DFLL controller along with restoring
>>>>>>> CPUG
>>>>>>> + * clock burst policy.
>>>>>>> + *
>>>>>>> + * DFLL programming needs dfll_ref and dfll_soc peripheral
>>>>>>> clocks
>>>>>>> + * to be restores which are part ofthe peripheral clocks.
>>>> ^ white-space
>>>>
>>>> Please use spellchecker to avoid typos.
>>>>
>>>>>>> + * So, peripheral clocks restore should happen prior to dfll
>>>>>>> clock
>>>>>>> + * restore.
>>>>>>> + */
>>>>>>> +
>>>>>>> + tegra_clk_osc_resume(clk_base);
>>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>>>>>> +
>>>>>>> + /* restore all plls and peripheral clocks */
>>>>>>> + tegra210_init_pllu();
>>>>>>> + clk_restore_context();
>>>>>>> +
>>>>>>> + fence_udelay(5, clk_base);
>>>>>>> +
>>>>>>> + /* resume SCLK and CPULP clocks */
>>>>>>> + tegra_sclk_cpulp_burst_policy_restore_context();
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * restore CPUG clocks:
>>>>>>> + * - enable DFLL in open loop mode
>>>>>>> + * - switch CPUG to DFLL clock source
>>>>>>> + * - close DFLL loop
>>>>>>> + * - sync PLLX state
>>>>>>> + */
>>>>>>> + if (dfll_pdev)
>>>>>>> + tegra_dfll_resume(dfll_pdev, false);
>>>>>>> +
>>>>>>> + tegra_cclkg_burst_policy_restore_context();
>>>>>>> + fence_udelay(2, clk_base);
>>>>>>> +
>>>>>>> + if (dfll_pdev)
>>>>>>> + tegra_dfll_resume(dfll_pdev, true);
>>>>>>> +
>>>>>>> + parent =
>>>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
>>>>>>> + clk = clks[TEGRA210_CLK_PLL_X];
>>>>>>> + if (parent != __clk_get_hw(clk))
>>>>>>> + tegra_clk_sync_state_pll(__clk_get_hw(clk));
>>>>>>> +
>>>>>>> + /* Disable PLL_OUT_CPU after DFLL resume */
>>>>>>> + tegra_clk_set_pllp_out_cpu(false);
>>>>>>> +}
>>>>>>> +
>>>>>>> static void tegra210_cpu_clock_suspend(void)
>>>>>>> {
>>>>>>> /* switch coresite to clk_m, save off original source */
>>>>>>> @@ -3298,6 +3400,11 @@ static void tegra210_cpu_clock_resume(void)
>>>>>>> }
>>>>>>> #endif
>>>>>>> +static struct syscore_ops tegra_clk_syscore_ops = {
>>>>>>> + .suspend = tegra210_clk_suspend,
>>>>>>> + .resume = tegra210_clk_resume,
>>>>>>> +};
>>>>>>> +
>>>>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>>>>>>> .wait_for_reset = tegra210_wait_cpu_in_reset,
>>>>>>> .disable_clock = tegra210_disable_cpu_clock,
>>>>>>> @@ -3583,5 +3690,7 @@ static void __init tegra210_clock_init(struct
>>>>>>> device_node *np)
>>>>>>> tegra210_mbist_clk_init();
>>>>>>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>>>>>>> +
>>>>>>> + register_syscore_ops(&tegra_clk_syscore_ops);
>>>>>>> }
>>>>>> Is it really worthwhile to use syscore_ops for suspend/resume given
>>>>>> that drivers for
>>>>>> won't resume before the CLK driver anyway? Are there any other options
>>>>>> for CLK
>>>>>> suspend/resume?
>>>>>>
>>>>>> I'm also not sure whether PM runtime API could be used at all in the
>>>>>> context of
>>>>>> syscore_ops ..
>>>>>>
>>>>>> Secondly, what about to use generic clk_save_context() /
>>>>>> clk_restore_context()
>>>>>> helpers for the suspend-resume? It looks to me that some other
>>>>>> essential (and proper)
>>>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the clocks
>>>>>> using the generic
>>>>>> CLK Framework API.
>>>>> Clock resume should happen very early to restore peripheral and cpu
>>>>> clocks very early than peripheral drivers resume happens.
>>>> If all peripheral drivers properly requested all of the necessary clocks
>>>> and CLK driver was a platform driver, then I guess the probe should have
>>>> been naturally ordered. But that's not very achievable with the
>>>> currently available infrastructure in the kernel, so I'm not arguing
>>>> that the clocks should be explicitly resumed before the users.
>>>>
>>>>> this patch series uses clk_save_context and clk_restore_context for
>>>>> corresponding divider, pll, pllout.. save and restore context.
>>>> Now I see that indeed this API is utilized in this patch, thank you for
>>>> the clarification.
>>>>
>>>>> But as there is dependency on dfll resume and cpu and pllx clocks
>>>>> restore, couldnt use clk_save_context and clk_restore_context for dfll.
>>>>>
>>>>> So implemented recommended dfll resume sequence in main Tegra210 clock
>>>>> driver along with invoking clk_save_context/clk_restore_context where
>>>>> all other clocks save/restore happens as per clock tree traversal.
>>>> Could you please clarify what part of peripherals clocks is required for
>>>> DFLL's restore? Couldn't DFLL driver be changed to avoid that quirkness
>>>> and thus to make DFLL driver suspend/resume the clock?
>>> DFLL source ref_clk and soc_clk need to be restored prior to dfll.
>>>
>>> I see dfllCPU_out parent to CCLK_G first in the clock tree and
>>> dfll_ref and dfll_soc peripheral clocks are not resumed by the time
>>> dfll resume happens first.
>>>
>>> ref_clk and soc_clk source is from pll_p and clock tree has these
>>> registered under pll_p which happens later.
>>>
>>> tegra210_clock_init registers in order plls, peripheral clocks,
>>> super_clk init for cclk_g during clock driver probe and dfll probe and
>>> register happens later.
>>>
>> One more thing, CLDVFS peripheral clock enable is also needed to be
>> enabled to program DFLL Controller and all peripheral clock context is
>> restored only after their PLL sources are restored.
>>
>> DFLL restore involves dfll source clock resume along with CLDVFS
>> periheral clock enable and reset
>>
> I don't quite see why you can't simply add suspend/resume callbacks to
> the CPUFreq driver to:
>
> On suspend:
> 1. Switch CPU to PLLP (or whatever "safe" parent)
> 2. Disable/teardown DFLL
>
> On resume:
> 1. Enable/restore DFLL
> 2. Switch CPU back to DFLL
dfll runtime suspend/resume are already part of dfll_pm_ops. Don't we
want to use it for suspend/resume as well?
currently no APIs are shared b/w clk/tegra driver and CPUFreq driver to
invoke dfll suspend/resume in CPUFreq driver
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Dmitry Osipenko @ 2019-07-16 3:50 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: <932d4d50-120c-9191-6a9a-23bf9c96633b@nvidia.com>
16.07.2019 6:00, Sowjanya Komatineni пишет:
>
> On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
>>
>> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
>>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
>>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
>>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
>>>>>> This patch adds system suspend and resume support for Tegra210
>>>>>> clocks.
>>>>>>
>>>>>> All the CAR controller settings are lost on suspend when core power
>>>>>> goes off.
>>>>>>
>>>>>> This patch has implementation for saving and restoring all the PLLs
>>>>>> and clocks context during system suspend and resume to have the
>>>>>> clocks back to same state for normal operation.
>>>>>>
>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>> drivers/clk/tegra/clk-tegra210.c | 115
>>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>> drivers/clk/tegra/clk.c | 14 +++++
>>>>>> drivers/clk/tegra/clk.h | 1 +
>>>>>> 3 files changed, 127 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>>>>> b/drivers/clk/tegra/clk-tegra210.c
>>>>>> index 1c08c53482a5..1b839544e086 100644
>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>> @@ -9,10 +9,12 @@
>>>>>> #include <linux/clkdev.h>
>>>>>> #include <linux/of.h>
>>>>>> #include <linux/of_address.h>
>>>>>> +#include <linux/of_platform.h>
>>>>>> #include <linux/delay.h>
>>>>>> #include <linux/export.h>
>>>>>> #include <linux/mutex.h>
>>>>>> #include <linux/clk/tegra.h>
>>>>>> +#include <linux/syscore_ops.h>
>>>>>> #include <dt-bindings/clock/tegra210-car.h>
>>>>>> #include <dt-bindings/reset/tegra210-car.h>
>>>>>> #include <linux/iopoll.h>
>>>>>> @@ -20,6 +22,7 @@
>>>>>> #include <soc/tegra/pmc.h>
>>>>>> #include "clk.h"
>>>>>> +#include "clk-dfll.h"
>>>>>> #include "clk-id.h"
>>>>>> /*
>>>>>> @@ -225,6 +228,7 @@
>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>> #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>> #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
>>>>>> struct tegra_clk_pll_freq_table *fentry;
>>>>>> struct tegra_clk_pll pllu;
>>>>>> u32 reg;
>>>>>> + int ret;
>>>>>> for (fentry = pll_u_freq_table; fentry->input_rate;
>>>>>> fentry++) {
>>>>>> if (fentry->input_rate == pll_ref_freq)
>>>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
>>>>>> fence_udelay(1, clk_base);
>>>>>> reg |= PLL_ENABLE;
>>>>>> writel(reg, clk_base + PLLU_BASE);
>>>>>> + fence_udelay(1, clk_base);
>>>>>> - readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
>>>>>> - if (!(reg & PLL_BASE_LOCK)) {
>>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>> + if (ret) {
>>>>>> pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>> return -ETIMEDOUT;
>>>>>> }
>>>>>> @@ -3283,6 +3288,103 @@ static void tegra210_disable_cpu_clock(u32
>>>>>> cpu)
>>>>>> }
>>>>>> #ifdef CONFIG_PM_SLEEP
>>>>>> +static u32 cpu_softrst_ctx[3];
>>>>>> +static struct platform_device *dfll_pdev;
>>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) +
>>>>>> ((_off) * 4))
>>>>>> +#define car_writel(_val, _base, _off) \
>>>>>> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>>>>>> +
>>>>>> +static int tegra210_clk_suspend(void)
>>>>>> +{
>>>>>> + unsigned int i;
>>>>>> + struct device_node *node;
>>>>>> +
>>>>>> + tegra_cclkg_burst_policy_save_context();
>>>>>> +
>>>>>> + if (!dfll_pdev) {
>>>>>> + node = of_find_compatible_node(NULL, NULL,
>>>>>> + "nvidia,tegra210-dfll");
>>>>>> + if (node)
>>>>>> + dfll_pdev = of_find_device_by_node(node);
>>>>>> +
>>>>>> + of_node_put(node);
>>>>>> + if (!dfll_pdev)
>>>>>> + pr_err("dfll node not found. no suspend for dfll\n");
>>>>>> + }
>>>>>> +
>>>>>> + if (dfll_pdev)
>>>>>> + tegra_dfll_suspend(dfll_pdev);
>>>>>> +
>>>>>> + /* Enable PLLP_OUT_CPU after dfll suspend */
>>>>>> + tegra_clk_set_pllp_out_cpu(true);
>>>>>> +
>>>>>> + tegra_sclk_cclklp_burst_policy_save_context();
>>>>>> +
>>>>>> + clk_save_context();
>>>>>> +
>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void tegra210_clk_resume(void)
>>>>>> +{
>>>>>> + unsigned int i;
>>>>>> + struct clk_hw *parent;
>>>>>> + struct clk *clk;
>>>>>> +
>>>>>> + /*
>>>>>> + * clk_restore_context restores clocks as per the clock tree.
>>>>>> + *
>>>>>> + * dfllCPU_out is first in the clock tree to get restored and it
>>>>>> + * involves programming DFLL controller along with restoring
>>>>>> CPUG
>>>>>> + * clock burst policy.
>>>>>> + *
>>>>>> + * DFLL programming needs dfll_ref and dfll_soc peripheral
>>>>>> clocks
>>>>>> + * to be restores which are part ofthe peripheral clocks.
>>> ^ white-space
>>>
>>> Please use spellchecker to avoid typos.
>>>
>>>>>> + * So, peripheral clocks restore should happen prior to dfll
>>>>>> clock
>>>>>> + * restore.
>>>>>> + */
>>>>>> +
>>>>>> + tegra_clk_osc_resume(clk_base);
>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>>>>> +
>>>>>> + /* restore all plls and peripheral clocks */
>>>>>> + tegra210_init_pllu();
>>>>>> + clk_restore_context();
>>>>>> +
>>>>>> + fence_udelay(5, clk_base);
>>>>>> +
>>>>>> + /* resume SCLK and CPULP clocks */
>>>>>> + tegra_sclk_cpulp_burst_policy_restore_context();
>>>>>> +
>>>>>> + /*
>>>>>> + * restore CPUG clocks:
>>>>>> + * - enable DFLL in open loop mode
>>>>>> + * - switch CPUG to DFLL clock source
>>>>>> + * - close DFLL loop
>>>>>> + * - sync PLLX state
>>>>>> + */
>>>>>> + if (dfll_pdev)
>>>>>> + tegra_dfll_resume(dfll_pdev, false);
>>>>>> +
>>>>>> + tegra_cclkg_burst_policy_restore_context();
>>>>>> + fence_udelay(2, clk_base);
>>>>>> +
>>>>>> + if (dfll_pdev)
>>>>>> + tegra_dfll_resume(dfll_pdev, true);
>>>>>> +
>>>>>> + parent =
>>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
>>>>>> + clk = clks[TEGRA210_CLK_PLL_X];
>>>>>> + if (parent != __clk_get_hw(clk))
>>>>>> + tegra_clk_sync_state_pll(__clk_get_hw(clk));
>>>>>> +
>>>>>> + /* Disable PLL_OUT_CPU after DFLL resume */
>>>>>> + tegra_clk_set_pllp_out_cpu(false);
>>>>>> +}
>>>>>> +
>>>>>> static void tegra210_cpu_clock_suspend(void)
>>>>>> {
>>>>>> /* switch coresite to clk_m, save off original source */
>>>>>> @@ -3298,6 +3400,11 @@ static void tegra210_cpu_clock_resume(void)
>>>>>> }
>>>>>> #endif
>>>>>> +static struct syscore_ops tegra_clk_syscore_ops = {
>>>>>> + .suspend = tegra210_clk_suspend,
>>>>>> + .resume = tegra210_clk_resume,
>>>>>> +};
>>>>>> +
>>>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>>>>>> .wait_for_reset = tegra210_wait_cpu_in_reset,
>>>>>> .disable_clock = tegra210_disable_cpu_clock,
>>>>>> @@ -3583,5 +3690,7 @@ static void __init tegra210_clock_init(struct
>>>>>> device_node *np)
>>>>>> tegra210_mbist_clk_init();
>>>>>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>>>>>> +
>>>>>> + register_syscore_ops(&tegra_clk_syscore_ops);
>>>>>> }
>>>>> Is it really worthwhile to use syscore_ops for suspend/resume given
>>>>> that drivers for
>>>>> won't resume before the CLK driver anyway? Are there any other options
>>>>> for CLK
>>>>> suspend/resume?
>>>>>
>>>>> I'm also not sure whether PM runtime API could be used at all in the
>>>>> context of
>>>>> syscore_ops ..
>>>>>
>>>>> Secondly, what about to use generic clk_save_context() /
>>>>> clk_restore_context()
>>>>> helpers for the suspend-resume? It looks to me that some other
>>>>> essential (and proper)
>>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the clocks
>>>>> using the generic
>>>>> CLK Framework API.
>>>> Clock resume should happen very early to restore peripheral and cpu
>>>> clocks very early than peripheral drivers resume happens.
>>> If all peripheral drivers properly requested all of the necessary clocks
>>> and CLK driver was a platform driver, then I guess the probe should have
>>> been naturally ordered. But that's not very achievable with the
>>> currently available infrastructure in the kernel, so I'm not arguing
>>> that the clocks should be explicitly resumed before the users.
>>>
>>>> this patch series uses clk_save_context and clk_restore_context for
>>>> corresponding divider, pll, pllout.. save and restore context.
>>> Now I see that indeed this API is utilized in this patch, thank you for
>>> the clarification.
>>>
>>>> But as there is dependency on dfll resume and cpu and pllx clocks
>>>> restore, couldnt use clk_save_context and clk_restore_context for dfll.
>>>>
>>>> So implemented recommended dfll resume sequence in main Tegra210 clock
>>>> driver along with invoking clk_save_context/clk_restore_context where
>>>> all other clocks save/restore happens as per clock tree traversal.
>>> Could you please clarify what part of peripherals clocks is required for
>>> DFLL's restore? Couldn't DFLL driver be changed to avoid that quirkness
>>> and thus to make DFLL driver suspend/resume the clock?
>>
>> DFLL source ref_clk and soc_clk need to be restored prior to dfll.
>>
>> I see dfllCPU_out parent to CCLK_G first in the clock tree and
>> dfll_ref and dfll_soc peripheral clocks are not resumed by the time
>> dfll resume happens first.
>>
>> ref_clk and soc_clk source is from pll_p and clock tree has these
>> registered under pll_p which happens later.
>>
>> tegra210_clock_init registers in order plls, peripheral clocks,
>> super_clk init for cclk_g during clock driver probe and dfll probe and
>> register happens later.
>>
> One more thing, CLDVFS peripheral clock enable is also needed to be
> enabled to program DFLL Controller and all peripheral clock context is
> restored only after their PLL sources are restored.
>
> DFLL restore involves dfll source clock resume along with CLDVFS
> periheral clock enable and reset
>
I don't quite see why you can't simply add suspend/resume callbacks to
the CPUFreq driver to:
On suspend:
1. Switch CPU to PLLP (or whatever "safe" parent)
2. Disable/teardown DFLL
On resume:
1. Enable/restore DFLL
2. Switch CPU back to DFLL
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Sowjanya Komatineni @ 2019-07-16 3:41 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: <932d4d50-120c-9191-6a9a-23bf9c96633b@nvidia.com>
On 7/15/19 8:00 PM, Sowjanya Komatineni wrote:
>
> On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
>>
>> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
>>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
>>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
>>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
>>>>>> This patch adds system suspend and resume support for Tegra210
>>>>>> clocks.
>>>>>>
>>>>>> All the CAR controller settings are lost on suspend when core power
>>>>>> goes off.
>>>>>>
>>>>>> This patch has implementation for saving and restoring all the PLLs
>>>>>> and clocks context during system suspend and resume to have the
>>>>>> clocks back to same state for normal operation.
>>>>>>
>>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>>> ---
>>>>>> drivers/clk/tegra/clk-tegra210.c | 115
>>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>>> drivers/clk/tegra/clk.c | 14 +++++
>>>>>> drivers/clk/tegra/clk.h | 1 +
>>>>>> 3 files changed, 127 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>>>>> b/drivers/clk/tegra/clk-tegra210.c
>>>>>> index 1c08c53482a5..1b839544e086 100644
>>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>>> @@ -9,10 +9,12 @@
>>>>>> #include <linux/clkdev.h>
>>>>>> #include <linux/of.h>
>>>>>> #include <linux/of_address.h>
>>>>>> +#include <linux/of_platform.h>
>>>>>> #include <linux/delay.h>
>>>>>> #include <linux/export.h>
>>>>>> #include <linux/mutex.h>
>>>>>> #include <linux/clk/tegra.h>
>>>>>> +#include <linux/syscore_ops.h>
>>>>>> #include <dt-bindings/clock/tegra210-car.h>
>>>>>> #include <dt-bindings/reset/tegra210-car.h>
>>>>>> #include <linux/iopoll.h>
>>>>>> @@ -20,6 +22,7 @@
>>>>>> #include <soc/tegra/pmc.h>
>>>>>> #include "clk.h"
>>>>>> +#include "clk-dfll.h"
>>>>>> #include "clk-id.h"
>>>>>> /*
>>>>>> @@ -225,6 +228,7 @@
>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>>> #define LVL2_CLK_GATE_OVRA 0xf8
>>>>>> #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
>>>>>> struct tegra_clk_pll_freq_table *fentry;
>>>>>> struct tegra_clk_pll pllu;
>>>>>> u32 reg;
>>>>>> + int ret;
>>>>>> for (fentry = pll_u_freq_table; fentry->input_rate;
>>>>>> fentry++) {
>>>>>> if (fentry->input_rate == pll_ref_freq)
>>>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
>>>>>> fence_udelay(1, clk_base);
>>>>>> reg |= PLL_ENABLE;
>>>>>> writel(reg, clk_base + PLLU_BASE);
>>>>>> + fence_udelay(1, clk_base);
>>>>>> - readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
>>>>>> - if (!(reg & PLL_BASE_LOCK)) {
>>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>>> + if (ret) {
>>>>>> pr_err("Timed out waiting for PLL_U to lock\n");
>>>>>> return -ETIMEDOUT;
>>>>>> }
>>>>>> @@ -3283,6 +3288,103 @@ static void
>>>>>> tegra210_disable_cpu_clock(u32 cpu)
>>>>>> }
>>>>>> #ifdef CONFIG_PM_SLEEP
>>>>>> +static u32 cpu_softrst_ctx[3];
>>>>>> +static struct platform_device *dfll_pdev;
>>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) +
>>>>>> ((_off) * 4))
>>>>>> +#define car_writel(_val, _base, _off) \
>>>>>> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>>>>>> +
>>>>>> +static int tegra210_clk_suspend(void)
>>>>>> +{
>>>>>> + unsigned int i;
>>>>>> + struct device_node *node;
>>>>>> +
>>>>>> + tegra_cclkg_burst_policy_save_context();
>>>>>> +
>>>>>> + if (!dfll_pdev) {
>>>>>> + node = of_find_compatible_node(NULL, NULL,
>>>>>> + "nvidia,tegra210-dfll");
>>>>>> + if (node)
>>>>>> + dfll_pdev = of_find_device_by_node(node);
>>>>>> +
>>>>>> + of_node_put(node);
>>>>>> + if (!dfll_pdev)
>>>>>> + pr_err("dfll node not found. no suspend for dfll\n");
>>>>>> + }
>>>>>> +
>>>>>> + if (dfll_pdev)
>>>>>> + tegra_dfll_suspend(dfll_pdev);
>>>>>> +
>>>>>> + /* Enable PLLP_OUT_CPU after dfll suspend */
>>>>>> + tegra_clk_set_pllp_out_cpu(true);
>>>>>> +
>>>>>> + tegra_sclk_cclklp_burst_policy_save_context();
>>>>>> +
>>>>>> + clk_save_context();
>>>>>> +
>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static void tegra210_clk_resume(void)
>>>>>> +{
>>>>>> + unsigned int i;
>>>>>> + struct clk_hw *parent;
>>>>>> + struct clk *clk;
>>>>>> +
>>>>>> + /*
>>>>>> + * clk_restore_context restores clocks as per the clock tree.
>>>>>> + *
>>>>>> + * dfllCPU_out is first in the clock tree to get restored
>>>>>> and it
>>>>>> + * involves programming DFLL controller along with restoring
>>>>>> CPUG
>>>>>> + * clock burst policy.
>>>>>> + *
>>>>>> + * DFLL programming needs dfll_ref and dfll_soc peripheral
>>>>>> clocks
>>>>>> + * to be restores which are part ofthe peripheral clocks.
>>> ^ white-space
>>>
>>> Please use spellchecker to avoid typos.
>>>
>>>>>> + * So, peripheral clocks restore should happen prior to dfll
>>>>>> clock
>>>>>> + * restore.
>>>>>> + */
>>>>>> +
>>>>>> + tegra_clk_osc_resume(clk_base);
>>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>>>>> +
>>>>>> + /* restore all plls and peripheral clocks */
>>>>>> + tegra210_init_pllu();
>>>>>> + clk_restore_context();
>>>>>> +
>>>>>> + fence_udelay(5, clk_base);
>>>>>> +
>>>>>> + /* resume SCLK and CPULP clocks */
>>>>>> + tegra_sclk_cpulp_burst_policy_restore_context();
>>>>>> +
>>>>>> + /*
>>>>>> + * restore CPUG clocks:
>>>>>> + * - enable DFLL in open loop mode
>>>>>> + * - switch CPUG to DFLL clock source
>>>>>> + * - close DFLL loop
>>>>>> + * - sync PLLX state
>>>>>> + */
>>>>>> + if (dfll_pdev)
>>>>>> + tegra_dfll_resume(dfll_pdev, false);
>>>>>> +
>>>>>> + tegra_cclkg_burst_policy_restore_context();
>>>>>> + fence_udelay(2, clk_base);
>>>>>> +
>>>>>> + if (dfll_pdev)
>>>>>> + tegra_dfll_resume(dfll_pdev, true);
>>>>>> +
>>>>>> + parent =
>>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
>>>>>> + clk = clks[TEGRA210_CLK_PLL_X];
>>>>>> + if (parent != __clk_get_hw(clk))
>>>>>> + tegra_clk_sync_state_pll(__clk_get_hw(clk));
>>>>>> +
>>>>>> + /* Disable PLL_OUT_CPU after DFLL resume */
>>>>>> + tegra_clk_set_pllp_out_cpu(false);
>>>>>> +}
>>>>>> +
>>>>>> static void tegra210_cpu_clock_suspend(void)
>>>>>> {
>>>>>> /* switch coresite to clk_m, save off original source */
>>>>>> @@ -3298,6 +3400,11 @@ static void tegra210_cpu_clock_resume(void)
>>>>>> }
>>>>>> #endif
>>>>>> +static struct syscore_ops tegra_clk_syscore_ops = {
>>>>>> + .suspend = tegra210_clk_suspend,
>>>>>> + .resume = tegra210_clk_resume,
>>>>>> +};
>>>>>> +
>>>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>>>>>> .wait_for_reset = tegra210_wait_cpu_in_reset,
>>>>>> .disable_clock = tegra210_disable_cpu_clock,
>>>>>> @@ -3583,5 +3690,7 @@ static void __init tegra210_clock_init(struct
>>>>>> device_node *np)
>>>>>> tegra210_mbist_clk_init();
>>>>>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>>>>>> +
>>>>>> + register_syscore_ops(&tegra_clk_syscore_ops);
>>>>>> }
>>>>> Is it really worthwhile to use syscore_ops for suspend/resume given
>>>>> that drivers for
>>>>> won't resume before the CLK driver anyway? Are there any other
>>>>> options
>>>>> for CLK
>>>>> suspend/resume?
>>>>>
>>>>> I'm also not sure whether PM runtime API could be used at all in the
>>>>> context of
>>>>> syscore_ops ..
>>>>>
>>>>> Secondly, what about to use generic clk_save_context() /
>>>>> clk_restore_context()
>>>>> helpers for the suspend-resume? It looks to me that some other
>>>>> essential (and proper)
>>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the clocks
>>>>> using the generic
>>>>> CLK Framework API.
>>>> Clock resume should happen very early to restore peripheral and cpu
>>>> clocks very early than peripheral drivers resume happens.
>>> If all peripheral drivers properly requested all of the necessary
>>> clocks
>>> and CLK driver was a platform driver, then I guess the probe should
>>> have
>>> been naturally ordered. But that's not very achievable with the
>>> currently available infrastructure in the kernel, so I'm not arguing
>>> that the clocks should be explicitly resumed before the users.
>>>
>>>> this patch series uses clk_save_context and clk_restore_context for
>>>> corresponding divider, pll, pllout.. save and restore context.
>>> Now I see that indeed this API is utilized in this patch, thank you for
>>> the clarification.
>>>
>>>> But as there is dependency on dfll resume and cpu and pllx clocks
>>>> restore, couldnt use clk_save_context and clk_restore_context for
>>>> dfll.
>>>>
>>>> So implemented recommended dfll resume sequence in main Tegra210 clock
>>>> driver along with invoking clk_save_context/clk_restore_context where
>>>> all other clocks save/restore happens as per clock tree traversal.
>>> Could you please clarify what part of peripherals clocks is required
>>> for
>>> DFLL's restore? Couldn't DFLL driver be changed to avoid that quirkness
>>> and thus to make DFLL driver suspend/resume the clock?
>>
>> DFLL source ref_clk and soc_clk need to be restored prior to dfll.
>>
>> I see dfllCPU_out parent to CCLK_G first in the clock tree and
>> dfll_ref and dfll_soc peripheral clocks are not resumed by the time
>> dfll resume happens first.
>>
>> ref_clk and soc_clk source is from pll_p and clock tree has these
>> registered under pll_p which happens later.
>>
>> tegra210_clock_init registers in order plls, peripheral clocks,
>> super_clk init for cclk_g during clock driver probe and dfll probe
>> and register happens later.
>>
> One more thing, CLDVFS peripheral clock enable is also needed to be
> enabled to program DFLL Controller and all peripheral clock context is
> restored only after their PLL sources are restored.
>
> DFLL restore involves dfll source clock resume along with CLDVFS
> periheral clock enable and reset
>
Will try with dfll_pm_ops instead of dfll suspend/resume in tegra210
clock driver...
^ permalink raw reply
* Re: [PATCH V5 11/18] clk: tegra210: Add support for Tegra210 clocks
From: Sowjanya Komatineni @ 2019-07-16 3:00 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: <3938092a-bbc7-b304-641d-31677539598d@nvidia.com>
On 7/15/19 5:35 PM, Sowjanya Komatineni wrote:
>
> On 7/14/19 2:41 PM, Dmitry Osipenko wrote:
>> 13.07.2019 8:54, Sowjanya Komatineni пишет:
>>> On 6/29/19 8:10 AM, Dmitry Osipenko wrote:
>>>> 28.06.2019 5:12, Sowjanya Komatineni пишет:
>>>>> This patch adds system suspend and resume support for Tegra210
>>>>> clocks.
>>>>>
>>>>> All the CAR controller settings are lost on suspend when core power
>>>>> goes off.
>>>>>
>>>>> This patch has implementation for saving and restoring all the PLLs
>>>>> and clocks context during system suspend and resume to have the
>>>>> clocks back to same state for normal operation.
>>>>>
>>>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>>> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
>>>>> ---
>>>>> drivers/clk/tegra/clk-tegra210.c | 115
>>>>> ++++++++++++++++++++++++++++++++++++++-
>>>>> drivers/clk/tegra/clk.c | 14 +++++
>>>>> drivers/clk/tegra/clk.h | 1 +
>>>>> 3 files changed, 127 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/tegra/clk-tegra210.c
>>>>> b/drivers/clk/tegra/clk-tegra210.c
>>>>> index 1c08c53482a5..1b839544e086 100644
>>>>> --- a/drivers/clk/tegra/clk-tegra210.c
>>>>> +++ b/drivers/clk/tegra/clk-tegra210.c
>>>>> @@ -9,10 +9,12 @@
>>>>> #include <linux/clkdev.h>
>>>>> #include <linux/of.h>
>>>>> #include <linux/of_address.h>
>>>>> +#include <linux/of_platform.h>
>>>>> #include <linux/delay.h>
>>>>> #include <linux/export.h>
>>>>> #include <linux/mutex.h>
>>>>> #include <linux/clk/tegra.h>
>>>>> +#include <linux/syscore_ops.h>
>>>>> #include <dt-bindings/clock/tegra210-car.h>
>>>>> #include <dt-bindings/reset/tegra210-car.h>
>>>>> #include <linux/iopoll.h>
>>>>> @@ -20,6 +22,7 @@
>>>>> #include <soc/tegra/pmc.h>
>>>>> #include "clk.h"
>>>>> +#include "clk-dfll.h"
>>>>> #include "clk-id.h"
>>>>> /*
>>>>> @@ -225,6 +228,7 @@
>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_SET 0x2a8
>>>>> #define CLK_RST_CONTROLLER_RST_DEV_Y_CLR 0x2ac
>>>>> +#define CPU_SOFTRST_CTRL 0x380
>>>>> #define LVL2_CLK_GATE_OVRA 0xf8
>>>>> #define LVL2_CLK_GATE_OVRC 0x3a0
>>>>> @@ -2820,6 +2824,7 @@ static int tegra210_enable_pllu(void)
>>>>> struct tegra_clk_pll_freq_table *fentry;
>>>>> struct tegra_clk_pll pllu;
>>>>> u32 reg;
>>>>> + int ret;
>>>>> for (fentry = pll_u_freq_table; fentry->input_rate;
>>>>> fentry++) {
>>>>> if (fentry->input_rate == pll_ref_freq)
>>>>> @@ -2847,10 +2852,10 @@ static int tegra210_enable_pllu(void)
>>>>> fence_udelay(1, clk_base);
>>>>> reg |= PLL_ENABLE;
>>>>> writel(reg, clk_base + PLLU_BASE);
>>>>> + fence_udelay(1, clk_base);
>>>>> - readl_relaxed_poll_timeout_atomic(clk_base + PLLU_BASE, reg,
>>>>> - reg & PLL_BASE_LOCK, 2, 1000);
>>>>> - if (!(reg & PLL_BASE_LOCK)) {
>>>>> + ret = tegra210_wait_for_mask(&pllu, PLLU_BASE, PLL_BASE_LOCK);
>>>>> + if (ret) {
>>>>> pr_err("Timed out waiting for PLL_U to lock\n");
>>>>> return -ETIMEDOUT;
>>>>> }
>>>>> @@ -3283,6 +3288,103 @@ static void tegra210_disable_cpu_clock(u32
>>>>> cpu)
>>>>> }
>>>>> #ifdef CONFIG_PM_SLEEP
>>>>> +static u32 cpu_softrst_ctx[3];
>>>>> +static struct platform_device *dfll_pdev;
>>>>> +#define car_readl(_base, _off) readl_relaxed(clk_base + (_base) +
>>>>> ((_off) * 4))
>>>>> +#define car_writel(_val, _base, _off) \
>>>>> + writel_relaxed(_val, clk_base + (_base) + ((_off) * 4))
>>>>> +
>>>>> +static int tegra210_clk_suspend(void)
>>>>> +{
>>>>> + unsigned int i;
>>>>> + struct device_node *node;
>>>>> +
>>>>> + tegra_cclkg_burst_policy_save_context();
>>>>> +
>>>>> + if (!dfll_pdev) {
>>>>> + node = of_find_compatible_node(NULL, NULL,
>>>>> + "nvidia,tegra210-dfll");
>>>>> + if (node)
>>>>> + dfll_pdev = of_find_device_by_node(node);
>>>>> +
>>>>> + of_node_put(node);
>>>>> + if (!dfll_pdev)
>>>>> + pr_err("dfll node not found. no suspend for dfll\n");
>>>>> + }
>>>>> +
>>>>> + if (dfll_pdev)
>>>>> + tegra_dfll_suspend(dfll_pdev);
>>>>> +
>>>>> + /* Enable PLLP_OUT_CPU after dfll suspend */
>>>>> + tegra_clk_set_pllp_out_cpu(true);
>>>>> +
>>>>> + tegra_sclk_cclklp_burst_policy_save_context();
>>>>> +
>>>>> + clk_save_context();
>>>>> +
>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>> + cpu_softrst_ctx[i] = car_readl(CPU_SOFTRST_CTRL, i);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void tegra210_clk_resume(void)
>>>>> +{
>>>>> + unsigned int i;
>>>>> + struct clk_hw *parent;
>>>>> + struct clk *clk;
>>>>> +
>>>>> + /*
>>>>> + * clk_restore_context restores clocks as per the clock tree.
>>>>> + *
>>>>> + * dfllCPU_out is first in the clock tree to get restored and it
>>>>> + * involves programming DFLL controller along with restoring
>>>>> CPUG
>>>>> + * clock burst policy.
>>>>> + *
>>>>> + * DFLL programming needs dfll_ref and dfll_soc peripheral
>>>>> clocks
>>>>> + * to be restores which are part ofthe peripheral clocks.
>> ^ white-space
>>
>> Please use spellchecker to avoid typos.
>>
>>>>> + * So, peripheral clocks restore should happen prior to dfll
>>>>> clock
>>>>> + * restore.
>>>>> + */
>>>>> +
>>>>> + tegra_clk_osc_resume(clk_base);
>>>>> + for (i = 0; i < ARRAY_SIZE(cpu_softrst_ctx); i++)
>>>>> + car_writel(cpu_softrst_ctx[i], CPU_SOFTRST_CTRL, i);
>>>>> +
>>>>> + /* restore all plls and peripheral clocks */
>>>>> + tegra210_init_pllu();
>>>>> + clk_restore_context();
>>>>> +
>>>>> + fence_udelay(5, clk_base);
>>>>> +
>>>>> + /* resume SCLK and CPULP clocks */
>>>>> + tegra_sclk_cpulp_burst_policy_restore_context();
>>>>> +
>>>>> + /*
>>>>> + * restore CPUG clocks:
>>>>> + * - enable DFLL in open loop mode
>>>>> + * - switch CPUG to DFLL clock source
>>>>> + * - close DFLL loop
>>>>> + * - sync PLLX state
>>>>> + */
>>>>> + if (dfll_pdev)
>>>>> + tegra_dfll_resume(dfll_pdev, false);
>>>>> +
>>>>> + tegra_cclkg_burst_policy_restore_context();
>>>>> + fence_udelay(2, clk_base);
>>>>> +
>>>>> + if (dfll_pdev)
>>>>> + tegra_dfll_resume(dfll_pdev, true);
>>>>> +
>>>>> + parent =
>>>>> clk_hw_get_parent(__clk_get_hw(clks[TEGRA210_CLK_CCLK_G]));
>>>>> + clk = clks[TEGRA210_CLK_PLL_X];
>>>>> + if (parent != __clk_get_hw(clk))
>>>>> + tegra_clk_sync_state_pll(__clk_get_hw(clk));
>>>>> +
>>>>> + /* Disable PLL_OUT_CPU after DFLL resume */
>>>>> + tegra_clk_set_pllp_out_cpu(false);
>>>>> +}
>>>>> +
>>>>> static void tegra210_cpu_clock_suspend(void)
>>>>> {
>>>>> /* switch coresite to clk_m, save off original source */
>>>>> @@ -3298,6 +3400,11 @@ static void tegra210_cpu_clock_resume(void)
>>>>> }
>>>>> #endif
>>>>> +static struct syscore_ops tegra_clk_syscore_ops = {
>>>>> + .suspend = tegra210_clk_suspend,
>>>>> + .resume = tegra210_clk_resume,
>>>>> +};
>>>>> +
>>>>> static struct tegra_cpu_car_ops tegra210_cpu_car_ops = {
>>>>> .wait_for_reset = tegra210_wait_cpu_in_reset,
>>>>> .disable_clock = tegra210_disable_cpu_clock,
>>>>> @@ -3583,5 +3690,7 @@ static void __init tegra210_clock_init(struct
>>>>> device_node *np)
>>>>> tegra210_mbist_clk_init();
>>>>> tegra_cpu_car_ops = &tegra210_cpu_car_ops;
>>>>> +
>>>>> + register_syscore_ops(&tegra_clk_syscore_ops);
>>>>> }
>>>> Is it really worthwhile to use syscore_ops for suspend/resume given
>>>> that drivers for
>>>> won't resume before the CLK driver anyway? Are there any other options
>>>> for CLK
>>>> suspend/resume?
>>>>
>>>> I'm also not sure whether PM runtime API could be used at all in the
>>>> context of
>>>> syscore_ops ..
>>>>
>>>> Secondly, what about to use generic clk_save_context() /
>>>> clk_restore_context()
>>>> helpers for the suspend-resume? It looks to me that some other
>>>> essential (and proper)
>>>> platform driver (soc/tegra/? PMC?) should suspend-resume the clocks
>>>> using the generic
>>>> CLK Framework API.
>>> Clock resume should happen very early to restore peripheral and cpu
>>> clocks very early than peripheral drivers resume happens.
>> If all peripheral drivers properly requested all of the necessary clocks
>> and CLK driver was a platform driver, then I guess the probe should have
>> been naturally ordered. But that's not very achievable with the
>> currently available infrastructure in the kernel, so I'm not arguing
>> that the clocks should be explicitly resumed before the users.
>>
>>> this patch series uses clk_save_context and clk_restore_context for
>>> corresponding divider, pll, pllout.. save and restore context.
>> Now I see that indeed this API is utilized in this patch, thank you for
>> the clarification.
>>
>>> But as there is dependency on dfll resume and cpu and pllx clocks
>>> restore, couldnt use clk_save_context and clk_restore_context for dfll.
>>>
>>> So implemented recommended dfll resume sequence in main Tegra210 clock
>>> driver along with invoking clk_save_context/clk_restore_context where
>>> all other clocks save/restore happens as per clock tree traversal.
>> Could you please clarify what part of peripherals clocks is required for
>> DFLL's restore? Couldn't DFLL driver be changed to avoid that quirkness
>> and thus to make DFLL driver suspend/resume the clock?
>
> DFLL source ref_clk and soc_clk need to be restored prior to dfll.
>
> I see dfllCPU_out parent to CCLK_G first in the clock tree and
> dfll_ref and dfll_soc peripheral clocks are not resumed by the time
> dfll resume happens first.
>
> ref_clk and soc_clk source is from pll_p and clock tree has these
> registered under pll_p which happens later.
>
> tegra210_clock_init registers in order plls, peripheral clocks,
> super_clk init for cclk_g during clock driver probe and dfll probe and
> register happens later.
>
One more thing, CLDVFS peripheral clock enable is also needed to be
enabled to program DFLL Controller and all peripheral clock context is
restored only after their PLL sources are restored.
DFLL restore involves dfll source clock resume along with CLDVFS
periheral clock enable and reset
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox