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

[Chris Morgan]

On Tue, Aug 23, 2022 at 02:16:03PM +0200, Heiko Stübner wrote:
> Am Samstag, 20. August 2022, 10:40:34 CEST schrieb Maya Matuszczyk:
> > sob., 20 sie 2022 o 00:26 Chris Morgan <macroalpha82 at gmail.com> napisał(a):
> > >
> > > From: Chris Morgan <macromorgan at 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.

There are no schematics, so I'll see if I can locate names on the boards.
If I can, I'll rename these, and if not leave them in numerical order.
Will that work?

> 
> 
> [...]
> 
> > > +&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 :-) .

Okay, will remove.

> 
> > > +/ {
> > > +       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.
> 

I don't have A-TF sources which have the output set at 1500000. I can
recompile U-Boot, but the stock bootloader also has it at 1500000. I'd
love to be at a sane 115200, but until we have A-TF sources that might
not be possible, right?

> 
> 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 :-)
> 

Ditto, I'll see if I can find names on the boards and rename
accordingly. If not, will this work? Thank you.

> 
> Heiko
> 
>