Messages in this thread | | | From | Uwe Kleine-König <> | Subject | Re: [PATCH v3 2/2] arm64: dts: rockchip: Add basic support for Kobol's Helios64 | Date | Tue, 13 Oct 2020 22:22:25 +0200 |
| |
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@kleine-koenig.org/.
> 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
[unhandled content-type:application/pgp-signature] | |