[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <88da307c-0403-405d-8356-c8baeb18eaba@ixit.cz>
Date: Sun, 9 Mar 2025 10:17:29 +0100
From: David Heidelberg <david@...t.cz>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Kees Cook <kees@...nel.org>,
Tony Luck <tony.luck@...el.com>, "Guilherme G. Piccoli"
<gpiccoli@...lia.com>, linux-arm-msm@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org, Ivan Belokobylskiy <belokobylskij@...il.com>
Subject: Re: [PATCH v3] ARM: dts: nexus4: Initial dts
Hello Dmitry!
Thank you for looking into it. See replies.
On 09/03/2025 09:33, Dmitry Baryshkov wrote:
> On Sun, Mar 09, 2025 at 01:45:51AM +0100, David Heidelberg via B4 Relay wrote:
>> From: Ivan Belokobylskiy <belokobylskij@...il.com>
>>
>> Add initial support for LG Nexus 4 (mako).
>>
>> Features currently working: regulators, eMMC, WiFi, and volume keys.
>>
>> Signed-off-by: Ivan Belokobylskiy <belokobylskij@...il.com>
>> Signed-off-by: David Heidelberg <david@...t.cz>
>> ---
>> Changes in v3:
>> - rebased against next-20250307
>> - dropped backlight until driver gets converted to DT
>>
>> Changes in v2:
>> - lge vendor doesn't exist anymore, rename to lg
>> - sdcc@ to mmc@ to comply with dt-schema
>> ---
>> arch/arm/boot/dts/qcom/Makefile | 1 +
>> .../boot/dts/qcom/qcom-apq8064-lg-nexus4-mako.dts | 342 +++++++++++++++++++++
>> 2 files changed, 343 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/qcom/Makefile b/arch/arm/boot/dts/qcom/Makefile
>> index f06c6d425e91dd73c2b453d15543d95bd32383b9..0c1d116f6e84f76994aa8c8286350bdcd1657a42 100644
>> --- a/arch/arm/boot/dts/qcom/Makefile
>> +++ b/arch/arm/boot/dts/qcom/Makefile
>> @@ -12,6 +12,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \
>> qcom-apq8064-ifc6410.dtb \
>> qcom-apq8064-sony-xperia-lagan-yuga.dtb \
>> qcom-apq8064-asus-nexus7-flo.dtb \
>> + qcom-apq8064-lg-nexus4-mako.dtb \
>> qcom-apq8074-dragonboard.dtb \
>> qcom-apq8084-ifc6540.dtb \
>> qcom-apq8084-mtp.dtb \
>> diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064-lg-nexus4-mako.dts b/arch/arm/boot/dts/qcom/qcom-apq8064-lg-nexus4-mako.dts
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..bcb57675aa24892b290d543601f7a6b36b6a65f6
>> --- /dev/null
>> +++ b/arch/arm/boot/dts/qcom/qcom-apq8064-lg-nexus4-mako.dts
>> @@ -0,0 +1,342 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +#include "qcom-apq8064-v2.0.dtsi"
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/mfd/qcom-rpm.h>
>> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>> +/ {
>> + model = "LG Nexus 4 (mako)";
>> + compatible = "lg,nexus4-mako", "qcom,apq8064";
>> +
>> + aliases {
>> + serial0 = &gsbi7_serial;
>> + serial1 = &gsbi6_serial;
>> + serial2 = &gsbi4_serial;
>> + };
>> +
>> + chosen {
>> + stdout-path = "serial2:115200n8";
>> + };
>> +
>> + reserved-memory {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges;
>> +
>> + ramoops@...00000{
>> + compatible = "ramoops";
>> + reg = <0x88d00000 0x100000>;
>> + record-size = <0x00020000>;
>> + console-size = <0x00020000>;
>> + ftrace-size = <0x00020000>;
>> + };
>> + };
>> +
>> + battery_cell: battery-cell {
>> + compatible = "simple-battery";
>> + constant-charge-current-max-microamp = <900000>;
>> + operating-range-celsius = <0 45>;
>> + };
>> +
>> + soc {
>> + rpm@...000 {
>
> Please use &rpm { ... }; instead.
>
>> + regulators {
>> + vdd_l1_l2_l12_l18-supply = <&pm8921_s4>;
>> + vin_lvs1_3_6-supply = <&pm8921_s4>;
>
> Please move vin_lvs2-supply here.
>
>> + vin_lvs4_5_7-supply = <&pm8921_s4>;
>> +
>
> Please move vdd_l1_l2_l12_l18 here.
>
>> + vdd_l24-supply = <&pm8921_s1>;
>> + vdd_l25-supply = <&pm8921_s1>;
>> + vin_lvs2-supply = <&pm8921_s1>;
>> +
>> + vdd_l26-supply = <&pm8921_s7>;
>> + vdd_l27-supply = <&pm8921_s7>;
>> + vdd_l28-supply = <&pm8921_s7>;
>> +
>> + /* Buck SMPS */
>> + s1 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <1225000>;
>> + regulator-max-microvolt = <1225000>;
>> + qcom,switch-mode-frequency = <3200000>;
>> + bias-pull-down;
>> + };
>
> empty line
>
>> + s2 {
>> + regulator-min-microvolt = <1300000>;
>> + regulator-max-microvolt = <1300000>;
>> + qcom,switch-mode-frequency = <1600000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* msm otg HSUSB_VDDCX */
>> + s3 {
>> + regulator-min-microvolt = <500000>;
>> + regulator-max-microvolt = <1150000>;
>> + qcom,switch-mode-frequency = <4800000>;
>> + bias-pull-down;
>> + };
>> +
>> + /*
>> + * msm_sdcc.1-sdc-vdd_io
>> + * tabla2x-slim-CDC_VDDA_RX
>> + * tabla2x-slim-CDC_VDDA_TX
>> + * tabla2x-slim-CDC_VDD_CP
>> + * tabla2x-slim-VDDIO_CDC
>> + */
>> + s4 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + qcom,switch-mode-frequency = <1600000>;
>> + bias-pull-down;
>> + qcom,force-mode = <QCOM_RPM_FORCE_MODE_AUTO>;
>> + };
>> +
>> + /*
>> + * supply vdd_l26, vdd_l27, vdd_l28
>> + */
>> + s7 {
>> + regulator-min-microvolt = <1300000>;
>> + regulator-max-microvolt = <1300000>;
>> + qcom,switch-mode-frequency = <3200000>;
>> + };
>> +
>> + s8 {
>> + regulator-min-microvolt = <2200000>;
>> + regulator-max-microvolt = <2200000>;
>> + qcom,switch-mode-frequency = <1600000>;
>> + };
>> +
>> + l1 {
>> + regulator-min-microvolt = <1100000>;
>> + regulator-max-microvolt = <1100000>;
>> + regulator-always-on;
>> + bias-pull-down;
>> + };
>> +
>> + /* mipi_dsi.1-dsi1_pll_vdda */
>> + l2 {
>> + regulator-min-microvolt = <1200000>;
>> + regulator-max-microvolt = <1200000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* msm_otg-HSUSB_3p3 */
>> + l3 {
>> + regulator-min-microvolt = <3075000>;
>> + regulator-max-microvolt = <3500000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* msm_otg-HSUSB_1p8 */
>> + l4 {
>> + regulator-always-on;
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + };
>> +
>> + /* msm_sdcc.1-sdc_vdd */
>> + l5 {
>> + regulator-min-microvolt = <2950000>;
>> + regulator-max-microvolt = <2950000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* earjack_debug */
>> + l6 {
>> + regulator-min-microvolt = <3000000>;
>> + regulator-max-microvolt = <3000000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* mipi_dsi.1-dsi_vci */
>> + l8 {
>> + regulator-min-microvolt = <2800000>;
>> + regulator-max-microvolt = <3000000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* wcnss_wlan.0-iris_vddpa */
>> + l10 {
>> + regulator-min-microvolt = <2900000>;
>> + regulator-max-microvolt = <2900000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* mipi_dsi.1-dsi1_avdd */
>> + l11 {
>> + regulator-min-microvolt = <2850000>;
>> + regulator-max-microvolt = <2850000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* touch_vdd */
>> + l15 {
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <2950000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* slimport_dvdd */
>> + l18 {
>> + regulator-min-microvolt = <1100000>;
>> + regulator-max-microvolt = <1100000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* touch_io */
>> + l22 {
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + bias-pull-down;
>> + };
>> +
>> + /*
>> + * mipi_dsi.1-dsi_vddio
>> + * pil_qdsp6v4.1-pll_vdd
>> + * pil_qdsp6v4.2-pll_vdd
>> + * msm_ehci_host.0-HSUSB_1p8
>> + * msm_ehci_host.1-HSUSB_1p8
>> + */
>> + l23 {
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + bias-pull-down;
>> + };
>> +
>> + /*
>> + * tabla2x-slim-CDC_VDDA_A_1P2V
>> + * tabla2x-slim-VDDD_CDC_D
>> + */
>> + l24 {
>> + regulator-min-microvolt = <750000>;
>> + regulator-max-microvolt = <1150000>;
>> + bias-pull-down;
>> + };
>> +
>> + l25 {
>> + regulator-min-microvolt = <1250000>;
>> + regulator-max-microvolt = <1250000>;
>> + regulator-always-on;
>> + bias-pull-down;
>> + };
>> +
>> + l26 {
>> + regulator-min-microvolt = <375000>;
>> + regulator-max-microvolt = <1050000>;
>> + regulator-always-on;
>> + bias-pull-down;
>> + };
>> +
>> + l27 {
>> + regulator-min-microvolt = <1100000>;
>> + regulator-max-microvolt = <1100000>;
>> + };
>> +
>> + l28 {
>> + regulator-min-microvolt = <1050000>;
>> + regulator-max-microvolt = <1050000>;
>> + bias-pull-down;
>> + };
>> +
>> + /* wcnss_wlan.0-iris_vddio */
>> + lvs1 {
>> + bias-pull-down;
>> + };
>> +
>> + /* wcnss_wlan.0-iris_vdddig */
>> + lvs2 {
>> + bias-pull-down;
>> + };
>> +
>> + lvs3 {
>> + bias-pull-down;
>> + };
>> +
>> + lvs4 {
>> + bias-pull-down;
>> + };
>> +
>> + lvs5 {
>> + bias-pull-down;
>> + };
>> +
>> + /* mipi_dsi.1-dsi_iovcc */
>> + lvs6 {
>> + bias-pull-down;
>> + };
>> +
>> + /*
>> + * pil_riva-pll_vdd
>> + * lvds.0-lvds_vdda
>> + * mipi_dsi.1-dsi1_vddio
>> + * hdmi_msm.0-hdmi_vdda
>> + */
>> + lvs7 {
>> + bias-pull-down;
>> + };
>> +
>> + ncp {
>> + regulator-min-microvolt = <1800000>;
>> + regulator-max-microvolt = <1800000>;
>> + qcom,switch-mode-frequency = <1600000>;
>> + };
>> + };
>> + };
>> + };
>> +};
>> +
>> +&pmicintc {
>> + keypad@148 {
>> + compatible = "qcom,pm8921-keypad";
>> + reg = <0x148>;
>> + interrupt-parent = <&pmicintc>;
>> + interrupts = <74 1>, <75 1>;
>
> IRQ_TYPE_EDGE_RISING
>
>
>> + linux,keymap = <
>> + MATRIX_KEY(0, 0, KEY_VOLUMEDOWN)
>> + MATRIX_KEY(0, 1, KEY_VOLUMEUP)
>> + >;
>> +
>> + keypad,num-rows = <1>;
>> + keypad,num-columns = <5>;
>> + debounce = <15>;
>> + scan-delay = <32>;
>> + row-hold = <91500>;
>> + };
>> +};
>> +
>> +&gsbi1 {
>> + status = "okay";
>
> Status should be the last property. Add empty line before it (here and
> further)
>
>> + qcom,mode = <GSBI_PROT_I2C>;
>> +};
>> +
>> +&gsbi1_i2c {
>> + status = "okay";
>> + clock-frequency = <200000>;
>> + pinctrl-0 = <&i2c1_pins>;
>> + pinctrl-names = "default";
>
> I don't see i2c1_pins being defined. Did it pass the build?
> Also there is already a pinctrl for this device. Why do you need to
> overwrite it?
>
>> +};
>> +
>> +&gsbi4 {
>> + status = "okay";
>> + qcom,mode = <GSBI_PROT_I2C_UART>;
>> +};
>> +
>> +&gsbi4_serial {
>> + status = "okay";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&gsbi4_uart_pin_a>;
>
> Unnecessary, can be dropped. It's set in qcom-apq8064.dtsi.
>
>> +};
>> +
>> +/* eMMC */
>> +&sdcc1 {
>> + status = "okay";
>> + vmmc-supply = <&pm8921_l5>;
>> + vqmmc-supply = <&pm8921_s4>;
>> +};
>> +
>> +&riva {
>> + status = "okay";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&riva_wlan_pin_a>;
>
> Where is it defined? Also pinctrl-names should come after pinctrl-N.
definition is kinda aside in qcom-apq8064-pins.dtsi .
All other suggestions incorporated, if it's OK otherwise, let me send v2
Thank you
David
>
>> +};
>>
>> ---
>> base-commit: 0a2f889128969dab41861b6e40111aa03dc57014
>> change-id: 20250309-lg-nexus4-mako-da0833885b26
>>
>> Best regards,
>> --
>> David Heidelberg <david@...t.cz>
>>
>>
>
--
David Heidelberg
Powered by blists - more mailing lists