lkml.org 
[lkml]   [2017]   [Nov]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/3] arm64: dts: actions: add s900 clock controller nodes
    Hi Andreas,
    On Sat, Nov 04, 2017 at 04:43:31PM +0800, Andreas Färber wrote:
    > Am 01.11.2017 um 03:54 schrieb Manivannan Sadhasivam:
    > > This patch adds clock controller nodes for Actions Semi s900 SoC
    >
    > Please avoid "This patch". Once applied, it's a commit. :)
    >
    Ack.
    > >
    > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
    > > ---
    > > arch/arm64/boot/dts/actions/s900.dtsi | 19 +++++++++++++++++++
    > > 1 file changed, 19 insertions(+)
    > >
    > > diff --git a/arch/arm64/boot/dts/actions/s900.dtsi b/arch/arm64/boot/dts/actions/s900.dtsi
    > > index 11406f6..c099ef1 100644
    > > --- a/arch/arm64/boot/dts/actions/s900.dtsi
    > > +++ b/arch/arm64/boot/dts/actions/s900.dtsi
    > > @@ -5,6 +5,7 @@
    > > */
    > >
    > > #include <dt-bindings/interrupt-controller/arm-gic.h>
    > > +#include <dt-bindings/clock/actions,s900-clock.h>
    >
    > Alphabetical order please.
    >
    Ack.
    > >
    > > / {
    > > compatible = "actions,s900";
    > > @@ -88,6 +89,24 @@
    > > #clock-cells = <0>;
    > > };
    > >
    > > + losc: losc {
    > > + compatible = "fixed-clock";
    > > + clock-frequency = <32768>;
    > > + #clock-cells = <0>;
    > > + };
    > > +
    > > + diff_24M: diff_24M {
    >
    > No underscore in node names please - maybe diff-24M or simpler diff24M?
    > Please double-check that name - is it from manual or from schematics?
    >
    Actually, this name comes from vendor driver and they have used "24M_diff".
    As per DT conventions I renamed it to "diff_24M".

    But I'm not clear about that clock. As I said in my reply to Rob's review
    there isn't much documentation also. So, I guess we can name it as diff24M
    for now.
    > > + compatible = "fixed-clock";
    > > + clock-frequency = <24000000>;
    > > + #clock-cells = <0>;
    > > + };
    > > +
    > > + clock: clock-controller@e0160000 {
    > > + compatible = "actions,s900-clock";
    > > + reg = <0 0xe0160000 0 0x1000>;
    > > + #clock-cells = <1>;
    > > + };
    >
    > This node belongs into the soc node below, ordered by unit address.
    > And maybe there's a better label than "clock"? "clkc" or some official
    > name from the manual?
    >
    Will use "cmu".

    Thanks,
    Mani
    > > +
    > > soc {
    > > compatible = "simple-bus";
    > > #address-cells = <2>;
    >
    > Regards,
    > Andreas
    >
    > --
    > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
    > GF: Felix Imendörffer, Jane Smithard, Graham Norton
    > HRB 21284 (AG Nürnberg)

    \
     
     \ /
      Last update: 2017-11-04 10:51    [W:3.169 / U:0.764 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site