| Message ID | 20231122122232.952696-1-tim@feathertop.org |
|---|---|
| Headers | show |
| Series | Add support for Sonoff iHost RV1126 Smart Home Gateway | expand |
Hi Tim, 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@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? Heiko, any chance to take a look here? Thanks, Andi > + 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) { > -- > 2.40.1 >
Hi Krzysztof, Am Mittwoch, 22. November 2023, 13:29:47 CET schrieb Krzysztof Kozlowski: > On 22/11/2023 13:22, Tim Lunn wrote: > > Add i2c2 node and i2c2_xfer pinctrl for Rockchip RV1126 > > > > Signed-off-by: Tim Lunn <tim@feathertop.org> > > --- > > > > (no changes since v1) > > > > arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi | 10 ++++++++++ > > arch/arm/boot/dts/rockchip/rv1126.dtsi | 15 +++++++++++++++ > > 2 files changed, 25 insertions(+) > > > > diff --git a/arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi b/arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi > > index 4f85b7b3fc4c..167a48afa3a4 100644 > > --- a/arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi > > +++ b/arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi > > @@ -87,6 +87,16 @@ i2c0_xfer: i2c0-xfer { > > <0 RK_PB5 1 &pcfg_pull_none_drv_level_0_smt>; > > }; > > }; > > + i2c2 { > > + /omit-if-no-ref/ > > + i2c2_xfer: i2c2-xfer { > > + rockchip,pins = > > + /* i2c2_scl */ > > + <0 RK_PC2 1 &pcfg_pull_none_drv_level_0_smt>, > > + /* i2c2_sda */ > > + <0 RK_PC3 1 &pcfg_pull_none_drv_level_0_smt>; > > + }; > > + }; > > pwm2 { > > /omit-if-no-ref/ > > pwm2m0_pins: pwm2m0-pins { > > diff --git a/arch/arm/boot/dts/rockchip/rv1126.dtsi b/arch/arm/boot/dts/rockchip/rv1126.dtsi > > index 6c5c928f06c7..cf1df75df418 100644 > > --- a/arch/arm/boot/dts/rockchip/rv1126.dtsi > > +++ b/arch/arm/boot/dts/rockchip/rv1126.dtsi > > @@ -21,6 +21,7 @@ / { > > > > aliases { > > i2c0 = &i2c0; > > + i2c2 = &i2c2; > > No, this should be per-board to match board labeling/schematics. At least for i2c, uarts and i.e. spi ... Rockchip manuals, pin namings and also all board schematics I've seen so far are very consistent for these ... i2c2 for example is labled i2c2 both in the pins in the socs and also in the board-schematics using them. So while I can agree that things like mmc-aliases might be board-specific, I do think aliases for the core busses should be able to live in the soc dtsi as for all Rockchip SoCs so far? Heiko
Hi Heiko, Am 27.11.23 um 10:45 schrieb Heiko Stübner: > Am Montag, 27. November 2023, 10:27:41 CET schrieb Krzysztof Kozlowski: >> On 27/11/2023 10:23, Tim Lunn wrote: >>> Hi >>> >>> On 11/27/23 17:52, Krzysztof Kozlowski wrote: >>>> On 27/11/2023 00:06, Heiko Stübner wrote: >>>>> Hi Krzysztof, >>>>> >>>>> Am Mittwoch, 22. November 2023, 13:29:47 CET schrieb Krzysztof Kozlowski: >>>>>> On 22/11/2023 13:22, Tim Lunn wrote: >>>>>>> Add i2c2 node and i2c2_xfer pinctrl for Rockchip RV1126 >>>>>>> >>>>>>> Signed-off-by: Tim Lunn <tim@feathertop.org> >>>>>>> --- >>>>>>> >>>>>>> (no changes since v1) >>>>>>> >>>>>>> arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi | 10 ++++++++++ >>>>>>> arch/arm/boot/dts/rockchip/rv1126.dtsi | 15 +++++++++++++++ >>>>>>> 2 files changed, 25 insertions(+) >>>>>>> >>>>>>> diff --git a/arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi b/arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi >>>>>>> index 4f85b7b3fc4c..167a48afa3a4 100644 >>>>>>> --- a/arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi >>>>>>> +++ b/arch/arm/boot/dts/rockchip/rv1126-pinctrl.dtsi >>>>>>> @@ -87,6 +87,16 @@ i2c0_xfer: i2c0-xfer { >>>>>>> <0 RK_PB5 1 &pcfg_pull_none_drv_level_0_smt>; >>>>>>> }; >>>>>>> }; >>>>>>> + i2c2 { >>>>>>> + /omit-if-no-ref/ >>>>>>> + i2c2_xfer: i2c2-xfer { >>>>>>> + rockchip,pins = >>>>>>> + /* i2c2_scl */ >>>>>>> + <0 RK_PC2 1 &pcfg_pull_none_drv_level_0_smt>, >>>>>>> + /* i2c2_sda */ >>>>>>> + <0 RK_PC3 1 &pcfg_pull_none_drv_level_0_smt>; >>>>>>> + }; >>>>>>> + }; >>>>>>> pwm2 { >>>>>>> /omit-if-no-ref/ >>>>>>> pwm2m0_pins: pwm2m0-pins { >>>>>>> diff --git a/arch/arm/boot/dts/rockchip/rv1126.dtsi b/arch/arm/boot/dts/rockchip/rv1126.dtsi >>>>>>> index 6c5c928f06c7..cf1df75df418 100644 >>>>>>> --- a/arch/arm/boot/dts/rockchip/rv1126.dtsi >>>>>>> +++ b/arch/arm/boot/dts/rockchip/rv1126.dtsi >>>>>>> @@ -21,6 +21,7 @@ / { >>>>>>> >>>>>>> aliases { >>>>>>> i2c0 = &i2c0; >>>>>>> + i2c2 = &i2c2; >>>>>> No, this should be per-board to match board labeling/schematics. >>>>> At least for i2c, uarts and i.e. spi ... Rockchip manuals, pin namings >>>>> and also all board schematics I've seen so far are very consistent for >>>>> these ... i2c2 for example is labled i2c2 both in the pins in the socs >>>>> and also in the board-schematics using them. >>>>> >>>>> So while I can agree that things like mmc-aliases might be board-specific, >>>>> I do think aliases for the core busses should be able to live in the soc dtsi >>>>> as for all Rockchip SoCs so far? >>>> If you do not list here all aliases, it is already board-specific, isn't it? >>>> >>>> https://lore.kernel.org/linux-rockchip/CAK8P3a25iYksubCnQb1-e5yj=crEsK37RB9Hn4ZGZMwcVVrG7g@mail.gmail.com/ >>> I had only added aliases for nodes that are implemented. RV1126 has 6 >>> i2c busses but so far not all these exist in the soc.dtsi. >>> >> >> OK, that would explain why you have only two. Anyway, it is just generic >> guideline, so up to Heiko what to do with it. > > People see "uart2-tx" on their pin-header description and then of course > want to use ttyS2 ... same with reading i2c2-sda on the pin-header and then > running i2cdetect on said i2c2 dev node. > If that's "the" rule, why was rk3128.dtsi merged after similar complaints came up [0] without having aliases for the core buses defined? It's pretty strange to have to define them on board-level there as well. I'm happy to submit a patch which changes that. [0] https://lore.kernel.org/all/22076018.EfDdHjke4D@diego/ Regards, Alex > And even if i2c1 is not populated (would even be very rare), then people > would still expect i2c2 to be named that way. > > As this is the same for _every_ board, it doesn't really make sense to > duplicate it every time. Which I guess is similar to what I wrote in reply > to the mail you linked above :-) . > > > It's different for mmc, where this naming scheme isn't normally used, > so we (mostly?) migrated to them being in the board-specific alias section. > > Thanks > Heiko > > > > _______________________________________________ > Linux-rockchip mailing list > Linux-rockchip@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip