Re: [EXT] Re: [PATCH 1/2] i2c: imx: I2C Driver doesn't consider I2C_IPGCLK_SEL RCW bit when using ls1046a SoC

[Sascha Hauer]

On Sat, May 04, 2019 at 09:28:48AM +0000, Chuanhua Han wrote:
> 
> 
> > -----Original Message-----
> > From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> > Sent: 2019å4æ30æ 20:51
> > To: Chuanhua Han <chuanhua.han@xxxxxxx>
> > Cc: shawnguo@xxxxxxxxxx; Leo Li <leoyang.li@xxxxxxx>; robh+dt@xxxxxxxxxx;
> > mark.rutland@xxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > linux-i2c@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; festevam@xxxxxxxxx; dl-linux-imx
> > <linux-imx@xxxxxxx>; wsa+renesas@xxxxxxxxxxxxxxxxxxxx;
> > u.kleine-koenig@xxxxxxxxxxxxxx; eha@xxxxxxxx; linux@xxxxxxxxxxxxxxxx;
> > l.stach@xxxxxxxxxxxxxx; peda@xxxxxxxxxx; Sumit Batra
> > <sumit.batra@xxxxxxx>
> > Subject: [EXT] Re: [PATCH 1/2] i2c: imx: I2C Driver doesn't consider
> > I2C_IPGCLK_SEL RCW bit when using ls1046a SoC
> > 
> > Caution: EXT Email
> > 
> > On Tue, Apr 30, 2019 at 12:47:18PM +0800, Chuanhua Han wrote:
> > > The current kernel driver does not consider I2C_IPGCLK_SEL (424 bit of
> > > RCW) in deciding  i2c_clk_rate in function i2c_imx_set_clk() { 0
> > > Platform clock/4, 1 Platform clock/2}.
> > >
> > > When using ls1046a SoC, this populates incorrect value in IBFD
> > > register if I2C_IPGCLK_SEL = 0, which generates half of the desired Clock.
> > >
> > > Therefore, if ls1046a SoC is used, we need to set the i2c clock
> > > according to the corresponding RCW.
> > 
> > So the clock driver reports the wrong clock. Please fix the clock driver then.
> No, this is a problem with the i2c driver. It is not a problem with
> the clock driver, so the i2c driver needs to be modified.

So how does this RCW bit get evaluated? According to the reference
manual only one clock goes to the i2c module (described as 1/2 Platform
Clock) and the i2c module only takes one clock. So it seems there must
be a /2 divider somewhere, either in each i2c module or somewhere
outside. Can your IC guys tell you where it is?

One reason I suggested the clock driver is that the clock driver
contains SoC specific code already, so it should be easier to integrate
there.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |