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]
Date:	Fri, 27 Nov 2015 13:42:00 +0800
From:	Jisheng Zhang <jszhang@...vell.com>
To:	Maxime Ripard <maxime.ripard@...e-electrons.com>
CC:	Chen-Yu Tsai <wens@...e.org>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Antoine Ténart 
	<antoine.tenart@...e-electrons.com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	<linux-sunxi@...glegroups.com>, <linux-clk@...r.kernel.org>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 4/5] ARM: dts: sun9i: Add A80 PRCM clocks and reset
 control nodes

Dear Maxime,

On Thu, 26 Nov 2015 21:09:42 +0100
Maxime Ripard wrote:

> On Tue, Nov 24, 2015 at 06:27:09PM +0800, Jisheng Zhang wrote:
> > + Sebastian
> > 
> > On Tue, 24 Nov 2015 17:32:15 +0800
> > Chen-Yu Tsai wrote:
> >   
> > > This adds the supported PRCM clocks and reset controls to the A80 dtsi.
> > > The DAUDIO module clocks are not supported yet.
> > > 
> > > Also update clock and reset phandles for r_uart.
> > > 
> > > Signed-off-by: Chen-Yu Tsai <wens@...e.org>
> > > ---
> > >  arch/arm/boot/dts/sun9i-a80.dtsi | 79 +++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 78 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/sun9i-a80.dtsi b/arch/arm/boot/dts/sun9i-a80.dtsi
> > > index 1118bf5cc4fb..a4ce348c0831 100644
> > > --- a/arch/arm/boot/dts/sun9i-a80.dtsi
> > > +++ b/arch/arm/boot/dts/sun9i-a80.dtsi
> > > @@ -164,6 +164,14 @@
> > >  					     "usb_phy2", "usb_hsic_12M";
> > >  		};
> > >  
> > > +		pll3: clk@...00008 {
> > > +			/* placeholder until implemented */
> > > +			#clock-cells = <0>;
> > > +			compatible = "fixed-clock";
> > > +			clock-rate = <0>;
> > > +			clock-output-names = "pll3";
> > > +		};
> > > +
> > >  		pll4: clk@...0000c {
> > >  			#clock-cells = <0>;
> > >  			compatible = "allwinner,sun9i-a80-pll4-clk";
> > > @@ -350,6 +358,68 @@
> > >  					"apb1_uart2", "apb1_uart3",
> > >  					"apb1_uart4", "apb1_uart5";
> > >  		};
> > > +
> > > +		cpus_clk: clk@...01410 {
> > > +			compatible = "allwinner,sun9i-a80-cpus-clk";
> > > +			reg = <0x08001410 0x4>;
> > > +			#clock-cells = <0>;
> > > +			clocks = <&osc32k>, <&osc24M>, <&pll4>, <&pll3>;
> > > +			clock-output-names = "cpus";
> > > +		};
> > > +
> > > +		ahbs: ahbs_clk {
> > > +			compatible = "fixed-factor-clock";
> > > +			#clock-cells = <0>;
> > > +			clock-div = <1>;
> > > +			clock-mult = <1>;
> > > +			clocks = <&cpus_clk>;
> > > +			clock-output-names = "ahbs";
> > > +		};  
> > 
> > Dear Sebastian and all,
> > 
> > I just want to take the sunxi clk support in mainline for example.
> > 
> > I'm not sure I understand correctly, it seems to me that some maintainers draw a
> > line: "having a node for every clock" is a no, no[1]. But here we saw one node for
> > cpus_clk and apbs below. And <0x08001410 0x4>; <0x0800141c 0x4>; shows they
> > are close each other, so should we merge them into a single clock complex node
> > then use mfd, regmap in clk driver?
> > 
> > But IMHO, sunxi dts nodes really represent real HW, so I still can't understand
> > why we could not have each node for cpus_clk and apbs. Can you please kindly
> > teach me?  
> 
> I'm totally lacking any context, but I'll reply. My view on this is
> that they both represent the hardware, simply with a different model.
> 
> This preference of the clk maintainers and active clk developers
> regarding the model to choose has evolved over time. When we started
> the sunxi support, having one node per clock was the preferred way to
> do things.
> 
> Berlin came much later, and the preference at that time was to have
> the entire clock controller represented as single opaque block to the
> consumers.
> 
> Both have pros and cons. The approach we took allows for an easier mix
> and match, especially if the clocks you have in one SoC are reused in
> others, without modifying the source code (or barely). AFAIK, this is
> also the approach took by mvebu, except that their clock tree is much
> much much simpler.
> 
> The approach Berlin took allows to have an easier maintainance and
> more flexibility, for example to deal with clock registration
> ordering, or clocks that share registers.
> 
> Our approach works just fine in our case, and I feel no incentive to
> move to the new model (quite the opposite actually), but I guess it
> also depends on how your clock controller is built, how many SoCs you
> have to support, and how much clocks they are sharing.

Great! Thank you a lot for the detailed explanation. Now I can understand
why the clk drivers follow different models, I think I get Sebastian and
Stephen's view points. Although the BG4CT clock/pll IP are shared in all
newer than BG2Q SoCs (the only difference is how many clocks, how many plls,
and their register address), I'll follow Sebastian's suggestions -- one
entire clock controller node for all clocks/plls block

> 
> I hope it clears things up.

Yes, it's clear now. I knew the history and the reason why there are different
models in clk drivers.

Will cook a newer BG4CT clk series for review.

Thank you,
Jisheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists