Re: [PATCH v3 3/4] arm64: dts: qcom: Add base QDU1000/QRU1000 DTSIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 11/2/2022 9:24 AM, Krzysztof Kozlowski wrote:
> On 31/10/2022 17:49, Melody Olvera wrote:
>>
>> On 10/27/2022 8:21 AM, Krzysztof Kozlowski wrote:
>>> On 26/10/2022 16:04, Melody Olvera wrote:
>>>> Add the base DTSI files for QDU1000 and QRU1000 SoCs, including base
>>>> descriptions of CPUs, GCC, RPMHCC, QUP, TLMM, and interrupt-controller
>>>> to boot to shell with console on these SoCs.
>>>>
>>>> Signed-off-by: Melody Olvera <quic_molvera@xxxxxxxxxxx>
>>>> ---
>>>>  arch/arm64/boot/dts/qcom/qdu1000.dtsi | 1406 +++++++++++++++++++++++++
>>> Please use scripts/get_maintainers.pl to get a list of necessary people
>>> and lists to CC.  It might happen, that command when run on an older
>>> kernel, gives you outdated entries.  Therefore please be sure you base
>>> your patches on recent Linux kernel.
>> Sure thing; we talked about this on a different patch.
>>>>  arch/arm64/boot/dts/qcom/qru1000.dtsi |   27 +
>>>>  2 files changed, 1433 insertions(+)
>>>>  create mode 100644 arch/arm64/boot/dts/qcom/qdu1000.dtsi
>>>>  create mode 100644 arch/arm64/boot/dts/qcom/qru1000.dtsi
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/qdu1000.dtsi b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
>>>> new file mode 100644
>>>> index 000000000000..76474106e931
>>>> --- /dev/null
>>>> +++ b/arch/arm64/boot/dts/qcom/qdu1000.dtsi
>>>> @@ -0,0 +1,1406 @@
>>>> +// SPDX-License-Identifier: BSD-3-Clause
>>>> +/*
>>>> + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved.
>>>> + */
>>> (...)
>>>
>>>> +
>>>> +	soc: soc@0 {
>>>> +		#address-cells = <2>;
>>>> +		#size-cells = <2>;
>>>> +		ranges = <0 0 0 0 0x10 0>;
>>>> +		dma-ranges = <0 0 0 0 0x10 0>;
>>>> +		compatible = "simple-bus";
>>>> +
>>>> +		gcc: clock-controller@80000 {
>>>> +			compatible = "qcom,gcc-qdu1000", "syscon";
>>>> +			reg = <0x0 0x80000 0x0 0x1f4200>;
>>>> +			#clock-cells = <1>;
>>>> +			#reset-cells = <1>;
>>>> +			#power-domain-cells = <1>;
>>>> +			clocks = <&rpmhcc RPMH_CXO_CLK>, <&sleep_clk>;
>>>> +			clock-names = "bi_tcxo", "sleep_clk";
>>>> +		};
>>>> +
>>>> +		gpi_dma0: dma-controller@900000  {
>>>> +			compatible = "qcom,sm6350-gpi-dma";
>>> You should add here a specific compatible as well. Same in other places.
>>> All places. I had impression we talked about this few times, so I don't
>>> know what is missing on your side.
>>>
>>> This must be:
>>> "qcom,qdu1000-gpi-dma", "qcom,sm6350-gpi-dma"
>> Got it. I talked to Stephan and he said either your suggestion or just using
>> preexisting compatibles would be ok. I thought it might be cleaner to not
>> have the qdu compats, but I'm fine either way.
>>>> +			#dma-cells = <3>;
>>>> +			reg = <0x0 0x900000 0x0 0x60000>;
>>>> +			interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 250 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 251 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 252 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 253 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 254 IRQ_TYPE_LEVEL_HIGH>,
>>>> +				     <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
>>>> +			dma-channels = <12>;
>>>> +			dma-channel-mask = <0x3f>;
>>>> +			iommus = <&apps_smmu 0xf6 0x0>;
>>>> +		};
>>>> +
>>> (...)
>>>
>>>
>>>> +
>>>> +		tlmm: pinctrl@f000000 {
>>>> +			compatible = "qcom,qdu1000-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_uart0_default: qup-uart0-default-state {
>>>> +				pins = "gpio6", "gpio7", "gpio8", "gpio9";
>>>> +				function = "qup00";
>>>> +			};
>>>> +
>>>> +			qup_i2c1_data_clk: qup-i2c1-data-clk-state {
>>>> +				pins = "gpio10", "gpio11";
>>>> +				function = "qup01";
>>>> +				drive-strength = <2>;
>>> Can we have some generic agreement where to put drive-strengths and bias?
>>>
>>> See also:
>>> https://lore.kernel.org/linux-devicetree/20221026200357.391635-2-krzysztof.kozlowski@xxxxxxxxxx/
>>>
>>> https://lore.kernel.org/lkml/CAD=FV=VUL4GmjaibAMhKNdpEso_Hg_R=XeMaqah1LSj_9-Ce4Q@xxxxxxxxxxxxxx/
>> Not sure how much two-sense I have for the conversation at large, but generally I agree with Doug's
>> point in the first paragraph. Pulls for this soc are consistent across boards so I don't think it makes
>> sense to move them to the board files here. I vote that these stay here.
> I would be great if Konrad and Bjorn shared their opinion on this... but
> wait, you did not Cc all maintainers... Eh.
I'm not sure why this is being brought up again; we've already discussed this here
https://lore.kernel.org/all/9707bf67-1b22-8a77-7193-fc909b4f49de@xxxxxxxxxxx/
Would you like to discuss this issue here, on the next version, or not at all?

On a side note, I'm uncomfortable with how our continued interactions are going
and do not believe this to be conductive to continued collaboration. I would ask that
we keep our correspondence polite and professional moving forward.

Thanks,
Melody




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux