From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc From: Benjamin Herrenschmidt To: Sylvain Munaut In-Reply-To: <4546F46E.5080202@246tNt.com> References: <200610292310.k9TNAHXZ013852@post.webmailer.de> <7BDB728E-0CC2-4940-9856-B496022F3482@kernel.crashing.org> <45468775.8040108@246tNt.com> <96623161-B847-4F61-94AB-D1F1B1767708@kernel.crashing.org> <4546F46E.5080202@246tNt.com> Content-Type: text/plain Date: Tue, 31 Oct 2006 18:05:20 +1100 Message-Id: <1162278320.25682.295.camel@localhost.localdomain> Mime-Version: 1.0 Cc: linuxppc-dev@ozlabs.org, sl@bplan-gmbh.de, sha@pengutronix.de, linuxppc-embedded@ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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.