On Wed, Nov 20, 2024 at 6:12 PM Andre Przywara <andre.przywara@xxxxxxx> wrote: > > On Wed, 13 Nov 2024 16:50:19 +0800 > Chen-Yu Tsai <wens@xxxxxxxx> wrote: > > Hi Chen-Yu, > > sorry for the late reply, I was away for a week. > > > On Mon, Nov 11, 2024 at 8:58 AM Andre Przywara <andre.przywara@xxxxxxx> wrote: > > > > > > The A523 contains a pin controller similar to previous SoCs, although > > > using 10 GPIO banks (PortB-PortK), all of them being IRQ capable. > > > This introduces a new style of binding, where the pinmux values for each > > > pin group is stored in the new "allwinner,pinmux" property in the DT > > > node, instead of requiring every driver to store a mapping between the > > > function names and the required pinmux. > > > > > > Add the new name to the list of compatible strings, and required it to > > > have 10 interrupts described. Also add the new pinmux property. > > > > > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx> > > > --- > > > .../pinctrl/allwinner,sun4i-a10-pinctrl.yaml | 23 +++++++++++++++++-- > > > 1 file changed, 21 insertions(+), 2 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml > > > index 4502405703145..6fc18e92e1e94 100644 > > > --- a/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml > > > +++ b/Documentation/devicetree/bindings/pinctrl/allwinner,sun4i-a10-pinctrl.yaml > > > @@ -56,6 +56,8 @@ properties: > > > - allwinner,sun50i-h6-r-pinctrl > > > - allwinner,sun50i-h616-pinctrl > > > - allwinner,sun50i-h616-r-pinctrl > > > + - allwinner,sun55i-a523-pinctrl > > > + - allwinner,sun55i-a523-r-pinctrl > > > - allwinner,suniv-f1c100s-pinctrl > > > - nextthing,gr8-pinctrl > > > > > > @@ -64,7 +66,7 @@ properties: > > > > > > interrupts: > > > minItems: 1 > > > - maxItems: 8 > > > + maxItems: 10 > > > description: > > > One interrupt per external interrupt bank supported on the > > > controller, sorted by bank number ascending order. > > > @@ -119,13 +121,17 @@ patternProperties: > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > enum: [10, 20, 30, 40] > > > > > > + allwinner,pinmux: > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > > + description: pinmux selector for each pin > > > + > > > > Why not just the standard "pinmux" property, as given in > > Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml > > I had it like this in my last post two years ago, but learned from > LinusW [1] that the generic pinmux property has a slightly different > meaning, and abusing it for just the pinmux index values would not match > the generic definition. > We *could* use the generic definition, but then this would include what's > in the "pins" property, like I sketched out in the cover letter, as an > alternative to this approach: > pinmux = <SUNXI_PIN(PB, 9, 2)>, <SUNXI_PIN(PB, 10, 2)>; > Where the SUNXI_PIN macro would combine the pin number and the pinmux into > one 32-bit cell. See the Apple GPIO DT nodes for an example. > This looks indeed nicer, but requires quite some rewrite of the existing > pinctrl driver, AFAICS. Sorry for taking so long to get back to this. Could we maybe add a generic replacement of the existing "function" property, which takes a string? Like "function-id" or "function-selector" that takes u32 (or u8). Then it could be one or the other. Not sure if the binding maintainers would accept this or not. I understand that we probably don't want the mux value combined with the pin number. ChenYu > [1] Previous reply from LinusW: > https://lore.kernel.org/linux-sunxi/CACRpkdbMc-Q6wjgsiddu6-tWC1dt2uFk+4LyerMdgFk2KRGK4w@xxxxxxxxxxxxxx/ > > > > > > required: > > > - pins > > > - function > > > > This section should be made to apply only to the existing > > compatibles? Maybe we could just split the files and have > > a clean slate for sun55i? > > Yeah, I couldn't find a good example how to make it *required* for one > compatible and *not allowed* for all the others. But creating a whole new > file is actually a good idea, as this also avoids adding another case to > the already quite indented if-else cascade. > > Cheers, > Andre > > > ChenYu > > > > > additionalProperties: false > > > > > > - "^vcc-p[a-ilm]-supply$": > > > + "^vcc-p[a-klm]-supply$": > > > description: > > > Power supplies for pin banks. > > > > > > @@ -156,6 +162,17 @@ allOf: > > > - interrupts > > > - interrupt-controller > > > > > > + - if: > > > + properties: > > > + compatible: > > > + enum: > > > + - allwinner,sun55i-a523-pinctrl > > > + > > > + then: > > > + properties: > > > + interrupts: > > > + minItems: 10 > > > + > > > - if: > > > properties: > > > compatible: > > > @@ -166,6 +183,7 @@ allOf: > > > properties: > > > interrupts: > > > minItems: 8 > > > + maxItems: 8 > > > > > > - if: > > > properties: > > > @@ -244,6 +262,7 @@ allOf: > > > - allwinner,sun8i-v3s-pinctrl > > > - allwinner,sun9i-a80-r-pinctrl > > > - allwinner,sun50i-h6-r-pinctrl > > > + - allwinner,sun55i-a523-r-pinctrl > > > > > > then: > > > properties: > > > -- > > > 2.46.2 > > > >
