Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Date:   Tue, 24 Oct 2023 22:42:57 +0900
From:   AKASHI Takahiro <takahiro.akashi@linaro.org>
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     Rob Herring <robh@kernel.org>, sudeep.holla@arm.com,
        cristian.marussi@arm.com, krzysztof.kozlowski+dt@linaro.org,
        conor+dt@kernel.org, Oleksii_Moisieiev@epam.com,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org
Subject: Re: [RFC v2 5/5] dt-bindings: gpio: Add bindings for pinctrl based
 generic gpio driver
Message-ID: <ZTfJ4b7VdTkA0sxo@octopus>
Mail-Followup-To: AKASHI Takahiro <takahiro.akashi@linaro.org>,
        Linus Walleij <linus.walleij@linaro.org>,
        Rob Herring <robh@kernel.org>, sudeep.holla@arm.com,
        cristian.marussi@arm.com, krzysztof.kozlowski+dt@linaro.org,
        conor+dt@kernel.org, Oleksii_Moisieiev@epam.com,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org
References: <20231006132346.GA3426353-robh@kernel.org>
 <CACRpkdaLsfSBEG-h9ZNT2_Lm8tW8AZO7tedDVNeuZoQAqSkyjw@mail.gmail.com>
 <ZSTgTC4cFFpofYAk@octopus>
 <CACRpkdYD6pkccYoy90AfzV3KT7oYkBPD2_4ZW-AXzT1eUVpchA@mail.gmail.com>
 <ZS3yK/f12Mxw9rXe@octopus>
 <CACRpkdarDrVkPmyDawhZ+H94S4F=dtDSDVuKegi-eNfQNDY3rg@mail.gmail.com>
 <ZTduWx7CH1ifI5Uc@octopus>
 <CACRpkdba=echR=rZYKVbROfaOp4mzjTQ9RphHFyzqSNgE1jZqg@mail.gmail.com>
 <ZTemAK/jBtv9b5xP@octopus>
 <CACRpkdY-5uS9EeXfDFVOiRKiFmwwSn3jRVGhT-n4JMqesHEumw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACRpkdY-5uS9EeXfDFVOiRKiFmwwSn3jRVGhT-n4JMqesHEumw@mail.gmail.com>
Precedence: bulk

On Tue, Oct 24, 2023 at 03:12:51PM +0200, Linus Walleij wrote:
> On Tue, Oct 24, 2023 at 1:10???PM AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> 
> > As far as I understand, there is only one way for SCMI gpio driver
> > to know which pins are actually GPIO pins; Calling PINCNTRL_CONFIG_GET
> > command with "Input-mode" or "Output-mode" configuration type
> > against *every* pin-controller's pins.
> > (Here I assume that the command would fail with INVALID_PARAMETERS or
> > NOT_SUPPORTED if configuring the given pin as a GPIO input or output
> > is not possible. But the specification seems to be a bit ambiguous.)
> 
> As I also wrote in the reply to Christian, I expect the SCMI firmware
> to consider GPIO a function on the pins, and either individual pins
> (groups with just one pin in it) or entire groups of pins can be switched
> to perform the "gpio function". ("gpio function" is akin to "i2c function"
> or "HDMI function" etc.)

First of all, there is no pre-defined naming convention either for
pins, groups or functions. SCMI firmware can give them any names.

Secondly, What you said in the above is already implemented in
my RFC patch. Please remember the example that I gave:

>     gpio-ranges = <&scmi_pinctrl 6 0 0>;
>     gpio-ranges-group-names = "pinmux_gpio";
> 
> means that SCMI *group*, "pinmux_gpio", are mapped to this driver's
> gpio range which starts with 5. If "pinmux_gpio" indicates SCMI *pin*
> range [20..24],
> 
>     baa-gpios = <&gpio0 7>;
> will refer to gpio pin#7 that is actually SCMI's 22 (=20 + (7-5)).

Given the fact there is no naming convention, we need to explicitly
specify an associated group name in "gpio-ranges-group-names" in any way.

What my driver doesn't care for now is the case where a group of GPIO pins
are multiplexed with other functions (UART, I2C or whatever else).
In this case, we need to configure "pinconf" setup prior to using those
pins as GPIO anyway. Simply, it is out of scope of my driver.
(We can still use existing generic GPIO interfaces to operate them once
set up, though.)

After all, I still believe we need "gpio-ranges" property in most of
all use cases (The only exception is, as I mentioned, to unconditionally
map all pinctrl's pins to GPIO (if possible) when SCMI firmware provides
only GPIO function for all pins. I think it is a simple and yet likely
use case.

Thanks,
-Takahiro Akashi


> 
> If the SCMI protocol considers GPIO usage to be something else
> than a function of a pin, that is going to be a problem. Then the SCMI
> protocol need some other way of determining that the pin is in
> GPIO mode, and perhaps something would need to be added to
> the protocol for that.
> 
> The reason is that in practice a lot of hardware has to decouple
> the pin from any internal function in order to use it as GPIO, and
> connect it to the GPIO block that can drive the line high and low.
> And we don't select that as a function, how is the firmware going
> to know that it needs to do this? Implicitly from the call requesting
> the line to be output perhaps. But if the firmware can be altered
> to do that, the firmware can just as well be altered to present
> GPIO as a proper function.
> 
> Using a function makes most sense, because the board firmware
> knows which pins are GPIO lines and what they are used for
> (such as a LED or camera flash) and at boot certainly put them
> into GPIO function and drive them high/low or set them as
> input (high impedance).
> 
> > It means that, if SCMI firmware has 100 pinctrl pins, the driver needs
> > to call the command 200 times in order to get the answer.
> 
> I think we should only need to check which function each pin
> has and those that are in the GPIO function we put into the ranges.
> 
> Yours,
> Linus Walleij