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@xxxxxxxxxxxxxx> > > > > --- > > > > > > > > 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@xxxxxxxxx> Thanks for your ack and answer. Will wait, then for Tim's v2. Andi