Re: [PATCH v3] ARM: at91: pit add DT support

[Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>]

On 01/05/2012 05:42 PM, Rob Herring :
> On 01/05/2012 11:25 AM, Nicolas Ferre wrote:
>> From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>>
>> Retreive registers address and IRQ from device tree entry. Fall back
>> to built-in values if an error occurs.
>>
>> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
>> [nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org: change error path and interrupts property handling]
>> Signed-off-by: Nicolas Ferre <nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>
>> ---
>> v3: - use irq_of_parse_and_map() for handling irq numbers specified by DT.
>>       Correction proposed by Jamie Iles.
>>
>> v2: - new specification of irq numbers in DT (due to modification of AIC code)
>>     - new error path in of_at91sam926x_pit_init()
>>     - fall back to built-in values if an error occurs
>>     - use of of_property_read_u32() to get irq property
>>
>>  .../devicetree/bindings/arm/atmel-at91.txt         |    8 +++
>>  arch/arm/boot/dts/at91sam9g20.dtsi                 |    5 ++
>>  arch/arm/boot/dts/at91sam9g45.dtsi                 |    6 ++
>>  arch/arm/mach-at91/at91sam926x_time.c              |   53 +++++++++++++++++++-
>>  4 files changed, 70 insertions(+), 2 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/arm/atmel-at91.txt

[..]

>> --- a/arch/arm/mach-at91/at91sam926x_time.c
>> +++ b/arch/arm/mach-at91/at91sam926x_time.c

[..]

>>  void __init at91sam926x_ioremap_pit(u32 addr)
>>  {
>> +	if (!of_at91sam926x_pit_init())
>> +		return;
> 
> This seems backwards to me. I don't have the ioremap changes in my tree,
> but shouldn't the caller of at91sam926x_ioremap_pit be changed to
> something like this:
> 
> 	if (of_at91sam926x_pit_init() < 0)
> 		at91sam926x_ioremap_pit(addr);

Yes, you are right, it will be more readable the other way around.

I repost another revision now.

> Otherwise,
> Acked-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

Thanks for your review.

Best regards,
-- 
Nicolas Ferre