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] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <PUZPR06MB5887887C93BFF42BC8417D96EF3BA@PUZPR06MB5887.apcprd06.prod.outlook.com>
Date: Thu, 28 Aug 2025 08:58:04 +0000
From: Gary Yang <gary.yang@...tech.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, "linus.walleij@...aro.org"
	<linus.walleij@...aro.org>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>
CC: "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 Krzysztof,

> On 28/08/2025 07:37, Gary Yang wrote:
> > Hi Krzysztof,
> >
> > Thanks for your comments
> >
> >>
> >> On 27/08/2025 04:42, Gary Yang wrote:
> >>> Add dt-bindings docs
> >>
> >> For what? Describe the hardware here in one, two sentences.
> >>
> >
> > OK, we will add some description for it next version
> >
> >>>
> >>> Signed-off-by: Gary Yang <gary.yang@...tech.com>
> >>> ---
> >>>  .../bindings/pinctrl/cix,sky1-pinctrl.yaml    |  77 +++
> >>>  include/dt-bindings/pinctrl/pads-sky1.h       | 592
> >> ++++++++++++++++++
> >>>  2 files changed, 669 insertions(+)
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> >>>  create mode 100644 include/dt-bindings/pinctrl/pads-sky1.h
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> >>> b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> >>> new file mode 100644
> >>> index 000000000000..10a4a292188e
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yam
> >>> +++ l
> >>> @@ -0,0 +1,77 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/pinctrl/cix,sky1-pinctrl.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Cix Sky1 Pin Controller
> >>> +
> >>> +maintainers:
> >>> +  - Gary Yang <gary.yang@...tech.com>
> >>> +
> >>> +description:
> >>> +  Please refer to pinctrl-bindings.txt in this directory for common
> >>> +  binding part and usage.
> >>
> >> Drop description, not desired really.
> >>
> >
> > Ok, this yaml file comes from other yaml file. If not needed, we
> > remove it next version
> >
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - cix,sky1-iomuxc
> >>> +      - cix,sky1-iomuxc-s5
> >>
> >> 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

> >
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +# Client device subnode's properties
> >>> +patternProperties:
> >>> +  '-pins$':
> >>> +    type: object
> >>> +    description:
> >>> +      Pinctrl node's client devices use subnodes for desired pin
> >> configuration.
> >>> +      Client device subnodes use below standard properties.
> >>> +
> >>> +    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

> 
> ...
> 
> >>> diff --git a/include/dt-bindings/pinctrl/pads-sky1.h
> >>> b/include/dt-bindings/pinctrl/pads-sky1.h
> >>> new file mode 100644
> >>> index 000000000000..44550e4105b3
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/pinctrl/pads-sky1.h
> >>
> >> Bindings follow compatible naming. See writing bindings.
> >>
> >
> > Did you suggest rename it to pinctrl-sky1.h ?
> 
> No. I suggest to be named EXACTLY like compatible.
> 

OK, I see

> >
> >>> @@ -0,0 +1,592 @@
> >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> >>> +/*
> >>> + * Copyright 2024-2025 Cix Technology Group Co., Ltd.
> >>> + */
> >>> +
> >>> +#ifndef __SKY1_PADS_H
> >>> +#define __SKY1_PADS_H
> >>> +
> >>> +#define CIX_PAD_GPIO001_OFFSET                       0x0
> >>> +#define CIX_PAD_GPIO002_OFFSET                       0x4
> >>
> >> Not bindings. Drop all this.
> >>
> >
> > Do you mean those macros not used need to delete?
> 
> Really, what is unlcear in "drop all this"? Drop means to remove.
> 
> You ask for confirmation for some really obvious comments.
> 
> BTW, if you disagree provide arguments (in terms of bindings) why these are
> bindings.

Sorry, make you misunderstand again. I know drop means remove. I want to know whether "all this" mean all macros in the file or not.
These macros will be used by client in the dtsi and dts file. If remove them, maybe add them again when they are used in the further.
It looks like the file which locate include/dt-bindings/pinctrl/pads-imx8qm.h

Best wishes
Gary