linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
@ 2012-01-18 15:17 Laxman Dewangan
  2012-01-18 17:54 ` Stephen Warren
  2012-01-20 12:11 ` Mark Brown
  0 siblings, 2 replies; 9+ messages in thread
From: Laxman Dewangan @ 2012-01-18 15:17 UTC (permalink / raw)
  To: lrg, broonie, linux-kernel; +Cc: linux-tegra, ldewangan

Initializing the number of voltages supported by different
rails of pmic device tps65911.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/regulator/tps65910-regulator.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
index 6742418..3ac5f91 100644
--- a/drivers/regulator/tps65910-regulator.c
+++ b/drivers/regulator/tps65910-regulator.c
@@ -188,56 +188,67 @@ static struct tps_info tps65911_regs[] = {
 		.name = "VDD1",
 		.min_uV = 600000,
 		.max_uV = 4500000,
+		.table_len = 73,
 	},
 	{
 		.name = "VDD2",
 		.min_uV = 600000,
 		.max_uV = 4500000,
+		.table_len = 73,
 	},
 	{
 		.name = "VDDCTRL",
 		.min_uV = 600000,
 		.max_uV = 1400000,
+		.table_len = 65,
 	},
 	{
 		.name = "LDO1",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
+		.table_len = 47,
 	},
 	{
 		.name = "LDO2",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
+		.table_len = 47,
 	},
 	{
 		.name = "LDO3",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
+		.table_len = 24,
 	},
 	{
 		.name = "LDO4",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
+		.table_len = 47,
 	},
 	{
 		.name = "LDO5",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
+		.table_len = 24,
 	},
 	{
 		.name = "LDO6",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
+		.table_len = 24,
 	},
 	{
 		.name = "LDO7",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
+		.table_len = 24,
 	},
 	{
 		.name = "LDO8",
 		.min_uV = 1000000,
 		.max_uV = 3300000,
+		.table_len = 24,
 	},
 };
 
-- 
1.7.1.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* RE: [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
  2012-01-18 15:17 [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails Laxman Dewangan
@ 2012-01-18 17:54 ` Stephen Warren
  2012-01-18 17:56   ` Mark Brown
  2012-01-20 12:11 ` Mark Brown
  1 sibling, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2012-01-18 17:54 UTC (permalink / raw)
  To: Laxman Dewangan, lrg, broonie, linux-kernel; +Cc: linux-tegra, Laxman Dewangan

Laxman Dewangan wrote at Wednesday, January 18, 2012 8:17 AM:
> Initializing the number of voltages supported by different
> rails of pmic device tps65911.
...
> diff --git a/drivers/regulator/tps65910-regulator.c b/drivers/regulator/tps65910-regulator.c
...
> @@ -188,56 +188,67 @@ static struct tps_info tps65911_regs[] = {
>  		.name = "VDD1",
>  		.min_uV = 600000,
>  		.max_uV = 4500000,
> +		.table_len = 73,
>  	},

Forgive me if these comments are bogus since I'm not familiar with
regulators much, but:

a) Don't you want to use ARRAY_SIZE() instead of hard-coding table_len's
value?

b) Don't you need to set .table to actually point at the table too? If not,
isn't tps65910_list_voltage() going to dereference a NULL pointer, since
it will dereference .table if .table_len is set?

-- 
nvpublic


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
  2012-01-18 17:54 ` Stephen Warren
@ 2012-01-18 17:56   ` Mark Brown
  2012-01-19  3:24     ` Laxman Dewangan
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2012-01-18 17:56 UTC (permalink / raw)
  To: Stephen Warren; +Cc: Laxman Dewangan, lrg, linux-kernel, linux-tegra

On Wed, Jan 18, 2012 at 09:54:27AM -0800, Stephen Warren wrote:
> Laxman Dewangan wrote at Wednesday, January 18, 2012 8:17 AM:

> > +		.table_len = 73,
> >  	},

> Forgive me if these comments are bogus since I'm not familiar with
> regulators much, but:

> a) Don't you want to use ARRAY_SIZE() instead of hard-coding table_len's
> value?

> b) Don't you need to set .table to actually point at the table too? If not,
> isn't tps65910_list_voltage() going to dereference a NULL pointer, since
> it will dereference .table if .table_len is set?

It's not actually a table at all any more as the driver calculates the
values, table_len is misnamed and should be renamed as well as set.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
  2012-01-18 17:56   ` Mark Brown
@ 2012-01-19  3:24     ` Laxman Dewangan
  2012-01-19 10:40       ` Mark Brown
  0 siblings, 1 reply; 9+ messages in thread
From: Laxman Dewangan @ 2012-01-19  3:24 UTC (permalink / raw)
  To: Mark Brown, Stephen Warren; +Cc: lrg, linux-kernel, linux-tegra

> On Wednesday, January 18, 2012 11:26 PM, Mark Brown wrote:
> 
> > > +		.table_len = 73,
> > >  	},
> 
> > Forgive me if these comments are bogus since I'm not familiar with
> > regulators much, but:
> 
> > a) Don't you want to use ARRAY_SIZE() instead of hard-coding table_len's
> > value?
> 
> > b) Don't you need to set .table to actually point at the table too? If not,
> > isn't tps65910_list_voltage() going to dereference a NULL pointer, since
> > it will dereference .table if .table_len is set?
> 
> It's not actually a table at all any more as the driver calculates the
> values, table_len is misnamed and should be renamed as well as set.


Do you want to rename the variable in this patch or in separate patch?
In place of table_len, I will say n_voltages as this is most common name
in regulator world.


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
  2012-01-19  3:24     ` Laxman Dewangan
