lkml.org 
[lkml]   [2023]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC v5 5/5] dt-bindings: firmware: arm,scmi: Add support for pinctrl protocol
From
Hi,

On 10/11/2023 15:24, Cristian Marussi wrote:
> On Fri, Nov 10, 2023 at 09:58:39AM +0900, Takahiro Akashi wrote:
>> Hi Arm folks,
>>
>
>> Do you have any comment?
>> I expect that you have had some assumption when you defined
>> SCMI pinctrl protocol specification.
>>
>
> [CC Souvik]
>
> @Souvik for context see:
> https://lore.kernel.org/all/CACRpkdZ4GborirSpa3GK_PwMgCvY0ePEmZO+CwnLcP6nAdieow@mail.gmail.com/
>
> Hi,
>
> I am not sure what is the full story here, BUT the spec was mainly aimed
> at supporting PINCTRL in SCMI with the idea to then, later on, base GPIO
> on top of it, "easily" building on the PINCTRL spec features in the future
> with a separate series from the one Oleksii is working on...but it like
> seems the future is already here and maybe we have discovered something
> to be clarified...
>
> Souvik/Oleksii can tell you better what were (if any) further assumptions
> related to GPIO on top on SCMI/PINCTRL, but the aim of this series was
> always to be just the basic Generic Pinctrl support when dealing with an
> SCMI server backend.

The initial assumption always was that GPIOs can be considered as a
specific function. Note that the spec does not define the types of
function and leaves it to the DT binding (or driver) to figure out the
function descriptions/names.

>
> Regarding the current Pinctrl series by Oleksii, I would also notice that,
> indeed, some "non-spec-dictated" naming assumptions are ALREADY present
> somehow, because, currently, the spec and the pinctrl SCMI protocol layer
> speak/refer about pins/groups/functions, as usual, only in terms of numeric
> identifiers/IDs (with an associated name of course), while the pinctrl
> driver (thanks to the Linux pictrl subsystem layer) describes and refers
> anything in the DT in terms of names: so all of this really works only
> because the names used in the DT happen to match the names reported by
> the backend server.
>
> My test DT uses just what Oleksii exemplified in the cover letter:
>
> pinctrl_i2c2: i2c2 {
> groups = "i2c2_a", "i2c2_b";
> function = "i2c2";
> };
>
> pinctrl_mdio: pins_mdio {
> groups = "avb_mdio";
> drive-strength = <24>;
> };
>
> keys_pins: keys {
> pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
> bias-pull-up;
> };
>
>
> with a dummmy test driver referring to it, so as to trigger the drivers
> core to initialize the pinctrl stuff.
>
> But all of this works just because, in the example of my emulated setup,
> my fake server exposes resources that are exactly named just as how the
> above DT expects pins/functions/pins to be named, because this is how
> the Generic Pinctrl subsystem in Linux is supposed to work, right ?
>
> The difference is that the names, in the case of pinctrl-scmi, are not
> hardcoded in the specific pin-controller driver BUT are provided dynamically
> by the SCMI server at runtime.
>
> And this is just a naming convention, between the Linux picntrl subsys AND
> the SCMI server, that allows the Linux Pinctrl subsys to map, under-hood,
> names to type/IDs as expected by the SCMI protocol layer (and by the spec):
> so when you will define and describe a real platform with a DT, you will
> will have to provide your name references, knowing that the shipped platform
> SCMI fw will advertise exactly the same (or a superset of them)
>
> As such, personally, I would find reasonable to use, equally, some
> conventional function name like 'gpio' to advertise and configure groups
> of pins as being used as GPIOs.

As a general principle, we dont try to put naming conventions in the
spec if it can be easily resolved via DT. If this is proving to be a
hassle then we can "recommend" in the spec that pins which can only be
GPIOs are named starting "GPIO". Similar for functions.

However looking at Linus' comments below, I am not sure we are at that
stage yet?

Regards,
Souvik

>
> Maybe, though, both of these expected naming comventions should be
> explicitly stated in the spec: indeed if you look at some Sensor protocol
> extensions added in v3.0, in 4.7.2.5.1 "Sensor Axis Descriptors"
> regarding naming we say:
>
> "It is recommended that the name ends with ‘_’
> followed by the axis of the sensor in uppercase. For
> example, the name for the x-axis of a triaxial
> accelerometer could be “acc_X” or “_X”."
>
> ...so maybe some similar remarks could be added here.
>
> Souvik is really the one who can have a say about the opportunity (or
> not) of these kind of explicit advised naming conventions on the spec,
> so I have CCed him.
>
>> On Mon, Nov 06, 2023 at 02:12:36PM +0100, Linus Walleij wrote:
>>> On Fri, Oct 27, 2023 at 8:28???AM Oleksii Moisieiev
>>> <Oleksii_Moisieiev@epam.com> wrote:
>>>
>>>> + keys_pins: keys-pins {
>>>> + pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
>>>> + bias-pull-up;
>>>> + };
>>>
>>> This is kind of interesting and relates to my question about naming groups and
>>> functions of GPIO pins.
>>>
>>> Here we see four pins suspiciously named "GP_*" which I read as
>>> "generic purpose"
>>> and they are not muxed to *any* function, yes pulled up.
>>>
>>> I would have expected something like:
>>>
>>> keys_pins: keys-pins {
>>> groups = "GP_5_17_grp", "GP_5_20_grp", "GP_5_22_grp", "GP_2_1_grp";
>>> function = "gpio";
>>> pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
>>> bias-pull-up;
>>> };
>>>
>>> I hope this illustrates what I see as a problem in not designing in
>>> GPIO as an explicit
>>> function, I get the impression that these pins are GPIO because it is hardware
>>> default.
>>
>> If you want to stick to "explicit", we may rather introduce a pre-defined
>> sub-node name, "gpio", in a device tree binding, i.e.
>>
>> protocol@19 { // pinctrl protocol
>> ... // other pinmux nodes
>>
>> scmi_gpio: gpio { // "gpio" is a fixed name
>> keys-pins {
>> pins = "GP_5_17", "GP_5_20", "GP_5_22", "GP_2_1";
>> bias-pull-up;
>> // possibly input or output
>> };
>> input-pins {
>> groups = "some group"; // any name
>> input-mode;
>> }
>> output-pins {
>> pins = "foo1", "foo2"; // any name
>> output-mode;
>> }
>> }
>> }
>>
>
> I suppose your proposal of a specially named "gpio" node would be
> another way, BUT it would also mean describing something in the DT that
> could be discoverable dynamically querying the server (while making the
> above assumptions about conventions).
>
> Thanks,
> Cristian

\
 
 \ /
  Last update: 2023-11-20 13:58    [W:0.105 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site