Re: [PATCH/RFC] powerpc: Add MPC5200 Interrupt Controller support.

[Nicolas DET <nd@bplan-gmbh.de>]

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

Sylvain Munaut wrote:
>> +
>> +	/*
>> +	 * Most of ours IRQs will be level low
>> +	 * Only external IRQs on some platform may be others
>> +	 */
>> +	type = IRQ_TYPE_LEVEL_LOW;
>>   
> I've been wondering : Why LEVEL_LOW ?
> Aren't they LEVEL_HIGH ?
> (not that it changes much here ...)
> 

It makes only sense for the ext interrupts and not for the internal 
peripherals.
I made everything level low because it's quiet common (for example: 
PCI). Moreover, On our hardware (Efika) the IRQ[0-3] are dedicated to 
the PCI.

Anyway, as the current code looks inside the chipset register to 
retrieve the real settings, the 'type' vaiiable will be overwrite if 
require.

In my opinion, others platforms (without proper hw init by the firmware) 
may overwrite the external interrupt control reg in their platforms 
specific part.

>> +
>> +end:
>> +	of_node_put(picnode);
>> +	of_node_put(sdmanode);
>>   
> Is of_node_put specified as resilient to NULL argument ? (in the error
> path, that could happen)

of_node_put() is said to be NULL pointer safe.

> 
> Also, I think "PANIC" would be appropriate in the error path. If we
> can't init
> the PIC properly we may as well give up ... It's not like we're going to
> do much
> without it ...
> 

Well, panic or not your board won't do much ;-)
Moreover, the serial console should work a bit without interrupt.

On another hand, init_IRQ may return a value to indicate it succeed or 
not. but, this is beyound the scope of this patch.

Regards,

[-- Attachment #2: nd.vcf --]
[-- Type: text/x-vcard, Size: 249 bytes --]

begin:vcard
fn:Nicolas DET ( bplan GmbH )
n:DET;Nicolas
org:bplan GmbH
adr:;;;;;;Germany
email;internet:nd@bplan-gmbh.de
title:Software Entwicklung
tel;work:+49 6171 9187 - 31
x-mozilla-html:FALSE
url:http://www.bplan-gmbh.de
version:2.1
end:vcard