| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4a0a8db7-a2bc-4c99-94b2-c13facbd1bef@linaro.org>
Date: Wed, 13 Mar 2024 14:00:01 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Sumit Garg <sumit.garg@...aro.org>, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org
Cc: andersson@...nel.org, konrad.dybcio@...aro.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, stephan@...hold.net,
caleb.connolly@...aro.org, neil.armstrong@...aro.org,
laetitia.mariottini@...com, pascal.eberhard@...com, abdou.saker@...com,
jimmy.lalande@...com, benjamin.missey@....se.com,
daniel.thompson@...aro.org, linux-kernel@...r.kernel.org,
Jagdish Gediya <jagdish.gediya@...aro.org>
Subject: Re: [PATCH v2 3/3] arm64: dts: qcom: apq8016: Add Schneider HMIBSC
board DTS
On 13/03/2024 13:30, 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.
>
..
> +
> +/ {
> + model = "Schneider Electric HMIBSC Board";
> + compatible = "schneider,apq8016-hmibsc", "qcom,apq8016";
> +
> + aliases {
> + mmc0 = &sdhc_1; /* eMMC */
> + mmc1 = &sdhc_2; /* SD card */
> + serial0 = &blsp_uart1;
> + serial1 = &blsp_uart2;
> + usid0 = &pm8916_0;
> + i2c1 = &blsp_i2c6;
> + i2c3 = &blsp_i2c4;
> + i2c4 = &blsp_i2c3;
The aliases should match schematics of the board, so I assume missing
i2c2 is intentional, right?
> + spi0 = &blsp_spi5;
> + };
> +
> + chosen {
> + stdout-path = "serial0";
> + };
> +
> + memory@...00000 {
> + reg = <0 0x80000000 0 0x40000000>;
> + };
> +
> + reserved-memory {
> + ramoops@...00000 {
> + compatible = "ramoops";
> + reg = <0x0 0xbff00000 0x0 0x100000>;
> +
> + record-size = <0x20000>;
> + console-size = <0x20000>;
> + ftrace-size = <0x20000>;
> + };
> + };
> +
> + usb2513 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
e.g. 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-names = "default";
> + pinctrl-0 = <&usb_id_default>;
> + };
> +
> + hdmi-out {
> + compatible = "hdmi-connector";
> + type = "a";
> +
> + port {
> + hdmi_con: endpoint {
> + remote-endpoint = <&adv7533_out>;
> + };
> + };
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> + autorepeat;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&msm_key_volp_n_default>;
> +
> + button {
> + label = "Volume Up";
> + linux,code = <KEY_VOLUMEUP>;
> + gpios = <&tlmm 107 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> + leds {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pm8916_mpps_leds>;
First property is always compatible. Please apply DTS coding style rules.
> +
> + compatible = "gpio-leds";
> + #address-cells = <1>;
> + #size-cells = <0>;
That's not a bus.
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
> +
> + led@5 {
> + reg = <5>;
> + label = "apq8016-hmibsc:green:wlan";
> + function = LED_FUNCTION_WLAN;
> + color = <LED_COLOR_ID_YELLOW>;
> + gpios = <&pm8916_mpps 2 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "phy0tx";
> + default-state = "off";
> + };
> +
> + led@6 {
> + reg = <6>;
> + label = "apq8016-hmibsc:yellow:bt";
> + function = LED_FUNCTION_BLUETOOTH;
> + color = <LED_COLOR_ID_BLUE>;
> + gpios = <&pm8916_mpps 3 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "bluetooth-power";
> + default-state = "off";
> + };
> + };
> +};
> +
> +&blsp_i2c3 {
> + status = "okay";
> +
> + eeprom@50 {
> + compatible = "atmel,24c32";
> + reg = <0x50>;
> + };
> +};
> +
> +&blsp_i2c4 {
> + status = "okay";
> +
> + adv_bridge: bridge@39 {
> + status = "okay";
Why do you need it? Was it disabled?
And why this is before compatible? If this stays, please use DTS coding
style rules for placement.
> +
> + compatible = "adi,adv7533";
> + reg = <0x39>;
> +
> + interrupt-parent = <&tlmm>;
> + interrupts = <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-names = "default","sleep";
> + pinctrl-0 = <&adv7533_int_active &adv7533_switch_active>;
> + pinctrl-1 = <&adv7533_int_suspend &adv7533_switch_suspend>;
> + #sound-dai-cells = <1>;
> +
> + 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 {
> + status = "okay";
> + cs-gpios = <&tlmm 18 GPIO_ACTIVE_LOW>;
> +
> + tpm@0 {
> + compatible = "tcg,tpm_tis-spi";
> + reg = <0>;
> + spi-max-frequency = <500000>;
> + };
> +};
> +
> +&blsp_uart1 {
> + status = "okay";
> + label = "UART0";
> +};
> +
> +&blsp_uart2 {
> + status = "okay";
> + label = "UART1";
> +};
> +
> +&lpass {
> + status = "okay";
> +};
> +
> +&mdss {
> + status = "okay";
> +};
> +
> +&mdss_dsi0_out {
> + data-lanes = <0 1 2 3>;
> + remote-endpoint = <&adv7533_in>;
> +};
> +
> +&pm8916_codec {
> + status = "okay";
> + qcom,mbhc-vthreshold-low = <75 150 237 450 500>;
> + qcom,mbhc-vthreshold-high = <75 150 237 450 500>;
> +};
> +
> +&pm8916_resin {
> + status = "okay";
> + linux,code = <KEY_POWER>;
> +};
> +
> +&pm8916_rpm_regulators {
> + pm8916_l17: l17 {
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +};
> +
> +&sdhc_1 {
> + status = "okay";
> +};
> +
> +&sdhc_2 {
> + status = "okay";
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&sdc2_default &sdc2_cd_default>;
> + pinctrl-1 = <&sdc2_sleep &sdc2_cd_default>;
> +
> + cd-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
> +};
> +
> +&sound {
> + status = "okay";
Is thi sneeded?
> +
> + pinctrl-0 = <&cdc_pdm_default &sec_mi2s_default>;
> + pinctrl-1 = <&cdc_pdm_sleep &sec_mi2s_sleep>;
> + pinctrl-names = "default", "sleep";
> + model = "DB410c";
> + audio-routing =
> + "AMIC2", "MIC BIAS Internal2",
> + "AMIC3", "MIC BIAS External1";
> +
> + 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>;
> + };
> + };
> +};
> +
> +&usb {
> + status = "okay";
> + extcon = <&usb_id>, <&usb_id>;
> +
> + pinctrl-names = "default", "device";
> + pinctrl-0 = <&usb_sw_sel_pm &usb_hub_reset_pm>;
> + pinctrl-1 = <&usb_sw_sel_pm_device &usb_hub_reset_pm_device>;
> +};
> +
> +&usb_hs_phy {
> + extcon = <&usb_id>;
> +};
> +
> +&wcnss {
> + status = "okay";
> + firmware-name = "qcom/apq8016/wcnss.mbn";
> +};
> +
> +&wcnss_ctrl {
> + firmware-name = "qcom/apq8016/WCNSS_qcom_wlan_nv_sbc.bin";
> +};
> +
> +&wcnss_iris {
> + compatible = "qcom,wcn3620";
> +};
> +
> +&wcnss_mem {
> + status = "okay";
> +};
> +
> +/* 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"; };
> +
> +/*
> + * 2mA drive strength is not enough when connecting multiple
> + * I2C devices with different pull up resistors.
> + */
> +
> +&blsp_i2c4_default {
None of your overrides look like have proper alphabetical order. Please
use alphabetical order.
Best regards,
Krzysztof
Powered by blists - more mailing lists