linux-kernel - 回复: [PATCH v3 2/3] pinctrl: cix: Add pin-controller support for sky1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <PUZPR06MB5887BE1631D6D6959067FDA5EFE9A@PUZPR06MB5887.apcprd06.prod.outlook.com>
Date: Thu, 16 Oct 2025 05:41:28 +0000
From: Gary Yang <gary.yang@...tech.com>
To: Linus Walleij <linus.walleij@...aro.org>
CC: "robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, cix-kernel-upstream
	<cix-kernel-upstream@...tech.com>
Subject:
 回复: [PATCH v3 2/3] pinctrl: cix: Add pin-controller support for sky1

Hi Linus:

Thanks for your comments

> 
> EXTERNAL EMAIL
> 
> Hi Gary,
> 
> this looks very nice, as long as we finish the bindings we can soon merge this I
> hope.
> 
> Some small comments!
> 
> On Tue, Oct 14, 2025 at 3:57 AM Gary Yang <gary.yang@...tech.com> wrote:
> 
> 
> > There are two pin-controllers on Cix Sky1 platform.
> > one is used under S0 state, the other is used under S0 and S5 state.
> >
> > Signed-off-by: Gary Yang <gary.yang@...tech.com>
> (...)
> 
> > +config PINCTRL_SKY1
> > +       tristate "Cix Sky1 pinctrl driver"
> > +       depends on ARCH_CIX
> 
> Maybe depends on ARCH_CIX || COMPILE_TEST so we get some compile
> testing in the test farms and also a test on the rest of the dependencies.
> 
> (Makes the bots complain so we can fix all such things!)
> 

OK, we will add COMPILE_TEST in next version.

> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/init.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_address.h>
> > +#include <linux/pinctrl/machine.h>
> 
> Do you really need <linux/pinctrl/machine.h>?
> 

Yes, You‘re right, it is a legacy issue. We will remove it next version

> Another thing you might want to consider is whether the designware GPIO will
> use this pin controller as "back-end" for the GPIOs using gpio-ranges in the
> GPIO controller nodes, for example (I just made this up):
> 
> gpio-ranges = <&pinctrl1 0 20 10>, <&pinctrl2 10 50 20>;
> 
> Then you might want to implement the GPIO accelerator operations in struct
> pinmux_ops, i.e. these:
> 
>  * @gpio_request_enable: requests and enables GPIO on a certain pin.
>  *      Implement this only if you can mux every pin individually as GPIO.
> The
>  *      affected GPIO range is passed along with an offset(pin number) into
> that
>  *      specific GPIO range - function selectors and pin groups are
> orthogonal
>  *      to this, the core will however make sure the pins do not collide.
>  * @gpio_disable_free: free up GPIO muxing on a certain pin, the reverse of
>  *      @gpio_request_enable
>  * @gpio_set_direction: Since controllers may need different configurations
>  *      depending on whether the GPIO is configured as input or output,
>  *      a direction selector function may be implemented as a backing
>  *      to the GPIO controllers that need pin muxing.
>  * @strict: do not allow simultaneous use of the same pin for GPIO and
> another
>  *      function. Check both gpio_owner and mux_owner strictly before
> approving
>  *      the pin request.
> 
> And nowadays it is also worth considering using:
> 
>         bool (*function_is_gpio) (struct pinctrl_dev *pctldev,
>                                   unsigned int selector);
> 
> To make the pinctrl core awara of a certain function selector being the GPIO
> function (which makes the accelerated functions work much better with the
> strict mode).
> 
> This can all be added later in separate patches, but this is a good time to make
> sure nothing stands in the way of doing this.
> 

GPIO IP on Sky1 is Cadence, not Synopsys designware. We wants to do it when upstream GPIO driver in the future. 
Are you agree?

> Yours,
> Linus Walleij

Best wishes
Gary