Re: [PATCH v1 3/4] arm64: dts: Add aqp8039-t2 board

[Stephan Gerhold <stephan@gerhold.net>]

On Tue, Apr 19, 2022 at 02:09:02AM +0100, Bryan O'Donoghue wrote:
> The apq8039-t2 is an apq8039/msm8939 based board paired with a wcn3680b
> WiFi chipset.
>
> [...]
> ---
>  arch/arm64/boot/dts/qcom/Makefile             |   1 +
>  .../boot/dts/qcom/apq8039-t2-pinctl.dtsi      | 277 +++++++++++++++
>  arch/arm64/boot/dts/qcom/apq8039-t2.dts       | 326 ++++++++++++++++++
>  3 files changed, 604 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/apq8039-t2-pinctl.dtsi
>  create mode 100644 arch/arm64/boot/dts/qcom/apq8039-t2.dts
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index f9e6343acd03..5b8a0eb34733 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
> +dtb-$(CONFIG_ARCH_QCOM)	+= apq8039-t2.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= apq8094-sony-xperia-kitakami-karin_windy.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-db820c.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= apq8096-ifc6640.dtb
> diff --git a/arch/arm64/boot/dts/qcom/apq8039-t2-pinctl.dtsi b/arch/arm64/boot/dts/qcom/apq8039-t2-pinctl.dtsi
> new file mode 100644
> index 000000000000..60adb3da826a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/apq8039-t2-pinctl.dtsi

We stopped separating the pinctrl in an extra .dtsi. Please just put it
at the end of the apq8039-t2.dts or sorted with the other nodes.
(I put them at the end for all the msm8916 device trees, but on newer
 SoCs it's usually just put normally in alphebetical order.)

> [...]
> @@ -0,0 +1,277 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> +
> +/*
> + * Line names are taken from the schematic of T2, Ver X03.
> + * July 14, 2018. Page 4 in particular.
> + */
> +
> +&msmgpio {
> + [...]
> +	typec_pins: typec_pins {
> +		pinmux_irqz {
> +			function = "gpio";
> +			pins = "gpio107";
> +		};
> +		pinconf_irqz {
> +			pins = "gpio107";
> +			bias-pull-up;
> +			input-enable;
> +		};
> +	};

You can write this like:

typec_pins: typec_pins {
	pins = "gpio107";
	function = "gpio";

	bias-pull-up;
	input-enable;
};

The separation and separate "pinmux" etc nodes shouldn't be needed. Also
see commit 6554a295042f7 ("arm64: dts: qcom: msm8916: Simplify pinctrl
configuration").

> + [...]
> +};
> + [...]
> diff --git a/arch/arm64/boot/dts/qcom/apq8039-t2.dts b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
> new file mode 100644
> index 000000000000..558f37945d94
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/apq8039-t2.dts
> @@ -0,0 +1,326 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2022, Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.

SPDX should be enough. :)

> + */
> + [...]
> +};
> +&otg {
> +	status = "okay";
> +	usb-role-switch;
> +
> +	pinctrl-names = "default", "host", "device";
> +	pinctrl-0 = <&pinctrl_otg_default>;
> +	pinctrl-1 = <&pinctrl_otg_host>;
> +	pinctrl-2 = <&pinctrl_otg_device>;
> +	pin-switch-delay-us = <100000>;
> +
> +	ulpi {
> +		usb_hs_phy: phy {
> +			qcom,enable-vbus-pullup;

Hm, what handles the USB detection on this port? The tps6598x?
Can it provide an extcon so that you don't need to keep the USB PHY
powered permanently?

> + [...]
> +&pronto {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&wcnss_pin_a &wcnss_pin_antennae>;
> +	status = "okay";
> +
> +	iris {
> +		compatible = "qcom,wcn3680";
> +	};
> +
> +	smd-edge {
> +		wcnss {
> +			bt {
> +				local-bd-address = [ 55 44 33 22 11 00 ];

Is this really needed?

The DB410c LK and U-Boot port (and also lk2nd) automatically generate a
WiFi/BT address based on the eMMC serial number. That's probably a
better solution than a fixed MAC address specified here.

Thanks,
Stephan