Re: [PATCH v2 4/9] ARM: dts: rockchip: rv1126: Add i2c2 nodes

[Alex Bee <knaerzche@gmail.com>]

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