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

[Oleksii Moisieiev]

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.

Best regards,

Oleksii