| Message ID | 20241022063158.5910-2-mjchen0829@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for nuvoton ma35d1 keypad controller | expand |
On Tue, Oct 22, 2024 at 06:31:57AM +0000, mjchen wrote: > From: mjchen <mjchen@nuvoton.com> > > Add YAML bindings for MA35D1 SoC keypad. > > Signed-off-by: mjchen <mjchen@nuvoton.com> > --- > .../bindings/input/nvt,ma35d1-keypad.yaml | 88 +++++++++++++++++++ > 1 file changed, 88 insertions(+) > create mode 100755 Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml > Please run scripts/checkpatch.pl and fix reported warnings. Then please run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. > diff --git a/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml > new file mode 100755 > index 000000000000..3d9fc26cc132 > --- /dev/null > +++ b/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml Filename based on compatible. There is no nvt prefix. Entire filename is somehowdifferent than compatible. > @@ -0,0 +1,88 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/input/nvt,ma35d1-keypad.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NVT MA35D1 Keypad NVT? Nuvoton? > + > +maintainers: > + - Ming-jen Chen <mjchen0829@gmail.com> > + > +allOf: > + - $ref: /schemas/input/matrix-keymap.yaml# > + > +properties: > + compatible: > + const: nuvoton,ma35d1-kpi > + > + debounce-period: > + $ref: /schemas/types.yaml#/definitions/uint32 Missing vendor prefix... or why are you not using existing properties? > + description: | > + key debounce period select select? or clock cycles? I don't understand this. Say something useful here. > + 0 = 0 clock > + 1 = 0 clock > + 2 = 0 clock Heh? So this is just 0 > + 3 = 8 clocks This is 8 > + 4 = 16 clocks 16, not 4 > + 5 = 32 clocks > + 6 = 64 clocks > + 7 = 128 clocks > + 8 = 256 clocks > + 9 = 512 clocks > + 10 = 1024 clocks > + 11 = 2048 clocks > + 12 = 4096 clocks > + 13 = 8192 clocks Use proper enum > + > + per-scale: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Row Scan Cycle Pre-scale Value (1 to 256). Missing constraints > + > + per-scalediv: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: Per-scale divider (1 to 256). Missing constraints Both properties are unexpected... aren't you duplicating existing properties? > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + - linux,keymap > + - debounce-period > + - per-scale > + - per-scalediv > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include <dt-bindings/input/input.h> > + keypad: keypad@404A0000 { Lowercase hex and drop the unused label > + compatible = "nuvoton,ma35d1-kpi"; > + reg = <0x404A0000 0x10000>; Lowercase hex Best regards, Krzysztof
On 22/10/2024 08:31, mjchen wrote: > From: mjchen <mjchen@nuvoton.com> > > Add YAML bindings for MA35D1 SoC keypad. > > Signed-off-by: mjchen <mjchen@nuvoton.com> Don't use your login name as name, but use your known identity or full legal name. Best regards, Krzysztof
On 2024/10/23 下午 04:40, Krzysztof Kozlowski wrote: > On Tue, Oct 22, 2024 at 06:31:57AM +0000, mjchen wrote: >> From: mjchen <mjchen@nuvoton.com> >> >> Add YAML bindings for MA35D1 SoC keypad. >> >> Signed-off-by: mjchen <mjchen@nuvoton.com> >> --- >> .../bindings/input/nvt,ma35d1-keypad.yaml | 88 +++++++++++++++++++ >> 1 file changed, 88 insertions(+) >> create mode 100755 Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml >> > Please run scripts/checkpatch.pl and fix reported warnings. Then please > run 'scripts/checkpatch.pl --strict' and (probably) fix more warnings. > Some warnings can be ignored, especially from --strict run, but the code > here looks like it needs a fix. Feel free to get in touch if the warning > is not clear. Sorry, I will remember to run checkpatch.pl before uploading the subsequent patches and fix all errors and warnings >> diff --git a/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml >> new file mode 100755 >> index 000000000000..3d9fc26cc132 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml > Filename based on compatible. There is no nvt prefix. Entire filename is > somehowdifferent than compatible. I will modify it to: nuvoton,ma35d1-keypad.yaml >> @@ -0,0 +1,88 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/input/nvt,ma35d1-keypad.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: NVT MA35D1 Keypad > NVT? Nuvoton? I will change NVT to Nuvoton > >> + >> +maintainers: >> + - Ming-jen Chen <mjchen0829@gmail.com> >> + >> +allOf: >> + - $ref: /schemas/input/matrix-keymap.yaml# >> + >> +properties: >> + compatible: >> + const: nuvoton,ma35d1-kpi >> + >> + debounce-period: >> + $ref: /schemas/types.yaml#/definitions/uint32 > Missing vendor prefix... or why are you not using existing properties? > >> + description: | >> + key debounce period select > select? or clock cycles? I don't understand this. Say something useful > here. > > >> + 0 = 0 clock >> + 1 = 0 clock >> + 2 = 0 clock > Heh? So this is just 0 > >> + 3 = 8 clocks > This is 8 > >> + 4 = 16 clocks > 16, not 4 > >> + 5 = 32 clocks >> + 6 = 64 clocks >> + 7 = 128 clocks >> + 8 = 256 clocks >> + 9 = 512 clocks >> + 10 = 1024 clocks >> + 11 = 2048 clocks >> + 12 = 4096 clocks >> + 13 = 8192 clocks > Use proper enum I will update the definition to specify the debounce period in terms of keypad IP clock cycles, as follow: nuvoton,debounce-period: type: integer enum: [0, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13] description: | Key debounce period select, specified in terms of keypad IP clock cycles. This value corresponds to the register setting for the keypad interface. The following values indicate the debounce time: - 0 = 0 clock cycles (no debounce) - 3 = 8 clock cycles - 4 = 16 clock cycles - 5 = 32 clock cycles - 6 = 64 clock cycles - 7 = 128 clock cycles - 8 = 256 clock cycles - 9 = 512 clock cycles - 10 = 1024 clock cycles - 11 = 2048 clock cycles - 12 = 4096 clock cycles - 13 = 8192 clock cycles > > >> + >> + per-scale: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: Row Scan Cycle Pre-scale Value (1 to 256). > Missing constraints > >> + >> + per-scalediv: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: Per-scale divider (1 to 256). > Missing constraints > > Both properties are unexpected... aren't you duplicating existing > properties? pre-scale: This value configures the IC register for the row scan cycle pre-scaling, with valid values ranging from 1 to 256 per-scalediv:(I will change pre-scalediv to pre-scale-div) This will describe its role in setting the divisor for the row scan cycle pre-scaling, allowing for finer control over the keypad scanning frequency I will change it to the following content: nuvoton,pre-scale: type: uint32 description: | Row Scan Cycle Pre-scale Value, used to pre-scale the row scan cycle. The valid range is from 1 to 256. minimum: 1 maximum: 256 nuvoton,pre-scale-div: type: uint32 description: | Divider for the pre-scale value, further adjusting the scan frequency for the keypad. minimum: 1 maximum: 256 > >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - clocks >> + - linux,keymap >> + - debounce-period >> + - per-scale >> + - per-scalediv >> + >> +unevaluatedProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/input/input.h> >> + keypad: keypad@404A0000 { > Lowercase hex and drop the unused label I will modify it to: keypad@404a0000 { > >> + compatible = "nuvoton,ma35d1-kpi"; >> + reg = <0x404A0000 0x10000>; > Lowercase hex I will modify it to: reg = <0x404a0000 0x10000> > > Best regards, > Krzysztof Thank you for your guidance! I look forward to your further comments. Best regards, Ming-Jen Chen
On 25/10/2024 07:36, Ming-Jen Chen wrote: >> >>> + 0 = 0 clock >>> + 1 = 0 clock >>> + 2 = 0 clock >> Heh? So this is just 0 >> >>> + 3 = 8 clocks >> This is 8 >> >>> + 4 = 16 clocks >> 16, not 4 >> >>> + 5 = 32 clocks >>> + 6 = 64 clocks >>> + 7 = 128 clocks >>> + 8 = 256 clocks >>> + 9 = 512 clocks >>> + 10 = 1024 clocks >>> + 11 = 2048 clocks >>> + 12 = 4096 clocks >>> + 13 = 8192 clocks >> Use proper enum > I will update the definition to specify the debounce period in terms of > keypad IP clock cycles, as follow: > > nuvoton,debounce-period: > type: integer > enum: [0, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13] > description: | > Key debounce period select, specified in terms of keypad IP > clock cycles. > This value corresponds to the register setting for the keypad > interface. > The following values indicate the debounce time: > - 0 = 0 clock cycles (no debounce) > - 3 = 8 clock cycles > - 4 = 16 clock cycles > - 5 = 32 clock cycles > - 6 = 64 clock cycles > - 7 = 128 clock cycles > - 8 = 256 clock cycles > - 9 = 512 clock cycles > - 10 = 1024 clock cycles > - 11 = 2048 clock cycles > - 12 = 4096 clock cycles > - 13 = 8192 clock cycles No. 0, 8, 16, 32 , 64 etc. >> >> >>> + >>> + per-scale: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: Row Scan Cycle Pre-scale Value (1 to 256). >> Missing constraints >> >>> + >>> + per-scalediv: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: Per-scale divider (1 to 256). >> Missing constraints >> >> Both properties are unexpected... aren't you duplicating existing >> properties? > pre-scale: > This value configures the IC register for the row scan cycle > pre-scaling, with valid values ranging from 1 to 256 > per-scalediv:(I will change pre-scalediv to pre-scale-div) Please look for matching existing properties first. > This will describe its role in setting the divisor for the row scan > cycle pre-scaling, allowing for finer control over the keypad scanning > frequency > > I will change it to the following content: > nuvoton,pre-scale: > type: uint32 > description: | > Row Scan Cycle Pre-scale Value, used to pre-scale the row scan > cycle. The valid range is from 1 to 256. > minimum: 1 > maximum: 256 > > nuvoton,pre-scale-div: > type: uint32 > description: | > Divider for the pre-scale value, further adjusting the scan > frequency for the keypad. > minimum: 1 > maximum: 256 Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml new file mode 100755 index 000000000000..3d9fc26cc132 --- /dev/null +++ b/Documentation/devicetree/bindings/input/nvt,ma35d1-keypad.yaml @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/nvt,ma35d1-keypad.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NVT MA35D1 Keypad + +maintainers: + - Ming-jen Chen <mjchen0829@gmail.com> + +allOf: + - $ref: /schemas/input/matrix-keymap.yaml# + +properties: + compatible: + const: nuvoton,ma35d1-kpi + + debounce-period: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + key debounce period select + 0 = 0 clock + 1 = 0 clock + 2 = 0 clock + 3 = 8 clocks + 4 = 16 clocks + 5 = 32 clocks + 6 = 64 clocks + 7 = 128 clocks + 8 = 256 clocks + 9 = 512 clocks + 10 = 1024 clocks + 11 = 2048 clocks + 12 = 4096 clocks + 13 = 8192 clocks + + per-scale: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Row Scan Cycle Pre-scale Value (1 to 256). + + per-scalediv: + $ref: /schemas/types.yaml#/definitions/uint32 + description: Per-scale divider (1 to 256). + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + - clocks + - linux,keymap + - debounce-period + - per-scale + - per-scalediv + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/input/input.h> + keypad: keypad@404A0000 { + compatible = "nuvoton,ma35d1-kpi"; + reg = <0x404A0000 0x10000>; + interrupts = <79>; + clocks = <&clk>; + keypad,num-rows = <2>; + keypad,num-columns = <2>; + + linux,keymap = < + MATRIX_KEY(0, 0, KEY_ENTER) + MATRIX_KEY(0, 1, KEY_ENTER) + MATRIX_KEY(1, 0, KEY_SPACE) + MATRIX_KEY(1, 1, KEY_Z) + >; + + debounce-period = <1>; + per-scale = <1>; + per-scalediv = <24>; + };