@ 2012-01-19 10:40       ` Mark Brown
  2012-01-19 10:55         ` Laxman Dewangan
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2012-01-19 10:40 UTC (permalink / raw)
  To: Laxman Dewangan; +Cc: Stephen Warren, lrg, linux-kernel, linux-tegra

On Thu, Jan 19, 2012 at 08:54:19AM +0530, Laxman Dewangan wrote:

> Do you want to rename the variable in this patch or in separate patch?
> In place of table_len, I will say n_voltages as this is most common name
> in regulator world.

Probably two is clearer, though if it's not set already for any of the
regulators just one should be fine.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
  2012-01-19 10:40       ` Mark Brown
@ 2012-01-19 10:55         ` Laxman Dewangan
  2012-01-19 11:01           ` Mark Brown
  0 siblings, 1 reply; 9+ messages in thread
From: Laxman Dewangan @ 2012-01-19 10:55 UTC (permalink / raw)
  To: Mark Brown; +Cc: Stephen Warren, lrg, linux-kernel, linux-tegra


On Thursday 19 January 2012 04:10 PM, Mark Brown wrote:
> > Do you want to rename the variable in this patch or in separate patch?
> > In place of table_len, I will say n_voltages as this is most common name
> > in regulator world.
> 
> Probably two is clearer, though if it's not set already for any of the
> regulators just one should be fine.

The table_len is already used in some of entry i.e. for tps65910. I filled
entry for another device tps65911. So I would like to go for separate patch
for renaming the variables "table" to "voltage_table" and "table_len" to
"n_voltages".
Please let me know your opinion.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
  2012-01-19 10:55         ` Laxman Dewangan
@ 2012-01-19 11:01           ` Mark Brown
  2012-01-19 12:23             ` Laxman Dewangan
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2012-01-19 11:01 UTC (permalink / raw)
  To: Laxman Dewangan; +Cc: Stephen Warren, lrg, linux-kernel, linux-tegra

On Thu, Jan 19, 2012 at 04:25:17PM +0530, Laxman Dewangan wrote:
> On Thursday 19 January 2012 04:10 PM, Mark Brown wrote:

> The table_len is already used in some of entry i.e. for tps65910. I filled
> entry for another device tps65911. So I would like to go for separate patch
> for renaming the variables "table" to "voltage_table" and "table_len" to
> "n_voltages".
> Please let me know your opinion.

That's fine.  If in doubt and it doesn't introduce build breakage
splitting up into more patches is rarely a problem.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
  2012-01-19 11:01           ` Mark Brown
@ 2012-01-19 12:23             ` Laxman Dewangan
  0 siblings, 0 replies; 9+ messages in thread
From: Laxman Dewangan @ 2012-01-19 12:23 UTC (permalink / raw)
  To: Mark Brown; +Cc: Stephen Warren, lrg, linux-kernel, linux-tegra

On Thursday 19 January 2012 04:31 PM, Mark Brown wrote:

> > The table_len is already used in some of entry i.e. for tps65910. I filled
> > entry for another device tps65911. So I would like to go for separate patch
> > for renaming the variables "table" to "voltage_table" and "table_len" to
> > "n_voltages".
> > Please let me know your opinion.
> 
> That's fine.  If in doubt and it doesn't introduce build breakage
> splitting up into more patches is rarely a problem.


Ok, So to avoid the mess-ups, I will fetch the code from your tree 
http://git.kernel.org/?p=linux/kernel/git/broonie/regulator.git;a=shortlog;h=refs/heads/for-next-next
once you apply this change and then push the variable name change in new patch.

Laxman

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails.
  2012-01-18 15:17 [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails Laxman Dewangan
  2012-01-18 17:54 ` Stephen Warren
@ 2012-01-20 12:11 ` Mark Brown
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2012-01-20 12:11 UTC (permalink / raw)
  To: Laxman Dewangan; +Cc: lrg, linux-kernel, linux-tegra

On Wed, Jan 18, 2012 at 08:47:16PM +0530, Laxman Dewangan wrote:
> Initializing the number of voltages supported by different
> rails of pmic device tps65911.

Applied, thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-01-20 12:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-18 15:17 [PATCH V1 2/2] regulator: tps65910: Initialize n_voltages for rails Laxman Dewangan
2012-01-18 17:54 ` Stephen Warren
2012-01-18 17:56   ` Mark Brown
2012-01-19  3:24     ` Laxman Dewangan
2012-01-19 10:40       ` Mark Brown
2012-01-19 10:55         ` Laxman Dewangan
2012-01-19 11:01           ` Mark Brown
2012-01-19 12:23             ` Laxman Dewangan
2012-01-20 12:11 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).