Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2006-10-31 at 07:59 +0100, Sylvain Munaut wrote:

> * struct mpc52xx_mmap_ctl;
> * struct mpc52xx_sdram;
> 
> Not really used any where that I can see/remember. Except for
> find_end_of_memory ...
> It should however be used in several place in the future ... (sleep support
> would need sdram iirc, ...).
> 
> But can be removed for now if it's annoying to have them there ...

Nah, keep them in. It's not like it was bloating the binary anyway :)
> 
> * struct mpc52xx_intr;
> 
> Was used before in platform support code to set the IRQ type of external
> IRQ (level/irq) ...
> but that can be done with set_irq_type. So can be safetly moved to a
> local mpc52xx_pic.h

Yup.

> * struct mpc52xx_rtc;
> 
> Was used before in some common code. When the bootloader didn't pass the
> bus frequency,
> we computed it and the rtc was used to do that. Now, with device tree,
> no need for
> that anymore. So can be safely removed.

Sounds good.

> 
> * struct mpc52xx_gpio;
> * struct mpc52xx_gpio_wkup;
> 
> Port config (pin multiplexing) is in those registers so they should stay
> there. This is used
> by several driver and platform code. Beside custom driver could use gpio
> for different
> purpose ...

Yup, though beware of concurrent access to GPIO registers... we might
want a bit of common code with a spinlock in it to "wrap" accesses to
them.

> It could be placed in a include/asm-powerpc/mpc52xx_gpio.h but that
> would just make
> one more file in include/asm-powerpc so it doesn't make much sens imho.
> It should
> just stay there.

Yeah, leave it there.
> 
> * struct mpc52xx_xlb;
> * struct mpc52xx_cdm;
> * struct mpc52xx_sdma;
> 
> Used at several place and should really stay there.

No need to be too anal about removing things from .h files.

Ben.