linux-kernel - Re: [PATCH v2 4/4] arm64: dts: mt7622: group clock DT nodes into separate DT file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4a57ab2f-03b0-3c68-23cd-ed8253978ed7@gmail.com>
Date:   Mon, 29 May 2017 17:09:47 +0200
From:   Matthias Brugger <matthias.bgg@...il.com>
To:     sean.wang@...iatek.com, robh+dt@...nel.org, mark.rutland@....com,
        devicetree@...r.kernel.org, linux-mediatek@...ts.infradead.org
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] arm64: dts: mt7622: group clock DT nodes into
 separate DT file



On 29/05/17 14:56, sean.wang@...iatek.com wrote:
> From: Sean Wang <sean.wang@...iatek.com>
> 
> For more readability and maintenance, all the clock related DT
> nodes for mt7622 SoC are grouped into a separate DT file. And
> currently mt7622-clock.dtsi only includes the fixed clock for
> uart0 and the oscillator which would be extended after MT7622
> clock driver is introduced.
> 

We've not done this for any other Mediatek SoC at the moment.
Do you expect the m7622 to have more clock nodes then for example mt7623?
Apart from that this clock nodes are just dummy clocks which should 
disappear as soon as the clock driver got merged.
Last point, you break bisectability if add a clock node in a patch 
*after* the consumer was added.

So please add the clock nodes to the dtsi as we did up to now. If in the 
future we see that mt7622 has a whole bunch of clock nodes, we can still 
abstract them in an external dtsi file.

Thanks,
Matthias

> Signed-off-by: Sean Wang <sean.wang@...iatek.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt7622-clock.dtsi | 20 ++++++++++++++++++++
>   arch/arm64/boot/dts/mediatek/mt7622.dtsi       |  1 +
>   2 files changed, 21 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7622-clock.dtsi
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt7622-clock.dtsi b/arch/arm64/boot/dts/mediatek/mt7622-clock.dtsi
> new file mode 100644
> index 0000000..264421d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7622-clock.dtsi
> @@ -0,0 +1,20 @@
> +/*
> + * Copyright (c) 2017 MediaTek Inc.
> + * Author: Sean Wang <sean.wang@...iatek.com>
> + *
> + * SPDX-License-Identifier: (GPL-2.0 OR MIT)
> + */
> +/ {
> +	clk25m: oscillator@0 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <25000000>;
> +		clock-output-names = "clkxtal";
> +	};
> +
> +	uart_clk: dummy26m {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <25000000>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
> index 3a729d9..3768c28 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
> @@ -8,6 +8,7 @@
>   
>   #include <dt-bindings/interrupt-controller/irq.h>
>   #include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include "mt7622-clock.dtsi"
>   
>   / {
>   	compatible = "mediatek,mt7622";
>