Re: [PATCH v4 3/3] arm64: dts: qcom: apq8016: Add Schneider HMIBSC board DTS

From: Caleb Connolly
Date: Wed Mar 27 2024 - 19:34:34 EST



On 27/03/2024 19:10, Stephan Gerhold wrote:
> On Wed, Mar 27, 2024 at 12:07:34PM +0530, Sumit Garg wrote:
>> Add Schneider Electric HMIBSC board DTS. The HMIBSC board is an IIoT Edge
>> Box Core board based on the Qualcomm APQ8016E SoC.
>>
>> Support for Schneider Electric HMIBSC. Features:
>> - Qualcomm Snapdragon 410C SoC - APQ8016 (4xCortex A53, Adreno 306)
>> - 1GiB RAM
>> - 8GiB eMMC, SD slot
>> - WiFi and Bluetooth
>> - 2x Host, 1x Device USB port
>> - HDMI
>> - Discrete TPM2 chip over SPI
>> - USB ethernet adaptors (soldered)
>>
>> Co-developed-by: Jagdish Gediya <jagdish.gediya@xxxxxxxxxx>
>> Signed-off-by: Jagdish Gediya <jagdish.gediya@xxxxxxxxxx>
>> Reviewed-by: Caleb Connolly <caleb.connolly@xxxxxxxxxx>
>> Signed-off-by: Sumit Garg <sumit.garg@xxxxxxxxxx>
>
> This looks good to me now. Thanks for making all the changes!
>
> Reviewed-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
>
> Please drop the tag again in case you send another version with any
> larger changes. I'm happy to look at the patch again then. Not sure if
> Caleb's R-b is still valid now after all the changes you made.

I appreciate the review and the re-submission. I would have rather given
it again explicitly -- but no doubt I'll make the same mistake 1000 times :P

Cheers, really happy that we can have Qualcomm boards land their DT
upstream and gain first-class U-Boot support as a result :D thanks for
all your work on this Sumit.
>
> I found one more nitpick below, but it also exists in apq8016-sbc.dts.
> Feel free to leave it as-is, I can fix it up for both sometime later.
>
> Thanks,
> Stephan
>
>> ---
>> arch/arm64/boot/dts/qcom/Makefile | 1 +
>> .../dts/qcom/apq8016-schneider-hmibsc.dts | 490 ++++++++++++++++++
>> 2 files changed, 491 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dts
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index 39889d5f8e12..ad55e52e950b 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -5,6 +5,7 @@ apq8016-sbc-usb-host-dtbs := apq8016-sbc.dtb apq8016-sbc-usb-host.dtbo
>>
>> dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc-usb-host.dtb
>> dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc-d3-camera-mezzanine.dtb
>> +dtb-$(CONFIG_ARCH_QCOM) += apq8016-schneider-hmibsc.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
>> diff --git a/arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dts b/arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dts
>> new file mode 100644
>> index 000000000000..62ce862ff5c0
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/apq8016-schneider-hmibsc.dts
>> @@ -0,0 +1,490 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024, Linaro Ltd.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "msm8916-pm8916.dtsi"
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/leds/common.h>
>> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>> +#include <dt-bindings/pinctrl/qcom,pmic-mpp.h>
>> +#include <dt-bindings/sound/apq8016-lpass.h>
>> +
>> +/ {
>> + model = "Schneider Electric HMIBSC Board";
>> + compatible = "schneider,apq8016-hmibsc", "qcom,apq8016";
>> +
>> + aliases {
>> + i2c1 = &blsp_i2c6;
>> + i2c3 = &blsp_i2c4;
>> + i2c4 = &blsp_i2c3;
>> + mmc0 = &sdhc_1; /* eMMC */
>> + mmc1 = &sdhc_2; /* SD card */
>> + serial0 = &blsp_uart1;
>> + serial1 = &blsp_uart2;
>> + spi0 = &blsp_spi5;
>> + usid0 = &pm8916_0;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial0";
>> + };
>> +
>> + hdmi-out {
>> + compatible = "hdmi-connector";
>> + type = "a";
>> +
>> + port {
>> + hdmi_con: endpoint {
>> + remote-endpoint = <&adv7533_out>;
>> + };
>> + };
>> + };
>> +
>> + gpio-keys {
>> + compatible = "gpio-keys";
>> + autorepeat;
>> + pinctrl-0 = <&msm_key_volp_n_default>;
>> + pinctrl-names = "default";
>> +
>> + button {
>> + label = "Volume Up";
>> + linux,code = <KEY_VOLUMEUP>;
>> + gpios = <&tlmm 107 GPIO_ACTIVE_LOW>;
>> + };
>> + };
>> +
>> + leds {
>> + compatible = "gpio-leds";
>> + pinctrl-0 = <&pm8916_mpps_leds>;
>> + pinctrl-names = "default";
>> +
>> + led-1 {
>> + function = LED_FUNCTION_WLAN;
>> + color = <LED_COLOR_ID_YELLOW>;
>> + gpios = <&pm8916_mpps 2 GPIO_ACTIVE_HIGH>;
>> + linux,default-trigger = "phy0tx";
>> + default-state = "off";
>> + };
>> +
>> + led-2 {
>> + function = LED_FUNCTION_BLUETOOTH;
>> + color = <LED_COLOR_ID_BLUE>;
>> + gpios = <&pm8916_mpps 3 GPIO_ACTIVE_HIGH>;
>> + linux,default-trigger = "bluetooth-power";
>> + default-state = "off";
>> + };
>> + };
>> +
>> + memory@80000000 {
>> + reg = <0 0x80000000 0 0x40000000>;
>> + };
>> +
>> + reserved-memory {
>> + ramoops@bff00000 {
>> + compatible = "ramoops";
>> + reg = <0x0 0xbff00000 0x0 0x100000>;
>> + record-size = <0x20000>;
>> + console-size = <0x20000>;
>> + ftrace-size = <0x20000>;
>> + ecc-size = <16>;
>> + };
>> + };
>> +
>> + usb-hub {
>> + compatible = "smsc,usb3503";
>> + reset-gpios = <&pm8916_gpios 1 GPIO_ACTIVE_LOW>;
>> + initial-mode = <1>;
>> + };
>> +
>> + usb_id: usb-id {
>> + compatible = "linux,extcon-usb-gpio";
>> + id-gpios = <&tlmm 110 GPIO_ACTIVE_HIGH>;
>> + pinctrl-0 = <&usb_id_default>;
>> + pinctrl-names = "default";
>> + };
>> +};
>> +
>> +&blsp_i2c3 {
>> + status = "okay";
>> +
>> + eeprom@50 {
>> + compatible = "atmel,24c32";
>> + reg = <0x50>;
>> + };
>> +};
>> +
>> +&blsp_i2c4 {
>> + status = "okay";
>> +
>> + adv_bridge: bridge@39 {
>> + compatible = "adi,adv7533";
>> + reg = <0x39>;
>> + interrupts-extended = <&tlmm 31 IRQ_TYPE_EDGE_FALLING>;
>> +
>> + adi,dsi-lanes = <4>;
>> + clocks = <&rpmcc RPM_SMD_BB_CLK2>;
>> + clock-names = "cec";
>> + pd-gpios = <&tlmm 32 GPIO_ACTIVE_HIGH>;
>> +
>> + avdd-supply = <&pm8916_l6>;
>> + a2vdd-supply = <&pm8916_l6>;
>> + dvdd-supply = <&pm8916_l6>;
>> + pvdd-supply = <&pm8916_l6>;
>> + v1p2-supply = <&pm8916_l6>;
>> + v3p3-supply = <&pm8916_l17>;
>> +
>> + pinctrl-0 = <&adv7533_int_active &adv7533_switch_active>;
>> + pinctrl-1 = <&adv7533_int_suspend &adv7533_switch_suspend>;
>> + pinctrl-names = "default","sleep";
>> + #sound-dai-cells = <0>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@0 {
>> + reg = <0>;
>> + adv7533_in: endpoint {
>> + remote-endpoint = <&mdss_dsi0_out>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> + adv7533_out: endpoint {
>> + remote-endpoint = <&hdmi_con>;
>> + };
>> + };
>> + };
>> + };
>> +};
>> +
>> +&blsp_i2c6 {
>> + status = "okay";
>> +
>> + rtc@30 {
>> + compatible = "sii,s35390a";
>> + reg = <0x30>;
>> + };
>> +
>> + eeprom@50 {
>> + compatible = "atmel,24c256";
>> + reg = <0x50>;
>> + };
>> +};
>> +
>> +&blsp_spi5 {
>> + cs-gpios = <&tlmm 18 GPIO_ACTIVE_LOW>;
>> + status = "okay";
>> +
>> + tpm@0 {
>> + compatible = "infineon,slb9670", "tcg,tpm_tis-spi";
>> + reg = <0>;
>> + spi-max-frequency = <500000>;
>> + };
>> +};
>> +
>> +&blsp_uart1 {
>> + label = "UART0";
>> + status = "okay";
>> +};
>> +
>> +&blsp_uart2 {
>> + label = "UART1";
>> + status = "okay";
>> +};
>> +
>> +&lpass {
>> + status = "okay";
>> +};
>> +
>> +&mdss {
>> + status = "okay";
>> +};
>> +
>> +&mdss_dsi0_out {
>> + data-lanes = <0 1 2 3>;
>> + remote-endpoint = <&adv7533_in>;
>> +};
>> +
>> +&pm8916_codec {
>> + qcom,mbhc-vthreshold-low = <75 150 237 450 500>;
>> + qcom,mbhc-vthreshold-high = <75 150 237 450 500>;
>> + status = "okay";
>> +};
>> +
>> +&pm8916_gpios {
>> + gpio-line-names =
>> + "USB_HUB_RESET_N_PM",
>> + "USB_SW_SEL_PM",
>> + "NC",
>> + "NC";
>> +
>> + usb_hub_reset_pm: usb-hub-reset-pm-state {
>> + pins = "gpio1";
>> + function = PMIC_GPIO_FUNC_NORMAL;
>> + input-disable;
>> + output-high;
>> + };
>> +
>> + usb_hub_reset_pm_device: usb-hub-reset-pm-device-state {
>> + pins = "gpio1";
>> + function = PMIC_GPIO_FUNC_NORMAL;
>> + output-low;
>> + };
>> +
>
> Nitpick: &usb_hub_reset_pm has input-disable (and the two others below),
> but this one doesn't.
>
>> + usb_sw_sel_pm: usb-sw-sel-pm-state {
>> + pins = "gpio2";
>> + function = PMIC_GPIO_FUNC_NORMAL;
>> + power-source = <PM8916_GPIO_VPH>;
>> + input-disable;
>> + output-high;
>> + };
>> +
>> + usb_sw_sel_pm_device: usb-sw-sel-pm-device-state {
>> + pins = "gpio2";
>> + function = PMIC_GPIO_FUNC_NORMAL;
>> + power-source = <PM8916_GPIO_VPH>;
>> + input-disable;
>> + output-low;
>> + };
>> +};
>> +
>> +&pm8916_mpps {
>> + gpio-line-names =
>> + "NC",
>> + "WLAN_LED_CTRL",
>> + "BT_LED_CTRL",
>> + "NC";
>> +
>> + pm8916_mpps_leds: pm8916-mpps-state {
>> + pins = "mpp2", "mpp3";
>> + function = "digital";
>> + output-low;
>> + };
>> +};
>> +
>> +&pm8916_resin {
>> + linux,code = <KEY_POWER>;
>> + status = "okay";
>> +};
>> +
>> +&pm8916_rpm_regulators {
>> + pm8916_l17: l17 {
>> + regulator-min-microvolt = <3300000>;
>> + regulator-max-microvolt = <3300000>;
>> + };
>> +};
>> +
>> +&sdhc_1 {
>> + status = "okay";
>> +};
>> +
>> +&sdhc_2 {
>> + pinctrl-0 = <&sdc2_default &sdc2_cd_default>;
>> + pinctrl-1 = <&sdc2_sleep &sdc2_cd_default>;
>> + pinctrl-names = "default", "sleep";
>> + cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
>> + status = "okay";
>> +};
>> +
>> +&sound {
>> + pinctrl-0 = <&cdc_pdm_default &sec_mi2s_default>;
>> + pinctrl-1 = <&cdc_pdm_sleep &sec_mi2s_sleep>;
>> + pinctrl-names = "default", "sleep";
>> + model = "HMIBSC";
>> + audio-routing =
>> + "AMIC2", "MIC BIAS Internal2",
>> + "AMIC3", "MIC BIAS External1";
>> + status = "okay";
>> +
>> + quaternary-dai-link {
>> + link-name = "ADV7533";
>> + cpu {
>> + sound-dai = <&lpass MI2S_QUATERNARY>;
>> + };
>> + codec {
>> + sound-dai = <&adv_bridge 0>;
>> + };
>> + };
>> +
>> + primary-dai-link {
>> + link-name = "WCD";
>> + cpu {
>> + sound-dai = <&lpass MI2S_PRIMARY>;
>> + };
>> + codec {
>> + sound-dai = <&lpass_codec 0>, <&pm8916_codec 0>;
>> + };
>> + };
>> +
>> + tertiary-dai-link {
>> + link-name = "WCD-Capture";
>> + cpu {
>> + sound-dai = <&lpass MI2S_TERTIARY>;
>> + };
>> + codec {
>> + sound-dai = <&lpass_codec 1>, <&pm8916_codec 1>;
>> + };
>> + };
>> +};
>> +
>> +&tlmm {
>> + pinctrl-0 = <&uart1_mux0_rs232_high &uart1_mux1_rs232_low>;
>> + pinctrl-names = "default";
>> +
>> + adv7533_int_active: adv533-int-active-state {
>> + pins = "gpio31";
>> + function = "gpio";
>> + drive-strength = <16>;
>> + bias-disable;
>> + };
>> +
>> + adv7533_int_suspend: adv7533-int-suspend-state {
>> + pins = "gpio31";
>> + function = "gpio";
>> + drive-strength = <2>;
>> + bias-disable;
>> + };
>> +
>> + adv7533_switch_active: adv7533-switch-active-state {
>> + pins = "gpio32";
>> + function = "gpio";
>> + drive-strength = <16>;
>> + bias-disable;
>> + };
>> +
>> + adv7533_switch_suspend: adv7533-switch-suspend-state {
>> + pins = "gpio32";
>> + function = "gpio";
>> + drive-strength = <2>;
>> + bias-disable;
>> + };
>> +
>> + msm_key_volp_n_default: msm-key-volp-n-default-state {
>> + pins = "gpio107";
>> + function = "gpio";
>> + drive-strength = <8>;
>> + bias-pull-up;
>> + };
>> +
>> + sdc2_cd_default: sdc2-cd-default-state {
>> + pins = "gpio38";
>> + function = "gpio";
>> + drive-strength = <2>;
>> + bias-disable;
>> + };
>> +
>> + /*
>> + * UART1 being the debug console supports various modes of
>> + * operation (RS-232/485/422) controlled via GPIOs configured
>> + * mux as follows:
>> + *
>> + * gpio100 gpio99 UART mode
>> + * 0 0 loopback
>> + * 0 1 RS-232
>> + * 1 0 RS-485
>> + * 1 1 RS-422
>> + *
>> + * The default mode configured here is RS-232 mode.
>> + */
>> + uart1_mux0_rs232_high: uart1-mux0-rs232-state {
>> + bootph-all;
>> + pins = "gpio99";
>> + function = "gpio";
>> + drive-strength = <16>;
>> + bias-disable;
>> + output-high;
>> + };
>> +
>> + uart1_mux1_rs232_low: uart1-mux1-rs232-state {
>> + bootph-all;
>> + pins = "gpio100";
>> + function = "gpio";
>> + drive-strength = <16>;
>> + bias-disable;
>> + output-low;
>> + };
>> +
>> + usb_id_default: usb-id-default-state {
>> + pins = "gpio110";
>> + function = "gpio";
>> + drive-strength = <8>;
>> + bias-pull-up;
>> + };
>> +};
>> +
>> +&usb {
>> + extcon = <&usb_id>, <&usb_id>;
>> + pinctrl-0 = <&usb_sw_sel_pm &usb_hub_reset_pm>;
>> + pinctrl-1 = <&usb_sw_sel_pm_device &usb_hub_reset_pm_device>;
>> + pinctrl-names = "default", "device";
>> + status = "okay";
>> +};
>> +
>> +&usb_hs_phy {
>> + extcon = <&usb_id>;
>> +};
>> +
>> +&wcnss {
>> + firmware-name = "qcom/apq8016/wcnss.mbn";
>> + status = "okay";
>> +};
>> +
>> +&wcnss_ctrl {
>> + firmware-name = "qcom/apq8016/WCNSS_qcom_wlan_nv_sbc.bin";
>> +};
>> +
>> +&wcnss_iris {
>> + compatible = "qcom,wcn3620";
>> +};
>> +
>> +&wcnss_mem {
>> + status = "okay";
>> +};
>> +
>> +/* PINCTRL - additions to nodes defined in msm8916.dtsi */
>> +
>> +/*
>> + * 2mA drive strength is not enough when connecting multiple
>> + * I2C devices with different pull up resistors.
>> + */
>> +&blsp_i2c4_default {
>> + drive-strength = <16>;
>> +};
>> +
>> +&blsp_i2c6_default {
>> + drive-strength = <16>;
>> +};
>> +
>> +&blsp_uart1_default {
>> + bootph-all;
>> +};
>> +
>> +/* Enable CoreSight */
>> +&cti0 { status = "okay"; };
>> +&cti1 { status = "okay"; };
>> +&cti12 { status = "okay"; };
>> +&cti13 { status = "okay"; };
>> +&cti14 { status = "okay"; };
>> +&cti15 { status = "okay"; };
>> +&debug0 { status = "okay"; };
>> +&debug1 { status = "okay"; };
>> +&debug2 { status = "okay"; };
>> +&debug3 { status = "okay"; };
>> +&etf { status = "okay"; };
>> +&etm0 { status = "okay"; };
>> +&etm1 { status = "okay"; };
>> +&etm2 { status = "okay"; };
>> +&etm3 { status = "okay"; };
>> +&etr { status = "okay"; };
>> +&funnel0 { status = "okay"; };
>> +&funnel1 { status = "okay"; };
>> +&replicator { status = "okay"; };
>> +&stm { status = "okay"; };
>> +&tpiu { status = "okay"; };
>> --
>> 2.34.1
>>

--
// Caleb (they/them)