[PATCH 0/2] soc: ti: wkup_m3_ipc: support vtt toggle

[PATCH 0/2] soc: ti: wkup_m3_ipc: support vtt toggle

[Drew Fustini]

This series enables the Cortex M3 processor found in AM33xx and AM43xx
SoCs to toggle the VTT regulator during low power mode transitions.

I recently converted the Wakeup M3 IPC bindings to YAML. Rob has applied
that patch [1]. It is a prerequisite for the wkup-m3-ipc.yaml patch in
this series.

[1] https://lore.kernel.org/linux-devicetree/YlCACSZx5xsPSwNC@robh.at.kernel.org/

Dave Gerlach (1):
  soc: ti: wkup_m3_ipc: Add support for toggling VTT regulator

Drew Fustini (1):
  dt-bindings: wkup-m3-ipc: Add vtt toggle bindings

 .../bindings/soc/ti/wkup-m3-ipc.yaml          | 21 +++++++++++++++
 drivers/soc/ti/wkup_m3_ipc.c                  | 27 +++++++++++++++++--
 include/linux/wkup_m3_ipc.h                   |  1 +
 3 files changed, 47 insertions(+), 2 deletions(-)

-- 
2.32.0

[PATCH 1/2] dt-bindings: wkup-m3-ipc: Add vtt toggle bindings

[Drew Fustini]

Document the Wakeup M3 IPC properties that can be used to toggle the VTT
regulator during low power mode transitions.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
[dfustini: converted to YAML]
Signed-off-by: Drew Fustini <dfustini@baylibre.com>
---
 .../bindings/soc/ti/wkup-m3-ipc.yaml          | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/ti/wkup-m3-ipc.yaml b/Documentation/devicetree/bindings/soc/ti/wkup-m3-ipc.yaml
index d855c01ce61c..6a8fcad8b2fd 100644
--- a/Documentation/devicetree/bindings/soc/ti/wkup-m3-ipc.yaml
+++ b/Documentation/devicetree/bindings/soc/ti/wkup-m3-ipc.yaml
@@ -24,6 +24,16 @@ description: |+
   A wkup_m3_ipc device node is used to represent the IPC registers within an
   SoC.
 
+  Support for VTT Toggle
+  ==================================
+  In order to enable the support for VTT toggle during Suspend/Resume sequence
+  needed by some boards (like AM335x EVM-SK & AM437x GP EVM), the vtt properties
+  below are required. It is possible to toggle VTT using one of two methods
+  depending on the SoC being used, either GPIO0 toggle (AM335x and AM437x), or
+  any GPIO with DS_PAD_CONFIG bits in the control module (AM437x only). Please
+  note that only pins on the GPIO0 module can be used for GPIO toggle. It will
+  not work with any other GPIO module.
+
 properties:
   compatible:
     enum:
@@ -51,6 +61,15 @@ properties:
       mbox_wkupm3 child node.
     maxItems: 1
 
+  ti,needs-vtt-toggle:
+    type: boolean
+    description: boards requires VTT toggling during suspend/resume
+
+  ti,vtt-gpio-pin:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: pin on the GPIO0 module used for VTT toggle
+    maxItems: 1
+
 required:
   - compatible
   - reg
@@ -76,6 +95,8 @@ examples:
            interrupts = <78>;
            ti,rproc = <&wkup_m3>;
            mboxes = <&mailbox &mbox_wkupm3>;
+           ti,needs-vtt-toggle;
+           ti,vtt-gpio-pin = <7>;
         };
     };
 ...
-- 
2.32.0

[PATCH 2/2] soc: ti: wkup_m3_ipc: Add support for toggling VTT regulator

[Drew Fustini]

From: Dave Gerlach <d-gerlach@ti.com>

Some boards (currently AM335x EVM-SK) provides s/w control via
GPIO to toggle VTT regulator to reduce power consumption in low
power state.

The VTT regulator should be disabled after enabling self-refresh on
suspend, and should be enabled before disabling self-refresh on resume.
This is to allow proper self-refresh entry/exit commands to be
transmitted to the memory.

Add support for toggling VTT regulator using DT properties.
Actual toggling happens in CM3 Firmware. The enable option & the GPIO
pin used is collected in A8 Core and then sent to CM3 using IPC
registers.

Note:
Here it is assumed that VTT Toggle will be done using a pin on GPIO-0
Instance. The reason is GPIO-0 is in wakeup domain.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
Signed-off-by: Keerthy <j-keerthy@ti.com>
Signed-off-by: Drew Fustini <dfustini@baylibre.com>
---
 drivers/soc/ti/wkup_m3_ipc.c | 27 +++++++++++++++++++++++++--
 include/linux/wkup_m3_ipc.h  |  1 +
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
index 2f03ced0f411..8b92425255a2 100644
--- a/drivers/soc/ti/wkup_m3_ipc.c
+++ b/drivers/soc/ti/wkup_m3_ipc.c
@@ -40,6 +40,13 @@
 #define M3_FW_VERSION_MASK		0xffff
 #define M3_WAKE_SRC_MASK		0xff
 
+#define IPC_MEM_TYPE_SHIFT		(0x0)
+#define IPC_MEM_TYPE_MASK		(0x7 << 0)
+#define IPC_VTT_STAT_SHIFT		(0x3)
+#define IPC_VTT_STAT_MASK		(0x1 << 3)
+#define IPC_VTT_GPIO_PIN_SHIFT		(0x4)
+#define IPC_VTT_GPIO_PIN_MASK		(0x3f << 4)
+
 #define M3_STATE_UNKNOWN		0
 #define M3_STATE_RESET			1
 #define M3_STATE_INITED			2
@@ -215,6 +222,12 @@ static int wkup_m3_is_available(struct wkup_m3_ipc *m3_ipc)
 		(m3_ipc->state != M3_STATE_UNKNOWN));
 }
 
+static void wkup_m3_set_vtt_gpio(struct wkup_m3_ipc *m3_ipc, int gpio)
+{
+	m3_ipc->vtt_conf = (1 << IPC_VTT_STAT_SHIFT) |
+			    (gpio << IPC_VTT_GPIO_PIN_SHIFT);
+}
+
 /* Public functions */
 /**
  * wkup_m3_set_mem_type - Pass wkup_m3 which type of memory is in use
@@ -294,7 +307,8 @@ static int wkup_m3_prepare_low_power(struct wkup_m3_ipc *m3_ipc, int state)
 	/* Program each required IPC register then write defaults to others */
 	wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->resume_addr, 0);
 	wkup_m3_ctrl_ipc_write(m3_ipc, m3_power_state, 1);
-	wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->mem_type, 4);
+	wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->mem_type |
+			       m3_ipc->vtt_conf, 4);
 
 	wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 2);
 	wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 3);
@@ -433,12 +447,13 @@ static int wkup_m3_rproc_boot_thread(void *arg)
 static int wkup_m3_ipc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	int irq, ret;
+	int irq, ret, temp;
 	phandle rproc_phandle;
 	struct rproc *m3_rproc;
 	struct resource *res;
 	struct task_struct *task;
 	struct wkup_m3_ipc *m3_ipc;
+	struct device_node *np = dev->of_node;
 
 	m3_ipc = devm_kzalloc(dev, sizeof(*m3_ipc), GFP_KERNEL);
 	if (!m3_ipc)
@@ -496,6 +511,14 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
 
 	m3_ipc->ops = &ipc_ops;
 
