Am 19/09/2011 22:34, schrieb Eric Jarrige: > Hi Stefano, > > On 19 sept. 2011, at 09:26, Stefano Babic wrote: > Hi Eric,
>> On 09/19/2011 08:57 AM, Marek Vasut wrote: >>> On Monday, August 22, 2011 10:56:43 PM Eric Jarrige wrote: >>>> Signed-off-by: Eric Jarrige <eric.jarr...@armadeus.org> >>>> Cc: Stefano Babic <sba...@denx.de> >>>> Cc: Heiko Schocher <h...@denx.de> >>>> @@ -94,6 +98,8 @@ void i2c_init(int speed, int unused) >>>> /* start the required I2C clock */ >>>> writel(readl(&sc_regs->cgr0) | (3 << I2C_CLK_OFFSET), >>>> &sc_regs->cgr0); >>>> +#elif defined(CONFIG_IMX) >>>> + freq = get_HCLK(); >>>> #else >>>> freq = mxc_get_clock(MXC_IPG_PERCLK); >>>> #endif >>> Please, no more ifdefs -- can't you actually modify your header files or >>> add >>> some which would define mxc_get_clock() ? That'd make this way much more >>> clean. >> In principl, I agree completely with Marek, and we have tried to get rid >> as much of possible of #ifdef in drivers. I know we have already >> discussed about the opportunity to fix the whole IMX stuff, and I agreed >> with you that this processor is obsolete and make no sense to invest a >> lot of time for it. However, what about to define mxc_get_clock() as >> simple macro ? Maybe in imx-regs.h, as exceptiion for this processor >> (normally the imx-regs.h contains only defines and structures for the >> internal registers) ? Something like: >> >> #define mxc_get_clock(a) (get_HCLK()) > I agree with you to get rid of all these #ifdef and my current patch does not > help at all. > I can update the imx-regs.h with the change you propose but that won't help > for the > future iMX CPU. > IMHO the problem comes from all the hard coded constants in the driver such as > mxc_get_clock(MXC_IPG_PERCLK) ...I see the clock name is not so generic, and it is not related to the interface.. > and others I2C_BASE... > Every chipset use a different root clock for its i2c controller. > > Why not changing the line > freq = mxc_get_clock(MXC_IPG_PERCLK); > > by something more generic such as a macro: > > freq = MXC_GET_I2C_CLOCK(); /* or GET_MXC_I2C_CLOCK() */ We have already this mechanism - this is the goal of the mxc_get_clock() function: a common way to get clocks for all i.MX processors. We have for example MXC_CSPI_CLK (SPI), MXC_UART_CLK, and so on. The line should become: freq = mxc_get_clock(MXC_I2C_CLK) and then each processor can return its own value. This already works for the other interfaces (FEC, UART, SPI,..) > > that can be defined in clock.h with the correct definition for each chipset? We need only to extend the mxc_clock enumeration type. The implementation of mxc_get_clock() is SOC specific. > Using a macro would clarify that is chipset dependent as each CPU use > its own clock.h > > For the mx1, I can add a clock.h with the correct implementation of the > macro: > #define MXC_GET_I2C_CLOCK() get_HCLK() > or any other solution that avoid any new ifdef. > > Does it makes sense? Yes, it makes sense to change, check my proposal Best regards, Stefano -- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot