* Re: [PATCH 04/10 v3] ARM: mach-shmobile: add fixed voltage regulators to armadillo800eva
2012-06-26 22:32 [PATCH 04/10 v3] ARM: mach-shmobile: add fixed voltage regulators to armadillo800eva Guennadi Liakhovetski
@ 2012-06-27 15:56 ` Guennadi Liakhovetski
2012-06-28 20:17 ` Rafael J. Wysocki
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Guennadi Liakhovetski @ 2012-06-27 15:56 UTC (permalink / raw)
To: linux-sh
On Wed, 27 Jun 2012, Guennadi Liakhovetski wrote:
> On armadillo800eva provide a 3.3V supply for its SD/MMC-card interfaces.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> arch/arm/mach-shmobile/board-armadillo800eva.c | 16 ++++++++++++++++
> 1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 9e37026..819cee5 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -28,6 +28,8 @@
> #include <linux/platform_device.h>
> #include <linux/gpio.h>
> #include <linux/gpio_keys.h>
> +#include <linux/regulator/fixed.h>
> +#include <linux/regulator/machine.h>
> #include <linux/sh_eth.h>
> #include <linux/videodev2.h>
> #include <linux/usb/renesas_usbhs.h>
> @@ -407,6 +409,17 @@ static struct platform_device gpio_keys_device = {
> },
> };
>
> +/* Fixed 3.3V regulator to be used by SDHI0, SDHI1, MMCIF */
> +static struct regulator_consumer_supply fixed3v3_power_consumers[] > +{
> + REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.0"),
> + REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.0"),
> + REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.1"),
> + REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.1"),
> + REGULATOR_SUPPLY("vmmc", "sh_mmcif.0"),
> + REGULATOR_SUPPLY("vqmmc", "sh_mmcif.0"),
There is a slight problem with this patch: a1 (r8a7740) SoCs are currently
the only ones registering their MMCIF device without an index, i.e., with
.id = -1. This is reflected in board-armadillo800eva.c and in
clock-r8a7740.c. This patch erroneously uses sh_mmcif.0 for device name,
which is also what every single other ARM and sh platform in the mainline,
using mmcif, AFAICS, currently does.
Therefore question: Magnus, Rafael: what would you prefer - fix the above
regulator entries to not use a device ID, or unify a1 with other SoCs to
use .id = 0?
Thanks
Guennadi
> +};
> +
> /* SDHI0 */
> /*
> * FIXME
> @@ -584,6 +597,9 @@ clock_error:
> */
> static void __init eva_init(void)
> {
> + regulator_register_always_on(0, "fixed-3.3V", fixed3v3_power_consumers,
> + ARRAY_SIZE(fixed3v3_power_consumers), 3300000);
> +
> eva_clock_init();
>
> r8a7740_pinmux_init();
> --
> 1.7.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 04/10 v3] ARM: mach-shmobile: add fixed voltage regulators to armadillo800eva
2012-06-26 22:32 [PATCH 04/10 v3] ARM: mach-shmobile: add fixed voltage regulators to armadillo800eva Guennadi Liakhovetski
2012-06-27 15:56 ` Guennadi Liakhovetski
@ 2012-06-28 20:17 ` Rafael J. Wysocki
2012-06-28 23:57 ` Magnus Damm
2012-06-29 7:49 ` Guennadi Liakhovetski
3 siblings, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2012-06-28 20:17 UTC (permalink / raw)
To: linux-sh
On Wednesday, June 27, 2012, Guennadi Liakhovetski wrote:
> On Wed, 27 Jun 2012, Guennadi Liakhovetski wrote:
>
> > On armadillo800eva provide a 3.3V supply for its SD/MMC-card interfaces.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > arch/arm/mach-shmobile/board-armadillo800eva.c | 16 ++++++++++++++++
> > 1 files changed, 16 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > index 9e37026..819cee5 100644
> > --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> > +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > @@ -28,6 +28,8 @@
> > #include <linux/platform_device.h>
> > #include <linux/gpio.h>
> > #include <linux/gpio_keys.h>
> > +#include <linux/regulator/fixed.h>
> > +#include <linux/regulator/machine.h>
> > #include <linux/sh_eth.h>
> > #include <linux/videodev2.h>
> > #include <linux/usb/renesas_usbhs.h>
> > @@ -407,6 +409,17 @@ static struct platform_device gpio_keys_device = {
> > },
> > };
> >
> > +/* Fixed 3.3V regulator to be used by SDHI0, SDHI1, MMCIF */
> > +static struct regulator_consumer_supply fixed3v3_power_consumers[] > > +{
> > + REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.0"),
> > + REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.0"),
> > + REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.1"),
> > + REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.1"),
> > + REGULATOR_SUPPLY("vmmc", "sh_mmcif.0"),
> > + REGULATOR_SUPPLY("vqmmc", "sh_mmcif.0"),
>
> There is a slight problem with this patch: a1 (r8a7740) SoCs are currently
> the only ones registering their MMCIF device without an index, i.e., with
> .id = -1. This is reflected in board-armadillo800eva.c and in
> clock-r8a7740.c. This patch erroneously uses sh_mmcif.0 for device name,
> which is also what every single other ARM and sh platform in the mainline,
> using mmcif, AFAICS, currently does.
>
> Therefore question: Magnus, Rafael: what would you prefer - fix the above
> regulator entries to not use a device ID, or unify a1 with other SoCs to
> use .id = 0?
I wonder what's going to be more straightforward?
Rafael
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 04/10 v3] ARM: mach-shmobile: add fixed voltage regulators to armadillo800eva
2012-06-26 22:32 [PATCH 04/10 v3] ARM: mach-shmobile: add fixed voltage regulators to armadillo800eva Guennadi Liakhovetski
2012-06-27 15:56 ` Guennadi Liakhovetski
2012-06-28 20:17 ` Rafael J. Wysocki
@ 2012-06-28 23:57 ` Magnus Damm
2012-06-29 7:49 ` Guennadi Liakhovetski
3 siblings, 0 replies; 5+ messages in thread
From: Magnus Damm @ 2012-06-28 23:57 UTC (permalink / raw)
To: linux-sh
On Thu, Jun 28, 2012 at 12:56 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Wed, 27 Jun 2012, Guennadi Liakhovetski wrote:
>
>> On armadillo800eva provide a 3.3V supply for its SD/MMC-card interfaces.
>>
>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> ---
>> arch/arm/mach-shmobile/board-armadillo800eva.c | 16 ++++++++++++++++
>> 1 files changed, 16 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
>> index 9e37026..819cee5 100644
>> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
>> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
>> @@ -28,6 +28,8 @@
>> #include <linux/platform_device.h>
>> #include <linux/gpio.h>
>> #include <linux/gpio_keys.h>
>> +#include <linux/regulator/fixed.h>
>> +#include <linux/regulator/machine.h>
>> #include <linux/sh_eth.h>
>> #include <linux/videodev2.h>
>> #include <linux/usb/renesas_usbhs.h>
>> @@ -407,6 +409,17 @@ static struct platform_device gpio_keys_device = {
>> },
>> };
>>
>> +/* Fixed 3.3V regulator to be used by SDHI0, SDHI1, MMCIF */
>> +static struct regulator_consumer_supply fixed3v3_power_consumers[] >> +{
>> + REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.0"),
>> + REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.0"),
>> + REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.1"),
>> + REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.1"),
>> + REGULATOR_SUPPLY("vmmc", "sh_mmcif.0"),
>> + REGULATOR_SUPPLY("vqmmc", "sh_mmcif.0"),
>
> There is a slight problem with this patch: a1 (r8a7740) SoCs are currently
> the only ones registering their MMCIF device without an index, i.e., with
> .id = -1. This is reflected in board-armadillo800eva.c and in
> clock-r8a7740.c. This patch erroneously uses sh_mmcif.0 for device name,
> which is also what every single other ARM and sh platform in the mainline,
> using mmcif, AFAICS, currently does.
>
> Therefore question: Magnus, Rafael: what would you prefer - fix the above
> regulator entries to not use a device ID, or unify a1 with other SoCs to
> use .id = 0?
As I mentioned over private email and as you know by now - I prefer to
focus on function over cosmetics. Morimoto-san picked a -1 in the A1
case but I can't really see how that is relevant to anything
functional in this series.
Anyway, so regarding this particular matter, as long as it works I'm
fine with anything.
In this case this means - please don't add any further dependencies -
just fix and post a new version. If you want to discuss these minor
things with Morimoto-san then feel free to do so, but that should be
dealt with incrementally on top of this if so.
Thanks,
/ magnus
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 04/10 v3] ARM: mach-shmobile: add fixed voltage regulators to armadillo800eva
2012-06-26 22:32 [PATCH 04/10 v3] ARM: mach-shmobile: add fixed voltage regulators to armadillo800eva Guennadi Liakhovetski
` (2 preceding siblings ...)
2012-06-28 23:57 ` Magnus Damm
@ 2012-06-29 7:49 ` Guennadi Liakhovetski
3 siblings, 0 replies; 5+ messages in thread
From: Guennadi Liakhovetski @ 2012-06-29 7:49 UTC (permalink / raw)
To: linux-sh
On Thu, 28 Jun 2012, Rafael J. Wysocki wrote:
> On Wednesday, June 27, 2012, Guennadi Liakhovetski wrote:
> > On Wed, 27 Jun 2012, Guennadi Liakhovetski wrote:
> >
> > > On armadillo800eva provide a 3.3V supply for its SD/MMC-card interfaces.
> > >
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > ---
> > > arch/arm/mach-shmobile/board-armadillo800eva.c | 16 ++++++++++++++++
> > > 1 files changed, 16 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > > index 9e37026..819cee5 100644
> > > --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> > > +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > > @@ -28,6 +28,8 @@
> > > #include <linux/platform_device.h>
> > > #include <linux/gpio.h>
> > > #include <linux/gpio_keys.h>
> > > +#include <linux/regulator/fixed.h>
> > > +#include <linux/regulator/machine.h>
> > > #include <linux/sh_eth.h>
> > > #include <linux/videodev2.h>
> > > #include <linux/usb/renesas_usbhs.h>
> > > @@ -407,6 +409,17 @@ static struct platform_device gpio_keys_device = {
> > > },
> > > };
> > >
> > > +/* Fixed 3.3V regulator to be used by SDHI0, SDHI1, MMCIF */
> > > +static struct regulator_consumer_supply fixed3v3_power_consumers[] > > > +{
> > > + REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.0"),
> > > + REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.0"),
> > > + REGULATOR_SUPPLY("vmmc", "sh_mobile_sdhi.1"),
> > > + REGULATOR_SUPPLY("vqmmc", "sh_mobile_sdhi.1"),
> > > + REGULATOR_SUPPLY("vmmc", "sh_mmcif.0"),
> > > + REGULATOR_SUPPLY("vqmmc", "sh_mmcif.0"),
> >
> > There is a slight problem with this patch: a1 (r8a7740) SoCs are currently
> > the only ones registering their MMCIF device without an index, i.e., with
> > .id = -1. This is reflected in board-armadillo800eva.c and in
> > clock-r8a7740.c. This patch erroneously uses sh_mmcif.0 for device name,
> > which is also what every single other ARM and sh platform in the mainline,
> > using mmcif, AFAICS, currently does.
> >
> > Therefore question: Magnus, Rafael: what would you prefer - fix the above
> > regulator entries to not use a device ID, or unify a1 with other SoCs to
> > use .id = 0?
>
> I wonder what's going to be more straightforward?
The easier fix is, of course, to redo the above hunk with just "sh_mmcif"
as a device name. This is also what Magnus wanted to have and what I've
sent a few minutes ago. But I'm pretty certain - if we keep this one
platform different from the rest, I've made this mistake once, I'll do it
again :-) So, eventually, maybe trying to keep the error probability as
low as possible is good _if_ we don't have to make any further sacrifices
for that.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply [flat|nested] 5+ messages in thread