+	if (of_find_property(np, "ti,needs-vtt-toggle", NULL) &&
+	    !(of_property_read_u32(np, "ti,vtt-gpio-pin", &temp))) {
+		if (temp >= 0 && temp <= 31)
+			wkup_m3_set_vtt_gpio(m3_ipc, temp);
+		else
+			dev_warn(dev, "Invalid VTT GPIO(%d) pin\n", temp);
+	}
+
 	/*
 	 * Wait for firmware loading completion in a thread so we
 	 * can boot the wkup_m3 as soon as it's ready without holding
diff --git a/include/linux/wkup_m3_ipc.h b/include/linux/wkup_m3_ipc.h
index 3f496967b538..2bc52c6381d5 100644
--- a/include/linux/wkup_m3_ipc.h
+++ b/include/linux/wkup_m3_ipc.h
@@ -33,6 +33,7 @@ struct wkup_m3_ipc {
 
 	int mem_type;
 	unsigned long resume_addr;
+	int vtt_conf;
 	int state;
 
 	struct completion sync_complete;
-- 
2.32.0

Re: [PATCH 1/2] dt-bindings: wkup-m3-ipc: Add vtt toggle bindings

[Krzysztof Kozlowski]

On 09/04/2022 06:51, Drew Fustini wrote:
> Document the Wakeup M3 IPC properties that can be used to toggle the VTT
> regulator during low power mode transitions.
>

(...)

> @@ -51,6 +61,15 @@ properties:
>        mbox_wkupm3 child node.
>      maxItems: 1
>  
> +  ti,needs-vtt-toggle:
> +    type: boolean
> +    description: boards requires VTT toggling during suspend/resume
> +
> +  ti,vtt-gpio-pin:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: pin on the GPIO0 module used for VTT toggle
> +    maxItems: 1

No need for maxItems, it's a single uint32. You could add "maximum" if
you know the constraints for the value.

Do you actually need two properties (bool + pin)? A case of
"ti,vtt-gpio-pin" without the "ti,needs-vtt-toggle" is valid?

The other way looks wrong, so you need to add a dependency (see
example-schema).


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: wkup-m3-ipc: Add vtt toggle bindings

[Drew Fustini]

On Sat, Apr 09, 2022 at 06:25:24PM +0200, Krzysztof Kozlowski wrote:
> On 09/04/2022 06:51, Drew Fustini wrote:
> > Document the Wakeup M3 IPC properties that can be used to toggle the VTT
> > regulator during low power mode transitions.
> >
> 
> (...)
> 
> > @@ -51,6 +61,15 @@ properties:
> >        mbox_wkupm3 child node.
> >      maxItems: 1
> >  
> > +  ti,needs-vtt-toggle:
> > +    type: boolean
> > +    description: boards requires VTT toggling during suspend/resume
> > +
> > +  ti,vtt-gpio-pin:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: pin on the GPIO0 module used for VTT toggle
> > +    maxItems: 1
> 
> No need for maxItems, it's a single uint32. You could add "maximum" if
> you know the constraints for the value.

Thank you for letting me know.

> Do you actually need two properties (bool + pin)? A case of
> "ti,vtt-gpio-pin" without the "ti,needs-vtt-toggle" is valid?

You raise a good point. In fact, the driver does nothing useful in the
case of "ti,needs-vtt-toggle" without "ti,vtt-gpio-pin". Therefore, I
will remove the "ti,needs-vtt-toggle" property.

> 
> The other way looks wrong, so you need to add a dependency (see
> example-schema).
> 
> 
> Best regards,
> Krzysztof

Thank you,
Drew

Re: [PATCH 1/2] dt-bindings: wkup-m3-ipc: Add vtt toggle bindings

[Rob Herring]

On Sat, Apr 09, 2022 at 06:25:24PM +0200, Krzysztof Kozlowski wrote:
> On 09/04/2022 06:51, Drew Fustini wrote:
> > Document the Wakeup M3 IPC properties that can be used to toggle the VTT
> > regulator during low power mode transitions.
> >
> 
> (...)
> 
> > @@ -51,6 +61,15 @@ properties:
> >        mbox_wkupm3 child node.
> >      maxItems: 1
> >  
> > +  ti,needs-vtt-toggle:
> > +    type: boolean
> > +    description: boards requires VTT toggling during suspend/resume
> > +
> > +  ti,vtt-gpio-pin:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: pin on the GPIO0 module used for VTT toggle
> > +    maxItems: 1
> 
> No need for maxItems, it's a single uint32. You could add "maximum" if
> you know the constraints for the value.

Whenever I see the same problem repeated, I ask if we can catch with 
meta-schema. Often we can, but we have to fix all the existing ones:

media/mediatek,vcodec-encoder.yaml: properties:mediatek,scp:maxItems: False schema does not allow 1
media/mediatek,vcodec-encoder.yaml: properties:mediatek,vpu:maxItems: False schema does not allow 1
remoteproc/qcom,sc7280-wpss-pil.yaml: properties:glink-edge:properties:qcom,remote-pid:maxItems: False schema does not allow 1
media/mediatek,vcodec-subdev-decoder.yaml: properties:mediatek,scp:maxItems: False schema does not allow 1
media/mediatek,vcodec-decoder.yaml: properties:mediatek,vpu:maxItems: False schema does not allow 1
media/mediatek,vcodec-decoder.yaml: properties:mediatek,scp:maxItems: False schema does not allow 1
media/coda.yaml: properties:iram:maxItems: False schema does not allow 1
display/panel/panel-timing.yaml: properties:vsync-len:oneOf:0:maxItems: False schema does not allow 1
display/panel/panel-timing.yaml: properties:vsync-len:oneOf:0:items: False schema does not allow {'description': 'typical number of lines'}
display/panel/panel-timing.yaml: properties:hfront-porch:oneOf:0:maxItems: False schema does not allow 1
display/panel/panel-timing.yaml: properties:hfront-porch:oneOf:0:items: False schema does not allow {'description': 'typical number of pixels'}
display/panel/panel-timing.yaml: properties:hback-porch:oneOf:0:maxItems: False schema does not allow 1
display/panel/panel-timing.yaml: properties:hback-porch:oneOf:0:items: False schema does not allow {'description': 'typical number of pixels'}
display/panel/panel-timing.yaml: properties:hsync-len:oneOf:0:maxItems: False schema does not allow 1
display/panel/panel-timing.yaml: properties:hsync-len:oneOf:0:items: False schema does not allow {'description': 'typical number of pixels'}
display/panel/panel-timing.yaml: properties:vfront-porch:oneOf:0:maxItems: False schema does not allow 1
display/panel/panel-timing.yaml: properties:vfront-porch:oneOf:0:items: False schema does not allow {'description': 'typical number of lines'}
display/panel/panel-timing.yaml: properties:vback-porch:oneOf:0:maxItems: False schema does not allow 1
display/panel/panel-timing.yaml: properties:vback-porch:oneOf:0:items: False schema does not allow {'description': 'typical number of lines'}
iio/adc/st,stm32-dfsdm-adc.yaml: patternProperties:^filter@[0-9]+$:properties:st,filter-order:items: False schema does not allow {'minimum': 0, 'maximum': 5}
spi/renesas,sh-msiof.yaml: properties:renesas,rx-fifo-size:maxItems: False schema does not allow 1
spi/renesas,sh-msiof.yaml: properties:renesas,tx-fifo-size:maxItems: False schema does not allow 1
hwmon/ti,tmp421.yaml: patternProperties:^channel@([0-3])$:properties:ti,n-factor:items: False schema does not allow {'minimum': 0, 'maximum': 255}
hwmon/ti,tmp464.yaml: patternProperties:^channel@([0-8])$:properties:ti,n-factor:items: False schema does not allow {'minimum': -128, 'maximum': 127}

I already have fixes for these.

FYI, the meta-schema change looks like this:

diff --git a/dtschema/meta-schemas/keywords.yaml b/dtschema/meta-schemas/keywords.yaml
index 6d6fdfd08d3b..73ed113aed3a 100644
--- a/dtschema/meta-schemas/keywords.yaml
+++ b/dtschema/meta-schemas/keywords.yaml
@@ -155,3 +155,17 @@ properties:
       - type: boolean
   uniqueItems:
     type: boolean
+
+allOf:
+  - description: Scalar properties should not have array keywords
+    if:
+      properties:
+        $ref:
+          pattern: '(int[1-8]+|phandle|flag)$'
+      required:
+        - $ref
+    then:
+      properties:
+        maxItems: false
+        minItems: false
+        items: false