Hello Johan, On 10/13/20 7:34 PM, Johan Jonker wrote: > Part 1 of 2 missing here. Please complain to gmail then, given that patch 1 can be found on https://lore.kernel.org/linux-arm-kernel/20201013161340.720403-2-uwe@xxxxxxxxxxxxxxxxx/. > Submit all patches to all maintainers and mail lists. > Don't forget robh+dt ! I'm really surprised you insist here. In my experience Rob (with his dt-hat on) is not interested in specifics of the machine files and he already acked patch 1. > Add a little change log here. I assume you also didn't get the cover letter? >> + fault-led { > fault_led: led-0 {} > > My fault. > Change ones more... > # The first form is preferred, but fall back to just 'led' anywhere in the > # node name to at least catch some child nodes. > "(^led-[0-9a-f]$|led)": ok, the label isn't necessary, is it? >> + label = "helios64:green:status"; >> + gpios = <&gpio0 RK_PB4 GPIO_ACTIVE_HIGH>; > >> + linux,default-trigger = "none"; > > Don't use 'none' for mainline. Oh, I missed that. Thanks for your persistence. >> + default-state = "on"; >> + }; >> + }; >> + >> + vcc1v8_sys_s0: vcc1v8-sys-s0 { >> + compatible = "regulator-fixed"; >> + regulator-name = "vcc1v8_sys_s0"; >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <1800000>; >> + vin-supply = <&vcc1v8_sys_s3>; >> + }; >> + >> + vcc3v0_sd: vcc3v0-sd { >> + compatible = "regulator-fixed"; >> + regulator-name = "vcc3v0_sd"; > > Doesn't a sd card need a on/off gpio? > Could you check the schematics? Hmm, there is a GPIO. I didn't consider a problem there given that the machine logs [ 31.708706] vcc3v0_sd: disabling when there is no SD-card in the slot. Will investigate further. >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pmic_int_l>; > > sort I would expect this is an exception from the sorting rule. $ git grep pinctrl-names linus/master:arch/arm64/boot/dts/ | wc -l 1905 $ git grep -A1 pinctrl-names linus/master:arch/arm64/boot/dts/ | \ grep pinctrl-0 | wc -l 1412 When grepping over arch/arm64/boot/dts/rockchip the numbers are 453 and 445 respectively. Will use pinctrl-names above pinctrl-0 consistently. >> + regulator-max-microvolt = <1350000>; >> + regulator-min-microvolt = <750000>; > > > The rest has min above max. > Exception to the sort rule, not sure what Heiko wants, but keep it every > where the same. OK, most rockchip dts have min before max, will fix up. >> + i2c-scl-rising-time-ns = <160>; >> + i2c-scl-falling-time-ns = <30>; > > sort I consider it logical to have rise before fall. In 43 of 59 cases in arch/arm64/boot/dts/rockchip/ it is done this way. >> + vqmmc-supply = <&vcc1v8_sys_s0>; >> + status = "okay"; >> +}; >> + >> +&sdmmc { >> + bus-width = <4>; >> + cap-sd-highspeed; > >> + cd-gpios = <&gpio0 RK_PA7 GPIO_ACTIVE_LOW>; > > see regulator? GPIO0_A7 is the card detect line. I don't understand your question. Is it the same as above, i.e. that it should be possible that the SD regulator can be disabled? >> + disable-wp; >> + pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_cd &sdmmc_bus4>; >> + pinctrl-names = "default"; >> + vmmc-supply = <&vcc3v0_sd>; >> + vqmmc-supply = <&vcc_sdio_s0>; >> + status = "okay"; >> +}; Best regards Uwe
Attachment:
signature.asc
Description: OpenPGP digital signature
