Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1614800 > unrolled thread

[PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers

Started byPaul Cercueil <paul@crapouillou.net>
First post2017-04-02 22:50 +0200
Last post2017-04-07 16:10 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers Paul Cercueil <paul@crapouillou.net> - 2017-04-02 22:50 +0200
    Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl  and gpio drivers Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-04-03 12:00 +0200
      Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl  and gpio drivers Paul Cercueil <paul@crapouillou.net> - 2017-04-03 12:30 +0200
        Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl  and gpio drivers Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-04-03 12:40 +0200
    Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl  and gpio drivers Linus Walleij <linus.walleij@linaro.org> - 2017-04-07 11:50 +0200
      Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl  and gpio drivers Paul Cercueil <paul@crapouillou.net> - 2017-04-07 16:10 +0200

#1614800 — [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers

FromPaul Cercueil <paul@crapouillou.net>
Date2017-04-02 22:50 +0200
Subject[PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers
Message-ID<trUBI-5rN-11@gated-at.bofh.it>
For a description of the pinctrl devicetree node, please read
Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt

For a description of the gpio devicetree nodes, please read
Documentation/devicetree/bindings/gpio/ingenic,gpio.txt

Signed-off-by: Paul Cercueil <paul@crapouillou.net>
---
 arch/mips/boot/dts/ingenic/jz4740.dtsi | 61 ++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)

 v2: Changed the devicetree bindings to match the new driver
 v3: No changes
 v4: Update the bindings for the v4 version of the drivers

diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
index 3e1587f1f77a..9c23c877fc34 100644
--- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
+++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
@@ -55,6 +55,67 @@
 		clock-names = "rtc";
 	};
 
+	pinctrl: ingenic-pinctrl@10010000 {
+		compatible = "ingenic,jz4740-pinctrl";
+		reg = <0x10010000 0x400>;
+
+		gpa: gpio-controller@0 {
+			compatible = "ingenic,gpio-bank-a", "ingenic,jz4740-gpio";
+
+			gpio-controller;
+			gpio-ranges = <&pinctrl 0 0 32>;
+			#gpio-cells = <2>;
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+
+			interrupt-parent = <&intc>;
+			interrupts = <28>;
+		};
+
+		gpb: gpio-controller@1 {
+			compatible = "ingenic,gpio-bank-b", "ingenic,jz4740-gpio";
+
+			gpio-controller;
+			gpio-ranges = <&pinctrl 0 32 32>;
+			#gpio-cells = <2>;
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+
+			interrupt-parent = <&intc>;
+			interrupts = <27>;
+		};
+
+		gpc: gpio-controller@2 {
+			compatible = "ingenic,gpio-bank-c", "ingenic,jz4740-gpio";
+
+			gpio-controller;
+			gpio-ranges = <&pinctrl 0 64 32>;
+			#gpio-cells = <2>;
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+
+			interrupt-parent = <&intc>;
+			interrupts = <26>;
+		};
+
+		gpd: gpio-controller@3 {
+			compatible = "ingenic,gpio-bank-d", "ingenic,jz4740-gpio";
+
+			gpio-controller;
+			gpio-ranges = <&pinctrl 0 96 32>;
+			#gpio-cells = <2>;
+
+			interrupt-controller;
+			#interrupt-cells = <2>;
+
+			interrupt-parent = <&intc>;
+			interrupts = <25>;
+		};
+	};
+
 	uart0: serial@10030000 {
 		compatible = "ingenic,jz4740-uart";
 		reg = <0x10030000 0x100>;
-- 
2.11.0

[toc] | [next] | [standalone]

#1615048 — Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2017-04-03 12:00 +0200
SubjectRe: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers
Message-ID<ts6We-5aL-15@gated-at.bofh.it>
In reply to#1614800
Hello!

On 4/2/2017 11:42 PM, Paul Cercueil wrote:

> For a description of the pinctrl devicetree node, please read
> Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt
>
> For a description of the gpio devicetree nodes, please read
> Documentation/devicetree/bindings/gpio/ingenic,gpio.txt
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
[...]

> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi b/arch/mips/boot/dts/ingenic/jz4740.dtsi
> index 3e1587f1f77a..9c23c877fc34 100644
> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
> @@ -55,6 +55,67 @@
>  		clock-names = "rtc";
>  	};
>
> +	pinctrl: ingenic-pinctrl@10010000 {

    The node name should be generic, so please rename it to something like 
"pin-controller@..."

> +		compatible = "ingenic,jz4740-pinctrl";
> +		reg = <0x10010000 0x400>;
> +
> +		gpa: gpio-controller@0 {

    The name should be just "gpio@0", according to ePAPR and its successor 
spec. Although, using the <unit-address> without the "reg" prop isn't allowed 
either...

[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1615078 — Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers

FromPaul Cercueil <paul@crapouillou.net>
Date2017-04-03 12:30 +0200
SubjectRe: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers
Message-ID<ts7pg-5zy-33@gated-at.bofh.it>
In reply to#1615048
Hi,

Le 2017-04-03 11:57, Sergei Shtylyov a écrit :
> Hello!
> 
> On 4/2/2017 11:42 PM, Paul Cercueil wrote:
> 
>> For a description of the pinctrl devicetree node, please read
>> Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt
>> 
>> For a description of the gpio devicetree nodes, please read
>> Documentation/devicetree/bindings/gpio/ingenic,gpio.txt
>> 
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> [...]
> 
>> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi 
>> b/arch/mips/boot/dts/ingenic/jz4740.dtsi
>> index 3e1587f1f77a..9c23c877fc34 100644
>> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
>> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
>> @@ -55,6 +55,67 @@
>>  		clock-names = "rtc";
>>  	};
>> 
>> +	pinctrl: ingenic-pinctrl@10010000 {
> 
>    The node name should be generic, so please rename it to something
> like "pin-controller@..."

OK.

>> +		compatible = "ingenic,jz4740-pinctrl";
>> +		reg = <0x10010000 0x400>;
>> +
>> +		gpa: gpio-controller@0 {
> 
>    The name should be just "gpio@0", according to ePAPR and its
> successor spec. Although, using the <unit-address> without the "reg"
> prop isn't allowed either...

ePAPR says: "If the node has no reg property, the unit-address may be
omitted if the node name alone differentiates the node from other nodes 
at
the same level in the tree."

I could use 'gpio@bank-a', it is allowed by the spec. Or do you prefer 
'gpio@0'?

> MBR, Sergei

I'll wait from some feedback on the other patches then send a v5.

Thanks,
-Paul

[toc] | [prev] | [next] | [standalone]

#1615084 — Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2017-04-03 12:40 +0200
SubjectRe: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers
Message-ID<ts7yW-5Em-11@gated-at.bofh.it>
In reply to#1615078
On 4/3/2017 1:20 PM, Paul Cercueil wrote:

>>> For a description of the pinctrl devicetree node, please read
>>> Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt
>>>
>>> For a description of the gpio devicetree nodes, please read
>>> Documentation/devicetree/bindings/gpio/ingenic,gpio.txt
>>>
>>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> [...]
>>
>>> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi
>>> b/arch/mips/boot/dts/ingenic/jz4740.dtsi
>>> index 3e1587f1f77a..9c23c877fc34 100644
>>> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi
>>> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi
>>> @@ -55,6 +55,67 @@
>>>          clock-names = "rtc";
>>>      };
>>>
>>> +    pinctrl: ingenic-pinctrl@10010000 {
>>
>>    The node name should be generic, so please rename it to something
>> like "pin-controller@..."
>
> OK.
>
>>> +        compatible = "ingenic,jz4740-pinctrl";
>>> +        reg = <0x10010000 0x400>;
>>> +
>>> +        gpa: gpio-controller@0 {
>>
>>    The name should be just "gpio@0", according to ePAPR and its
>> successor spec. Although, using the <unit-address> without the "reg"
>> prop isn't allowed either...
>
> ePAPR says: "If the node has no reg property, the unit-address may be

    My copy of ePAPR 1.1 says "must be omitted". :-)

> omitted if the node name alone differentiates the node from other nodes at
> the same level in the tree."

> I could use 'gpio@bank-a', it is allowed by the spec. Or do you prefer 'gpio@0'?

    Hm... maybe you should just use the "reg" prop.

> I'll wait from some feedback on the other patches then send a v5.
>
> Thanks,
> -Paul

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1618632 — Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers

FromLinus Walleij <linus.walleij@linaro.org>
Date2017-04-07 11:50 +0200
SubjectRe: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers
Message-ID<ttyGJ-5mq-7@gated-at.bofh.it>
In reply to#1614800
On Sun, Apr 2, 2017 at 10:42 PM, Paul Cercueil <paul@crapouillou.net> wrote:

> For a description of the pinctrl devicetree node, please read
> Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt
>
> For a description of the gpio devicetree nodes, please read
> Documentation/devicetree/bindings/gpio/ingenic,gpio.txt
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
>  arch/mips/boot/dts/ingenic/jz4740.dtsi | 61 ++++++++++++++++++++++++++++++++++
>  1 file changed, 61 insertions(+)
>
>  v2: Changed the devicetree bindings to match the new driver
>  v3: No changes
>  v4: Update the bindings for the v4 version of the drivers
(...)

> +       pinctrl: ingenic-pinctrl@10010000 {
> +               compatible = "ingenic,jz4740-pinctrl";
> +               reg = <0x10010000 0x400>;
> +
> +               gpa: gpio-controller@0 {
> +                       compatible = "ingenic,gpio-bank-a", "ingenic,jz4740-gpio";

As Sergei and Rob notes, the bank compatible properties look
a bit strange. Especially if they are all the same essentially.

I like Sergei's idea to simply use the reg property if what you want
is really a unique ID number. What do you think about this?

Yours,
Linus Walleij

[toc] | [prev] | [next] | [standalone]

#1618827 — Re: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers

FromPaul Cercueil <paul@crapouillou.net>
Date2017-04-07 16:10 +0200
SubjectRe: [PATCH v4 06/14] MIPS: jz4740: DTS: Add nodes for ingenic pinctrl and gpio drivers
Message-ID<ttCKn-8a9-41@gated-at.bofh.it>
In reply to#1618632
Le 2017-04-07 11:44, Linus Walleij a écrit :
> On Sun, Apr 2, 2017 at 10:42 PM, Paul Cercueil <paul@crapouillou.net> 
> wrote:
> 
>> For a description of the pinctrl devicetree node, please read
>> Documentation/devicetree/bindings/pinctrl/ingenic,pinctrl.txt
>> 
>> For a description of the gpio devicetree nodes, please read
>> Documentation/devicetree/bindings/gpio/ingenic,gpio.txt
>> 
>> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
>> ---
>>  arch/mips/boot/dts/ingenic/jz4740.dtsi | 61 
>> ++++++++++++++++++++++++++++++++++
>>  1 file changed, 61 insertions(+)
>> 
>>  v2: Changed the devicetree bindings to match the new driver
>>  v3: No changes
>>  v4: Update the bindings for the v4 version of the drivers
> (...)
> 
>> +       pinctrl: ingenic-pinctrl@10010000 {
>> +               compatible = "ingenic,jz4740-pinctrl";
>> +               reg = <0x10010000 0x400>;
>> +
>> +               gpa: gpio-controller@0 {
>> +                       compatible = "ingenic,gpio-bank-a", 
>> "ingenic,jz4740-gpio";
> 
> As Sergei and Rob notes, the bank compatible properties look
> a bit strange. Especially if they are all the same essentially.
> 
> I like Sergei's idea to simply use the reg property if what you want
> is really a unique ID number. What do you think about this?
> 
> Yours,
> Linus Walleij

I think the 'reg' property makes more sense, yes. I'll fix this in the 
v5, this
week-end. Do you think it can go in 4.12?

Thanks,
-Paul

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web