From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EED37CA5531 for ; Wed, 13 Sep 2023 09:52:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239635AbjIMJw4 (ORCPT ); Wed, 13 Sep 2023 05:52:56 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37640 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239674AbjIMJwp (ORCPT ); Wed, 13 Sep 2023 05:52:45 -0400 Received: from mail-pf1-x434.google.com (mail-pf1-x434.google.com [IPv6:2607:f8b0:4864:20::434]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B145426B5 for ; Wed, 13 Sep 2023 02:52:19 -0700 (PDT) Received: by mail-pf1-x434.google.com with SMTP id d2e1a72fcca58-68fee7789f5so314825b3a.0 for ; Wed, 13 Sep 2023 02:52:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1694598739; x=1695203539; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:mail-followup-to:message-id:subject:cc:to :from:date:from:to:cc:subject:date:message-id:reply-to; bh=QrmmJ1EhzDfhE1It1FZH9PBgVmiPeKtR0wpyJLUEgCc=; b=MKVkRJaLkPY6HdZsih8tG836xfN9ZHgcXf8qEIlM0siG/yENdhpTA3Jj39/dU3Izem 3FcMalBkLUs6a0fimd8deLPnfhhfs2UzoEQg6KB3bEzi5qIoLMPLObKX/77UVJfsUX2F NvwwEI0mF06T1FZ5Dpfo+OLJhN1wsDA7I8C2cvRQz+fWlPY7eCxAm7HuqZIJR2rPttav 4THVomGAK6pCeVRtheIOBbkcdlQ1yUd79/V2nMCZGr9r3kfhD7YDAZxd8ta1Q7o1nHGc eA5xot+O3WlrbcVVbDjHQAW3Id8te6uvY6uN6ImJkmEArsyVMoF2ktKkKhim1VkPr1wv f5JQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694598739; x=1695203539; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:mail-followup-to:message-id:subject:cc:to :from:date:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=QrmmJ1EhzDfhE1It1FZH9PBgVmiPeKtR0wpyJLUEgCc=; b=SV9jijnhRLEGz5vK2X/HqfwojThQ0tGE3qOl/Wo6lVHDFWHMKLmYgjmgJIz6YwrHAH yYMsZJoW6csUNgBPwMKX9/LQgBbI3VusG9rPbQzu4o3GXosv1P7S7CKTRJ/dw96TNlwC vnKPNOVuYoU3LXqADuiGWm94HS0I8s7YaCyLzIaAwNvzltIKwtShXHkjQswDxow3Ttjt 5VyuV8Owu9xSExXFIotRq9xt7wvErrvS/C3Dy9lqMXd+kYgdICLDXyykZ2tEr9tM1kN0 xHipW1IjfFy3kQWwM+lgc2Ypgp7LIK9Cy2Gtsw/K/0BBVn/QpyCc5wLst+IMIzCOEu+a 0BHg== X-Gm-Message-State: AOJu0Yw/62Vt9XxqxPL3K4Kb5fB+c2AvHYIFGxlH4KwjuqncDpP+q0nQ lX8e0qCplgI6NoJQRP0aniP4sA== X-Google-Smtp-Source: AGHT+IFNMyd7W6bzJAVhVsLMqM5/Rsp+ZCNz86eI47s+5sormTNvfQzRlfpOcWEKUIkMMRrzXX+XAA== X-Received: by 2002:a05:6a21:6da4:b0:137:3941:17b3 with SMTP id wl36-20020a056a216da400b00137394117b3mr1839667pzb.6.1694598738528; Wed, 13 Sep 2023 02:52:18 -0700 (PDT) Received: from octopus ([2400:4050:c3e1:100:d859:5a28:b58:fb87]) by smtp.gmail.com with ESMTPSA id o12-20020a170902d4cc00b001bba373919bsm4467319plg.261.2023.09.13.02.52.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 13 Sep 2023 02:52:18 -0700 (PDT) Date: Wed, 13 Sep 2023 18:52:13 +0900 From: AKASHI Takahiro To: Cristian Marussi Cc: Peng Fan , Linus Walleij , "Peng Fan (OSS)" , "oleksii_moisieiev@epam.com" , "sudeep.holla@arm.com" , Aisheng Dong , "festevam@gmail.com" , Jacky Bai , "s.hauer@pengutronix.de" , "shawnguo@kernel.org" , "kernel@pengutronix.de" , dl-linux-imx , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-gpio@vger.kernel.org" Subject: Re: [RFC] scmi: pinctrl: support i.MX9 Message-ID: Mail-Followup-To: AKASHI Takahiro , Cristian Marussi , Peng Fan , Linus Walleij , "Peng Fan (OSS)" , "oleksii_moisieiev@epam.com" , "sudeep.holla@arm.com" , Aisheng Dong , "festevam@gmail.com" , Jacky Bai , "s.hauer@pengutronix.de" , "shawnguo@kernel.org" , "kernel@pengutronix.de" , dl-linux-imx , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-gpio@vger.kernel.org" References: <20230824070611.3335107-1-peng.fan@oss.nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On Thu, Sep 07, 2023 at 11:05:52AM +0100, Cristian Marussi wrote: > On Wed, Sep 06, 2023 at 02:43:46PM +0900, AKASHI Takahiro wrote: > > On Wed, Aug 30, 2023 at 02:37:48PM +0100, Cristian Marussi wrote: > > > On Wed, Aug 30, 2023 at 12:48:37PM +0000, Peng Fan wrote: > > > > Hi Cristian, > > > > > > > > > > Hi, > > > > > Hi, > > > > > > Subject: Re: [RFC] scmi: pinctrl: support i.MX9 > > > > > > > > > > On Fri, Aug 25, 2023 at 08:43:38AM +0000, Peng Fan wrote: > > > > > > > Subject: Re: [RFC] scmi: pinctrl: support i.MX9 > > > > > > > > > > > > > > On Thu, Aug 24, 2023 at 2:47 PM Peng Fan wrote: > > > > > > > > Me: > > > > > > > > > > Hi Peng, > > > > > > > > > > > > > > > > > > > >> it is merely making things more complex and also slower > > > > > > > > > bymaking the registers only accessible from this SCMI link. > > > > > > > > > > > > > > > > This is for safety reason, the pinctrl hardware must be handled by > > > > > > > > a system manager entity. So mmio direct access not allowed from > > > > > > > > Cortex-A side. > > > > > > > > > > > > > > Yeah I understood as much. But I don't think that the firmware is > > > > > > > really filtering any of the access, it will just poke into any > > > > > > > pinctrl register as instructed anyway so what's the point. Just looks like a > > > > > layer of indirection. > > > > > > > > > > > > No, the firmware has a check on whether a pin is allowed to be > > > > > > configured by the agent that wanna to configure the pin. > > > > > > > > > > > > > But I'm not your system manager, so it's not my decision. > > > > > > > > > > > > > > > The SCMI firmware is very straightforward, there is no group or > > > > > > > > function. > > > > > > > > > > > > > > > > It just accepts the format as this: > > > > > > > > MUX_TYPE, MUX VALUE, CONF_TYPE, CONF_VAL, DAISY_TYPE, DAISY > > > > > ID, > > > > > > > > DAISY_CFG, DAISY_VALUE. > > > > > > > > > > > > > > > > Similar as linux MMIO format. > > > > > > > > > > > > > > > > Our i.MX95 platform will support two settings, one with SCMI > > > > > > > > firmware, one without SCMI. These two settings will share the same > > > > > > > > pinctrl header file. > > > > > > > > > > > > > > > > And to simplify the scmi firmware design(anyway I am not owner of > > > > > > > > the firmware), to make pinctrl header shared w/o scmi, we take the > > > > > > > > current in-upstream freescale imx binding format. > > > > > > > > > > > > > > The SCMI people will have to state their position on this. > > > > > > > Like what they consider conformance and what extensions are allowed. > > > > > > > This is more a standardization question than an implementation > > > > > > > question so it's not really my turf. > > > > > > > > > > > > The i.MX95 SCMI firmware uses OEM extension type. So I just follow > > > > > > what the firmware did and support it in linux. Anyway let's wait > > > > > > Sudeep's reply. > > > > > > > > > > > > > > > > So my unsderstanding on this matter as of now is that: > > > > > > > > > > 1. the current SCMI Pinctrl specification can support your usecase by using > > > > > OEM Types and multiple pins/values CONFIG_GET/SET commands > > > > > > > > Yes, based on the Oleksii patchset with my local multiple configs support. > > > > > > > > > > Yes, I know, I pointed out on his series that the protocol has still to > > > be fixed to be aligned with the latest BETA2 spec (we changed the spec > > > on the fly while he was already posting indeed..) > > > > > > > > > > > > > 2. the Kernel SCMI protocol layer (driver/firmware/arm_scmi/pinctrl.c) > > > > > is equally fine and can support your usecase, AFTER Oleksii fixes it to > > > > > align it to the latest v3.2-BETA2 specification changes. > > > > > IOW, this means that, using the SCMI Pinctrl protocol operations > > > > > exposed in scmi_protocol.h, from somewhere, you are able to properly > > > > > configure multiple pins/values with your specific OEM types. > > > > > > > > Yes. > > > > > > Good. > > > > > > > > > > > > > > > > > 3. The SCMI Pinctrl driver (by Oleksii) built on top of the pinctrl protocol > > > > > operations is instead NOT suitable for your usecase since it uses the Linux > > > > > Generic Pinconf and IMX does not make use of it, and instead IMX has > > > > > its own bindings and related parsing logic. > > > > > > > > Yes. > > > > > > > > > > > > > > Am I right ? > > > > > > > > You are right. > > > > > > > > > > > > > > If this is the case, I would NOT try to abuse the current SCMI Pinctrl Generic > > > > > driver (by Oleksii) by throwing into it a bunch of IMX specific DT parsing, > > > > > also because you'll end-up NOT using most of the generic SCMI Pinctrl driver > > > > > but just reusing a bit of the probe (customized with your own DT maps > > > > > parsing) > > > > > > > > Only DT map to parse the dts and map to config array. Others are same, > > > > so need to export some symbols for pinctrl-scmi-imx.c driver if build imx > > > > scmi driver. > > > > > > > > > > Yes, but you are basically using some exported symbol to parse the DT in > > > your way and then you do not use anything of the various > > > functions/groups stuff...you just leverage some of the probing stuff and > > > then issue you OEM Type configs....I mean most of the picntrl-scmi > > > driver would be unused anyway in this scenario. > > > > > > > > > > > > > Instead, given that the spec[1.] and the protocol layer[2.] are fine for your > > > > > use case and you indeed have already a custom way to parse your DT > > > > > mappings, I would say that you could just write your own custom SCMI > > > > > driver ( ? pinctrl-imx-scmi), distinct and much more simple than the generic > > > > > one, that does its own IMX DT parsing and calls just the SCMI protocol > > > > > operations that it needs in the way that your platform expects: so basically > > > > > another Pinctrl SCMI driver that does not use the generic pinconf DT > > > > > configuration BUT DO USE the underlying SCMI Pinctrl protocol (via its > > > > > exposed protocol operations...) > > > > > > > > I am ok with this approach, but I need use the other ID, saying 0x99, not 0x19, > > > > because 0x19 will bind with the pinctrl-scmi.c driver, I could not reuse > > > > this ID for i.MX pinctrl-scmi-imx driver. Otherwise there will be issue if both > > > > driver are built in kernel image. > > > > > > > > > > Ok here I lost you. > > > > > > The protocol ID 0x19 is bound to the protocol layer and identifies the > > > standard Pinctrl protocol: usually you use a 0x99 to define and describe > > > you own specific NEW vendor protocol, BUT here you are saying you are fine to > > > use std Pinctrl spec AND the protocol operations as exposed in pinctrl.c, so > > > I dont see why you should use a new vendor protocol_id to basically > > > expose the same operations. (and I also dont see how you can do that > > > without hacks in the current codebase) > > > > > > You CAN have multiple SCMI drivers using the same protocol at the same > > > time (even more than one protocol at the same time), even though we try > > > to avoid it if there are no good reason to have more than one driver, there > > > is nothing in the spec or in the current SCMI platform or agent stacks that > > > inhibits such scenario (and I use iot heavily for my offline testing > > > indeed.) > > > > > > Look at: > > > > > > - drivers/hwmon/scmi-hwmon > > > - drivers/iio/common/scmi_sensors/scmi_iio.c > > > > > > and you'll see that these 2 drivers uses the same SENSOR protocol, just for > > > different sensor types so they do not interfere one with each other. > > > > Then, how are those two devices identified in a device tree? > > At SCMI probe time the SCMI core stack creates one device for each > 'protocol_id/name' as provided by the registered SCMI drivers in > their respective scmi_device_id/id_tableS, as long as the requested > protocol is active in the DT (protocol@XX defined) and the 'name' > is not duplicated; each of these devices, sharing the same SCMI > protocol, are created sharing also the same DT node protocol@XX. That's fine. > Their respective SCMI drivers are subsequently separately matched on > protocol_id/name and then probed as usual: it is up to the drivers not > to step on each other feet by competing for the same SCMI resources... > ...like issuing comflicting request for the same reosurce domain > on the same protocol. That's fine. My question/idea is simple; how two pinctrl drivers, if possible, be represented in a single device tree. To allow the case of Peng Fan, for example, I suppose that we need some DT binding like: scmi { scmi_pinctrl: protocol@19 { compatible = "arm, scmi-pinctrl-generic"; // not sure if needed pinmux ... // standard binding ... } } scmi_pinctrl_fsl { compatible = "fsl,scmi-pinctrl-imx9"; fsl,pinmux = <&scmi_pinctrl, ...>, <&scmi_pinctrl, ...>; ... } "scmi_pinctrl_fsl" driver may reuse generic SCMI pinctrl helper functions, except dt_node_to_map, in pictrl_ops. -Takahiro Akashi > I suppose, though, a lot depends on how the respective drivers interact > with their subsystems, like IIO SCMI does not really need any info > from DT, and the hwmon uses just the phandle to pick the sensor_domain_id > to associate, as an example, to thermal sensors. > > > That is the point in Peng's case and why he wants to have a dedicated > > protocol id (I don't agree to this, though.) > > If we follow Cristian's idea, we may want to have two dt nodes, say > > pinctrl-scmi-generic and pinctrl-scmi-imx, as phandles for other device > > nodes to refer to pins, respectively. > > I think there is currently no mechanism (or binding?) to allow this > > except adding a protocol id. > > > > Beside the uglyness of having 2 different DT bindings schemas for 2 > different drivers coexisting in the same parent protocol node, it is > still not clear to me why this cant be done technically without the need > of a new dummy 0x99 protocol_node, given that each driver can use its > own parsing logic in its probe, which is what Peng is doing in > dt_node_to_map indeed ...even though, maybe, the result would be so > ugly and/or not accepatble from bindings point of view that we will > not want to do it at the end anyway....I mean maybe it is just the > attempt itself to combine the Generic Pinctrl approach with the highly > specific IMX approach that inevitably leads to these clashes... > > ...I maybe missing something, though, so I'll made some experiments on > my side about this before I'll keep on blabbing :P > > Thanks, > Cristian