Re: [Patch v4 01/12] microblaze: irqchip: Move intc driver to irqchip

[Michal Simek]

On 2.9.2016 12:06, Zubair Lutfullah Kakakhel wrote:
> Hi,
> 
> On 09/02/2016 07:25 AM, Michal Simek wrote:
>> On 1.9.2016 18:50, Zubair Lutfullah Kakakhel wrote:
>>> The Xilinx AXI Interrupt Controller IP block is used by the MIPS
>>> based xilfpga platform.
>>>
>>> Move the interrupt controller code out of arch/microblaze so that
>>> it can be used by everyone
>>>
>>> Signed-off-by: Zubair Lutfullah Kakakhel <Zubair.Kakakhel@xxxxxxxxxx>
>>>
>>> ---
>>> V3 -> V4
>>> No change
>>>
>>> V2 -> V3
>>> No change here. Cleanup patches follow after this patch.
>>> Its debatable to cleanup before/after move. Decided to place cleanup
>>> after move to put history in new place.
>>>
>>> V1 -> V2
>>>
>>> Renamed irq-xilinx to irq-axi-intc
>>> Renamed CONFIG_XILINX_INTC to CONFIG_XILINX_AXI_INTC
>>
>>
>> I see that this was suggested by Jason Cooper but using axi name here is
>> not correct.
>> There is xps-intc name which is the name used on old OPB hardware
>> designs. It means this driver can be still used only on system which
>> uses it.
> 
> Wouldn't axi-intc be more suitable moving forwards?
> The IP block is now known as axi intc for 5 years as far as I can tell.
> 
> Searching "axi intc" online results in the right docs for current and
> future platforms.

yes but we still should support older platform and it is more then this.
This is soft-IP core and in future when there is new bus then IP will
just change bus interface, etc.

> 
> The binding is still xps-intc as that won't change. So older systems
> should still be able to find their way.

yes that's not a problem. But in general having bus name in name is not
a good way to go.

> 
>> Also there is another copy of this driver in the tree which was using
>> old ppc405 and ppc440 xilinx platforms.
>>
>> arch/powerpc/include/asm/xilinx_intc.h
>> arch/powerpc/sysdev/xilinx_intc.c
>>
>> These should be also removed by moving this driver to generic folder.
> 
> I didn't know about that drivers existence.
> 
> This patch series already touches microblaze, mips and irqchip.
> Both microblaze and mips platforms using this driver are little-endian.

MB is big ending too and as you see there is big endian support in the
driver already.

> 
> Adding a big-endian powerpc driver + platform to the mix is going to
> complicate
> the series further and make it super hard to synchronize various
> subsystems,
> test stuff, and then move the drivers without breakage.
> 
> I'd highly recommend letting this move happen. And then the powerpc
> driver can
> transition over time to this driver.

I have no problem with this but you should be aware about it.
PPC will remain to use big endiand PLB bus. It means it is not axi too.

Thanks,
Michal