[PATCH 12/14] ARM: dts: Introduce STM32F429 MCU

[Maxime Coquelin]

Hi Philipp,

2015-02-13 20:18 GMT+01:00 Philipp Zabel <p.zabel at pengutronix.de>:
> Am Freitag, den 13.02.2015, 17:41 +0100 schrieb Maxime Coquelin:
>> 2015-02-13 17:25 GMT+01:00 Philipp Zabel <p.zabel at pengutronix.de>:
>> > Hi Maxime,
>> >
>> > Am Freitag, den 13.02.2015, 16:59 +0100 schrieb Maxime Coquelin:
>> >> Hi Philipp,
>> >>
>> >> 2015-02-13 12:47 GMT+01:00 Philipp Zabel <p.zabel at pengutronix.de>:
>> >> > Hi Maxime,
>> >> >
>> >> > Am Donnerstag, den 12.02.2015, 18:46 +0100 schrieb Maxime Coquelin:
>> >> > [...]
>> >> >> +     soc {
>> >> >> +             reset_ahb1: reset at 40023810 {
>> >> >> +                     #reset-cells = <1>;
>> >> >> +                     compatible = "st,stm32-reset";
>> >> >> +                     reg = <0x40023810 0x4>;
>> >> >> +             };
>> >> >> +
>> >> >> +             reset_ahb2: reset at 40023814 {
>> >> >> +                     #reset-cells = <1>;
>> >> >> +                     compatible = "st,stm32-reset";
>> >> >> +                     reg = <0x40023814 0x4>;
>> >> >> +             };
>> >> >> +
>> >> >> +             reset_ahb3: reset at 40023818 {
>> >> >> +                     #reset-cells = <1>;
>> >> >> +                     compatible = "st,stm32-reset";
>> >> >> +                     reg = <0x40023818 0x4>;
>> >> >> +             };
>> >> >> +
>> >> >> +             reset_apb1: reset at 40023820 {
>> >> >> +                     #reset-cells = <1>;
>> >> >> +                     compatible = "st,stm32-reset";
>> >> >> +                     reg = <0x40023820 0x4>;
>> >> >> +             };
>> >> >> +
>> >> >> +             reset_apb2: reset at 40023824 {
>> >> >> +                     #reset-cells = <1>;
>> >> >> +                     compatible = "st,stm32-reset";
>> >> >> +                     reg = <0x40023824 0x4>;
>> >> >> +             };
>> >> >
>> >> > These are mostly consecutive, single registers. I wonder if these are
>> >> > part of the same IP block and thus should be grouped together into the
>> >> > same reset controller node?
>> >>
>> >> What I could to is to have two instances. One for AHB and one for APB domain.
>> >> Doing this, I will have one instance per domain, and only consecutive registers.
>> >> Is it fine for you?
>> >
>> > Looking at
>> > http://www.st.com/web/en/resource/technical/document/reference_manual/DM00031020.pdf
>> > Table 34 (RCC register map and reset values), I'd say there is a single
>> > "Reset and Clock Control" device at 0x40023800 - 0x40023884:
>> >
>> >         soc {
>> >                 rcc: rcc at 40023800 {
>> >                         #clock-cells = <1>;
>> >                         #reset-cells = <1>;
>> >                         compatible = "st,stm32-rcc";
>> >                         reg = <0x40023800 0x84>;
>> >                 };
>> >
>> >                 ...
>> >
>> > If you really want to describe the reset controller parts (offsets +0x10
>> > to +0x24) in a separate node, I won't argue against it too long,
>> > although this is a somewhat arbitrary decision.
>> >
>> > In any case, the whole register at offset +0x1c is reserved, so there is
>> > no reason to split the reset controller. It is ok to have unused ranges
>> > as is already the case with reserved bits inside the used registers.
>>
>> Ok. I understand your point.
>> But it will be more difficult at usage, because the node referencing
>> the fourth reset bit of apb2 register will have to pass 164 as
>> parameter.
>> It is error prone IMHO.
>>
>> Other solution would be to add some defines for each reset line in the
>> DT-Bindings, as we do today for STi platform.
>> But it is giving an unneeded constraint between DT and reset trees.
>
> That is a bit unfortunate, but providing the named constants in
> include/dt-bindings/reset/ makes for a much better readable device tree,
> so I'd prefer that solution, even if it means having to coordinate pull
> requests.

Ok, I will add constants in include/dt-bindings/reset/

Thanks,
Maxime

>
> regards
> Philipp
>