Re: [PATCH v2 3/3] arm64: dts: rockchip: add Anbernic RG353P and RG503

["Heiko Stübner" <heiko@sntech.de>]

Am Samstag, 20. August 2022, 10:40:34 CEST schrieb Maya Matuszczyk:
> sob., 20 sie 2022 o 00:26 Chris Morgan <macroalpha82@gmail.com> napisał(a):
> >
> > From: Chris Morgan <macromorgan@hotmail.com>

[...]

> > +&gpio_keys_control {
> > +       button-5 {
> > +               gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_LOW>;
> > +               label = "DPAD-LEFT";
> > +               linux,code = <BTN_DPAD_RIGHT>;
> > +       };
> > +
> > +       button-6 {
> > +               gpios = <&gpio3 RK_PA6 GPIO_ACTIVE_LOW>;
> > +               label = "DPAD-RIGHT";
> > +               linux,code = <BTN_DPAD_LEFT>;
> > +       };
> > +
> > +       button-9 {
> > +               gpios = <&gpio3 RK_PB3 GPIO_ACTIVE_LOW>;
> > +               label = "TR";
> > +               linux,code = <BTN_TR2>;
> > +       };
> > +
> > +       button-10 {
> > +               gpios = <&gpio3 RK_PB4 GPIO_ACTIVE_LOW>;
> > +               label = "TR2";
> > +               linux,code = <BTN_TR>;
> > +       };
> > +
> > +       button-14 {
> > +               gpios = <&gpio3 RK_PC1 GPIO_ACTIVE_LOW>;
> > +               label = "WEST";
> > +               linux,code = <BTN_WEST>;
> > +       };
> > +
> > +       button-15 {
> I don't think just having the buttons numbered sequentially
> is the best course of action, but this preserves the GPIO
> ordering while other options don't...
> I'm thinking about either having them named after
> their function, or named after what they're labeled
> on the PCB of the device.
> Can any of DT maintainers give their input on this?

Personally, I'd prefer going with what is on the PCB
or defined in the schematics.

This makes it way easier finding dt-elements either in
schematics or on the board itself.

This is true for all names ;-)

On the Odroid-Go for example buttons are really named
sw1, sw2, ... so the dt-name became button-sw1 etc.


[...]

> > +&pinctrl {
> > +       gpio-lcd {
> > +               lcd_rst: lcd-rst {
> > +                       rockchip,pins =
> > +                               <4 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
> > +               };
> Is it unused? If it is I think it would belong to patch that would add
> panel to this device

I tend to agree :-) .

> > +/ {
> > +       chosen: chosen {
> > +               stdout-path = "serial2:1500000n8";
> I'm wondering if this should be changed to 115200 baud rate
> so it would end up the same as on other devices,
> like Odroid Go Advance.

That heavily depends on the bootloader. I.e. speeds should be
consistent between them.

A lot of cheaper usb-ttl adapters tend to have difficulties with the
faster speeds, so 115200 is easier for those, but you need u-boot
to also use this speed.


On the Odroid-Go I did both the u-boot and kernel parts, so could
make sure those matched.


[...]

> > +       adc_keys: adc-keys {
> > +               compatible = "adc-keys";
> > +               io-channels = <&saradc 0>;
> > +               io-channel-names = "buttons";
> > +               keyup-threshold-microvolt = <1800000>;
> > +               poll-interval = <60>;
> > +
> > +               /*
> > +                * Button is mapped to F key in BSP kernel, but
> > +                * according to input guidelines it should be mode.
> > +                */
> > +               button-mode {
> > +                       label = "MODE";
> The physical button is labeled "F", so maybe this should be "F"
> too?

same comment about ideally using board/schematics names.
But then again, I won't make a fuss if it's named differently :-)


Heiko