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

[Jernej Škrabec]

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.

> > 
> >>>  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";
> >>>  >>  +};