| Message ID | 20221001030656.29365-4-quic_molvera@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add base device tree files for QDU1000/QRU1000 | expand |
On Sat, 1 Oct 2022 at 06:09, Melody Olvera <quic_molvera@quicinc.com> wrote: > > Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin > configuration. > > Signed-off-by: Melody Olvera <quic_molvera@quicinc.com> > --- > arch/arm64/boot/dts/qcom/qdru1000.dtsi | 30 ++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qdru1000.dtsi b/arch/arm64/boot/dts/qcom/qdru1000.dtsi > index 3610f94bef35..39b9a00d3ad8 100644 > --- a/arch/arm64/boot/dts/qcom/qdru1000.dtsi > +++ b/arch/arm64/boot/dts/qcom/qdru1000.dtsi > @@ -235,6 +235,8 @@ uart7: serial@99c000 { > reg = <0x0 0x99c000 0x0 0x4000>; > clock-names = "se"; > clocks = <&gcc GCC_QUPV3_WRAP0_S7_CLK>; > + pinctrl-names = "default"; > + pinctrl-0 = <&qup_uart7_default>; > interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>; > #address-cells = <1>; > #size-cells = <0>; > @@ -248,6 +250,34 @@ tcsr_mutex: hwlock@1f40000 { > #hwlock-cells = <1>; > }; > > + tlmm: pinctrl@f000000 { > + compatible = "qcom,qdu1000-tlmm", "qcom,qru1000-tlmm"; > + reg = <0x0 0xf000000 0x0 0x1000000>; > + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-controller; > + #interrupt-cells = <2>; > + gpio-ranges = <&tlmm 0 0 151>; > + wakeup-parent = <&pdc>; > + > + qup_uart7_default: qup-uart7-default { > + tx { > + pins = "gpio134"; > + function = "qup0_se7_l2"; This looks strange. Usually we'd have a single 'qup7' function here. I'd go back to the interconnect driver. Maybe the functions are not correctly defined there. > + drive-strength = <2>; > + bias-disable; 'drive-strength' and 'bias-disable' are to be patched in in the board dts file. > + }; > + > + rx { > + pins = "gpio135"; > + function = "qup0_se7_l3"; > + drive-strength = <2>; > + bias-disable; > + }; > + }; > + }; > + > pdc: interrupt-controller@b220000 { > compatible = "qcom,pdc"; > reg = <0x0 0xb220000 0x0 0x30000>, <0x0 0x174000f0 0x0 0x64>; > -- > 2.37.3 >
On 01/10/2022 05:06, Melody Olvera wrote: > Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin > configuration. The patchset should be squashed with previous. There is no point in bringing support piece by piece. You can bring support in steps if you submissions are separate in time. But if you have everything ready - your patch must be complete and bisectable. > > Signed-off-by: Melody Olvera <quic_molvera@quicinc.com> > --- > arch/arm64/boot/dts/qcom/qdru1000.dtsi | 30 ++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/qdru1000.dtsi b/arch/arm64/boot/dts/qcom/qdru1000.dtsi > index 3610f94bef35..39b9a00d3ad8 100644 > --- a/arch/arm64/boot/dts/qcom/qdru1000.dtsi > +++ b/arch/arm64/boot/dts/qcom/qdru1000.dtsi > @@ -235,6 +235,8 @@ uart7: serial@99c000 { > reg = <0x0 0x99c000 0x0 0x4000>; > clock-names = "se"; > clocks = <&gcc GCC_QUPV3_WRAP0_S7_CLK>; > + pinctrl-names = "default"; > + pinctrl-0 = <&qup_uart7_default>; > interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>; > #address-cells = <1>; > #size-cells = <0>; > @@ -248,6 +250,34 @@ tcsr_mutex: hwlock@1f40000 { > #hwlock-cells = <1>; > }; > > + tlmm: pinctrl@f000000 { > + compatible = "qcom,qdu1000-tlmm", "qcom,qru1000-tlmm"; > + reg = <0x0 0xf000000 0x0 0x1000000>; > + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-controller; > + #interrupt-cells = <2>; > + gpio-ranges = <&tlmm 0 0 151>; > + wakeup-parent = <&pdc>; > + > + qup_uart7_default: qup-uart7-default { Suffix "-state" > + tx { Suffix "-pins" > + pins = "gpio134"; > + function = "qup0_se7_l2"; > + drive-strength = <2>; > + bias-disable; > + }; > + > + rx { Suffix "-pins" > + pins = "gpio135"; > + function = "qup0_se7_l3"; > + drive-strength = <2>; > + bias-disable; > + }; > + }; > + }; > + > pdc: interrupt-controller@b220000 { > compatible = "qcom,pdc"; > reg = <0x0 0xb220000 0x0 0x30000>, <0x0 0x174000f0 0x0 0x64>; Best regards, Krzysztof
On 10/1/2022 12:26 AM, Dmitry Baryshkov wrote: > On Sat, 1 Oct 2022 at 06:09, Melody Olvera <quic_molvera@quicinc.com> wrote: >> Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin >> configuration. >> >> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/qdru1000.dtsi | 30 ++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/qdru1000.dtsi b/arch/arm64/boot/dts/qcom/qdru1000.dtsi >> index 3610f94bef35..39b9a00d3ad8 100644 >> --- a/arch/arm64/boot/dts/qcom/qdru1000.dtsi >> +++ b/arch/arm64/boot/dts/qcom/qdru1000.dtsi >> @@ -235,6 +235,8 @@ uart7: serial@99c000 { >> reg = <0x0 0x99c000 0x0 0x4000>; >> clock-names = "se"; >> clocks = <&gcc GCC_QUPV3_WRAP0_S7_CLK>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&qup_uart7_default>; >> interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>; >> #address-cells = <1>; >> #size-cells = <0>; >> @@ -248,6 +250,34 @@ tcsr_mutex: hwlock@1f40000 { >> #hwlock-cells = <1>; >> }; >> >> + tlmm: pinctrl@f000000 { >> + compatible = "qcom,qdu1000-tlmm", "qcom,qru1000-tlmm"; >> + reg = <0x0 0xf000000 0x0 0x1000000>; >> + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>; >> + gpio-controller; >> + #gpio-cells = <2>; >> + interrupt-controller; >> + #interrupt-cells = <2>; >> + gpio-ranges = <&tlmm 0 0 151>; >> + wakeup-parent = <&pdc>; >> + >> + qup_uart7_default: qup-uart7-default { >> + tx { >> + pins = "gpio134"; >> + function = "qup0_se7_l2"; > This looks strange. Usually we'd have a single 'qup7' function here. > I'd go back to the interconnect driver. Maybe the functions are not > correctly defined there. Yeah; will correct. Pinctrl driver was not in line with upstream standards. > >> + drive-strength = <2>; >> + bias-disable; > 'drive-strength' and 'bias-disable' are to be patched in in the board dts file. Really? Looking at sm8450.dtsi and sm8350.dtsi I see them defined in the dtsi file instead of the dts file. Is this new? > >> + }; >> + >> + rx { >> + pins = "gpio135"; >> + function = "qup0_se7_l3"; >> + drive-strength = <2>; >> + bias-disable; >> + }; >> + }; >> + }; >> + >> pdc: interrupt-controller@b220000 { >> compatible = "qcom,pdc"; >> reg = <0x0 0xb220000 0x0 0x30000>, <0x0 0x174000f0 0x0 0x64>; >> -- >> 2.37.3 >> > Thanks, Melody
On 10/1/2022 2:14 AM, Krzysztof Kozlowski wrote: > On 01/10/2022 05:06, Melody Olvera wrote: >> Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin >> configuration. > The patchset should be squashed with previous. There is no point in > bringing support piece by piece. You can bring support in steps if you > submissions are separate in time. But if you have everything ready - > your patch must be complete and bisectable. To be clear, does it make more sense to submit the base dt first, then submit each driver with all the dt changes as one patchset? > >> Signed-off-by: Melody Olvera <quic_molvera@quicinc.com> >> --- >> arch/arm64/boot/dts/qcom/qdru1000.dtsi | 30 ++++++++++++++++++++++++++ >> 1 file changed, 30 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/qdru1000.dtsi b/arch/arm64/boot/dts/qcom/qdru1000.dtsi >> index 3610f94bef35..39b9a00d3ad8 100644 >> --- a/arch/arm64/boot/dts/qcom/qdru1000.dtsi >> +++ b/arch/arm64/boot/dts/qcom/qdru1000.dtsi >> @@ -235,6 +235,8 @@ uart7: serial@99c000 { >> reg = <0x0 0x99c000 0x0 0x4000>; >> clock-names = "se"; >> clocks = <&gcc GCC_QUPV3_WRAP0_S7_CLK>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&qup_uart7_default>; >> interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>; >> #address-cells = <1>; >> #size-cells = <0>; >> @@ -248,6 +250,34 @@ tcsr_mutex: hwlock@1f40000 { >> #hwlock-cells = <1>; >> }; >> >> + tlmm: pinctrl@f000000 { >> + compatible = "qcom,qdu1000-tlmm", "qcom,qru1000-tlmm"; >> + reg = <0x0 0xf000000 0x0 0x1000000>; >> + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>; >> + gpio-controller; >> + #gpio-cells = <2>; >> + interrupt-controller; >> + #interrupt-cells = <2>; >> + gpio-ranges = <&tlmm 0 0 151>; >> + wakeup-parent = <&pdc>; >> + >> + qup_uart7_default: qup-uart7-default { > Suffix "-state" Ack. > >> + tx { > Suffix "-pins" Ack. > >> + pins = "gpio134"; >> + function = "qup0_se7_l2"; >> + drive-strength = <2>; >> + bias-disable; >> + }; >> + >> + rx { > Suffix "-pins" Ack. > > >> + pins = "gpio135"; >> + function = "qup0_se7_l3"; >> + drive-strength = <2>; >> + bias-disable; >> + }; >> + }; >> + }; >> + >> pdc: interrupt-controller@b220000 { >> compatible = "qcom,pdc"; >> reg = <0x0 0xb220000 0x0 0x30000>, <0x0 0x174000f0 0x0 0x64>; > Best regards, > Krzysztof > Thanks, Melody
On 11/10/2022 14:48, Melody Olvera wrote: > > > On 10/1/2022 2:14 AM, Krzysztof Kozlowski wrote: >> On 01/10/2022 05:06, Melody Olvera wrote: >>> Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin >>> configuration. >> The patchset should be squashed with previous. There is no point in >> bringing support piece by piece. You can bring support in steps if you >> submissions are separate in time. But if you have everything ready - >> your patch must be complete and bisectable. > To be clear, does it make more sense to submit the base dt first, then submit each > driver with all the dt changes as one patchset? No, because you have DTS ready. There is no incremental work here. Best regards, Krzysztof
On 10/11/2022 11:57 AM, Krzysztof Kozlowski wrote: > On 11/10/2022 14:48, Melody Olvera wrote: >> >> On 10/1/2022 2:14 AM, Krzysztof Kozlowski wrote: >>> On 01/10/2022 05:06, Melody Olvera wrote: >>>> Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin >>>> configuration. >>> The patchset should be squashed with previous. There is no point in >>> bringing support piece by piece. You can bring support in steps if you >>> submissions are separate in time. But if you have everything ready - >>> your patch must be complete and bisectable. >> To be clear, does it make more sense to submit the base dt first, then submit each >> driver with all the dt changes as one patchset? > No, because you have DTS ready. There is no incremental work here. Ah ok so just squash all these commits into one and submit. > > Best regards, > Krzysztof > Thanks, Melody
On 11/10/2022 15:05, Melody Olvera wrote: > > > On 10/11/2022 11:57 AM, Krzysztof Kozlowski wrote: >> On 11/10/2022 14:48, Melody Olvera wrote: >>> >>> On 10/1/2022 2:14 AM, Krzysztof Kozlowski wrote: >>>> On 01/10/2022 05:06, Melody Olvera wrote: >>>>> Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin >>>>> configuration. >>>> The patchset should be squashed with previous. There is no point in >>>> bringing support piece by piece. You can bring support in steps if you >>>> submissions are separate in time. But if you have everything ready - >>>> your patch must be complete and bisectable. >>> To be clear, does it make more sense to submit the base dt first, then submit each >>> driver with all the dt changes as one patchset? >> No, because you have DTS ready. There is no incremental work here. > Ah ok so just squash all these commits into one and submit. Except the board DTS. Other bigger, self-contained pieces of work can be also kept separate, but such work is not "add a DMA". Such work could be - add display (with clocks, DMA, GPU, power domains) or sound (again multiple separate devices added). Best regards, Krzysztof
On 10/11/2022 12:19 PM, Krzysztof Kozlowski wrote: > On 11/10/2022 15:05, Melody Olvera wrote: >> >> On 10/11/2022 11:57 AM, Krzysztof Kozlowski wrote: >>> On 11/10/2022 14:48, Melody Olvera wrote: >>>> On 10/1/2022 2:14 AM, Krzysztof Kozlowski wrote: >>>>> On 01/10/2022 05:06, Melody Olvera wrote: >>>>>> Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin >>>>>> configuration. >>>>> The patchset should be squashed with previous. There is no point in >>>>> bringing support piece by piece. You can bring support in steps if you >>>>> submissions are separate in time. But if you have everything ready - >>>>> your patch must be complete and bisectable. >>>> To be clear, does it make more sense to submit the base dt first, then submit each >>>> driver with all the dt changes as one patchset? >>> No, because you have DTS ready. There is no incremental work here. >> Ah ok so just squash all these commits into one and submit. > Except the board DTS. Other bigger, self-contained pieces of work can be > also kept separate, but such work is not "add a DMA". Such work could be > - add display (with clocks, DMA, GPU, power domains) or sound (again > multiple separate devices added). Understood. Yeah I figured leave the dts files as a separate commit, but do one big commit for the dtsi file, and one for the dts files. > > Best regards, > Krzysztof > Thanks, Melody
diff --git a/arch/arm64/boot/dts/qcom/qdru1000.dtsi b/arch/arm64/boot/dts/qcom/qdru1000.dtsi index 3610f94bef35..39b9a00d3ad8 100644 --- a/arch/arm64/boot/dts/qcom/qdru1000.dtsi +++ b/arch/arm64/boot/dts/qcom/qdru1000.dtsi @@ -235,6 +235,8 @@ uart7: serial@99c000 { reg = <0x0 0x99c000 0x0 0x4000>; clock-names = "se"; clocks = <&gcc GCC_QUPV3_WRAP0_S7_CLK>; + pinctrl-names = "default"; + pinctrl-0 = <&qup_uart7_default>; interrupts = <GIC_SPI 608 IRQ_TYPE_LEVEL_HIGH>; #address-cells = <1>; #size-cells = <0>; @@ -248,6 +250,34 @@ tcsr_mutex: hwlock@1f40000 { #hwlock-cells = <1>; }; + tlmm: pinctrl@f000000 { + compatible = "qcom,qdu1000-tlmm", "qcom,qru1000-tlmm"; + reg = <0x0 0xf000000 0x0 0x1000000>; + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>; + gpio-controller; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + gpio-ranges = <&tlmm 0 0 151>; + wakeup-parent = <&pdc>; + + qup_uart7_default: qup-uart7-default { + tx { + pins = "gpio134"; + function = "qup0_se7_l2"; + drive-strength = <2>; + bias-disable; + }; + + rx { + pins = "gpio135"; + function = "qup0_se7_l3"; + drive-strength = <2>; + bias-disable; + }; + }; + }; + pdc: interrupt-controller@b220000 { compatible = "qcom,pdc"; reg = <0x0 0xb220000 0x0 0x30000>, <0x0 0x174000f0 0x0 0x64>;
Add tlmm node for the QDU1000 and QRU1000 SoCs and the uart pin configuration. Signed-off-by: Melody Olvera <quic_molvera@quicinc.com> --- arch/arm64/boot/dts/qcom/qdru1000.dtsi | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)