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

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

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.

> The complexity of the binding would be the same, so is it worth
> reworking this patch? At the end, there is just one clock we can toggle
> through the firmware here and I doubt anyone uses this SoC without the
> RPM firmware.

This is not really about how we use it, even though an integration
specific compatible is probably allowed to simplify the inputs. So now
that I raised that point, I'll let the final decision to Krzyztof who
knows better the Qcomm ecosystem than me.

Cheers,
Miquèl