Eugene Surovegin wrote: >On Wed, May 04, 2005 at 06:35:16PM +0400, Vitaly Bordug wrote: > > >>This patch adds generic PlatformDevice support to the 82xx family. >>Only FCC's exist currently in the structure, as there is the driver >>which will utilize this. >> >> >> > >[snip] > > > >>+#ifdef CONFIG_CPM2 >>+ identify_ppc_sys_by_id(cpm2_immr->im_memctl.memc_immr << 16); >>+ >>+ /* Set up the MAC addresses for the FECs >>+ */ >>+ fec = ppc_sys_platform_devices[MPC82xx_FCC1].dev.platform_data; >>+ memcpy(fec->mac_addr,bi->bi_enetaddr,6); >>+ >>+ fec = ppc_sys_platform_devices[MPC82xx_FCC2].dev.platform_data; >>+#ifdef CONFIG_ADS8272 >>+ memcpy(fec->mac_addr,bi->bi_enet1addr,6); >>+#else >>+ memcpy(fec->mac_addr,bi->bi_enetaddr,6); >>+ fec->macaddr[5] ^= 1; >>+#endif >>+#endif >> >> > >What is this? Why does common file contain board specific ifdefs???? > >[snip] > > > >>+/* FCC1 Clock Source Configuration. There are board specific. >>+ Can only choose from CLK9-12 */ >>+#if defined(CONFIG_ADS8272) >>+#define F1_RXCLK 11 >>+#define F1_TXCLK 10 >>+#else >>+#define F1_RXCLK 12 >>+#define F1_TXCLK 11 >>+#endif >> >> > >Same thing. Why on earth you continue current 8xxx trend of putting >board specific crap into common files? > > > >>+ >>+/* FCC2 Clock Source Configuration. There are board specific. >>+ Can only choose from CLK13-16 */ >>+#ifdef CONFIG_ADS8272 >>+#define F2_RXCLK 15 >>+#define F2_TXCLK 16 >>+#else >>+#define F2_RXCLK 13 >>+#define F2_TXCLK 14 >>+#endif >> >> > >Ditto. > > > >>+#ifdef CONFIG_ADS8272 >>+#define PC_MDIO 0x00002000U >>+#define PC_MDCK 0x00001000U >>+#else >>+#define PC_MDIO 0x00000004U >>+#define PC_MDCK 0x00000020U >>+#endif >> >> > >Ditto. > > > >>+ .name = "phyinterrupt", >>+ .start = SIU_INT_IRQ5, >>+ .end = SIU_INT_IRQ5, >>+ .flags = IORESOURCE_IRQ, >>+ }, >> >> > >Why is this here? PHY interrupt routing is _board_ specific. > > > I have fixed these yet, but I think it will be more reasonable to update this together with respective driver which I'll work on together with Pantelis. Thank you for review.
-- Sincerely, Vitaly -------------- next part -------------- An HTML attachment was scrubbed... URL: http://ozlabs.org/pipermail/linuxppc-embedded/attachments/20050505/dd1cb08d/attachment.htm
