Prev: select useful number of entries for DMA debugging v3...
Next: check name_len before down_read xattr_sem and sb_read in ext2_xattr_get
From: Mark Brown on 13 Jul 2010 10:20
On Tue, Jul 13, 2010 at 07:39:33PM +0530, Sundar Iyer wrote:

> +static struct regulator_consumer_supply ab8500_vaux1_consumers[] = {
> + { .supply = "vaux1", },
> +};

It is extremely disappointing to see you reposting this without engaging
with my previous review at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Sundar R IYER on 13 Jul 2010 10:50
Mark,

>It is extremely disappointing to see you reposting this without engaging
>with my previous review at all.
Sincere apologies. As I said, I tried to repost after fixing your comments; but my
mistake that I didn't engage prior to the posting.

>> +static struct regulator_consumer_supply ab8500_vaux1_consumers[] = {
>> + { .dev = NULL, .supply = "vaux1", },
>> +};
>
>All these supplies with NULL devices are bogus, supplies are in terms of
>the device being supplied not the labels on the board. If you've got a
>supply with no device and the name of the supply on either the regulator
>or the board you're most likely doing it wrong. The only exception is
>for supplies used in cpufreq since we don't have a struct device we can
>use there.

I had these supplies as NULL, so that later on, when we add devices for our platform,
each developer can edit this file to hook up his own device. The reason I wanted this
file in the patch set was to include the machine constraints for the regulators on AB8500.

As admitted earlier, I will wait for your valuable comments before posting the next patch set.
(Also, I will try to make sure(still learning) to post the next patches into the same thread.)

Regards,
Sundar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mark Brown on 13 Jul 2010 11:00
On Tue, Jul 13, 2010 at 04:41:34PM +0200, Sundar R IYER wrote:
> >> +static struct regulator_consumer_supply ab8500_vaux1_consumers[] = {
> >> + { .dev = NULL, .supply = "vaux1", },
> >> +};

> >All these supplies with NULL devices are bogus, supplies are in terms of
> >the device being supplied not the labels on the board. If you've got a
> >supply with no device and the name of the supply on either the regulator
> >or the board you're most likely doing it wrong. The only exception is
> >for supplies used in cpufreq since we don't have a struct device we can
> >use there.

> I had these supplies as NULL, so that later on, when we add devices for our platform,
> each developer can edit this file to hook up his own device. The reason I wanted this
> file in the patch set was to include the machine constraints for the regulators on AB8500.

I'm pretty sure that people will be able to figure out how to add a
consumer list by themselves when they need it, and the lists you're
currently including are actively harmful in that they are encouraging
people to hard code board specific supply names or start passing supply
names around as platform data.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Mark Brown on 13 Jul 2010 11:10
On Tue, Jul 13, 2010 at 08:38:15PM +0530, Sundar R IYER wrote:

> OK. so I include only regulator_init_data and keep the
> regulator_consumer_supply empty. Is this the right thing forward?

Yes, if you don't have any actual devices to supply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Sundar R IYER on 13 Jul 2010 11:10
> I'm pretty sure that people will be able to figure out how to add a
> consumer list by themselves when they need it, and the lists you're
> currently including are actively harmful in that they are encouraging
> people to hard code board specific supply names or start passing supply
> names around as platform data.
OK. so I include only regulator_init_data and keep the
regulator_consumer_supply empty. Is this the right thing forward?

Regards,
Sundar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2 3 4
Prev: select useful number of entries for DMA debugging v3...
Next: check name_len before down_read xattr_sem and sb_read in ext2_xattr_get