Re: [PATCH 12/12] i2c: pxa: enable/disable i2c module across msg xfer

[Yi Zhang <yizhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>]

On Wed, Jun 03, 2015 at 12:59:23AM +0800, Vaibhav Hiremath wrote:
> 
> 
> On Tuesday 02 June 2015 10:22 PM, Vaibhav Hiremath wrote:
> >
> >
> > On Thursday 28 May 2015 06:53 PM, Russell King - ARM Linux wrote:
> >> On Thu, May 28, 2015 at 06:33:44PM +0530, Vaibhav Hiremath wrote:
> >>> From: Yi Zhang <yizhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> >>>
> >>> Enable i2c module/unit before transmission and disable when it finishes.
> >>>
> >>> why?
> >>> It's because the i2c bus may be distrubed if the slave device,
> >>> typically a touch, powers on.
> >>
> >> "disturbed"
> >>
> >> I'd recommend that this is a DT property - not every platform is going to
> >> want this, and as there is rudimentary I2C slave support in this driver,
> >> this change breaks that.
> >>
> >
> > I would take it as two different comments here, and I believe you also
> > meant same,
> >
> > 1. Not breaking I2C slave support
> >
> > Not sure whether enabling/disabling module in ISR would suffice here.
> > To be specific, in the functions i2c_pxa_slave_start() &
> > i2c_pxa_slave_stop()
> >
> >
> 
> Please ignore this option, as enable is important to generate interrupt
> on slave start.

  Hi, Vaibhav:

  sorry there is something wrong with my mailbox, so I list the LCR/WCR
  information here, hoping not to confuse you;

  LCR: it's used to minor adjustments to the SCL, increasing it
  decreases the SCL frequency, vice versa;
  yes, it has relation with the I2C input clock:
  if input_clk = 32MHz, LCR = 0x0804_1c96
     LCR[SLV] = 0x096, bit 8~0
     LCR[FLV] = 0xe,   bit 17~9
     LCR[HLVH] = 0x1,  bit 26~18
     LCR[HLVL] = 0x1,  bit 31~27
  if input_clk = 52MHz,   LCR = 0x081c_60f9
  if input_clk = 62.4MHz, LCR = 0x082c_8330

  WCR: it's used to control setup and hold timing during slave mode
  WCR[31: 15], reserved, always write 0;
  WCR[14: 10], default 0x5, and should not be changed
  WCR[9: 5], default 0x1, and should not be changed
  WCR[4:0], default 0x1a, if input_clk = 33MHz, it's 0x0a;
                          if input_clk = 66MHz, it's 0x14;

  hope it's helpful;

  ---
  Yi Zhang <yizhang-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>

> 
> I will add DT property with clear note on SLAVE support issue, so that
> it can be disabled and SLAVE support should work as is.
> 
> Thanks,
> Vaibhav