[PATCH 0/4] mtd: rawnand: qcom: Add MDM9607

[PATCH 0/4] mtd: rawnand: qcom: Add MDM9607

[Stephan Gerhold]

MDM9607 has QPIC v1.5 that supports the OP_PAGE_READ_ONFI_READ command, but
is missing the rest of the hardware changes in QPIC v2. There is also only
a single clock that can be controlled using the RPM firmware. Document and
add the new qcom,mdm9607-nand compatible for this setup.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
Stephan Gerhold (4):
      dt-bindings: mtd: qcom,nandc: Add MDM9607 QPIC NAND controller
      mtd: rawnand: qcom: Make "aon" clock optional
      mtd: rawnand: qcom: Make has_onfi_read_op separate from qpic_version2
      mtd: rawnand: qcom: Add MDM9607 compatible

 .../devicetree/bindings/mtd/qcom,nandc.yaml        | 24 +++++++++++++++++
 drivers/mtd/nand/raw/qcom_nandc.c                  | 30 ++++++++++++++++------
 include/linux/mtd/nand-qpic-common.h               |  2 ++
 3 files changed, 48 insertions(+), 8 deletions(-)
---
base-commit: 19ed11aee966d91beebdef9d32ce926474872f79
change-id: 20260608-qcom-nandc-mdm9607-f2196eeb6bc0

Best regards,
--  
Stephan Gerhold <stephan.gerhold@linaro.org>

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

[Stephan Gerhold]

Add the qcom,mdm9607-nand compatible for the QPIC NAND controller used
inside the MDM9607 SoC.

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.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 .../devicetree/bindings/mtd/qcom,nandc.yaml        | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
index 5511389960f0..4a9ba32cbddd 100644
--- a/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
+++ b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
@@ -22,17 +22,20 @@ properties:
               - qcom,ipq4019-nand
               - qcom,ipq6018-nand
               - qcom,ipq8074-nand
+              - qcom,mdm9607-nand
               - qcom,sdx55-nand
 
   reg:
     maxItems: 1
 
   clocks:
+    minItems: 1
     items:
       - description: Core Clock
       - description: Always ON Clock
 
   clock-names:
+    minItems: 1
     items:
       - const: core
       - const: aon
@@ -101,6 +104,26 @@ allOf:
           items:
             - const: rxtx
 
+  # MDM9607 has no (separately controllable) "aon" clock, only "core"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,mdm9607-nand
+    then:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names:
+          maxItems: 1
+    else:
+      properties:
+        clocks:
+          minItems: 2
+        clock-names:
+          minItems: 2
+
   - if:
       properties:
         compatible:
@@ -121,6 +144,7 @@ allOf:
               - qcom,ipq4019-nand
               - qcom,ipq6018-nand
               - qcom,ipq8074-nand
+              - qcom,mdm9607-nand
               - qcom,sdx55-nand
 
     then:

-- 
2.54.0

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

[Krzysztof Kozlowski]

On Mon, Jun 08, 2026 at 03:20:22PM +0200, Stephan Gerhold wrote:
> Add the qcom,mdm9607-nand compatible for the QPIC NAND controller used
> inside the MDM9607 SoC.
> 
> 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.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
>  .../devicetree/bindings/mtd/qcom,nandc.yaml        | 24 ++++++++++++++++++++++
>  1 file changed, 24 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

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

[Miquel Raynal]

Hello,

>> 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?

Thanks,
Miquèl

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

[Stephan Gerhold]

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" ...

I think it's better to omit it and describe what we know for sure
instead of describing some dummy hardware resources that do not actually
exist.

Thanks,
Stephan

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

[Konrad Dybcio]

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

Konrad

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

[Konrad Dybcio]

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)

Konrad

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

[Stephan Gerhold]

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).

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.
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.

Thanks,
Stephan

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

[Miquel Raynal]

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

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

[Stephan Gerhold]

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.

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. :-)

Thanks,
Stephan

[PATCH 2/4] mtd: rawnand: qcom: Make "aon" clock optional

[Stephan Gerhold]

Some SoCs (e.g. MDM9607, SDX55) have only a single separately controllable
clock for the NAND controller. The actual clocks in the hardware are
managed by the firmware and turned on all together when needed. In this
case, there is no separate "aon" clock that can be described in the device
tree.

Make the second "aon" clock optional to avoid an error when it is missing.
For platforms that really need it, the dt-bindings are responsible for
validating that.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 4b80ce084d9a..0251dd591d40 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2280,7 +2280,7 @@ static int qcom_nandc_probe(struct platform_device *pdev)
 	if (IS_ERR(nandc->core_clk))
 		return PTR_ERR(nandc->core_clk);
 
-	nandc->aon_clk = devm_clk_get(dev, "aon");
+	nandc->aon_clk = devm_clk_get_optional(dev, "aon");
 	if (IS_ERR(nandc->aon_clk))
 		return PTR_ERR(nandc->aon_clk);
 

-- 
2.54.0

Re: [PATCH 2/4] mtd: rawnand: qcom: Make "aon" clock optional

[Krzysztof Kozlowski]

On Mon, Jun 08, 2026 at 03:20:23PM +0200, Stephan Gerhold wrote:
> Some SoCs (e.g. MDM9607, SDX55) have only a single separately controllable
> clock for the NAND controller. The actual clocks in the hardware are
> managed by the firmware and turned on all together when needed. In this
> case, there is no separate "aon" clock that can be described in the device
> tree.
> 
> Make the second "aon" clock optional to avoid an error when it is missing.
> For platforms that really need it, the dt-bindings are responsible for
> validating that.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH 3/4] mtd: rawnand: qcom: Make has_onfi_read_op separate from qpic_version2

[Stephan Gerhold]

QPIC v1.5 requires using the OP_PAGE_READ_ONFI_READ command, but is missing
the rest of the hardware changes that are currently covered by the QPIC v2
(qpic_version2) check in the driver. Split that into an extra
has_onfi_read_op feature flag so it can be separately enabled.

No functional change.

Signed-off-by: Stephan Gerhold <stephan.gerhold@linaro.org>
---
 drivers/mtd/nand/raw/qcom_nandc.c    | 15 ++++++++-------
 include/linux/mtd/nand-qpic-common.h |  2 ++
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 0251dd591d40..9217e8de5512 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -1564,7 +1564,7 @@ static int qcom_op_cmd_mapping(struct nand_chip *chip, u8 opcode,
 		cmd = OP_FETCH_ID;
 		break;
 	case NAND_CMD_PARAM:
-		if (nandc->props->qpic_version2)
+		if (nandc->props->has_onfi_read_op)
 			cmd = OP_PAGE_READ_ONFI_READ;
 		else
 			cmd = OP_PAGE_READ;
@@ -1903,7 +1903,7 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 		nandc->regs->ecc_buf_cfg = cpu_to_le32(ECC_CFG_ECC_DISABLE);
 
 	/* configure CMD1 and VLD for ONFI param probing in QPIC v1 */
-	if (!nandc->props->qpic_version2) {
+	if (!nandc->props->has_onfi_read_op) {
 		nandc->regs->vld = cpu_to_le32((nandc->vld & ~READ_START_VLD));
 		nandc->regs->cmd1 = cpu_to_le32((nandc->cmd1 & ~READ_ADDR_MASK) |
 						FIELD_PREP(READ_ADDR_MASK, NAND_CMD_PARAM));
@@ -1911,7 +1911,7 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 
 	nandc->regs->exec = cpu_to_le32(1);
 
-	if (!nandc->props->qpic_version2) {
+	if (!nandc->props->has_onfi_read_op) {
 		nandc->regs->orig_cmd1 = cpu_to_le32(nandc->cmd1);
 		nandc->regs->orig_vld = cpu_to_le32(nandc->vld);
 	}
@@ -1925,7 +1925,7 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 	else
 		nandc_set_read_loc_first(chip, reg_base, 0, len, 1);
 
-	if (!nandc->props->qpic_version2) {
+	if (!nandc->props->has_onfi_read_op) {
 		qcom_write_reg_dma(nandc, &nandc->regs->vld, NAND_DEV_CMD_VLD, 1, 0);
 		qcom_write_reg_dma(nandc, &nandc->regs->cmd1, NAND_DEV_CMD1, 1, NAND_BAM_NEXT_SGL);
 	}
@@ -1939,7 +1939,7 @@ static int qcom_param_page_type_exec(struct nand_chip *chip,  const struct nand_
 			   nandc->buf_count, 0);
 
 	/* restore CMD1 and VLD regs */
-	if (!nandc->props->qpic_version2) {
+	if (!nandc->props->has_onfi_read_op) {
 		qcom_write_reg_dma(nandc, &nandc->regs->orig_cmd1, NAND_DEV_CMD1_RESTORE, 1, 0);
 		qcom_write_reg_dma(nandc, &nandc->regs->orig_vld, NAND_DEV_CMD_VLD_RESTORE, 1,
 				   NAND_BAM_NEXT_SGL);
@@ -2041,7 +2041,7 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
 	if (!nandc->props->nandc_part_of_qpic)
 		nandc_write(nandc, SFLASHC_BURST_CFG, 0);
 
-	if (!nandc->props->qpic_version2)
+	if (!nandc->props->has_onfi_read_op)
 		nandc_write(nandc, dev_cmd_reg_addr(nandc, NAND_DEV_CMD_VLD),
 			    NAND_DEV_CMD_VLD_VAL);
 
@@ -2063,7 +2063,7 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
 	}
 
 	/* save the original values of these registers */
-	if (!nandc->props->qpic_version2) {
+	if (!nandc->props->has_onfi_read_op) {
 		nandc->cmd1 = nandc_read(nandc, dev_cmd_reg_addr(nandc, NAND_DEV_CMD1));
 		nandc->vld = NAND_DEV_CMD_VLD_VAL;
 	}
@@ -2385,6 +2385,7 @@ static const struct qcom_nandc_props sdx55_nandc_props = {
 	.ecc_modes = (ECC_BCH_4BIT | ECC_BCH_8BIT),
 	.supports_bam = true,
 	.nandc_part_of_qpic = true,
+	.has_onfi_read_op = true,
 	.qpic_version2 = true,
 	.dev_cmd_reg_start = 0x7000,
 	.bam_offset = 0x30000,
diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
index 006ca8c978a9..437448995187 100644
--- a/include/linux/mtd/nand-qpic-common.h
+++ b/include/linux/mtd/nand-qpic-common.h
@@ -443,6 +443,7 @@ struct qcom_nand_controller {
  * @dev_cmd_reg_start - NAND_DEV_CMD_* registers starting offset
  * @supports_bam - whether NAND controller is using BAM
  * @nandc_part_of_qpic - whether NAND controller is part of qpic IP
+ * @has_onfi_read_op - whether ONFI param page read command is supported
  * @qpic_version2 - flag to indicate QPIC IP version 2
  * @use_codeword_fixup - whether NAND has different layout for boot partitions
  */
@@ -452,6 +453,7 @@ struct qcom_nandc_props {
 	u32 bam_offset;
 	bool supports_bam;
 	bool nandc_part_of_qpic;
+	bool has_onfi_read_op;
 	bool qpic_version2;
 	bool use_codeword_fixup;
 };

-- 
2.54.0