Update the Icicle Kit device tree

From: Conor Dooley <[email protected]>

The Polarfire SoC is currently using two different compatible string prefixes. Fix this by changing "polarfire-soc-*" strings to "mpfs-*" in its system controller in order to match the compatible string used in the soc binding and device tree

Reviewed-by Geert Uytterhoeven <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
 ...larfire-soc-mailbox.yaml => microchip,mpfs-mailbox.yaml} | 6 +++---
 ...s-controller.yaml => microchip,mpfs-sys-controller.yaml} | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)
 rename Documentation/devicetree/bindings/mailbox/{microchip,polarfire-soc-mailbox.yaml => microchip,mpfs-mailbox.yaml} (82%)
 rename Documentation/devicetree/bindings/soc/microchip/{microchip,polarfire-soc-sys-controller.yaml => microchip,mpfs-sys-controller.yaml} (75%)

From: Conor Dooley <[email protected]>

Add mpfs-rng and mpfs-generic-services as children of the system controller.

Signed-off-by Conor Dooley <[email protected]>

---
 .../microchip,mpfs-sys-controller.yaml        | 41 ++++++++++++++++++-
 1 file changed, 39 insertions(+), 2 deletions(-)

Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-sys-controller.yaml +32 -1

index f699772fedf3..5e9977bc114e 100644
@@ -13,7 +13,6 @@ description: |
   The PolarFire SoC system controller is communicated with via a mailbox.
   This document describes the bindings for the client portion of that mailbox.
 
-
 properties:
   mboxes:
     maxItems: 1

@@ -21,6 +20,38 @@ properties:
   compatible:
     const: microchip,mpfs-sys-controller
 
+  rng:
+    type: object
+
+    description: |
+      The hardware random number generator on the Polarfire SoC is
+      accessed via the mailbox interface provided by the system controller
+
+    properties:
+      compatible:
+        const: microchip,mpfs-rng
+
+    required:
+      - compatible
+
+  sysserv:
+    type: object
+
+    description: |
+      The PolarFire SoC system controller is communicated with via a mailbox.
+      This binding represents several of the functions provided by the system
+      controller which do not belong in a specific subsystem, such as reading
+      the fpga device certificate, all of which follow the same format:
+        - a command + optional payload sent to the sys controller
+        - a status + a payload returned to Linux
+
+    properties:
+      compatible:
+        const: microchip,mpfs-generic-service
+
+    required:
+      - compatible
+
 required:
   - compatible
   - mboxes

@@ -29,7 +60,13 @@ additionalProperties: false
 
 examples:
   - |

From: Conor Dooley <[email protected]>

Add device tree bindings for the i2c controller on the Microchip PolarFire SoC.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Daire McNamara <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
 .../bindings/i2c/microchip,mpfs-i2c.yaml      | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml

From: Conor Dooley <[email protected]>

Add device tree bindings for the i2c controller on the Microchip PolarFire SoC.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Daire McNamara <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 8 lines · On Mon, Jan 31, 2022 at 5:45 AM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the i2c controller on the Microchip PolarFire SoC.

Reviewed-by Rob Herring <[email protected]>

There should not be a blank line here.

Also, tags should be in chronological order typically. If Daire sent this patch out with my tag, then the order is correct. If I gave it on a version you sent, then it goes between Daire's and your S-o-b which is the case here.

Rob

Quoted 2 lines

Signed-off-by Daire McNamara <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

Quoted 12 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 8 lines · On Mon, Jan 31, 2022 at 5:45 AM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the i2c controller on the Microchip PolarFire SoC.

Reviewed-by Rob Herring <[email protected]>

Quoted 6 lines

There should not be a blank line here.

Also, tags should be in chronological order typically. If Daire sent this patch out with my tag, then the order is correct. If I gave it on a version you sent, then it goes between Daire's and your S-o-b which is the case here.

Oh, thanks. Probably been messing this up right/left/centre.

On another note, I know I'm still missing a RB still on some of the bindings, but what is the acceptance path for this series? Any left over bindings not taken by subsystems via yourself and the dts changes via Palmer's tree?

Quoted 21 lines · On Mon, Jan 31, 2022 at 03:55:32PM +0000, [email protected] wrote:

Quoted 12 lines · Rob Herring

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 8 lines · On Mon, Jan 31, 2022 at 5:45 AM <[email protected]> wrote:

From: Conor Dooley <[email protected]>

Add device tree bindings for the i2c controller on the Microchip PolarFire SoC.

Reviewed-by Rob Herring <[email protected]>

Quoted 6 lines

There should not be a blank line here.

Also, tags should be in chronological order typically. If Daire sent this patch out with my tag, then the order is correct. If I gave it on a version you sent, then it goes between Daire's and your S-o-b which is the case here.

Oh, thanks. Probably been messing this up right/left/centre.

Quoted 4 lines

On another note, I know I'm still missing a RB still on some of the bindings, but what is the acceptance path for this series? Any left over bindings not taken by subsystems via yourself and the dts changes via Palmer's tree?

They should go via subsystems. I can take if you want, but not with missing dependencies. I need my tree working.

From: Conor Dooley <[email protected]>

Add device tree bindings for the real time clock on the Microchip PolarFire SoC.

Signed-off-by Daire McNamara <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
 .../bindings/rtc/microchip,mfps-rtc.yaml      | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml

From: Conor Dooley <[email protected]>

Add device tree bindings for the real time clock on the Microchip PolarFire SoC.

Signed-off-by Daire McNamara <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
 .../bindings/rtc/microchip,mfps-rtc.yaml      | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml

Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +20 -0

new file mode 100644
index 000000000000..f35cca4e8656
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/microchip,mfps-rtc.yaml#
+
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PolarFire Soc (MPFS) RTC Device Tree Bindings
+
+allOf:
+  - $ref: rtc.yaml#
+
+maintainers:
+  - Daire McNamara <[email protected]>
+  - Lewis Hanly <[email protected]>
+
+properties:
+  compatible:
+    enum:
+      - microchip,mpfs-rtc

Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml new file +28 -0

+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: rtc
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/microchip,mpfs-clock.h>
+    rtc@20124000 {
+        compatible = "microchip,mpfs-rtc";
+        reg = <0x20124000 0x1000>;
+        clocks = <&clkcfg CLK_RTC>;
+        clock-names = "rtc";
+        interrupts = <80>, <81>;
+    };
+...

Signature · 1 line

2.35.0

From: Conor Dooley <[email protected]>

Add device tree bindings for the gpio controller on the Microchip PolarFire SoC.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
 .../bindings/gpio/microchip,mpfs-gpio.yaml    | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml

From: Conor Dooley <[email protected]>

Add device tree bindings for the Microchip fpga fabric based "core" PWM controller.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
 .../bindings/pwm/microchip,corepwm.yaml       | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml

From: Conor Dooley <[email protected]>

Add device tree bindings for the Microchip fpga fabric based "core" PWM controller.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
 .../bindings/pwm/microchip,corepwm.yaml       | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +34 -0

new file mode 100644
index 000000000000..26a77cde2465
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/microchip,corepwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip ip core PWM controller bindings
+
+maintainers:
+  - Conor Dooley <[email protected]>
+
+description: |
+  corePWM is an 16 channel pulse width modulator FPGA IP
+
+  https://www.microsemi.com/existing-parts/parts/152118
+
+properties:
+  compatible:
+    items:
+      - const: microchip,corepwm-rtl-v4
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  "#pwm-cells":
+    const: 2
+
+  microchip,sync-update:
+    description: |
+      In synchronous mode, all channels are updated at the beginning of the PWM period.

Quoted 12 lines

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On Mon, Jan 31, 2022 at 11:47:21AM +0000, [email protected] wrote:
From: Conor Dooley <[email protected]>

Add device tree bindings for the Microchip fpga fabric based "core" PWM controller.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
.../bindings/pwm/microchip,corepwm.yaml | 75 +++++++++++++++++++

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml line 44

Quoted 13 lines

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +13 -0

+  microchip,sync-update:
+    description: |
+      In synchronous mode, all channels are updated at the beginning of the PWM period.
+      Asynchronous mode is relevant to applications such as LED control, where
+      synchronous updates are not required. Asynchronous mode lowers the area size,
+      reducing shadow register requirements. This can be set at run time, provided
+      SHADOW_REG_EN is asserted. SHADOW_REG_EN is set by the FPGA bitstream programmed
+      to the device.
+      Each bit corresponds to a PWM channel & represents whether synchronous mode is
+      possible for the PWM channel.
+
+    $ref: /schemas/types.yaml#/definitions/uint16
+    default: 0

I'm not sure I understand this correctly. This is a soft-core and you can synthesize it either with or without the ability to do synchronous updates or not, right? All 16 channels share the same period length and in the simple implementation changing the duty cycle is done at once (maybe introducing a glitch) and in the more expensive implementation there is a register to implement both variants?

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml line 53

Quoted 8 lines

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +8 -0

+  microchip,dac-mode:
+    description: |
+      Optional, per-channel Low Ripple DAC mode is possible on this IP core. It creates
+      a minimum period pulse train whose High/Low average is that of the chosen duty
+      cycle. This "DAC" will have far better bandwidth and ripple performance than the
+      standard PWM algorithm can achieve.
+      Each bit corresponds to a PWM channel & represents whether dac mode is enabled
+      that PWM channel.

In the last sentence a "for" is missing?

These two properties are not detectable in software?

Quoted 28 lines · Uwe Kleine-König

Quoted 16 lines

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On Mon, Jan 31, 2022 at 11:47:21AM +0000, [email protected] wrote:
From: Conor Dooley <[email protected]>

Add device tree bindings for the Microchip fpga fabric based "core" PWM controller.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
.../bindings/pwm/microchip,corepwm.yaml | 75 +++++++++++++++++++

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +3 -0

+  microchip,sync-update:
+    description: |
+      In synchronous mode, all channels are updated at the beginning of the PWM period.

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml line 44

Quoted 10 lines

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +10 -0

+      Asynchronous mode is relevant to applications such as LED control, where
+      synchronous updates are not required. Asynchronous mode lowers the area size,
+      reducing shadow register requirements. This can be set at run time, provided
+      SHADOW_REG_EN is asserted. SHADOW_REG_EN is set by the FPGA bitstream programmed
+      to the device.
+      Each bit corresponds to a PWM channel & represents whether synchronous mode is
+      possible for the PWM channel.
+
+    $ref: /schemas/types.yaml#/definitions/uint16
+    default: 0

Quoted 6 lines

I'm not sure I understand this correctly. This is a soft-core and you can synthesize it either with or without the ability to do synchronous updates or not, right? All 16 channels share the same period length and in the simple implementation changing the duty cycle is done at once (maybe introducing a glitch) and in the more expensive implementation there is a register to implement both variants?

Correct. If the IP is instantiated with SHADOW_REG_ENx=1, both registers that control the duty cycle for channel x have a second "shadow reg" synthesised. At runtime a bit wide register exposed to APB can be used to toggle on/off synchronised mode for all channels it has been synthesised for.

I will reword this description since it is not clear.

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Conor,

Quoted 41 lines · On Wed, Feb 2, 2022 at 1:33 PM <[email protected]> wrote:

Quoted 28 lines · Uwe Kleine-König

Quoted 16 lines

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On Mon, Jan 31, 2022 at 11:47:21AM +0000, [email protected] wrote:
From: Conor Dooley <[email protected]>

Add device tree bindings for the Microchip fpga fabric based "core" PWM controller.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
.../bindings/pwm/microchip,corepwm.yaml | 75 +++++++++++++++++++

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +3 -0

