| Message ID | 20201027104132.105485-5-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: ti: AM6 and J721e display dts changes | expand |
On 12:41-20201027, Tomi Valkeinen wrote: > The fixed clock for OV5640 is named 'clock' which is a very generic name > and easily leads to conflicts. I encountered this with a similarly named > fixed-clock node in k3-am654-evm-tc358876.dtso, which then overrode the > OV5640 fixed clock, causing OV5640 not to work when tc358876 overlay had > been loaded. > > Rename the node to 'fixed-clock-ov5640'. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > --- > arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts > index d12dd89f3405..6801dbddeac5 100644 > --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts > +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts > @@ -55,7 +55,7 @@ sw6 { > }; > }; > > - clk_ov5640_fixed: clock { > + clk_ov5640_fixed: fixed-clock-ov5640 { > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <24000000>; > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > I think you could post this independently as well. https://github.com/devicetree-org/devicetree-specification/blob/v0.3/source/devicetree-basics.rst#generic-names-recommendation There is a strong desire to use standard node names and clock is recommended. even though there are tons of fixed-clock compatible clocks in the kernel today, as of v5.10-rc1: $ git grep fixed-clock- arch/arm64/boot/dts/ $ git grep fixed-clock- arch/arm/boot/dts/ As a node name is not used. Do you want to see how other platforms are trying to resolve similar issues?
On 27/10/2020 14:39, Nishanth Menon wrote: > On 12:41-20201027, Tomi Valkeinen wrote: >> The fixed clock for OV5640 is named 'clock' which is a very generic name >> and easily leads to conflicts. I encountered this with a similarly named >> fixed-clock node in k3-am654-evm-tc358876.dtso, which then overrode the >> OV5640 fixed clock, causing OV5640 not to work when tc358876 overlay had >> been loaded. >> >> Rename the node to 'fixed-clock-ov5640'. >> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> >> --- >> arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts >> index d12dd89f3405..6801dbddeac5 100644 >> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts >> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts >> @@ -55,7 +55,7 @@ sw6 { >> }; >> }; >> >> - clk_ov5640_fixed: clock { >> + clk_ov5640_fixed: fixed-clock-ov5640 { >> compatible = "fixed-clock"; >> #clock-cells = <0>; >> clock-frequency = <24000000>; >> -- >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki >> > > I think you could post this independently as well. > > https://github.com/devicetree-org/devicetree-specification/blob/v0.3/source/devicetree-basics.rst#generic-names-recommendation > > There is a strong desire to use standard node names and > clock is recommended. even though there are tons of fixed-clock > compatible clocks in the kernel today, as of v5.10-rc1: > > $ git grep fixed-clock- arch/arm64/boot/dts/ > $ git grep fixed-clock- arch/arm/boot/dts/ > > As a node name is not used. Do you want to see how other > platforms are trying to resolve similar issues? There doesn't seem to be a standard: $ git grep -B2 fixed-clock arch/arm/boot/dts/ The node names are just about everything. Tomi
On 16:55-20201027, Tomi Valkeinen wrote: > On 27/10/2020 14:39, Nishanth Menon wrote: > > On 12:41-20201027, Tomi Valkeinen wrote: > >> The fixed clock for OV5640 is named 'clock' which is a very generic name > >> and easily leads to conflicts. I encountered this with a similarly named > >> fixed-clock node in k3-am654-evm-tc358876.dtso, which then overrode the > >> OV5640 fixed clock, causing OV5640 not to work when tc358876 overlay had > >> been loaded. > >> > >> Rename the node to 'fixed-clock-ov5640'. > >> > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> > >> --- > >> arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts > >> index d12dd89f3405..6801dbddeac5 100644 > >> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts > >> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts > >> @@ -55,7 +55,7 @@ sw6 { > >> }; > >> }; > >> > >> - clk_ov5640_fixed: clock { > >> + clk_ov5640_fixed: fixed-clock-ov5640 { > >> compatible = "fixed-clock"; > >> #clock-cells = <0>; > >> clock-frequency = <24000000>; > > > > I think you could post this independently as well. > > > > https://github.com/devicetree-org/devicetree-specification/blob/v0.3/source/devicetree-basics.rst#generic-names-recommendation > > > > There is a strong desire to use standard node names and > > clock is recommended. even though there are tons of fixed-clock > > compatible clocks in the kernel today, as of v5.10-rc1: > > > > $ git grep fixed-clock- arch/arm64/boot/dts/ > > $ git grep fixed-clock- arch/arm/boot/dts/ > > > > As a node name is not used. Do you want to see how other > > platforms are trying to resolve similar issues? > > There doesn't seem to be a standard: > > $ git grep -B2 fixed-clock arch/arm/boot/dts/ > > The node names are just about everything. Yeah - I just dont want us top be the one to go create yet another variant. You could make the node name stricter by adding something like the following to the yaml: Documentation/devicetree/bindings/clock/fixed-clock.yaml I dont see anything explicit here: https://github.com/devicetree-org/dt-schema/blob/master/schemas/clock/clock.yaml properties: nodename: pattern: "^(clock)(@[0-9a-f]+|-[0-9a-f]+)?$" Adding Mike and Stephen for thoughts..
diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts index d12dd89f3405..6801dbddeac5 100644 --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts @@ -55,7 +55,7 @@ sw6 { }; }; - clk_ov5640_fixed: clock { + clk_ov5640_fixed: fixed-clock-ov5640 { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <24000000>;
The fixed clock for OV5640 is named 'clock' which is a very generic name and easily leads to conflicts. I encountered this with a similarly named fixed-clock node in k3-am654-evm-tc358876.dtso, which then overrode the OV5640 fixed clock, causing OV5640 not to work when tc358876 overlay had been loaded. Rename the node to 'fixed-clock-ov5640'. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)