| Message ID | 1507647516-28993-1-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On 10/10/2017 04:58 PM, Geert Uytterhoeven wrote: > Add a device node for the ROHM BD9571MWV PMIC, based on the example in > the DT binding documentation, but using INTC-EX instead. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Do we need to describe more regulators? To my knowledge, no. > This depends on: > - [PATCH] pinctrl: sh-pfc: r8a7796: Add support for INTC-EX IRQ pins, > - [PATCH] pinctrl: sh-pfc: r8a7795: Add INTC-EX pins, groups and > function. > --- > arch/arm64/boot/dts/renesas/salvator-common.dtsi | 29 ++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi > index 3dcb26b1cb6bdec0..c171718bac7f52a1 100644 > --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi > +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi > @@ -353,6 +353,30 @@ > > &i2c_dvfs { > status = "okay"; > + > + pmic: pmic@30 { > + pinctrl-0 = <&irq0_pins>; > + pinctrl-names = "default"; > + > + compatible = "rohm,bd9571mwv"; > + reg = <0x30>; > + interrupt-parent = <&intc_ex>; Shouldn't this be gpio2 ? Why intc-ex ? > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > + interrupt-controller; > + #interrupt-cells = <2>; > + gpio-controller; > + #gpio-cells = <2>; > + > + regulators { > + dvfs: dvfs { > + regulator-name = "dvfs"; > + regulator-min-microvolt = <750000>; > + regulator-max-microvolt = <1030000>; > + regulator-boot-on; > + regulator-always-on; > + }; > + }; > + }; > }; > > &ohci0 { > @@ -407,6 +431,11 @@ > function = "i2c2"; > }; > > + irq0_pins: irq0 { > + groups = "intc_ex_irq0"; > + function = "intc_ex"; > + }; > + > pwm1_pins: pwm1 { > groups = "pwm1_a"; > function = "pwm1"; >
Hi Marek, On Wed, Oct 11, 2017 at 11:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote: > On 10/10/2017 04:58 PM, Geert Uytterhoeven wrote: >> Add a device node for the ROHM BD9571MWV PMIC, based on the example in >> the DT binding documentation, but using INTC-EX instead. >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> --- >> Do we need to describe more regulators? > > To my knowledge, no. OK, thanks! >> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi >> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi >> @@ -353,6 +353,30 @@ >> >> &i2c_dvfs { >> status = "okay"; >> + >> + pmic: pmic@30 { >> + pinctrl-0 = <&irq0_pins>; >> + pinctrl-names = "default"; >> + >> + compatible = "rohm,bd9571mwv"; >> + reg = <0x30>; >> + interrupt-parent = <&intc_ex>; > > Shouldn't this be gpio2 ? Why intc-ex ? Because we now have INTC-EX support ;-) Serious: if a pin used for interrupt signalling can be configured for both GPIO and INTC-EX aka IRQC, we typically configure it for INTC-EX. Probably because the latter is a simpler block, and thus consumes less power? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 10/12/2017 08:56 AM, Geert Uytterhoeven wrote: > Hi Marek, Hi! > On Wed, Oct 11, 2017 at 11:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >> On 10/10/2017 04:58 PM, Geert Uytterhoeven wrote: >>> Add a device node for the ROHM BD9571MWV PMIC, based on the example in >>> the DT binding documentation, but using INTC-EX instead. >>> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >>> --- >>> Do we need to describe more regulators? >> >> To my knowledge, no. > > OK, thanks! > >>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>> @@ -353,6 +353,30 @@ >>> >>> &i2c_dvfs { >>> status = "okay"; >>> + >>> + pmic: pmic@30 { >>> + pinctrl-0 = <&irq0_pins>; >>> + pinctrl-names = "default"; >>> + >>> + compatible = "rohm,bd9571mwv"; >>> + reg = <0x30>; >>> + interrupt-parent = <&intc_ex>; >> >> Shouldn't this be gpio2 ? Why intc-ex ? > > Because we now have INTC-EX support ;-) > > Serious: if a pin used for interrupt signalling can be configured for both > GPIO and INTC-EX aka IRQC, we typically configure it for INTC-EX. Probably > because the latter is a simpler block, and thus consumes less power? That should be in the commit message :)
Hi Marek, On Thu, Oct 12, 2017 at 10:35 AM, Marek Vasut <marek.vasut@gmail.com> wrote: > On 10/12/2017 08:56 AM, Geert Uytterhoeven wrote: >> On Wed, Oct 11, 2017 at 11:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >>> On 10/10/2017 04:58 PM, Geert Uytterhoeven wrote: >>>> Add a device node for the ROHM BD9571MWV PMIC, based on the example in >>>> the DT binding documentation, but using INTC-EX instead. >>>> >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >>>> --- >>>> Do we need to describe more regulators? >>> >>> To my knowledge, no. >> >> OK, thanks! >> >>>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>>> @@ -353,6 +353,30 @@ >>>> >>>> &i2c_dvfs { >>>> status = "okay"; >>>> + >>>> + pmic: pmic@30 { >>>> + pinctrl-0 = <&irq0_pins>; >>>> + pinctrl-names = "default"; >>>> + >>>> + compatible = "rohm,bd9571mwv"; >>>> + reg = <0x30>; >>>> + interrupt-parent = <&intc_ex>; >>> >>> Shouldn't this be gpio2 ? Why intc-ex ? >> >> Because we now have INTC-EX support ;-) >> >> Serious: if a pin used for interrupt signalling can be configured for both >> GPIO and INTC-EX aka IRQC, we typically configure it for INTC-EX. Probably >> because the latter is a simpler block, and thus consumes less power? > That should be in the commit message :) Does it? The schematics clearly mark the signal as IRQ0n, not GP2_00. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 10/12/2017 11:15 AM, Geert Uytterhoeven wrote: > Hi Marek, Hi Geert, > On Thu, Oct 12, 2017 at 10:35 AM, Marek Vasut <marek.vasut@gmail.com> wrote: >> On 10/12/2017 08:56 AM, Geert Uytterhoeven wrote: >>> On Wed, Oct 11, 2017 at 11:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >>>> On 10/10/2017 04:58 PM, Geert Uytterhoeven wrote: >>>>> Add a device node for the ROHM BD9571MWV PMIC, based on the example in >>>>> the DT binding documentation, but using INTC-EX instead. >>>>> >>>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >>>>> --- >>>>> Do we need to describe more regulators? >>>> >>>> To my knowledge, no. >>> >>> OK, thanks! >>> >>>>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>>>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>>>> @@ -353,6 +353,30 @@ >>>>> >>>>> &i2c_dvfs { >>>>> status = "okay"; >>>>> + >>>>> + pmic: pmic@30 { >>>>> + pinctrl-0 = <&irq0_pins>; >>>>> + pinctrl-names = "default"; >>>>> + >>>>> + compatible = "rohm,bd9571mwv"; >>>>> + reg = <0x30>; >>>>> + interrupt-parent = <&intc_ex>; >>>> >>>> Shouldn't this be gpio2 ? Why intc-ex ? >>> >>> Because we now have INTC-EX support ;-) >>> >>> Serious: if a pin used for interrupt signalling can be configured for both >>> GPIO and INTC-EX aka IRQC, we typically configure it for INTC-EX. Probably >>> because the latter is a simpler block, and thus consumes less power? >> That should be in the commit message :) > > Does it? The schematics clearly mark the signal as IRQ0n, not GP2_00. I have a patch in my tree which connects the ROHM PMIC to GPIO 2 , so if there is such a benefit to connecting it to intc-ex , I think it should be explained in the commit message.
Hi Geert, On Thu, Oct 12, 2017 at 3:56 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > Hi Marek, > > On Wed, Oct 11, 2017 at 11:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >> On 10/10/2017 04:58 PM, Geert Uytterhoeven wrote: >>> Add a device node for the ROHM BD9571MWV PMIC, based on the example in >>> the DT binding documentation, but using INTC-EX instead. >>> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >>> --- >>> Do we need to describe more regulators? >> >> To my knowledge, no. > > OK, thanks! > >>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>> @@ -353,6 +353,30 @@ >>> >>> &i2c_dvfs { >>> status = "okay"; >>> + >>> + pmic: pmic@30 { >>> + pinctrl-0 = <&irq0_pins>; >>> + pinctrl-names = "default"; >>> + >>> + compatible = "rohm,bd9571mwv"; >>> + reg = <0x30>; >>> + interrupt-parent = <&intc_ex>; >> >> Shouldn't this be gpio2 ? Why intc-ex ? > > Because we now have INTC-EX support ;-) > > Serious: if a pin used for interrupt signalling can be configured for both > GPIO and INTC-EX aka IRQC, we typically configure it for INTC-EX. Probably > because the latter is a simpler block, and thus consumes less power? I agree with your decision to use INTC-EX over GPIO, however I do think that this discussion smells like software policy... Isn't DT supposed to describe the hardware? =) It's almost like we want to DT to point out the pin, not the function.... Cheers, / magnus
Hi Magnus, On Fri, Oct 13, 2017 at 4:30 AM, Magnus Damm <magnus.damm@gmail.com> wrote: > On Thu, Oct 12, 2017 at 3:56 PM, Geert Uytterhoeven > <geert@linux-m68k.org> wrote: >> On Wed, Oct 11, 2017 at 11:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >>> On 10/10/2017 04:58 PM, Geert Uytterhoeven wrote: >>>> Add a device node for the ROHM BD9571MWV PMIC, based on the example in >>>> the DT binding documentation, but using INTC-EX instead. >>>> >>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >>>> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>>> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi >>>> @@ -353,6 +353,30 @@ >>>> >>>> &i2c_dvfs { >>>> status = "okay"; >>>> + >>>> + pmic: pmic@30 { >>>> + pinctrl-0 = <&irq0_pins>; >>>> + pinctrl-names = "default"; >>>> + >>>> + compatible = "rohm,bd9571mwv"; >>>> + reg = <0x30>; >>>> + interrupt-parent = <&intc_ex>; >>> >>> Shouldn't this be gpio2 ? Why intc-ex ? >> >> Because we now have INTC-EX support ;-) >> >> Serious: if a pin used for interrupt signalling can be configured for both >> GPIO and INTC-EX aka IRQC, we typically configure it for INTC-EX. Probably >> because the latter is a simpler block, and thus consumes less power? > > I agree with your decision to use INTC-EX over GPIO, however I do > think that this discussion smells like software policy... > > Isn't DT supposed to describe the hardware? =) > > It's almost like we want to DT to point out the pin, not the function.... Then we should describe all INTC-EX pins using the (not yet upstream?) DT connector framework, which can describe the pins can be served by both INTC-EX and GPIO2. I'm afraid it's too early for that. Note that lots of functionality can be served by General Purpose I/O instead of dedicated hardware. That doesn't mean it's always a good idea to do so.... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi index 3dcb26b1cb6bdec0..c171718bac7f52a1 100644 --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi @@ -353,6 +353,30 @@ &i2c_dvfs { status = "okay"; + + pmic: pmic@30 { + pinctrl-0 = <&irq0_pins>; + pinctrl-names = "default"; + + compatible = "rohm,bd9571mwv"; + reg = <0x30>; + interrupt-parent = <&intc_ex>; + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; + interrupt-controller; + #interrupt-cells = <2>; + gpio-controller; + #gpio-cells = <2>; + + regulators { + dvfs: dvfs { + regulator-name = "dvfs"; + regulator-min-microvolt = <750000>; + regulator-max-microvolt = <1030000>; + regulator-boot-on; + regulator-always-on; + }; + }; + }; }; &ohci0 { @@ -407,6 +431,11 @@ function = "i2c2"; }; + irq0_pins: irq0 { + groups = "intc_ex_irq0"; + function = "intc_ex"; + }; + pwm1_pins: pwm1 { groups = "pwm1_a"; function = "pwm1";
Add a device node for the ROHM BD9571MWV PMIC, based on the example in the DT binding documentation, but using INTC-EX instead. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- Do we need to describe more regulators? This depends on: - [PATCH] pinctrl: sh-pfc: r8a7796: Add support for INTC-EX IRQ pins, - [PATCH] pinctrl: sh-pfc: r8a7795: Add INTC-EX pins, groups and function. --- arch/arm64/boot/dts/renesas/salvator-common.dtsi | 29 ++++++++++++++++++++++++ 1 file changed, 29 insertions(+)