+  microchip,sync-update:
+    description: |
+      In synchronous mode, all channels are updated at the beginning of the PWM period.

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml line 44

Quoted 10 lines

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +10 -0

+      Asynchronous mode is relevant to applications such as LED control, where
+      synchronous updates are not required. Asynchronous mode lowers the area size,
+      reducing shadow register requirements. This can be set at run time, provided
+      SHADOW_REG_EN is asserted. SHADOW_REG_EN is set by the FPGA bitstream programmed
+      to the device.
+      Each bit corresponds to a PWM channel & represents whether synchronous mode is
+      possible for the PWM channel.
+
+    $ref: /schemas/types.yaml#/definitions/uint16
+    default: 0

Quoted 6 lines

I'm not sure I understand this correctly. This is a soft-core and you can synthesize it either with or without the ability to do synchronous updates or not, right? All 16 channels share the same period length and in the simple implementation changing the duty cycle is done at once (maybe introducing a glitch) and in the more expensive implementation there is a register to implement both variants?

Correct. If the IP is instantiated with SHADOW_REG_ENx=1, both registers that control the duty cycle for channel x have a second "shadow reg" synthesised. At runtime a bit wide register exposed to APB can be used to toggle on/off synchronised mode for all channels it has been synthesised for.

Quoted 1 line

I will reword this description since it is not clear.

Shouldn't it use a different compatible value instead? Differentiation by properties is not recommended, as it's easy to miss a difference.

Quoted 42 lines · Geert Uytterhoeven

Quoted 41 lines · On Wed, Feb 2, 2022 at 1:33 PM <[email protected]> wrote:

Quoted 28 lines · Uwe Kleine-König

Quoted 16 lines

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On Mon, Jan 31, 2022 at 11:47:21AM +0000, [email protected] wrote:
From: Conor Dooley <[email protected]>

Add device tree bindings for the Microchip fpga fabric based "core" PWM controller.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
.../bindings/pwm/microchip,corepwm.yaml | 75 +++++++++++++++++++

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +3 -0

+  microchip,sync-update:
+    description: |
+      In synchronous mode, all channels are updated at the beginning of the PWM period.

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml line 44

Quoted 10 lines

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +10 -0

+      Asynchronous mode is relevant to applications such as LED control, where
+      synchronous updates are not required. Asynchronous mode lowers the area size,
+      reducing shadow register requirements. This can be set at run time, provided
+      SHADOW_REG_EN is asserted. SHADOW_REG_EN is set by the FPGA bitstream programmed
+      to the device.
+      Each bit corresponds to a PWM channel & represents whether synchronous mode is
+      possible for the PWM channel.
+
+    $ref: /schemas/types.yaml#/definitions/uint16
+    default: 0

Quoted 6 lines

I'm not sure I understand this correctly. This is a soft-core and you can synthesize it either with or without the ability to do synchronous updates or not, right? All 16 channels share the same period length and in the simple implementation changing the duty cycle is done at once (maybe introducing a glitch) and in the more expensive implementation there is a register to implement both variants?

Correct. If the IP is instantiated with SHADOW_REG_ENx=1, both registers that control the duty cycle for channel x have a second "shadow reg" synthesised. At runtime a bit wide register exposed to APB can be used to toggle on/off synchronised mode for all channels it has been synthesised for.

Quoted 5 lines

Quoted 1 line

I will reword this description since it is not clear.

Shouldn't it use a different compatible value instead? Differentiation by properties is not recommended, as it's easy to miss a difference.

Either you have something in mind that I've not thought of, or I've done a bad job of explaining again. The buffer/"shadow" registers are synthesised on a per channel basis, so any combination of the 16 channels may have this capability. The same applies to the DAC mode, per channel there too.

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 49 lines · On Wed, Feb 2, 2022 at 2:46 PM <[email protected]> wrote:

Quoted 42 lines · Geert Uytterhoeven

Quoted 41 lines · On Wed, Feb 2, 2022 at 1:33 PM <[email protected]> wrote:

Quoted 28 lines · Uwe Kleine-König

Quoted 16 lines

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On Mon, Jan 31, 2022 at 11:47:21AM +0000, [email protected] wrote:
From: Conor Dooley <[email protected]>

Add device tree bindings for the Microchip fpga fabric based "core" PWM controller.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
.../bindings/pwm/microchip,corepwm.yaml | 75 +++++++++++++++++++

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +3 -0

+  microchip,sync-update:
+    description: |
+      In synchronous mode, all channels are updated at the beginning of the PWM period.

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml line 44

Quoted 10 lines

Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml new file +10 -0

+      Asynchronous mode is relevant to applications such as LED control, where
+      synchronous updates are not required. Asynchronous mode lowers the area size,
+      reducing shadow register requirements. This can be set at run time, provided
+      SHADOW_REG_EN is asserted. SHADOW_REG_EN is set by the FPGA bitstream programmed
+      to the device.
+      Each bit corresponds to a PWM channel & represents whether synchronous mode is
+      possible for the PWM channel.
+
+    $ref: /schemas/types.yaml#/definitions/uint16
+    default: 0

Quoted 6 lines

I'm not sure I understand this correctly. This is a soft-core and you can synthesize it either with or without the ability to do synchronous updates or not, right? All 16 channels share the same period length and in the simple implementation changing the duty cycle is done at once (maybe introducing a glitch) and in the more expensive implementation there is a register to implement both variants?

Correct. If the IP is instantiated with SHADOW_REG_ENx=1, both registers that control the duty cycle for channel x have a second "shadow reg" synthesised. At runtime a bit wide register exposed to APB can be used to toggle on/off synchronised mode for all channels it has been synthesised for.

Quoted 5 lines

Quoted 1 line

I will reword this description since it is not clear.

Shouldn't it use a different compatible value instead? Differentiation by properties is not recommended, as it's easy to miss a difference.

Quoted 5 lines

Either you have something in mind that I've not thought of, or I've done a bad job of explaining again. The buffer/"shadow" registers are synthesised on a per channel basis, so any combination of the 16 channels may have this capability. The same applies to the DAC mode, per channel there too.

Oops, hadn't noticed this is per channel. Indeed, then a different compatible value is futile.
So since "microchip,sync-update" is a bitmask, perhaps it should be called "microchip,sync-update-mask"?
Same for "microchip,dac-mode" -> "microchip,dac-mode-mask"?

Also, using different integer sizes than uint32 is frowned upon, unless there is a very good reason to do so. I can imagine a future version would support more channels, and then uint16 becomes a limitation.

For both: Rob?

Quoted 57 lines · Geert Uytterhoeven

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Quoted 49 lines · On Wed, Feb 2, 2022 at 2:46 PM <[email protected]> wrote:

Quoted 42 lines · Geert Uytterhoeven

Quoted 41 lines · On Wed, Feb 2, 2022 at 1:33 PM <[email protected]> wrote:

Quoted 28 lines · Uwe Kleine-König

Quoted 16 lines

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe On Mon, Jan 31, 2022 at 11:47:21AM +0000, [email protected] wrote:
From: Conor Dooley <[email protected]>

Add device tree bindings for the Microchip fpga fabric based "core" PWM controller.

Reviewed-by Rob Herring <[email protected]>

Signed-off-by Conor Dooley <[email protected]>

---
.../bindings/pwm/microchip,corepwm.yaml | 75 +++++++++++++++++++

(quoted diff) +3 -0

+  microchip,sync-update:
+    description: |
+      In synchronous mode, all channels are updated at the beginning of the PWM period.

Review

Quoted 10 lines

(quoted diff) +10 -0

+      Asynchronous mode is relevant to applications such as LED control, where
+      synchronous updates are not required. Asynchronous mode lowers the area size,
+      reducing shadow register requirements. This can be set at run time, provided
+      SHADOW_REG_EN is asserted. SHADOW_REG_EN is set by the FPGA bitstream programmed
+      to the device.
+      Each bit corresponds to a PWM channel & represents whether synchronous mode is
+      possible for the PWM channel.
+
+    $ref: /schemas/types.yaml#/definitions/uint16
+    default: 0

Quoted 6 lines

I'm not sure I understand this correctly. This is a soft-core and you can synthesize it either with or without the ability to do synchronous updates or not, right? All 16 channels share the same period length and in the simple implementation changing the duty cycle is done at once (maybe introducing a glitch) and in the more expensive implementation there is a register to implement both variants?

Correct. If the IP is instantiated with SHADOW_REG_ENx=1, both registers that control the duty cycle for channel x have a second "shadow reg" synthesised. At runtime a bit wide register exposed to APB can be used to toggle on/off synchronised mode for all channels it has been synthesised for.

Quoted 5 lines

Quoted 1 line

I will reword this description since it is not clear.

Shouldn't it use a different compatible value instead? Differentiation by properties is not recommended, as it's easy to miss a difference.

Quoted 5 lines

Either you have something in mind that I've not thought of, or I've done a bad job of explaining again. The buffer/"shadow" registers are synthesised on a per channel basis, so any combination of the 16 channels may have this capability. The same applies to the DAC mode, per channel there too.

Quoted 5 lines

Oops, hadn't noticed this is per channel. Indeed, then a different compatible value is futile.
So since "microchip,sync-update" is a bitmask, perhaps it should be called "microchip,sync-update-mask"?
Same for "microchip,dac-mode" -> "microchip,dac-mode-mask"?

Adding -mask sounds good to me.

Quoted 3 lines

Also, using different integer sizes than uint32 is frowned upon, unless there is a very good reason to do so. I can imagine a future version would support more channels, and then uint16 becomes a limitation.

Sure, uint32 it is.

Quoted 1 line

For both: Rob?

Both of these properties fall under the "DO attempt to make bindings complete even if a driver doesn’t support some features" category, so I am perfectly happy to change these properties to whatever is convention (or ultimately drop them for the sake of the remainder of the series).

Thanks,
Conor.

From: Conor Dooley <[email protected]>

This series updates the Microchip Icicle Kit device tree by adding a host of peripherals, and some updates to the memory map. In addition, the device tree has been split into a third part, which contains "soft" peripherals that are in the fpga fabric.

Several of the entries are for peripherals that have not get had their drivers upstreamed, so in those cases the dt bindings are included where appropriate in order to avoid the many "DT compatible string <x> appears un-documented" errors.

Depends on mpfs clock driver binding (on clk/next) to provide dt-bindings/clock/microchip,mpfs-clock.h
and on the other changes to the icicle/mpfs device tree from geert that are already in linux/riscv/for-next.

Additionally, the interrupt-extended warnings on the plic/clint are cleared by [1] & [2].

[1] https://lore.kernel.org/linux-riscv/[email protected]/
[2] https://lore.kernel.org/linux-riscv/[email protected]/

Changes from v4:
- dont include icicle_kit_defconfig, accidentally added in v3
- drop prescaler from mpfs-rtc & calculate the value instead
- use corei2c as a fallback device for mpfs-i2c
- drop spi dt-binding (on spi-next)
commit 2da187304e556ac59cf2dacb323cc78ded988169
- drop usb dt-binding (on usb-next)

Changes from v3:
- drop "mailbox: change mailbox-mpfs compatible string", already upstream:
commit f10b1fc0161cd99e ("mailbox: change mailbox-mpfs compatible string")
- fix copy paste error in microchip,mpfs-mailbox dt-binding
- remove whitespace in syscontroller dt entry

Changes from v2:
- dropped plic int header & corresponding defines in dts{,i}
- use $ref to drmode in mpfs-musb binding
- split changes to dts{,i} again: functional changes to existing elements now are in a new patch
- drop num-cs property in mpfs-spi binding
- dont make the system controller a simple-mfd
- move the separate bindings for rng/generic system services into the system controller binding
- added an instance corei2c as i2c2 in the fabric dtsi
- add version numbering to corepwm and corei2c compat string (-rtl-vN)

Conor Dooley (12):
dt-bindings: soc/microchip: update syscontroller compatibles
dt-bindings: soc/microchip: add services as children of sys ctrlr
dt-bindings: i2c: add bindings for microchip mpfs i2c
dt-bindings: rtc: add bindings for microchip mpfs rtc
dt-bindings: gpio: add bindings for microchip mpfs gpio
dt-bindings: pwm: add microchip corepwm binding
riscv: dts: microchip: use clk defines for icicle kit
riscv: dts: microchip: add fpga fabric section to icicle kit
riscv: dts: microchip: refactor icicle kit device tree
riscv: dts: microchip: update peripherals in icicle kit device tree
riscv: dts: microchip: add new peripherals to icicle kit device tree
MAINTAINERS: update riscv/microchip entry

 .../bindings/gpio/microchip,mpfs-gpio.yaml    |  80 ++++++
 .../bindings/i2c/microchip,mpfs-i2c.yaml      |  57 ++++
 ...ilbox.yaml => microchip,mpfs-mailbox.yaml} |   6 +-
 .../bindings/pwm/microchip,corepwm.yaml       |  75 +++++
 .../bindings/rtc/microchip,mfps-rtc.yaml      |  58 ++++
 .../microchip,mpfs-sys-controller.yaml        |  72 +++++
 ...icrochip,polarfire-soc-sys-controller.yaml |  35 ---
 MAINTAINERS                                   |   2 +
 .../dts/microchip/microchip-mpfs-fabric.dtsi  |  25 ++
 .../microchip/microchip-mpfs-icicle-kit.dts   | 115 ++++++--
 .../boot/dts/microchip/microchip-mpfs.dtsi    | 262 +++++++++++++++---
 11 files changed, 683 insertions(+), 104 deletions(-)

 create mode 100644 Documentation/devicetree/bindings/gpio/microchip,mpfs-gpio.yaml
 create mode 100644 Documentation/devicetree/bindings/i2c/microchip,mpfs-i2c.yaml
 rename Documentation/devicetree/bindings/mailbox/{microchip,polarfire-soc-mailbox.yaml => microchip,mpfs-mailbox.yaml} (82%)
 create mode 100644 Documentation/devicetree/bindings/pwm/microchip,corepwm.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-sys-controller.yaml
 delete mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,polarfire-soc-sys-controller.yaml
 create mode 100644 arch/riscv/boot/dts/microchip/microchip-mpfs-fabric.dtsi

Quoted 25 lines · On Mon, 31 Jan 2022 03:47:15 PST (-0800), [email protected] wrote:

From: Conor Dooley <[email protected]>

This series updates the Microchip Icicle Kit device tree by adding a host of peripherals, and some updates to the memory map. In addition, the device tree has been split into a third part, which contains "soft" peripherals that are in the fpga fabric.

Several of the entries are for peripherals that have not get had their drivers upstreamed, so in those cases the dt bindings are included where appropriate in order to avoid the many "DT compatible string <x> appears un-documented" errors.

Depends on mpfs clock driver binding (on clk/next) to provide dt-bindings/clock/microchip,mpfs-clock.h
and on the other changes to the icicle/mpfs device tree from geert that are already in linux/riscv/for-next.

Additionally, the interrupt-extended warnings on the plic/clint are cleared by [1] & [2].

[1] https://lore.kernel.org/linux-riscv/[email protected]/

[2] https://lore.kernel.org/linux-riscv/[email protected]/

Looks like Rob still has some feedback that still needs to be addressed.

I'm happy to take these via the RISC-V tree when the bindings are set

(assuming the DTs match whatever gets agreed upons), but also fine if someone else wants to take it > Acked-by: Palmer Dabbelt <[email protected]>

Either way, I'm going to drop this (and the v4, which was at the top of my inbox) as it looks like there'll be at least a v6.