diff mbox

[V5,06/14] soc: tegra: pmc: Fix checking of valid partitions

Message ID 1453998832-27383-7-git-send-email-jonathanh@nvidia.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Jon Hunter Jan. 28, 2016, 4:33 p.m. UTC
The tegra power partitions are referenced by a numerical ID which are
the same values programmed into the PMC registers for controlling the
partition. For a given device, the valid partition IDs may not be
contiguous and so simply checking that an ID is not greater than the
maximum ID supported may not mean it is valid. Fix this by adding a
bitmap for representing the valid partitions of a device and add a
helper function will test if the partition is valid.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 21 +++++++++++++++++----
 include/soc/tegra/pmc.h |  1 +
 2 files changed, 18 insertions(+), 4 deletions(-)

Comments

Mathieu Poirier Jan. 29, 2016, 5:08 p.m. UTC | #1
On 28 January 2016 at 09:33, Jon Hunter <jonathanh@nvidia.com> wrote:
> The tegra power partitions are referenced by a numerical ID which are
> the same values programmed into the PMC registers for controlling the
> partition. For a given device, the valid partition IDs may not be
> contiguous and so simply checking that an ID is not greater than the
> maximum ID supported may not mean it is valid. Fix this by adding a
> bitmap for representing the valid partitions of a device and add a
> helper function will test if the partition is valid.
>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 21 +++++++++++++++++----
>  include/soc/tegra/pmc.h |  1 +
>  2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 35ee60fd17be..032dd5c17130 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -132,6 +132,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_valid: Bitmap of valid power gates
>   * @powergates_lock: mutex for power gate register access
>   */
>  struct tegra_pmc {
> @@ -156,6 +157,7 @@ struct tegra_pmc {
>         bool cpu_pwr_good_en;
>         u32 lp0_vec_phys;
>         u32 lp0_vec_size;
> +       DECLARE_BITMAP(powergates_valid, TEGRA_POWERGATE_MAX);
>
>         struct mutex powergates_lock;
>  };
> @@ -180,6 +182,11 @@ static inline bool tegra_powergate_state(int id)
>         return (tegra_pmc_readl(PWRGATE_STATUS) & BIT(id)) != 0;
>  }
>
> +static inline bool tegra_powergate_is_valid(int id)
> +{
> +       return test_bit(id, pmc->powergates_valid);
> +}
> +
>  /**
>   * tegra_powergate_set() - set the state of a partition
>   * @id: partition ID
> @@ -213,7 +220,7 @@ static int tegra_powergate_set(unsigned int id, bool new_state)
>   */
>  int tegra_powergate_power_on(unsigned int id)
>  {
> -       if (!pmc->soc || id >= pmc->soc->num_powergates)
> +       if (!tegra_powergate_is_valid(id))
>                 return -EINVAL;

The "!pmc-soc" condition is no longer needed?  If so the changelog
should reflect that or a new patch that deals with just that should be
etched.  The same comment applies for the rest of the patch.

>
>         return tegra_powergate_set(id, true);
> @@ -225,7 +232,7 @@ int tegra_powergate_power_on(unsigned int id)
>   */
>  int tegra_powergate_power_off(unsigned int id)
>  {
> -       if (!pmc->soc || id >= pmc->soc->num_powergates)
> +       if (!tegra_powergate_is_valid(id))
>                 return -EINVAL;
>
>         return tegra_powergate_set(id, false);
> @@ -240,7 +247,7 @@ int tegra_powergate_is_powered(unsigned int id)
>  {
>         int status;
>
> -       if (!pmc->soc || id >= pmc->soc->num_powergates)
> +       if (!tegra_powergate_is_valid(id))
>                 return -EINVAL;
>
>         mutex_lock(&pmc->powergates_lock);
> @@ -258,7 +265,7 @@ int tegra_powergate_remove_clamping(unsigned int id)
>  {
>         u32 mask;
>
> -       if (!pmc->soc || id >= pmc->soc->num_powergates)
> +       if (!tegra_powergate_is_valid(id))
>                 return -EINVAL;
>
>         mutex_lock(&pmc->powergates_lock);
> @@ -1118,6 +1125,7 @@ static int __init tegra_pmc_early_init(void)
>         const struct of_device_id *match;
>         struct device_node *np;
>         struct resource regs;
> +       unsigned int i;
>         bool invert;
>         u32 value;
>
> @@ -1167,6 +1175,11 @@ static int __init tegra_pmc_early_init(void)
>                 return -ENXIO;
>         }
>
> +       /* Create a bit-mask of the valid partitions */
> +       for (i = 0; i < pmc->soc->num_powergates; i++)
> +               if (pmc->soc->powergates[i])
> +                       set_bit(i, pmc->powergates_valid);
> +
>         mutex_init(&pmc->powergates_lock);
>
>         /*
> diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
> index 07e332dd44fb..e9e53473a63e 100644
> --- a/include/soc/tegra/pmc.h
> +++ b/include/soc/tegra/pmc.h
> @@ -72,6 +72,7 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid);
>  #define TEGRA_POWERGATE_AUD    27
>  #define TEGRA_POWERGATE_DFD    28
>  #define TEGRA_POWERGATE_VE2    29
> +#define TEGRA_POWERGATE_MAX    TEGRA_POWERGATE_VE2
>
>  #define TEGRA_POWERGATE_3D0    TEGRA_POWERGATE_3D
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter Feb. 1, 2016, 1:45 p.m. UTC | #2
On 29/01/16 17:08, Mathieu Poirier wrote:
> On 28 January 2016 at 09:33, Jon Hunter <jonathanh@nvidia.com> wrote:
>> The tegra power partitions are referenced by a numerical ID which are
>> the same values programmed into the PMC registers for controlling the
>> partition. For a given device, the valid partition IDs may not be
>> contiguous and so simply checking that an ID is not greater than the
>> maximum ID supported may not mean it is valid. Fix this by adding a
>> bitmap for representing the valid partitions of a device and add a
>> helper function will test if the partition is valid.
>>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/soc/tegra/pmc.c | 21 +++++++++++++++++----
>>  include/soc/tegra/pmc.h |  1 +
>>  2 files changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index 35ee60fd17be..032dd5c17130 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -132,6 +132,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_valid: Bitmap of valid power gates
>>   * @powergates_lock: mutex for power gate register access
>>   */
>>  struct tegra_pmc {
>> @@ -156,6 +157,7 @@ struct tegra_pmc {
>>         bool cpu_pwr_good_en;
>>         u32 lp0_vec_phys;
>>         u32 lp0_vec_size;
>> +       DECLARE_BITMAP(powergates_valid, TEGRA_POWERGATE_MAX);
>>
>>         struct mutex powergates_lock;
>>  };
>> @@ -180,6 +182,11 @@ static inline bool tegra_powergate_state(int id)
>>         return (tegra_pmc_readl(PWRGATE_STATUS) & BIT(id)) != 0;
>>  }
>>
>> +static inline bool tegra_powergate_is_valid(int id)
>> +{
>> +       return test_bit(id, pmc->powergates_valid);
>> +}
>> +
>>  /**
>>   * tegra_powergate_set() - set the state of a partition
>>   * @id: partition ID
>> @@ -213,7 +220,7 @@ static int tegra_powergate_set(unsigned int id, bool new_state)
>>   */
>>  int tegra_powergate_power_on(unsigned int id)
>>  {
>> -       if (!pmc->soc || id >= pmc->soc->num_powergates)
>> +       if (!tegra_powergate_is_valid(id))
>>                 return -EINVAL;
> 
> The "!pmc-soc" condition is no longer needed?  If so the changelog
> should reflect that or a new patch that deals with just that should be
> etched.  The same comment applies for the rest of the patch.

Yes it is not necessary. I will update the changelog.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 35ee60fd17be..032dd5c17130 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -132,6 +132,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_valid: Bitmap of valid power gates
  * @powergates_lock: mutex for power gate register access
  */
 struct tegra_pmc {
@@ -156,6 +157,7 @@  struct tegra_pmc {
 	bool cpu_pwr_good_en;
 	u32 lp0_vec_phys;
 	u32 lp0_vec_size;
+	DECLARE_BITMAP(powergates_valid, TEGRA_POWERGATE_MAX);
 
 	struct mutex powergates_lock;
 };
@@ -180,6 +182,11 @@  static inline bool tegra_powergate_state(int id)
 	return (tegra_pmc_readl(PWRGATE_STATUS) & BIT(id)) != 0;
 }
 
+static inline bool tegra_powergate_is_valid(int id)
+{
+	return test_bit(id, pmc->powergates_valid);
+}
+
 /**
  * tegra_powergate_set() - set the state of a partition
  * @id: partition ID
@@ -213,7 +220,7 @@  static int tegra_powergate_set(unsigned int id, bool new_state)
  */
 int tegra_powergate_power_on(unsigned int id)
 {
-	if (!pmc->soc || id >= pmc->soc->num_powergates)
+	if (!tegra_powergate_is_valid(id))
 		return -EINVAL;
 
 	return tegra_powergate_set(id, true);
@@ -225,7 +232,7 @@  int tegra_powergate_power_on(unsigned int id)
  */
 int tegra_powergate_power_off(unsigned int id)
 {
-	if (!pmc->soc || id >= pmc->soc->num_powergates)
+	if (!tegra_powergate_is_valid(id))
 		return -EINVAL;
 
 	return tegra_powergate_set(id, false);
@@ -240,7 +247,7 @@  int tegra_powergate_is_powered(unsigned int id)
 {
 	int status;
 
-	if (!pmc->soc || id >= pmc->soc->num_powergates)
+	if (!tegra_powergate_is_valid(id))
 		return -EINVAL;
 
 	mutex_lock(&pmc->powergates_lock);
@@ -258,7 +265,7 @@  int tegra_powergate_remove_clamping(unsigned int id)
 {
 	u32 mask;
 
-	if (!pmc->soc || id >= pmc->soc->num_powergates)
+	if (!tegra_powergate_is_valid(id))
 		return -EINVAL;
 
 	mutex_lock(&pmc->powergates_lock);
@@ -1118,6 +1125,7 @@  static int __init tegra_pmc_early_init(void)
 	const struct of_device_id *match;
 	struct device_node *np;
 	struct resource regs;
+	unsigned int i;
 	bool invert;
 	u32 value;
 
@@ -1167,6 +1175,11 @@  static int __init tegra_pmc_early_init(void)
 		return -ENXIO;
 	}
 
+	/* Create a bit-mask of the valid partitions */
+	for (i = 0; i < pmc->soc->num_powergates; i++)
+		if (pmc->soc->powergates[i])
+			set_bit(i, pmc->powergates_valid);
+
 	mutex_init(&pmc->powergates_lock);
 
 	/*
diff --git a/include/soc/tegra/pmc.h b/include/soc/tegra/pmc.h
index 07e332dd44fb..e9e53473a63e 100644
--- a/include/soc/tegra/pmc.h
+++ b/include/soc/tegra/pmc.h
@@ -72,6 +72,7 @@  int tegra_pmc_cpu_remove_clamping(unsigned int cpuid);
 #define TEGRA_POWERGATE_AUD	27
 #define TEGRA_POWERGATE_DFD	28
 #define TEGRA_POWERGATE_VE2	29
+#define TEGRA_POWERGATE_MAX	TEGRA_POWERGATE_VE2
 
 #define TEGRA_POWERGATE_3D0	TEGRA_POWERGATE_3D