Re: [RFC v1 1/2] scmi: Introduce pinctrl SCMI protocol driver

[Peng Fan <peng.fan@oss.nxp.com>]

On 4/21/2023 5:48 PM, Oleksii Moisieiev wrote:
> Hi Cristian,
> 
> On 21.04.23 12:30, Cristian Marussi wrote:
>> On Fri, Apr 21, 2023 at 08:40:47AM +0000, Oleksii Moisieiev wrote:
>>> Hi Peng Fan,
>>>
>>> On 17.04.23 05:55, Peng Fan wrote:
>>>>
>>>> On 4/13/2023 6:04 AM, Cristian Marussi wrote:
>>>>> On Fri, Apr 07, 2023 at 10:18:27AM +0000, Oleksii Moisieiev wrote:
>>>>>> Implementation of the SCMI client driver, which implements
>>>>>> PINCTRL_PROTOCOL. This protocol has ID 19 and is described
>>>>>> in the latest DEN0056 document.
>>>>> Hi,
>>>>>
>>>>>> This protocol is part of the feature that was designed to
>>>>>> separate the pinctrl subsystem from the SCP firmware.
>>>>>> The idea is to separate communication of the pin control
>>>>>> subsystem with the hardware to SCP firmware
>>>>>> (or a similar system, such as ATF), which provides an interface
>>>>>> to give the OS ability to control the hardware through SCMI protocol.
>>>>>> This is a generic driver that implements SCMI protocol,
>>>>>> independent of the platform type.
>>>>>>
>>>>>> DEN0056 document:
>>>>>> https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!y2hR3PEGGxiPjVeXBcgGyV03DPDhzgUKR0uHvsTpiafKgBar8Egc6oOOs-IkFIquhSf-qBzltqEMyzRZHq8eC4g$
>>>>>> [developer[.]arm[.]com]
>>>>>>
>>>>> No need to specify all of this in the commit message, just a note that
>>>>> you are adding a new SCMIv3.2 Pincontrol protocol, highlighting anything
>>>>> that has been left out in this patch (if any) will be enough.
>>>> Is it possible to extend the spec to support multilple uint32_t for PIN
>>>> CONFIG SET?
>>>>
>>>> With only one uint32_t could not satisfy i.MX requirement.
>>>>
>>>> Thanks,
>>>> Peng.
>>>>
>>> IIUC you are expecting to have an ability to set some kind of array of
>>> uint32_t config values to some specific ConfigType?
>>>
>>> I'm not sure if it's supported by pintctrl subsystem right now. I was
>>> unable to find an example in the existing device-tree pinctrl bindings.
>>> This makes me think that this kind of binding is OEM specific.
>>>
>>> Maybe it can be implemented by adding new IDs to OEM specific range
>>> (192-255) which is reserved for OEM specific units (See Table 23 of
>>> DEN0056E).
>>>
>> If I understood correctly the aim of Peng multi-valued request, I think
>> that even if Linux does not support using this kind of multiple valued
>> requests (as of now), if it is useful or required by some of the possibly
>> supported hardware, it should be described and allowed by the specification
>> and supported by the core SCMI protocol support at least, while the pinctrl
>> SCMI driver can ignore this and keep using a one-sized array protocol_ops
>> call internally (since it cannot do any different anyway as of now)
>>
>> IOW I dont think we should model too strictly the SCMI spec against only
>> what the Linux pinctrl subsystem support today, since Linux it is just
>> really only one of the possible SCMI agents and Linux implementation itself
>> can possibly change: it is better to model the spec on the HW requirements
>> or the possible usage patterns across all the possibly participating agents.
>>
>> As an example, for similar reasons, when the SCMI Voltage protocol was added
>> to the spec, at the very last minute, a change was made to the spec to allow
>> for negative voltages, even though the Linux regulator subsystem was not
>> and still is not supporting at all negative voltages as of now; so basically
>> the SCMI voltage protocol API now exposes a per-domain flag (negative_volts_allowed),
>> that allows any kind of voltage domain to be enumerated and handled at the SCMI
>> spec and core layer but that also allows any SCMI driver user, like the SCMI
>> Regulator driver, to decide on his own if negative voltages domains can be
>> supported: indeed the scmi-regulator driver just skips the initialization of
>> any voltage domain that is found to be describing negative voltages.
>>
>> Here is a bit different, it is more of an optimization in the call path
>> than an HW difference, but I would follow the same approach: with the
>> SCMI spec and the core SCMI stack (the protocol) that supports a multi-uint32
>> call as a general case, if useful for some scenarios, and instead the SCMI
>> pinctrl driver that just ignores this possibility and keep using a single-value
>> array anyway....then, it will be up to the guys leveraging this multi-valued
>> call to come up with a way to use it on their systems, possibly maybe contributing
>> back to upstream any needed modification if general enough
>> (not sure about the details of how this multi-vals operation should be...we'll have
>> to discuss that about the spec all together I think.)
>>
>> In any case, I would definitely NOT relegate such possibility to vendor space,
>> since it is something generic and, especially being just (as it seems to me) an
>> optimization on the call path at the end, it will just lead to uneeded duplication
>> of functionalities in the vendor implementation of stuff that it is already
>> very slightly differently supported by the standard.
>>
>> ...just my opinion anyway, I'll happily let other guys in this thread discuss and
>> decide about this :P
>>
>> Thanks,
>> Cristian
> 
> That sounds reasonable for me, although I can't imagine the use case of
> multi-valued config values (most likely this is the problem of my
> imagination). So I'd appreciate if Peng Fan could provide us with some
> examples.
> 
>   From my standpoint - ConfigTypes are meant to be simple value because
> they are mostly related to the electronic properties. But I agree that
> protocol should be platform-agnostic.
> 
> It will be great if Peng Fan could provide some examples, so we can
> think about the best solution.

Just replied here:
https://lore.kernel.org/all/b9eca9a4-f4b6-b98c-2861-eb14380d2d5f@oss.nxp.com/

Thanks,
Peng.

> 
> Best regards,
> 
> Oleksii