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

From: Oleksii Moisieiev <Oleksii_Moisieiev@epam.com>
To: Cristian Marussi <cristian.marussi@arm.com>,
	Linus Walleij <linus.walleij@linaro.org>
Cc: Peng Fan <peng.fan@oss.nxp.com>,
	"sudeep.holla@arm.com" <sudeep.holla@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	"michal.simek@amd.com" <michal.simek@amd.com>,
	"vincent.guittot@linaro.org" <vincent.guittot@linaro.org>,
	"souvik.chakravarty@arm.com" <souvik.chakravarty@arm.com>
Subject: Re: [RFC v1 1/2] scmi: Introduce pinctrl SCMI protocol driver
Date: Fri, 21 Apr 2023 09:53:37 +0000	[thread overview]
Message-ID: <11507608-323a-33fa-28f4-5436f484cb3b@epam.com> (raw)
In-Reply-To: <ZEJbxTzv6tlBkExS@e120937-lin>

Hi Cristian,

On 21.04.23 12:47, Cristian Marussi wrote:
> On Fri, Apr 21, 2023 at 11:28:38AM +0200, Linus Walleij wrote:
>> On Fri, Apr 21, 2023 at 10:40 AM Oleksii Moisieiev
>> <Oleksii_Moisieiev@epam.com> wrote:
>>> On 17.04.23 05:55, Peng Fan wrote:
>>>> On 4/13/2023 6:04 AM, Cristian Marussi wrote:
>>>> 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).
> Hi Linus,
>
>>  From a pinctrl point of view I do not understand this requirement.
>>
>> The pinctrl subsystem in the Linux kernel certainly does not support
>> an array of u32 for the pin config, we only support passing a single
>> u32 value along with the enumerator (config type), or well it is
>> actually 24 bits in Linux, the uppermost 8 bits is for the config type:
>> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/pinctrl/pinconf-generic.h__;!!GF_29dbcQIUBPA!0kv3XDV9f6l7rFEw9QFccCFZcl8t0vXfh_76DD80uq_dI_znPtdt4CVU6mLCZA5bHpliIRnzkIady9IAUEeIP7AB7fwWXg$ [git[.]kernel[.]org]
>>
>> /*
>>   * Helpful configuration macro to be used in tables etc.
>>   */
>> #define PIN_CONF_PACKED(p, a) ((a << 8) | ((unsigned long) p & 0xffUL))
>>
>> p = parameter (PIN_CONFIG_DRIVE_STRENGTH etc)
>> a = argument (value such as in mA)
>>
> My (possibly wrong) reasoning on the other reply, is based on the
> (possibly equally wrong :D) understanding that what Peng wants is just
> the possibility at the spec and the SCMI protocol layer (exposed in
> protocol operations) to issue PINCTRL_SET requests containing optionally
> an array of multiple ConfigType/Value pairs (which is anyway not supported
> by PinCtrl as I understand) instead of a single pair.
>
> ... but I can divine (:D)....that soon a new SCMI spec review/comment/amend
> cycle will be coming for people reading this...
>
> Thanks,
> Cristian

Pinctrl subsystem does support pinctrl_set call with an array of 
ConfigType/Value pairs. I'm sending those comfigs one-by-one from 
pinctrl_scmi_pinconf_set function. I think that can be added to the 
protocol to avoid extra scmi calls.

My understanding on Peng Fan words was that he want's to have multiple 
values for one ConfigType.

Let's wait for more information from Peng Fang.

Oleksii
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel