| Message ID | 20210922105433.11744-4-pali@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | serial: mvebu-uart: Support for higher baudrates | expand |
On Wed, Sep 22, 2021 at 5:56 AM Pali Rohár <pali@kernel.org> wrote: > > This change adds DT bindings documentation for device nodes with compatible > string "marvell,armada-3700-uart-clock". Please resend to the DT list so that checks run and this gets reviewed in a timely manner. > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > .../bindings/clock/armada3700-uart-clock.yaml | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > diff --git a/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > new file mode 100644 > index 000000000000..5bdb23e0ba3e > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > @@ -0,0 +1,57 @@ > +# SPDX-License-Identifier: GPL-2.0 Dual license. checkpatch will tell you which ones. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/marvell,armada-3700-uart-clock# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > +title: Marvell Armada 3720 UART clocks > + > +properties: > + compatible: > + const: marvell,armada-3700-uart-clock > + > + reg: > + items: > + - description: UART Clock Control Register > + - description: UART 2 Baud Rate Divisor Register > + > + clocks: > + description: | > + List of parent clocks suitable for UART from following set: > + "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal" > + UART clock can use one from this set and when more are provided > + then kernel would choose and configure the most suitable one. > + It is suggest to specify at least one TBG clock to achieve > + baudrates above 230400 and also to specify clock which bootloader > + used for UART (most probably xtal) for smooth boot log on UART. > + > + clock-names: > + items: > + - const: TBG-A-P > + - const: TBG-B-P > + - const: TBG-A-S > + - const: TBG-B-S > + - const: xtal > + minItems: 1 > + maxItems: 5 Don't need maxItems equal to length of 'items'. > + > + '#clock-cells': > + const: 1 > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - '#clock-cells' > + > +additionalProperties: false > + > +examples: > + - | > + uartclk: uartclk@12000 { clock-controller@12010 > + compatible = "marvell,armada-3700-uart-clock"; > + reg = <0x12010 0x4>, <0x12210 0x4>; However, looks like this is part of some other block. The whole block needs a binding (or at least the parent and whatever sub-functions you know about). > + clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>, <&tbg 3>, <&xtalclk>; > + clock-names = "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal"; > + #clock-cells = <1>; > + }; > -- > 2.20.1 >
On Monday 27 September 2021 15:17:59 Rob Herring wrote: > On Wed, Sep 22, 2021 at 5:56 AM Pali Rohár <pali@kernel.org> wrote: > > > > This change adds DT bindings documentation for device nodes with compatible > > string "marvell,armada-3700-uart-clock". > > Please resend to the DT list so that checks run and this gets reviewed > in a timely manner. OK > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > .../bindings/clock/armada3700-uart-clock.yaml | 57 +++++++++++++++++++ > > 1 file changed, 57 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > > > diff --git a/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > new file mode 100644 > > index 000000000000..5bdb23e0ba3e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > @@ -0,0 +1,57 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > Dual license. checkpatch will tell you which ones. OK > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/clock/marvell,armada-3700-uart-clock# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > +title: Marvell Armada 3720 UART clocks > > + > > +properties: > > + compatible: > > + const: marvell,armada-3700-uart-clock > > + > > + reg: > > + items: > > + - description: UART Clock Control Register > > + - description: UART 2 Baud Rate Divisor Register > > + > > + clocks: > > + description: | > > + List of parent clocks suitable for UART from following set: > > + "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal" > > + UART clock can use one from this set and when more are provided > > + then kernel would choose and configure the most suitable one. > > + It is suggest to specify at least one TBG clock to achieve > > + baudrates above 230400 and also to specify clock which bootloader > > + used for UART (most probably xtal) for smooth boot log on UART. > > + > > + clock-names: > > + items: > > + - const: TBG-A-P > > + - const: TBG-B-P > > + - const: TBG-A-S > > + - const: TBG-B-S > > + - const: xtal > > + minItems: 1 > > + maxItems: 5 > > Don't need maxItems equal to length of 'items'. OK > > + > > + '#clock-cells': > > + const: 1 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + - '#clock-cells' > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + uartclk: uartclk@12000 { > > clock-controller@12010 > > > + compatible = "marvell,armada-3700-uart-clock"; > > + reg = <0x12010 0x4>, <0x12210 0x4>; > > However, looks like this is part of some other block. Yes, it is part of UART block. Explanation is in commit message of patch 2/6. And also discussed here: https://lore.kernel.org/linux-serial/20210812200804.i4kbcs6ut27mapd3@pali/ > The whole block > needs a binding (or at least the parent and whatever sub-functions you > know about). Whole UART block has already binding. Clock driver just needs access to these clock bits of these two registers which are in UART block. HW designers decided that clock which drives UART2 has configuration in UART1 address space. As explained in commit message of patch 2/6 there is no easy way how to deal with it in DTS backward compatible way. So clock and UART driver shares mutex for accessing these two shared registers, and these two registers are defined in all 3 DT nodes: UART1, UART2 and UART-clock. > > + clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>, <&tbg 3>, <&xtalclk>; > > + clock-names = "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal"; > > + #clock-cells = <1>; > > + }; > > -- > > 2.20.1 > >
On Monday 27 September 2021 15:17:59 Rob Herring wrote: > On Wed, Sep 22, 2021 at 5:56 AM Pali Rohár <pali@kernel.org> wrote: > > > > This change adds DT bindings documentation for device nodes with compatible > > string "marvell,armada-3700-uart-clock". > > Please resend to the DT list so that checks run and this gets reviewed > in a timely manner. > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > .../bindings/clock/armada3700-uart-clock.yaml | 57 +++++++++++++++++++ > > 1 file changed, 57 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > > > diff --git a/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > new file mode 100644 > > index 000000000000..5bdb23e0ba3e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > @@ -0,0 +1,57 @@ > > +# SPDX-License-Identifier: GPL-2.0 > > Dual license. checkpatch will tell you which ones. Did not tell me :-( $ ./scripts/checkpatch.pl -f Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml Traceback (most recent call last): File "scripts/spdxcheck.py", line 6, in <module> from ply import lex, yacc ModuleNotFoundError: No module named 'ply' total: 0 errors, 0 warnings, 57 lines checked Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml has no obvious style problems and is ready for submission. Huh, Perl needs Python?? Anyway... $ sudo apt install python3-ply ... $ ./scripts/checkpatch.pl -f Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml Traceback (most recent call last): File "scripts/spdxcheck.py", line 10, in <module> import git ModuleNotFoundError: No module named 'git' total: 0 errors, 0 warnings, 57 lines checked Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml has no obvious style problems and is ready for submission. Second attempt... $ sudo apt install python3-git ... $ ./scripts/checkpatch.pl -f Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml total: 0 errors, 0 warnings, 57 lines checked Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml has no obvious style problems and is ready for submission. And no error :-(
On Mon, Sep 27, 2021 at 3:45 PM Pali Rohár <pali@kernel.org> wrote: > > On Monday 27 September 2021 15:17:59 Rob Herring wrote: > > On Wed, Sep 22, 2021 at 5:56 AM Pali Rohár <pali@kernel.org> wrote: > > > > > > This change adds DT bindings documentation for device nodes with compatible > > > string "marvell,armada-3700-uart-clock". > > > > Please resend to the DT list so that checks run and this gets reviewed > > in a timely manner. > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > --- > > > .../bindings/clock/armada3700-uart-clock.yaml | 57 +++++++++++++++++++ > > > 1 file changed, 57 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > > new file mode 100644 > > > index 000000000000..5bdb23e0ba3e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > > @@ -0,0 +1,57 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > > Dual license. checkpatch will tell you which ones. > > Did not tell me :-( > > $ ./scripts/checkpatch.pl -f Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > Traceback (most recent call last): > File "scripts/spdxcheck.py", line 6, in <module> > from ply import lex, yacc > ModuleNotFoundError: No module named 'ply' > total: 0 errors, 0 warnings, 57 lines checked > > Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml has no obvious style problems and is ready for submission. > > Huh, Perl needs Python?? Anyway... > > $ sudo apt install python3-ply > ... > > $ ./scripts/checkpatch.pl -f Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > Traceback (most recent call last): > File "scripts/spdxcheck.py", line 10, in <module> > import git > ModuleNotFoundError: No module named 'git' > total: 0 errors, 0 warnings, 57 lines checked > > Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml has no obvious style problems and is ready for submission. > > Second attempt... > > $ sudo apt install python3-git > ... > > $ ./scripts/checkpatch.pl -f Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > total: 0 errors, 0 warnings, 57 lines checked > > Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml has no obvious style problems and is ready for submission. > > And no error :-( Looks like file mode requires '--strict' to enable while patch mode doesn't. Rob
On Mon, Sep 27, 2021 at 3:34 PM Pali Rohár <pali@kernel.org> wrote: > > On Monday 27 September 2021 15:17:59 Rob Herring wrote: > > On Wed, Sep 22, 2021 at 5:56 AM Pali Rohár <pali@kernel.org> wrote: > > > > > > This change adds DT bindings documentation for device nodes with compatible > > > string "marvell,armada-3700-uart-clock". > > > > Please resend to the DT list so that checks run and this gets reviewed > > in a timely manner. > > OK > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > --- > > > .../bindings/clock/armada3700-uart-clock.yaml | 57 +++++++++++++++++++ > > > 1 file changed, 57 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > > new file mode 100644 > > > index 000000000000..5bdb23e0ba3e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml > > > @@ -0,0 +1,57 @@ > > > +# SPDX-License-Identifier: GPL-2.0 > > > > Dual license. checkpatch will tell you which ones. > > OK > > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/clock/marvell,armada-3700-uart-clock# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > +title: Marvell Armada 3720 UART clocks > > > + > > > +properties: > > > + compatible: > > > + const: marvell,armada-3700-uart-clock > > > + > > > + reg: > > > + items: > > > + - description: UART Clock Control Register > > > + - description: UART 2 Baud Rate Divisor Register > > > + > > > + clocks: > > > + description: | > > > + List of parent clocks suitable for UART from following set: > > > + "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal" > > > + UART clock can use one from this set and when more are provided > > > + then kernel would choose and configure the most suitable one. > > > + It is suggest to specify at least one TBG clock to achieve > > > + baudrates above 230400 and also to specify clock which bootloader > > > + used for UART (most probably xtal) for smooth boot log on UART. > > > + > > > + clock-names: > > > + items: > > > + - const: TBG-A-P > > > + - const: TBG-B-P > > > + - const: TBG-A-S > > > + - const: TBG-B-S > > > + - const: xtal > > > + minItems: 1 > > > + maxItems: 5 > > > > Don't need maxItems equal to length of 'items'. > > OK > > > > + > > > + '#clock-cells': > > > + const: 1 > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - clocks > > > + - clock-names > > > + - '#clock-cells' > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + uartclk: uartclk@12000 { > > > > clock-controller@12010 > > > > > + compatible = "marvell,armada-3700-uart-clock"; > > > + reg = <0x12010 0x4>, <0x12210 0x4>; > > > > However, looks like this is part of some other block. > > Yes, it is part of UART block. > > Explanation is in commit message of patch 2/6. > > And also discussed here: > https://lore.kernel.org/linux-serial/20210812200804.i4kbcs6ut27mapd3@pali/ > > > The whole block > > needs a binding (or at least the parent and whatever sub-functions you > > know about). > > Whole UART block has already binding. Clock driver just needs access to > these clock bits of these two registers which are in UART block. HW > designers decided that clock which drives UART2 has configuration in > UART1 address space. As explained in commit message of patch 2/6 there > is no easy way how to deal with it in DTS backward compatible way. So > clock and UART driver shares mutex for accessing these two shared > registers, and these two registers are defined in all 3 DT nodes: UART1, > UART2 and UART-clock. That's awful. I guess I don't have a better suggestion. Rob
diff --git a/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml new file mode 100644 index 000000000000..5bdb23e0ba3e --- /dev/null +++ b/Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/marvell,armada-3700-uart-clock# +$schema: http://devicetree.org/meta-schemas/core.yaml# +title: Marvell Armada 3720 UART clocks + +properties: + compatible: + const: marvell,armada-3700-uart-clock + + reg: + items: + - description: UART Clock Control Register + - description: UART 2 Baud Rate Divisor Register + + clocks: + description: | + List of parent clocks suitable for UART from following set: + "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal" + UART clock can use one from this set and when more are provided + then kernel would choose and configure the most suitable one. + It is suggest to specify at least one TBG clock to achieve + baudrates above 230400 and also to specify clock which bootloader + used for UART (most probably xtal) for smooth boot log on UART. + + clock-names: + items: + - const: TBG-A-P + - const: TBG-B-P + - const: TBG-A-S + - const: TBG-B-S + - const: xtal + minItems: 1 + maxItems: 5 + + '#clock-cells': + const: 1 + +required: + - compatible + - reg + - clocks + - clock-names + - '#clock-cells' + +additionalProperties: false + +examples: + - | + uartclk: uartclk@12000 { + compatible = "marvell,armada-3700-uart-clock"; + reg = <0x12010 0x4>, <0x12210 0x4>; + clocks = <&tbg 0>, <&tbg 1>, <&tbg 2>, <&tbg 3>, <&xtalclk>; + clock-names = "TBG-A-P", "TBG-B-P", "TBG-A-S", "TBG-B-S", "xtal"; + #clock-cells = <1>; + };
This change adds DT bindings documentation for device nodes with compatible string "marvell,armada-3700-uart-clock". Signed-off-by: Pali Rohár <pali@kernel.org> --- .../bindings/clock/armada3700-uart-clock.yaml | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/armada3700-uart-clock.yaml