Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Thierry Reding]

On Wed, Jul 19, 2017 at 05:59:08PM +0200, Philipp Zabel wrote:
> From: Vivek Gautam 
> 
> Make use of of_reset_control_array_get_exclusive() to manage
> an array of reset controllers available with the device.
> 
> Cc: Jon Hunter 
> Cc: Thierry Reding 
> Signed-off-by: Vivek Gautam 
> [p.za...@pengutronix.de: switch to hidden reset control array]
> Signed-off-by: Philipp Zabel 
> ---
> No changes since v6.
> ---
>  drivers/soc/tegra/pmc.c | 82 
> -
>  1 file changed, 20 insertions(+), 62 deletions(-)

I've applied this with the static inline change that Jon suggested.

Thanks,
Thierry


signature.asc
Description: PGP signature

Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Philipp Zabel]

On Fri, Oct 20, 2017 at 04:51:24PM +0100, Jon Hunter wrote:
> Hi Philipp,
> 
> On 19/10/17 16:17, Philipp Zabel wrote:
> > Hi Jon, Thierry,
> > 
> > On Wed, 2017-07-19 at 17:59 +0200, Philipp Zabel wrote:
> >> From: Vivek Gautam 
> >>
> >> Make use of of_reset_control_array_get_exclusive() to manage
> >> an array of reset controllers available with the device.
> >>
> >> Cc: Jon Hunter 
> >> Cc: Thierry Reding 
> >> Signed-off-by: Vivek Gautam 
> >> [p.za...@pengutronix.de: switch to hidden reset control array]
> >> Signed-off-by: Philipp Zabel 
> > 
> > will you pick this up now that the prerequisite patch 1 is contained in
> > master?
> > Please let me know if there are any issues with this patch.
> > 
> > regards
> > Philipp
> > 
> >> ---
> >> No changes since v6.
> >> ---
> >>  drivers/soc/tegra/pmc.c | 82 
> >> -
> >>  1 file changed, 20 insertions(+), 62 deletions(-)
> >>
> >> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> >> index e233dd5dcab3d..749b218147a19 100644
> >> --- a/drivers/soc/tegra/pmc.c
> >> +++ b/drivers/soc/tegra/pmc.c
> >> @@ -124,8 +124,7 @@ struct tegra_powergate {
> >>unsigned int id;
> >>struct clk **clks;
> >>unsigned int num_clks;
> >> -  struct reset_control **resets;
> >> -  unsigned int num_resets;
> >> +  struct reset_control *reset;
> >>  };
> >>  
> >>  struct tegra_io_pad_soc {
> >> @@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct 
> >> tegra_powergate *pg)
> >>return err;
> >>  }
> >>  
> >> -static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
> >> +static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
> >>  {
> >> -  unsigned int i;
> >> -  int err;
> >> -
> >> -  for (i = 0; i < pg->num_resets; i++) {
> >> -  err = reset_control_assert(pg->resets[i]);
> >> -  if (err)
> >> -  return err;
> >> -  }
> >> -
> >> -  return 0;
> >> +  return reset_control_assert(pg->reset);
> >>  }
> >>  
> >> -static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
> >> +static inline int tegra_powergate_reset_deassert(struct tegra_powergate 
> >> *pg)
> >>  {
> >> -  unsigned int i;
> >> -  int err;
> >> -
> >> -  for (i = 0; i < pg->num_resets; i++) {
> >> -  err = reset_control_deassert(pg->resets[i]);
> >> -  if (err)
> >> -  return err;
> >> -  }
> >> -
> >> -  return 0;
> >> +  return reset_control_deassert(pg->reset);
> >>  }
> 
> Nit-pick ... I think we should just get rid of this inline functions now
> and just call reset_control_assert/deassert() where these are used.
> 
[...]
> 
> Otherwise ...
> 
> Acked-by: Jon Hunter 
> 
> Thierry has just sent out his pull requests for v4.15 so we may have
> missed this release :-(

Thanks, I'll drop the inline functions and send a new version, then.

regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Jon Hunter]

Hi Philipp,

