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@xxxxxxxxx>
> Tested-by: Nicolas Chauvet <kwizart@xxxxxxxxx>
> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: PGP signature
