Re: [PATCH V7 3/4] soc: tegra: pmc: Add generic PM domain support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 4 March 2016 at 13:23, Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
> Adds generic PM support to the PMC driver where the PM domains are
> populated from device-tree and the PM domain consumer devices are
> bound to their relevant PM domains via device-tree as well.
>
> Update the tegra_powergate_sequence_power_up() API so that internally
> it calls the same tegra_powergate_xxx functions that are used by the
> tegra generic power domain code for consistency.
>
> To ensure that the Tegra power domains (a.k.a powergates) cannot be
> controlled via both the legacy tegra_powergate_xxx functions as well
> as the generic PM domain framework, add a bit map for available
> powergates that can be controlled via the legacy powergate functions.
>
> Move the majority of the tegra_powergate_remove_clamping() function
> to a sub-function, so that this can be used by both the legacy and
> generic power domain code.
>
> Currently, the power domains are not removed once added because this
> is not yet supported by the PM domains framework. An error message
> will be displayed if we are unable to instantiate a power domain.
>
> This is based upon work by Thierry Reding <treding@xxxxxxxxxx>
> and Vince Hsu <vinceh@xxxxxxxxxx>.
>
> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> ---
>  drivers/soc/tegra/pmc.c | 485 ++++++++++++++++++++++++++++++++++++++++++------
>  include/soc/tegra/pmc.h |   1 +
>  2 files changed, 425 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 08966c26d65c..bb173456bbff 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -31,10 +31,13 @@
>  #include <linux/iopoll.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
>  #include <linux/reboot.h>
>  #include <linux/reset.h>
>  #include <linux/seq_file.h>
> +#include <linux/slab.h>
>  #include <linux/spinlock.h>
>
>  #include <soc/tegra/common.h>
> @@ -102,6 +105,16 @@
>
>  #define GPU_RG_CNTRL                   0x2d4
>
> +struct tegra_powergate {
> +       struct generic_pm_domain genpd;
> +       struct tegra_pmc *pmc;
> +       unsigned int id;
> +       struct clk **clks;
> +       unsigned int num_clks;
> +       struct reset_control **resets;
> +       unsigned int num_resets;
> +};
> +
>  struct tegra_pmc_soc {
>         unsigned int num_powergates;
>         const char *const *powergates;
> @@ -132,6 +145,7 @@ struct tegra_pmc_soc {
>   * @cpu_pwr_good_en: CPU power good signal is enabled
>   * @lp0_vec_phys: physical base address of the LP0 warm boot code
>   * @lp0_vec_size: size of the LP0 warm boot code
> + * @powergates_available: Bitmap of available power gates
>   * @powergates_lock: mutex for power gate register access
>   */
>  struct tegra_pmc {
> @@ -156,6 +170,7 @@ struct tegra_pmc {
>         bool cpu_pwr_good_en;
>         u32 lp0_vec_phys;
>         u32 lp0_vec_size;
> +       DECLARE_BITMAP(powergates_available, TEGRA_POWERGATE_MAX);
>
>         struct mutex powergates_lock;
>  };
> @@ -165,6 +180,12 @@ static struct tegra_pmc *pmc = &(struct tegra_pmc) {
>         .suspend_mode = TEGRA_SUSPEND_NONE,
>  };
>
> +static inline struct tegra_powergate *
> +to_powergate(struct generic_pm_domain *domain)
> +{
> +       return container_of(domain, struct tegra_powergate, genpd);
> +}
> +
>  static u32 tegra_pmc_readl(unsigned long offset)
>  {
>         return readl(pmc->base + offset);
> @@ -188,6 +209,31 @@ static inline bool tegra_powergate_is_valid(int id)
>         return (pmc->soc && pmc->soc->powergates[id]);
>  }
>
> +static inline bool tegra_powergate_is_available(int id)
> +{
> +       return test_bit(id, pmc->powergates_available);
> +}
> +
> +static int tegra_powergate_lookup(struct tegra_pmc *pmc, const char *name)
> +{
> +       unsigned int i;
> +
> +       if (!pmc || !pmc->soc || !name)
> +               return -EINVAL;
> +
> +       for (i = 0; i < pmc->soc->num_powergates; i++) {
> +               if (!tegra_powergate_is_valid(i))
> +                       continue;
> +
> +               if (!strcmp(name, pmc->soc->powergates[i]))
> +                       return i;

Instead of having this name based lookup, why not provide the id using
DT instead?

In other words use of_genpd_add_provider_onecell() when adding the OF
genpd provider. In that way I think you will get a bit simplier code
dealing with the error path when initialzing the genpds, won't you?

Anyway, I guess it's more a matter of taste, so feel free to keep as is.

[...]

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux