[PATCH v2] dt-bindings: pinctrl: mt8186: Fix 'reg-names' for pinctrl nodes

Allen-KH Cheng posted 1 patch 2 years, 1 month ago
There is a newer version of this series
.../bindings/pinctrl/pinctrl-mt8186.yaml         | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
[PATCH v2] dt-bindings: pinctrl: mt8186: Fix 'reg-names' for pinctrl nodes
Posted by Allen-KH Cheng 2 years, 1 month ago
The mt8186 contains 8 GPIO physical address bases that correspond to
the 'reg-names' of the pinctrl driver. The 'reg-names' entries in
bindings are ordered incorrectly, though. The system crashes due of an
erroneous address when the regulator initializes.

We fix the 'reg-names' for the pinctrl nodes and the pinctrl-mt8186
example in bindings.

Fixes: 338e953f1bd1 ("dt-bindings: pinctrl: mt8186: add pinctrl file and binding document")
Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
Signed-off-by: Guodong Liu <guodong.liu@mediatek.com>
---
Change in v1:
  * Capitalize First Letter of Sentences and rephrase the commit message
    [Allen-KH Cheng <allen-kh.cheng@mediatek.com>]
---
---
 .../bindings/pinctrl/pinctrl-mt8186.yaml         | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8186.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8186.yaml
index 1eeb885ce0c6..604445e390a7 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8186.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8186.yaml
@@ -41,12 +41,12 @@ properties:
       Gpio base register names.
     items:
       - const: iocfg0
-      - const: iocfg_bm
-      - const: iocfg_bl
-      - const: iocfg_br
+      - const: iocfg_lt
       - const: iocfg_lm
+      - const: iocfg_lb
+      - const: iocfg_bl
       - const: iocfg_rb
-      - const: iocfg_tl
+      - const: iocfg_rt
       - const: eint
 
   interrupt-controller: true
@@ -232,12 +232,12 @@ examples:
             <0x10002200 0x0200>,
             <0x10002400 0x0200>,
             <0x10002600 0x0200>,
-            <0x10002A00 0x0200>,
+            <0x10002a00 0x0200>,
             <0x10002c00 0x0200>,
             <0x1000b000 0x1000>;
-      reg-names = "iocfg0", "iocfg_bm", "iocfg_bl",
-                  "iocfg_br", "iocfg_lm", "iocfg_rb",
-                  "iocfg_tl", "eint";
+      reg-names = "iocfg0", "iocfg_lt", "iocfg_lm",
+                  "iocfg_lb", "iocfg_bl", "iocfg_rb",
+                  "iocfg_rt", "eint";
       gpio-controller;
       #gpio-cells = <2>;
       gpio-ranges = <&pio 0 0 185>;
-- 
2.18.0
Re: [PATCH v2] dt-bindings: pinctrl: mt8186: Fix 'reg-names' for pinctrl nodes
Posted by Nícolas F. R. A. Prado 2 years, 1 month ago
Hi Allen,

On Wed, Aug 17, 2022 at 01:26:15PM +0800, Allen-KH Cheng wrote:
> The mt8186 contains 8 GPIO physical address bases that correspond to
> the 'reg-names' of the pinctrl driver. The 'reg-names' entries in
> bindings are ordered incorrectly, though. The system crashes due of an
> erroneous address when the regulator initializes.
> 
> We fix the 'reg-names' for the pinctrl nodes and the pinctrl-mt8186
> example in bindings.
> 
> Fixes: 338e953f1bd1 ("dt-bindings: pinctrl: mt8186: add pinctrl file and binding document")
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
> Signed-off-by: Guodong Liu <guodong.liu@mediatek.com>

The sender's (your) signed-off-by should come last [1]. Also, what is the
participation of Guodong in this patch? If Guodong helped in writing the patch
there should also be a Co-developed-by tag for Guodong, as shown in [1].

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

> ---
[..]
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8186.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8186.yaml
[..]
> @@ -232,12 +232,12 @@ examples:
>              <0x10002200 0x0200>,
>              <0x10002400 0x0200>,
>              <0x10002600 0x0200>,
> -            <0x10002A00 0x0200>,
> +            <0x10002a00 0x0200>,

Also, you should avoid making changes that don't have to do with the patch
itself. This patch is about fixing the reg names, so no need to touch the
formatting of addresses. It just adds noise.

Other than that,

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Thanks,
Nícolas
Re: [PATCH v2] dt-bindings: pinctrl: mt8186: Fix 'reg-names' for pinctrl nodes
Posted by Allen-KH Cheng 2 years, 1 month ago
Hi Nícolas,

On Wed, 2022-08-17 at 12:09 -0400, Nícolas F. R. A. Prado wrote:
> Hi Allen,
> 
> On Wed, Aug 17, 2022 at 01:26:15PM +0800, Allen-KH Cheng wrote:
> > The mt8186 contains 8 GPIO physical address bases that correspond
> > to
> > the 'reg-names' of the pinctrl driver. The 'reg-names' entries in
> > bindings are ordered incorrectly, though. The system crashes due of
> > an
> > erroneous address when the regulator initializes.
> > 
> > We fix the 'reg-names' for the pinctrl nodes and the pinctrl-mt8186
> > example in bindings.
> > 
> > Fixes: 338e953f1bd1 ("dt-bindings: pinctrl: mt8186: add pinctrl
> > file and binding document")
> > Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
> > Signed-off-by: Guodong Liu <guodong.liu@mediatek.com>
> 
> The sender's (your) signed-off-by should come last [1]. Also, what is
> the
> participation of Guodong in this patch? If Guodong helped in writing
> the patch
> there should also be a Co-developed-by tag for Guodong, as shown in
> [1].
> 
> [1] 
> https://urldefense.com/v3/__https://www.kernel.org/doc/html/latest/process/submitting-patches.html*when-to-use-acked-by-cc-and-co-developed-by__;Iw!!CTRNKA9wMg0ARbw!1_mWpRIIYugoUq7AlCzsuuJoxY3jETMdZ6U3QXxh8a8FKe2mb5kCUOSAvZGqK3bUHBje7g$
>  

Guodong and I consult on this patch. I will add a Co-developed-by tag
for Guodong.

> 
> > ---
> 
> [..]
> > --- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8186.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8186.yaml
> 
> [..]
> > @@ -232,12 +232,12 @@ examples:
> >              <0x10002200 0x0200>,
> >              <0x10002400 0x0200>,
> >              <0x10002600 0x0200>,
> > -            <0x10002A00 0x0200>,
> > +            <0x10002a00 0x0200>,
> 
> Also, you should avoid making changes that don't have to do with the
> patch
> itself. This patch is about fixing the reg names, so no need to touch
> the
> formatting of addresses. It just adds noise.
> 
> Other than that,
> 
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> 
> Thanks,
> Nícolas


Ok, no problem. I will remove unrelated part.

Thanks,
Allen


Re: [PATCH v2] dt-bindings: pinctrl: mt8186: Fix 'reg-names' for pinctrl nodes
Posted by Krzysztof Kozlowski 2 years, 1 month ago
On 17/08/2022 08:26, Allen-KH Cheng wrote:
> The mt8186 contains 8 GPIO physical address bases that correspond to
> the 'reg-names' of the pinctrl driver. The 'reg-names' entries in
> bindings are ordered incorrectly, though. The system crashes due of an
> erroneous address when the regulator initializes.
> 
> We fix the 'reg-names' for the pinctrl nodes and the pinctrl-mt8186
> example in bindings.
> 
> Fixes: 338e953f1bd1 ("dt-bindings: pinctrl: mt8186: add pinctrl file and binding document")
> Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com>
> Signed-off-by: Guodong Liu <guodong.liu@mediatek.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof