lkml.org 
[lkml]   [2015]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 4/9] ARM: dts: add dts files for hi3519-demb board
Date
On Friday 04 December 2015 10:27:58 xuejiancheng wrote:
> >
> >> + sysctrl: system-controller@12020000 {
> >> + compatible = "hisilicon,sysctrl";
> >> + reg = <0x12020000 0x1000>;
> >> + reboot-offset = <0x4>;
> >> + };
> >
> > Is this one identical to the one in hip04?
> >
> > If not, pick a new unique compatible string
>
> Yes. It's compatible with the one in hip04.

Ok, we should add a compatible string for that then, as the hip04 apparently
has a slightly different layout from hip01.

Can you add a separate patch to clarify the existing hisilicon,sysctrl nodes?

It look like hi3620 accidentally uses the same string as hip04 already
but is actually a bit different.

Maybe split out the sysctrl binding from
Documentation/devicetree/bindings/arm/hisilicon/hisilicon.txt, as it has
you already have a couple of those, and it's not clear how they relate
to one another.

If we introduce a string for all hip04 compatible sysctrl devices, we should
document that and use it consistently, so hi3519 becomes

compatible = "hisilicon,hi3519-sysctrl", "hisilicon,hip04-sysctrl", "hisilicon,sysctrl";

but I'd clarify in the binding documentation that "hisilicon,sysctrl" should
only be used for hip04 and hi3519 but not the others.

As this seems to be a standard part, we can also think about making a
high-level driver for in in drivers/soc rather than relying on the syscon
driver which we tend to use more for one-off devices with random register
layouts.

> >> +
> >> + crg: crg@12010000 {
> >> + compatible = "hisilicon,hi3519-crg";
> >
> >
> > what is a "crg"? Is there a standard name for these?
>
> The "crg" means Clock and Reset Generator. It's a block which supplies clock
> and reset signals for other modules in the chip. I used "hi3519-clock"
> in last version patch. Rob Herring suggested that it's better to use "hi3519-crg"
> as the compatible string if it's a whole block.
>
> what about writing like this?
>
> crg: clock-reset-controller@12010000 {
> compatible = "hisilicon,hi3519-crg";
> }
>

Ok, that's better.

Arnd


\
 
 \ /
  Last update: 2015-12-04 12:01    [W:0.577 / U:0.044 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site