Message ID | 20240121154010.168440-5-i@rong.moe |
---|---|
State | Superseded |
Headers | show |
Series | ARM: dts: qcom: msm8974: Add Samsung Galaxy S5 China support | expand |
On 21/01/2024 16:39, Rong Zhang wrote: > This device has little difference compared to Samsung Galaxy S5 (klte), > so the device tree is based on qcom-msm8974pro-samsung-klte.dts. The > only difference is the gpio pins of i2c_led_gpio. With pins corrected, > the LEDs and WiFi are able to work properly. > > Signed-off-by: Rong Zhang <i@rong.moe> > diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts > new file mode 100644 > index 000000000000..5a8d59ea4439 > --- /dev/null > +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts > @@ -0,0 +1,16 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include "qcom-msm8974pro-samsung-klte.dts" > + > +/ { > + model = "Samsung Galaxy S5 China"; > + compatible = "samsung,kltechn", "samsung,klte", "qcom,msm8974pro", "qcom,msm8974"; That's not what you said in the binding. 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). Best regards, Krzysztof
在 2024-01-22星期一的 11:50 +0100,Konrad Dybcio写道: > On 21.01.2024 16:39, Rong Zhang wrote: > > This device has little difference compared to Samsung Galaxy S5 > > (klte), > > so the device tree is based on qcom-msm8974pro-samsung-klte.dts. > > The > > only difference is the gpio pins of i2c_led_gpio. With pins > > corrected, > > the LEDs and WiFi are able to work properly. > > > > Signed-off-by: Rong Zhang <i@rong.moe> > > --- > > Looks like you didn't change the brcm,board-type though? This should be intentional to allow kltechn and klte to share Wi-Fi NVRAM file. > > [...] > > > +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts > > @@ -0,0 +1,16 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include "qcom-msm8974pro-samsung-klte.dts" > > It's customary not to include .dts files, instead split the common > parts > into e.g. qcom-msm8974pro-samsung-klte-common.dtsi and include this > in > both the existing and the new one. > > Konrad
On Mon, 2024-01-22 at 16:17 +0100, Krzysztof Kozlowski wrote: > On 22/01/2024 15:51, Rong Zhang wrote: > > I've glanced similar dts. To solve this, I think we could either: > > 1. keep the dt-binding in [PATCH 3/4], and delete "samsung,klte" here > > 2. rewrite dt-binding like crystalfontz,cfa100{36,37,49,55,56,57,58}: [...] > > My preference is (2.) since other variants of klte may be added in the > > future. I would like to hear your preferences. > > It depends whether the devices are compatible. IOW, entire klte DTB > should work fine on kltechn, just without few new devices. Yes, they are compatible. I'd used the klte DTB on kltechn for a long time before I made this patchset. It worked totally fine expect for LEDs and WiFi, as I've said in the cover letter. Thanks, Rong > Best regards, > Krzysztof >
On Mon, 2024-01-22 at 11:50 +0100, Konrad Dybcio wrote: > Looks like you didn't change the brcm,board-type though? In "[PATCH 2/4] ARM: dts: qcom: msm8974-samsung-klte: Pin brcm,board- type in wifi": /* * This aims to allow other klte* variants to load the same firmware, * as klte variants have little differences in the wifi part. */ So it is intentional, in order to let them share the same FW, in particular, the NVRAM file. Without [PATCH 2/4] and with this [PATCH 4/4]: - klte DT probes "brcmfmac*.samsung,klte.txt" - kltechn DT probes "brcmfmac*.samsung,kltechn.txt", but never probes "brcmfmac*.samsung,klte.txt" With both [PATCH 2/4] and this [PATCH 4/4]: - klte DT probes "brcmfmac*.samsung,klte.txt" - kltechn DT probes "brcmfmac*.samsung,klte.txt" I pinned "brcm,board-type" in the klte DT instead of the kltechn one because other klte* variants are known to have little difference in the WiFi part. By pinning it in the klte DT, future ports could be easier. If you'd prefer not doing this, I am OK to drop [PATCH 2/4] in the v2 patchset. FYI: LineageOS considers all klte variants use "common" WiFi FW: https://github.com/search?q=repo%3ALineageOS%2Fandroid_kernel_samsung_klte+CONFIG_BCMDHD_NVRAM_PATH&type=code https://github.com/LineageOS/android_device_samsung_klte-common/blob/8f71a63415397def5ba886f4030b0d91e2253262/common-proprietary-files.txt#L251 I've tested the klte port of PostmarketOS (with klte FW and this patchset) on kltechn, and it worked fine. For the FW used by PostmarketOS, see also: https://gitlab.com/postmarketOS/pmaports/-/blob/439227770ffcd32eb7e26436598c804dc35637ad/device/testing/firmware-samsung-klte/APKBUILD#L17 > > +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts > > @@ -0,0 +1,16 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +#include "qcom-msm8974pro-samsung-klte.dts" > > It's customary not to include .dts files, instead split the common parts > into e.g. qcom-msm8974pro-samsung-klte-common.dtsi and include this in > both the existing and the new one. At the very beginning, I thought including .dts could make the patchset tiny (considering that the difference among klte* is also tiny). I agree that splitting the common parts into a .dtsi will make things more elegant and make klte* DTs consistent with the style of qcom DTs. Will do in v2. Thanks for your advice. Thanks, Rong
diff --git a/arch/arm/boot/dts/qcom/Makefile b/arch/arm/boot/dts/qcom/Makefile index 9cc1e14e6cd0..5d7a34adf826 100644 --- a/arch/arm/boot/dts/qcom/Makefile +++ b/arch/arm/boot/dts/qcom/Makefile @@ -44,6 +44,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \ qcom-msm8974pro-fairphone-fp2.dtb \ qcom-msm8974pro-oneplus-bacon.dtb \ qcom-msm8974pro-samsung-klte.dtb \ + qcom-msm8974pro-samsung-kltechn.dtb \ qcom-msm8974pro-sony-xperia-shinano-castor.dtb \ qcom-mdm9615-wp8548-mangoh-green.dtb \ qcom-sdx55-mtp.dtb \ diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts new file mode 100644 index 000000000000..5a8d59ea4439 --- /dev/null +++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "qcom-msm8974pro-samsung-klte.dts" + +/ { + model = "Samsung Galaxy S5 China"; + compatible = "samsung,kltechn", "samsung,klte", "qcom,msm8974pro", "qcom,msm8974"; +}; + +&i2c_led_gpio { + scl-gpios = <&tlmm 61 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; + sda-gpios = <&tlmm 60 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; +}; + +&i2c_led_gpioex_pins { + pins = "gpio60", "gpio61"; +};
This device has little difference compared to Samsung Galaxy S5 (klte), so the device tree is based on qcom-msm8974pro-samsung-klte.dts. The only difference is the gpio pins of i2c_led_gpio. With pins corrected, the LEDs and WiFi are able to work properly. Signed-off-by: Rong Zhang <i@rong.moe> --- arch/arm/boot/dts/qcom/Makefile | 1 + .../dts/qcom/qcom-msm8974pro-samsung-kltechn.dts | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 arch/arm/boot/dts/qcom/qcom-msm8974pro-samsung-kltechn.dts