On 19/10/17 16:17, Philipp Zabel wrote:
> Hi Jon, Thierry,
> 
> On Wed, 2017-07-19 at 17:59 +0200, Philipp Zabel wrote:
>> From: Vivek Gautam 
>>
>> Make use of of_reset_control_array_get_exclusive() to manage
>> an array of reset controllers available with the device.
>>
>> Cc: Jon Hunter 
>> Cc: Thierry Reding 
>> Signed-off-by: Vivek Gautam 
>> [p.za...@pengutronix.de: switch to hidden reset control array]
>> Signed-off-by: Philipp Zabel 
> 
> will you pick this up now that the prerequisite patch 1 is contained in
> master?
> Please let me know if there are any issues with this patch.
> 
> regards
> Philipp
> 
>> ---
>> No changes since v6.
>> ---
>>  drivers/soc/tegra/pmc.c | 82 
>> -
>>  1 file changed, 20 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
>> index e233dd5dcab3d..749b218147a19 100644
>> --- a/drivers/soc/tegra/pmc.c
>> +++ b/drivers/soc/tegra/pmc.c
>> @@ -124,8 +124,7 @@ struct tegra_powergate {
>>  unsigned int id;
>>  struct clk **clks;
>>  unsigned int num_clks;
>> -struct reset_control **resets;
>> -unsigned int num_resets;
>> +struct reset_control *reset;
>>  };
>>  
>>  struct tegra_io_pad_soc {
>> @@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct 
>> tegra_powergate *pg)
>>  return err;
>>  }
>>  
>> -static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
>> +static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
>>  {
>> -unsigned int i;
>> -int err;
>> -
>> -for (i = 0; i < pg->num_resets; i++) {
>> -err = reset_control_assert(pg->resets[i]);
>> -if (err)
>> -return err;
>> -}
>> -
>> -return 0;
>> +return reset_control_assert(pg->reset);
>>  }
>>  
>> -static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
>> +static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
>>  {
>> -unsigned int i;
>> -int err;
>> -
>> -for (i = 0; i < pg->num_resets; i++) {
>> -err = reset_control_deassert(pg->resets[i]);
>> -if (err)
>> -return err;
>> -}
>> -
>> -return 0;
>> +return reset_control_deassert(pg->reset);
>>  }

Nit-pick ... I think we should just get rid of this inline functions now
and just call reset_control_assert/deassert() where these are used.

