From: Stephan Gerhold <stephan@gerhold.net>
To: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: agross@kernel.org, andersson@kernel.org,
konrad.dybcio@linaro.org, djakov@kernel.org, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, linux-arm-msm@vger.kernel.org,
linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, benl@squareup.com,
shawn.guo@linaro.org, fabien.parent@linaro.org,
leo.yan@linaro.org, dmitry.baryshkov@linaro.org
Subject: Re: [PATCH v7 5/5] arm64: dts: qcom: Add msm8939 Sony Xperia M4 Aqua
Date: Mon, 6 Mar 2023 17:14:08 +0100 [thread overview]
Message-ID: <ZAYRUIg0SwKOnBGx@gerhold.net> (raw)
In-Reply-To: <20230223153655.262783-6-bryan.odonoghue@linaro.org>
On Thu, Feb 23, 2023 at 03:36:55PM +0000, Bryan O'Donoghue wrote:
> Add a basic booting DTS for the Sony Xperia M4 Aqua aka "tulip".
>
> Tulip is paired with:
>
> - wcn3660
> - smb1360 battery charger
> - 720p Truly NT35521 Panel
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> .../qcom/msm8939-sony-xperia-kanuti-tulip.dts | 457 ++++++++++++++++++
> 2 files changed, 458 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 2983e83a19061..81a38d46deba5 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -29,6 +29,7 @@ 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) += msm8939-sony-xperia-kanuti-tulip.dtb
> dtb-$(CONFIG_ARCH_QCOM) += msm8953-motorola-potter.dtb
> dtb-$(CONFIG_ARCH_QCOM) += msm8953-xiaomi-daisy.dtb
> dtb-$(CONFIG_ARCH_QCOM) += msm8953-xiaomi-mido.dtb
> diff --git a/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
> new file mode 100644
> index 0000000000000..c646fada11a5a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8939-sony-xperia-kanuti-tulip.dts
> @@ -0,0 +1,457 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2022-2023, Bryan O'Donoghue.
> + *
> + */
> +
> +/dts-v1/;
> +
> +#include "msm8939.dtsi"
> +#include "msm8939-pm8916.dtsi"
> +#include <dt-bindings/arm/qcom,ids.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
> +
> +/ {
> + model = "Sony Xperia M4 Aqua";
> + compatible = "sony,kanuti-tulip", "qcom,msm8939";
> +
> + qcom,board-id = <8 0>;
Could use QCOM_BOARD_ID_MTP instead of 8 for more clarity here.
> [...]
> +&dsi0 {
> + status = "okay";
> +
> + panel@0 {
> + compatible = "sony,tulip-truly-nt35521";
> + reg = <0>;
> + positive5-supply = <&vreg_positive5_reg>;
> + negative5-supply = <&vreg_negative5_reg>;
> + reset-gpios = <&tlmm 25 GPIO_ACTIVE_LOW>;
> + enable-gpios = <&tlmm 10 GPIO_ACTIVE_LOW>;
Have you tested this? While this matches the (likely incorrect)
bindings, as far as I can tell the Linux tulip-truly-nt35521 driver
wants "backlight-gpios" instead of "enable-gpios". And it's not an
optional GPIO so I don't quite understand how the existing driver could
probe successfully with what you have here.
> +
> + ports {
> + port {
> + panel_in: endpoint {
> + remote-endpoint = <&dsi0_out>;
> + };
> + };
> + };
> +
> + };
> +};
> +
> [...]
> +&tlmm {
> + ak8963_default: ak8963-default-state {
> + pins = "gpio69";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <6>;
> + };
> +
> + ak8963_sleep: ak8963-sleep-state {
> + pins = "gpio69";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <2>;
> + };
> +
> + /* Ambient light and proximity sensor apds9930 and apds9900 */
> + apds99xx_default: apds99xx-default-state {
> + pins = "gpio113";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <6>;
> + };
> +
> + apds99xx_sleep: apds99xx-sleep-state {
> + pins = "gpio113";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <2>;
> + };
> +
> + cam_sensor_flash_default: cam-sensor-flash-default-state {
> + pins = "gpio98", "gpio97";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
> + };
> +
> + cci1_default: cci1-default-state {
> + pins = "gpio31", "gpio32";
> + function = "cci_i2c";
> + bias-disable;
> + drive-strength = <2>;
> + };
> +
> + cdc_ext_spk_pa_active: cdc-ext-spk-pa-on-state {
> + pins = "gpio0";
> + function = "gpio";
> + drive-strength = <8>;
> + output-low;
> + };
> +
> + cdc_ext_spk_pa_sus: cdc-ext-spk-pa-off-state {
> + pins = "gpio0";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
> + };
> +
> + cdc_slim_lines_act: lines-on-state {
> + pins = "gpio63";
> + function = "cdc_pdm0";
> + drive-strength = <8>;
> + output-high;
> + };
> +
> + cdc_slim_lines_sus: lines-off-state {
> + pins = "gpio63";
> + function = "cdc_pdm0";
> + bias-disable;
> + drive-strength = <2>;
> + };
> +
> + cross_conn_det_act: lines-on-state {
> + pins = "gpio120";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <8>;
> + output-low;
> + };
> +
> + cross_conn_det_sus: lines-off-state {
> + pins = "gpio120";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <2>;
> + };
> +
> + ext_buck_vsel: vsel0-state {
> + pins = "gpio111";
> + function = "gpio";
> + drive-strength = <2>;
> + };
> +
> + ext_cdc_tlmm_lines_act: tlmm-lines-on-state {
> + pins = "gpio116", "gpio112", "gpio117", "gpio118", "gpio119";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <8>;
> + };
> +
> + ext_cdc_tlmm_lines_sus: tlmm-lines-off-state {
> + pins = "gpio116", "gpio112", "gpio117", "gpio118", "gpio119";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <2>;
> + };
> +
> + gpio_key_suspend: gpio-key-suspend-state {
> + pins = "gpio107", "gpio108", "gpio109";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <2>;
> + };
> +
> + negative5_reg_default: negative5-reg-default-state {
> + pins = "gpio17";
> + function = "gpio";
> + output-low;
> + };
> +
> + positive5_reg_default: positive5-reg-default-state {
> + pins = "gpio114";
> + function = "gpio";
> + output-low;
> + };
> +
> + /* Gyroscope and accelerometer sensor combo */
> + mpu6050_default: mpu6050-default-state {
> + pins = "gpio115";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <6>;
> + };
> +
> + mpu6050_sleep: mpu6050-sleep-state {
> + pins = "gpio115";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <2>;
> + };
> +
> + nfc_disable_active: nfc-disable-active-state {
> + pins = "gpio20";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <6>;
> + };
> +
> + nfc_disable_suspend: nfc-disable-suspend-state {
> + pins = "gpio20";
> + function = "gpio";
> + bias-disable;
> + drive-strength = <6>;
> + };
> +
> + nfc_int_active: nfc-int-active-state {
> + pins = "gpio21";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <6>;
> + };
> +
> + nfc_int_suspend: nfc-int-suspend-state {
> + pins = "gpio21";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <6>;
> + };
> +
> + nt35521_te_default: nt35521-te-default-state {
> + pins = "gpio24";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <6>;
> + };
> +
> + nt35521_backlight: nt35521-backlight-default-state {
> + pins = "gpio10";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <6>;
> + };
> +
> + smb_int: smb-int-default-state {
> + pins = "gpio62";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <2>;
> + };
> +
> + ts_int_active: ts-int-active-state {
> + pins = "gpio13";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <16>;
> + };
> +
> + ts_int_suspend: ts-int-suspend-state {
> + pins = "gpio13";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <2>;
> + };
> +
> + ts_reset_active: ts-reset-active-state {
> + pins = "gpio12";
> + function = "gpio";
> + bias-pull-up;
> + drive-strength = <16>;
> + };
> +
> + ts_reset_suspend: ts-reset-suspend-state {
> + pins = "gpio12";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <2>;
> + };
> +
> + ts_release: ts-release-default-state {
> + pins = "gpio13", "gpio12";
> + function = "gpio";
> + bias-pull-down;
> + drive-strength = <2>;
> + };
> +
There are *a lot* of unused pinctrl entries here. If you don't reference
them anywhere (by referencing them via label) they just waste space in
the device tree. The GPIOs will still remain unconfigured.
Please save them locally for later usage and only keep the used entries.
This will ease review of upcoming patches since these are better
understandable together with the actual device node making use of them.
Thanks,
Stephan
next prev parent reply other threads:[~2023-03-06 16:47 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-23 15:36 [PATCH v7 0/5] Add MSM8939 SoC support with two devices Bryan O'Donoghue
2023-02-23 15:36 ` [PATCH v7 1/5] dt-bindings: vendor-prefixes: Add Square Bryan O'Donoghue
2023-02-23 15:36 ` [PATCH v7 2/5] arm64: dts: qcom: Add msm8939 SoC Bryan O'Donoghue
2023-02-23 15:43 ` Konrad Dybcio
2023-03-06 15:04 ` Stephan Gerhold
2023-03-06 15:25 ` Bryan O'Donoghue
2023-02-23 15:36 ` [PATCH v7 3/5] arm64: dts: qcom: Add msm8939-pm8916.dtsi include Bryan O'Donoghue
2023-02-23 15:36 ` [PATCH v7 4/5] arm64: dts: qcom: Add Square apq8039-t2 board Bryan O'Donoghue
2023-03-06 15:47 ` Stephan Gerhold
2023-02-23 15:36 ` [PATCH v7 5/5] arm64: dts: qcom: Add msm8939 Sony Xperia M4 Aqua Bryan O'Donoghue
2023-03-06 16:14 ` Stephan Gerhold [this message]
2023-03-06 22:56 ` Bryan O'Donoghue
2023-06-15 20:42 ` Pavel Machek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZAYRUIg0SwKOnBGx@gerhold.net \
--to=stephan@gerhold.net \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=benl@squareup.com \
--cc=bryan.odonoghue@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=djakov@kernel.org \
--cc=dmitry.baryshkov@linaro.org \
--cc=fabien.parent@linaro.org \
--cc=konrad.dybcio@linaro.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=leo.yan@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=shawn.guo@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox