| Message ID | 20220910143213.477261-1-iskren.chernev@gmail.com |
|---|---|
| Headers | show |
| Series | Add support for sm6115,4250 and OnePlus Nord N100 | expand |
On 10/09/2022 16:32, Iskren Chernev wrote: > Add support for Qualcomm SM6115 SoC. This includes: > - GCC > - Pinctrl > - RPM (CC+PD) > - USB > - MMC > - UFS > > Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com> > --- > pending issues with dtschema: > - for some reason, using pinctrl phandles (in mmc) breaks the pinctrl > schema (4 times) > .output/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dtb: pinctrl@500000: sdc1-on-state: 'oneOf' conditional failed, one must be fixed: > 'pins' is a required property > 'clk', 'cmd', 'data', 'rclk' do not match any of the regexes: 'pinctrl-[0-9]+' > [[26]] is not of type 'object' > From schema: /home/iskren/src/pmos/linux-postmarketos/Documentation/devicetree/bindings/pinctrl/qcom,sm6115-pinctrl.yaml It's the same as 06367559766b7c9bd96d2baef8bfc5a9bb451e25. I propose to fix it the same way. I can do a biger change for all pinctrls, so here you would need to add "-pins" prefix to entries (see patch 4fcdaf4b0320f93d0ccb4d36b795ed258fb07b27). Best regards, Krzysztof
On 11/09/2022 11:09, Iskren Chernev wrote: > > > On 9/11/22 11:40, Krzysztof Kozlowski wrote: >> On 10/09/2022 16:32, Iskren Chernev wrote: >>> Add support for Qualcomm SM6115 SoC. This includes: >>> - GCC >>> - Pinctrl >>> - RPM (CC+PD) >>> - USB >>> - MMC >>> - UFS >>> >>> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com> >>> --- >>> pending issues with dtschema: >>> - for some reason, using pinctrl phandles (in mmc) breaks the pinctrl >>> schema (4 times) >>> .output/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dtb: pinctrl@500000: sdc1-on-state: 'oneOf' conditional failed, one must be fixed: >>> 'pins' is a required property >>> 'clk', 'cmd', 'data', 'rclk' do not match any of the regexes: 'pinctrl-[0-9]+' >>> [[26]] is not of type 'object' >>> From schema: /home/iskren/src/pmos/linux-postmarketos/Documentation/devicetree/bindings/pinctrl/qcom,sm6115-pinctrl.yaml >> >> It's the same as 06367559766b7c9bd96d2baef8bfc5a9bb451e25. I propose to >> fix it the same way. I can do a biger change for all pinctrls, so here >> you would need to add "-pins" prefix to entries (see patch >> 4fcdaf4b0320f93d0ccb4d36b795ed258fb07b27). > > OK, that makes sense. One thing that is a bit odd -- the current pattern > "(pinconf|-pins)$" matches anything that ends in pinconf OR -pins (so it could > be sth-pinconf). Yeah, I am fixing it to ^(pinconf|.*-pins)$ > Also, if you only have a single block, isn't the idea to just > list it in the -states node. I mean we either force everybody to nest with > a pinconf, or we allow -pins for nested stuff and directly in -state for the > non-nested. Just my 2c. I didn't get this one... We allow exactly this, don't we (in PMIC GPIOs)? Best regards, Krzysztof
On 11/09/2022 12:22, Iskren Chernev wrote: > basic-state { // this matches the first state in oneOf > pins: "gpio1"; > funciton: "normal"; > }; > > nested-state { > some-pins { // this matches the second state in oneOf > pins: "gpio1"; > funciton: "normal"; > }; > other-pins { > pins: "gpio2" > funciton: "normal"; > }; > } > > // but also, matching second state in oneOf > nested-basic-state { > pinconf { > pins: "gpio1"; > funciton: "normal"; > }; > }; > }; > > So I'm saying, we should either choose basic-state and nested-state, in which > case we don't need the "^pinconf$" variant, or we can have nested-state and > nested-basic-state, in which case we don't need the 1st case of the oneOf. Ah, I get it. > > Otherwise people have to choose between basic-state and nested-basic-state, > which are equivalent in semantics. Yeah, I can drop pinconf. I put it in the PMIC because it was used, but I don't find it for TLMM pinctrl nodes. > > On a tangent -- why specifying the .* regex of pinctrl subnodes has effect on > pinctrl references in other nodes. I.e I don't understand why this fix fixes > the issue (but it does). Because it works on DTB and finds linux,phandle. This might be some bug in dtschema, but anyway better to have a bit stricter patterns in bindings. Best regards, Krzysztof
On Sat, Sep 10, 2022 at 05:32:13PM +0300, Iskren Chernev wrote: > Add initial support for OnePlus Nord N100, based on SM4250. Currently > working: > - boots > - usb > - buildin flash storage (UFS) > - SD card reader > > Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com> > --- > arch/arm64/boot/dts/qcom/Makefile | 1 + > .../boot/dts/qcom/sm4250-oneplus-billie2.dts | 241 ++++++++++++++++++ > 2 files changed, 242 insertions(+) > create mode 100644 arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts > > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile > index f4126f7e7640..5d2570b600e0 100644 > --- a/arch/arm64/boot/dts/qcom/Makefile > +++ b/arch/arm64/boot/dts/qcom/Makefile > @@ -137,6 +137,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-xiaomi-polaris.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm845-shift-axolotl.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm850-lenovo-yoga-c630.dtb > dtb-$(CONFIG_ARCH_QCOM) += sdm850-samsung-w737.dtb > +dtb-$(CONFIG_ARCH_QCOM) += sm4250-oneplus-billie2.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm6125-sony-xperia-seine-pdx201.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm6350-sony-xperia-lena-pdx213.dtb > dtb-$(CONFIG_ARCH_QCOM) += sm7225-fairphone-fp4.dtb > diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts > new file mode 100644 > index 000000000000..b9094f1efca0 > --- /dev/null > +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts > @@ -0,0 +1,241 @@ > +// SPDX-License-Identifier: GPL-2.0-only Would it be possible for you to dual license this? > +/* > + * Copyright (c) 2021, Iskren Chernev <iskren.chernev@gmail.com> > + */ > + > +/dts-v1/; > + > +#include "sm4250.dtsi" > + > +/ { > + model = "OnePlus Nord N100"; > + compatible = "oneplus,billie2", "qcom,sm4250"; > + > + /* required for bootloader to select correct board */ > + qcom,msm-id = <0x1a1 0x10000 0x1bc 0x10000>; > + qcom,board-id = <0x1000b 0x00>; > + > + aliases { > + }; > + > + chosen { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + stdout-path = "framebuffer0"; > + > + framebuffer0: framebuffer@9d400000 { > + compatible = "simple-framebuffer"; > + reg = <0 0x5c000000 0 (1600 * 720 * 4)>; > + width = <720>; > + height = <1600>; > + stride = <(720 * 4)>; > + format = "a8r8g8b8"; > + }; > + }; > +}; > + > +&xo_board { > + clock-frequency = <19200000>; > +}; > + > +&sleep_clk { > + clock-frequency = <32764>; > +}; > + > +&reserved_memory { As the number of nodes grow it would be nice if these were sorted alphabetically. > + bootloader_log_mem: memory@5fff7000 { > + reg = <0x00 0x5fff7000 0x00 0x8000>; > + no-map; > + }; > + > + ramoops@cbe00000 { > + compatible = "ramoops"; > + reg = <0x0 0xcbe00000 0x0 0x400000>; > + record-size = <0x40000>; > + pmsg-size = <0x200000>; > + console-size = <0x40000>; > + ftrace-size = <0x40000>; > + }; > + > + param_mem: memory@cc200000 { > + reg = <0x00 0xcc200000 0x00 0x100000>; > + no-map; > + }; > + > + mtp_mem: memory@cc300000 { > + reg = <0x00 0xcc300000 0x00 0xb00000>; > + no-map; > + }; > +}; > + > +&usb3 { > + status = "okay"; > +}; [..] > +&rpm_requests { > + regulators-0 { Is there a reason why you don't call this node pm6125-regulators ? > + compatible = "qcom,rpm-pm6125-regulators"; > + > + vreg_s6a: s6 { > + regulator-min-microvolt = <320000>; > + regulator-max-microvolt = <1456000>; > + }; [..] > + vreg_l24a: l24 { > + regulator-min-microvolt = <2704000>; > + regulator-max-microvolt = <3544000>; > + }; Just as a heads up, by not ensuring that your regulators are in high-power-mode you risk seeing brown-outs - something we keep running into for e.g. SD-cards. Regards, Bjorn > + }; > +}; > -- > 2.37.2 >
On Sat, Sep 10, 2022 at 05:32:11PM +0300, Iskren Chernev wrote: [..] > diff --git a/arch/arm64/boot/dts/qcom/sm6115.dtsi b/arch/arm64/boot/dts/qcom/sm6115.dtsi [..] > + > + smem { > + compatible = "qcom,smem"; Please move the compatible, qcom,rpm-msg-ram and hwlocks into the &smem_mem node. > + memory-region = <&smem_mem>; > + qcom,rpm-msg-ram = <&rpm_msg_ram>; > + hwlocks = <&tcsr_mutex 3>; > + }; > + > + soc: soc { I expect that you should be told that you're missing a @0 on your soc. > + compatible = "simple-bus"; > + #address-cells = <1>; > + #size-cells = <1>; > + ranges = <0 0 0 0xffffffff>; > + > + tlmm: pinctrl@500000 { Please sort your nodes based on address, followed by node name alphabetically, followed by label. > + compatible = "qcom,sm6115-tlmm"; > + reg = <0x500000 0x400000>, <0x900000 0x400000>, <0xd00000 0x400000>; Please pad your address to 8 digits, to make it faster to see if the sort order makes sense. > + reg-names = "west", "south", "east"; > + interrupts = <GIC_SPI 227 IRQ_TYPE_LEVEL_HIGH>; > + gpio-controller; > + gpio-ranges = <&tlmm 0 0 121>; > + #gpio-cells = <2>; > + interrupt-controller; > + #interrupt-cells = <2>; > + [..] > + }; > + > + timer { > + compatible = "arm,armv8-timer"; > + interrupts = <GIC_PPI 1 0xf08>, > + <GIC_PPI 2 0xf08>, > + <GIC_PPI 3 0xf08>, > + <GIC_PPI 0 0xf08>; Please use (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW) for your flags. Regards, Bjorn
On Sat, 10 Sept 2022 at 16:32, Iskren Chernev <iskren.chernev@gmail.com> wrote: > > Most mmc blocks contain two pinctrls, default and sleep. But then > dt-schema complains about pinctrl-1 not being defined. > > Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com> Applied for next, thanks! Kind regards Uffe > --- > Documentation/devicetree/bindings/mmc/sdhci-msm.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml > index a792fa5574a0..775476d7f9f0 100644 > --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml > +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml > @@ -97,6 +97,10 @@ properties: > description: > Should specify pin control groups used for this controller. > > + pinctrl-1: > + description: > + Should specify sleep pin control groups used for this controller. > + > resets: > maxItems: 1 > > -- > 2.37.2 >
On 13/09/2022 22:30, Bjorn Andersson wrote: > On Sat, Sep 10, 2022 at 05:32:13PM +0300, Iskren Chernev wrote: >> Add initial support for OnePlus Nord N100, based on SM4250. Currently >> working: >> - boots >> - usb >> - buildin flash storage (UFS) >> - SD card reader >> >> Signed-off-by: Iskren Chernev <iskren.chernev@gmail.com> >> --- >> arch/arm64/boot/dts/qcom/Makefile | 1 + >> .../boot/dts/qcom/sm4250-oneplus-billie2.dts | 241 ++++++++++++++++++ >> 2 files changed, 242 insertions(+) >> create mode 100644 arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts >> >> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile >> index f4126f7e7640..5d2570b600e0 100644 >> --- a/arch/arm64/boot/dts/qcom/Makefile >> +++ b/arch/arm64/boot/dts/qcom/Makefile >> @@ -137,6 +137,7 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-xiaomi-polaris.dtb >> dtb-$(CONFIG_ARCH_QCOM) += sdm845-shift-axolotl.dtb >> dtb-$(CONFIG_ARCH_QCOM) += sdm850-lenovo-yoga-c630.dtb >> dtb-$(CONFIG_ARCH_QCOM) += sdm850-samsung-w737.dtb >> +dtb-$(CONFIG_ARCH_QCOM) += sm4250-oneplus-billie2.dtb >> dtb-$(CONFIG_ARCH_QCOM) += sm6125-sony-xperia-seine-pdx201.dtb >> dtb-$(CONFIG_ARCH_QCOM) += sm6350-sony-xperia-lena-pdx213.dtb >> dtb-$(CONFIG_ARCH_QCOM) += sm7225-fairphone-fp4.dtb >> diff --git a/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts >> new file mode 100644 >> index 000000000000..b9094f1efca0 >> --- /dev/null >> +++ b/arch/arm64/boot/dts/qcom/sm4250-oneplus-billie2.dts >> @@ -0,0 +1,241 @@ >> +// SPDX-License-Identifier: GPL-2.0-only > > Would it be possible for you to dual license this? > >> +/* >> + * Copyright (c) 2021, Iskren Chernev <iskren.chernev@gmail.com> >> + */ >> + >> +/dts-v1/; >> + >> +#include "sm4250.dtsi" >> + >> +/ { >> + model = "OnePlus Nord N100"; >> + compatible = "oneplus,billie2", "qcom,sm4250"; >> + >> + /* required for bootloader to select correct board */ >> + qcom,msm-id = <0x1a1 0x10000 0x1bc 0x10000>; >> + qcom,board-id = <0x1000b 0x00>; >> + >> + aliases { >> + }; >> + >> + chosen { >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + stdout-path = "framebuffer0"; >> + >> + framebuffer0: framebuffer@9d400000 { >> + compatible = "simple-framebuffer"; >> + reg = <0 0x5c000000 0 (1600 * 720 * 4)>; >> + width = <720>; >> + height = <1600>; >> + stride = <(720 * 4)>; >> + format = "a8r8g8b8"; >> + }; >> + }; >> +}; >> + >> +&xo_board { >> + clock-frequency = <19200000>; >> +}; >> + >> +&sleep_clk { >> + clock-frequency = <32764>; >> +}; >> + >> +&reserved_memory { > > As the number of nodes grow it would be nice if these were sorted > alphabetically. > >> + bootloader_log_mem: memory@5fff7000 { >> + reg = <0x00 0x5fff7000 0x00 0x8000>; >> + no-map; >> + }; >> + >> + ramoops@cbe00000 { >> + compatible = "ramoops"; >> + reg = <0x0 0xcbe00000 0x0 0x400000>; >> + record-size = <0x40000>; >> + pmsg-size = <0x200000>; >> + console-size = <0x40000>; >> + ftrace-size = <0x40000>; >> + }; >> + >> + param_mem: memory@cc200000 { >> + reg = <0x00 0xcc200000 0x00 0x100000>; >> + no-map; >> + }; >> + >> + mtp_mem: memory@cc300000 { >> + reg = <0x00 0xcc300000 0x00 0xb00000>; >> + no-map; >> + }; >> +}; >> + >> +&usb3 { >> + status = "okay"; >> +}; > [..] >> +&rpm_requests { >> + regulators-0 { > > Is there a reason why you don't call this node pm6125-regulators ? It's the follow up of: https://lore.kernel.org/all/20220901093243.134288-1-krzysztof.kozlowski@linaro.org/ Node names should be rather generic, so just regulators. Best regards, Krzysztof