[U-Boot] [PATCH] mx1: add mx1/l support for mxc_i2c

Eric Jarrige eric.jarrige at armadeus.org
Tue Sep 20 22:54:38 CEST 2011


Hi Stafano,

On 19 sept. 2011, at 22:59, stefano babic wrote:
> Am 19/09/2011 22:34, schrieb Eric Jarrige:
>> On 19 sept. 2011, at 09:26, Stefano Babic wrote:
>>> On 09/19/2011 08:57 AM, Marek Vasut wrote:
>>>> On Monday, August 22, 2011 10:56:43 PM Eric Jarrige wrote:
>>> 	#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..

Right, IMHO that's the point.

> 
> 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 

Your proposal looks good and should be easy to implement for all new iMX CPU.

What is the main added values of the unique method "mxc_get_clock" compared
to the other model? 
On PowerPC the frequencies are saved in the gd-> structure whereas the iMX legacy
principle used one accessor method for each clock but that lacks of a common
naming convention.
I can imagine that using the gd-> structure optimize the memory footprint on PowerPC.

What is your strategy regarding the legacy SOCs? 
Do you have a refactoring plan to update the other CPU to the new interface
mxc_get_clock() ? 

A wrapping "#define mxc_get_clock(a)	(get_HCLK())" in clock.h should do the job for
the mx1 but is it a common rule or a kind of exception for the mx1?

Sorry still few questions regarding some remaining open points?
What are your plan/ideas to remove the list of #ifdef that provides the i2C_BASE
and CLOCK_OFFSET for each SOC?

Same question for the hard coded table of clock dividers  "u16 div[]" as this table may
change from CPU to CPU ?

Thanks and best regards,
Eric

 





More information about the U-Boot mailing list