Bug 217444

Summary: i regret to inform you the rockchip dtsi is wrong
Product: Linux Reporter: antwain.schneider
Component: KernelAssignee: io_other
Status: NEW ---    
Severity: enhancement    
Priority: P3    
Hardware: ARM   
OS: Linux   
Kernel Version: all Subsystem: ARM/Rockchip SoC support
Regression: No Bisected commit-id:
Bug Depends on: 217334    
Bug Blocks:    

Description antwain.schneider 2023-05-15 07:44:36 UTC
after applying the patch in bug 217334, and after playing around with the affected gpios, i looked and saw something curious

% ma -k /dev/gpiomem dd 0x24 8
00000020  -------- 00000400  00000000

which in binary for the mux translates to

6  5  4  3  2  1  0
00 01 00 00 00 00 00

and then checking the various device tree files found the relevant value

                        otp_out: otp-out {
                                rockchip,pins = <2 RK_PB5 1 &pcfg_pull_none>;
                        };

which in turn is used in the tsadc entry

                pinctrl-1 = <&otp_out>;

but upon further investigations, i found out two interesting issues about this

1. the mode is wrong
2. mainline kernel doesn't even use the function

expanding further on point 1, while the tsadc documentation in the trm doesn't explicitly state what the mode for the pin should be, elsewhere in the document you can see what it should be (inside the i2c documentation (19.5 ss table 19-1))

i2c2_sda | I2C2sda _ TSADCshut _ GPIO2B5 _ vccio5 | GRF_GPIO2B_IOMUX[11:10]=2’b01

which makes clear that mode 1 is meant for i2c and mode 2 is meant for tsadc_shut

so it should be

                        otp_out: otp-out {
                                rockchip,pins = <2 RK_PB5 2 &pcfg_pull_none>;
                        };

but as for point 2, while various rockchip kernels do mainline kernel rockchip-thermal.c do not use the otp information, so maybe if in the future it will be used, it should be changed, or if not maybe it should just be removed from the dtsi
of course, you could be extra pedantic and complain that 2b5 doesn't even seem to have an output on the chip