[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e6ee1ec2-1649-3e7e-9dbe-e94d636d8476@linaro.org>
Date: Mon, 5 Dec 2022 14:26:59 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Chester Lin <clin@...e.com>
Cc: Rob Herring <robh+dt@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
s32@....com, linux-gpio@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
Larisa Grigore <larisa.grigore@....com>,
Ghennadi Procopciuc <Ghennadi.Procopciuc@....com>,
Andrei Stefanescu <andrei.stefanescu@....com>,
Andreas Färber <afaerber@...e.de>,
Matthias Brugger <mbrugger@...e.com>,
ghennadi.procopciuc@....nxp.com
Subject: Re: [PATCH v2 1/2] dt-bindings: pinctrl: add schema for NXP S32 SoCs
On 05/12/2022 12:05, Chester Lin wrote:
> On Mon, Dec 05, 2022 at 10:02:14AM +0100, Krzysztof Kozlowski wrote:
>> On 05/12/2022 07:16, Chester Lin wrote:
>>> Hi Krzysztof,
>>>
>>> On Wed, Nov 30, 2022 at 03:58:52PM +0100, Krzysztof Kozlowski wrote:
>>>> On 28/11/2022 06:48, Chester Lin wrote:
>>>>> Add DT schema for the pinctrl driver of NXP S32 SoC family.
>>>>>
>>>>> Signed-off-by: Larisa Grigore <larisa.grigore@....com>
>>>>> Signed-off-by: Ghennadi Procopciuc <Ghennadi.Procopciuc@....com>
>>>>> Signed-off-by: Andrei Stefanescu <andrei.stefanescu@....com>
>>>>> Signed-off-by: Chester Lin <clin@...e.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - Remove the "nxp,pins" property since it has been moved into the driver.
>>>>> - Add descriptions for reg entries.
>>>>> - Refine the compatible name from "nxp,s32g-..." to "nxp,s32g2-...".
>>>>> - Fix schema issues and revise the example.
>>>>> - Fix the copyright format suggested by NXP.
>>>>>
>>>>> .../pinctrl/nxp,s32cc-siul2-pinctrl.yaml | 125 ++++++++++++++++++
>>>>> 1 file changed, 125 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..2fc25a9362af
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/pinctrl/nxp,s32cc-siul2-pinctrl.yaml
>>>>
>>>> Usually filename matches the compatible (or family name), so any reason
>>>> why compatible is "nxp,s32g2" but filename is "nxp,s32cc"?
>>>>
>>>
>>> According to NXP, the S32CC is a microarch which is adapted by different S32 SoCs,
>>> such as S32G2/G3 and S32R45. Some common IPs are implemented in S32CC, such as
>>> serial, pinctrl, mmc, gmac and some other peripheral interfaces. S32R45 has
>>> different pinouts compared to S32G2, which means that there would not be just
>>> "s32g2-siul2-pinctrl" but also "s32r45-siul2-pinctrl" in the compatible enum if
>>> S32R45 has to be upstreamed in the future. For this case, it seems to be
>>> inappropriate that adding a compatible name without any "s32g" keyword in the
>>> filename "nxp,s32g2-.." unless creating a new yaml for each platform, such as
>>> nxp,s32r45-siul2-pinctl.yaml.
>>
>> First, you can always rename a file if such need arises. Maybe new SoCs
>> will come, maybe not.
>>
>> Second, when you actually upstream new SoC it might anyway require new
>> bindings file, because pinctrls are quite specific and it is usually
>> difficult to support multiple devices in a nice, readable way in one
>> file. Therefore anyway another file is quite likely.
>>
>
> Thanks for your guidance. Will fix it.
>
>> (...)
>>
>>>>> +
>>>>> +patternProperties:
>>>>> + '-pins$':
>>>>> + type: object
>>>>> + additionalProperties: false
>>>>> +
>>>>> + patternProperties:
>>>>> + '-grp[0-9]$':
>>>>> + 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.
>>>>
>>>> All properties are accepted? What about values, e.g. for drive strength?
>>>
>>> For those unsupported properties such as drive-strength, the s32g2 pinctrl driver
>>> returns -EOPNOTSUPP.
>>
>> I don't care what the driver is doing, we do not discuss the driver. You
>> need to describe properly the hardware and I doubt that hardware accepts
>> all drive-strengths, all forms of pull resistors (so any Ohm value).
>>
>> Add constrains.
>>
>
> Thanks for the suggestion. IIUC, I should specifically described the supported
> pinmux and pincfg properties in this schema and then add an "additionalProperties: false"
Yes.
> in the end in order to constrain unsupported properties listed in the pattern
> pin groups.
You mean functions? or node names? The node names can be anything, can't
they?
If your drive strengths or slew rates have some limits, then I expect
them here.
Best regards,
Krzysztof
Powered by blists - more mailing lists