Hi Bjorn, Thanks a lot for your review! On Thu, Nov 18, 2021 at 09:31:39PM -0600, Bjorn Andersson wrote: > On Tue 16 Nov 14:07 CST 2021, Julian Ribbeck wrote: > > > Samsung J5 2015 is a MSM8916 based Smartphone. It is similar to some of the > > other MSM8916 devices, especially the Samsung ones. > > > > With this patch initial support for the following is added: > > - eMMC/SD card > > - Buttons > > - USB (although no suiting MUIC driver currently) > > - UART (untested for lack of equipment) > > - WiFi/Bluetooth (WCNSS) > > > > It is worth noting that Samsung J5 with MSM8916 exists in different > > generations (e.g Samsung J5 2015 and Samsung J5 2016) which each have > > different models (e.g. samsung-j5nlte, samsung-j5xnlte, etc). This patch > > is only regarding the 2015 generation, but should work with all of it's > > models, as far as we could test. > > > > Co-developed-by: Stephan Gerhold <stephan@xxxxxxxxxxx> > > Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx> > > Signed-off-by: Julian Ribbeck <julian.ribbeck@xxxxxx> > > --- > > arch/arm64/boot/dts/qcom/Makefile | 1 + > > .../boot/dts/qcom/msm8916-samsung-j5.dts | 209 ++++++++++++++++++ > > 2 files changed, 210 insertions(+) > > create mode 100644 arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts > > > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > > index 6b816eb33309..08bfccb0daeb 100644 > > --- a/arch/arm64/boot/dts/qcom/Makefile > > +++ b/arch/arm64/boot/dts/qcom/Makefile > > @@ -15,6 +15,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-longcheer-l8910.dtb > > dtb-$(CONFIG_ARCH_QCOM) += msm8916-mtp.dtb > > dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-a3u-eur.dtb > > dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-a5u-eur.dtb > > +dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-j5.dtb > > dtb-$(CONFIG_ARCH_QCOM) += msm8916-samsung-serranove.dtb > > dtb-$(CONFIG_ARCH_QCOM) += msm8916-wingtech-wt88047.dtb > > dtb-$(CONFIG_ARCH_QCOM) += msm8992-bullhead-rev-101.dtb > > diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts > > new file mode 100644 > > index 000000000000..687bea438a57 > > --- /dev/null > > +++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-j5.dts > > @@ -0,0 +1,209 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > If you authored this, could we please get it under BSD license? > I'm afraid the same problem applies to all MSM8916-related device trees: https://lore.kernel.org/linux-arm-msm/YMIznk4scPv1qOzP@xxxxxxxxxxx/ Given the similarities between the devices it's easiest to take portions from existing device trees and just change some properties. But this means that many people were involved and I'm not sure if it is appropriate to apply a different license without asking all of them. It's an unfortunate situation that will likely also apply to more MSM8916 device trees submitted in the future. I hope it's still acceptable even with the GPL-2.0-only license. :) > > + > > +/dts-v1/; > > + > > +#include "msm8916-pm8916.dtsi" > > +#include <dt-bindings/gpio/gpio.h> > > + > > +/ { > > + model = "Samsung Galaxy J5 (2015)"; > > + compatible = "samsung,j5", "qcom,msm8916"; > > + chassis-type = "handset"; > > + > > + aliases { > > + serial0 = &blsp1_uart2; > > + }; > > + > > + chosen { > > + stdout-path = "serial0"; > > + }; > > + > > + reserved-memory { > > + /* Additional memory used by Samsung firmware modifications */ > > + tz-apps@85500000 { > > + reg = <0x0 0x85500000 0x0 0xb00000>; > > + no-map; > > + }; > > + }; > > + > > + gpio-keys { > > + compatible = "gpio-keys"; > > + > > + pinctrl-names = "default"; > > + pinctrl-0 = <&gpio_keys_default>; > > + > > + label = "GPIO Buttons"; > > + > > + volume-up { > > + label = "Volume Up"; > > + gpios = <&msmgpio 107 GPIO_ACTIVE_LOW>; > > + linux,code = <KEY_VOLUMEUP>; > > + }; > > + > > + home-key { > > + lable = "Home Key"; > > + gpios = <&msmgpio 109 GPIO_ACTIVE_LOW>; > > + linux,code = <KEY_HOMEPAGE>; > > + }; > > + }; > > +}; > > + > > +&blsp1_uart2 { > > Can you please sort these nodes alphabetically? > It looks mostly alphabetically ordered to me, could you clarify which nodes you are referring to exactly? The exceptions are &smd_rpm_regulators and &msmgpio, which are explicitly at the end of the file (consistent with all other MSM8916 device trees). I think it's easier to focus on the main interesting part of the device tree that way (the device definitions). If you prefer to have strict alphebtical order I can prepare a bulk patch that changes the order in all the existing MSM8916 device trees. The most important thing for me is that they are consistent. Thanks, Stephan > > > + status = "okay"; > > +}; > > + > > +&pm8916_resin { > > + status = "okay"; > > + linux,code = <KEY_VOLUMEDOWN>; > > +}; > > + > > +/* FIXME: Replace with SM5703 MUIC when driver is available */ > > +&pm8916_usbin { > > + status = "okay"; > > +}; > > + > > +&pronto { > > + status = "okay"; > > +}; > > + > > +&sdhc_1 { > > + status = "okay"; > > + > > + pinctrl-names = "default", "sleep"; > > + pinctrl-0 = <&sdc1_clk_on &sdc1_cmd_on &sdc1_data_on>; > > + pinctrl-1 = <&sdc1_clk_off &sdc1_cmd_off &sdc1_data_off>; > > +}; > > + > > +&sdhc_2 { > > + status = "okay"; > > + > > + pinctrl-names = "default", "sleep"; > > + pinctrl-0 = <&sdc2_clk_on &sdc2_cmd_on &sdc2_data_on &sdc2_cd_on>; > > + pinctrl-1 = <&sdc2_clk_off &sdc2_cmd_off &sdc2_data_off &sdc2_cd_off>; > > + > > + cd-gpios = <&msmgpio 38 GPIO_ACTIVE_LOW>; > > +}; > > + > > +&usb { > > + status = "okay"; > > + dr_mode = "peripheral"; > > + extcon = <&pm8916_usbin>; > > +}; > > + > > +&usb_hs_phy { > > + extcon = <&pm8916_usbin>; > > + qcom,init-seq = /bits/ 8 <0x1 0x19 0x2 0x0b>; > > +}; > > + > > +&smd_rpm_regulators { > > + 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-allow-set-load; > > + regulator-system-load = <200000>; > > + }; > > + > > + 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 = <3000000>; > > + regulator-max-microvolt = <3000000>; > > + }; > > + > > + l18 { > > + regulator-min-microvolt = <2700000>; > > + regulator-max-microvolt = <2700000>; > > + }; > > +}; > > + > > +&msmgpio { > > + gpio_keys_default: gpio-keys-default { > > + pins = "gpio107", "gpio109"; > > + function = "gpio"; > > + > > + drive-strength = <2>; > > + bias-pull-up; > > + }; > > +}; > > -- > > 2.33.1 > >
