On Fri 19 Nov 03:56 CST 2021, Stephan Gerhold wrote: > 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. :) > Unfortunate indeed, but now I've asked at least... > > > + > > > +/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. > You're right, I had forgotten that we put the &smd_rpm_regulators at the end on these related devices. Should probably make this consistent across all platforms, but let's merge this for now. Thanks, Bjorn > 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 > > >
