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

[Manivannan Sadhasivam]

On Wed, Jun 17, 2026 at 01:42:44PM +0200, Konrad Dybcio wrote:
> 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@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> >>>>>>>
> >>>>>>> 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
> 

QPIC hardware in recent SoCs requires 3 clocks as Konrad described. But older
SoCs like IPQ8064 require only 2 clocks:

1. QPIC_AHB_CLOCK
2. QPIC_CLOCK - Core clock
3. QPIC_SYSTEM_CLOCK - AON clock

These 3 clocks are supplied by the Global Clock Controller (GCC), but the access
is restricted to RPMh (the entity that controls the power to the whole SoC) on
SoCs where RPMh is available. On non-RPMh SoCs like IPQ8064, these clocks are
directly controlled by the kernel via GCC. But on RPMh SoCs, kernel cannot
control these clocks independently and it can only vote for RPM_SMD_QPIC_CLK,
which implicitly controls these 3 clocks.

So from kernel PoV, there is only one clock. I think we can just call it 'Core
Clock' as per the current binding and drop the AON clock for SoCs supporting
RPMh.

So this series is good as-is IMO.

- Mani

-- 
மணிவண்ணன் சதாசிவம்