[U-Boot] I2C on iMX25

[Matthias Weißer]

Am 24.09.2012 13:05, schrieb Benoît Thébaudeau:
> Hi Stefano, Matthias,
>
> On Monday, September 24, 2012 11:45:33 AM, Stefano Babic wrote:
>> On 24/09/2012 11:32, Matthias Weißer wrote:
>> > Hi Stefano
>> >
>>
>> Hi Matthias,
>>
>> > I am currently in the process of updating my zmx25 board support
>> > for a new
>> > hardware revision where I need I2C access. I2C on imx25 currently
>> > fails
>> > to build:
>> >
>> > mxc_i2c.c: In function 'i2c_imx_get_clk':
>> > mxc_i2c.c:101:31: error: 'MXC_IPG_PERCLK' undeclared (first use in
>> > this
>> > function)
>>
>> Ok, I see.
>
> I had the same issue a while ago. I have a fix for that. I will try to post it
> tonight.
>
>> > I can easily fix this by replacing MXC_IPG_PERCLK with MXC_I2C_CLK.
>> > But
>> > MXC_I2C_CLK is only defined for imx25. So, this change will break
>> > all other
>> > imx chips.
>>
>> But this seems the right solution. The mxc_get_clk() gets as
>> parameter
>> an enum representing a peripheral or a special clock name, valid for
>> a
>> SOC. The driver should use the peripheral name.
>
> Yes and no. The best would be to add a clock abstraction function
> imx_get_i2cclk(), like what exists for UART. This is what I did.

What is the advantage of such a function over i2c_imx_get_clk(MXC_I2C_CLK)?

>> Really I think the right way is to add MXC_I2C_CLK to the other SOCs,
>> adding the case in their specific mxc_get_clock() implementation, for
>> example for mx6 something like this:
>>
>> diff --git a/arch/arm/cpu/armv7/mx5/clock.c
>> b/arch/arm/cpu/armv7/mx5/clock.c
>> index c67c3cf..8fa737a 100644
>> --- a/arch/arm/cpu/armv7/mx5/clock.c
>> +++ b/arch/arm/cpu/armv7/mx5/clock.c
>> @@ -482,6 +482,7 @@ unsigned int mxc_get_clock(enum mxc_clock clk)
>>         case MXC_IPG_CLK:
>>                 return get_ipg_clk();
>>         case MXC_IPG_PERCLK:
>> +       case MXC_I2C_CLK:
>>                 return get_ipg_per_clk();
>>         case MXC_UART_CLK:
>>                 return get_uart_clk();
>>
>>
>> and updating the mxc_i2c driver to follow the same rule.
>
> That can be a good solution. What do you think about my imx_get_i2cclk()?
>
> Also, note that there are some broken clocks for i.MX25. I²C is one of them. It
> should be:
> 	case MXC_I2C_CLK:
> 		return imx_get_perclk(I2C_PER_CLK);

Why that? My understanding is that imx_get_perclk picks the right clock 
as long as the 16 first entries of enum mxc_clock ar in the rigth order.

Regards
Matthias