Re: [PATCH 1/2] ARM: dts: exynos4210-origen: Add fixed voltage regulator to simple bus

[Sachin Kamat <sachin.kamat@linaro.org>]

Hi Tomasz,

On 26 September 2013 13:50, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Sachin,
>
> I must have missed this patch so better later than never ;).

Since you are quite prompt at reviewing I thought you did not have any
comments :)

> Please see my comments inline.
>
> On Thursday 26 of September 2013 10:32:01 Sachin Kamat wrote:
>> > +       regulators {
>> > +               compatible = "simple-bus";
>> > +               #address-cells = <1>;
>> > +               #size-cells = <0>;
>> > +
>> > +               mmc_reg: voltage-regulator {
>
> For consistency and correctness, since this is a bus, even if not
> physical, it is worth to add reg property and unit-address to subnodes.
> Since this bus is not physical, the reg property would not be anything
> more than purely an index.
>
> Also for consistency I'd use "regulator" as node name here, as this is
> what is used most commonly across all the dts files in arch/arm/boot/dts.

Sounds good. Will update accordingly. While at it will also update the
Arndale dts
file to reflect your suggestions for a similar node there.

Thank you for your review and feedback.
-- 
With warm regards,
Sachin