Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 16 Jan 2023 16:35:59 +0200
From:   Abel Vesa <abel.vesa@linaro.org>
To:     Dario Binacchi <dario.binacchi@amarulasolutions.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc:     linux-kernel@vger.kernel.org, angelo@amarulasolutions.com,
        michael@amarulasolutions.com, tommaso.merciai@amarulasolutions.com,
        Chen-Yu Tsai <wenst@chromium.org>,
        linux-amarula@amarulasolutions.com, anthony@amarulasolutions.com,
        jagan@amarulasolutions.com, Abel Vesa <abelvesa@kernel.org>,
        Adam Ford <aford173@gmail.com>,
        Fabio Estevam <festevam@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Li Jun <jun.li@nxp.com>, Lucas Stach <l.stach@pengutronix.de>,
        Marek Vasut <marex@denx.de>,
        Markus Niebel <Markus.Niebel@ew.tq-group.com>,
        Michael Turquette <mturquette@baylibre.com>,
        NXP Linux Team <linux-imx@nxp.com>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Rob Herring <robh+dt@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Shawn Guo <shawnguo@kernel.org>,
        Stephen Boyd <sboyd@kernel.org>, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org
Subject: Re: [RFC PATCH v2 11/11] arm64: dts: imx8mn: add clocks description
Message-ID: <Y8VgzyNxGpqSvJ2b@linaro.org>
References: <20230101175740.1010258-1-dario.binacchi@amarulasolutions.com>
 <20230101175740.1010258-12-dario.binacchi@amarulasolutions.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230101175740.1010258-12-dario.binacchi@amarulasolutions.com>
Precedence: bulk

On 23-01-01 18:57:40, Dario Binacchi wrote:
> The patch creates a unique node for each clock in the imx8mn clock
> control module (CCM).
> 
> To ensure backwards compatibility it was not possible to separate the
> changes to the device tree from those applied to the clocks setup code.
> In doing so, all clocks are initialized from the device tree and the
> legacy setup code with hardwired parameters is removed.
> 
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> 
> ---
> 
> (no changes since v1)
> 
>  .../boot/dts/freescale/imx8mn-clocks.dtsi     | 1885 +++++++++++++++++
>  arch/arm64/boot/dts/freescale/imx8mn.dtsi     |   54 +-
>  drivers/clk/imx/clk-imx8mn.c                  |  714 ++-----
>  3 files changed, 2086 insertions(+), 567 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/freescale/imx8mn-clocks.dtsi
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn-clocks.dtsi b/arch/arm64/boot/dts/freescale/imx8mn-clocks.dtsi
> new file mode 100644
> index 000000000000..21e02ea996d0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mn-clocks.dtsi
> @@ -0,0 +1,1885 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Device Tree Source for imx8mn clock data
> + *
> + * Copyright (c) 2022 Amarula Solutions
> + *
> + * Dario Binacchi <dario.binacchi@amarulasolutions.com>
> + */
> +
> +/ {
> +	osc_32k: clock-osc-32k {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +		clock-output-names = "osc_32k";
> +	};
> +

[...]

> +
> +	clk_audio_pll2_bypass: clock-audio-pll2-bypass@14 {
> +		compatible = "fsl,imx8mn-mux-clock";
> +		#clock-cells = <0>;
> +		clocks = <&clk_audio_pll2>, <&clk_audio_pll2_ref_sel>;
> +		fsl,anatop = <&anatop 0x14>;
> +		fsl,bit-shift = <16>;
> +		fsl,set-rate-parent;

NACK. I'm sorry, but this creates a huge effort on maintaining the
bindings. Plus the vendor specific properties will keep increasing.

I don't think Rob and Krzysztof will be OK with this either.


> +		clock-output-names = "audio_pll2_bypass";
> +	};
> +
> +	clk_audio_pll2_out: clock-audio-pll2-out@14 {
> +		compatible = "fsl,imx8mn-gate-clock";
> +		#clock-cells = <0>;
> +		clocks = <&clk_audio_pll2_bypass>;
> +		fsl,anatop = <&anatop 0x14>;
> +		fsl,bit-shift = <13>;
> +		clock-output-names = "audio_pll2_out";
> +	};
> +

[...]

> -- 
> 2.32.0
>