[U-Boot] I2C on iMX25

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

> > I can now add MXC_IPG_PERCLK to arch-mx25/clock.h and adopt
> > generic.c accordingly but I don't think that this is the right way
> > to go
> > as the i2c clock can be different from perclk. Doing this
> > #define MXC_IPG_PERCLK MXC_I2C_CLK
> > in my config file is even more ugly.
> 
> This is wrong.

I agree. MXC_IPG_PERCLK means something else, and the I²C clock is not that
clock on i.MX25.

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

Best regards,
Benoît