Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 9 Nov 2022 23:04:37 +0800
From:   Chester Lin <clin@suse.com>
To:     Rob Herring <robh@kernel.org>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        s32@nxp.com, linux-gpio@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        Larisa Grigore <larisa.grigore@nxp.com>,
        Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>,
        Andreas =?iso-8859-1?Q?F=E4rber?= <afaerber@suse.de>,
        Matthias Brugger <mbrugger@suse.com>,
        Chester Lin <clin@suse.com>
Subject: Re: [PATCH 1/2] dt-bindings: pinctrl: add schema for NXP S32 SoCs
Message-ID: <Y2vBhcN7IQwgWwMS@linux-8mug>
References: <20221031100843.14579-1-clin@suse.com>
 <20221031100843.14579-2-clin@suse.com>
 <20221102154903.GA3726664-robh@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221102154903.GA3726664-robh@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?EwxBMJo1lD+8586i+7Jqtv2M9L+a1o0NpSNk8YPfJmLuIvfP+o1b/HoljvAZ?=
 =?us-ascii?Q?v+kQqPUroG6ej6vN1UCqjqAnaiONdufHpMCyVmanLbTPMjcqIW8WeU0qc/jJ?=
 =?us-ascii?Q?QFJPw7zGGZiFVstBU9BErKIKtX3IooL91UC0MBYEYDZHIBX1/5Cs4+3I4aLy?=
 =?us-ascii?Q?eDPZoeYkjQ6R//mLX1eBlOzzHfC2rFeGzKERQE2K4clWPgqAnJ+2lFdL9Trd?=
 =?us-ascii?Q?YAyhYx+MDzzojeuI2nWY2NgUv4mQU8pElX64HHRZo/rcCuRVMUgXiSHeDVCl?=
 =?us-ascii?Q?6+cAZStIZgYhWL4y1g3fZGfe/6Vu1MT663NTf+R5G9Dmff1O2M0ljgVvqK2X?=
 =?us-ascii?Q?0vzZwrrW0seyrHMAvvH6Hc9CcED+vexItQ/SI0MBj3Ib34l0/uMnpel8jp3B?=
 =?us-ascii?Q?r8qBD92vy0xzwMXPhuhUom5YpzaxfR2IGJ7XJNAph00aOKX7E7XoG0o4iS0i?=
 =?us-ascii?Q?szehlqwkvbX48HcUjblSnGzF0bXClZg/chWLStaE3S+ZizVpcNT8MnuQDMSh?=
 =?us-ascii?Q?roykRVVSZcko9G8TALkUWigD/eBf4H7oE0dw+JiD0Ax7IxS6Q/fMsflzFdXc?=
 =?us-ascii?Q?f6YtcSGvqybPnjg6Re3BMWcnIHjXhMkwm2nm+xpnT50i6c5EOY+SbcmK6qsl?=
 =?us-ascii?Q?AOhCOD1V62rjXUKmFxgBnd55few4TP1t1f6n4bJ3mIWIShkl8YnG7rK+FXSN?=
 =?us-ascii?Q?0oxMU42Uch3ejZpnUNdaVaSDBwh14tmB1Eap4lK7XTJ36EkRjDW/mEWknpXb?=
 =?us-ascii?Q?rxD5pMFszTYsc2Ux3xyBYr41DuwbRJTnghAhICGv7YSGrBCMHCCnwL9JZG+q?=
 =?us-ascii?Q?AMEVEKleU+89boWWybRd8/oYfIICimnUPMrIn6nE8zDYjsLbnNNgpv5ENFis?=
 =?us-ascii?Q?3ecvaepMrslSYshEgpccY+twEc+u3lkTxcVEt9ISPRKIoQBspN07OSuTasbb?=
 =?us-ascii?Q?MSIln2kPR8zD29v29B8HQe6BniU6y+lQvXJvRXGIWZ6QWi27tm0EtAj2+uRq?=
 =?us-ascii?Q?Jj4m+c2qmhflj1RISJHd2mkWxJDYh47CBO3D44A12DLJeilRj4x1SPAsCSZh?=
 =?us-ascii?Q?wo17xa5kS9el5ybuI91+n6n3WmKkQtySJjO0EGKEzC6JOfSdLbf/vYsS0XAZ?=
 =?us-ascii?Q?J1gQmaEjmWnO0Sn6DeruyFIy385YY9cDWlwoB1UfCOGwZI/XNDsxHhMWS/b8?=
 =?us-ascii?Q?Y2y+RmQAIg/BUOUnAe3Bogy3Z0tOM5QaKRCQcxTCAvGx4NWt2Z5YY3+jm6Wu?=
 =?us-ascii?Q?B+gVKkb5DdrawplyA1/pdahzdt5gkM0VtbwEgguPhgebAW1f68tmHXVbphN9?=
 =?us-ascii?Q?eDpoTwVzmdZRZy57mC+iXQ5IpTfkVw+5oZoBLjJqvhkfueRCIpPse9cXFIIx?=
 =?us-ascii?Q?FL1nHKf6W54sw1UYHO1ax7UiU+Ot9fFVUCx334ZtXyRTYX1lnRVIyhVjUznS?=
 =?us-ascii?Q?syMjGqoKWFZ667wj8tAuVp78Non4m7Mtwlq7GobYhCObOnGBprJUEyXoyVXC?=
 =?us-ascii?Q?gdSar4lNYLdvZWhUYs3wORGVpWyl79nBPNxhXAMaBo3xe9NxukCvycyXUhMl?=
 =?us-ascii?Q?xbdfPjpk14gGDOSKLeY=3D?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 316eed6a-d7b9-4e25-80ad-08dac263bf90
X-MS-Exchange-CrossTenant-AuthSource: VI1PR0402MB3439.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Nov 2022 15:04:50.0471
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Z0MJeNu2H0DhMJXp4f17Fc03BBQPI6tG1iRxIt9TrYHxRBfJlpMucz8ZsF1wI7lY
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR04MB7550
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Rob,

Thank you for reviewing this patch.

On Wed, Nov 02, 2022 at 10:49:03AM -0500, Rob Herring wrote:
> On Mon, Oct 31, 2022 at 06:08:42PM +0800, Chester Lin wrote:
> > Add DT schema for the pinctrl driver of NXP S32 SoC family.
> > 
> > Signed-off-by: Larisa Grigore <larisa.grigore@nxp.com>
> > Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
> > Signed-off-by: Chester Lin <clin@suse.com>
> > ---
> >  .../pinctrl/nxp,s32cc-siul2-pinctrl.yaml      | 91 +++++++++++++++++++
> >  1 file changed, 91 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pinctrl/nxp,s32cc-siul2-pinctrl.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/nxp,s32cc-siul2-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/nxp,s32cc-siul2-pinctrl.yaml
> > new file mode 100644
> > index 000000000000..eafb9091cbf7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32cc-siul2-pinctrl.yaml
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (c) 2022 NXP
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/nxp,s32cc-siul2-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP S32 Common Chassis SIUL2 iomux controller
> > +
> > +maintainers:
> > +  - Ghennadi Procopciuc <Ghennadi.Procopciuc@nxp.com>
> > +  - Chester Lin <clin@suse.com>
> > +
> > +description: |
> > +  Core driver for the pin controller found on S32 Common Chassis SoC.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - nxp,s32g-siul2-pinctrl
> > +
> > +  reg:
> > +    minItems: 5
> > +    maxItems: 6
> > +    description: A list of register regions to be reserved.
> 
> Need to be explicit about what each entry is.
> 

They are
  - MSCR registers group 0 managed by the SIUL2 controller #0
  - MSCR registers group 1 managed by the SIUL2 controller #1
  - MSCR registers group 2 managed by the SIUL2 controller #1
  - IMCR registers group 0 managed by the SIUL2 controller #0
  - IMCR registers group 1 managed by the SIUL2 controller #1
  - IMCR registers group 2 managed by the SIUL2 controller #1

  - MSCR: Multiplexed Signal Configuration Register
    An MSCR register can configure the associated pin as either a GPIO pin
    or a function output pin depends on the selected signal source.
  - IMCR: Input Multiplexed Signal Configuration Register
    An IMCR register can configure the associated pin as function input
    pin depends on the selected signal source.


I will add descriptions for each entry in v2, thanks.

> > +
> > +  nxp,pins:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +    description:
> > +      A list of [start, end] pin ID boundaries that correspond to each of
> > +      the register regions reserved.
> 
> Looks like a matrix rather than an array.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - nxp,pins
> > +
> > +patternProperties:
> > +  '_pins$':
> 
> s/_/-/
> 
> > +    type: object
> 
>        additionalProperties: false
> 
> (and a blank line after)
> 
> > +    patternProperties:
> > +      '_grp[0-9]$':
> 
> s/_/-/
> 
> > +        type: object
> > +        allOf:
> > +          - $ref: pinmux-node.yaml#
> > +          - $ref: pincfg-node.yaml#
> 
>            unevaluatedProperties: false
> 
> > +        description:
> > +          Pinctrl node's client devices specify pin muxes using subnodes,
> > +          which in turn use the standard properties below.
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +
> > +    pinctrl: siul2-pinctrl@4009c240 {
> 
> pinctrl@...
> 
> > +        compatible = "nxp,s32g-siul2-pinctrl";
> > +
> > +              /* MSCR range */
> > +        reg = <0x4009c240 0x198>,
> > +              <0x44010400 0x2c>,
> > +              <0x44010480 0xbc>,
> > +              /* MSCR range */

It's IMCR range but not the 2nd MSCR range, will fix it in v2.

> > +              <0x4009ca40 0x150>,
> > +              <0x44010c1c 0x45c>,
> > +              <0x440110f8 0x108>;
> 
> What is in these holes in the memory map? Is this part of some larger 
> block? If so, that block needs to be described.
> 

In this case S32G contains two SIUL2 controllers, which are located in two
different memory regions, siul2_0@4009C000 and siul2_1@44010000. The siul2_0
contains the MSCR0-MSCR101 [starts at 0x4009c240] and IMCR0-IMCR83 [starts
at 0x4009ca40] registers, and there are some other reserved regions and
registers which are not related to pin muxing in siul2 memory ranges. Not sure
what the original reason it could be, but the expanded MSCR112-MSCR122, MSCR144
-MSCR190 and IMCR119-IMCR397, IMCR430-IMCR495 registers are present in
siul2_1's memory space. Besides, in the S32G pin map, some indexes are hidden
since they're not used as pinouts, such as MSCR indexes 102-111, 123-143, and
IMCR indexes 84-118, 398-429.

Anyway, that's why they look so fragmented. I will add more descriptions for
each entry in v2.

Thanks,
Chester

> > +
> > +                   /* MSCR range */
> > +        nxp,pins = <0   101>,
> > +                   <112 122>,
> > +                   <144 190>,
> > +                   /* IMCR range */
> > +                   <512 595>,
> > +                   <631 909>,
> > +                   <942 1007>;
> > +
> > +        llce_can0_pins {
> > +            llce_can0_grp0 {
> > +                pinmux = <0x2b0>;
> > +                input-enable;
> > +                slew-rate = <0x00>;
> > +            };
> > +
> > +            llce_can0_grp1 {
> > +                pinmux = <0x2c2>;
> > +                output-enable;
> > +                slew-rate = <0x00>;
> > +            };
> > +        };
> > +    };
> > +...
> > -- 
> > 2.37.3
> > 
> >