Re: [PATCH 1/4] dt-bindings: mtd: qcom,nandc: Add MDM9607 QPIC NAND controller

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hello,

On 17/06/2026 at 13:42:44 +02, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:

(keeping the context for Krzysztof)

> On 6/9/26 12:02 PM, Stephan Gerhold wrote:
>> On Tue, Jun 09, 2026 at 11:30:54AM +0200, Miquel Raynal wrote:
>>> On 09/06/2026 at 11:08:03 +02, Stephan Gerhold <stephan.gerhold@linaro.org> wrote:
>>>
>>>> On Tue, Jun 09, 2026 at 11:01:18AM +0200, Konrad Dybcio wrote:
>>>>> On 6/9/26 10:55 AM, Konrad Dybcio wrote:
>>>>>> On 6/9/26 10:10 AM, Stephan Gerhold wrote:
>>>>>>> On Tue, Jun 09, 2026 at 09:52:51AM +0200, Miquel Raynal wrote:
>>>>>>>>>> On MDM9607, there is only a single controllable clock for the NAND
>>>>>>>>>> controller (RPM_SMD_QPIC_CLK). The same situation also applies e.g. for
>>>>>>>>>> qcom,sdx55-nand, but the corresponding device tree (qcom-sdx55.dtsi) works
>>>>>>>>>> around that by assigning a dummy clock (&nand_clk_dummy) to the second
>>>>>>>>>> clock ("aon") that is required by the dt-bindings. This is not really
>>>>>>>>>> useful, so avoid doing that for new platforms by excluding the second "aon"
>>>>>>>>>> clock entry in the dt-bindings.
>>>>>>>>>
>>>>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>>>>>>>>
>>>>>>>> What is the problem in giving twice the same clock? If this is what is
>>>>>>>> done in the hardware routing, I do not see the reason for more
>>>>>>>> complexity in the binding?
>>>>>>>>
>>>>>>>
>>>>>>> I had that in my first draft for this series, but this would be wrong
>>>>>>> IMO. I suspect there is no QPIC/NAND related "aon" (always-on) clock on
>>>>>>> this platform at all. I'm not sure about MDM9607 in particular (maybe
>>>>>>> someone from Qualcomm can confirm), but a similar platform I was looking
>>>>>>> into at some point actually had *3* separate clocks for QPIC in the
>>>>>>> hardware and none of them were called "aon" ...
>>>>>>
>>>>>> gcc_qpic_ahb_clk (50/100/133.(3) MHz sourced from PCNoC_bfdcd_clk_src)
>>>>>> gcc_qpic_clk (likewise, sourced from qpic_clk_src which is sourced
>>>>>> from GPLLs)
>>>>>> gcc_qpic_system_clk (32 KHz)
>>>>>>
>>>>>> No clock containing the substring 'aon' in its name on this platform
>>>>>
>>>>> Looking at SDX65, perhaps the 32 Khz clock is the "aon" one after all..
>>>>> The NAND documentation says
>>>>>
>>>>> CC_QPIC_SYSTEM_CLK - Always-on timeout clock (32 KHz)
>>>>>
>>>>
>>>> Thanks for looking this up.
>>>>
>>>> IMO, if we want to describe the actual hardware routing, we should
>>>> describe all 3 clocks and assign all of them to RPM_SMD_QPIC_CLK for
>>>> MDM9607).
>>>
>>> Sounds more accurate to me.
>>>
>>>> The resulting diff would be basically the same as this patch just
>>>> inversed (3 clocks for MDM9607+SDX(?) and 2 clocks for the IPQ* SoCs.
>>>
>>> Diff would not be simpler but more accurate. So if we go for a
>>> modification of the bindings, I would prefer that path.
>>>
>> 
>> IMO the result wouldn't be much more accurate from the perspective of
>> the kernel. If we assign RPM_SMD_QPIC_CLK to all 3 clocks we would be
>> effectively saying "there is a single clock with a single rate that is
>> sourcing 'core', 'ahb' and 'system'(/'aon')". But in reality, these are
>> 3 separate clock domains with separate rates, as shown by Konrad above.
>> 
>> We could try defining dummy clocks like the &nand_clk_dummy in
>> qcom-sdx55.dtsi, but this isn't very accurate either. Presumably, all of
>> these clocks are toggled by RPM_SMD_QPIC_CLK. So if we define a dummy
>> clock for 'ahb', then enabling that clock without also enabling the
>> non-dummy 'core' (RPM_SMD_QPIC_CLK) will do nothing.
>
> I can't find a good answer for what RPM_SMD_QPIC_CLK controls, maybe
> +Mani or +Kathiravan know where to look
>
> Konrad
>
>> 
>> At the end, the truth for the OS/kernel running on this hardware is that
>> it can only see the 'core' clock (with the option to change its rate).
>> All others are invisible, with no way to influence or check the status,
>> so pretending that we have separate resources for them doesn't really
>> make things more accurate in my opinion.
>> 
>> But yeah, let's leave the decision up to Krzysztof. I'm happy to change
>> this patch as needed as long it works at the end. :-)

Sorry to bother you Krzysztof. Based on the previous discussion, would
you mind giving an updated point of view? If you don't have time, no
problem, I can take the series as-is.

Thanks a lot,
Miquèl