Re: [PATCH v4 4/4] arm64: dts: allwinner: h616: Add BigTreeTech Pi support

From: Jernej Škrabec
Date: Wed Sep 06 2023 - 11:40:12 EST


Dne sreda, 06. september 2023 ob 12:24:34 CEST je Andre Przywara napisal(a):
> On Tue, 05 Sep 2023 22:45:27 +0200
> Jernej Škrabec <jernej.skrabec@xxxxxxxxx> wrote:
>
> Hi,
>
> > On Thursday, August 24, 2023 10:48:20 PM CEST Martin Botka wrote:
> > > On Mon, Aug 14 2023 at 12:47:59 PM +02:00:00, Martin Botka
> > >
> > > <martin.botka@xxxxxxxxxxxxxx> wrote:
> > > > On Mon, Aug 14 2023 at 12:26:07 PM +02:00:00, Jernej Škrabec
> > > >
> > > > <jernej.skrabec@xxxxxxxxx> wrote:
> > > >> Dne nedelja, 13. avgust 2023 ob 18:22:49 CEST je Martin Botka
> > > >>
> > > >> napisal(a):
> > > >>> On Sun, Aug 13 2023 at 05:55:35 PM +02:00:00, Jernej Škrabec
> > > >>>
> > > >>> <jernej.skrabec@xxxxxxxxx> wrote:
> > > >>> > Dne ponedeljek, 07. avgust 2023 ob 16:53:24 CEST je Martin Botka
> > > >>> >
> > > >>> > napisal(a):
> > > >>> >> The BigTreeTech Pi is an H616 based board based on CB1.
> > > >>> >> Just in Rpi format board.
> > > >>> >>
> > > >>> >> It features the same internals as BTT CB1 but adds:
> > > >>> >> - Fan port
> > > >>> >> - IR receiver
> > > >>> >> - ADXL345 Accelerometer connector via SPI
> > > >>> >> - 24V DC power supply via terminal plugs
> > > >>> >> - USB to CAN module connector (The actual USB to CAN
> > > >>>
> > > >>> happens on
> > > >>>
> > > >>> >> the
> > > >>> >>
> > > >>> >> external module)
> > > >>> >>
> > > >>> >> List of currently working things is same as BTT CB1 but also:
> > > >>> >> - IR receiver
> > > >>> >> - ADXL345 connector
> > > >>> >>
> > > >>> >> Signed-off-by: Martin Botka <martin@xxxxxxxxxx>
> > > >>> >> Reviewed-by: Andre Przywara <andre.przywara@xxxxxxx>
> > > >>> >> ---
> > > >>> >>
> > > >>> >> Changes in V2:
> > > >>> >> - Add UART alongside aliases and chosen for it
> > > >>> >> - Add model string
> > > >>> >> - Enable IR receiver
> > > >>> >> - Enable SPI0 for ADXL345 connector
> > > >>> >>
> > > >>> >> Changes in V3:
> > > >>> >> - Add missing semicolons
> > > >>> >> - Add pinctrl for SPI0
> > > >>> >>
> > > >>> >> arch/arm64/boot/dts/allwinner/Makefile | 1 +
> > > >>> >> .../allwinner/sun50i-h616-bigtreetech-pi.dts | 70
> > > >>> >>
> > > >>> >> +++++++++++++++++++
> > > >>> >>
> > > >>> >> 2 files changed, 71 insertions(+)
> > > >>> >> create mode 100644
> > > >>> >>
> > > >>> >> arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-pi.dts
> > > >>> >>
> > > >>> >> diff --git a/arch/arm64/boot/dts/allwinner/Makefile
> > > >>> >> b/arch/arm64/boot/dts/allwinner/Makefile index
> > > >>> >>
> > > >>> >> 7b386428510b..0b6232a7f328
> > > >>> >>
> > > >>> >> 100644
> > > >>> >> --- a/arch/arm64/boot/dts/allwinner/Makefile
> > > >>> >> +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > > >>> >> @@ -39,5 +39,6 @@ dtb-$(CONFIG_ARCH_SUNXI) +=
> > > >>> >>
> > > >>> >> sun50i-h6-pine-h64-model-b.dtb
> > > >>> >>
> > > >>> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6.dtb
> > > >>> >>
> > > >>> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-tanix-tx6-mini.dtb
> > > >>> >> dtb-$(CONFIG_ARCH_SUNXI) +=
> > > >>>
> > > >>> sun50i-h616-bigtreetech-cb1-manta.dtb
> > > >>>
> > > >>> >> +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-bigtreetech-pi.dtb
> > > >>> >>
> > > >>> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-orangepi-zero2.dtb
> > > >>> >> dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h616-x96-mate.dtb
> > > >>> >>
> > > >>> >> diff --git
> > > >>> >>
> > > >>> >> a/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-pi.dts
> > > >>> >>
> > > >>> >> b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-pi.dts
> > > >>>
> > > >>> new
> > > >>>
> > > >>> >> file
> > > >>> >>
> > > >>> >> mode 100644
> > > >>> >> index 000000000000..b0d0386e8f13
> > > >>> >> --- /dev/null
> > > >>> >> +++
> > > >>>
> > > >>> b/arch/arm64/boot/dts/allwinner/sun50i-h616-bigtreetech-pi.dts
> > > >>>
> > > >>> >> @@ -0,0 +1,70 @@
> > > >>> >> +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > > >>> >> +/*
> > > >>> >> + * Copyright (C) 2023 Martin Botka <martin@xxxxxxxxxx>.
> > > >>> >> + */
> > > >>> >> +
> > > >>> >> +/dts-v1/;
> > > >>> >> +
> > > >>> >> +#include "sun50i-h616-bigtreetech-cb1.dtsi"
> > > >>> >> +
> > > >>> >> +/ {
> > > >>> >> + model = "BigTreeTech Pi";
> > > >>> >> + compatible = "bigtreetech,pi", "allwinner,sun50i-
h616";
> > > >>> >> +
> > > >>> >> + aliases {
> > > >>> >> + serial0 = &uart0;
> > > >>> >> + };
> > > >>> >> +
> > > >>> >> + chosen {
> > > >>> >> + stdout-path = "serial0:115200n8";
> > > >>> >> + };
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ehci0 {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ehci1 {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ehci2 {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ehci3 {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ir {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ohci0 {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ohci1 {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ohci2 {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&ohci3 {
> > > >>> >> + status = "okay";
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&spi0 {
> > > >>> >> + /* SPI connection for onboard connector for ADXL345
> > > >>>
> > > >>> accelerometer
> > > >>>
> > > >>> > */
> > > >>> >
> > > >>> >> + status = "okay";
> > > >>> >> + pinctrl-names = "default";
> > > >>> >> + pinctrl-0 = <&spi0_pins>, <&spi0_cs0_pin>;
> > > >>> >
> > > >>> > Driver and compatible for ADXL345 already exists, why don't you
> > > >>>
> > > >>> add
> > > >>>
> > > >>> > child node
> > > >>> > for it?
> > > >>> >
> > > >>> > Best regards,
> > > >>> > Jernej
> > > >>>
> > > >>> Ah. So the ADXL345 actually wont be driven by kernel.
> > > >>
> > > >> DT is hardware description, it's not concerned what is done on
> > > >> software side,
> > > >> either kernel or user space.
> > > >
> > > > Im aware. But this is not a device that is on the board. Its simply a
> > > > connector for the device.
> > > > Like Rpi has connectors for camera module :)
> >
> > Actually it matters only if this other board is firmly connected with base
> > board. If it is not, then SPI node should be dropped and be handled with
> > DT
> > overlays.
>
> So I do understand the reasoning behind not describing those generic pin
> header connectors, where people could hook up anything, and use it as a
> GPIO or as a special function device, but just wanted to check on this
> situation:
> Don't we even describe pins dedicated to those custom connectors,
> especially if there is an "obvious" external device to connect to? Is it
> the same situation like with these FPC camera connectors?

I would say yes. SPI connector can still be used as GPIOs or anything else,
since there is no device soldered directly to it.

Best regards,
Jernej

>
> Cheers,
> Andre
>
> > > >>> The SPI connection is enabled so that klipper (3d printer
> > > >>>
> > > >>> firmware) can
> > > >>>
> > > >>> be told to look for ADXL345 at this SPI and use it on its own.
> > > >>>
> > > >>> Klipper will initialize and communicate with the ADXL on its own.
> > > >>
> > > >> What do you mean by firmware? User space app? In this case I suppose
> > > >> you'll use
> > > >> direct SPI commands from user space? AFAIK that's less and less
> > > >> supported by
> > > >> kernel (in contrast to I2C).
> > > >
> > > > Firmware as in 3d printer firmware. Klipper runs on the board (CB1 or
> > > > BTT Pi) and is indeed an userspace app.
> > > > And indeed uses direct SPI commands to the device.
> > > >
> > > > The reason for this is the flexibility.
> > > > If Klipper read the values from kernel or well from the files the
> > > > ADXL driver would create
> > > > then it would be unable to communicate with ADXL that is on toolhead
> > > > board. Or would have to have
> > > > direct initialization either way for those. Thus it just controls the
> > > > ADXL itself :)
> > > >
> > > > I understand that this may be bit confusing. If there is still
> > > > something not clear im more then happy to explain in
> > > > full detail how the userspace and 3D printer communicate :)
> >
> > As I said, DT doesn't care about implementation. DT is HW description, so
> > either if it's fixed connection (soldered wires), then describe in full,
> > otherwise it's considered addon board and thus non-essential, so it should
> > be handled with DT overlays.
> >
> > Can be CB1 bought separately from 3D printer package and thus used without
> > ADXL sensor?
> >
> > > > Cheers,
> > > > Martin.
> > >
> > > Hello,
> > > Jernej any comments on this ?
> > > I would like to resolve this conversation :)
> > > We do have a bunch of time before the new cycle but never hurts to get
> > > it figured out ahead of time :)
> >
> > Sorry, not enough free time over summer.
> >
> > Best regards,
> > Jernej
> >
> > > Cheers,
> > > Martin
> > >
> > > >> Best regards,
> > > >> Jernej
> > > >>
> > > >>> >> +};
> > > >>> >> +
> > > >>> >> +&uart0 {
> > > >>> >> + pinctrl-names = "default";
> > > >>> >> + pinctrl-0 = <&uart0_ph_pins>;
> > > >>> >> + status = "okay";
> > > >>> >> +};