linux-kernel - 回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

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:
 <PUZPR06MB58871C2E108BF1AC057EF461EF05A@PUZPR06MB5887.apcprd06.prod.outlook.com>
Date: Sat, 30 Aug 2025 13:20:14 +0000
From: Gary Yang <gary.yang@...tech.com>
To: Linus Walleij <linus.walleij@...aro.org>
CC: Krzysztof Kozlowski <krzk@...nel.org>, "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>, cix-kernel-upstream
	<cix-kernel-upstream@...tech.com>
Subject:
 回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

Hi Linus,

Thanks for your comments!

Sorry for delay reply

> 
> On Thu, Aug 28, 2025 at 10:58 AM Gary Yang <gary.yang@...tech.com> wrote:
> > > On 28/08/2025 07:37, Gary Yang wrote:
> 
> > > >> Whats the difference between? You have entire description field
> > > >> to explain this but instead you said something obvious there.
> > > >>
> > > > Cix sky1 has three power states. S0 means work state. S3 means STR
> state.
> > > S5 means SD state.
> > > >
> > > > The pin-controller on sky1 has two power states. They are S0 and S5.
> > >
> > >
> > > State != device. Please create bindings for devices, not states.
> > >
> >
> > Sorry, maybe I didn't explain it correctly before, and then make you
> > misunderstand
> >
> > There are two pin-controller on sky1. One is used under s0 state, other is
> used under s5 state.
> >
> > They are two devices
> 
> Just explain this in the description: and everyone will understand what is going
> on. Since "S0" and "S5" can be easy to confuse for "states"
> it is extra helpful with some extended descriptions.
> 

Yes, I have realized this problem, Thanks for your remind.

> > > >>> +    properties:
> > > >>> +      cix,pins:
> > > >>
> > > >> No, use generic properties from pinmux schema.
> > > >>
> > > >> You should also reference it.
> > > >
> > > > Did you suggest us to refer to
> > > Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml?
> > > >
> > > > Make us support drive-strength, bias-pull-down properties?
> > >
> > > and pinmux. There is a standard pins property.
> >
> > Ok, I see, try our best to support standard
> 
> Unfortunately many pin controllers have forged ahead with custom foo,pins =
> <....>; settings where they set up mux and electrical config by OR:in together
> different bits, and then they just poke this into some registers.
> 
> This isn't very helpful for users.
> 
> I initially wanted all functions and groups to be strings and then to associate
> groups with functions using strings in the device tree.
> 
> But I have realized (though much pain) that many developers don't like this.
> They want a magic number to write to a register to configure a pin, because
> their hardware has one (or several) register for each pin.
> 
> So nowadays the most common is to use a compromise.
> 
> A magic number in the pinmux property to set up the muxing.
> 
> For example:
> 
> arch/arm/boot/dts/mediatek/mt7623.dtsi:
> pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
>                <MT7623_PIN_76_SCL0_FUNC_SCL0>;
> 
> Then the electric properties like bias-pull-down; to set these on the state:
> 
>         i2c0_pins_a: i2c0-default {
>                 pins-i2c0 {
>                         pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
>                                  <MT7623_PIN_76_SCL0_FUNC_SCL0>;
>                         bias-disable;
>                 };
>         };
> 
> This is a good compromis becaus it looks similar on all SoCs and you see
> immediately what is going on: we enable
> SDA0 And SCL0 and disable bias, so there must be external pull-up resistors on
> this bus since I2C is open drain. Very easy for an electronics engineer to grasp,
> they don't need to be computer engineers or device tree experts.
> 

I appreciate your comments. You are very kind and nice.

I understand your thinking and try to support the standard referred to above.

I only need to spend some time to research this scheme and debug it on Radax O6 board.

If miss any information, please remind me. Thanks for your kind again.

> Yours,
> Linus Walleij

Best wishes
Gary