Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1680793130.git.oleksii_moisieiev@epam.com>
In-Reply-To: <cover.1680793130.git.oleksii_moisieiev@epam.com>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Tue, 11 Apr 2023 14:27:53 +0200
Message-ID: <CACRpkdaY1KjkJ_it0okrixrbCbe5Yy8PZiFAYacr_C03adP_hQ@mail.gmail.com>
Subject: Re: [RFC v1 0/2] Introducing generic SCMI pinctrl driver implementation
To:     Oleksii Moisieiev <Oleksii_Moisieiev@epam.com>
Cc:     "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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Oleksii,

thanks for your patches!

On Fri, Apr 7, 2023 at 12:18=E2=80=AFPM Oleksii Moisieiev
<Oleksii_Moisieiev@epam.com> wrote:

> This RFC patch series is intended to introduce the potential generic driv=
er for
> pin controls over SCMI protocol, provided in the latest beta version of D=
EN0056 [0].
>
> On ARM-based systems, a separate Cortex-M based System Control Processor =
(SCP)
> provides control on pins, as well as with power, clocks, reset controller=
s. In this case,
> kernel should use one of the possible transports, described in [0] to acc=
ess SCP and
> control clocks/power-domains etc. This driver is using SMC transport to c=
ommunicate with SCP via
> SCMI protocol and access to the Pin Control Subsystem.
>
> The provided driver consists of 2 parts:
>  - firmware/arm_scmi/pinctrl.c - the SCMI pinctrl protocol inmplementatio=
n
>    responsible for the communication with SCP firmware.
>
>  - drivers/pinctrl/pinctrl-scmi.c - pinctrl driver, which is using pinctr=
l
>   protocol implementation to access all necessary data.

TBH this looks so good that I am happy to merge it once you send a non-RFC
version.

My main concern would have been the protocol itself, but that was very
carefully tailored to match what the pin control subsystem needs and
I am quite happy with it the way it came out: using strings for groups and
functions.

The scmi code in patch 1 adds an extra layer of abstraction and a vtable
that would not have been necessary if all of the code was confined in
one file in drivers/pinctrl but it is not up to me how the SCMI people
want to abstract their stuff and there seems to be precedents to do things
this way.

I heard that someone wanted to also implement GPIO over SCMI, but
it is not part of this driver so I guess that will be a future addition.
It's a good starting point to add GPIO later.

Yours,
Linus Walleij