[PATCH v2 3/9] i2c: rk3x: Adjust offset for i2c2 on rv1126

[Andi Shyti]

Hi Heiko and Tim,

On Mon, Nov 27, 2023 at 09:11:57PM +1100, Tim Lunn wrote:
> On 11/27/23 11:26, Heiko Stübner wrote:
> > Am Sonntag, 26. November 2023, 20:43:11 CET schrieb Andi Shyti:
> > > On Wed, Nov 22, 2023 at 11:22:26PM +1100, Tim Lunn wrote:
> > > > Rockchip RV1126 has special case mask bits for i2c2.
> > > > 
> > > > i2c2 wasnt previously enabled in rv1126.dtsi, adding DT node alone
> > > > is not sufficient to enable i2c2. This patch fixes the i2c2 bus.
> > > If I don't have sufficient information about the hardware this
> > > description is completely meaningless to me.
> > > 
> > > > Signed-off-by: Tim Lunn <tim at feathertop.org>
> > > > ---
> > > > 
> > > > Changes in v2:
> > > > - i2c: clarify commit message
> > > > 
> > > >   drivers/i2c/busses/i2c-rk3x.c | 7 +++++--
> > > >   1 file changed, 5 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
> > > > index a044ca0c35a1..151927466d1d 100644
> > > > --- a/drivers/i2c/busses/i2c-rk3x.c
> > > > +++ b/drivers/i2c/busses/i2c-rk3x.c
> > > > @@ -1288,8 +1288,11 @@ static int rk3x_i2c_probe(struct platform_device *pdev)
> > > >   			return -EINVAL;
> > > >   		}
> > > > -		/* 27+i: write mask, 11+i: value */
> > > > -		value = BIT(27 + bus_nr) | BIT(11 + bus_nr);
> > > > +		if (i2c->soc_data == &rv1126_soc_data && bus_nr == 2)
> > > > +			value = BIT(20) | BIT(4);
> > > Any chance to put a comment here as it is in the other
> > > assignment?
> > > 
> > > Are the two assignment mutually exclusive?
> Yes they are mutually exclusive, and its only i2c2 that is non-sequential
> (as per Heikos description below).
> > > 
> > > Heiko, any chance to take a look here?
> > So the background is, that on some SoCs Rockchip implemented to
> > different variants for the i2c controller. One new-style controller
> > that they started using in rk3066 and are using even today.
> > 
> > For these old socs they kept the "old" controller block as a sort
> > of fallback if the new thing didn't work out, and the bit above is
> > switching between the
> > 
> > Hence that is limited to rk3066, rk3188 and seemingly the rv1126.
> > And while the bits controlling the i2c controllers on the original socs
> > are order sequentially in the grf register, the rv1126 seems to have
> > those bits in non-consequtive places.
> > 
> > 
> > So TL;DR the change itself is likely good, and hopefully there won't
> > be any more of those, as all the new socs don't need this anymore.
> rv1108 is also similar but different bits again (only going off the BSP
> sources).
> I dont have hardware or the TRM to validate this on rv1108.
> > 
> > I do agree with the request for a comment describing the issue
> > in the code, but otherwise
> 
> I will fix this.
> 
> > Acked-by: Heiko Stuebner <heiko at sntech.de>

Thanks for your ack and answer. Will wait, then for Tim's v2.

Andi