| 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
| ||
|
Date: Thu, 12 Jan 2023 17:29:39 +0100
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Nikita Travkin <nikita@...n.ru>
Cc: Yang Xiwen <forbidden405@...mail.com>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org,
Jaime Breva <jbreva@...arsystems.com>,
~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH v2 3/3] arm64: dts: qcom: msm8916-thwc: Add initial device
tree for Tong Heng Wei Chuang Wifi/LTE dongle UFI-001C and uf896
On 12.01.2023 17:28, Nikita Travkin wrote:
> Konrad Dybcio писал(а) 12.01.2023 21:16:
>> On 11.01.2023 15:13, Yang Xiwen wrote:
>>> This commit adds support for the ufi-001C and uf896 WiFi/LTE dongle made by
>>> Tong Heng Wei Chuang based on MSM8916.
>>> uf896 is another variant for the usb stick. The board design
>>> differs by using different gpios for the keys and leds.
>>>
>>> Note: The original firmware does not support 64-bit OS. It is necessary
>>> to flash 64-bit TZ firmware to boot arm64.
>>>
>>> Currently supported:
>>> - All CPU cores
>>> - Buttons
>>> - LEDs
>>> - Modem
>>> - SDHC
>>> - USB Device Mode
>>> - UART
>>>
>>> Co-developed-by: Jaime Breva <jbreva@...arsystems.com>
>>> Signed-off-by: Jaime Breva <jbreva@...arsystems.com>
>>> Co-developed-by: Nikita Travkin <nikita@...n.ru>
>>> Signed-off-by: Nikita Travkin <nikita@...n.ru>
>>> Signed-off-by: Yang Xiwen <forbidden405@...mail.com>
>>> ---
>>> arch/arm64/boot/dts/qcom/Makefile | 2 +
>>> .../boot/dts/qcom/msm8916-thwc-uf896.dts | 41 +++
>>> .../boot/dts/qcom/msm8916-thwc-ufi001c.dts | 39 +++
>>> arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi | 246 ++++++++++++++++++
>>> 4 files changed, 328 insertions(+)
>>> create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>>> create mode 100644 arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>>> create mode 100644 arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>>> index b42362c7be1bf..523dbe6d06be8 100644
>>> --- a/arch/arm64/boot/dts/qcom/Makefile
>>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>>> @@ -21,6 +21,8 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-e7.dtb
>>> dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-grandmax.dtb
>>> dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-j5.dtb
>>> dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-serranove.dtb
>>> +dtb-$(CONFIG_ARCH_QCOM) += msm8916-thwc-uf896.dtb
>>> +dtb-$(CONFIG_ARCH_QCOM) += msm8916-thwc-ufi001c.dtb
>>> dtb-$(CONFIG_ARCH_QCOM) += msm8916-wingtech-wt88047.dtb
>>> dtb-$(CONFIG_ARCH_QCOM) += msm8953-motorola-potter.dtb
>>> dtb-$(CONFIG_ARCH_QCOM) += msm8953-xiaomi-daisy.dtb
>>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>>> new file mode 100644
>>> index 0000000000000..79447ca48dd3a
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-uf896.dts
>>> @@ -0,0 +1,41 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "msm8916-ufi.dtsi"
>>> +
>>> +/ {
>>> + model = "uf896 4G Modem Stick";
>>> + compatible = "thwc,uf896", "qcom,msm8916";
>>> +};
>>> +
>>> +&button_restart {
>>> + gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
>>> +};
>>> +
>>> +&led_r {
>>> + gpios = <&msmgpio 82 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&led_g {
>>> + gpios = <&msmgpio 83 GPIO_ACTIVE_HIGH>;
>>> + function = LED_FUNCTION_WLAN;
>> This is a software property, maybe it would be worth
>> commonizing?
>>
>
> The functions are put in-line with the labels on the housing.
> The uf896 has green and blue leds labeled with icons but I was told
> by Yang that the uf001c doesn't have any labels.
Okay, that probably deserves a comment.
>
>>> +};
>>> +
>>> +&led_b {
>>> + gpios = <&msmgpio 81 GPIO_ACTIVE_HIGH>;
>>> + function = LED_FUNCTION_WAN;
>>> +};
>>> +
>>> +&button_default {
>>> + pins = "gpio35";
>>> + bias-pull-up;
>>> +};
>>> +
>>> +&gpio_leds_default {
>>> + pins = "gpio81", "gpio82", "gpio83";
>>> +};
>>> +
>>> +&sim_ctrl_default {
>>> + pins = "gpio1", "gpio2";
>>> +};
>>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>>> new file mode 100644
>>> index 0000000000000..700cf81cbf8c0
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/msm8916-thwc-ufi001c.dts
>>> @@ -0,0 +1,39 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +/dts-v1/;
>>> +
>>> +#include "msm8916-ufi.dtsi"
>>> +
>>> +/ {
>>> + model = "ufi-001c/ufi-001b 4G Modem Stick";
>>> + compatible = "thwc,ufi001c", "qcom,msm8916";
>>> +};
>>> +
>>> +&button_restart {
>>> + gpios = <&msmgpio 37 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&led_r {
>>> + gpios = <&msmgpio 22 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&led_g {
>>> + gpios = <&msmgpio 21 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&led_b {
>>> + gpios = <&msmgpio 20 GPIO_ACTIVE_HIGH>;
>>> +};
>>> +
>>> +&button_default {
>>> + pins = "gpio37";
>>> + bias-pull-down;
>>> +};
>>> +
>>> +&gpio_leds_default {
>>> + pins = "gpio20", "gpio21", "gpio22";
>>> +};
>>> +
>>> +&sim_ctrl_default {
>>> + pins = "gpio1", "gpio2";
>> Identical between both boards, move to common please.
>>
>>> +};
>>> diff --git a/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>>> new file mode 100644
>>> index 0000000000000..286768d0fbe70
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/qcom/msm8916-ufi.dtsi
>>> @@ -0,0 +1,246 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +
>>> +#include "msm8916-pm8916.dtsi"
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +
>>> +/ {
>>> + chassis-type = "embedded";
>>> +
>>> + aliases {
>>> + serial0 = &blsp1_uart2;
>>> + };
>>> +
>>> + chosen {
>>> + stdout-path = "serial0";
>>> + };
>>> +
>>> + reserved-memory {
>>> + mpss_mem: mpss@...00000 {
>>> + reg = <0x0 0x86800000 0x0 0x5500000>;
>>> + no-map;
>>> + };
>>> +
>>> + gps_mem: gps@...00000 {
>>> + reg = <0x0 0x8bd00000 0x0 0x200000>;
>>> + no-map;
>>> + };
>>> + };
>>> +
>>> + gpio-keys {
>>> + compatible = "gpio-keys";
>>> +
>>> + pinctrl-0 = <&button_default>;
>>> + pinctrl-names = "default";
>>> +
>>> + label = "GPIO Buttons";
>>> +
>>> + /* gpio is board-specific */
>> GPIO is an acronym, please make it uppercase, as you
>> did 10 lines below..
>>
>>> + button_restart: button-restart {
>>> + label = "Restart";
>>> + linux,code = <KEY_RESTART>;
>>> + };
>>> + };
>>> +
>>> + leds {
>>> + compatible = "gpio-leds";
>>> +
>>> + pinctrl-0 = <&gpio_leds_default>;
>>> + pinctrl-names = "default";
>>> +
>>> + /* GPIOs are board-specific */
>>> + led_r: led-r {
>>> + color = <LED_COLOR_ID_RED>;
>>> + default-state = "on";
>>> + function = LED_FUNCTION_INDICATOR;
>>> + };
>>> +
>>> + led_g: led-g {
>>> + color = <LED_COLOR_ID_GREEN>;
>>> + default-state = "off";
>>> + function = LED_FUNCTION_INDICATOR;
>>> + };
>>> +
>>> + led_b: led-b {
>>> + color = <LED_COLOR_ID_BLUE>;
>>> + default-state = "off";
>>> + function = LED_FUNCTION_INDICATOR;
>>> + };
>>> + };
>>> +};
>>> +
>>> +&bam_dmux {
>>> + status = "okay";
>>> +};
>>> +
>>> +&bam_dmux_dma {
>>> + status = "okay";
>>> +};
>>> +
>>> +&blsp1_uart2 {
>>> + status = "okay";
>>> +};
>>> +
>>> +/* Remove &dsi_phy0 from clocks to make sure that gcc probes with display disabled */
>> That sounds like a bad regression and shouldn't be an issue..
>>
>>> +&gcc {
>>> + clocks = <&xo_board>, <&sleep_clk>, <0>, <0>, <0>, <0>, <0>;
>>> +};
>>> +
>>> +&mpss {
>>> + pinctrl-0 = <&sim_ctrl_default>;
>>> + pinctrl-names = "default";
>>> +
>>> + status = "okay";
>>> +};
>>> +
>>> +&pm8916_usbin {
>>> + status = "okay";
>>> +};
>>> +
>>> +&pronto {
>>> + status = "okay";
>>> +};
>>> +
>>> +&sdhc_1 {
>>> + pinctrl-0 = <&sdc1_clk_on &sdc1_cmd_on &sdc1_data_on>;
>>> + pinctrl-1 = <&sdc1_clk_off &sdc1_cmd_off &sdc1_data_off>;
>>> + pinctrl-names = "default", "sleep";
>>> +
>>> + status = "okay";
>>> +};
>>> +
>>> +&usb {
>>> + extcon = <&pm8916_usbin>;
>>> + dr_mode = "peripheral";
>>> +
>>> + status = "okay";
>>> +};
>>> +
>>> +&usb_hs_phy {
>>> + extcon = <&pm8916_usbin>;
>>> +};
>>> +
>>> +&smd_rpm_regulators {
>> Please sort this reference alphabetically.
>>
>
> The regulators and tlmm nodes are deliberately put last,
> in line with all other msm8916 devices. We move them away
> so they don't clutter more interesting nodes with these "dry"
> and somewhat uninteresting voltage/pin definitions.
>
> Thanks for the review,
> Nikita
Ugh, I don't know anymore.. wild west..
Konrad
>
>> Konrad
>>> + vdd_l1_l2_l3-supply = <&pm8916_s3>;
>>> + vdd_l4_l5_l6-supply = <&pm8916_s4>;
>>> + vdd_l7-supply = <&pm8916_s4>;
>>> +
>>> + s3 {
>>> + regulator-min-microvolt = <1200000>;
>>> + regulator-max-microvolt = <1300000>;
>>> + };
>>> +
>>> + s4 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <2100000>;
>>> + };
>>> +
>>> + l1 {
>>> + regulator-min-microvolt = <1225000>;
>>> + regulator-max-microvolt = <1225000>;
>>> + };
>>> +
>>> + l2 {
>>> + regulator-min-microvolt = <1200000>;
>>> + regulator-max-microvolt = <1200000>;
>>> + };
>>> +
>>> + l4 {
>>> + regulator-min-microvolt = <2050000>;
>>> + regulator-max-microvolt = <2050000>;
>>> + };
>>> +
>>> + l5 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <1800000>;
>>> + };
>>> +
>>> + l6 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <1800000>;
>>> + };
>>> +
>>> + l7 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <1800000>;
>>> + };
>>> +
>>> + l8 {
>>> + regulator-min-microvolt = <2850000>;
>>> + regulator-max-microvolt = <2900000>;
>>> + };
>>> +
>>> + l9 {
>>> + regulator-min-microvolt = <3300000>;
>>> + regulator-max-microvolt = <3300000>;
>>> + };
>>> +
>>> + l10 {
>>> + regulator-min-microvolt = <2700000>;
>>> + regulator-max-microvolt = <2800000>;
>>> + };
>>> +
>>> + l11 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <2950000>;
>>> + regulator-system-load = <200000>;
>>> + regulator-allow-set-load;
>>> + };
>>> +
>>> + l12 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <2950000>;
>>> + };
>>> +
>>> + l13 {
>>> + regulator-min-microvolt = <3075000>;
>>> + regulator-max-microvolt = <3075000>;
>>> + };
>>> +
>>> + l14 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <3300000>;
>>> + };
>>> +
>>> + l15 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <3300000>;
>>> + };
>>> +
>>> + l16 {
>>> + regulator-min-microvolt = <1800000>;
>>> + regulator-max-microvolt = <3300000>;
>>> + };
>>> +
>>> + l17 {
>>> + regulator-min-microvolt = <2850000>;
>>> + regulator-max-microvolt = <2850000>;
>>> + };
>>> +
>>> + l18 {
>>> + regulator-min-microvolt = <2700000>;
>>> + regulator-max-microvolt = <2700000>;
>>> + };
>>> +};
>>> +
>>> +&msmgpio {
>>> + /* pins are board-specific */
>>> + button_default: button-default-state {
>>> + function = "gpio";
>>> + drive-strength = <2>;
>>> + };
>>> +
>>> + gpio_leds_default: gpio-leds-default-state {
>>> + function = "gpio";
>>> + drive-strength = <2>;
>>> + bias-disable;
>>> + };
>>> +
>>> + sim_ctrl_default: sim-ctrl-default-state {
>>> + function = "gpio";
>>> + drive-strength = <2>;
>>> + bias-disable;
>>> + output-low;
>>> + };
>>> +};
Powered by blists - more mailing lists