[PATCH v11 3/5] dt-bindings: clock: meson: add A1 PLL and Peripherals clkcs bindings
Dmitry Rokosov
ddrokosov at sberdevices.ru
Mon Mar 27 06:19:27 PDT 2023
On Mon, Mar 27, 2023 at 02:03:25PM +0200, neil.armstrong at linaro.org wrote:
> On 27/03/2023 13:39, Jerome Brunet wrote:
> >
> > On Mon 27 Mar 2023 at 13:51, Dmitry Rokosov <ddrokosov at sberdevices.ru> wrote:
> >
> > > On Mon, Mar 27, 2023 at 11:51:21AM +0200, Jerome Brunet wrote:
> > > >
> > > > On Tue 21 Mar 2023 at 22:30, Dmitry Rokosov <ddrokosov at sberdevices.ru> wrote:
> > > >
> > > > > Add the documentation for Amlogic A1 PLL and Amlogic A1 Peripherals
> > > > > clock drivers.
> > > > > Introduce Amlogic A1 PLL and Amlogic A1 Peripherals device tree
> > > > > bindings and include them to MAINTAINERS.
> > > > >
> > > > > Signed-off-by: Jian Hu <jian.hu at amlogic.com>
> > > > > Signed-off-by: Dmitry Rokosov <ddrokosov at sberdevices.ru>
> > > > > ---
> > > > > .../bindings/clock/amlogic,a1-clkc.yaml | 73 +++++++++++
> > > > > .../bindings/clock/amlogic,a1-pll-clkc.yaml | 59 +++++++++
> > > > > MAINTAINERS | 1 +
> > > > > include/dt-bindings/clock/amlogic,a1-clkc.h | 113 ++++++++++++++++++
> > > > > .../dt-bindings/clock/amlogic,a1-pll-clkc.h | 21 ++++
> > > > > 5 files changed, 267 insertions(+)
> > > > > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> > > > > create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-pll-clkc.yaml
> > > >
> > > > There is two drivers (and 2 independent patches). There should be 2
> > > > bindings patches as well.
> > > >
> > >
> > > Before, in previous versions I had two versions, but it wasn't bisectable
> > > approach.
> >
> > You are confusing bisectable and Rob's robot. Splitting patches is more
> > that likely to help bisect (and patches backport) - not the other way around.
> >
> > > a1-clkc schema depends on a1-pll-clkc headers and vice versa.
> > > It means dt schemas checkers will show us failure if we split them into two
> > > patchsets.
> >
> > Only because you are patches are not upstream yet ...
> >
> > > I know, that we can use raw digits instead of CLKID names, but IMO it doesn't
> > > look like production schema and it requires one more patchset above the
> > > series with proper CLKID definitons usage and proper header including.
> > >
> > > BTW, there is an example of Rob's test bot failure found in the previous
> > > v10 patch series due to chicken or the egg problem.
> > > https://lore.kernel.org/all/167769997208.7087.5344356236212731922.robh@kernel.org/
> > >
> > > Please advise what's the best practice to resolve that..
> >
> > Don't use the header in your example would solve the problem and
> > still be correct DT wise.
> >
> > The examples are just examples, they are not required to actually
> > matches a real HW, as far as I know.
>
> Exact, you can use fake lables instead of defined:
>
> <&clkc_pll CLKID_FCLK_DIV2>,
>
> =>
> remove "#include <dt-bindings/clock/amlogic,a1-pll-clkc.h>"
>
> <&clkc_pll_fclk_div2>,
>
> is perfectly ok and will permit have 2 separate patches.
>
> The dependency is only if you have a common yaml file for
> both bindings files, but this is not the case here.
Simple removal of "#include <dt-bindings/clock/amlogic,a1-pll-clkc.h>"
header doesn't work, dt_binding_check make rule is failed:
Error: Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dts:28.37-38 syntax error
FATAL ERROR: Unable to parse input tree
It happens, because 'dt_binding_check' generates simple dts example and
tries to compile it:
cat Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dts
===
/dts-v1/;
/plugin/; // silence any missing phandle references
/{
compatible = "foo";
model = "foo";
#address-cells = <1>;
#size-cells = <1>;
example-0 {
#address-cells = <1>;
#size-cells = <1>;
apb {
#address-cells = <2>;
#size-cells = <2>;
clock-controller at 800 {
compatible = "amlogic,a1-clkc";
reg = <0 0x800 0 0x104>;
#clock-cells = <1>;
clocks = <&clkc_pll CLKID_FCLK_DIV2>,
<&clkc_pll CLKID_FCLK_DIV3>,
<&clkc_pll CLKID_FCLK_DIV5>,
<&clkc_pll CLKID_FCLK_DIV7>,
<&clkc_pll CLKID_HIFI_PLL>,
<&xtal>;
clock-names = "fclk_div2", "fclk_div3",
"fclk_div5", "fclk_div7",
"hifi_pll", "xtal";
};
};
};
};
===
As you can see, header is required.
But looks like, dt binding checker is happy with the fake references hack :)
Below there is generated example dts:
cat Documentation/devicetree/bindings/clock/amlogic,a1-clkc.example.dts
===
/dts-v1/;
/plugin/; // silence any missing phandle references
/{
compatible = "foo";
model = "foo";
#address-cells = <1>;
#size-cells = <1>;
example-0 {
#address-cells = <1>;
#size-cells = <1>;
apb {
#address-cells = <2>;
#size-cells = <2>;
clock-controller at 800 {
compatible = "amlogic,a1-clkc";
reg = <0 0x800 0 0x104>;
#clock-cells = <1>;
clocks = <&clkc_pll_fclk_div2>,
<&clkc_pll_fclk_div3>,
<&clkc_pll_fclk_div5>,
<&clkc_pll_fclk_div7>,
<&clkc_pll_hifi_pll>,
<&xtal>;
clock-names = "fclk_div2", "fclk_div3",
"fclk_div5", "fclk_div7",
"hifi_pll", "xtal";
};
};
};
};
===
Yep, we are able to cheat dt checkers, but we don't help dt developers
with such example.
May be, it's better to prepare two patches in such hierarchy:
1) A1 PLL clkc bindings with fake references without clkc headers
2) A1 clkc bindings with real CLKID bindings + A1 PLL clkc bindings fix
with real CLKID A1 clkc bindings + header.
The such approach resolves DT checkers failures and split DT bindings
into two patchsets.
[...]
--
Thank you,
Dmitry
More information about the linux-arm-kernel
mailing list