>>  static int tegra_powergate_power_up(struct tegra_powergate *pg,
>> @@ -566,8 +547,7 @@ int tegra_powergate_sequence_power_up(unsigned int id, 
>> struct clk *clk,
>>  pg.id = id;
>>  pg.clks = 
>>  pg.num_clks = 1;
>> -pg.resets = 
>> -pg.num_resets = 1;
>> +pg.reset = IS_ERR(rst) ? NULL : rst;
>>  
>>  err = tegra_powergate_power_up(, false);
>>  if (err)
>> @@ -755,45 +735,26 @@ static int tegra_powergate_of_get_clks(struct 
>> tegra_powergate *pg,
>>  static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
>>   struct device_node *np, bool off)
>>  {
>> -struct reset_control *rst;
>> -unsigned int i, count;
>>  int err;
>>  
>> -count = of_count_phandle_with_args(np, "resets", "#reset-cells");
>> -if (count == 0)
>> -return -ENODEV;
>> -
>> -pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
>> -if (!pg->resets)
>> -return -ENOMEM;
>> -
>> -for (i = 0; i < count; i++) {
>> -pg->resets[i] = of_reset_control_get_by_index(np, i);
>> -if (IS_ERR(pg->resets[i])) {
>> -err = PTR_ERR(pg->resets[i]);
>> -goto error;
>> -}
>> -
>> -if (off)
>> -err = reset_control_assert(pg->resets[i]);
>> -else
>> -err = reset_control_deassert(pg->resets[i]);
>> -
>> -if (err) {
>> -reset_control_put(pg->resets[i]);
>> -goto error;
>> -}
>> +pg->reset = of_reset_control_array_get_exclusive(np);
>> +if (IS_ERR(pg->reset)) {
>> +pr_err("failed to get device resets\n");
>> +return PTR_ERR(pg->reset);
>>  }
>>  
>> -pg->num_resets = count;
>> +if (off)
>> +err = reset_control_assert(pg->reset);
>> +else
>> +err = reset_control_deassert(pg->reset);
>>  
>> -return 0;
>> +if (err)
>> +goto put_reset;
>>  
>> -error:
>> -while (i--)
>> -reset_control_put(pg->resets[i]);
>> +return 0;
>>  
>> -kfree(pg->resets);
>> +put_reset:
>> +reset_control_put(pg->reset);
>>  
>>  return err;
>>  }
>> @@ -885,10 +846,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, 
>> struct device_node *np)
>>  pm_genpd_remove(>genpd);

Re: [PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Philipp Zabel]

Hi Jon, Thierry,

On Wed, 2017-07-19 at 17:59 +0200, Philipp Zabel wrote:
> From: Vivek Gautam 
> 
> Make use of of_reset_control_array_get_exclusive() to manage
> an array of reset controllers available with the device.
> 
> Cc: Jon Hunter 
> Cc: Thierry Reding 
> Signed-off-by: Vivek Gautam 
> [p.za...@pengutronix.de: switch to hidden reset control array]
> Signed-off-by: Philipp Zabel 

will you pick this up now that the prerequisite patch 1 is contained in
master?
Please let me know if there are any issues with this patch.

regards
Philipp

> ---
> No changes since v6.
> ---
>  drivers/soc/tegra/pmc.c | 82 
> -
>  1 file changed, 20 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index e233dd5dcab3d..749b218147a19 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -124,8 +124,7 @@ struct tegra_powergate {
>   unsigned int id;
>   struct clk **clks;
>   unsigned int num_clks;
> - struct reset_control **resets;
> - unsigned int num_resets;
> + struct reset_control *reset;
>  };
>  
>  struct tegra_io_pad_soc {
> @@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct 
> tegra_powergate *pg)
>   return err;
>  }
>  
> -static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
> +static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
>  {
> - unsigned int i;
> - int err;
> -
> - for (i = 0; i < pg->num_resets; i++) {
> - err = reset_control_assert(pg->resets[i]);
> - if (err)
> - return err;
> - }
> -
> - return 0;
> + return reset_control_assert(pg->reset);
>  }
>  
> -static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
> +static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
>  {
> - unsigned int i;
> - int err;
> -
> - for (i = 0; i < pg->num_resets; i++) {
> - err = reset_control_deassert(pg->resets[i]);
> - if (err)
> - return err;
> - }
> -
> - return 0;
> + return reset_control_deassert(pg->reset);
>  }
>  
>  static int tegra_powergate_power_up(struct tegra_powergate *pg,
> @@ -566,8 +547,7 @@ int tegra_powergate_sequence_power_up(unsigned int id, 
> struct clk *clk,
>   pg.id = id;
>   pg.clks = 
>   pg.num_clks = 1;
> - pg.resets = 
> - pg.num_resets = 1;
> + pg.reset = IS_ERR(rst) ? NULL : rst;
>  
>   err = tegra_powergate_power_up(, false);
>   if (err)
> @@ -755,45 +735,26 @@ static int tegra_powergate_of_get_clks(struct 
> tegra_powergate *pg,
>  static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
>struct device_node *np, bool off)
>  {
> - struct reset_control *rst;
> - unsigned int i, count;
>   int err;
>  
> - count = of_count_phandle_with_args(np, "resets", "#reset-cells");
> - if (count == 0)
> - return -ENODEV;
> -
> - pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
> - if (!pg->resets)
> - return -ENOMEM;
> -
> - for (i = 0; i < count; i++) {
> - pg->resets[i] = of_reset_control_get_by_index(np, i);
> - if (IS_ERR(pg->resets[i])) {
> - err = PTR_ERR(pg->resets[i]);
> - goto error;
> - }
> -
> - if (off)
> - err = reset_control_assert(pg->resets[i]);
> - else
> - err = reset_control_deassert(pg->resets[i]);
> -
> - if (err) {
> - reset_control_put(pg->resets[i]);
> - goto error;
> - }
> + pg->reset = of_reset_control_array_get_exclusive(np);
> + if (IS_ERR(pg->reset)) {
> + pr_err("failed to get device resets\n");
> + return PTR_ERR(pg->reset);
>   }
>  
> - pg->num_resets = count;
> + if (off)
> + err = reset_control_assert(pg->reset);
> + else
> + err = reset_control_deassert(pg->reset);
>  
> - return 0;
> + if (err)
> + goto put_reset;
>  
> -error:
> - while (i--)
> - reset_control_put(pg->resets[i]);
> + return 0;
>  
> - kfree(pg->resets);
> +put_reset:
> + reset_control_put(pg->reset);
>  
>   return err;
>  }
> @@ -885,10 +846,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, 
> struct device_node *np)
>   pm_genpd_remove(>genpd);
>  
>  remove_resets:
> - while (pg->num_resets--)
> - reset_control_put(pg->resets[pg->num_resets]);
> -
> - kfree(pg->resets);
> + reset_control_put(pg->reset);
>  
>  remove_clks:
>   while (pg->num_clks--)
--
To unsubscribe from this list: send 

[PATCH v7 4/4] soc/tegra: pmc: Use the new reset APIs to manage reset controllers

[Philipp Zabel]

From: Vivek Gautam 

Make use of of_reset_control_array_get_exclusive() to manage
an array of reset controllers available with the device.

Cc: Jon Hunter 
Cc: Thierry Reding 
Signed-off-by: Vivek Gautam 
[p.za...@pengutronix.de: switch to hidden reset control array]
Signed-off-by: Philipp Zabel 
---
No changes since v6.
---
 drivers/soc/tegra/pmc.c | 82 -
 1 file changed, 20 insertions(+), 62 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index e233dd5dcab3d..749b218147a19 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -124,8 +124,7 @@ struct tegra_powergate {
unsigned int id;
struct clk **clks;
unsigned int num_clks;
-   struct reset_control **resets;
-   unsigned int num_resets;
+   struct reset_control *reset;
 };
 
 struct tegra_io_pad_soc {
@@ -348,32 +347,14 @@ static int tegra_powergate_enable_clocks(struct 
tegra_powergate *pg)
return err;
 }
 
-static int tegra_powergate_reset_assert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_assert(struct tegra_powergate *pg)
 {
-   unsigned int i;
-   int err;
-
-   for (i = 0; i < pg->num_resets; i++) {
-   err = reset_control_assert(pg->resets[i]);
-   if (err)
-   return err;
-   }
-
-   return 0;
+   return reset_control_assert(pg->reset);
 }
 
-static int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
+static inline int tegra_powergate_reset_deassert(struct tegra_powergate *pg)
 {
-   unsigned int i;
-   int err;
-
-   for (i = 0; i < pg->num_resets; i++) {
-   err = reset_control_deassert(pg->resets[i]);
-   if (err)
-   return err;
-   }
-
-   return 0;
+   return reset_control_deassert(pg->reset);
 }
 
 static int tegra_powergate_power_up(struct tegra_powergate *pg,
@@ -566,8 +547,7 @@ int tegra_powergate_sequence_power_up(unsigned int id, 
struct clk *clk,
pg.id = id;
pg.clks = 
pg.num_clks = 1;
-   pg.resets = 
-   pg.num_resets = 1;
+   pg.reset = IS_ERR(rst) ? NULL : rst;
 
err = tegra_powergate_power_up(, false);
if (err)
@@ -755,45 +735,26 @@ static int tegra_powergate_of_get_clks(struct 
tegra_powergate *pg,
 static int tegra_powergate_of_get_resets(struct tegra_powergate *pg,
 struct device_node *np, bool off)
 {
-   struct reset_control *rst;
-   unsigned int i, count;
int err;
 
-   count = of_count_phandle_with_args(np, "resets", "#reset-cells");
-   if (count == 0)
-   return -ENODEV;
-
-   pg->resets = kcalloc(count, sizeof(rst), GFP_KERNEL);
-   if (!pg->resets)
-   return -ENOMEM;
-
-   for (i = 0; i < count; i++) {
-   pg->resets[i] = of_reset_control_get_by_index(np, i);
-   if (IS_ERR(pg->resets[i])) {
-   err = PTR_ERR(pg->resets[i]);
-   goto error;
-   }
-
-   if (off)
-   err = reset_control_assert(pg->resets[i]);
-   else
-   err = reset_control_deassert(pg->resets[i]);
-
-   if (err) {
-   reset_control_put(pg->resets[i]);
-   goto error;
-   }
+   pg->reset = of_reset_control_array_get_exclusive(np);
+   if (IS_ERR(pg->reset)) {
+   pr_err("failed to get device resets\n");
+   return PTR_ERR(pg->reset);
}
 
-   pg->num_resets = count;
+   if (off)
+   err = reset_control_assert(pg->reset);
+   else
+   err = reset_control_deassert(pg->reset);
 
-   return 0;
+   if (err)
+   goto put_reset;
 
-error:
-   while (i--)
-   reset_control_put(pg->resets[i]);
+   return 0;
 
-   kfree(pg->resets);
+put_reset:
+   reset_control_put(pg->reset);
 
return err;
 }
@@ -885,10 +846,7 @@ static void tegra_powergate_add(struct tegra_pmc *pmc, 
struct device_node *np)
pm_genpd_remove(>genpd);
 
 remove_resets:
-   while (pg->num_resets--)
-   reset_control_put(pg->resets[pg->num_resets]);
-
-   kfree(pg->resets);
+   reset_control_put(pg->reset);
 
 remove_clks:
while (pg->num_clks--)
-- 
2.11.0

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