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

From: Chris Morgan <macromorgan@hotmail.com>
To: Maya Matuszczyk <maccraft123mc@gmail.com>
Cc: "Heiko Stübner" <heiko@sntech.de>,
	"Chris Morgan" <macroalpha82@gmail.com>,
	devicetree <devicetree@vger.kernel.org>,
	"open list:ARM/Rockchip SoC..."
	<linux-rockchip@lists.infradead.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Peter Geis" <pgwipeout@gmail.com>,
	"Chris Healy" <cphealy@gmail.com>
Subject: Re: [PATCH v2 3/3] arm64: dts: rockchip: add Anbernic RG353P and RG503
Date: Tue, 23 Aug 2022 08:29:32 -0500	[thread overview]
Message-ID: <SN6PR06MB53427782CA5417F2FE3B9B32A5709@SN6PR06MB5342.namprd06.prod.outlook.com> (raw)
In-Reply-To: <CAO_Mup+D7B+GJcUpy+JpaNFsxC=nxnNTYCdzkyT7qSdRS-JcSA@mail.gmail.com>

On Tue, Aug 23, 2022 at 02:28:27PM +0200, Maya Matuszczyk wrote:
> wt., 23 sie 2022 o 14:16 Heiko Stübner <heiko@sntech.de> napisał(a):
> >
> > 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.
> I disassembled my device and DPAD buttons on pcb have
> silkscreened labels U/D/L/R, for up/down/left/right,
> Select and start buttons are named SELECT and START,
> action buttons are named A/B/X/Y for East, South,
> North and West buttons, The "F" button on front of the
> device has F label, but on PCB it's "RECOVERY".
> And TR/TL/TR2/TL2 are named R1/L1/R2/L2 on their
> test points, I wasn't able to find a silkscreen label as
> they are on their own PCBs.
> 
> Volume buttons are SW3 for Volume up and SW2 for
> Volume down.

Thanks Maya, this is for the 353P correct? I was looking at my spare
503 board to get the labels earlier. Of course, this brings up another
intersting point... the labels are different on each board (at least
for the R/L buttons, the volume buttons, and the recovery button which
is sw11 on the 503). Does that warrant separating them out despite the
GPIOs themselves being identical?

> 
> >
> >
> > [...]
> >
> > > > +&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.
> Yeah I've had troubles finding an adapter that could do the default
> 1500000 baud rate.
> 
> >
> >
> > On the Odroid-Go I did both the u-boot and kernel parts, so could
> > make sure those matched.
> I think we can just use 115200 baud rate, as Odroid Go Advance
> already uses it, and it's likely that it's the first thing people would
> try.
> 
> >
> >
> > [...]
> >
> > > > +       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 :-)
> So I guess it'd be "btn-recovery" as it's labeled "RECOVERY"
> on PCB, with "F" label as it's what's the user sees?
> 
> Best Regards,
> Maya Matuszczyk
> 
> >
> >
> > Heiko
> >
> >

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip