| Message ID | 20201104234427.26477-8-digetx@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Introduce core voltage scaling for NVIDIA Tegra20/30 SoCs | expand |
On Thu, Nov 05, 2020 at 02:44:04AM +0300, Dmitry Osipenko wrote: > Introduce sync state API that will be used by Tegra device drivers. This > new API is primarily needed for syncing state of SoC devices that are left > ON after bootloader or permanently enabled. All these devices belong to a > shared CORE voltage domain, and thus, we needed to bring all the devices > into expected state before the voltage scaling could be performed. > > All drivers of DVFS-critical devices shall sync theirs the state before > Tegra's voltage regulator coupler will be allowed to perform a system-wide > voltage scaling. > > Tested-by: Peter Geis <pgwipeout@gmail.com> > Tested-by: Nicolas Chauvet <kwizart@gmail.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/soc/tegra/common.c | 152 ++++++++++++++++++++++++++++++++++++- > include/soc/tegra/common.h | 22 ++++++ > 2 files changed, 170 insertions(+), 4 deletions(-) > > diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c > index 3dc54f59cafe..f9b2b6f57887 100644 > --- a/drivers/soc/tegra/common.c > +++ b/drivers/soc/tegra/common.c > @@ -3,13 +3,52 @@ > * Copyright (C) 2014 NVIDIA CORPORATION. All rights reserved. > */ > > +#define dev_fmt(fmt) "%s: " fmt, __func__ > +#define pr_fmt(fmt) "%s: " fmt, __func__ > + > +#include <linux/export.h> > +#include <linux/init.h> > +#include <linux/mutex.h> > #include <linux/of.h> > +#include <linux/of_device.h> > > #include <soc/tegra/common.h> > > +#define terga_soc_for_each_device(__dev) \ tegra_soc_for_each_device > + for ((__dev) = tegra_soc_devices; (__dev) && (__dev)->compatible; \ > + (__dev)++) > + > +struct tegra_soc_device { > + const char *compatible; > + const bool dvfs_critical; > + unsigned int sync_count; > +}; > + > +static DEFINE_MUTEX(tegra_soc_lock); > +static struct tegra_soc_device *tegra_soc_devices; > + > +/* > + * DVFS-critical devices are either active at a boot time or permanently > + * active, like EMC for example. System-wide DVFS should be deferred until > + * drivers of the critical devices synced theirs state. > + */ > + > +static struct tegra_soc_device tegra20_soc_devices[] = { > + { .compatible = "nvidia,tegra20-dc", .dvfs_critical = true, }, > + { .compatible = "nvidia,tegra20-emc", .dvfs_critical = true, }, > + { } > +}; > + > +static struct tegra_soc_device tegra30_soc_devices[] = { > + { .compatible = "nvidia,tegra30-dc", .dvfs_critical = true, }, > + { .compatible = "nvidia,tegra30-emc", .dvfs_critical = true, }, > + { .compatible = "nvidia,tegra30-pwm", .dvfs_critical = true, }, > + { } > +}; > + > static const struct of_device_id tegra_machine_match[] = { > - { .compatible = "nvidia,tegra20", }, > - { .compatible = "nvidia,tegra30", }, > + { .compatible = "nvidia,tegra20", .data = tegra20_soc_devices, }, > + { .compatible = "nvidia,tegra30", .data = tegra30_soc_devices, }, > { .compatible = "nvidia,tegra114", }, > { .compatible = "nvidia,tegra124", }, > { .compatible = "nvidia,tegra132", }, > @@ -17,7 +56,7 @@ static const struct of_device_id tegra_machine_match[] = { > { } > }; > > -bool soc_is_tegra(void) > +static const struct of_device_id *tegra_soc_of_match(void) > { > const struct of_device_id *match; > struct device_node *root; > @@ -29,5 +68,110 @@ bool soc_is_tegra(void) > match = of_match_node(tegra_machine_match, root); > of_node_put(root); > > - return match != NULL; > + return match; > +} > + > +bool soc_is_tegra(void) > +{ > + return tegra_soc_of_match() != NULL; > +} > + > +void tegra_soc_device_sync_state(struct device *dev) > +{ > + struct tegra_soc_device *soc_dev; > + > + mutex_lock(&tegra_soc_lock); > + terga_soc_for_each_device(soc_dev) { tegra_soc_for_each_device > + if (!of_device_is_compatible(dev->of_node, soc_dev->compatible)) > + continue; > + > + if (!soc_dev->sync_count) { > + dev_err(dev, "already synced\n"); > + break; > + } > + > + /* > + * All DVFS-capable devices should have the CORE regulator > + * phandle. Older device-trees don't have it, hence state > + * won't be synced for the older DTBs, allowing them to work > + * properly. > + */ > + if (soc_dev->dvfs_critical && > + !device_property_present(dev, "core-supply")) { > + dev_dbg(dev, "doesn't have core supply\n"); > + break; > + } > + > + soc_dev->sync_count--; > + dev_dbg(dev, "sync_count=%u\n", soc_dev->sync_count); > + break; > + } > + mutex_unlock(&tegra_soc_lock); > +} > +EXPORT_SYMBOL_GPL(tegra_soc_device_sync_state); > + > +bool tegra_soc_dvfs_state_synced(void) > +{ > + struct tegra_soc_device *soc_dev; > + bool synced_state = true; > + > + /* > + * CORE voltage scaling is limited until drivers of the critical > + * devices synced theirs state. > + */ > + mutex_lock(&tegra_soc_lock); > + terga_soc_for_each_device(soc_dev) { tegra_soc_for_each_device I wonder if you copy/pasted this or if you got really lucky to mistype this all three times. > + if (!soc_dev->sync_count || !soc_dev->dvfs_critical) > + continue; > + > + pr_debug_ratelimited("%s: sync_count=%u\n", > + soc_dev->compatible, soc_dev->sync_count); > + > + synced_state = false; > + break; > + } > + mutex_unlock(&tegra_soc_lock); > + > + return synced_state; > +} > + > +static int __init tegra_soc_devices_init(void) > +{ > + struct device_node *np, *prev_np = NULL; > + struct tegra_soc_device *soc_dev; > + const struct of_device_id *match; > + > + if (!soc_is_tegra()) > + return 0; > + > + match = tegra_soc_of_match(); > + tegra_soc_devices = (void *)match->data; > + > + /* > + * If device node is disabled in a device-tree, then we shouldn't > + * care about this device. Even if device is active during boot, > + * its clock will be disabled by CCF as unused. > + */ > + terga_soc_for_each_device(soc_dev) { > + do { > + /* > + * Devices like display controller have multiple > + * instances with the same compatible. Hence we need > + * to walk up the whole tree in order to account those > + * multiple instances. > + */ > + np = of_find_compatible_node(prev_np, NULL, > + soc_dev->compatible); > + of_node_put(prev_np); > + prev_np = np; > + > + if (of_device_is_available(np)) { > + pr_debug("added %s\n", soc_dev->compatible); > + soc_dev->sync_count++; > + } > + } while (np); Maybe use for_each_compatible_node() for that inside loop? > + } > + > + return 0; > } > +postcore_initcall_sync(tegra_soc_devices_init); This is unfortunate. I recall having this discussion multiple times and one idea that has been floating around for a while was to let a driver bind against the top-level "bus" node. That has the advantage that it both anchors the code somewhere, so we don't have to play this game of checking for the SoC with soc_is_tegra(), and it properly orders this with respect to the child devices, so we wouldn't have to make this a postcore_initcall. Might be worth looking at that again, but for now this seems okay. Thierry
10.11.2020 23:47, Thierry Reding пишет: ... > tegra_soc_for_each_device > > I wonder if you copy/pasted this or if you got really lucky to mistype > this all three times. Copied of course :) I added a special spell checking rule for this typo, but it does help reliably. ... >> + terga_soc_for_each_device(soc_dev) { >> + do { >> + /* >> + * Devices like display controller have multiple >> + * instances with the same compatible. Hence we need >> + * to walk up the whole tree in order to account those >> + * multiple instances. >> + */ >> + np = of_find_compatible_node(prev_np, NULL, >> + soc_dev->compatible); >> + of_node_put(prev_np); >> + prev_np = np; >> + >> + if (of_device_is_available(np)) { >> + pr_debug("added %s\n", soc_dev->compatible); >> + soc_dev->sync_count++; >> + } >> + } while (np); > > Maybe use for_each_compatible_node() for that inside loop? Good point! I think there is actually an of_node_put() bug in current variant, which for_each_compatible_node() would safe from. >> + } >> + >> + return 0; >> } >> +postcore_initcall_sync(tegra_soc_devices_init); > > This is unfortunate. I recall having this discussion multiple times and > one idea that has been floating around for a while was to let a driver > bind against the top-level "bus" node. That has the advantage that it > both anchors the code somewhere, so we don't have to play this game of > checking for the SoC with soc_is_tegra(), and it properly orders this > with respect to the child devices, so we wouldn't have to make this a > postcore_initcall. > > Might be worth looking at that again, but for now this seems okay. Thanks
11.11.2020 00:22, Dmitry Osipenko пишет: > I added a special spell checking rule for this typo, but it does help > reliably. does *not*
diff --git a/drivers/soc/tegra/common.c b/drivers/soc/tegra/common.c index 3dc54f59cafe..f9b2b6f57887 100644 --- a/drivers/soc/tegra/common.c +++ b/drivers/soc/tegra/common.c @@ -3,13 +3,52 @@ * Copyright (C) 2014 NVIDIA CORPORATION. All rights reserved. */ +#define dev_fmt(fmt) "%s: " fmt, __func__ +#define pr_fmt(fmt) "%s: " fmt, __func__ + +#include <linux/export.h> +#include <linux/init.h> +#include <linux/mutex.h> #include <linux/of.h> +#include <linux/of_device.h> #include <soc/tegra/common.h> +#define terga_soc_for_each_device(__dev) \ + for ((__dev) = tegra_soc_devices; (__dev) && (__dev)->compatible; \ + (__dev)++) + +struct tegra_soc_device { + const char *compatible; + const bool dvfs_critical; + unsigned int sync_count; +}; + +static DEFINE_MUTEX(tegra_soc_lock); +static struct tegra_soc_device *tegra_soc_devices; + +/* + * DVFS-critical devices are either active at a boot time or permanently + * active, like EMC for example. System-wide DVFS should be deferred until + * drivers of the critical devices synced theirs state. + */ + +static struct tegra_soc_device tegra20_soc_devices[] = { + { .compatible = "nvidia,tegra20-dc", .dvfs_critical = true, }, + { .compatible = "nvidia,tegra20-emc", .dvfs_critical = true, }, + { } +}; + +static struct tegra_soc_device tegra30_soc_devices[] = { + { .compatible = "nvidia,tegra30-dc", .dvfs_critical = true, }, + { .compatible = "nvidia,tegra30-emc", .dvfs_critical = true, }, + { .compatible = "nvidia,tegra30-pwm", .dvfs_critical = true, }, + { } +}; + static const struct of_device_id tegra_machine_match[] = { - { .compatible = "nvidia,tegra20", }, - { .compatible = "nvidia,tegra30", }, + { .compatible = "nvidia,tegra20", .data = tegra20_soc_devices, }, + { .compatible = "nvidia,tegra30", .data = tegra30_soc_devices, }, { .compatible = "nvidia,tegra114", }, { .compatible = "nvidia,tegra124", }, { .compatible = "nvidia,tegra132", }, @@ -17,7 +56,7 @@ static const struct of_device_id tegra_machine_match[] = { { } }; -bool soc_is_tegra(void) +static const struct of_device_id *tegra_soc_of_match(void) { const struct of_device_id *match; struct device_node *root; @@ -29,5 +68,110 @@ bool soc_is_tegra(void) match = of_match_node(tegra_machine_match, root); of_node_put(root); - return match != NULL; + return match; +} + +bool soc_is_tegra(void) +{ + return tegra_soc_of_match() != NULL; +} + +void tegra_soc_device_sync_state(struct device *dev) +{ + struct tegra_soc_device *soc_dev; + + mutex_lock(&tegra_soc_lock); + terga_soc_for_each_device(soc_dev) { + if (!of_device_is_compatible(dev->of_node, soc_dev->compatible)) + continue; + + if (!soc_dev->sync_count) { + dev_err(dev, "already synced\n"); + break; + } + + /* + * All DVFS-capable devices should have the CORE regulator + * phandle. Older device-trees don't have it, hence state + * won't be synced for the older DTBs, allowing them to work + * properly. + */ + if (soc_dev->dvfs_critical && + !device_property_present(dev, "core-supply")) { + dev_dbg(dev, "doesn't have core supply\n"); + break; + } + + soc_dev->sync_count--; + dev_dbg(dev, "sync_count=%u\n", soc_dev->sync_count); + break; + } + mutex_unlock(&tegra_soc_lock); +} +EXPORT_SYMBOL_GPL(tegra_soc_device_sync_state); + +bool tegra_soc_dvfs_state_synced(void) +{ + struct tegra_soc_device *soc_dev; + bool synced_state = true; + + /* + * CORE voltage scaling is limited until drivers of the critical + * devices synced theirs state. + */ + mutex_lock(&tegra_soc_lock); + terga_soc_for_each_device(soc_dev) { + if (!soc_dev->sync_count || !soc_dev->dvfs_critical) + continue; + + pr_debug_ratelimited("%s: sync_count=%u\n", + soc_dev->compatible, soc_dev->sync_count); + + synced_state = false; + break; + } + mutex_unlock(&tegra_soc_lock); + + return synced_state; +} + +static int __init tegra_soc_devices_init(void) +{ + struct device_node *np, *prev_np = NULL; + struct tegra_soc_device *soc_dev; + const struct of_device_id *match; + + if (!soc_is_tegra()) + return 0; + + match = tegra_soc_of_match(); + tegra_soc_devices = (void *)match->data; + + /* + * If device node is disabled in a device-tree, then we shouldn't + * care about this device. Even if device is active during boot, + * its clock will be disabled by CCF as unused. + */ + terga_soc_for_each_device(soc_dev) { + do { + /* + * Devices like display controller have multiple + * instances with the same compatible. Hence we need + * to walk up the whole tree in order to account those + * multiple instances. + */ + np = of_find_compatible_node(prev_np, NULL, + soc_dev->compatible); + of_node_put(prev_np); + prev_np = np; + + if (of_device_is_available(np)) { + pr_debug("added %s\n", soc_dev->compatible); + soc_dev->sync_count++; + } + } while (np); + } + + return 0; } +postcore_initcall_sync(tegra_soc_devices_init); diff --git a/include/soc/tegra/common.h b/include/soc/tegra/common.h index 98027a76ce3d..d3ddb96d0fe2 100644 --- a/include/soc/tegra/common.h +++ b/include/soc/tegra/common.h @@ -6,6 +6,28 @@ #ifndef __SOC_TEGRA_COMMON_H__ #define __SOC_TEGRA_COMMON_H__ +#include <linux/types.h> + +struct device; + +#ifdef CONFIG_ARCH_TEGRA bool soc_is_tegra(void); +void tegra_soc_device_sync_state(struct device *dev); +bool tegra_soc_dvfs_state_synced(void); +#else +static inline bool soc_is_tegra(void) +{ + return false; +} + +static inline void tegra_soc_device_sync_state(struct device *dev) +{ +} + +static inline tegra_soc_dvfs_state_synced(void) +{ + return false; +} +#endif #endif /* __SOC_TEGRA_COMMON_H__ */