On Tue, Sep 19, 2023 at 09:47:52AM +0200, Michal Simek wrote: > > > On 9/18/23 16:56, Laurent Pinchart wrote: > > Hi Michal, > > > > Thank you for the patch. > > > > On Mon, Sep 18, 2023 at 02:41:12PM +0200, Michal Simek wrote: > >> Character '_' not recommended in node name. Use '-' instead. > >> Pretty much run seds below for node names. > >> s/zynqmp_ipi/zynqmp-ipi/ > >> s/nvmem_firmware/nvmem-firmware/ > >> s/soc_revision/soc-revision/ > >> s/si5335_/si5335-/ > >> > >> Signed-off-by: Michal Simek <michal.simek@xxxxxxx> > > > > The si5335 nodes may be better named after the clock name instead of the > > component type, but that's nitpicking. > > I don't know what's the guidance on this. fixed-clock.yaml is using generic > "clock" name. I have no problem to do it if this is recommended way to go. > > > >> --- > >> > >> arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts | 4 ++-- > >> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 6 +++--- > >> 2 files changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts > >> index d0091d3cb764..52f998c22538 100644 > >> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts > >> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts > >> @@ -123,13 +123,13 @@ ina226 { > >> io-channels = <&u35 0>, <&u35 1>, <&u35 2>, <&u35 3>; > >> }; > >> > >> - si5335_0: si5335_0 { /* clk0_usb - u23 */ > >> + si5335_0: si5335-0 { /* clk0_usb - u23 */ > >> compatible = "fixed-clock"; > >> #clock-cells = <0>; > >> clock-frequency = <26000000>; > >> }; > >> > >> - si5335_1: si5335_1 { /* clk1_dp - u23 */ > >> + si5335_1: si5335-1 { /* clk1_dp - u23 */ > >> compatible = "fixed-clock"; > >> #clock-cells = <0>; > >> clock-frequency = <27000000>; > >> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > >> index b61fc99cd911..e50e95cbe817 100644 > >> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > >> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > >> @@ -129,7 +129,7 @@ rproc_1_fw_image: memory@3ef00000 { > >> }; > >> }; > >> > >> - zynqmp_ipi: zynqmp_ipi { > >> + zynqmp_ipi: zynqmp-ipi { > >> bootph-all; > >> compatible = "xlnx,zynqmp-ipi-mailbox"; > >> interrupt-parent = <&gic>; > >> @@ -194,12 +194,12 @@ zynqmp_power: zynqmp-power { > >> mbox-names = "tx", "rx"; > >> }; > >> > >> - nvmem_firmware { > >> + nvmem-firmware { > >> compatible = "xlnx,zynqmp-nvmem-fw"; > >> #address-cells = <1>; > >> #size-cells = <1>; > >> > >> - soc_revision: soc_revision@0 { > >> + soc_revision: soc-revision@0 { > > > > Unless I'm mistaken, this will change the userspace API, as it changes > > the nvmem cell name. Is it an issue ? > > Based on > https://docs.kernel.org/driver-api/nvmem.html#userspace-binary-interface > > The only interface to user space is via nvmem file which has all of them > together. And reference to this node is the same if used inside kernel itself. > That's why I think there is no change in connection to user space API from nvmem > side. Of course entry is listed differently if you parse DT names. Ah my bad. It should be fine then, as long as nobody in the kernel calls nvmem_cell_get("soc_revision"), which I didn't find any occurrence of. -- Regards, Laurent Pinchart