Re: [linux-sunxi] Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Hans de Goede]

Hi,

On 26-10-15 22:06, Maxime Ripard wrote:

On Sat, Oct 24, 2015 at 10:47:49AM +0200, Jean-Francois Moine wrote:

On Sat, 24 Oct 2015 09:13:28 +0200
Maxime Ripard  wrote:


Or simply

bus_gates {
clocks = <>, <>;
clock-indices = <5>, <6>, <8>, ...
clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
};


I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
programmed independently to different frequencies


I don't understand why you're talking about frequencies here.


and you have to know which of them is the parent of each leaf clock.


Indeed, but that's also doable here. Just not in the DT.


So, either you hard-code the parents as Jens did in a first proposal,
or you define the full list of parents in the DT as in the last
proposal, or you use a container per parent in the DT as I proposed.

There could be an other solution using the output clock name to define
the parent clock:

bus_gates {
clocks = <>, <>, <>, <>;
clock-indices = <5>, <6>, <8>, ...
clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
};

with the documentation:

"the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."

and the code

if (strncmp(clock_name, "ahb1", 4) == 0)
clk_parent = of_clk_get_parent_name(node, 0);
else if (..)

but it seems a bit hacky.


It's exactly what I suggested, without the string comparison, but
relying on the ID instead.


I'm not following you here, what do you mean with "the ID" ?

Regards,

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

Re: [linux-sunxi] Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Hans de Goede]

Hi,

On 26-10-15 22:06, Maxime Ripard wrote:

On Sat, Oct 24, 2015 at 10:47:49AM +0200, Jean-Francois Moine wrote:

On Sat, 24 Oct 2015 09:13:28 +0200
Maxime Ripard  wrote:


Or simply

bus_gates {
clocks = <>, <>;
clock-indices = <5>, <6>, <8>, ...
clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
};


I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
programmed independently to different frequencies


I don't understand why you're talking about frequencies here.


and you have to know which of them is the parent of each leaf clock.


Indeed, but that's also doable here. Just not in the DT.


So, either you hard-code the parents as Jens did in a first proposal,
or you define the full list of parents in the DT as in the last
proposal, or you use a container per parent in the DT as I proposed.

There could be an other solution using the output clock name to define
the parent clock:

bus_gates {
clocks = <>, <>, <>, <>;
clock-indices = <5>, <6>, <8>, ...
clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
};

with the documentation:

"the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."

and the code

if (strncmp(clock_name, "ahb1", 4) == 0)
clk_parent = of_clk_get_parent_name(node, 0);
else if (..)

but it seems a bit hacky.


It's exactly what I suggested, without the string comparison, but
relying on the ID instead.


I'm not following you here, what do you mean with "the ID" ?

Regards,

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

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Sat, Oct 24, 2015 at 10:47:49AM +0200, Jean-Francois Moine wrote:
> On Sat, 24 Oct 2015 09:13:28 +0200
> Maxime Ripard  wrote:
> 
> > Or simply
> > 
> > bus_gates {
> > clocks = <>, <>;
> > clock-indices = <5>, <6>, <8>, ...
> > clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
> > };
> 
> I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
> programmed independently to different frequencies

I don't understand why you're talking about frequencies here.

> and you have to know which of them is the parent of each leaf clock.

Indeed, but that's also doable here. Just not in the DT.

> So, either you hard-code the parents as Jens did in a first proposal,
> or you define the full list of parents in the DT as in the last
> proposal, or you use a container per parent in the DT as I proposed.
> 
> There could be an other solution using the output clock name to define
> the parent clock:
> 
> bus_gates {
>   clocks = <>, <>, <>, <>;
>   clock-indices = <5>, <6>, <8>, ...
>   clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
> };
> 
> with the documentation:
> 
>   "the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."
> 
> and the code
> 
>   if (strncmp(clock_name, "ahb1", 4) == 0)
>   clk_parent = of_clk_get_parent_name(node, 0);
>   else if (..)
> 
> but it seems a bit hacky.

It's exactly what I suggested, without the string comparison, but
relying on the ID instead.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [linux-sunxi] Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Sat, Oct 24, 2015 at 10:39:49AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10/23/2015 08:14 PM, Maxime Ripard wrote:
> >On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> >>+   bus_gates: clk@01c20060 {
> >>+   #clock-cells = <1>;
> >>+   compatible = "allwinner,sun8i-h3-bus-gates-clk";
> >>+   reg = <0x01c20060 0x14>;
> >>+   clock-indices = <5>, <6>, <8>,
> >>+   <9>, <10>, <13>,
> >>+   <14>, <17>, <18>,
> >>+   <19>, <20>,
> >>+   <21>, <23>,
> >>+   <24>, <25>,
> >>+   <26>, <27>,
> >>+   <28>, <29>,
> >>+   <30>, <31>, <32>,
> >>+   <35>, <36>, <37>,
> >>+   <40>, <41>, <43>,
> >>+   <44>, <52>, <53>,
> >>+   <54>, <64>,
> >>+   <65>, <69>, <72>,
> >>+   <76>, <77>, <78>,
> >>+   <96>, <97>, <98>,
> >>+   <112>, <113>,
> >>+   <114>, <115>, <116>,
> >>+   <128>, <135>;
> >>+   clocks = <>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>,
> >>+<>, <>,
> >>+<>, <>,
> >>+<>, <>,
> >>+<>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>,
> >>+<>, <>, <>,
> >>+<>, <>;
> >
> >This is not really what I had in mind...
> 
> I came to the same solution independently, I took my inspiration from
> the rockchip clocks driver which is dealing with this problem in the
> same way, so there is precedent for doing things this way, and this
> does give us lot of flexibility. Given that I expect other new allwinnner
> SoCs to have the same problem I believe it is good to have that
> flexibility.

The rockchip clocks driver are very different from our own regarding
the DT bindings. Being consistent within our own platform seems to
bring much more value than getting bits and pieces here and there when
it's convenient.

Plus, you actually forgot in your argument to mention that this
binding was documented as deprecated, and not used anywhere since
3.17... So apparently, someone tried that, and finally changed its
mind.

Again, this is clearly a workaround, with no way to easily identify if
a given clock has the right parent afterwards. We can't review it
properly, and it's going to be a pain to fix after the facts.

Having some association masks in the driver itself, using the BIT
macro, will be much easier to maintain in the long run.

> >This IP has 2 parents, and only two parents.
> 
> Nope it has 4: apb1, apb2, ahb1 and ahb2.

The point still remains though.

> >The mapping between the IPs should be done in the driver itself,
> >not in the DT where it is very error prone and barely readable.
> 
> It is just as error prone and barely readable in C-code, see Jens original
> patchset, he did an array of clock indices there (range 0-3 with an index
> into the parent clocks array), which is arguably even more unreadable since
> there is an extra level of indirection here.

I agree, and it's why I suggested another approach at the time.

> The problem with the unreadability simply comes from allwinner's decision
> to no longer have a gates register per bus but instead shove everything
> in a single bit-array in random order, there is nothing we can do to fix
> that.

Indeed. Except one solution is easier to maintain than the other.

> Also the argument "this belongs in the driver not in the DT" is a
> bit inconsistent with the moving of the mask of valid gates from the
> driver into the clock-indices in devicetree.

Except I never used that argument.

> The way things are done here actually are doing pretty much the same
> thing, putting info which could be derived from the compatible
> string into devicetree.
>
> Last as said already there is precedent for doing things this way
> in the rockchip driver, and given that 2 people have come up
> with this approach independently of of each other this clearly
> seems to be the most straight-foward / logical way to 

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Sat, Oct 24, 2015 at 10:47:49AM +0200, Jean-Francois Moine wrote:
> On Sat, 24 Oct 2015 09:13:28 +0200
> Maxime Ripard  wrote:
> 
> > Or simply
> > 
> > bus_gates {
> > clocks = <>, <>;
> > clock-indices = <5>, <6>, <8>, ...
> > clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
> > };
> 
> I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
> programmed independently to different frequencies

I don't understand why you're talking about frequencies here.

> and you have to know which of them is the parent of each leaf clock.

Indeed, but that's also doable here. Just not in the DT.

> So, either you hard-code the parents as Jens did in a first proposal,
> or you define the full list of parents in the DT as in the last
> proposal, or you use a container per parent in the DT as I proposed.
> 
> There could be an other solution using the output clock name to define
> the parent clock:
> 
> bus_gates {
>   clocks = <>, <>, <>, <>;
>   clock-indices = <5>, <6>, <8>, ...
>   clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
> };
> 
> with the documentation:
> 
>   "the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."
> 
> and the code
> 
>   if (strncmp(clock_name, "ahb1", 4) == 0)
>   clk_parent = of_clk_get_parent_name(node, 0);
>   else if (..)
> 
> but it seems a bit hacky.

It's exactly what I suggested, without the string comparison, but
relying on the ID instead.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [linux-sunxi] Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Sat, Oct 24, 2015 at 10:39:49AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 10/23/2015 08:14 PM, Maxime Ripard wrote:
> >On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> >>+   bus_gates: clk@01c20060 {
> >>+   #clock-cells = <1>;
> >>+   compatible = "allwinner,sun8i-h3-bus-gates-clk";
> >>+   reg = <0x01c20060 0x14>;
> >>+   clock-indices = <5>, <6>, <8>,
> >>+   <9>, <10>, <13>,
> >>+   <14>, <17>, <18>,
> >>+   <19>, <20>,
> >>+   <21>, <23>,
> >>+   <24>, <25>,
> >>+   <26>, <27>,
> >>+   <28>, <29>,
> >>+   <30>, <31>, <32>,
> >>+   <35>, <36>, <37>,
> >>+   <40>, <41>, <43>,
> >>+   <44>, <52>, <53>,
> >>+   <54>, <64>,
> >>+   <65>, <69>, <72>,
> >>+   <76>, <77>, <78>,
> >>+   <96>, <97>, <98>,
> >>+   <112>, <113>,
> >>+   <114>, <115>, <116>,
> >>+   <128>, <135>;
> >>+   clocks = <>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>,
> >>+<>, <>,
> >>+<>, <>,
> >>+<>, <>,
> >>+<>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>, <>,
> >>+<>, <>,
> >>+<>, <>, <>,
> >>+<>, <>;
> >
> >This is not really what I had in mind...
> 
> I came to the same solution independently, I took my inspiration from
> the rockchip clocks driver which is dealing with this problem in the
> same way, so there is precedent for doing things this way, and this
> does give us lot of flexibility. Given that I expect other new allwinnner
> SoCs to have the same problem I believe it is good to have that
> flexibility.

The rockchip clocks driver are very different from our own regarding
the DT bindings. Being consistent within our own platform seems to
bring much more value than getting bits and pieces here and there when
it's convenient.

Plus, you actually forgot in your argument to mention that this
binding was documented as deprecated, and not used anywhere since
3.17... So apparently, someone tried that, and finally changed its
mind.

Again, this is clearly a workaround, with no way to easily identify if
a given clock has the right parent afterwards. We can't review it
properly, and it's going to be a pain to fix after the facts.

Having some association masks in the driver itself, using the BIT
macro, will be much easier to maintain in the long run.

> >This IP has 2 parents, and only two parents.
> 
> Nope it has 4: apb1, apb2, ahb1 and ahb2.

The point still remains though.

> >The mapping between the IPs should be done in the driver itself,
> >not in the DT where it is very error prone and barely readable.
> 
> It is just as error prone and barely readable in C-code, see Jens original
> patchset, he did an array of clock indices there (range 0-3 with an index
> into the parent clocks array), which is arguably even more unreadable since
> there is an extra level of indirection here.

I agree, and it's why I suggested another approach at the time.

> The problem with the unreadability simply comes from allwinner's decision
> to no longer have a gates register per bus but instead shove everything
> in a single bit-array in random order, there is nothing we can do to fix
> that.

Indeed. Except one solution is easier to maintain than the other.

> Also the argument "this belongs in the driver not in the DT" is a
> bit inconsistent with the moving of the mask of valid gates from the
> driver into the clock-indices in devicetree.

Except I never used that argument.

> The way things are done here actually are doing pretty much the same
> thing, putting info which could be derived from the compatible
> string into devicetree.
>
> Last as said already there is precedent for doing things this way
> in the rockchip driver, and given that 2 people have come up
> with this approach independently of of each other this clearly
> seems to be the most straight-foward / logical way to 

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Sat, 24 Oct 2015 09:13:28 +0200
Maxime Ripard  wrote:

> Or simply
> 
> bus_gates {
>   clocks = <>, <>;
>   clock-indices = <5>, <6>, <8>, ...
>   clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
> };

I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
programmed independently to different frequencies and you have to know
which of them is the parent of each leaf clock.

So, either you hard-code the parents as Jens did in a first proposal,
or you define the full list of parents in the DT as in the last
proposal, or you use a container per parent in the DT as I proposed.

There could be an other solution using the output clock name to define
the parent clock:

bus_gates {
clocks = <>, <>, <>, <>;
clock-indices = <5>, <6>, <8>, ...
clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
};

with the documentation:

"the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."

and the code

if (strncmp(clock_name, "ahb1", 4) == 0)
clk_parent = of_clk_get_parent_name(node, 0);
else if (..)

but it seems a bit hacky.

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [linux-sunxi] Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Hans de Goede]

Hi,

On 10/23/2015 08:14 PM, Maxime Ripard wrote:

On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:

+   bus_gates: clk@01c20060 {
+   #clock-cells = <1>;
+   compatible = "allwinner,sun8i-h3-bus-gates-clk";
+   reg = <0x01c20060 0x14>;
+   clock-indices = <5>, <6>, <8>,
+   <9>, <10>, <13>,
+   <14>, <17>, <18>,
+   <19>, <20>,
+   <21>, <23>,
+   <24>, <25>,
+   <26>, <27>,
+   <28>, <29>,
+   <30>, <31>, <32>,
+   <35>, <36>, <37>,
+   <40>, <41>, <43>,
+   <44>, <52>, <53>,
+   <54>, <64>,
+   <65>, <69>, <72>,
+   <76>, <77>, <78>,
+   <96>, <97>, <98>,
+   <112>, <113>,
+   <114>, <115>, <116>,
+   <128>, <135>;
+   clocks = <>, <>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>,
+<>, <>,
+<>, <>,
+<>, <>,
+<>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>,
+<>, <>, <>,
+<>, <>;


This is not really what I had in mind...


I came to the same solution independently, I took my inspiration from
the rockchip clocks driver which is dealing with this problem in the
same way, so there is precedent for doing things this way, and this
does give us lot of flexibility. Given that I expect other new allwinnner
SoCs to have the same problem I believe it is good to have that
flexibility.


This IP has 2 parents, and only two parents.


Nope it has 4: apb1, apb2, ahb1 and ahb2.


The mapping between the
IPs should be done in the driver itself, not in the DT where it is
very error prone and barely readable.


It is just as error prone and barely readable in C-code, see Jens original
patchset, he did an array of clock indices there (range 0-3 with an index
into the parent clocks array), which is arguably even more unreadable since
there is an extra level of indirection here.

The problem with the unreadability simply comes from allwinner's decision
to no longer have a gates register per bus but instead shove everything
in a single bit-array in random order, there is nothing we can do to fix
that.

Also the argument "this belongs in the driver not in the DT" is a bit
inconsistent with the moving of the mask of valid gates from the
driver into the clock-indices in devicetree. The way things are done
here actually are doing pretty much the same thing, putting info which
could be derived from the compatible string into devicetree.

Last as said already there is precedent for doing things this way
in the rockchip driver, and given that 2 people have come up
with this approach independently of of each other this clearly
seems to be the most straight-foward / logical way to deal with
this.


And note that I never have expected you to use clk-simple-gates
either. This is a complicated clock


No it is not complicated, have you looked at the changes to the
simple-clk-gates driver which Jean Francois Moine suggested?

Those 5 extra lines (4 new lines) are all that is needed when
going with the approach of listing a parent per gate. This is
actually still a quite simple clock, we only need to find a way
to specify a parent per gate, preferably via DT since this gives
us greater flexibility which will be quite useful when adding
support for other SoCs.

Regards,

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

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Fri, Oct 23, 2015 at 09:20:13PM +0200, Jean-Francois Moine wrote:
> On Fri, 23 Oct 2015 20:14:06 +0200
> Maxime Ripard  wrote:
> 
> > On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> > > + bus_gates: clk@01c20060 {
> > > + #clock-cells = <1>;
> > > + compatible = "allwinner,sun8i-h3-bus-gates-clk";
> > > + reg = <0x01c20060 0x14>;
> > > + clock-indices = <5>, <6>, <8>,
> > > + <9>, <10>, <13>,
> > > + <14>, <17>, <18>,
> > > + <19>, <20>,
> > > + <21>, <23>,
> > > + <24>, <25>,
> > > + <26>, <27>,
> > > + <28>, <29>,
> > > + <30>, <31>, <32>,
> > > + <35>, <36>, <37>,
> > > + <40>, <41>, <43>,
> > > + <44>, <52>, <53>,
> > > + <54>, <64>,
> > > + <65>, <69>, <72>,
> > > + <76>, <77>, <78>,
> > > + <96>, <97>, <98>,
> > > + <112>, <113>,
> > > + <114>, <115>, <116>,
> > > + <128>, <135>;
> > > + clocks = <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>,
> > > +  <>, <>,
> > > +  <>, <>,
> > > +  <>, <>,
> > > +  <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>;  
> > 
> > This is not really what I had in mind...
> > 
> > This IP has 2 parents, and only two parents. The mapping between the
> > IPs should be done in the driver itself, not in the DT where it is
> > very error prone and barely readable.
> > 
> > And note that I never have expected you to use clk-simple-gates
> > either. This is a complicated clock, unlike the other we've seen so
> > far, it definitely deserves a driver of its own.
> 
> It seems that Allwinner puts the gate definitions anywhere in the array
> of registers, so, I think that the H3 scheme will not be the last
> complicated one,

Maybe, but that's the first one. It doesn't prevent us from reusing
the driver later if it happens.

> and if the parent clocks are in the code instead of in the DT, we
> will have more and more code to develop.

I never asked that either.

> An other way to describe the gates would be to add containers per parent
> (with still a small patch in the clk-simple-gates):
> 
>   bus_gates: clk@01c20060 {
>   #clock-cells = <1>;
>   compatible = "allwinner,sun8i-h3-bus-gates-clk";
>   reg = <0x01c20060 0x14>;
>   ahb1_gates {
>   clocks = <>;
>   clock-indices = <5>, <6>, <8>,
>   <9>, <10>, <13>,
>   <14>, <18>,
>   <19>, <20>,
>   ...;
>   };
>   clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0",
>   "ahb1_mmc1", "ahb1_mmc2", "ahb1_nand",
>   "ahb1_sdram", "ahb1_ts",
>   "ahb1_hstimer", "ahb1_spi0",
>   ...;
>   };
>   ahb2_gates {
>   clocks = <>;
>   clock-indices = <17>, <29>,
>   <30>, <31>, <32>,
>   ...;
>   clock-output-names = "ahb2_gmac", "ahb2_ohic1",
>   "ahb2_ohic2", "ahb2_ohic3", "ahb1_ve",
>   ...;
>   };
>   apb1_gates {
>   ...
>   };
>   apb2_gates {
>   ...
>   };
>   };

Or simply

bus_gates {
clocks = <>, <>;
clock-indices = <5>, <6>, <8>, ...
clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
};

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Sat, 24 Oct 2015 09:13:28 +0200
Maxime Ripard  wrote:

> Or simply
> 
> bus_gates {
>   clocks = <>, <>;
>   clock-indices = <5>, <6>, <8>, ...
>   clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
> };

I don't understand: the apb1, apb2, ahb1 and ahb2 clocks may be
programmed independently to different frequencies and you have to know
which of them is the parent of each leaf clock.

So, either you hard-code the parents as Jens did in a first proposal,
or you define the full list of parents in the DT as in the last
proposal, or you use a container per parent in the DT as I proposed.

There could be an other solution using the output clock name to define
the parent clock:

bus_gates {
clocks = <>, <>, <>, <>;
clock-indices = <5>, <6>, <8>, ...
clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0"
};

with the documentation:

"the clocks MUST be defined in order: ahb1, ahb2, apb1, apb2."

and the code

if (strncmp(clock_name, "ahb1", 4) == 0)
clk_parent = of_clk_get_parent_name(node, 0);
else if (..)

but it seems a bit hacky.

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [linux-sunxi] Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Hans de Goede]

Hi,

On 10/23/2015 08:14 PM, Maxime Ripard wrote:

On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:

+   bus_gates: clk@01c20060 {
+   #clock-cells = <1>;
+   compatible = "allwinner,sun8i-h3-bus-gates-clk";
+   reg = <0x01c20060 0x14>;
+   clock-indices = <5>, <6>, <8>,
+   <9>, <10>, <13>,
+   <14>, <17>, <18>,
+   <19>, <20>,
+   <21>, <23>,
+   <24>, <25>,
+   <26>, <27>,
+   <28>, <29>,
+   <30>, <31>, <32>,
+   <35>, <36>, <37>,
+   <40>, <41>, <43>,
+   <44>, <52>, <53>,
+   <54>, <64>,
+   <65>, <69>, <72>,
+   <76>, <77>, <78>,
+   <96>, <97>, <98>,
+   <112>, <113>,
+   <114>, <115>, <116>,
+   <128>, <135>;
+   clocks = <>, <>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>,
+<>, <>,
+<>, <>,
+<>, <>,
+<>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>, <>,
+<>, <>,
+<>, <>, <>,
+<>, <>;


This is not really what I had in mind...


I came to the same solution independently, I took my inspiration from
the rockchip clocks driver which is dealing with this problem in the
same way, so there is precedent for doing things this way, and this
does give us lot of flexibility. Given that I expect other new allwinnner
SoCs to have the same problem I believe it is good to have that
flexibility.


This IP has 2 parents, and only two parents.


Nope it has 4: apb1, apb2, ahb1 and ahb2.


The mapping between the
IPs should be done in the driver itself, not in the DT where it is
very error prone and barely readable.


It is just as error prone and barely readable in C-code, see Jens original
patchset, he did an array of clock indices there (range 0-3 with an index
into the parent clocks array), which is arguably even more unreadable since
there is an extra level of indirection here.

The problem with the unreadability simply comes from allwinner's decision
to no longer have a gates register per bus but instead shove everything
in a single bit-array in random order, there is nothing we can do to fix
that.

Also the argument "this belongs in the driver not in the DT" is a bit
inconsistent with the moving of the mask of valid gates from the
driver into the clock-indices in devicetree. The way things are done
here actually are doing pretty much the same thing, putting info which
could be derived from the compatible string into devicetree.

Last as said already there is precedent for doing things this way
in the rockchip driver, and given that 2 people have come up
with this approach independently of of each other this clearly
seems to be the most straight-foward / logical way to deal with
this.


And note that I never have expected you to use clk-simple-gates
either. This is a complicated clock


No it is not complicated, have you looked at the changes to the
simple-clk-gates driver which Jean Francois Moine suggested?

Those 5 extra lines (4 new lines) are all that is needed when
going with the approach of listing a parent per gate. This is
actually still a quite simple clock, we only need to find a way
to specify a parent per gate, preferably via DT since this gives
us greater flexibility which will be quite useful when adding
support for other SoCs.

Regards,

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

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Fri, Oct 23, 2015 at 09:20:13PM +0200, Jean-Francois Moine wrote:
> On Fri, 23 Oct 2015 20:14:06 +0200
> Maxime Ripard  wrote:
> 
> > On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> > > + bus_gates: clk@01c20060 {
> > > + #clock-cells = <1>;
> > > + compatible = "allwinner,sun8i-h3-bus-gates-clk";
> > > + reg = <0x01c20060 0x14>;
> > > + clock-indices = <5>, <6>, <8>,
> > > + <9>, <10>, <13>,
> > > + <14>, <17>, <18>,
> > > + <19>, <20>,
> > > + <21>, <23>,
> > > + <24>, <25>,
> > > + <26>, <27>,
> > > + <28>, <29>,
> > > + <30>, <31>, <32>,
> > > + <35>, <36>, <37>,
> > > + <40>, <41>, <43>,
> > > + <44>, <52>, <53>,
> > > + <54>, <64>,
> > > + <65>, <69>, <72>,
> > > + <76>, <77>, <78>,
> > > + <96>, <97>, <98>,
> > > + <112>, <113>,
> > > + <114>, <115>, <116>,
> > > + <128>, <135>;
> > > + clocks = <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>,
> > > +  <>, <>,
> > > +  <>, <>,
> > > +  <>, <>,
> > > +  <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>,
> > > +  <>, <>, <>,
> > > +  <>, <>;  
> > 
> > This is not really what I had in mind...
> > 
> > This IP has 2 parents, and only two parents. The mapping between the
> > IPs should be done in the driver itself, not in the DT where it is
> > very error prone and barely readable.
> > 
> > And note that I never have expected you to use clk-simple-gates
> > either. This is a complicated clock, unlike the other we've seen so
> > far, it definitely deserves a driver of its own.
> 
> It seems that Allwinner puts the gate definitions anywhere in the array
> of registers, so, I think that the H3 scheme will not be the last
> complicated one,

Maybe, but that's the first one. It doesn't prevent us from reusing
the driver later if it happens.

> and if the parent clocks are in the code instead of in the DT, we
> will have more and more code to develop.

I never asked that either.

> An other way to describe the gates would be to add containers per parent
> (with still a small patch in the clk-simple-gates):
> 
>   bus_gates: clk@01c20060 {
>   #clock-cells = <1>;
>   compatible = "allwinner,sun8i-h3-bus-gates-clk";
>   reg = <0x01c20060 0x14>;
>   ahb1_gates {
>   clocks = <>;
>   clock-indices = <5>, <6>, <8>,
>   <9>, <10>, <13>,
>   <14>, <18>,
>   <19>, <20>,
>   ...;
>   };
>   clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0",
>   "ahb1_mmc1", "ahb1_mmc2", "ahb1_nand",
>   "ahb1_sdram", "ahb1_ts",
>   "ahb1_hstimer", "ahb1_spi0",
>   ...;
>   };
>   ahb2_gates {
>   clocks = <>;
>   clock-indices = <17>, <29>,
>   <30>, <31>, <32>,
>   ...;
>   clock-output-names = "ahb2_gmac", "ahb2_ohic1",
>   "ahb2_ohic2", "ahb2_ohic3", "ahb1_ve",
>   ...;
>   };
>   apb1_gates {
>   ...
>   };
>   apb2_gates {
>   ...
>   };
>   };

Or simply

bus_gates {
clocks = <>, <>;
clock-indices = <5>, <6>, <8>, ...
clock-output-names = "bus_ce", "bus_dma", "bus_mmc0"
};

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Fri, 23 Oct 2015 20:14:06 +0200
Maxime Ripard  wrote:

> On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> > +   bus_gates: clk@01c20060 {
> > +   #clock-cells = <1>;
> > +   compatible = "allwinner,sun8i-h3-bus-gates-clk";
> > +   reg = <0x01c20060 0x14>;
> > +   clock-indices = <5>, <6>, <8>,
> > +   <9>, <10>, <13>,
> > +   <14>, <17>, <18>,
> > +   <19>, <20>,
> > +   <21>, <23>,
> > +   <24>, <25>,
> > +   <26>, <27>,
> > +   <28>, <29>,
> > +   <30>, <31>, <32>,
> > +   <35>, <36>, <37>,
> > +   <40>, <41>, <43>,
> > +   <44>, <52>, <53>,
> > +   <54>, <64>,
> > +   <65>, <69>, <72>,
> > +   <76>, <77>, <78>,
> > +   <96>, <97>, <98>,
> > +   <112>, <113>,
> > +   <114>, <115>, <116>,
> > +   <128>, <135>;
> > +   clocks = <>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>,
> > +<>, <>,
> > +<>, <>,
> > +<>, <>,
> > +<>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>,
> > +<>, <>, <>,
> > +<>, <>;  
> 
> This is not really what I had in mind...
> 
> This IP has 2 parents, and only two parents. The mapping between the
> IPs should be done in the driver itself, not in the DT where it is
> very error prone and barely readable.
> 
> And note that I never have expected you to use clk-simple-gates
> either. This is a complicated clock, unlike the other we've seen so
> far, it definitely deserves a driver of its own.

It seems that Allwinner puts the gate definitions anywhere in the array
of registers, so, I think that the H3 scheme will not be the last
complicated one, and if the parent clocks are in the code instead of in
the DT, we will have more and more code to develop.

An other way to describe the gates would be to add containers per parent
(with still a small patch in the clk-simple-gates):

bus_gates: clk@01c20060 {
#clock-cells = <1>;
compatible = "allwinner,sun8i-h3-bus-gates-clk";
reg = <0x01c20060 0x14>;
ahb1_gates {
clocks = <>;
clock-indices = <5>, <6>, <8>,
<9>, <10>, <13>,
<14>, <18>,
<19>, <20>,
...;
};
clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0",
"ahb1_mmc1", "ahb1_mmc2", "ahb1_nand",
"ahb1_sdram", "ahb1_ts",
"ahb1_hstimer", "ahb1_spi0",
...;
};
ahb2_gates {
clocks = <>;
clock-indices = <17>, <29>,
<30>, <31>, <32>,
...;
clock-output-names = "ahb2_gmac", "ahb2_ohic1",
"ahb2_ohic2", "ahb2_ohic3", "ahb1_ve",
...;
};
apb1_gates {
...
};
apb2_gates {
...
};
};

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> + bus_gates: clk@01c20060 {
> + #clock-cells = <1>;
> + compatible = "allwinner,sun8i-h3-bus-gates-clk";
> + reg = <0x01c20060 0x14>;
> + clock-indices = <5>, <6>, <8>,
> + <9>, <10>, <13>,
> + <14>, <17>, <18>,
> + <19>, <20>,
> + <21>, <23>,
> + <24>, <25>,
> + <26>, <27>,
> + <28>, <29>,
> + <30>, <31>, <32>,
> + <35>, <36>, <37>,
> + <40>, <41>, <43>,
> + <44>, <52>, <53>,
> + <54>, <64>,
> + <65>, <69>, <72>,
> + <76>, <77>, <78>,
> + <96>, <97>, <98>,
> + <112>, <113>,
> + <114>, <115>, <116>,
> + <128>, <135>;
> + clocks = <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>,
> +  <>, <>,
> +  <>, <>,
> +  <>, <>,
> +  <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>,
> +  <>, <>, <>,
> +  <>, <>;

This is not really what I had in mind...

This IP has 2 parents, and only two parents. The mapping between the
IPs should be done in the driver itself, not in the DT where it is
very error prone and barely readable.

And note that I never have expected you to use clk-simple-gates
either. This is a complicated clock, unlike the other we've seen so
far, it definitely deserves a driver of its own.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Thu, Oct 22, 2015 at 01:30:42PM +0200, Jens Kuske wrote:
> On 22/10/15 11:14, Maxime Ripard wrote:
> > On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
> >> On Thu, 22 Oct 2015 10:47:35 +0200
> >> Maxime Ripard  wrote:
> >>
> >>> Not really. The uart0 reset is the bit 16, in the reset register 4.
> >>>
> >>> 4 * 32 + 16 = 44.
> >>>
> >>> Not 112, but still not 208 either.
> >>
> >> The registers are numbered 1..5, then
> >>
> >> (4 - 1) * 32 + 16 = 112
> > 
> > Not on my version, and even then, UARTs are on the last reset
> > register, which would still make 144.
> > 
> > Maxime
> > 
> 
> There are holes between reg2 and reg3 and reg4 for some reason, but even
> if we would correct that with some of_xlate() function they won't
> completely line up with the gates.

Indeed. Still, dealing with the holes and sticking to what the
datasheet says seems like the right solution.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Thu, Oct 22, 2015 at 01:30:42PM +0200, Jens Kuske wrote:
> On 22/10/15 11:14, Maxime Ripard wrote:
> > On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
> >> On Thu, 22 Oct 2015 10:47:35 +0200
> >> Maxime Ripard  wrote:
> >>
> >>> Not really. The uart0 reset is the bit 16, in the reset register 4.
> >>>
> >>> 4 * 32 + 16 = 44.
> >>>
> >>> Not 112, but still not 208 either.
> >>
> >> The registers are numbered 1..5, then
> >>
> >> (4 - 1) * 32 + 16 = 112
> > 
> > Not on my version, and even then, UARTs are on the last reset
> > register, which would still make 144.
> > 
> > Maxime
> > 
> 
> There are holes between reg2 and reg3 and reg4 for some reason, but even
> if we would correct that with some of_xlate() function they won't
> completely line up with the gates.

Indeed. Still, dealing with the holes and sticking to what the
datasheet says seems like the right solution.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> + bus_gates: clk@01c20060 {
> + #clock-cells = <1>;
> + compatible = "allwinner,sun8i-h3-bus-gates-clk";
> + reg = <0x01c20060 0x14>;
> + clock-indices = <5>, <6>, <8>,
> + <9>, <10>, <13>,
> + <14>, <17>, <18>,
> + <19>, <20>,
> + <21>, <23>,
> + <24>, <25>,
> + <26>, <27>,
> + <28>, <29>,
> + <30>, <31>, <32>,
> + <35>, <36>, <37>,
> + <40>, <41>, <43>,
> + <44>, <52>, <53>,
> + <54>, <64>,
> + <65>, <69>, <72>,
> + <76>, <77>, <78>,
> + <96>, <97>, <98>,
> + <112>, <113>,
> + <114>, <115>, <116>,
> + <128>, <135>;
> + clocks = <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>,
> +  <>, <>,
> +  <>, <>,
> +  <>, <>,
> +  <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>, <>,
> +  <>, <>,
> +  <>, <>, <>,
> +  <>, <>;

This is not really what I had in mind...

This IP has 2 parents, and only two parents. The mapping between the
IPs should be done in the driver itself, not in the DT where it is
very error prone and barely readable.

And note that I never have expected you to use clk-simple-gates
either. This is a complicated clock, unlike the other we've seen so
far, it definitely deserves a driver of its own.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Fri, 23 Oct 2015 20:14:06 +0200
Maxime Ripard  wrote:

> On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> > +   bus_gates: clk@01c20060 {
> > +   #clock-cells = <1>;
> > +   compatible = "allwinner,sun8i-h3-bus-gates-clk";
> > +   reg = <0x01c20060 0x14>;
> > +   clock-indices = <5>, <6>, <8>,
> > +   <9>, <10>, <13>,
> > +   <14>, <17>, <18>,
> > +   <19>, <20>,
> > +   <21>, <23>,
> > +   <24>, <25>,
> > +   <26>, <27>,
> > +   <28>, <29>,
> > +   <30>, <31>, <32>,
> > +   <35>, <36>, <37>,
> > +   <40>, <41>, <43>,
> > +   <44>, <52>, <53>,
> > +   <54>, <64>,
> > +   <65>, <69>, <72>,
> > +   <76>, <77>, <78>,
> > +   <96>, <97>, <98>,
> > +   <112>, <113>,
> > +   <114>, <115>, <116>,
> > +   <128>, <135>;
> > +   clocks = <>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>,
> > +<>, <>,
> > +<>, <>,
> > +<>, <>,
> > +<>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>, <>,
> > +<>, <>,
> > +<>, <>, <>,
> > +<>, <>;  
> 
> This is not really what I had in mind...
> 
> This IP has 2 parents, and only two parents. The mapping between the
> IPs should be done in the driver itself, not in the DT where it is
> very error prone and barely readable.
> 
> And note that I never have expected you to use clk-simple-gates
> either. This is a complicated clock, unlike the other we've seen so
> far, it definitely deserves a driver of its own.

It seems that Allwinner puts the gate definitions anywhere in the array
of registers, so, I think that the H3 scheme will not be the last
complicated one, and if the parent clocks are in the code instead of in
the DT, we will have more and more code to develop.

An other way to describe the gates would be to add containers per parent
(with still a small patch in the clk-simple-gates):

bus_gates: clk@01c20060 {
#clock-cells = <1>;
compatible = "allwinner,sun8i-h3-bus-gates-clk";
reg = <0x01c20060 0x14>;
ahb1_gates {
clocks = <>;
clock-indices = <5>, <6>, <8>,
<9>, <10>, <13>,
<14>, <18>,
<19>, <20>,
...;
};
clock-output-names = "ahb1_ce", "ahb1_dma", "ahb1_mmc0",
"ahb1_mmc1", "ahb1_mmc2", "ahb1_nand",
"ahb1_sdram", "ahb1_ts",
"ahb1_hstimer", "ahb1_spi0",
...;
};
ahb2_gates {
clocks = <>;
clock-indices = <17>, <29>,
<30>, <31>, <32>,
...;
clock-output-names = "ahb2_gmac", "ahb2_ohic1",
"ahb2_ohic2", "ahb2_ohic3", "ahb1_ve",
...;
};
apb1_gates {
...
};
apb2_gates {
...
};
};

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Wed, 21 Oct 2015 18:20:27 +0200
Jens Kuske  wrote:

> The Allwinner H3 is a home entertainment system oriented SoC with
> four Cortex-A7 cores and a Mali-400MP2 GPU.
> 
> Signed-off-by: Jens Kuske 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 499 
> 
>  1 file changed, 499 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> new file mode 100644
> index 000..4114e17
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -0,0 +1,499 @@
[snip]
> + pll6: clk@01c20028 {
> + #clock-cells = <1>;
> + compatible = "allwinner,sun6i-a31-pll6-clk";
> + reg = <0x01c20028 0x4>;
> + clocks = <>;
> + clock-output-names = "pll6", "pll6x2", "pll6d2";
> + };
> +
> + pll8: clk@01c20044 {
> + #clock-cells = <0>;

Should be <1>

> + compatible = "allwinner,sun6i-a31-pll6-clk";
> + reg = <0x01c20044 0x4>;
> + clocks = <>;
> + clock-output-names = "pll8", "pll8x2";
> + };
> +
> + cpu: cpu_clk@01c20050 {
> + #clock-cells = <0>;
> + compatible = "allwinner,sun4i-a10-cpu-clk";
> + reg = <0x01c20050 0x4>;
> + clocks = <>, <>, <>, <>;
> + clock-output-names = "cpu";
> + };
[snip]

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jens Kuske]

On 22/10/15 11:14, Maxime Ripard wrote:
> On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
>> On Thu, 22 Oct 2015 10:47:35 +0200
>> Maxime Ripard  wrote:
>>
>>> Not really. The uart0 reset is the bit 16, in the reset register 4.
>>>
>>> 4 * 32 + 16 = 44.
>>>
>>> Not 112, but still not 208 either.
>>
>> The registers are numbered 1..5, then
>>
>> (4 - 1) * 32 + 16 = 112
> 
> Not on my version, and even then, UARTs are on the last reset
> register, which would still make 144.
> 
> Maxime
> 

There are holes between reg2 and reg3 and reg4 for some reason, but even
if we would correct that with some of_xlate() function they won't
completely line up with the gates.

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

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
> On Thu, 22 Oct 2015 10:47:35 +0200
> Maxime Ripard  wrote:
> 
> > Not really. The uart0 reset is the bit 16, in the reset register 4.
> > 
> > 4 * 32 + 16 = 44.
> > 
> > Not 112, but still not 208 either.
> 
> The registers are numbered 1..5, then
> 
> (4 - 1) * 32 + 16 = 112

Not on my version, and even then, UARTs are on the last reset
register, which would still make 144.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Thu, 22 Oct 2015 10:47:35 +0200
Maxime Ripard  wrote:

> Not really. The uart0 reset is the bit 16, in the reset register 4.
> 
> 4 * 32 + 16 = 44.
> 
> Not 112, but still not 208 either.

The registers are numbered 1..5, then

(4 - 1) * 32 + 16 = 112

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Thu, Oct 22, 2015 at 10:29:59AM +0200, Jean-Francois Moine wrote:
> On Thu, 22 Oct 2015 10:05:08 +0200
> Maxime Ripard  wrote:
> 
> > > + uart0: serial@01c28000 {
> > > + compatible = "snps,dw-apb-uart";
> > > + reg = <0x01c28000 0x400>;
> > > + interrupts = ;
> > > + reg-shift = <2>;
> > > + reg-io-width = <4>;
> > > + clocks = <_gates 112>;
> > > + resets = <_rst 208>;  
> > 
> > It's a bit weird that the clocks and reset indices don't match,
> > usually they do.
> > 
> > What's even weirder is that there's a 96 offset between the two (4 *
> > 32), is this expected?
> 
> Yes, this is conform to the H3 documentation.

Not really. The uart0 reset is the bit 16, in the reset register 4.

4 * 32 + 16 = 44.

Not 112, but still not 208 either.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Thu, 22 Oct 2015 10:05:08 +0200
Maxime Ripard  wrote:

> > +   uart0: serial@01c28000 {
> > +   compatible = "snps,dw-apb-uart";
> > +   reg = <0x01c28000 0x400>;
> > +   interrupts = ;
> > +   reg-shift = <2>;
> > +   reg-io-width = <4>;
> > +   clocks = <_gates 112>;
> > +   resets = <_rst 208>;  
> 
> It's a bit weird that the clocks and reset indices don't match,
> usually they do.
> 
> What's even weirder is that there's a 96 offset between the two (4 *
> 32), is this expected?

Yes, this is conform to the H3 documentation.

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

Hi,

On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> The Allwinner H3 is a home entertainment system oriented SoC with
> four Cortex-A7 cores and a Mali-400MP2 GPU.
> 
> Signed-off-by: Jens Kuske 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 499 
> 
>  1 file changed, 499 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> new file mode 100644
> index 000..4114e17
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -0,0 +1,499 @@
> +/*
> + * Copyright (C) 2015 Jens Kuske 
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "skeleton.dtsi"
> +
> +#include 
> +#include 
> +
> +/ {
> + interrupt-parent = <>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <0>;
> + };
> +
> + cpu@1 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <1>;
> + };
> +
> + cpu@2 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <2>;
> + };
> +
> + cpu@3 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <3>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv7-timer";
> + interrupts =  IRQ_TYPE_LEVEL_LOW)>,
> +   IRQ_TYPE_LEVEL_LOW)>,
> +   IRQ_TYPE_LEVEL_LOW)>,
> +   IRQ_TYPE_LEVEL_LOW)>;
> + clock-frequency = <2400>;
> + arm,cpu-registers-not-fw-configured;
> + };
> +
> + memory {
> + reg = <0x4000 0x8000>;
> + };
> +
> + clocks {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + osc24M: osc24M_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <2400>;
> + clock-output-names = "osc24M";
> + };
> +
> + osc32k: osc32k_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <32768>;
> + clock-output-names = "osc32k";
> + };
> +
> + pll1: clk@01c2 {
> + #clock-cells = <0>;
> + compatible = "allwinner,sun8i-a23-pll1-clk";
> + reg = <0x01c2 0x4>;
> + clocks = <>;
> + 

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Wed, 21 Oct 2015 18:20:27 +0200
Jens Kuske  wrote:

> The Allwinner H3 is a home entertainment system oriented SoC with
> four Cortex-A7 cores and a Mali-400MP2 GPU.
> 
> Signed-off-by: Jens Kuske 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 499 
> 
>  1 file changed, 499 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> new file mode 100644
> index 000..4114e17
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -0,0 +1,499 @@
[snip]
> + pll6: clk@01c20028 {
> + #clock-cells = <1>;
> + compatible = "allwinner,sun6i-a31-pll6-clk";
> + reg = <0x01c20028 0x4>;
> + clocks = <>;
> + clock-output-names = "pll6", "pll6x2", "pll6d2";
> + };
> +
> + pll8: clk@01c20044 {
> + #clock-cells = <0>;

Should be <1>

> + compatible = "allwinner,sun6i-a31-pll6-clk";
> + reg = <0x01c20044 0x4>;
> + clocks = <>;
> + clock-output-names = "pll8", "pll8x2";
> + };
> +
> + cpu: cpu_clk@01c20050 {
> + #clock-cells = <0>;
> + compatible = "allwinner,sun4i-a10-cpu-clk";
> + reg = <0x01c20050 0x4>;
> + clocks = <>, <>, <>, <>;
> + clock-output-names = "cpu";
> + };
[snip]

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Thu, Oct 22, 2015 at 10:29:59AM +0200, Jean-Francois Moine wrote:
> On Thu, 22 Oct 2015 10:05:08 +0200
> Maxime Ripard  wrote:
> 
> > > + uart0: serial@01c28000 {
> > > + compatible = "snps,dw-apb-uart";
> > > + reg = <0x01c28000 0x400>;
> > > + interrupts = ;
> > > + reg-shift = <2>;
> > > + reg-io-width = <4>;
> > > + clocks = <_gates 112>;
> > > + resets = <_rst 208>;  
> > 
> > It's a bit weird that the clocks and reset indices don't match,
> > usually they do.
> > 
> > What's even weirder is that there's a 96 offset between the two (4 *
> > 32), is this expected?
> 
> Yes, this is conform to the H3 documentation.

Not really. The uart0 reset is the bit 16, in the reset register 4.

4 * 32 + 16 = 44.

Not 112, but still not 208 either.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

Hi,

On Wed, Oct 21, 2015 at 06:20:27PM +0200, Jens Kuske wrote:
> The Allwinner H3 is a home entertainment system oriented SoC with
> four Cortex-A7 cores and a Mali-400MP2 GPU.
> 
> Signed-off-by: Jens Kuske 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 499 
> 
>  1 file changed, 499 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> new file mode 100644
> index 000..4114e17
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -0,0 +1,499 @@
> +/*
> + * Copyright (C) 2015 Jens Kuske 
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "skeleton.dtsi"
> +
> +#include 
> +#include 
> +
> +/ {
> + interrupt-parent = <>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <0>;
> + };
> +
> + cpu@1 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <1>;
> + };
> +
> + cpu@2 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <2>;
> + };
> +
> + cpu@3 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <3>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv7-timer";
> + interrupts =  IRQ_TYPE_LEVEL_LOW)>,
> +   IRQ_TYPE_LEVEL_LOW)>,
> +   IRQ_TYPE_LEVEL_LOW)>,
> +   IRQ_TYPE_LEVEL_LOW)>;
> + clock-frequency = <2400>;
> + arm,cpu-registers-not-fw-configured;
> + };
> +
> + memory {
> + reg = <0x4000 0x8000>;
> + };
> +
> + clocks {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + osc24M: osc24M_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <2400>;
> + clock-output-names = "osc24M";
> + };
> +
> + osc32k: osc32k_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <32768>;
> + clock-output-names = "osc32k";
> + };
> +
> + pll1: clk@01c2 {
> + #clock-cells = <0>;
> + compatible = "allwinner,sun8i-a23-pll1-clk";
> + reg = <0x01c2 0x4>;
> + 

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Thu, 22 Oct 2015 10:05:08 +0200
Maxime Ripard  wrote:

> > +   uart0: serial@01c28000 {
> > +   compatible = "snps,dw-apb-uart";
> > +   reg = <0x01c28000 0x400>;
> > +   interrupts = ;
> > +   reg-shift = <2>;
> > +   reg-io-width = <4>;
> > +   clocks = <_gates 112>;
> > +   resets = <_rst 208>;  
> 
> It's a bit weird that the clocks and reset indices don't match,
> usually they do.
> 
> What's even weirder is that there's a 96 offset between the two (4 *
> 32), is this expected?

Yes, this is conform to the H3 documentation.

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jean-Francois Moine]

On Thu, 22 Oct 2015 10:47:35 +0200
Maxime Ripard  wrote:

> Not really. The uart0 reset is the bit 16, in the reset register 4.
> 
> 4 * 32 + 16 = 44.
> 
> Not 112, but still not 208 either.

The registers are numbered 1..5, then

(4 - 1) * 32 + 16 = 112

-- 
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef |   http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jens Kuske]

On 22/10/15 11:14, Maxime Ripard wrote:
> On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
>> On Thu, 22 Oct 2015 10:47:35 +0200
>> Maxime Ripard  wrote:
>>
>>> Not really. The uart0 reset is the bit 16, in the reset register 4.
>>>
>>> 4 * 32 + 16 = 44.
>>>
>>> Not 112, but still not 208 either.
>>
>> The registers are numbered 1..5, then
>>
>> (4 - 1) * 32 + 16 = 112
> 
> Not on my version, and even then, UARTs are on the last reset
> register, which would still make 144.
> 
> Maxime
> 

There are holes between reg2 and reg3 and reg4 for some reason, but even
if we would correct that with some of_xlate() function they won't
completely line up with the gates.

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

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Thu, Oct 22, 2015 at 10:57:45AM +0200, Jean-Francois Moine wrote:
> On Thu, 22 Oct 2015 10:47:35 +0200
> Maxime Ripard  wrote:
> 
> > Not really. The uart0 reset is the bit 16, in the reset register 4.
> > 
> > 4 * 32 + 16 = 44.
> > 
> > Not 112, but still not 208 either.
> 
> The registers are numbered 1..5, then
> 
> (4 - 1) * 32 + 16 = 112

Not on my version, and even then, UARTs are on the last reset
register, which would still make 144.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Chen-Yu Tsai]

On Sat, May 9, 2015 at 7:44 PM, Maxime Ripard
 wrote:
> On Wed, May 06, 2015 at 10:47:33PM +0200, Jens Kuske wrote:
>> >> + * You should have received a copy of the GNU General Public
>> >> + * License along with this file; if not, write to the Free
>> >> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
>> >> + * MA 02110-1301 USA
>> >
>> > Could you remove that last paragraph?
>> > It generates a checkpatch warning.
>>
>> Sure, will be removed. Just copied it from some other sunxi dtsi.
>
> Yeah, I know, I'm even the one that introduced this in the first place
> :)
>
> I sent a patch earlier this week to remove it from the other DT.
>
>> >> +  ahb12_rst: reset@01c202c0 {
>> >> +  #reset-cells = <1>;
>> >> +  compatible = "allwinner,sun6i-a31-clock-reset";
>> >> +  reg = <0x01c202c0 0xc>;
>> >> +  };
>> >
>> > This reset controller also resets the timers, it should be initialised
>> > much earlier.
>> >
>> > What about having an allwinner,sun8i-h3-bus-reset, and adding it to
>> > the list of compatibles to initialise earlier in
>> > drivers/reset/reset-sunxi.c?
>> >
>> > Of course, it would cover the other reset controllers that you have
>> > below.
>> >
>>
>> You mean using a single bus_rst instead of the three?
>
> Yes.
>
>> Or, why not using allwinner,sun6i-a31-ahb1-reset for ahb12_rst
>
> Strictly speaking, they do not control the same set of devices. I'd
> prefer to have a different compatible in case we need to setup a
> particular behaviour on a given SoC (for example, force out of reset a
> particular device, even if no driver is actually using it), without
> impacting the other.
>
>> and adding a .init_time = sun6i_timer_init to the sun8i machine.
>
> But we will need to do that yes.
>
>> I'm a bit confused here now, because for A23, which is almost
>> identical, it got removed after your comment:
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265064.html
>
> H, I think I somehow overlooked the fact that the timer was there,
> even though Chen-Yu said it. My bad :/

On the A23, the high speed timer block only has 1 timer. The sun5i-hrtimer
driver requires 2, and turns out we weren't using them anyway, so I just
dropped sun5i-hrtimer support on A23.

If the other sun8i SoCs have 2 or more timers, feel free to support them.

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

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Chen-Yu Tsai]

On Sat, May 9, 2015 at 7:44 PM, Maxime Ripard
maxime.rip...@free-electrons.com wrote:
 On Wed, May 06, 2015 at 10:47:33PM +0200, Jens Kuske wrote:
  + * You should have received a copy of the GNU General Public
  + * License along with this file; if not, write to the Free
  + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
  + * MA 02110-1301 USA
 
  Could you remove that last paragraph?
  It generates a checkpatch warning.

 Sure, will be removed. Just copied it from some other sunxi dtsi.

 Yeah, I know, I'm even the one that introduced this in the first place
 :)

 I sent a patch earlier this week to remove it from the other DT.

  +  ahb12_rst: reset@01c202c0 {
  +  #reset-cells = 1;
  +  compatible = allwinner,sun6i-a31-clock-reset;
  +  reg = 0x01c202c0 0xc;
  +  };
 
  This reset controller also resets the timers, it should be initialised
  much earlier.
 
  What about having an allwinner,sun8i-h3-bus-reset, and adding it to
  the list of compatibles to initialise earlier in
  drivers/reset/reset-sunxi.c?
 
  Of course, it would cover the other reset controllers that you have
  below.
 

 You mean using a single bus_rst instead of the three?

 Yes.

 Or, why not using allwinner,sun6i-a31-ahb1-reset for ahb12_rst

 Strictly speaking, they do not control the same set of devices. I'd
 prefer to have a different compatible in case we need to setup a
 particular behaviour on a given SoC (for example, force out of reset a
 particular device, even if no driver is actually using it), without
 impacting the other.

 and adding a .init_time = sun6i_timer_init to the sun8i machine.

 But we will need to do that yes.

 I'm a bit confused here now, because for A23, which is almost
 identical, it got removed after your comment:
 http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265064.html

 H, I think I somehow overlooked the fact that the timer was there,
 even though Chen-Yu said it. My bad :/

On the A23, the high speed timer block only has 1 timer. The sun5i-hrtimer
driver requires 2, and turns out we weren't using them anyway, so I just
dropped sun5i-hrtimer support on A23.

If the other sun8i SoCs have 2 or more timers, feel free to support them.

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

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Wed, May 06, 2015 at 10:47:33PM +0200, Jens Kuske wrote:
> >> + * You should have received a copy of the GNU General Public
> >> + * License along with this file; if not, write to the Free
> >> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> >> + * MA 02110-1301 USA
> > 
> > Could you remove that last paragraph?
> > It generates a checkpatch warning.
> 
> Sure, will be removed. Just copied it from some other sunxi dtsi.

Yeah, I know, I'm even the one that introduced this in the first place
:)

I sent a patch earlier this week to remove it from the other DT.

> >> +  ahb12_rst: reset@01c202c0 {
> >> +  #reset-cells = <1>;
> >> +  compatible = "allwinner,sun6i-a31-clock-reset";
> >> +  reg = <0x01c202c0 0xc>;
> >> +  };
> > 
> > This reset controller also resets the timers, it should be initialised
> > much earlier.
> > 
> > What about having an allwinner,sun8i-h3-bus-reset, and adding it to
> > the list of compatibles to initialise earlier in
> > drivers/reset/reset-sunxi.c?
> > 
> > Of course, it would cover the other reset controllers that you have
> > below.
> > 
> 
> You mean using a single bus_rst instead of the three?

Yes.

> Or, why not using allwinner,sun6i-a31-ahb1-reset for ahb12_rst

Strictly speaking, they do not control the same set of devices. I'd
prefer to have a different compatible in case we need to setup a
particular behaviour on a given SoC (for example, force out of reset a
particular device, even if no driver is actually using it), without
impacting the other.

> and adding a .init_time = sun6i_timer_init to the sun8i machine.

But we will need to do that yes.

> I'm a bit confused here now, because for A23, which is almost
> identical, it got removed after your comment:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265064.html

H, I think I somehow overlooked the fact that the timer was there,
even though Chen-Yu said it. My bad :/

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Wed, May 06, 2015 at 10:47:33PM +0200, Jens Kuske wrote:
  + * You should have received a copy of the GNU General Public
  + * License along with this file; if not, write to the Free
  + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
  + * MA 02110-1301 USA
  
  Could you remove that last paragraph?
  It generates a checkpatch warning.
 
 Sure, will be removed. Just copied it from some other sunxi dtsi.

Yeah, I know, I'm even the one that introduced this in the first place
:)

I sent a patch earlier this week to remove it from the other DT.

  +  ahb12_rst: reset@01c202c0 {
  +  #reset-cells = 1;
  +  compatible = allwinner,sun6i-a31-clock-reset;
  +  reg = 0x01c202c0 0xc;
  +  };
  
  This reset controller also resets the timers, it should be initialised
  much earlier.
  
  What about having an allwinner,sun8i-h3-bus-reset, and adding it to
  the list of compatibles to initialise earlier in
  drivers/reset/reset-sunxi.c?
  
  Of course, it would cover the other reset controllers that you have
  below.
  
 
 You mean using a single bus_rst instead of the three?

Yes.

 Or, why not using allwinner,sun6i-a31-ahb1-reset for ahb12_rst

Strictly speaking, they do not control the same set of devices. I'd
prefer to have a different compatible in case we need to setup a
particular behaviour on a given SoC (for example, force out of reset a
particular device, even if no driver is actually using it), without
impacting the other.

 and adding a .init_time = sun6i_timer_init to the sun8i machine.

But we will need to do that yes.

 I'm a bit confused here now, because for A23, which is almost
 identical, it got removed after your comment:
 http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/265064.html

H, I think I somehow overlooked the fact that the timer was there,
even though Chen-Yu said it. My bad :/

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jens Kuske]

Hi,

On 06/05/15 14:19, Maxime Ripard wrote:
> On Wed, May 06, 2015 at 11:31:32AM +0200, Jens Kuske wrote:
>> The Allwinner H3 is a home entertainment system oriented SoC with
>> four Cortex-A7 cores and a Mali-400MP2 GPU.
>>
>> Signed-off-by: Jens Kuske 
>> ---
>>  arch/arm/boot/dts/sun8i-h3.dtsi | 468 
>> 
>>  1 file changed, 468 insertions(+)
>>  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
>>
>> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi 
>> b/arch/arm/boot/dts/sun8i-h3.dtsi
>> new file mode 100644
>> index 000..53aab95
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
>> @@ -0,0 +1,468 @@
>> +/*
>> + * Copyright (C) 2015 Jens Kuske 
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
>> + *
>> + *  a) This file is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of the
>> + * License, or (at your option) any later version.
>> + *
>> + * This file is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public
>> + * License along with this file; if not, write to the Free
>> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
>> + * MA 02110-1301 USA
> 
> Could you remove that last paragraph?
> It generates a checkpatch warning.

Sure, will be removed. Just copied it from some other sunxi dtsi.

> 
>> + *
>> + * Or, alternatively,
>> + *
>> + *  b) Permission is hereby granted, free of charge, to any person
>> + * obtaining a copy of this software and associated documentation
>> + * files (the "Software"), to deal in the Software without
>> + * restriction, including without limitation the rights to use,
>> + * copy, modify, merge, publish, distribute, sublicense, and/or
>> + * sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following
>> + * conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> + * included in all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
>> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
>> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
>> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
>> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#include "skeleton.dtsi"
>> +
>> +#include 
>> +#include 
>> +
>> +/ {
>> +interrupt-parent = <>;
>> +
>> +cpus {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +cpu@0 {
>> +compatible = "arm,cortex-a7";
>> +device_type = "cpu";
>> +reg = <0>;
>> +};
>> +
>> +cpu@1 {
>> +compatible = "arm,cortex-a7";
>> +device_type = "cpu";
>> +reg = <1>;
>> +};
>> +
>> +cpu@2 {
>> +compatible = "arm,cortex-a7";
>> +device_type = "cpu";
>> +reg = <2>;
>> +};
>> +
>> +cpu@3 {
>> +compatible = "arm,cortex-a7";
>> +device_type = "cpu";
>> +reg = <3>;
>> +};
>> +};
>> +
>> +memory {
>> +reg = <0x4000 0x8000>;
>> +};
>> +
>> +clocks {
>> +#address-cells = <1>;
>> +#size-cells = <1>;
>> +ranges;
>> +
>> +osc24M: osc24M_clk {
>> +#clock-cells = <0>;
>> +compatible = "fixed-clock";
>> +clock-frequency = <2400>;
>> +clock-output-names = "osc24M";
>> +};
>> +
>> +osc32k: osc32k_clk {
>> +#clock-cells = <0>;
>> +compatible = "fixed-clock";
>> +clock-frequency = <32768>;
>> +clock-output-names = "osc32k";
>> +};
>> +
>> +pll1: clk@01c2 {
>> +#clock-cells = <0>;
>> +compatible = 

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Wed, May 06, 2015 at 11:31:32AM +0200, Jens Kuske wrote:
> The Allwinner H3 is a home entertainment system oriented SoC with
> four Cortex-A7 cores and a Mali-400MP2 GPU.
> 
> Signed-off-by: Jens Kuske 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 468 
> 
>  1 file changed, 468 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
> 
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> new file mode 100644
> index 000..53aab95
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -0,0 +1,468 @@
> +/*
> + * Copyright (C) 2015 Jens Kuske 
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public
> + * License along with this file; if not, write to the Free
> + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
> + * MA 02110-1301 USA

Could you remove that last paragraph?
It generates a checkpatch warning.

> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "skeleton.dtsi"
> +
> +#include 
> +#include 
> +
> +/ {
> + interrupt-parent = <>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu@0 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <0>;
> + };
> +
> + cpu@1 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <1>;
> + };
> +
> + cpu@2 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <2>;
> + };
> +
> + cpu@3 {
> + compatible = "arm,cortex-a7";
> + device_type = "cpu";
> + reg = <3>;
> + };
> + };
> +
> + memory {
> + reg = <0x4000 0x8000>;
> + };
> +
> + clocks {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + osc24M: osc24M_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <2400>;
> + clock-output-names = "osc24M";
> + };
> +
> + osc32k: osc32k_clk {
> + #clock-cells = <0>;
> + compatible = "fixed-clock";
> + clock-frequency = <32768>;
> + clock-output-names = "osc32k";
> + };
> +
> + pll1: clk@01c2 {
> + #clock-cells = <0>;
> + compatible = "allwinner,sun8i-a23-pll1-clk";
> + reg = <0x01c2 0x4>;
> + clocks = <>;
> + clock-output-names = "pll1";
> + };
> +
> + pll6: 

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Maxime Ripard]

On Wed, May 06, 2015 at 11:31:32AM +0200, Jens Kuske wrote:
 The Allwinner H3 is a home entertainment system oriented SoC with
 four Cortex-A7 cores and a Mali-400MP2 GPU.
 
 Signed-off-by: Jens Kuske jensku...@gmail.com
 ---
  arch/arm/boot/dts/sun8i-h3.dtsi | 468 
 
  1 file changed, 468 insertions(+)
  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi
 
 diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
 new file mode 100644
 index 000..53aab95
 --- /dev/null
 +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
 @@ -0,0 +1,468 @@
 +/*
 + * Copyright (C) 2015 Jens Kuske jensku...@gmail.com
 + *
 + * This file is dual-licensed: you can use it either under the terms
 + * of the GPL or the X11 license, at your option. Note that this dual
 + * licensing only applies to this file, and not this project as a
 + * whole.
 + *
 + *  a) This file is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as
 + * published by the Free Software Foundation; either version 2 of the
 + * License, or (at your option) any later version.
 + *
 + * This file is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public
 + * License along with this file; if not, write to the Free
 + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
 + * MA 02110-1301 USA

Could you remove that last paragraph?
It generates a checkpatch warning.

 + *
 + * Or, alternatively,
 + *
 + *  b) Permission is hereby granted, free of charge, to any person
 + * obtaining a copy of this software and associated documentation
 + * files (the Software), to deal in the Software without
 + * restriction, including without limitation the rights to use,
 + * copy, modify, merge, publish, distribute, sublicense, and/or
 + * sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following
 + * conditions:
 + *
 + * The above copyright notice and this permission notice shall be
 + * included in all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND,
 + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
 + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
 + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
 + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 + * OTHER DEALINGS IN THE SOFTWARE.
 + */
 +
 +#include skeleton.dtsi
 +
 +#include dt-bindings/interrupt-controller/arm-gic.h
 +#include dt-bindings/pinctrl/sun4i-a10.h
 +
 +/ {
 + interrupt-parent = gic;
 +
 + cpus {
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + cpu@0 {
 + compatible = arm,cortex-a7;
 + device_type = cpu;
 + reg = 0;
 + };
 +
 + cpu@1 {
 + compatible = arm,cortex-a7;
 + device_type = cpu;
 + reg = 1;
 + };
 +
 + cpu@2 {
 + compatible = arm,cortex-a7;
 + device_type = cpu;
 + reg = 2;
 + };
 +
 + cpu@3 {
 + compatible = arm,cortex-a7;
 + device_type = cpu;
 + reg = 3;
 + };
 + };
 +
 + memory {
 + reg = 0x4000 0x8000;
 + };
 +
 + clocks {
 + #address-cells = 1;
 + #size-cells = 1;
 + ranges;
 +
 + osc24M: osc24M_clk {
 + #clock-cells = 0;
 + compatible = fixed-clock;
 + clock-frequency = 2400;
 + clock-output-names = osc24M;
 + };
 +
 + osc32k: osc32k_clk {
 + #clock-cells = 0;
 + compatible = fixed-clock;
 + clock-frequency = 32768;
 + clock-output-names = osc32k;
 + };
 +
 + pll1: clk@01c2 {
 + #clock-cells = 0;
 + compatible = allwinner,sun8i-a23-pll1-clk;
 + reg = 0x01c2 0x4;
 + clocks = osc24M;
 + clock-output-names = pll1;
 + };
 +
 + pll6: clk@01c20028 {
 + #clock-cells = 1;
 + 

Re: [PATCH 5/6] ARM: dts: sunxi: Add Allwinner H3 DTSI

[Jens Kuske]

Hi,

On 06/05/15 14:19, Maxime Ripard wrote:
 On Wed, May 06, 2015 at 11:31:32AM +0200, Jens Kuske wrote:
 The Allwinner H3 is a home entertainment system oriented SoC with
 four Cortex-A7 cores and a Mali-400MP2 GPU.

 Signed-off-by: Jens Kuske jensku...@gmail.com
 ---
  arch/arm/boot/dts/sun8i-h3.dtsi | 468 
 
  1 file changed, 468 insertions(+)
  create mode 100644 arch/arm/boot/dts/sun8i-h3.dtsi

 diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi 
 b/arch/arm/boot/dts/sun8i-h3.dtsi
 new file mode 100644
 index 000..53aab95
 --- /dev/null
 +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
 @@ -0,0 +1,468 @@
 +/*
 + * Copyright (C) 2015 Jens Kuske jensku...@gmail.com
 + *
 + * This file is dual-licensed: you can use it either under the terms
 + * of the GPL or the X11 license, at your option. Note that this dual
 + * licensing only applies to this file, and not this project as a
 + * whole.
 + *
 + *  a) This file is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as
 + * published by the Free Software Foundation; either version 2 of the
 + * License, or (at your option) any later version.
 + *
 + * This file is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public
 + * License along with this file; if not, write to the Free
 + * Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston,
 + * MA 02110-1301 USA
 
 Could you remove that last paragraph?
 It generates a checkpatch warning.

Sure, will be removed. Just copied it from some other sunxi dtsi.

 
 + *
 + * Or, alternatively,
 + *
 + *  b) Permission is hereby granted, free of charge, to any person
 + * obtaining a copy of this software and associated documentation
 + * files (the Software), to deal in the Software without
 + * restriction, including without limitation the rights to use,
 + * copy, modify, merge, publish, distribute, sublicense, and/or
 + * sell copies of the Software, and to permit persons to whom the
 + * Software is furnished to do so, subject to the following
 + * conditions:
 + *
 + * The above copyright notice and this permission notice shall be
 + * included in all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND,
 + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
 + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
 + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
 + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 + * OTHER DEALINGS IN THE SOFTWARE.
 + */
 +
 +#include skeleton.dtsi
 +
 +#include dt-bindings/interrupt-controller/arm-gic.h
 +#include dt-bindings/pinctrl/sun4i-a10.h
 +
 +/ {
 +interrupt-parent = gic;
 +
 +cpus {
 +#address-cells = 1;
 +#size-cells = 0;
 +
 +cpu@0 {
 +compatible = arm,cortex-a7;
 +device_type = cpu;
 +reg = 0;
 +};
 +
 +cpu@1 {
 +compatible = arm,cortex-a7;
 +device_type = cpu;
 +reg = 1;
 +};
 +
 +cpu@2 {
 +compatible = arm,cortex-a7;
 +device_type = cpu;
 +reg = 2;
 +};
 +
 +cpu@3 {
 +compatible = arm,cortex-a7;
 +device_type = cpu;
 +reg = 3;
 +};
 +};
 +
 +memory {
 +reg = 0x4000 0x8000;
 +};
 +
 +clocks {
 +#address-cells = 1;
 +#size-cells = 1;
 +ranges;
 +
 +osc24M: osc24M_clk {
 +#clock-cells = 0;
 +compatible = fixed-clock;
 +clock-frequency = 2400;
 +clock-output-names = osc24M;
 +};
 +
 +osc32k: osc32k_clk {
 +#clock-cells = 0;
 +compatible = fixed-clock;
 +clock-frequency = 32768;
 +clock-output-names = osc32k;
 +};
 +
 +pll1: clk@01c2 {
 +#clock-cells = 0;
 +compatible = allwinner,sun8i-a23-pll1-clk;
 +reg = 0x01c2 0x4;
 +clocks = osc24M;
 +clock-output-names = pll1;
 +};
 +
 +pll6: clk@01c20028 {