Message-ID: <56d061ea-1458-43b5-a823-eb625204913a@oss.qualcomm.com>
Date: Sat, 26 Oct 2024 13:14:02 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: "Aiqun Yu (Maria)" <quic_aiquny@...cinc.com>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Krzysztof Kozlowski
 <krzk@...nel.org>,
        Lijuan Gao <quic_lijuang@...cinc.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>, kernel@...cinc.com,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH 6/6] arm64: dts: qcom: add base QCS615 RIDE dts

On 5.09.2024 7:29 AM, Aiqun Yu (Maria) wrote:
> 
> 
> On 9/4/2024 6:23 PM, Konrad Dybcio wrote:
>> On 4.09.2024 11:32 AM, Krzysztof Kozlowski wrote:
>>> On 04/09/2024 10:35, Lijuan Gao wrote:
>>>>
>>>>
>>>> 在 8/28/2024 5:34 PM, Krzysztof Kozlowski 写道:
>>>>> On 28/08/2024 11:31, Lijuan Gao wrote:
>>>>>>>>>> +/ {
>>>>>>>>>> +	model = "Qualcomm Technologies, Inc. QCS615 Ride";
>>>>>>>>>> +	compatible = "qcom,qcs615-ride", "qcom,qcs615";
>>>>>>>>>> +
>>>>>>>>>> +	chosen {
>>>>>>>>>> +		bootargs = "console=hvc0";
>>>>>>>>>
>>>>>>>>> Noooo, last time I agreed on this, you told me later it is different.
>>>>>>>>>
>>>>>>>> In the early stages, enabling HVC is to more easily verify clock and
>>>>>>>> PMIC related functions, as it’s difficult to debug without the console
>>>>>>>> log. After the clock and PMIC are ready, we will enable the UART console.
>>>>>>>
>>>>>>> Working serial is supposed to be part of the early submission.
>>>>>>>
>>>>>> Okay, I will remove it in the next patch.
>>>>>
>>>>> Can you post next version with proper serial device?
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Can we use the dts without console enabled as the first version? When 
>>>> the clock is ready, we will submit new changes to enable the UART console.
>>>
>>> It is very surprising not to have console available in the first, early
>>> submission, but it is not a blocker for me.
>>
>> Lijuan,
>>
>> I see that the initial submission is very slim. GCC+UART+TLMM is
>> usually the smallest we tend to accept.
> 
> We are exploring various ways to improve the efficiency of the upstream
> change merge process. In the current QCS615 project, we are
> experimenting with a slim "HVC console" verified base device tree to
> minimize dependencies and enhance parallel work efficiency.
> 
> Currently, different developers are working on the same QCS615 project.
> One developer is focusing on clock support for QCS615, another is
> working on interconnect support, and a third is handling TLMM pinctrl
> support. Additionally, the QUP UART validation depends on above soc
> specific GCC clock/TLMM support.
> 
> Here is the proposed process chart for reference, Clock/TLMM, even other
> functionality like LLCC can be validated apart from current Base support
> with HVC console enabled:
>                                +---------------+
> 
>                                | Clock         |
> 
>                                |               |
> 
>                                +---------------+
> 
> +---------------------+
> 
> |    Base support:    |        +---------------+       +-----+
> 
> | HVC console enabled |------> | TLMM          | ----->| UART|
> 
> +---------------------+        +---------------+       +-----+
> 
> 
> 
>                                +---------------+
> 
>                                | Interconnect  |
> 
>                                +---------------+
> 
> 
> It is suggested to have process like this:
> 1. Have hvc console enabled base device tree support.
> 2. TLMM/GCC/Interconnect/LLCC/etc drivers can be pushed along with the
> needful dt changes.
> 3. QUP uart support change after TLMM/GCC dependency uploaded.
> 
> Here is an original example of qcs8300 project that the base device tree
> wait until have all qup uart enabled support for reference:
> 1. The first soc support[1] pushed at 08/14.
> 2. TLMM support[2] pushed at 08/19.
> 3. GCC clock support[3] pushed at 08/20.
> 4. Interconnect support[4] pushed at 08/27.
> 5. LLCC support[5] pushed at 09/03.
> 6. Initial device tree support[6] pushed at 09/04. And it have 5
> co-developer in the initial device tree support.

Right, plumbing up all of the UART dependencies properly on these
platforms is very much not straightforward.

> 
> 
> [1]https://lore.kernel.org/all/20240814072806.4107079-1-quic_jingyw@quicinc.com/
> [2]https://lore.kernel.org/all/20240819064933.1778204-1-quic_jingyw@quicinc.com/
> [3]https://lore.kernel.org/all/20240820-qcs8300-gcc-v1-0-d81720517a82@quicinc.com/
> [4]https://lore.kernel.org/all/20240827151622.305-1-quic_rlaggysh@quicinc.com/
> [5]https://lore.kernel.org/all/20240903-qcs8300_llcc_driver-v1-0-228659bdf067@quicinc.com/
>>
>> While hooking up these drivers may take some time, please consider
>> at least describing a subset of the clocks and the QUP UART, as
>> everything non-SoC-specific is already in place.
> 
> To be more specific, are you suggesting like adding the base device tree
> describing with current nodes subset which only have non-soc-specific
> info, like:
> 1. "apps_rsc" nodes without info of
> "qcom,qcs615-rpmh-clk","qcom,qcs615-gcc"?
> 2."qcom,geni-debug-uart" nodes description without the clock properties?

When writing that email, I thought about sending a small version of the
GCC driver with just enough to turn on the UART clocks, but thinking
about it again, that sounds like a really bad idea..

I think it's best if we stick to what you did in this patch, and validate
things with `earlycon=qcom_geni,0xaddress` in cmdline. This way, we can
take advantage of the bootloader setting it up for us.

You could then include the base address of the serial engine in the
commit message to let others reproduce it easily.

Krzysztof, would you agree ^?

Konrad

Powered by blists - more mailing lists