Message ID | 20231203124004.2676174-4-tim@feathertop.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add support for Sonoff iHost RV1126 Smart Home Gateway | expand |
Hi Tim, On Sun, Dec 03, 2023 at 11:39:59PM +1100, Tim Lunn wrote: > Rockchip RV1126 is using old style i2c controller, the i2c2 > bus uses a non-sequential offset in the grf register for the > mask/value bits for this bus. > > This patch fixes i2c2 bus on rv1126 SoCs. > > Signed-off-by: Tim Lunn <tim@feathertop.org> > Acked-by: Heiko Stuebner <heiko@sntech.de> Reviewed-by: Andi Shyti <andi.shyti@kernel.org> Thanks, Andi
Hi Andi, Am Sonntag, 3. Dezember 2023, 20:06:20 CET schrieb Andi Shyti: > Hi Tim, > > On Sun, Dec 03, 2023 at 11:39:59PM +1100, Tim Lunn wrote: > > Rockchip RV1126 is using old style i2c controller, the i2c2 > > bus uses a non-sequential offset in the grf register for the > > mask/value bits for this bus. > > > > This patch fixes i2c2 bus on rv1126 SoCs. > > > > Signed-off-by: Tim Lunn <tim@feathertop.org> > > Acked-by: Heiko Stuebner <heiko@sntech.de> > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> you're listed as the maintainer for the i2c busses, are you going to pick this one patch yourself? Or do you expect a different handling? I.e. it doesn't really tie into the dts patches and everything will come together nicely in linux-next and during the next merge-window. Thanks Heiko
Hi, On Sun, Dec 03, 2023 at 08:48:43PM +0100, Heiko Stübner wrote: > Hi Andi, > > Am Sonntag, 3. Dezember 2023, 20:06:20 CET schrieb Andi Shyti: > > Hi Tim, > > > > On Sun, Dec 03, 2023 at 11:39:59PM +1100, Tim Lunn wrote: > > > Rockchip RV1126 is using old style i2c controller, the i2c2 > > > bus uses a non-sequential offset in the grf register for the > > > mask/value bits for this bus. > > > > > > This patch fixes i2c2 bus on rv1126 SoCs. > > > > > > Signed-off-by: Tim Lunn <tim@feathertop.org> > > > Acked-by: Heiko Stuebner <heiko@sntech.de> > > > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > > you're listed as the maintainer for the i2c busses, > are you going to pick this one patch yourself? > Or do you expect a different handling? > > I.e. it doesn't really tie into the dts patches and everything will come > together nicely in linux-next and during the next merge-window. Wolfram are you going to take this? Andi
Am Sonntag, 3. Dezember 2023, 22:55:30 CET schrieb Andi Shyti: > Hi, > > On Sun, Dec 03, 2023 at 08:48:43PM +0100, Heiko Stübner wrote: > > Hi Andi, > > > > Am Sonntag, 3. Dezember 2023, 20:06:20 CET schrieb Andi Shyti: > > > Hi Tim, > > > > > > On Sun, Dec 03, 2023 at 11:39:59PM +1100, Tim Lunn wrote: > > > > Rockchip RV1126 is using old style i2c controller, the i2c2 > > > > bus uses a non-sequential offset in the grf register for the > > > > mask/value bits for this bus. > > > > > > > > This patch fixes i2c2 bus on rv1126 SoCs. > > > > > > > > Signed-off-by: Tim Lunn <tim@feathertop.org> > > > > Acked-by: Heiko Stuebner <heiko@sntech.de> > > > > > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > > > > you're listed as the maintainer for the i2c busses, > > are you going to pick this one patch yourself? > > Or do you expect a different handling? > > > > I.e. it doesn't really tie into the dts patches and everything will come > > together nicely in linux-next and during the next merge-window. > > Wolfram are you going to take this? I was going by get_maintainer.pl showing Andi Shyti <andi.shyti@kernel.org> (maintainer:I2C SUBSYSTEM HOST DRIVERS) hence the question :-)
Hi Heiko, On Mon, Dec 04, 2023 at 12:36:43PM +0100, Heiko Stübner wrote: > Am Sonntag, 3. Dezember 2023, 22:55:30 CET schrieb Andi Shyti: > > On Sun, Dec 03, 2023 at 08:48:43PM +0100, Heiko Stübner wrote: > > > Am Sonntag, 3. Dezember 2023, 20:06:20 CET schrieb Andi Shyti: > > > > On Sun, Dec 03, 2023 at 11:39:59PM +1100, Tim Lunn wrote: > > > > > Rockchip RV1126 is using old style i2c controller, the i2c2 > > > > > bus uses a non-sequential offset in the grf register for the > > > > > mask/value bits for this bus. > > > > > > > > > > This patch fixes i2c2 bus on rv1126 SoCs. > > > > > > > > > > Signed-off-by: Tim Lunn <tim@feathertop.org> > > > > > Acked-by: Heiko Stuebner <heiko@sntech.de> > > > > > > > > Reviewed-by: Andi Shyti <andi.shyti@kernel.org> > > > > > > you're listed as the maintainer for the i2c busses, > > > are you going to pick this one patch yourself? > > > Or do you expect a different handling? > > > > > > I.e. it doesn't really tie into the dts patches and everything will come > > > together nicely in linux-next and during the next merge-window. > > > > Wolfram are you going to take this? > > I was going by get_maintainer.pl showing > Andi Shyti <andi.shyti@kernel.org> (maintainer:I2C SUBSYSTEM HOST DRIVERS) > > hence the question :-) Ack, patch 3 we will take it through i2c. Thanks, Andi
On Sun, Dec 03, 2023 at 11:39:59PM +1100, Tim Lunn wrote: > Rockchip RV1126 is using old style i2c controller, the i2c2 > bus uses a non-sequential offset in the grf register for the > mask/value bits for this bus. > > This patch fixes i2c2 bus on rv1126 SoCs. > > Signed-off-by: Tim Lunn <tim@feathertop.org> > Acked-by: Heiko Stuebner <heiko@sntech.de> > Applied to for-next, thanks! But, phew, the fact that this driver _needs_ i2c-aliases in DT should be fixed somewhen. I totally overlooked this so far :/
Hi Wolfram, On 12/20/23 04:06, Wolfram Sang wrote: > On Sun, Dec 03, 2023 at 11:39:59PM +1100, Tim Lunn wrote: >> Rockchip RV1126 is using old style i2c controller, the i2c2 >> bus uses a non-sequential offset in the grf register for the >> mask/value bits for this bus. >> >> This patch fixes i2c2 bus on rv1126 SoCs. >> >> Signed-off-by: Tim Lunn <tim@feathertop.org> >> Acked-by: Heiko Stuebner <heiko@sntech.de> >> > Applied to for-next, thanks! Thanks. > But, phew, the fact that this driver _needs_ i2c-aliases in DT should be > fixed somewhen. I totally overlooked this so far :/ > Not sure what you mean here? I did add an alias for i2c2 in the DT in another patch. Regards Tim >
Hi Wolfram, Am Dienstag, 19. Dezember 2023, 18:06:26 CET schrieb Wolfram Sang: > On Sun, Dec 03, 2023 at 11:39:59PM +1100, Tim Lunn wrote: > > Rockchip RV1126 is using old style i2c controller, the i2c2 > > bus uses a non-sequential offset in the grf register for the > > mask/value bits for this bus. > > > > This patch fixes i2c2 bus on rv1126 SoCs. > > > > Signed-off-by: Tim Lunn <tim@feathertop.org> > > Acked-by: Heiko Stuebner <heiko@sntech.de> > > > > Applied to for-next, thanks! > > But, phew, the fact that this driver _needs_ i2c-aliases in DT should be > fixed somewhen. I totally overlooked this so far :/ Yeah, relying on aliases for this is probably not the most elegant thing to do ;-) Though right now I don't see the perfect way to change that. Options I can think of, could be: (1) As this really is "just" a thing for older socs that offer both a very legacy i2c and the more modern one we use, I guess one possibility would be to move this completely out of the i2c driver and into the "grf-cleanup" driver [0]. We never actually implemented the "old"-style i2c driver for rk29xx and earlier - and that thing is more than 10 years old now, so noone ever will. So we always want to switch to the new one anyway in the kernel. (2) The other option would be to try to identify the busses by their address. We do this in some places, like the dsi driver [1] with the entry matched against the reg property. I guess from a "being done with it" perspective, the first option would be nicer ;-) . Thoughts? Heiko [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/rockchip/grf.c That code does a number of settings the in the "General Register Files" that we simply expect, so also doing the i2c controller switch there for all i2c busses in one go would make sense. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1586
diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c index a044ca0c35a1..06fec2344575 100644 --- a/drivers/i2c/busses/i2c-rk3x.c +++ b/drivers/i2c/busses/i2c-rk3x.c @@ -1288,8 +1288,12 @@ 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); + /* rv1126 i2c2 uses non-sequential write mask 20, value 4 */ + if (i2c->soc_data == &rv1126_soc_data && bus_nr == 2) + value = BIT(20) | BIT(4); + else + /* 27+i: write mask, 11+i: value */ + value = BIT(27 + bus_nr) | BIT(11 + bus_nr); ret = regmap_write(grf, i2c->soc_data->grf_offset, value); if (ret != 0) {