* [PATCH v3 net-next 1/8] dt-bindings: dpll: Add device tree bindings for DPLL device and pin
2025-04-16 16:21 [PATCH v3 net-next 00/10] Add Microchip ZL3073x support (part 1) Ivan Vecera
@ 2025-04-16 16:21 ` Ivan Vecera
2025-04-21 22:20 ` Rob Herring
2025-04-16 16:21 ` [PATCH v3 net-next 2/8] dt-bindings: dpll: Add support for Microchip Azurite chip family Ivan Vecera
` (6 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 16:21 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
Add a common DT schema for DPLL device and associated pin.
The DPLL (device phase-locked loop) is a device used for precise clock
synchronization in networking and telecom hardware.
The device itself is equipped with one or more DPLLs (channels) and
one or more physical input and output pins.
Each DPLL channel is used either to provide pulse-per-clock signal or
to drive ethernet equipment clock.
The input and output pins have a label (specifies board label),
type (specifies its usage depending on wiring), list of supported
or allowed frequencies (depending on how the pin is connected and
where) and can support embedded sync capability.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
v1->v3:
* rewritten description for both device and pin
* dropped num-dplls property
* supported-frequencies property renamed to supported-frequencies-hz
---
.../devicetree/bindings/dpll/dpll-device.yaml | 76 +++++++++++++++++++
.../devicetree/bindings/dpll/dpll-pin.yaml | 44 +++++++++++
MAINTAINERS | 2 +
3 files changed, 122 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml
create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml
diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
new file mode 100644
index 0000000000000..11a02b74e28b7
--- /dev/null
+++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dpll/dpll-device.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Digital Phase-Locked Loop (DPLL) Device
+
+maintainers:
+ - Ivan Vecera <ivecera@redhat.com>
+
+description:
+ Digital Phase-Locked Loop (DPLL) device is used for precise clock
+ synchronization in networking and telecom hardware. The device can
+ have one or more channels (DPLLs) and one or more physical input and
+ output pins. Each DPLL channel can either produce pulse-per-clock signal
+ or drive ethernet equipment clock. The type of each channel can be
+ indicated by dpll-types property.
+
+properties:
+ $nodename:
+ pattern: "^dpll(@.*)?$"
+
+ "#address-cells":
+ const: 0
+
+ "#size-cells":
+ const: 0
+
+ dpll-types:
+ description: List of DPLL channel types, one per DPLL instance.
+ $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+ items:
+ enum: [pps, eec]
+
+ input-pins:
+ type: object
+ description: DPLL input pins
+ unevaluatedProperties: false
+
+ properties:
+ "#address-cells":
+ const: 1
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ "^pin@[0-9]+$":
+ $ref: /schemas/dpll/dpll-pin.yaml
+ unevaluatedProperties: false
+
+ required:
+ - "#address-cells"
+ - "#size-cells"
+
+ output-pins:
+ type: object
+ description: DPLL output pins
+ unevaluatedProperties: false
+
+ properties:
+ "#address-cells":
+ const: 1
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ "^pin@[0-9]+$":
+ $ref: /schemas/dpll/dpll-pin.yaml
+ unevaluatedProperties: false
+
+ required:
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: true
diff --git a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
new file mode 100644
index 0000000000000..44af3a4398a5f
--- /dev/null
+++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dpll/dpll-pin.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: DPLL Pin
+
+maintainers:
+ - Ivan Vecera <ivecera@redhat.com>
+
+description: |
+ The DPLL pin is either a physical input or output pin that is provided
+ by a DPLL( Digital Phase-Locked Loop) device. The pin is identified by
+ its physical order number that is stored in reg property and can have
+ an additional set of properties like supported (allowed) frequencies,
+ label, type and may support embedded sync.
+ Note that the pin in this context has nothing to do with pinctrl.
+
+properties:
+ reg:
+ description: Hardware index of the DPLL pin.
+ $ref: /schemas/types.yaml#/definitions/uint32
+
+ esync-control:
+ description: Indicates whether the pin supports embedded sync functionality.
+ type: boolean
+
+ label:
+ description: String exposed as the pin board label
+ $ref: /schemas/types.yaml#/definitions/string
+
+ supported-frequencies-hz:
+ description: List of supported frequencies for this pin, expressed in Hz.
+
+ type:
+ description: Type of the pin
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ext, gnss, int, mux, synce]
+
+required:
+ - reg
+
+additionalProperties: false
diff --git a/MAINTAINERS b/MAINTAINERS
index 1248443035f43..f645ef38d2224 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7187,6 +7187,8 @@ M: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
M: Jiri Pirko <jiri@resnulli.us>
L: netdev@vger.kernel.org
S: Supported
+F: Documentation/devicetree/bindings/dpll/dpll-device.yaml
+F: Documentation/devicetree/bindings/dpll/dpll-pin.yaml
F: Documentation/driver-api/dpll.rst
F: drivers/dpll/*
F: include/linux/dpll.h
--
2.48.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 1/8] dt-bindings: dpll: Add device tree bindings for DPLL device and pin
2025-04-16 16:21 ` [PATCH v3 net-next 1/8] dt-bindings: dpll: Add device tree bindings for DPLL device and pin Ivan Vecera
@ 2025-04-21 22:20 ` Rob Herring
2025-04-21 22:29 ` Rob Herring
0 siblings, 1 reply; 39+ messages in thread
From: Rob Herring @ 2025-04-21 22:20 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
On Wed, Apr 16, 2025 at 06:21:37PM +0200, Ivan Vecera wrote:
> Add a common DT schema for DPLL device and associated pin.
> The DPLL (device phase-locked loop) is a device used for precise clock
> synchronization in networking and telecom hardware.
In the subject, drop 'device tree binding for'. You already said that
with 'dt-bindings'.
>
> The device itself is equipped with one or more DPLLs (channels) and
> one or more physical input and output pins.
>
> Each DPLL channel is used either to provide pulse-per-clock signal or
> to drive ethernet equipment clock.
>
> The input and output pins have a label (specifies board label),
> type (specifies its usage depending on wiring), list of supported
> or allowed frequencies (depending on how the pin is connected and
> where) and can support embedded sync capability.
Convince me this is something generic... Some example parts or
datasheets would help. For example, wouldn't these devices have 1 or
more power supplies or a reset line?
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> v1->v3:
> * rewritten description for both device and pin
> * dropped num-dplls property
> * supported-frequencies property renamed to supported-frequencies-hz
> ---
> .../devicetree/bindings/dpll/dpll-device.yaml | 76 +++++++++++++++++++
> .../devicetree/bindings/dpll/dpll-pin.yaml | 44 +++++++++++
> MAINTAINERS | 2 +
> 3 files changed, 122 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml
> create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml
>
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> new file mode 100644
> index 0000000000000..11a02b74e28b7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dpll/dpll-device.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Digital Phase-Locked Loop (DPLL) Device
> +
> +maintainers:
> + - Ivan Vecera <ivecera@redhat.com>
> +
> +description:
> + Digital Phase-Locked Loop (DPLL) device is used for precise clock
> + synchronization in networking and telecom hardware. The device can
> + have one or more channels (DPLLs) and one or more physical input and
> + output pins. Each DPLL channel can either produce pulse-per-clock signal
> + or drive ethernet equipment clock. The type of each channel can be
> + indicated by dpll-types property.
> +
> +properties:
> + $nodename:
> + pattern: "^dpll(@.*)?$"
There's no 'reg' property, so you can't ever have a unit-address. I
suppose you can have more than 1, so you need a '-[0-9]+' suffix.
> +
> + "#address-cells":
> + const: 0
> +
> + "#size-cells":
> + const: 0
> +
> + dpll-types:
> + description: List of DPLL channel types, one per DPLL instance.
> + $ref: /schemas/types.yaml#/definitions/non-unique-string-array
> + items:
> + enum: [pps, eec]
> +
> + input-pins:
> + type: object
> + description: DPLL input pins
> + unevaluatedProperties: false
> +
> + properties:
> + "#address-cells":
> + const: 1
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + "^pin@[0-9]+$":
Unit-addresses are generally hex.
> + $ref: /schemas/dpll/dpll-pin.yaml
> + unevaluatedProperties: false
> +
> + required:
> + - "#address-cells"
> + - "#size-cells"
> +
> + output-pins:
> + type: object
> + description: DPLL output pins
> + unevaluatedProperties: false
> +
> + properties:
> + "#address-cells":
> + const: 1
> + "#size-cells":
> + const: 0
> +
> + patternProperties:
> + "^pin@[0-9]+$":
> + $ref: /schemas/dpll/dpll-pin.yaml
> + unevaluatedProperties: false
> +
> + required:
> + - "#address-cells"
> + - "#size-cells"
> +
> +additionalProperties: true
> diff --git a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> new file mode 100644
> index 0000000000000..44af3a4398a5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/dpll/dpll-pin.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: DPLL Pin
> +
> +maintainers:
> + - Ivan Vecera <ivecera@redhat.com>
> +
> +description: |
> + The DPLL pin is either a physical input or output pin that is provided
> + by a DPLL( Digital Phase-Locked Loop) device. The pin is identified by
> + its physical order number that is stored in reg property and can have
> + an additional set of properties like supported (allowed) frequencies,
> + label, type and may support embedded sync.
blank line here if this is a separate paragraph:
> + Note that the pin in this context has nothing to do with pinctrl.
> +
> +properties:
> + reg:
> + description: Hardware index of the DPLL pin.
> + $ref: /schemas/types.yaml#/definitions/uint32
'reg' already has a type. You need to say how many entries (i.e.
'maxItems: 1')
> +
> + esync-control:
> + description: Indicates whether the pin supports embedded sync functionality.
> + type: boolean
> +
> + label:
> + description: String exposed as the pin board label
> + $ref: /schemas/types.yaml#/definitions/string
> +
> + supported-frequencies-hz:
> + description: List of supported frequencies for this pin, expressed in Hz.
> +
> + type:
'type' is too generic of a property name.
> + description: Type of the pin
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [ext, gnss, int, mux, synce]
> +
> +required:
> + - reg
> +
> +additionalProperties: false
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1248443035f43..f645ef38d2224 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7187,6 +7187,8 @@ M: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> M: Jiri Pirko <jiri@resnulli.us>
> L: netdev@vger.kernel.org
> S: Supported
> +F: Documentation/devicetree/bindings/dpll/dpll-device.yaml
> +F: Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> F: Documentation/driver-api/dpll.rst
> F: drivers/dpll/*
> F: include/linux/dpll.h
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 net-next 1/8] dt-bindings: dpll: Add device tree bindings for DPLL device and pin
2025-04-21 22:20 ` Rob Herring
@ 2025-04-21 22:29 ` Rob Herring
0 siblings, 0 replies; 39+ messages in thread
From: Rob Herring @ 2025-04-21 22:29 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
On Mon, Apr 21, 2025 at 5:20 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Apr 16, 2025 at 06:21:37PM +0200, Ivan Vecera wrote:
> > Add a common DT schema for DPLL device and associated pin.
> > The DPLL (device phase-locked loop) is a device used for precise clock
> > synchronization in networking and telecom hardware.
>
> In the subject, drop 'device tree binding for'. You already said that
> with 'dt-bindings'.
>
> >
> > The device itself is equipped with one or more DPLLs (channels) and
> > one or more physical input and output pins.
> >
> > Each DPLL channel is used either to provide pulse-per-clock signal or
> > to drive ethernet equipment clock.
> >
> > The input and output pins have a label (specifies board label),
> > type (specifies its usage depending on wiring), list of supported
> > or allowed frequencies (depending on how the pin is connected and
> > where) and can support embedded sync capability.
>
> Convince me this is something generic... Some example parts or
> datasheets would help. For example, wouldn't these devices have 1 or
> more power supplies or a reset line?
Never mind, I read the next patch...
>
> >
> > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > ---
> > v1->v3:
> > * rewritten description for both device and pin
> > * dropped num-dplls property
> > * supported-frequencies property renamed to supported-frequencies-hz
> > ---
> > .../devicetree/bindings/dpll/dpll-device.yaml | 76 +++++++++++++++++++
> > .../devicetree/bindings/dpll/dpll-pin.yaml | 44 +++++++++++
> > MAINTAINERS | 2 +
> > 3 files changed, 122 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml
> > create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/dpll/dpll-device.yaml b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> > new file mode 100644
> > index 0000000000000..11a02b74e28b7
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dpll/dpll-device.yaml
> > @@ -0,0 +1,76 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/dpll/dpll-device.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Digital Phase-Locked Loop (DPLL) Device
> > +
> > +maintainers:
> > + - Ivan Vecera <ivecera@redhat.com>
> > +
> > +description:
> > + Digital Phase-Locked Loop (DPLL) device is used for precise clock
> > + synchronization in networking and telecom hardware. The device can
> > + have one or more channels (DPLLs) and one or more physical input and
> > + output pins. Each DPLL channel can either produce pulse-per-clock signal
> > + or drive ethernet equipment clock. The type of each channel can be
> > + indicated by dpll-types property.
> > +
> > +properties:
> > + $nodename:
> > + pattern: "^dpll(@.*)?$"
>
> There's no 'reg' property, so you can't ever have a unit-address. I
> suppose you can have more than 1, so you need a '-[0-9]+' suffix.
And forget this too.
Rob
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 net-next 2/8] dt-bindings: dpll: Add support for Microchip Azurite chip family
2025-04-16 16:21 [PATCH v3 net-next 00/10] Add Microchip ZL3073x support (part 1) Ivan Vecera
2025-04-16 16:21 ` [PATCH v3 net-next 1/8] dt-bindings: dpll: Add device tree bindings for DPLL device and pin Ivan Vecera
@ 2025-04-16 16:21 ` Ivan Vecera
2025-04-16 17:42 ` Rob Herring (Arm)
2025-04-16 16:21 ` [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support Ivan Vecera
` (5 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 16:21 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
Add DT bindings for Microchip Azurite DPLL chip family. These chips
provides up to 5 independent DPLL channels, 10 differential or
single-ended inputs and 10 differential or 20 single-ended outputs.
It can be connected via I2C or SPI busses.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
v1->v3:
* single file for both i2c & spi
* 5 compatibles for all supported chips from the family
---
.../bindings/dpll/microchip,zl30731.yaml | 115 ++++++++++++++++++
1 file changed, 115 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
new file mode 100644
index 0000000000000..cb1486d140382
--- /dev/null
+++ b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dpll/microchip,zl3073x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip Azurite DPLL device
+
+maintainers:
+ - Ivan Vecera <ivecera@redhat.com>
+
+description:
+ Microchip Azurite DPLL (ZL3073x) is a family of DPLL devices that
+ provides up to 5 independent DPLL channels, up to 10 differential or
+ single-ended inputs and 10 differential or 20 single-ended outputs.
+ These devices support both I2C and SPI interfaces.
+
+properties:
+ compatible:
+ enum:
+ - microchip,zl30731
+ - microchip,zl30732
+ - microchip,zl30733
+ - microchip,zl30734
+ - microchip,zl30735
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+allOf:
+ - $ref: /schemas/dpll/dpll-device.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dpll@70 {
+ compatible = "microchip,zl30732";
+ reg = <0x70>;
+ dpll-types = "pps", "eec";
+
+ input-pins {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pin@0 { /* REF0P */
+ reg = <0>;
+ label = "Input 0";
+ supported-frequencies = /bits/ 64 <1 1000>;
+ type = "ext";
+ };
+ };
+
+ output-pins {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pin@3 { /* OUT1N */
+ reg = <3>;
+ esync-control;
+ label = "Output 1";
+ supported-frequencies = /bits/ 64 <1 10000>;
+ type = "gnss";
+ };
+ };
+ };
+ };
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dpll@70 {
+ compatible = "microchip,zl30731";
+ reg = <0x70>;
+ spi-max-frequency = <12500000>;
+
+ dpll-types = "pps";
+
+ input-pins {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pin@0 { /* REF0P */
+ reg = <0>;
+ label = "Input 0";
+ supported-frequencies = /bits/ 64 <1 1000>;
+ type = "ext";
+ };
+ };
+
+ output-pins {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pin@3 { /* OUT1N */
+ reg = <3>;
+ esync-control;
+ label = "Output 1";
+ supported-frequencies = /bits/ 64 <1 10000>;
+ type = "gnss";
+ };
+ };
+ };
+ };
+...
--
2.48.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 2/8] dt-bindings: dpll: Add support for Microchip Azurite chip family
2025-04-16 16:21 ` [PATCH v3 net-next 2/8] dt-bindings: dpll: Add support for Microchip Azurite chip family Ivan Vecera
@ 2025-04-16 17:42 ` Rob Herring (Arm)
2025-04-16 18:29 ` Ivan Vecera
0 siblings, 1 reply; 39+ messages in thread
From: Rob Herring (Arm) @ 2025-04-16 17:42 UTC (permalink / raw)
To: Ivan Vecera
Cc: Krzysztof Kozlowski, netdev, Prathosh Satish, Vadim Fedorenko,
Kees Cook, Michal Schmidt, Conor Dooley, Arkadiusz Kubalewski,
linux-hardening, Andrew Morton, linux-kernel, Lee Jones,
Jiri Pirko, Andy Shevchenko, devicetree
On Wed, 16 Apr 2025 18:21:38 +0200, Ivan Vecera wrote:
> Add DT bindings for Microchip Azurite DPLL chip family. These chips
> provides up to 5 independent DPLL channels, 10 differential or
> single-ended inputs and 10 differential or 20 single-ended outputs.
> It can be connected via I2C or SPI busses.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> v1->v3:
> * single file for both i2c & spi
> * 5 compatibles for all supported chips from the family
> ---
> .../bindings/dpll/microchip,zl30731.yaml | 115 ++++++++++++++++++
> 1 file changed, 115 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
$id: http://devicetree.org/schemas/dpll/microchip,zl3073x.yaml
file: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250416162144.670760-3-ivecera@redhat.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 net-next 2/8] dt-bindings: dpll: Add support for Microchip Azurite chip family
2025-04-16 17:42 ` Rob Herring (Arm)
@ 2025-04-16 18:29 ` Ivan Vecera
2025-04-17 5:54 ` Krzysztof Kozlowski
0 siblings, 1 reply; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 18:29 UTC (permalink / raw)
To: Rob Herring (Arm)
Cc: Krzysztof Kozlowski, netdev, Prathosh Satish, Vadim Fedorenko,
Kees Cook, Michal Schmidt, Conor Dooley, Arkadiusz Kubalewski,
linux-hardening, Andrew Morton, linux-kernel, Lee Jones,
Jiri Pirko, Andy Shevchenko, devicetree
On Wed, Apr 16, 2025 at 7:42 PM Rob Herring (Arm) <robh@kernel.org> wrote:
>
>
> On Wed, 16 Apr 2025 18:21:38 +0200, Ivan Vecera wrote:
> > Add DT bindings for Microchip Azurite DPLL chip family. These chips
> > provides up to 5 independent DPLL channels, 10 differential or
> > single-ended inputs and 10 differential or 20 single-ended outputs.
> > It can be connected via I2C or SPI busses.
> >
> > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > ---
> > v1->v3:
> > * single file for both i2c & spi
> > * 5 compatibles for all supported chips from the family
> > ---
> > .../bindings/dpll/microchip,zl30731.yaml | 115 ++++++++++++++++++
> > 1 file changed, 115 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
> >
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
> $id: http://devicetree.org/schemas/dpll/microchip,zl3073x.yaml
Oops, my bad... I forgot to update $id after rename of the file...
Will fix.
Thanks,
Ivan
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 net-next 2/8] dt-bindings: dpll: Add support for Microchip Azurite chip family
2025-04-16 18:29 ` Ivan Vecera
@ 2025-04-17 5:54 ` Krzysztof Kozlowski
0 siblings, 0 replies; 39+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-17 5:54 UTC (permalink / raw)
To: Ivan Vecera
Cc: Rob Herring (Arm), Krzysztof Kozlowski, netdev, Prathosh Satish,
Vadim Fedorenko, Kees Cook, Michal Schmidt, Conor Dooley,
Arkadiusz Kubalewski, linux-hardening, Andrew Morton,
linux-kernel, Lee Jones, Jiri Pirko, Andy Shevchenko, devicetree
On Wed, Apr 16, 2025 at 08:29:33PM GMT, Ivan Vecera wrote:
> On Wed, Apr 16, 2025 at 7:42 PM Rob Herring (Arm) <robh@kernel.org> wrote:
> >
> >
> > On Wed, 16 Apr 2025 18:21:38 +0200, Ivan Vecera wrote:
> > > Add DT bindings for Microchip Azurite DPLL chip family. These chips
> > > provides up to 5 independent DPLL channels, 10 differential or
> > > single-ended inputs and 10 differential or 20 single-ended outputs.
> > > It can be connected via I2C or SPI busses.
> > >
> > > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > > ---
> > > v1->v3:
> > > * single file for both i2c & spi
> > > * 5 compatibles for all supported chips from the family
> > > ---
> > > .../bindings/dpll/microchip,zl30731.yaml | 115 ++++++++++++++++++
> > > 1 file changed, 115 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
> > >
> >
> > My bot found errors running 'make dt_binding_check' on your patch:
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml: $id: Cannot determine base path from $id, relative path/filename doesn't match actual path or filename
> > $id: http://devicetree.org/schemas/dpll/microchip,zl3073x.yaml
>
> Oops, my bad... I forgot to update $id after rename of the file...
> Will fix.
No, you forgot to test. You are expected to build your code (and this is
here testing) BEFORE you post, not after or not through community
resources.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support
2025-04-16 16:21 [PATCH v3 net-next 00/10] Add Microchip ZL3073x support (part 1) Ivan Vecera
2025-04-16 16:21 ` [PATCH v3 net-next 1/8] dt-bindings: dpll: Add device tree bindings for DPLL device and pin Ivan Vecera
2025-04-16 16:21 ` [PATCH v3 net-next 2/8] dt-bindings: dpll: Add support for Microchip Azurite chip family Ivan Vecera
@ 2025-04-16 16:21 ` Ivan Vecera
2025-04-16 17:11 ` Andrew Lunn
` (2 more replies)
2025-04-16 16:21 ` [PATCH v3 net-next 4/8] mfd: zl3073x: Add support for devlink device info Ivan Vecera
` (4 subsequent siblings)
7 siblings, 3 replies; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 16:21 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
Add base MFD driver for Microchip Azurite ZL3073x chip family.
These chips provide DPLL and PHC (PTP) functionality and they can
be connected over I2C or SPI bus.
The MFD driver provide basic communication and synchronization
over the bus and common functionality that are used by the DPLL
driver (in the part 2) and by the PTP driver (will be added later).
The chip family is characterized by following properties:
* up to 5 separate DPLL units (channels)
* 5 synthesizers
* 10 input pins (references)
* 10 outputs
* 20 output pins (output pin pair shares one output)
* Each reference and output can act in differential or single-ended
mode (reference or output in differential mode consumes 2 pins)
* Each output is connected to one of the synthesizers
* Each synthesizer is driven by one of the DPLL unit
The device uses 7-bit addresses and 8-bits for values. It exposes 8-, 16-,
32- and 48-bits registers in address range <0x000,0x77F>. Due to 7bit
addressing the range is organized into pages of size 128 and each page
contains page selector register (0x7F). To read/write multi-byte registers
the device supports bulk transfers.
There are 2 kinds of registers, simple ones that are present at register
pages 0..9 and mailbox ones that are present at register pages 10..14.
To access mailbox type registers a caller has to take mailbox_mutex that
ensures the reading and committing mailbox content is done atomically.
More information about it in later patch from the series.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
v2->v3:
* added chip_info with valid chip ids and num of DPLLs for compatibles
* regmap is using implicit locking
* mailbox registers requires extra mutex to be held
* added helpers to access registers
* report device firmware and config version using dev_dbg
* fixed regmap ranges
* enabled rbtree regcache for page selector
v1->v2:
* fixed header issues
* removed usage of of_match_ptr
* added check for devm_mutex_init
* removed commas after sentinels
* removed variable initialization in zl3073x_i2c_probe()
* moved device tables closer to their users
* renamed zl3073x_dev_alloc() to zl3073x_devm_alloc()
* removed empty zl3073x_dev_exit()
* spidev renamed to spi
* squashed together with device DT bindings
* used dev_err_probe() instead of dev_err() during probe
* added some function documentation
DT bindings:
* spliced to separate files for i2c and spi
* fixed property order in DT bindings' examples
* added description
---
MAINTAINERS | 9 ++
drivers/mfd/Kconfig | 30 +++++
drivers/mfd/Makefile | 5 +
drivers/mfd/zl3073x-core.c | 218 +++++++++++++++++++++++++++++++
drivers/mfd/zl3073x-i2c.c | 68 ++++++++++
drivers/mfd/zl3073x-spi.c | 68 ++++++++++
drivers/mfd/zl3073x.h | 31 +++++
include/linux/mfd/zl3073x.h | 50 +++++++
include/linux/mfd/zl3073x_regs.h | 105 +++++++++++++++
9 files changed, 584 insertions(+)
create mode 100644 drivers/mfd/zl3073x-core.c
create mode 100644 drivers/mfd/zl3073x-i2c.c
create mode 100644 drivers/mfd/zl3073x-spi.c
create mode 100644 drivers/mfd/zl3073x.h
create mode 100644 include/linux/mfd/zl3073x.h
create mode 100644 include/linux/mfd/zl3073x_regs.h
diff --git a/MAINTAINERS b/MAINTAINERS
index f645ef38d2224..cf050f59696e0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15992,6 +15992,15 @@ L: linux-wireless@vger.kernel.org
S: Supported
F: drivers/net/wireless/microchip/
+MICROCHIP ZL3073X DRIVER
+M: Ivan Vecera <ivecera@redhat.com>
+M: Prathosh Satish <Prathosh.Satish@microchip.com>
+L: netdev@vger.kernel.org
+S: Supported
+F: Documentation/devicetree/bindings/dpll/microchip,zl3073x*.yaml
+F: drivers/mfd/zl3073x*
+F: include/linux/mfd/zl3073x.h
+
MICROSEMI MIPS SOCS
M: Alexandre Belloni <alexandre.belloni@bootlin.com>
M: UNGLinuxDriver@microchip.com
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 22b9363100394..7d7902ec1d89a 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2422,5 +2422,35 @@ config MFD_UPBOARD_FPGA
To compile this driver as a module, choose M here: the module will be
called upboard-fpga.
+config MFD_ZL3073X_CORE
+ tristate
+ select MFD_CORE
+
+config MFD_ZL3073X_I2C
+ tristate "Microchip Azurite DPLL/PTP/SyncE with I2C"
+ depends on I2C
+ select MFD_ZL3073X_CORE
+ select REGMAP_I2C
+ help
+ Say Y here if you want to build I2C support for the Microchip
+ Azurite DPLL/PTP/SyncE chip family.
+
+ To compile this driver as a module, choose M here: the module
+ will be called zl3073x_i2c and you will also get zl3073x for
+ the core module.
+
+config MFD_ZL3073X_SPI
+ tristate "Microchip Azurite DPLL/PTP/SyncE with SPI"
+ depends on SPI
+ select MFD_ZL3073X_CORE
+ select REGMAP_SPI
+ help
+ Say Y here if you want to build SPI support for the Microchip
+ Azurite DPLL/PTP/SyncE chip family.
+
+ To compile this driver as a module, choose M here: the module
+ will be called zl3073x_spi and you will also get zl3073x for
+ the core module.
+
endmenu
endif
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 948cbdf42a18b..76e2babc1538f 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -290,3 +290,8 @@ obj-$(CONFIG_MFD_RSMU_I2C) += rsmu_i2c.o rsmu_core.o
obj-$(CONFIG_MFD_RSMU_SPI) += rsmu_spi.o rsmu_core.o
obj-$(CONFIG_MFD_UPBOARD_FPGA) += upboard-fpga.o
+
+zl3073x-y := zl3073x-core.o
+obj-$(CONFIG_MFD_ZL3073X_CORE) += zl3073x.o
+obj-$(CONFIG_MFD_ZL3073X_I2C) += zl3073x-i2c.o
+obj-$(CONFIG_MFD_ZL3073X_SPI) += zl3073x-spi.o
diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
new file mode 100644
index 0000000000000..0455d6ae37da5
--- /dev/null
+++ b/drivers/mfd/zl3073x-core.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/export.h>
+#include <linux/mfd/zl3073x.h>
+#include <linux/mfd/zl3073x_regs.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include "zl3073x.h"
+
+/* Chip IDs for zl30731 */
+static const u16 zl30731_ids[] = {
+ 0x0E93,
+ 0x1E93,
+ 0x2E93,
+};
+
+/* Chip IDs for zl30732 */
+static const u16 zl30732_ids[] = {
+ 0x0E30,
+ 0x0E94,
+ 0x1E94,
+ 0x1F60,
+ 0x2E94,
+ 0x3FC4,
+};
+
+/* Chip IDs for zl30733 */
+static const u16 zl30733_ids[] = {
+ 0x0E95,
+ 0x1E95,
+ 0x2E95,
+};
+
+/* Chip IDs for zl30734 */
+static const u16 zl30734_ids[] = {
+ 0x0E96,
+ 0x1E96,
+ 0x2E96,
+};
+
+/* Chip IDs for zl30735 */
+static const u16 zl30735_ids[] = {
+ 0x0E97,
+ 0x1E97,
+ 0x2E97,
+};
+
+const struct zl3073x_chip_info zl3073x_chip_info[] = {
+ [ZL30731] = {
+ .ids = zl30731_ids,
+ .num_ids = ARRAY_SIZE(zl30731_ids),
+ .num_channels = 1,
+ },
+ [ZL30732] = {
+ .ids = zl30732_ids,
+ .num_ids = ARRAY_SIZE(zl30732_ids),
+ .num_channels = 2,
+ },
+ [ZL30733] = {
+ .ids = zl30733_ids,
+ .num_ids = ARRAY_SIZE(zl30733_ids),
+ .num_channels = 3,
+ },
+ [ZL30734] = {
+ .ids = zl30734_ids,
+ .num_ids = ARRAY_SIZE(zl30734_ids),
+ .num_channels = 4,
+ },
+ [ZL30735] = {
+ .ids = zl30735_ids,
+ .num_ids = ARRAY_SIZE(zl30735_ids),
+ .num_channels = 5,
+ },
+};
+EXPORT_SYMBOL_NS_GPL(zl3073x_chip_info, "ZL3073X");
+
+#define ZL_NUM_PAGES 15
+#define ZL_NUM_SIMPLE_PAGES 10
+#define ZL_PAGE_SEL 0x7F
+#define ZL_PAGE_SEL_MASK GENMASK(3, 0)
+#define ZL_NUM_REGS (ZL_NUM_PAGES * ZL_PAGE_SIZE)
+
+/* Regmap range configuration */
+static const struct regmap_range_cfg zl3073x_regmap_range = {
+ .range_min = ZL_RANGE_OFF,
+ .range_max = ZL_RANGE_OFF + ZL_NUM_REGS - 1,
+ .selector_reg = ZL_PAGE_SEL,
+ .selector_mask = ZL_PAGE_SEL_MASK,
+ .selector_shift = 0,
+ .window_start = 0,
+ .window_len = ZL_PAGE_SIZE,
+};
+
+static bool
+zl3073x_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+ /* Only page selector is non-volatile */
+ return (reg != ZL_PAGE_SEL);
+}
+
+static const struct regmap_config zl3073x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = ZL_RANGE_OFF + ZL_NUM_REGS - 1,
+ .ranges = &zl3073x_regmap_range,
+ .num_ranges = 1,
+ .cache_type = REGCACHE_RBTREE,
+ .volatile_reg = zl3073x_is_volatile_reg,
+};
+
+/**
+ * zl3073x_devm_alloc - allocates zl3073x device structure
+ * @dev: pointer to device structure
+ *
+ * Allocates zl3073x device structure as device resource and initializes
+ * regmap_mutex.
+ *
+ * Return: pointer to zl3073x device on success, error pointer on error
+ */
+struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev)
+{
+ struct zl3073x_dev *zldev;
+ int rc;
+
+ zldev = devm_kzalloc(dev, sizeof(*zldev), GFP_KERNEL);
+ if (!zldev)
+ return ERR_PTR(-ENOMEM);
+
+ zldev->dev = dev;
+
+ /* We have to initialize regmap mutex here because during
+ * zl3073x_dev_probe() is too late as the regmaps are already
+ * initialized.
+ */
+ rc = devm_mutex_init(zldev->dev, &zldev->mailbox_lock);
+ if (rc) {
+ dev_err_probe(zldev->dev, rc, "Failed to initialize mutex\n");
+ return ERR_PTR(rc);
+ }
+
+ return zldev;
+}
+EXPORT_SYMBOL_NS_GPL(zl3073x_devm_alloc, "ZL3073X");
+
+/**
+ * zl3073x_dev_init_regmap_config - initialize regmap config
+ * @regmap_cfg: regmap_config structure to fill
+ *
+ * Initializes regmap config common for I2C and SPI.
+ */
+void zl3073x_dev_init_regmap_config(struct regmap_config *regmap_cfg)
+{
+ *regmap_cfg = zl3073x_regmap_config;
+}
+EXPORT_SYMBOL_NS_GPL(zl3073x_dev_init_regmap_config, "ZL3073X");
+
+/**
+ * zl3073x_dev_probe - initialize zl3073x device
+ * @zldev: pointer to zl3073x device
+ * @chip_info: chip info based on compatible
+ *
+ * Common initialization of zl3073x device structure.
+ *
+ * Returns: 0 on success, <0 on error
+ */
+int zl3073x_dev_probe(struct zl3073x_dev *zldev,
+ const struct zl3073x_chip_info *chip_info)
+{
+ u16 id, revision, fw_ver;
+ u32 cfg_ver;
+ int i, rc;
+
+ /* Read chip ID */
+ rc = zl3073x_read_id(zldev, &id);
+ if (rc)
+ return rc;
+
+ /* Check it matches */
+ for (i = 0; i < chip_info->num_ids; i++) {
+ if (id == chip_info->ids[i])
+ break;
+ }
+
+ if (i == chip_info->num_ids) {
+ dev_err(zldev->dev, "Unknown or non-match chip ID: 0x%0x\n", id);
+ return -ENODEV;
+ }
+
+ /* Read revision, firmware version and custom config version */
+ rc = zl3073x_read_revision(zldev, &revision);
+ if (rc)
+ return rc;
+ rc = zl3073x_read_fw_ver(zldev, &fw_ver);
+ if (rc)
+ return rc;
+ rc = zl3073x_read_custom_config_ver(zldev, &cfg_ver);
+ if (rc)
+ return rc;
+
+ dev_dbg(zldev->dev, "ChipID(%X), ChipRev(%X), FwVer(%u)\n", id,
+ revision, fw_ver);
+ dev_dbg(zldev->dev, "Custom config version: %lu.%lu.%lu.%lu\n",
+ FIELD_GET(GENMASK(31, 24), cfg_ver),
+ FIELD_GET(GENMASK(23, 16), cfg_ver),
+ FIELD_GET(GENMASK(15, 8), cfg_ver),
+ FIELD_GET(GENMASK(7, 0), cfg_ver));
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(zl3073x_dev_probe, "ZL3073X");
+
+MODULE_AUTHOR("Ivan Vecera <ivecera@redhat.com>");
+MODULE_DESCRIPTION("Microchip ZL3073x core driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/zl3073x-i2c.c b/drivers/mfd/zl3073x-i2c.c
new file mode 100644
index 0000000000000..76bc9a0463180
--- /dev/null
+++ b/drivers/mfd/zl3073x-i2c.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/device.h>
+#include <linux/dev_printk.h>
+#include <linux/i2c.h>
+#include <linux/mfd/zl3073x.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include "zl3073x.h"
+
+static int zl3073x_i2c_probe(struct i2c_client *client)
+{
+ struct regmap_config regmap_cfg;
+ struct device *dev = &client->dev;
+ struct zl3073x_dev *zldev;
+
+ zldev = zl3073x_devm_alloc(dev);
+ if (IS_ERR(zldev))
+ return PTR_ERR(zldev);
+
+ i2c_set_clientdata(client, zldev);
+
+ zl3073x_dev_init_regmap_config(®map_cfg);
+
+ zldev->regmap = devm_regmap_init_i2c(client, ®map_cfg);
+ if (IS_ERR(zldev->regmap)) {
+ dev_err_probe(dev, PTR_ERR(zldev->regmap),
+ "Failed to initialize regmap\n");
+ return PTR_ERR(zldev->regmap);
+ }
+
+ return zl3073x_dev_probe(zldev, i2c_get_match_data(client));
+}
+
+static const struct i2c_device_id zl3073x_i2c_id[] = {
+ { "zl30731", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30731] },
+ { "zl30732", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30732] },
+ { "zl30733", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30733] },
+ { "zl30734", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30734] },
+ { "zl30735", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30735] },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, zl3073x_i2c_id);
+
+static const struct of_device_id zl3073x_i2c_of_match[] = {
+ { .compatible = "microchip,zl30731", .data = &zl3073x_chip_info[ZL30731] },
+ { .compatible = "microchip,zl30732", .data = &zl3073x_chip_info[ZL30732] },
+ { .compatible = "microchip,zl30733", .data = &zl3073x_chip_info[ZL30733] },
+ { .compatible = "microchip,zl30734", .data = &zl3073x_chip_info[ZL30734] },
+ { .compatible = "microchip,zl30735", .data = &zl3073x_chip_info[ZL30735] },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, zl3073x_i2c_of_match);
+
+static struct i2c_driver zl3073x_i2c_driver = {
+ .driver = {
+ .name = "zl3073x-i2c",
+ .of_match_table = zl3073x_i2c_of_match,
+ },
+ .probe = zl3073x_i2c_probe,
+ .id_table = zl3073x_i2c_id,
+};
+module_i2c_driver(zl3073x_i2c_driver);
+
+MODULE_AUTHOR("Ivan Vecera <ivecera@redhat.com>");
+MODULE_DESCRIPTION("Microchip ZL3073x I2C driver");
+MODULE_IMPORT_NS("ZL3073X");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/zl3073x-spi.c b/drivers/mfd/zl3073x-spi.c
new file mode 100644
index 0000000000000..d0fc2d2221c0d
--- /dev/null
+++ b/drivers/mfd/zl3073x-spi.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/device.h>
+#include <linux/dev_printk.h>
+#include <linux/mfd/zl3073x.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include "zl3073x.h"
+
+static int zl3073x_spi_probe(struct spi_device *spi)
+{
+ struct regmap_config regmap_cfg;
+ struct device *dev = &spi->dev;
+ struct zl3073x_dev *zldev;
+
+ zldev = zl3073x_devm_alloc(dev);
+ if (IS_ERR(zldev))
+ return PTR_ERR(zldev);
+
+ spi_set_drvdata(spi, zldev);
+
+ zl3073x_dev_init_regmap_config(®map_cfg);
+
+ zldev->regmap = devm_regmap_init_spi(spi, ®map_cfg);
+ if (IS_ERR(zldev->regmap)) {
+ dev_err_probe(dev, PTR_ERR(zldev->regmap),
+ "Failed to initialize regmap\n");
+ return PTR_ERR(zldev->regmap);
+ }
+
+ return zl3073x_dev_probe(zldev, spi_get_device_match_data(spi));
+}
+
+static const struct spi_device_id zl3073x_spi_id[] = {
+ { "zl30731", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30731] },
+ { "zl30731", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30732] },
+ { "zl30731", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30733] },
+ { "zl30731", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30734] },
+ { "zl30731", .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30735] },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(spi, zl3073x_spi_id);
+
+static const struct of_device_id zl3073x_spi_of_match[] = {
+ { .compatible = "microchip,zl30731", .data = &zl3073x_chip_info[ZL30731] },
+ { .compatible = "microchip,zl30732", .data = &zl3073x_chip_info[ZL30732] },
+ { .compatible = "microchip,zl30733", .data = &zl3073x_chip_info[ZL30733] },
+ { .compatible = "microchip,zl30734", .data = &zl3073x_chip_info[ZL30734] },
+ { .compatible = "microchip,zl30735", .data = &zl3073x_chip_info[ZL30735] },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, zl3073x_spi_of_match);
+
+static struct spi_driver zl3073x_spi_driver = {
+ .driver = {
+ .name = "zl3073x-spi",
+ .of_match_table = zl3073x_spi_of_match,
+ },
+ .probe = zl3073x_spi_probe,
+ .id_table = zl3073x_spi_id,
+};
+module_spi_driver(zl3073x_spi_driver);
+
+MODULE_AUTHOR("Ivan Vecera <ivecera@redhat.com>");
+MODULE_DESCRIPTION("Microchip ZL3073x SPI driver");
+MODULE_IMPORT_NS("ZL3073X");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/zl3073x.h b/drivers/mfd/zl3073x.h
new file mode 100644
index 0000000000000..3a2fea61cf579
--- /dev/null
+++ b/drivers/mfd/zl3073x.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ZL3073X_CORE_H
+#define __ZL3073X_CORE_H
+
+struct device;
+struct regmap_config;
+struct zl3073x_dev;
+
+enum zl3073x_chip_type {
+ ZL30731,
+ ZL30732,
+ ZL30733,
+ ZL30734,
+ ZL30735,
+};
+
+struct zl3073x_chip_info {
+ const u16 *ids;
+ size_t num_ids;
+ int num_channels;
+};
+
+extern const struct zl3073x_chip_info zl3073x_chip_info[];
+
+struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev);
+void zl3073x_dev_init_regmap_config(struct regmap_config *regmap_cfg);
+int zl3073x_dev_probe(struct zl3073x_dev *zldev,
+ const struct zl3073x_chip_info *chip_info);
+
+#endif /* __ZL3073X_CORE_H */
diff --git a/include/linux/mfd/zl3073x.h b/include/linux/mfd/zl3073x.h
new file mode 100644
index 0000000000000..b68481dcf77a5
--- /dev/null
+++ b/include/linux/mfd/zl3073x.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __LINUX_MFD_ZL3073X_H
+#define __LINUX_MFD_ZL3073X_H
+
+#include <linux/mutex.h>
+
+struct device;
+struct regmap;
+
+/**
+ * struct zl3073x_dev - zl3073x device
+ * @dev: pointer to device
+ * @regmap: regmap to access device registers
+ * @mailbox_lock: mutex protecting an access to mailbox registers
+ */
+struct zl3073x_dev {
+ struct device *dev;
+ struct regmap *regmap;
+ struct mutex mailbox_lock;
+};
+
+/**
+ * zl3073x_mailbox_lock - Lock the device mailbox registers
+ * @zldev: zl3073x device pointer
+ *
+ * Caller has to held this lock when it needs to access device mailbox
+ * registers.
+ */
+static inline void zl3073x_mailbox_lock(struct zl3073x_dev *zldev)
+{
+ mutex_lock(&zldev->mailbox_lock);
+}
+
+/**
+ * zl3073x_mailbox_unlock - Unlock the device mailbox registers
+ * @zldev: zl3073x device pointer
+ *
+ * Caller has to unlock this lock when it finishes accessing device mailbox
+ * registers.
+ */
+static inline void zl3073x_mailbox_unlock(struct zl3073x_dev *zldev)
+{
+ mutex_unlock(&zldev->mailbox_lock);
+}
+
+DEFINE_GUARD(zl3073x_mailbox, struct zl3073x_dev *, zl3073x_mailbox_lock(_T),
+ zl3073x_mailbox_unlock(_T));
+
+#endif /* __LINUX_MFD_ZL3073X_H */
diff --git a/include/linux/mfd/zl3073x_regs.h b/include/linux/mfd/zl3073x_regs.h
new file mode 100644
index 0000000000000..453a5da8ac63f
--- /dev/null
+++ b/include/linux/mfd/zl3073x_regs.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __LINUX_MFD_ZL3073X_REGS_H
+#define __LINUX_MFD_ZL3073X_REGS_H
+
+#include <asm/byteorder.h>
+#include <linux/lockdep.h>
+#include <linux/mfd/zl3073x.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+/* Registers are mapped at offset 0x100 */
+#define ZL_RANGE_OFF 0x100
+#define ZL_PAGE_SIZE 0x80
+#define ZL_REG_ADDR(_pg, _off) (ZL_RANGE_OFF + (_pg) * ZL_PAGE_SIZE + (_off))
+
+/**************************
+ * Register Page 0, General
+ **************************/
+
+/*
+ * Register 'id'
+ * Page: 0, Offset: 0x01, Size: 16 bits
+ */
+#define ZL_REG_ID ZL_REG_ADDR(0, 0x01)
+
+static inline __maybe_unused int
+zl3073x_read_id(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_ID, &temp, sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+/*
+ * Register 'revision'
+ * Page: 0, Offset: 0x03, Size: 16 bits
+ */
+#define ZL_REG_REVISION ZL_REG_ADDR(0, 0x03)
+
+static inline __maybe_unused int
+zl3073x_read_revision(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_REVISION, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+/*
+ * Register 'fw_ver'
+ * Page: 0, Offset: 0x05, Size: 16 bits
+ */
+#define ZL_REG_FW_VER ZL_REG_ADDR(0, 0x05)
+
+static inline __maybe_unused int
+zl3073x_read_fw_ver(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_FW_VER, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+/*
+ * Register 'custom_config_ver'
+ * Page: 0, Offset: 0x07, Size: 32 bits
+ */
+#define ZL_REG_CUSTOM_CONFIG_VER ZL_REG_ADDR(0, 0x07)
+
+static inline __maybe_unused int
+zl3073x_read_custom_config_ver(struct zl3073x_dev *zldev, u32 *value)
+{
+ __be32 temp;
+ int rc;
+
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_CUSTOM_CONFIG_VER, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be32_to_cpu(temp);
+ return rc;
+}
+
+#endif /* __LINUX_MFD_ZL3073X_REGS_H */
--
2.48.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support
2025-04-16 16:21 ` [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support Ivan Vecera
@ 2025-04-16 17:11 ` Andrew Lunn
[not found] ` <CAAVpwAsw4-7n_iV=8aXp7=X82Mj7M-vGAc3f-fVbxxg0qgAQQA@mail.gmail.com>
2025-04-17 15:51 ` Andy Shevchenko
2025-04-17 15:57 ` Mark Brown
2 siblings, 1 reply; 39+ messages in thread
From: Andrew Lunn @ 2025-04-16 17:11 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
> +++ b/include/linux/mfd/zl3073x_regs.h
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __LINUX_MFD_ZL3073X_REGS_H
> +#define __LINUX_MFD_ZL3073X_REGS_H
> +
> +#include <asm/byteorder.h>
> +#include <linux/lockdep.h>
lockdep?
> +#include <linux/mfd/zl3073x.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +
> +/* Registers are mapped at offset 0x100 */
> +#define ZL_RANGE_OFF 0x100
> +#define ZL_PAGE_SIZE 0x80
> +#define ZL_REG_ADDR(_pg, _off) (ZL_RANGE_OFF + (_pg) * ZL_PAGE_SIZE + (_off))
> +
> +/**************************
> + * Register Page 0, General
> + **************************/
> +
> +/*
> + * Register 'id'
> + * Page: 0, Offset: 0x01, Size: 16 bits
> + */
> +#define ZL_REG_ID ZL_REG_ADDR(0, 0x01)
> +
> +static inline __maybe_unused int
> +zl3073x_read_id(struct zl3073x_dev *zldev, u16 *value)
> +{
> + __be16 temp;
> + int rc;
> +
> + rc = regmap_bulk_read(zldev->regmap, ZL_REG_ID, &temp, sizeof(temp));
> + if (rc)
> + return rc;
> +
> + *value = be16_to_cpu(temp);
> + return rc;
> +}
It seems odd these are inline functions in a header file.
> +
> +/*
> + * Register 'revision'
> + * Page: 0, Offset: 0x03, Size: 16 bits
> + */
> +#define ZL_REG_REVISION ZL_REG_ADDR(0, 0x03)
> +
> +static inline __maybe_unused int
> +zl3073x_read_revision(struct zl3073x_dev *zldev, u16 *value)
> +{
> + __be16 temp;
> + int rc;
> +
> + rc = regmap_bulk_read(zldev->regmap, ZL_REG_REVISION, &temp,
> + sizeof(temp));
> + if (rc)
> + return rc;
> +
> + *value = be16_to_cpu(temp);
> + return rc;
> +}
> +
> +/*
> + * Register 'fw_ver'
> + * Page: 0, Offset: 0x05, Size: 16 bits
> + */
> +#define ZL_REG_FW_VER ZL_REG_ADDR(0, 0x05)
> +
> +static inline __maybe_unused int
> +zl3073x_read_fw_ver(struct zl3073x_dev *zldev, u16 *value)
> +{
> + __be16 temp;
> + int rc;
> +
> + rc = regmap_bulk_read(zldev->regmap, ZL_REG_FW_VER, &temp,
> + sizeof(temp));
> + if (rc)
> + return rc;
> +
> + *value = be16_to_cpu(temp);
> + return rc;
> +}
Seems like it would make sense to add a zl3073x_read_b16() helper.
Then all these functions become one liners.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support
2025-04-16 16:21 ` [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support Ivan Vecera
2025-04-16 17:11 ` Andrew Lunn
@ 2025-04-17 15:51 ` Andy Shevchenko
2025-04-17 15:57 ` Mark Brown
2 siblings, 0 replies; 39+ messages in thread
From: Andy Shevchenko @ 2025-04-17 15:51 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andrew Morton, Michal Schmidt, devicetree,
linux-kernel, linux-hardening
On Wed, Apr 16, 2025 at 06:21:39PM +0200, Ivan Vecera wrote:
> Add base MFD driver for Microchip Azurite ZL3073x chip family.
> These chips provide DPLL and PHC (PTP) functionality and they can
> be connected over I2C or SPI bus.
>
> The MFD driver provide basic communication and synchronization
> over the bus and common functionality that are used by the DPLL
> driver (in the part 2) and by the PTP driver (will be added later).
>
> The chip family is characterized by following properties:
> * up to 5 separate DPLL units (channels)
> * 5 synthesizers
> * 10 input pins (references)
> * 10 outputs
> * 20 output pins (output pin pair shares one output)
> * Each reference and output can act in differential or single-ended
> mode (reference or output in differential mode consumes 2 pins)
> * Each output is connected to one of the synthesizers
> * Each synthesizer is driven by one of the DPLL unit
>
> The device uses 7-bit addresses and 8-bits for values. It exposes 8-, 16-,
> 32- and 48-bits registers in address range <0x000,0x77F>. Due to 7bit
> addressing the range is organized into pages of size 128 and each page
> contains page selector register (0x7F). To read/write multi-byte registers
> the device supports bulk transfers.
>
> There are 2 kinds of registers, simple ones that are present at register
> pages 0..9 and mailbox ones that are present at register pages 10..14.
>
> To access mailbox type registers a caller has to take mailbox_mutex that
> ensures the reading and committing mailbox content is done atomically.
> More information about it in later patch from the series.
...
> +#define ZL_NUM_PAGES 15
> +#define ZL_NUM_SIMPLE_PAGES 10
> +#define ZL_PAGE_SEL 0x7F
> +#define ZL_PAGE_SEL_MASK GENMASK(3, 0)
> +#define ZL_NUM_REGS (ZL_NUM_PAGES * ZL_PAGE_SIZE)
> +
> +/* Regmap range configuration */
> +static const struct regmap_range_cfg zl3073x_regmap_range = {
> + .range_min = ZL_RANGE_OFF,
> + .range_max = ZL_RANGE_OFF + ZL_NUM_REGS - 1,
> + .selector_reg = ZL_PAGE_SEL,
> + .selector_mask = ZL_PAGE_SEL_MASK,
> + .selector_shift = 0,
> + .window_start = 0,
> + .window_len = ZL_PAGE_SIZE,
> +};
On the first glance this looks good now, thanks for addressing that.
...
> +static bool
> +zl3073x_is_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + /* Only page selector is non-volatile */
> + return (reg != ZL_PAGE_SEL);
Unneeded parentheses.
> +}
...
> +/**
> + * zl3073x_devm_alloc - allocates zl3073x device structure
> + * @dev: pointer to device structure
> + *
> + * Allocates zl3073x device structure as device resource and initializes
> + * regmap_mutex.
> + *
> + * Return: pointer to zl3073x device on success, error pointer on error
> + */
> +struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev)
> +{
> + struct zl3073x_dev *zldev;
> + int rc;
> +
> + zldev = devm_kzalloc(dev, sizeof(*zldev), GFP_KERNEL);
> + if (!zldev)
> + return ERR_PTR(-ENOMEM);
> +
> + zldev->dev = dev;
> +
> + /* We have to initialize regmap mutex here because during
> + * zl3073x_dev_probe() is too late as the regmaps are already
> + * initialized.
> + */
> + rc = devm_mutex_init(zldev->dev, &zldev->mailbox_lock);
> + if (rc) {
> + dev_err_probe(zldev->dev, rc, "Failed to initialize mutex\n");
> + return ERR_PTR(rc);
return dev_err_probe(...);
> + }
> +
> + return zldev;
> +}
...
> +int zl3073x_dev_probe(struct zl3073x_dev *zldev,
> + const struct zl3073x_chip_info *chip_info)
> +{
> + u16 id, revision, fw_ver;
> + u32 cfg_ver;
> + int i, rc;
> +
> + /* Read chip ID */
> + rc = zl3073x_read_id(zldev, &id);
> + if (rc)
> + return rc;
> +
> + /* Check it matches */
> + for (i = 0; i < chip_info->num_ids; i++) {
> + if (id == chip_info->ids[i])
> + break;
> + }
> +
> + if (i == chip_info->num_ids) {
> + dev_err(zldev->dev, "Unknown or non-match chip ID: 0x%0x\n", id);
> + return -ENODEV;
return dev_err_probe(...);
> + }
> +
> + /* Read revision, firmware version and custom config version */
> + rc = zl3073x_read_revision(zldev, &revision);
> + if (rc)
> + return rc;
> + rc = zl3073x_read_fw_ver(zldev, &fw_ver);
> + if (rc)
> + return rc;
> + rc = zl3073x_read_custom_config_ver(zldev, &cfg_ver);
> + if (rc)
> + return rc;
> +
> + dev_dbg(zldev->dev, "ChipID(%X), ChipRev(%X), FwVer(%u)\n", id,
> + revision, fw_ver);
> + dev_dbg(zldev->dev, "Custom config version: %lu.%lu.%lu.%lu\n",
> + FIELD_GET(GENMASK(31, 24), cfg_ver),
> + FIELD_GET(GENMASK(23, 16), cfg_ver),
> + FIELD_GET(GENMASK(15, 8), cfg_ver),
> + FIELD_GET(GENMASK(7, 0), cfg_ver));
> +
> + return 0;
> +}
...
> +#include <linux/device.h>
Is it used?
> +#include <linux/dev_printk.h>
+ err.h
> +#include <linux/i2c.h>
> +#include <linux/mfd/zl3073x.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
...
> +static int zl3073x_i2c_probe(struct i2c_client *client)
> +{
> + struct regmap_config regmap_cfg;
> + struct device *dev = &client->dev;
> + struct zl3073x_dev *zldev;
> +
> + zldev = zl3073x_devm_alloc(dev);
> + if (IS_ERR(zldev))
> + return PTR_ERR(zldev);
> + i2c_set_clientdata(client, zldev);
Is it used anywhere?
> + zl3073x_dev_init_regmap_config(®map_cfg);
> +
> + zldev->regmap = devm_regmap_init_i2c(client, ®map_cfg);
> + if (IS_ERR(zldev->regmap)) {
> + dev_err_probe(dev, PTR_ERR(zldev->regmap),
> + "Failed to initialize regmap\n");
return dev_err_probe(...);
> + return PTR_ERR(zldev->regmap);
> + }
> +
> + return zl3073x_dev_probe(zldev, i2c_get_match_data(client));
> +}
...
> +++ b/drivers/mfd/zl3073x-spi.c
Same comments as per i2c part.
...
> +static inline void zl3073x_mailbox_lock(struct zl3073x_dev *zldev)
> +{
> + mutex_lock(&zldev->mailbox_lock);
Do you need sparse annotations? (build with `make C=1 ...` to check)
> +}
> +static inline void zl3073x_mailbox_unlock(struct zl3073x_dev *zldev)
> +{
> + mutex_unlock(&zldev->mailbox_lock);
> +}
> +
> +DEFINE_GUARD(zl3073x_mailbox, struct zl3073x_dev *, zl3073x_mailbox_lock(_T),
> + zl3073x_mailbox_unlock(_T));
Seems to be they are useless as you share the lock. One may use
guard(nutex)(...) directly.
> +#endif /* __LINUX_MFD_ZL3073X_H */
...
> +#include <asm/byteorder.h>
asm/* usually goes after linux/* as they are more specific and linux/* are more
generic.
> +#include <linux/lockdep.h>
> +#include <linux/mfd/zl3073x.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
...
> +static inline __maybe_unused int
Why __maybe_unused? Please, get rid of those.
> +zl3073x_read_id(struct zl3073x_dev *zldev, u16 *value)
> +{
> + __be16 temp;
> + int rc;
> +
> + rc = regmap_bulk_read(zldev->regmap, ZL_REG_ID, &temp, sizeof(temp));
> + if (rc)
> + return rc;
> +
> + *value = be16_to_cpu(temp);
> + return rc;
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support
2025-04-16 16:21 ` [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support Ivan Vecera
2025-04-16 17:11 ` Andrew Lunn
2025-04-17 15:51 ` Andy Shevchenko
@ 2025-04-17 15:57 ` Mark Brown
2 siblings, 0 replies; 39+ messages in thread
From: Mark Brown @ 2025-04-17 15:57 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
[-- Attachment #1: Type: text/plain, Size: 494 bytes --]
On Wed, Apr 16, 2025 at 06:21:39PM +0200, Ivan Vecera wrote:
> +static const struct regmap_config zl3073x_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = ZL_RANGE_OFF + ZL_NUM_REGS - 1,
> + .ranges = &zl3073x_regmap_range,
> + .num_ranges = 1,
> + .cache_type = REGCACHE_RBTREE,
Unless you have a specific reason to use something else you should
probably use _MAPLE these days, it's a more modern data structure and
makes choices more suited to current hardware.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 net-next 4/8] mfd: zl3073x: Add support for devlink device info
2025-04-16 16:21 [PATCH v3 net-next 00/10] Add Microchip ZL3073x support (part 1) Ivan Vecera
` (2 preceding siblings ...)
2025-04-16 16:21 ` [PATCH v3 net-next 3/8] mfd: Add Microchip ZL3073x support Ivan Vecera
@ 2025-04-16 16:21 ` Ivan Vecera
2025-04-17 15:53 ` Andy Shevchenko
2025-04-16 16:21 ` [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes Ivan Vecera
` (3 subsequent siblings)
7 siblings, 1 reply; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 16:21 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
Use devlink_alloc() to alloc zl3073x_dev structure, register the device
as a devlink device and add devlink callback to provide devlink device
info.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
v2->v3:
* merged devlink device allocation, registration and device info
callback
v1->v2:
- dependency on NET moved to MFD_ZL3073X_CORE in Kconfig
- devlink register managed way
---
drivers/mfd/Kconfig | 2 +
drivers/mfd/zl3073x-core.c | 106 ++++++++++++++++++++++++++++++++++++-
2 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 7d7902ec1d89a..e4eca15af175d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2424,6 +2424,8 @@ config MFD_UPBOARD_FPGA
config MFD_ZL3073X_CORE
tristate
+ depends on NET
+ select NET_DEVLINK
select MFD_CORE
config MFD_ZL3073X_I2C
diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
index 0455d6ae37da5..8a15237e0f731 100644
--- a/drivers/mfd/zl3073x-core.c
+++ b/drivers/mfd/zl3073x-core.c
@@ -8,7 +8,10 @@
#include <linux/mfd/zl3073x.h>
#include <linux/mfd/zl3073x_regs.h>
#include <linux/module.h>
+#include <linux/netlink.h>
#include <linux/regmap.h>
+#include <linux/sprintf.h>
+#include <net/devlink.h>
#include "zl3073x.h"
/* Chip IDs for zl30731 */
@@ -112,6 +115,83 @@ static const struct regmap_config zl3073x_regmap_config = {
.volatile_reg = zl3073x_is_volatile_reg,
};
+/**
+ * zl3073x_devlink_info_get - Devlink device info callback
+ * @devlink: devlink structure pointer
+ * @req: devlink request pointer to store information
+ * @extack: netlink extack pointer to report errors
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int zl3073x_devlink_info_get(struct devlink *devlink,
+ struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ struct zl3073x_dev *zldev = devlink_priv(devlink);
+ u16 id, revision, fw_ver;
+ char buf[16];
+ u32 cfg_ver;
+ int rc;
+
+ rc = zl3073x_read_id(zldev, &id);
+ if (rc)
+ return rc;
+
+ snprintf(buf, sizeof(buf), "%X", id);
+ rc = devlink_info_version_fixed_put(req,
+ DEVLINK_INFO_VERSION_GENERIC_ASIC_ID,
+ buf);
+ if (rc)
+ return rc;
+
+ rc = zl3073x_read_revision(zldev, &revision);
+ if (rc)
+ return rc;
+
+ snprintf(buf, sizeof(buf), "%X", revision);
+ rc = devlink_info_version_fixed_put(req,
+ DEVLINK_INFO_VERSION_GENERIC_ASIC_REV,
+ buf);
+ if (rc)
+ return rc;
+
+ rc = zl3073x_read_fw_ver(zldev, &fw_ver);
+ if (rc)
+ return rc;
+
+ snprintf(buf, sizeof(buf), "%u", fw_ver);
+ rc = devlink_info_version_fixed_put(req,
+ DEVLINK_INFO_VERSION_GENERIC_FW,
+ buf);
+ if (rc)
+ return rc;
+
+ rc = zl3073x_read_custom_config_ver(zldev, &cfg_ver);
+ if (rc)
+ return rc;
+
+ /* No custom config version */
+ if (cfg_ver == U32_MAX)
+ return 0;
+
+ snprintf(buf, sizeof(buf), "%lu.%lu.%lu.%lu",
+ FIELD_GET(GENMASK(31, 24), cfg_ver),
+ FIELD_GET(GENMASK(23, 16), cfg_ver),
+ FIELD_GET(GENMASK(15, 8), cfg_ver),
+ FIELD_GET(GENMASK(7, 0), cfg_ver));
+
+ return devlink_info_version_running_put(req, "cfg.custom_ver", buf);
+}
+
+static const struct devlink_ops zl3073x_devlink_ops = {
+ .info_get = zl3073x_devlink_info_get,
+};
+
+static void zl3073x_devlink_free(void *ptr)
+{
+ devlink_free(ptr);
+}
+
/**
* zl3073x_devm_alloc - allocates zl3073x device structure
* @dev: pointer to device structure
@@ -124,12 +204,18 @@ static const struct regmap_config zl3073x_regmap_config = {
struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev)
{
struct zl3073x_dev *zldev;
+ struct devlink *devlink;
int rc;
- zldev = devm_kzalloc(dev, sizeof(*zldev), GFP_KERNEL);
- if (!zldev)
+ devlink = devlink_alloc(&zl3073x_devlink_ops, sizeof(*zldev), dev);
+ if (!devlink)
return ERR_PTR(-ENOMEM);
+ /* Add devres action to free devlink device */
+ if (devm_add_action_or_reset(dev, zl3073x_devlink_free, devlink))
+ return ERR_PTR(-ENOMEM);
+
+ zldev = devlink_priv(devlink);
zldev->dev = dev;
/* We have to initialize regmap mutex here because during
@@ -158,6 +244,11 @@ void zl3073x_dev_init_regmap_config(struct regmap_config *regmap_cfg)
}
EXPORT_SYMBOL_NS_GPL(zl3073x_dev_init_regmap_config, "ZL3073X");
+static void zl3073x_devlink_unregister(void *ptr)
+{
+ devlink_unregister(ptr);
+}
+
/**
* zl3073x_dev_probe - initialize zl3073x device
* @zldev: pointer to zl3073x device
@@ -171,6 +262,7 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
const struct zl3073x_chip_info *chip_info)
{
u16 id, revision, fw_ver;
+ struct devlink *devlink;
u32 cfg_ver;
int i, rc;
@@ -209,6 +301,16 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
FIELD_GET(GENMASK(15, 8), cfg_ver),
FIELD_GET(GENMASK(7, 0), cfg_ver));
+ /* Register the device as devlink device */
+ devlink = priv_to_devlink(zldev);
+ devlink_register(devlink);
+
+ /* Add devres action to unregister devlink device */
+ rc = devm_add_action_or_reset(zldev->dev, zl3073x_devlink_unregister,
+ devlink);
+ if (rc)
+ return rc;
+
return 0;
}
EXPORT_SYMBOL_NS_GPL(zl3073x_dev_probe, "ZL3073X");
--
2.48.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 4/8] mfd: zl3073x: Add support for devlink device info
2025-04-16 16:21 ` [PATCH v3 net-next 4/8] mfd: zl3073x: Add support for devlink device info Ivan Vecera
@ 2025-04-17 15:53 ` Andy Shevchenko
0 siblings, 0 replies; 39+ messages in thread
From: Andy Shevchenko @ 2025-04-17 15:53 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andrew Morton, Michal Schmidt, devicetree,
linux-kernel, linux-hardening
On Wed, Apr 16, 2025 at 06:21:40PM +0200, Ivan Vecera wrote:
> Use devlink_alloc() to alloc zl3073x_dev structure, register the device
> as a devlink device and add devlink callback to provide devlink device
> info.
...
> /**
> * zl3073x_devm_alloc - allocates zl3073x device structure
> * @dev: pointer to device structure
> @@ -124,12 +204,18 @@ static const struct regmap_config zl3073x_regmap_config = {
> struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev)
> {
> struct zl3073x_dev *zldev;
> + struct devlink *devlink;
> int rc;
>
> - zldev = devm_kzalloc(dev, sizeof(*zldev), GFP_KERNEL);
> - if (!zldev)
> + devlink = devlink_alloc(&zl3073x_devlink_ops, sizeof(*zldev), dev);
> + if (!devlink)
> return ERR_PTR(-ENOMEM);
>
> + /* Add devres action to free devlink device */
> + if (devm_add_action_or_reset(dev, zl3073x_devlink_free, devlink))
> + return ERR_PTR(-ENOMEM);
Please, do not shadow the error codes. You might miss something. Shadowing
error codes needs a good justification.
> + zldev = devlink_priv(devlink);
> zldev->dev = dev;
> }
...
> + /* Add devres action to unregister devlink device */
> + rc = devm_add_action_or_reset(zldev->dev, zl3073x_devlink_unregister,
> + devlink);
> + if (rc)
> + return rc;
The code is even inconsistent in one patch!
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-16 16:21 [PATCH v3 net-next 00/10] Add Microchip ZL3073x support (part 1) Ivan Vecera
` (3 preceding siblings ...)
2025-04-16 16:21 ` [PATCH v3 net-next 4/8] mfd: zl3073x: Add support for devlink device info Ivan Vecera
@ 2025-04-16 16:21 ` Ivan Vecera
2025-04-16 17:32 ` Andrew Lunn
2025-04-17 16:13 ` Lee Jones
2025-04-16 16:21 ` [PATCH v3 net-next 6/8] mfd: zl3073x: Add clock_id field Ivan Vecera
` (2 subsequent siblings)
7 siblings, 2 replies; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 16:21 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
Registers present in page 10 and higher are called mailbox type
registers. Each page represents a mailbox and is used to read and write
configuration of particular object (dpll, output, reference & synth).
The mailbox page contains mask register that is used to select an index of
requested object to work with and semaphore register to indicate what
operation is requested.
The rest of registers in the particular register page are latch
registers that are filled by the firmware during read operation or by
the driver prior write operation.
For read operation the driver...
1) ... updates the mailbox mask register with index of particular object
2) ... sets the mailbox semaphore register read bit
3) ... waits for the semaphore register read bit to be cleared by FW
4) ... reads the configuration from latch registers
For write operation the driver...
1) ... writes the requested configuration to latch registers
2) ... sets the mailbox mask register for the DPLL to be updated
3) ... sets the mailbox semaphore register bit for the write operation
4) ... waits for the semaphore register bit to be cleared by FW
Add functions to read and write mailboxes for all supported object types.
All these functions as well as functions accessing mailbox latch registers
(zl3073x_mb_* functions) have to be called with zl3073x_dev->mailbox_lock
held and a caller is responsible to take this lock.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
v1->v3:
* dropped ZL3073X_MB_OP macro usage
---
drivers/mfd/zl3073x-core.c | 232 +++++++++++++++++++++++
include/linux/mfd/zl3073x.h | 12 ++
include/linux/mfd/zl3073x_regs.h | 304 +++++++++++++++++++++++++++++++
3 files changed, 548 insertions(+)
diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
index 8a15237e0f731..3d18786c769d2 100644
--- a/drivers/mfd/zl3073x-core.c
+++ b/drivers/mfd/zl3073x-core.c
@@ -115,6 +115,238 @@ static const struct regmap_config zl3073x_regmap_config = {
.volatile_reg = zl3073x_is_volatile_reg,
};
+/**
+ * zl3073x_mb_dpll_read - read given DPLL configuration to mailbox
+ * @zldev: pointer to device structure
+ * @index: DPLL index
+ *
+ * Reads configuration of given DPLL into DPLL mailbox.
+ *
+ * Context: Process context. Expects zldev->regmap_lock to be held by caller.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index)
+{
+ int rc;
+
+ /* Select requested index in mask register */
+ rc = zl3073x_mb_write_dpll_mb_mask(zldev, BIT(index));
+ if (rc)
+ return rc;
+
+ /* Perform read operation */
+ rc = zl3073x_mb_write_dpll_mb_sem(zldev, ZL_DPLL_MB_SEM_RD);
+ if (rc)
+ return rc;
+
+ /* Wait for the command to actually finish */
+ return zl3073x_mb_poll_dpll_mb_sem(zldev, ZL_DPLL_MB_SEM_RD);
+}
+EXPORT_SYMBOL_GPL(zl3073x_mb_dpll_read);
+
+/**
+ * zl3073x_mb_dpll_write - write given DPLL configuration from mailbox
+ * @zldev: pointer to device structure
+ * @index: DPLL index
+ *
+ * Writes (commits) configuration of given DPLL from DPLL mailbox.
+ *
+ * Context: Process context. Expects zldev->regmap_lock to be held by caller.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index)
+{
+ int rc;
+
+ /* Select requested index in mask register */
+ rc = zl3073x_mb_write_dpll_mb_mask(zldev, BIT(index));
+ if (rc)
+ return rc;
+
+ /* Perform read operation */
+ rc = zl3073x_mb_write_dpll_mb_sem(zldev, ZL_DPLL_MB_SEM_WR);
+ if (rc)
+ return rc;
+
+ /* Wait for the command to actually finish */
+ return zl3073x_mb_poll_dpll_mb_sem(zldev, ZL_DPLL_MB_SEM_WR);
+}
+EXPORT_SYMBOL_GPL(zl3073x_mb_dpll_write);
+
+/**
+ * zl3073x_mb_output_read - read given output configuration to mailbox
+ * @zldev: pointer to device structure
+ * @index: output index
+ *
+ * Reads configuration of given output into output mailbox.
+ *
+ * Context: Process context. Expects zldev->regmap_lock to be held by caller.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index)
+{
+ int rc;
+
+ /* Select requested index in mask register */
+ rc = zl3073x_mb_write_output_mb_mask(zldev, BIT(index));
+ if (rc)
+ return rc;
+
+ /* Perform read operation */
+ rc = zl3073x_mb_write_output_mb_sem(zldev, ZL_OUTPUT_MB_SEM_RD);
+ if (rc)
+ return rc;
+
+ /* Wait for the command to actually finish */
+ return zl3073x_mb_poll_output_mb_sem(zldev, ZL_OUTPUT_MB_SEM_RD);
+}
+EXPORT_SYMBOL_GPL(zl3073x_mb_output_read);
+
+/**
+ * zl3073x_mb_output_write - write given output configuration from mailbox
+ * @zldev: pointer to device structure
+ * @index: output index
+ *
+ * Writes (commits) configuration of given output from output mailbox.
+ *
+ * Context: Process context. Expects zldev->regmap_lock to be held by caller.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index)
+{
+ int rc;
+
+ /* Select requested index in mask register */
+ rc = zl3073x_mb_write_output_mb_mask(zldev, BIT(index));
+ if (rc)
+ return rc;
+
+ /* Perform read operation */
+ rc = zl3073x_mb_write_output_mb_sem(zldev, ZL_OUTPUT_MB_SEM_WR);
+ if (rc)
+ return rc;
+
+ /* Wait for the command to actually finish */
+ return zl3073x_mb_poll_output_mb_sem(zldev, ZL_OUTPUT_MB_SEM_WR);
+}
+EXPORT_SYMBOL_GPL(zl3073x_mb_output_write);
+
+/**
+ * zl3073x_mb_ref_read - read given reference configuration to mailbox
+ * @zldev: pointer to device structure
+ * @index: reference index
+ *
+ * Reads configuration of given reference into ref mailbox.
+ *
+ * Context: Process context. Expects zldev->regmap_lock to be held by caller.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index)
+{
+ int rc;
+
+ /* Select requested index in mask register */
+ rc = zl3073x_mb_write_ref_mb_mask(zldev, BIT(index));
+ if (rc)
+ return rc;
+
+ /* Perform read operation */
+ rc = zl3073x_mb_write_ref_mb_sem(zldev, ZL_REF_MB_SEM_RD);
+ if (rc)
+ return rc;
+
+ /* Wait for the command to actually finish */
+ return zl3073x_mb_poll_ref_mb_sem(zldev, ZL_REF_MB_SEM_RD);
+}
+EXPORT_SYMBOL_GPL(zl3073x_mb_ref_read);
+
+/**
+ * zl3073x_mb_ref_write - write given reference configuration from mailbox
+ * @zldev: pointer to device structure
+ * @index: reference index
+ *
+ * Writes (commits) configuration of given reference from ref mailbox.
+ *
+ * Context: Process context. Expects zldev->regmap_lock to be held by caller.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index)
+{
+ int rc;
+
+ /* Select requested index in mask register */
+ rc = zl3073x_mb_write_ref_mb_mask(zldev, BIT(index));
+ if (rc)
+ return rc;
+
+ /* Perform read operation */
+ rc = zl3073x_mb_write_ref_mb_sem(zldev, ZL_REF_MB_SEM_WR);
+ if (rc)
+ return rc;
+
+ /* Wait for the command to actually finish */
+ return zl3073x_mb_poll_ref_mb_sem(zldev, ZL_REF_MB_SEM_WR);
+}
+EXPORT_SYMBOL_GPL(zl3073x_mb_ref_write);
+
+/**
+ * zl3073x_mb_synth_read - read given synth configuration to mailbox
+ * @zldev: pointer to device structure
+ * @index: synth index
+ *
+ * Reads configuration of given synth into synth mailbox.
+ *
+ * Context: Process context. Expects zldev->regmap_lock to be held by caller.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index)
+{
+ int rc;
+
+ /* Select requested index in mask register */
+ rc = zl3073x_mb_write_synth_mb_mask(zldev, BIT(index));
+ if (rc)
+ return rc;
+
+ /* Perform read operation */
+ rc = zl3073x_mb_write_synth_mb_sem(zldev, ZL_SYNTH_MB_SEM_RD);
+ if (rc)
+ return rc;
+
+ /* Wait for the command to actually finish */
+ return zl3073x_mb_poll_synth_mb_sem(zldev, ZL_SYNTH_MB_SEM_RD);
+}
+EXPORT_SYMBOL_GPL(zl3073x_mb_synth_read);
+
+/**
+ * zl3073x_mb_synth_write - write given synth configuration from mailbox
+ * @zldev: pointer to device structure
+ * @index: synth index
+ *
+ * Writes (commits) configuration of given synth from synth mailbox.
+ *
+ * Context: Process context. Expects zldev->regmap_lock to be held by caller.
+ * Return: 0 on success, <0 on error
+ */
+int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index)
+{
+ int rc;
+
+ /* Select requested index in mask register */
+ rc = zl3073x_mb_write_synth_mb_mask(zldev, BIT(index));
+ if (rc)
+ return rc;
+
+ /* Perform read operation */
+ rc = zl3073x_mb_write_synth_mb_sem(zldev, ZL_SYNTH_MB_SEM_WR);
+ if (rc)
+ return rc;
+
+ /* Wait for the command to actually finish */
+ return zl3073x_mb_poll_synth_mb_sem(zldev, ZL_SYNTH_MB_SEM_WR);
+}
+EXPORT_SYMBOL_GPL(zl3073x_mb_synth_write);
+
/**
* zl3073x_devlink_info_get - Devlink device info callback
* @devlink: devlink structure pointer
diff --git a/include/linux/mfd/zl3073x.h b/include/linux/mfd/zl3073x.h
index b68481dcf77a5..53813a8c39660 100644
--- a/include/linux/mfd/zl3073x.h
+++ b/include/linux/mfd/zl3073x.h
@@ -47,4 +47,16 @@ static inline void zl3073x_mailbox_unlock(struct zl3073x_dev *zldev)
DEFINE_GUARD(zl3073x_mailbox, struct zl3073x_dev *, zl3073x_mailbox_lock(_T),
zl3073x_mailbox_unlock(_T));
+/*
+ * Mailbox operations
+ */
+int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
+int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
+int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
+int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
+int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
+int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
+int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
+int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
+
#endif /* __LINUX_MFD_ZL3073X_H */
diff --git a/include/linux/mfd/zl3073x_regs.h b/include/linux/mfd/zl3073x_regs.h
index 453a5da8ac63f..d155650349c97 100644
--- a/include/linux/mfd/zl3073x_regs.h
+++ b/include/linux/mfd/zl3073x_regs.h
@@ -15,6 +15,10 @@
#define ZL_PAGE_SIZE 0x80
#define ZL_REG_ADDR(_pg, _off) (ZL_RANGE_OFF + (_pg) * ZL_PAGE_SIZE + (_off))
+/* Register polling sleep & timeout */
+#define ZL_POLL_SLEEP_US 10
+#define ZL_POLL_TIMEOUT_US 2000000
+
/**************************
* Register Page 0, General
**************************/
@@ -102,4 +106,304 @@ zl3073x_read_custom_config_ver(struct zl3073x_dev *zldev, u32 *value)
return rc;
}
+/*******************************
+ * Register Page 10, Ref Mailbox
+ *******************************/
+
+/*
+ * Register 'ref_mb_mask'
+ * Page: 10, Offset: 0x02, Size: 16 bits
+ */
+#define ZL_REG_REF_MB_MASK ZL_REG_ADDR(10, 0x02)
+
+static inline __maybe_unused int
+zl3073x_mb_read_ref_mb_mask(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_REF_MB_MASK, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+static inline __maybe_unused int
+zl3073x_mb_write_ref_mb_mask(struct zl3073x_dev *zldev, u16 value)
+{
+ __be16 temp;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ temp = cpu_to_be16(value);
+ return regmap_bulk_write(zldev->regmap, ZL_REG_REF_MB_MASK, &temp,
+ sizeof(temp));
+}
+
+/*
+ * Register 'ref_mb_sem'
+ * Page: 10, Offset: 0x04, Size: 8 bits
+ */
+#define ZL_REG_REF_MB_SEM ZL_REG_ADDR(10, 0x04)
+#define ZL_REF_MB_SEM_WR BIT(0)
+#define ZL_REF_MB_SEM_RD BIT(1)
+
+static inline __maybe_unused int
+zl3073x_mb_read_ref_mb_sem(struct zl3073x_dev *zldev, u8 *value)
+{
+ unsigned int v;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_read(zldev->regmap, ZL_REG_REF_MB_SEM, &v);
+ *value = v;
+ return rc;
+}
+
+static inline __maybe_unused int
+zl3073x_mb_write_ref_mb_sem(struct zl3073x_dev *zldev, u8 value)
+{
+ lockdep_assert_held(&zldev->mailbox_lock);
+ return regmap_write(zldev->regmap, ZL_REG_REF_MB_SEM, value);
+}
+
+static inline __maybe_unused int
+zl3073x_mb_poll_ref_mb_sem(struct zl3073x_dev *zldev, u8 bitmask)
+{
+ unsigned int v;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ return regmap_read_poll_timeout(zldev->regmap, ZL_REG_REF_MB_SEM, v,
+ !(v & bitmask), ZL_POLL_SLEEP_US,
+ ZL_POLL_TIMEOUT_US);
+}
+
+/********************************
+ * Register Page 12, DPLL Mailbox
+ ********************************/
+
+/*
+ * Register 'dpll_mb_mask'
+ * Page: 12, Offset: 0x02, Size: 16 bits
+ */
+#define ZL_REG_DPLL_MB_MASK ZL_REG_ADDR(12, 0x02)
+
+static inline __maybe_unused int
+zl3073x_mb_read_dpll_mb_mask(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_DPLL_MB_MASK, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+static inline __maybe_unused int
+zl3073x_mb_write_dpll_mb_mask(struct zl3073x_dev *zldev, u16 value)
+{
+ __be16 temp;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ temp = cpu_to_be16(value);
+ return regmap_bulk_write(zldev->regmap, ZL_REG_DPLL_MB_MASK, &temp,
+ sizeof(temp));
+}
+
+/*
+ * Register 'dpll_mb_sem'
+ * Page: 12, Offset: 0x04, Size: 8 bits
+ */
+#define ZL_REG_DPLL_MB_SEM ZL_REG_ADDR(12, 0x04)
+#define ZL_DPLL_MB_SEM_WR BIT(0)
+#define ZL_DPLL_MB_SEM_RD BIT(1)
+
+static inline __maybe_unused int
+zl3073x_mb_read_dpll_mb_sem(struct zl3073x_dev *zldev, u8 *value)
+{
+ unsigned int v;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_read(zldev->regmap, ZL_REG_DPLL_MB_SEM, &v);
+ *value = v;
+ return rc;
+}
+
+static inline __maybe_unused int
+zl3073x_mb_write_dpll_mb_sem(struct zl3073x_dev *zldev, u8 value)
+{
+ lockdep_assert_held(&zldev->mailbox_lock);
+ return regmap_write(zldev->regmap, ZL_REG_DPLL_MB_SEM, value);
+}
+
+static inline __maybe_unused int
+zl3073x_mb_poll_dpll_mb_sem(struct zl3073x_dev *zldev, u8 bitmask)
+{
+ unsigned int v;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ return regmap_read_poll_timeout(zldev->regmap, ZL_REG_DPLL_MB_SEM, v,
+ !(v & bitmask), ZL_POLL_SLEEP_US,
+ ZL_POLL_TIMEOUT_US);
+}
+
+/*********************************
+ * Register Page 13, Synth Mailbox
+ *********************************/
+
+/*
+ * Register 'synth_mb_mask'
+ * Page: 13, Offset: 0x02, Size: 16 bits
+ */
+#define ZL_REG_SYNTH_MB_MASK ZL_REG_ADDR(13, 0x02)
+
+static inline __maybe_unused int
+zl3073x_mb_read_synth_mb_mask(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_SYNTH_MB_MASK, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+static inline __maybe_unused int
+zl3073x_mb_write_synth_mb_mask(struct zl3073x_dev *zldev, u16 value)
+{
+ __be16 temp;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ temp = cpu_to_be16(value);
+ return regmap_bulk_write(zldev->regmap, ZL_REG_SYNTH_MB_MASK, &temp,
+ sizeof(temp));
+}
+
+/*
+ * Register 'synth_mb_sem'
+ * Page: 13, Offset: 0x04, Size: 8 bits
+ */
+#define ZL_REG_SYNTH_MB_SEM ZL_REG_ADDR(13, 0x04)
+#define ZL_SYNTH_MB_SEM_WR BIT(0)
+#define ZL_SYNTH_MB_SEM_RD BIT(1)
+
+static inline __maybe_unused int
+zl3073x_mb_read_synth_mb_sem(struct zl3073x_dev *zldev, u8 *value)
+{
+ unsigned int v;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_read(zldev->regmap, ZL_REG_SYNTH_MB_SEM, &v);
+ *value = v;
+ return rc;
+}
+
+static inline __maybe_unused int
+zl3073x_mb_write_synth_mb_sem(struct zl3073x_dev *zldev, u8 value)
+{
+ lockdep_assert_held(&zldev->mailbox_lock);
+ return regmap_write(zldev->regmap, ZL_REG_SYNTH_MB_SEM, value);
+}
+
+static inline __maybe_unused int
+zl3073x_mb_poll_synth_mb_sem(struct zl3073x_dev *zldev, u8 bitmask)
+{
+ unsigned int v;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ return regmap_read_poll_timeout(zldev->regmap, ZL_REG_SYNTH_MB_SEM, v,
+ !(v & bitmask), ZL_POLL_SLEEP_US,
+ ZL_POLL_TIMEOUT_US);
+}
+
+/**********************************
+ * Register Page 14, Output Mailbox
+ **********************************/
+
+/*
+ * Register 'output_mb_mask'
+ * Page: 14, Offset: 0x02, Size: 16 bits
+ */
+#define ZL_REG_OUTPUT_MB_MASK ZL_REG_ADDR(14, 0x02)
+
+static inline __maybe_unused int
+zl3073x_mb_read_output_mb_mask(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_OUTPUT_MB_MASK, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+static inline __maybe_unused int
+zl3073x_mb_write_output_mb_mask(struct zl3073x_dev *zldev, u16 value)
+{
+ __be16 temp;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ temp = cpu_to_be16(value);
+ return regmap_bulk_write(zldev->regmap, ZL_REG_OUTPUT_MB_MASK, &temp,
+ sizeof(temp));
+}
+
+/*
+ * Register 'output_mb_sem'
+ * Page: 14, Offset: 0x04, Size: 8 bits
+ */
+#define ZL_REG_OUTPUT_MB_SEM ZL_REG_ADDR(14, 0x04)
+#define ZL_OUTPUT_MB_SEM_WR BIT(0)
+#define ZL_OUTPUT_MB_SEM_RD BIT(1)
+
+static inline __maybe_unused int
+zl3073x_mb_read_output_mb_sem(struct zl3073x_dev *zldev, u8 *value)
+{
+ unsigned int v;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_read(zldev->regmap, ZL_REG_OUTPUT_MB_SEM, &v);
+ *value = v;
+ return rc;
+}
+
+static inline __maybe_unused int
+zl3073x_mb_write_output_mb_sem(struct zl3073x_dev *zldev, u8 value)
+{
+ lockdep_assert_held(&zldev->mailbox_lock);
+ return regmap_write(zldev->regmap, ZL_REG_OUTPUT_MB_SEM, value);
+}
+
+static inline __maybe_unused int
+zl3073x_mb_poll_output_mb_sem(struct zl3073x_dev *zldev, u8 bitmask)
+{
+ unsigned int v;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ return regmap_read_poll_timeout(zldev->regmap, ZL_REG_OUTPUT_MB_SEM, v,
+ !(v & bitmask), ZL_POLL_SLEEP_US,
+ ZL_POLL_TIMEOUT_US);
+}
+
#endif /* __LINUX_MFD_ZL3073X_REGS_H */
--
2.48.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-16 16:21 ` [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes Ivan Vecera
@ 2025-04-16 17:32 ` Andrew Lunn
2025-04-16 18:27 ` Ivan Vecera
2025-04-17 16:13 ` Lee Jones
1 sibling, 1 reply; 39+ messages in thread
From: Andrew Lunn @ 2025-04-16 17:32 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
> +/**
> + * zl3073x_mb_dpll_read - read given DPLL configuration to mailbox
> + * @zldev: pointer to device structure
> + * @index: DPLL index
> + *
> + * Reads configuration of given DPLL into DPLL mailbox.
> + *
> + * Context: Process context. Expects zldev->regmap_lock to be held by caller.
> + * Return: 0 on success, <0 on error
> + */
> +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index)
> +{
> + int rc;
lockdep_assert_held(zldev->regmap_lock) is stronger than having a
comment. When talking about i2c and spi devices, it costs nothing, and
catches bugs early.
> +/*
> + * Mailbox operations
> + */
> +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
I assume these are the only valid ways to access a mailbox?
If so:
> +static inline __maybe_unused int
> +zl3073x_mb_read_ref_mb_mask(struct zl3073x_dev *zldev, u16 *value)
> +{
> + __be16 temp;
> + int rc;
> +
> + lockdep_assert_held(&zldev->mailbox_lock);
> + rc = regmap_bulk_read(zldev->regmap, ZL_REG_REF_MB_MASK, &temp,
> + sizeof(temp));
> + if (rc)
> + return rc;
> +
> + *value = be16_to_cpu(temp);
> + return rc;
> +}
These helpers can be made local to the core. You can then drop the
lockdep_assert_held() from here, since the only way to access them is
via the API you defined above, and add the checks in those API
functions.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-16 17:32 ` Andrew Lunn
@ 2025-04-16 18:27 ` Ivan Vecera
2025-04-17 10:02 ` Ivan Vecera
2025-04-17 13:22 ` Andrew Lunn
0 siblings, 2 replies; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 18:27 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
On Wed, Apr 16, 2025 at 7:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +/**
> > + * zl3073x_mb_dpll_read - read given DPLL configuration to mailbox
> > + * @zldev: pointer to device structure
> > + * @index: DPLL index
> > + *
> > + * Reads configuration of given DPLL into DPLL mailbox.
> > + *
> > + * Context: Process context. Expects zldev->regmap_lock to be held by caller.
> > + * Return: 0 on success, <0 on error
> > + */
> > +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index)
> > +{
> > + int rc;
>
> lockdep_assert_held(zldev->regmap_lock) is stronger than having a
> comment. When talking about i2c and spi devices, it costs nothing, and
> catches bugs early.
Makes sense to put the assert here...
Will add.
>
> > +/*
> > + * Mailbox operations
> > + */
> > +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
>
> I assume these are the only valid ways to access a mailbox?
>
> If so:
>
> > +static inline __maybe_unused int
> > +zl3073x_mb_read_ref_mb_mask(struct zl3073x_dev *zldev, u16 *value)
> > +{
> > + __be16 temp;
> > + int rc;
> > +
> > + lockdep_assert_held(&zldev->mailbox_lock);
> > + rc = regmap_bulk_read(zldev->regmap, ZL_REG_REF_MB_MASK, &temp,
> > + sizeof(temp));
> > + if (rc)
> > + return rc;
> > +
> > + *value = be16_to_cpu(temp);
> > + return rc;
> > +}
>
> These helpers can be made local to the core. You can then drop the
> lockdep_assert_held() from here, since the only way to access them is
> via the API you defined above, and add the checks in those API
> functions.
This cannot be done this way... the above API just simplifies the
operation of read and write latch registers from/to mailbox.
Whole operation is described in the commit description.
E.g. read something about DPLL1
1. Call zl3073x_mb_dpll_read(..., 1)
This selects DPLL1 in the DPLL mailbox and performs read operation
and waits for finish
2. Call zl3073x_mb_read_dpll_mode()
This reads dpll_mode latch register
write:
1. Call zl3073x_mb_write_dpll_mode(...)
This writes mode to dpll_mode latch register
2. Call zl3073x_mb_dpll_read(..., 1)
This writes all info from latch registers to DPLL1
The point is that between step 1 and 2 nobody else cannot touch
latch_registers or mailbox select register and op semaphore.
Thanks,
Ivan
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-16 18:27 ` Ivan Vecera
@ 2025-04-17 10:02 ` Ivan Vecera
2025-04-17 13:27 ` Andrew Lunn
2025-04-17 13:22 ` Andrew Lunn
1 sibling, 1 reply; 39+ messages in thread
From: Ivan Vecera @ 2025-04-17 10:02 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
On 16. 04. 25 8:27 odp., Ivan Vecera wrote:
> On Wed, Apr 16, 2025 at 7:32 PM Andrew Lunn <andrew@lunn.ch> wrote:
>>
>>> +/**
>>> + * zl3073x_mb_dpll_read - read given DPLL configuration to mailbox
>>> + * @zldev: pointer to device structure
>>> + * @index: DPLL index
>>> + *
>>> + * Reads configuration of given DPLL into DPLL mailbox.
>>> + *
>>> + * Context: Process context. Expects zldev->regmap_lock to be held by caller.
>>> + * Return: 0 on success, <0 on error
>>> + */
>>> +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index)
>>> +{
>>> + int rc;
>>
>> lockdep_assert_held(zldev->regmap_lock) is stronger than having a
>> comment. When talking about i2c and spi devices, it costs nothing, and
>> catches bugs early.
>
> Makes sense to put the assert here...
>
> Will add.
>
>>
>>> +/*
>>> + * Mailbox operations
>>> + */
>>> +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
>>> +int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
>>> +int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
>>> +int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
>>> +int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
>>> +int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
>>> +int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
>>> +int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
>>
>> I assume these are the only valid ways to access a mailbox?
>>
>> If so:
>>
>>> +static inline __maybe_unused int
>>> +zl3073x_mb_read_ref_mb_mask(struct zl3073x_dev *zldev, u16 *value)
>>> +{
>>> + __be16 temp;
>>> + int rc;
>>> +
>>> + lockdep_assert_held(&zldev->mailbox_lock);
>>> + rc = regmap_bulk_read(zldev->regmap, ZL_REG_REF_MB_MASK, &temp,
>>> + sizeof(temp));
>>> + if (rc)
>>> + return rc;
>>> +
>>> + *value = be16_to_cpu(temp);
>>> + return rc;
>>> +}
>>
>> These helpers can be made local to the core. You can then drop the
>> lockdep_assert_held() from here, since the only way to access them is
>> via the API you defined above, and add the checks in those API
>> functions.
>
> This cannot be done this way... the above API just simplifies the
> operation of read and write latch registers from/to mailbox.
>
> Whole operation is described in the commit description.
>
> E.g. read something about DPLL1
> 1. Call zl3073x_mb_dpll_read(..., 1)
> This selects DPLL1 in the DPLL mailbox and performs read operation
> and waits for finish
> 2. Call zl3073x_mb_read_dpll_mode()
> This reads dpll_mode latch register
>
> write:
> 1. Call zl3073x_mb_write_dpll_mode(...)
> This writes mode to dpll_mode latch register
> 2. Call zl3073x_mb_dpll_read(..., 1)
> This writes all info from latch registers to DPLL1
>
> The point is that between step 1 and 2 nobody else cannot touch
> latch_registers or mailbox select register and op semaphore.
>
Anyway, I have a different idea... completely abstract mailboxes from
the caller. The mailbox content can be large and the caller is barely
interested in all registers from the mailbox but this could be resolved
this way:
The proposed API e.g for Ref mailbox:
int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index,
struct zl3073x_mb_ref *mb);
int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index,
struct zl3073x_mb_ref *mb);
struct zl3073x_mb_ref {
u32 flags;
u16 freq_base;
u16 freq_mult;
u16 ratio_m;
u16 ratio_n;
u8 config;
u64 phase_offset_compensation;
u8 sync_ctrl;
u32 esync_div;
}
#define ZL3073X_MB_REF_FREQ_BASE BIT(0)
#define ZL3073X_MB_REF_FREQ_MULT BIT(1)
#define ZL3073X_MB_REF_RATIO_M BIT(2)
#define ZL3073X_MB_REF_RATIO_N BIT(3)
#define ZL3073X_MB_REF_CONFIG BIT(4)
#define ZL3073X_MB_REF_PHASE_OFFSET_COMPENSATION BIT(5)
#define ZL3073X_MB_REF_SYNC_CTRL BIT(6)
#define ZL3073X_MB_REF_ESYNC_DIV BIT(7)
Then a reader can read this way (read freq and ratio of 3rd ref):
{
struct zl3073x_mb_ref mb;
...
mb.flags = ZL3073X_MB_REF_FREQ_BASE |
ZL3073X_MB_REF_FREQ_MULT |
ZL3073X_MB_REF_RATIO_M |
ZL3073X_MB_REF_RATIO_N;
rc = zl3073x_mb_ref_read(zldev, 3, &mb);
if (rc)
return rc;
/* at this point mb fields requested via flags are filled */
}
A writer similarly (write config of 5th ref):
{
struct zl3073x_mb_ref mb;
...
mb.flags = ZL3073X_MB_REF_CONFIG;
mb.config = FIELD_PREP(SOME_MASK, SOME_VALUE);
rc = zl3073x_mb_ref_write(zldev, 5, &mb);
...
/* config of 5th ref was commited */
}
The advantages:
* no explicit locking required from the callers
* locking is done inside mailbox API
* each mailbox type can have different mutex so multiple calls for
different mailbox types (e.g ref & output) can be done in parallel
WDYT about this approach?
Thanks,
Ivan
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-17 10:02 ` Ivan Vecera
@ 2025-04-17 13:27 ` Andrew Lunn
2025-04-17 14:15 ` Ivan Vecera
0 siblings, 1 reply; 39+ messages in thread
From: Andrew Lunn @ 2025-04-17 13:27 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
> Anyway, I have a different idea... completely abstract mailboxes from the
> caller. The mailbox content can be large and the caller is barely interested
> in all registers from the mailbox but this could be resolved this way:
>
> The proposed API e.g for Ref mailbox:
>
> int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index,
> struct zl3073x_mb_ref *mb);
> int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index,
> struct zl3073x_mb_ref *mb);
>
> struct zl3073x_mb_ref {
> u32 flags;
> u16 freq_base;
> u16 freq_mult;
> u16 ratio_m;
> u16 ratio_n;
> u8 config;
> u64 phase_offset_compensation;
> u8 sync_ctrl;
> u32 esync_div;
> }
>
> #define ZL3073X_MB_REF_FREQ_BASE BIT(0)
> #define ZL3073X_MB_REF_FREQ_MULT BIT(1)
> #define ZL3073X_MB_REF_RATIO_M BIT(2)
> #define ZL3073X_MB_REF_RATIO_N BIT(3)
> #define ZL3073X_MB_REF_CONFIG BIT(4)
> #define ZL3073X_MB_REF_PHASE_OFFSET_COMPENSATION BIT(5)
> #define ZL3073X_MB_REF_SYNC_CTRL BIT(6)
> #define ZL3073X_MB_REF_ESYNC_DIV BIT(7)
>
> Then a reader can read this way (read freq and ratio of 3rd ref):
> {
> struct zl3073x_mb_ref mb;
> ...
> mb.flags = ZL3073X_MB_REF_FREQ_BASE |
> ZL3073X_MB_REF_FREQ_MULT |
> ZL3073X_MB_REF_RATIO_M |
> ZL3073X_MB_REF_RATIO_N;
> rc = zl3073x_mb_ref_read(zldev, 3, &mb);
> if (rc)
> return rc;
> /* at this point mb fields requested via flags are filled */
> }
> A writer similarly (write config of 5th ref):
> {
> struct zl3073x_mb_ref mb;
> ...
> mb.flags = ZL3073X_MB_REF_CONFIG;
> mb.config = FIELD_PREP(SOME_MASK, SOME_VALUE);
> rc = zl3073x_mb_ref_write(zldev, 5, &mb);
> ...
> /* config of 5th ref was commited */
> }
>
> The advantages:
> * no explicit locking required from the callers
> * locking is done inside mailbox API
> * each mailbox type can have different mutex so multiple calls for
> different mailbox types (e.g ref & output) can be done in parallel
>
> WDYT about this approach?
I would say this is actually your next layer on top of the basic
mailbox API. This makes it more friendly to your sub driver and puts
all the locking in one place where it can easily be reviewed.
One question would be, where does this code belong. Is it in the MFD,
or in the subdrivers? I guess it is in the subdrivers.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-17 13:27 ` Andrew Lunn
@ 2025-04-17 14:15 ` Ivan Vecera
2025-04-24 15:49 ` Lee Jones
0 siblings, 1 reply; 39+ messages in thread
From: Ivan Vecera @ 2025-04-17 14:15 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
On 17. 04. 25 3:27 odp., Andrew Lunn wrote:
>> Anyway, I have a different idea... completely abstract mailboxes from the
>> caller. The mailbox content can be large and the caller is barely interested
>> in all registers from the mailbox but this could be resolved this way:
>>
>> The proposed API e.g for Ref mailbox:
>>
>> int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index,
>> struct zl3073x_mb_ref *mb);
>> int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index,
>> struct zl3073x_mb_ref *mb);
>>
>> struct zl3073x_mb_ref {
>> u32 flags;
>> u16 freq_base;
>> u16 freq_mult;
>> u16 ratio_m;
>> u16 ratio_n;
>> u8 config;
>> u64 phase_offset_compensation;
>> u8 sync_ctrl;
>> u32 esync_div;
>> }
>>
>> #define ZL3073X_MB_REF_FREQ_BASE BIT(0)
>> #define ZL3073X_MB_REF_FREQ_MULT BIT(1)
>> #define ZL3073X_MB_REF_RATIO_M BIT(2)
>> #define ZL3073X_MB_REF_RATIO_N BIT(3)
>> #define ZL3073X_MB_REF_CONFIG BIT(4)
>> #define ZL3073X_MB_REF_PHASE_OFFSET_COMPENSATION BIT(5)
>> #define ZL3073X_MB_REF_SYNC_CTRL BIT(6)
>> #define ZL3073X_MB_REF_ESYNC_DIV BIT(7)
>>
>> Then a reader can read this way (read freq and ratio of 3rd ref):
>> {
>> struct zl3073x_mb_ref mb;
>> ...
>> mb.flags = ZL3073X_MB_REF_FREQ_BASE |
>> ZL3073X_MB_REF_FREQ_MULT |
>> ZL3073X_MB_REF_RATIO_M |
>> ZL3073X_MB_REF_RATIO_N;
>> rc = zl3073x_mb_ref_read(zldev, 3, &mb);
>> if (rc)
>> return rc;
>> /* at this point mb fields requested via flags are filled */
>> }
>> A writer similarly (write config of 5th ref):
>> {
>> struct zl3073x_mb_ref mb;
>> ...
>> mb.flags = ZL3073X_MB_REF_CONFIG;
>> mb.config = FIELD_PREP(SOME_MASK, SOME_VALUE);
>> rc = zl3073x_mb_ref_write(zldev, 5, &mb);
>> ...
>> /* config of 5th ref was commited */
>> }
>>
>> The advantages:
>> * no explicit locking required from the callers
>> * locking is done inside mailbox API
>> * each mailbox type can have different mutex so multiple calls for
>> different mailbox types (e.g ref & output) can be done in parallel
>>
>> WDYT about this approach?
>
> I would say this is actually your next layer on top of the basic
> mailbox API. This makes it more friendly to your sub driver and puts
> all the locking in one place where it can easily be reviewed.
>
> One question would be, where does this code belong. Is it in the MFD,
> or in the subdrivers? I guess it is in the subdrivers.
No, it should be part of MFD because it does not make sense to implement
API above in each sub-driver separately.
Sub-driver would use this MB ABI for MB access and
zl3073x_{read,write}_u{8,16,32,48} for non-MB registers.
Ivan
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-17 14:15 ` Ivan Vecera
@ 2025-04-24 15:49 ` Lee Jones
0 siblings, 0 replies; 39+ messages in thread
From: Lee Jones @ 2025-04-24 15:49 UTC (permalink / raw)
To: Ivan Vecera
Cc: Andrew Lunn, netdev, Vadim Fedorenko, Arkadiusz Kubalewski,
Jiri Pirko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Prathosh Satish, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
On Thu, 17 Apr 2025, Ivan Vecera wrote:
>
>
> On 17. 04. 25 3:27 odp., Andrew Lunn wrote:
> > > Anyway, I have a different idea... completely abstract mailboxes from the
> > > caller. The mailbox content can be large and the caller is barely interested
> > > in all registers from the mailbox but this could be resolved this way:
> > >
> > > The proposed API e.g for Ref mailbox:
> > >
> > > int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index,
> > > struct zl3073x_mb_ref *mb);
> > > int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index,
> > > struct zl3073x_mb_ref *mb);
> > >
> > > struct zl3073x_mb_ref {
> > > u32 flags;
> > > u16 freq_base;
> > > u16 freq_mult;
> > > u16 ratio_m;
> > > u16 ratio_n;
> > > u8 config;
> > > u64 phase_offset_compensation;
> > > u8 sync_ctrl;
> > > u32 esync_div;
> > > }
> > >
> > > #define ZL3073X_MB_REF_FREQ_BASE BIT(0)
> > > #define ZL3073X_MB_REF_FREQ_MULT BIT(1)
> > > #define ZL3073X_MB_REF_RATIO_M BIT(2)
> > > #define ZL3073X_MB_REF_RATIO_N BIT(3)
> > > #define ZL3073X_MB_REF_CONFIG BIT(4)
> > > #define ZL3073X_MB_REF_PHASE_OFFSET_COMPENSATION BIT(5)
> > > #define ZL3073X_MB_REF_SYNC_CTRL BIT(6)
> > > #define ZL3073X_MB_REF_ESYNC_DIV BIT(7)
> > >
> > > Then a reader can read this way (read freq and ratio of 3rd ref):
> > > {
> > > struct zl3073x_mb_ref mb;
> > > ...
> > > mb.flags = ZL3073X_MB_REF_FREQ_BASE |
> > > ZL3073X_MB_REF_FREQ_MULT |
> > > ZL3073X_MB_REF_RATIO_M |
> > > ZL3073X_MB_REF_RATIO_N;
> > > rc = zl3073x_mb_ref_read(zldev, 3, &mb);
> > > if (rc)
> > > return rc;
> > > /* at this point mb fields requested via flags are filled */
> > > }
> > > A writer similarly (write config of 5th ref):
> > > {
> > > struct zl3073x_mb_ref mb;
> > > ...
> > > mb.flags = ZL3073X_MB_REF_CONFIG;
> > > mb.config = FIELD_PREP(SOME_MASK, SOME_VALUE);
> > > rc = zl3073x_mb_ref_write(zldev, 5, &mb);
> > > ...
> > > /* config of 5th ref was commited */
> > > }
> > >
> > > The advantages:
> > > * no explicit locking required from the callers
> > > * locking is done inside mailbox API
> > > * each mailbox type can have different mutex so multiple calls for
> > > different mailbox types (e.g ref & output) can be done in parallel
> > >
> > > WDYT about this approach?
> >
> > I would say this is actually your next layer on top of the basic
> > mailbox API. This makes it more friendly to your sub driver and puts
> > all the locking in one place where it can easily be reviewed.
> >
> > One question would be, where does this code belong. Is it in the MFD,
> > or in the subdrivers? I guess it is in the subdrivers.
>
> No, it should be part of MFD because it does not make sense to implement API
> above in each sub-driver separately.
>
> Sub-driver would use this MB ABI for MB access and
> zl3073x_{read,write}_u{8,16,32,48} for non-MB registers.
Regardless of whether you decide to place the API in the sub-drivers or
not, it doesn't belong in MFD. 600 lines of any API is too heavyweight
to live here. If you can't justify placing it in Mailbox, my next
suggestion would be drivers/platform.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-16 18:27 ` Ivan Vecera
2025-04-17 10:02 ` Ivan Vecera
@ 2025-04-17 13:22 ` Andrew Lunn
2025-04-17 14:18 ` Ivan Vecera
1 sibling, 1 reply; 39+ messages in thread
From: Andrew Lunn @ 2025-04-17 13:22 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
> > > +/*
> > > + * Mailbox operations
> > > + */
> > > +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
> > > +int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
> > > +int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
> > > +int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
> > > +int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
> > > +int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
> > > +int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
> > > +int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
> >
> > I assume these are the only valid ways to access a mailbox?
> >
> > If so:
> >
> > > +static inline __maybe_unused int
> > > +zl3073x_mb_read_ref_mb_mask(struct zl3073x_dev *zldev, u16 *value)
> > > +{
> > > + __be16 temp;
> > > + int rc;
> > > +
> > > + lockdep_assert_held(&zldev->mailbox_lock);
> > > + rc = regmap_bulk_read(zldev->regmap, ZL_REG_REF_MB_MASK, &temp,
> > > + sizeof(temp));
> > > + if (rc)
> > > + return rc;
> > > +
> > > + *value = be16_to_cpu(temp);
> > > + return rc;
> > > +}
> >
> > These helpers can be made local to the core. You can then drop the
> > lockdep_assert_held() from here, since the only way to access them is
> > via the API you defined above, and add the checks in those API
> > functions.
>
> This cannot be done this way... the above API just simplifies the
> operation of read and write latch registers from/to mailbox.
>
> Whole operation is described in the commit description.
>
> E.g. read something about DPLL1
> 1. Call zl3073x_mb_dpll_read(..., 1)
> This selects DPLL1 in the DPLL mailbox and performs read operation
> and waits for finish
> 2. Call zl3073x_mb_read_dpll_mode()
> This reads dpll_mode latch register
>
> write:
> 1. Call zl3073x_mb_write_dpll_mode(...)
> This writes mode to dpll_mode latch register
> 2. Call zl3073x_mb_dpll_read(..., 1)
> This writes all info from latch registers to DPLL1
>
> The point is that between step 1 and 2 nobody else cannot touch
> latch_registers or mailbox select register and op semaphore.
Again, think about your layering. What does your lower level mailbox
API look like? What does the MFD need to export for a safe API?
So maybe you need zl3073x_mb_read_u8(), zl3073x_mb_read_u16(),
zl3073x_mb_read_u32(), as part of your mailbox API. These assert the
lock is held.
You could even make zl3073x_mb_read_u8() validate the register is in
the upper range, and that zl3073x_read_u8() the register is in the
lower range.
Andrew
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-17 13:22 ` Andrew Lunn
@ 2025-04-17 14:18 ` Ivan Vecera
0 siblings, 0 replies; 39+ messages in thread
From: Ivan Vecera @ 2025-04-17 14:18 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Lee Jones, Kees Cook, Andy Shevchenko, Andrew Morton,
Michal Schmidt, devicetree, linux-kernel, linux-hardening
On 17. 04. 25 3:22 odp., Andrew Lunn wrote:
>>>> +/*
>>>> + * Mailbox operations
>>>> + */
>>>> +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
>>>> +int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
>>>> +int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
>>>> +int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
>>>> +int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
>>>> +int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
>>>> +int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
>>>> +int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
>>>
>>> I assume these are the only valid ways to access a mailbox?
>>>
>>> If so:
>>>
>>>> +static inline __maybe_unused int
>>>> +zl3073x_mb_read_ref_mb_mask(struct zl3073x_dev *zldev, u16 *value)
>>>> +{
>>>> + __be16 temp;
>>>> + int rc;
>>>> +
>>>> + lockdep_assert_held(&zldev->mailbox_lock);
>>>> + rc = regmap_bulk_read(zldev->regmap, ZL_REG_REF_MB_MASK, &temp,
>>>> + sizeof(temp));
>>>> + if (rc)
>>>> + return rc;
>>>> +
>>>> + *value = be16_to_cpu(temp);
>>>> + return rc;
>>>> +}
>>>
>>> These helpers can be made local to the core. You can then drop the
>>> lockdep_assert_held() from here, since the only way to access them is
>>> via the API you defined above, and add the checks in those API
>>> functions.
>>
>> This cannot be done this way... the above API just simplifies the
>> operation of read and write latch registers from/to mailbox.
>>
>> Whole operation is described in the commit description.
>>
>> E.g. read something about DPLL1
>> 1. Call zl3073x_mb_dpll_read(..., 1)
>> This selects DPLL1 in the DPLL mailbox and performs read operation
>> and waits for finish
>> 2. Call zl3073x_mb_read_dpll_mode()
>> This reads dpll_mode latch register
>>
>> write:
>> 1. Call zl3073x_mb_write_dpll_mode(...)
>> This writes mode to dpll_mode latch register
>> 2. Call zl3073x_mb_dpll_read(..., 1)
>> This writes all info from latch registers to DPLL1
>>
>> The point is that between step 1 and 2 nobody else cannot touch
>> latch_registers or mailbox select register and op semaphore.
>
> Again, think about your layering. What does your lower level mailbox
> API look like? What does the MFD need to export for a safe API?
>
> So maybe you need zl3073x_mb_read_u8(), zl3073x_mb_read_u16(),
> zl3073x_mb_read_u32(), as part of your mailbox API. These assert the
> lock is held.
>
> You could even make zl3073x_mb_read_u8() validate the register is in
> the upper range, and that zl3073x_read_u8() the register is in the
> lower range.
Yes, this LGTM... Anyway if the MB API would provide reading and writing
MBs at once then zl3073x_mb_{read,write}_u* are not necessary as the
only caller of these functions is MFD itself and they would be called
under MB API that holds the lock.
Ivan
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-16 16:21 ` [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes Ivan Vecera
2025-04-16 17:32 ` Andrew Lunn
@ 2025-04-17 16:13 ` Lee Jones
2025-04-17 16:35 ` Ivan Vecera
1 sibling, 1 reply; 39+ messages in thread
From: Lee Jones @ 2025-04-17 16:13 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
On Wed, 16 Apr 2025, Ivan Vecera wrote:
> Registers present in page 10 and higher are called mailbox type
> registers. Each page represents a mailbox and is used to read and write
> configuration of particular object (dpll, output, reference & synth).
>
> The mailbox page contains mask register that is used to select an index of
> requested object to work with and semaphore register to indicate what
> operation is requested.
>
> The rest of registers in the particular register page are latch
> registers that are filled by the firmware during read operation or by
> the driver prior write operation.
>
> For read operation the driver...
> 1) ... updates the mailbox mask register with index of particular object
> 2) ... sets the mailbox semaphore register read bit
> 3) ... waits for the semaphore register read bit to be cleared by FW
> 4) ... reads the configuration from latch registers
>
> For write operation the driver...
> 1) ... writes the requested configuration to latch registers
> 2) ... sets the mailbox mask register for the DPLL to be updated
> 3) ... sets the mailbox semaphore register bit for the write operation
> 4) ... waits for the semaphore register bit to be cleared by FW
>
> Add functions to read and write mailboxes for all supported object types.
>
> All these functions as well as functions accessing mailbox latch registers
> (zl3073x_mb_* functions) have to be called with zl3073x_dev->mailbox_lock
> held and a caller is responsible to take this lock.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> v1->v3:
> * dropped ZL3073X_MB_OP macro usage
> ---
> drivers/mfd/zl3073x-core.c | 232 +++++++++++++++++++++++
> include/linux/mfd/zl3073x.h | 12 ++
> include/linux/mfd/zl3073x_regs.h | 304 +++++++++++++++++++++++++++++++
> 3 files changed, 548 insertions(+)
> +/*
> + * Mailbox operations
> + */
> +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
> +int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
Why aren't these being placed into drivers/mailbox?
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes
2025-04-17 16:13 ` Lee Jones
@ 2025-04-17 16:35 ` Ivan Vecera
0 siblings, 0 replies; 39+ messages in thread
From: Ivan Vecera @ 2025-04-17 16:35 UTC (permalink / raw)
To: Lee Jones
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
On 17. 04. 25 6:13 odp., Lee Jones wrote:
> On Wed, 16 Apr 2025, Ivan Vecera wrote:
>
>> Registers present in page 10 and higher are called mailbox type
>> registers. Each page represents a mailbox and is used to read and write
>> configuration of particular object (dpll, output, reference & synth).
>>
>> The mailbox page contains mask register that is used to select an index of
>> requested object to work with and semaphore register to indicate what
>> operation is requested.
>>
>> The rest of registers in the particular register page are latch
>> registers that are filled by the firmware during read operation or by
>> the driver prior write operation.
>>
>> For read operation the driver...
>> 1) ... updates the mailbox mask register with index of particular object
>> 2) ... sets the mailbox semaphore register read bit
>> 3) ... waits for the semaphore register read bit to be cleared by FW
>> 4) ... reads the configuration from latch registers
>>
>> For write operation the driver...
>> 1) ... writes the requested configuration to latch registers
>> 2) ... sets the mailbox mask register for the DPLL to be updated
>> 3) ... sets the mailbox semaphore register bit for the write operation
>> 4) ... waits for the semaphore register bit to be cleared by FW
>>
>> Add functions to read and write mailboxes for all supported object types.
>>
>> All these functions as well as functions accessing mailbox latch registers
>> (zl3073x_mb_* functions) have to be called with zl3073x_dev->mailbox_lock
>> held and a caller is responsible to take this lock.
>>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> v1->v3:
>> * dropped ZL3073X_MB_OP macro usage
>> ---
>> drivers/mfd/zl3073x-core.c | 232 +++++++++++++++++++++++
>> include/linux/mfd/zl3073x.h | 12 ++
>> include/linux/mfd/zl3073x_regs.h | 304 +++++++++++++++++++++++++++++++
>> 3 files changed, 548 insertions(+)
>
>> +/*
>> + * Mailbox operations
>> + */
>> +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
>> +int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
>> +int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
>> +int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
>> +int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
>> +int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
>> +int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
>> +int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
>
> Why aren't these being placed into drivers/mailbox?
I think the only common thing of this with drivers/mailbox is only the
name. Mailbox (this comes from datasheet) here is just an atomic way to
read or write some range of registers.
How can be that used here?
Ivan
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH v3 net-next 6/8] mfd: zl3073x: Add clock_id field
2025-04-16 16:21 [PATCH v3 net-next 00/10] Add Microchip ZL3073x support (part 1) Ivan Vecera
` (4 preceding siblings ...)
2025-04-16 16:21 ` [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes Ivan Vecera
@ 2025-04-16 16:21 ` Ivan Vecera
2025-04-16 16:21 ` [PATCH v3 net-next 7/8] mfd: zl3073x: Fetch invariants during probe Ivan Vecera
2025-04-16 16:21 ` [PATCH v3 net-next 8/8] mfd: zl3073x: Register DPLL sub-device during init Ivan Vecera
7 siblings, 0 replies; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 16:21 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
Add .clock_id to zl3073x_dev structure that will be used by later
commits introducing DPLL driver. The clock ID is necessary for DPLL
device registration. To generate such ID use chip ID read during
device initialization for this. For the case where are multiple
zl3073x based chips the chip ID is shifted and lower bits are filled
by an unique value. For I2C case it is I2C device address and for SPI
case it is chip-select value.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/mfd/zl3073x-core.c | 6 +++++-
drivers/mfd/zl3073x-i2c.c | 4 +++-
drivers/mfd/zl3073x-spi.c | 4 +++-
drivers/mfd/zl3073x.h | 2 +-
include/linux/mfd/zl3073x.h | 2 ++
5 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
index 3d18786c769d2..5a0e2bb1a969f 100644
--- a/drivers/mfd/zl3073x-core.c
+++ b/drivers/mfd/zl3073x-core.c
@@ -485,13 +485,14 @@ static void zl3073x_devlink_unregister(void *ptr)
* zl3073x_dev_probe - initialize zl3073x device
* @zldev: pointer to zl3073x device
* @chip_info: chip info based on compatible
+ * @dev_id: device ID to be used as part of clock ID
*
* Common initialization of zl3073x device structure.
*
* Returns: 0 on success, <0 on error
*/
int zl3073x_dev_probe(struct zl3073x_dev *zldev,
- const struct zl3073x_chip_info *chip_info)
+ const struct zl3073x_chip_info *chip_info, u8 dev_id)
{
u16 id, revision, fw_ver;
struct devlink *devlink;
@@ -533,6 +534,9 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
FIELD_GET(GENMASK(15, 8), cfg_ver),
FIELD_GET(GENMASK(7, 0), cfg_ver));
+ /* Use chip ID and given dev ID as clock ID */
+ zldev->clock_id = ((u64)id << 8) | dev_id;
+
/* Register the device as devlink device */
devlink = priv_to_devlink(zldev);
devlink_register(devlink);
diff --git a/drivers/mfd/zl3073x-i2c.c b/drivers/mfd/zl3073x-i2c.c
index 76bc9a0463180..ae0ad6b7b0129 100644
--- a/drivers/mfd/zl3073x-i2c.c
+++ b/drivers/mfd/zl3073x-i2c.c
@@ -29,7 +29,9 @@ static int zl3073x_i2c_probe(struct i2c_client *client)
return PTR_ERR(zldev->regmap);
}
- return zl3073x_dev_probe(zldev, i2c_get_match_data(client));
+ /* Initialize device and use I2C address as dev ID */
+ return zl3073x_dev_probe(zldev, i2c_get_match_data(client),
+ client->addr);
}
static const struct i2c_device_id zl3073x_i2c_id[] = {
diff --git a/drivers/mfd/zl3073x-spi.c b/drivers/mfd/zl3073x-spi.c
index d0fc2d2221c0d..6e6ec88b40d72 100644
--- a/drivers/mfd/zl3073x-spi.c
+++ b/drivers/mfd/zl3073x-spi.c
@@ -29,7 +29,9 @@ static int zl3073x_spi_probe(struct spi_device *spi)
return PTR_ERR(zldev->regmap);
}
- return zl3073x_dev_probe(zldev, spi_get_device_match_data(spi));
+ /* Initialize device and use SPI chip select value as dev ID */
+ return zl3073x_dev_probe(zldev, spi_get_device_match_data(spi),
+ spi_get_chipselect(spi, 0));
}
static const struct spi_device_id zl3073x_spi_id[] = {
diff --git a/drivers/mfd/zl3073x.h b/drivers/mfd/zl3073x.h
index 3a2fea61cf579..abd1ab9a56ded 100644
--- a/drivers/mfd/zl3073x.h
+++ b/drivers/mfd/zl3073x.h
@@ -26,6 +26,6 @@ extern const struct zl3073x_chip_info zl3073x_chip_info[];
struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev);
void zl3073x_dev_init_regmap_config(struct regmap_config *regmap_cfg);
int zl3073x_dev_probe(struct zl3073x_dev *zldev,
- const struct zl3073x_chip_info *chip_info);
+ const struct zl3073x_chip_info *chip_info, u8 dev_id);
#endif /* __ZL3073X_CORE_H */
diff --git a/include/linux/mfd/zl3073x.h b/include/linux/mfd/zl3073x.h
index 53813a8c39660..5074f64a70137 100644
--- a/include/linux/mfd/zl3073x.h
+++ b/include/linux/mfd/zl3073x.h
@@ -13,11 +13,13 @@ struct regmap;
* @dev: pointer to device
* @regmap: regmap to access device registers
* @mailbox_lock: mutex protecting an access to mailbox registers
+ * @clock_id: clock id of the device
*/
struct zl3073x_dev {
struct device *dev;
struct regmap *regmap;
struct mutex mailbox_lock;
+ u64 clock_id;
};
/**
--
2.48.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 net-next 7/8] mfd: zl3073x: Fetch invariants during probe
2025-04-16 16:21 [PATCH v3 net-next 00/10] Add Microchip ZL3073x support (part 1) Ivan Vecera
` (5 preceding siblings ...)
2025-04-16 16:21 ` [PATCH v3 net-next 6/8] mfd: zl3073x: Add clock_id field Ivan Vecera
@ 2025-04-16 16:21 ` Ivan Vecera
2025-04-16 16:21 ` [PATCH v3 net-next 8/8] mfd: zl3073x: Register DPLL sub-device during init Ivan Vecera
7 siblings, 0 replies; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 16:21 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
Several configuration parameters will not be changed in runtime so we
can load them during probe to avoid excessive reads from the hardware.
These parameters will be frequently read by the DPLL driver from this
series and later by PHC/PTP sub-device driver.
Read the following parameters from the device during probe and store
them for later use:
* synthesizers' frequencies and driving DPLL channel
* input pins' enablement and type (single-ended or differential)
* outputs'associated synths, signal format and enablement
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
v2->v3:
* dropped usage of macros for generating helper functions
v1->v2:
* fixed and added inline documentation
---
drivers/mfd/zl3073x-core.c | 237 +++++++++++++++++++++++++++++++
include/linux/mfd/zl3073x.h | 161 +++++++++++++++++++++
include/linux/mfd/zl3073x_regs.h | 184 ++++++++++++++++++++++++
3 files changed, 582 insertions(+)
diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
index 5a0e2bb1a969f..0bd31591245a2 100644
--- a/drivers/mfd/zl3073x-core.c
+++ b/drivers/mfd/zl3073x-core.c
@@ -5,6 +5,7 @@
#include <linux/dev_printk.h>
#include <linux/device.h>
#include <linux/export.h>
+#include <linux/math64.h>
#include <linux/mfd/zl3073x.h>
#include <linux/mfd/zl3073x_regs.h>
#include <linux/module.h>
@@ -481,6 +482,237 @@ static void zl3073x_devlink_unregister(void *ptr)
devlink_unregister(ptr);
}
+/**
+ * zl3073x_input_state_fetch - get input state
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: input pin index to fetch state for
+ *
+ * Function fetches information for the given input reference that are
+ * invariant and stores them for later use.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_input_state_fetch(struct zl3073x_dev *zldev, u8 index)
+{
+ struct zl3073x_input *input;
+ u8 ref_config;
+ int rc;
+
+ if (index >= ZL3073X_NUM_INPUTS)
+ return -EINVAL;
+
+ input = &zldev->input[index];
+
+ /* If the input is differential then the configuration for N-pin
+ * reference is ignored and P-pin config is used for both.
+ */
+ if (zl3073x_is_n_pin(index) &&
+ zl3073x_input_is_diff(zldev, index - 1)) {
+ input->enabled = zl3073x_input_is_enabled(zldev, index - 1);
+ input->diff = true;
+
+ return 0;
+ }
+
+ guard(zl3073x_mailbox)(zldev);
+
+ /* Read reference configuration into mailbox */
+ rc = zl3073x_mb_ref_read(zldev, index);
+ if (rc)
+ return rc;
+
+ /* Read reference config register */
+ rc = zl3073x_mb_read_ref_config(zldev, &ref_config);
+ if (rc)
+ return rc;
+
+ /* Store info about input reference enablement and if it is
+ * configured in differential mode or not.
+ */
+ input->enabled = FIELD_GET(ZL_REF_CONFIG_ENABLE, ref_config);
+ input->diff = FIELD_GET(ZL_REF_CONFIG_DIFF_EN, ref_config);
+
+ dev_dbg(zldev->dev, "INPUT%u is %sabled and configured as %s\n", index,
+ input->enabled ? "en" : "dis",
+ input->diff ? "differential" : "single-ended");
+
+ return rc;
+}
+
+/**
+ * zl3073x_output_state_fetch - get output state
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: output index to fetch state for
+ *
+ * Function fetches information for the given output (not output pin)
+ * that are invariant and stores them for later use.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_output_state_fetch(struct zl3073x_dev *zldev, u8 index)
+{
+ struct zl3073x_output *output;
+ u8 output_ctrl, output_mode;
+ int rc;
+
+ if (index >= ZL3073X_NUM_OUTPUTS)
+ return -EINVAL;
+
+ output = &zldev->output[index];
+
+ /* Read output control register */
+ rc = zl3073x_read_output_ctrl(zldev, index, &output_ctrl);
+ if (rc)
+ return rc;
+
+ /* Store info about output enablement and synthesizer the output
+ * is connected to.
+ */
+ output->enabled = FIELD_GET(ZL_OUTPUT_CTRL_EN, output_ctrl);
+ output->synth = FIELD_GET(ZL_OUTPUT_CTRL_SYNTH_SEL, output_ctrl);
+
+ dev_dbg(zldev->dev, "OUTPUT%u is %sabled, connected to SYNTH%u\n",
+ index, output->enabled ? "en" : "dis", output->synth);
+
+ guard(zl3073x_mailbox)(zldev);
+
+ /* Load given output config into mailbox */
+ rc = zl3073x_mb_output_read(zldev, index);
+ if (rc)
+ return rc;
+
+ /* Read output mode from mailbox */
+ rc = zl3073x_mb_read_output_mode(zldev, &output_mode);
+ if (rc)
+ return rc;
+
+ /* Extract and store output signal format */
+ output->signal_format = FIELD_GET(ZL_OUTPUT_MODE_SIGNAL_FORMAT,
+ output_mode);
+
+ dev_dbg(zldev->dev, "OUTPUT%u has signal format 0x%02x\n", index,
+ output->signal_format);
+
+ return rc;
+}
+
+/**
+ * zl3073x_synth_state_fetch - get synth state
+ * @zldev: pointer to zl3073x_dev structure
+ * @index: synth index to fetch state for
+ *
+ * Function fetches information for the given synthesizer that are
+ * invariant and stores them for later use.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_synth_state_fetch(struct zl3073x_dev *zldev, u8 index)
+{
+ u16 base, numerator, denominator;
+ u8 synth_ctrl;
+ u32 mult;
+ int rc;
+
+ /* Read synth control register */
+ rc = zl3073x_read_synth_ctrl(zldev, index, &synth_ctrl);
+ if (rc)
+ return rc;
+
+ /* Extract and store DPLL channel the synth is driven by */
+ zldev->synth[index].dpll = FIELD_GET(ZL_SYNTH_CTRL_DPLL_SEL,
+ synth_ctrl);
+
+ dev_dbg(zldev->dev, "SYNTH%u is connected to DPLL%u\n", index,
+ zldev->synth[index].dpll);
+
+ guard(zl3073x_mailbox)(zldev);
+
+ /* Read synth configuration into mailbox */
+ rc = zl3073x_mb_synth_read(zldev, index);
+ if (rc)
+ return rc;
+
+ /* The output frequency is determined by the following formula:
+ * base * multiplier * numerator / denominator
+ *
+ * Therefore get all this number and calculate the output frequency
+ */
+ rc = zl3073x_mb_read_synth_freq_base(zldev, &base);
+ if (rc)
+ return rc;
+
+ rc = zl3073x_mb_read_synth_freq_mult(zldev, &mult);
+ if (rc)
+ return rc;
+
+ rc = zl3073x_mb_read_synth_freq_m(zldev, &numerator);
+ if (rc)
+ return rc;
+
+ rc = zl3073x_mb_read_synth_freq_n(zldev, &denominator);
+ if (rc)
+ return rc;
+
+ /* Check denominator for zero to avoid div by 0 */
+ if (!denominator) {
+ dev_err(zldev->dev,
+ "Zero divisor for SYNTH%u retrieved from device\n",
+ index);
+ return -EINVAL;
+ }
+
+ /* Compute and store synth frequency */
+ zldev->synth[index].freq = mul_u64_u32_div(mul_u32_u32(base, mult),
+ numerator, denominator);
+
+ dev_dbg(zldev->dev, "SYNTH%u frequency: %llu Hz\n", index,
+ zldev->synth[index].freq);
+
+ return rc;
+}
+
+static int
+zl3073x_dev_state_fetch(struct zl3073x_dev *zldev)
+{
+ int rc;
+ u8 i;
+
+ for (i = 0; i < ZL3073X_NUM_INPUTS; i++) {
+ rc = zl3073x_input_state_fetch(zldev, i);
+ if (rc) {
+ dev_err(zldev->dev,
+ "Failed to fetch input state: %pe\n",
+ ERR_PTR(rc));
+ return rc;
+ }
+ }
+
+ for (i = 0; i < ZL3073X_NUM_SYNTHS; i++) {
+ rc = zl3073x_synth_state_fetch(zldev, i);
+ if (rc) {
+ dev_err(zldev->dev,
+ "Failed to fetch synth state: %pe\n",
+ ERR_PTR(rc));
+ return rc;
+ }
+ }
+
+ for (i = 0; i < ZL3073X_NUM_OUTPUTS; i++) {
+ rc = zl3073x_output_state_fetch(zldev, i);
+ if (rc) {
+ dev_err(zldev->dev,
+ "Failed to fetch output state: %pe\n",
+ ERR_PTR(rc));
+ return rc;
+ }
+ }
+
+ return rc;
+}
+
/**
* zl3073x_dev_probe - initialize zl3073x device
* @zldev: pointer to zl3073x device
@@ -537,6 +769,11 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
/* Use chip ID and given dev ID as clock ID */
zldev->clock_id = ((u64)id << 8) | dev_id;
+ /* Fetch device state */
+ rc = zl3073x_dev_state_fetch(zldev);
+ if (rc)
+ return rc;
+
/* Register the device as devlink device */
devlink = priv_to_devlink(zldev);
devlink_register(devlink);
diff --git a/include/linux/mfd/zl3073x.h b/include/linux/mfd/zl3073x.h
index 5074f64a70137..c77d0815b4dfd 100644
--- a/include/linux/mfd/zl3073x.h
+++ b/include/linux/mfd/zl3073x.h
@@ -8,18 +8,75 @@
struct device;
struct regmap;
+/*
+ * Hardware limits for ZL3073x chip family
+ */
+#define ZL3073X_NUM_INPUTS 10
+#define ZL3073X_NUM_OUTPUTS 10
+#define ZL3073X_NUM_SYNTHS 5
+
+/**
+ * struct zl3073x_input - input invariant info
+ * @enabled: input is enabled or disabled
+ * @diff: true if input is differential
+ */
+struct zl3073x_input {
+ bool enabled;
+ bool diff;
+};
+
+/**
+ * struct zl3073x_output - output invariant info
+ * @enabled: output is enabled or disabled
+ * @synth: synthesizer the output is connected to
+ * @signal_format: output signal format
+ */
+struct zl3073x_output {
+ bool enabled;
+ u8 synth;
+ u8 signal_format;
+#define ZL_OUTPUT_SIGNAL_FORMAT_DISABLED 0
+#define ZL_OUTPUT_SIGNAL_FORMAT_LVDS 1
+#define ZL_OUTPUT_SIGNAL_FORMAT_DIFFERENTIAL 2
+#define ZL_OUTPUT_SIGNAL_FORMAT_LOWVCM 3
+#define ZL_OUTPUT_SIGNAL_FORMAT_TWO 4
+#define ZL_OUTPUT_SIGNAL_FORMAT_ONE_P 5
+#define ZL_OUTPUT_SIGNAL_FORMAT_ONE_N 6
+#define ZL_OUTPUT_SIGNAL_FORMAT_TWO_INV 7
+#define ZL_OUTPUT_SIGNAL_FORMAT_TWO_N_DIV 12
+#define ZL_OUTPUT_SIGNAL_FORMAT_TWO_N_DIV_INV 15
+};
+
+/**
+ * struct zl3073x_synth - synthesizer invariant info
+ * @freq: synthesizer frequency
+ * @dpll: ID of DPLL the synthesizer is driven by
+ */
+struct zl3073x_synth {
+ u64 freq;
+ u8 dpll;
+};
+
/**
* struct zl3073x_dev - zl3073x device
* @dev: pointer to device
* @regmap: regmap to access device registers
* @mailbox_lock: mutex protecting an access to mailbox registers
* @clock_id: clock id of the device
+ * @input: array of inputs' invariants
+ * @output: array of outputs' invariants
+ * @synth: array of synthesizers' invariants
*/
struct zl3073x_dev {
struct device *dev;
struct regmap *regmap;
struct mutex mailbox_lock;
u64 clock_id;
+
+ /* Invariants */
+ struct zl3073x_input input[ZL3073X_NUM_INPUTS];
+ struct zl3073x_output output[ZL3073X_NUM_OUTPUTS];
+ struct zl3073x_synth synth[ZL3073X_NUM_SYNTHS];
};
/**
@@ -61,4 +118,108 @@ int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
+static inline
+bool zl3073x_is_n_pin(u8 index)
+{
+ /* P-pins indices are even while N-pins are odd */
+ return index & 1;
+}
+
+static inline
+bool zl3073x_is_p_pin(u8 index)
+{
+ return !zl3073x_is_n_pin(index);
+}
+
+/**
+ * zl3073x_input_is_diff - check if the given input ref is differential
+ * @zldev: pointer to zl3073x device
+ * @index: output index
+ *
+ * Return: true if input is differential, false if input is single-ended
+ */
+static inline
+bool zl3073x_input_is_diff(struct zl3073x_dev *zldev, u8 index)
+{
+ return zldev->input[index].diff;
+}
+
+/**
+ * zl3073x_input_is_enabled - check if the given input ref is enabled
+ * @zldev: pointer to zl3073x device
+ * @index: input index
+ *
+ * Return: true if input is enabled, false if input is disabled
+ */
+static inline
+bool zl3073x_input_is_enabled(struct zl3073x_dev *zldev, u8 index)
+{
+ return zldev->input[index].enabled;
+}
+
+/**
+ * zl3073x_output_is_enabled - check if the given output is enabled
+ * @zldev: pointer to zl3073x device
+ * @index: output index
+ *
+ * Return: true if output is enabled, false if output is disabled
+ */
+static inline
+u8 zl3073x_output_is_enabled(struct zl3073x_dev *zldev, u8 index)
+{
+ return zldev->output[index].enabled;
+}
+
+/**
+ * zl3073x_output_signal_format_get - get output signal format
+ * @zldev: pointer to zl3073x device
+ * @index: output index
+ *
+ * Return: signal format of given output
+ */
+static inline
+u8 zl3073x_output_signal_format_get(struct zl3073x_dev *zldev, u8 index)
+{
+ return zldev->output[index].signal_format;
+}
+
+/**
+ * zl3073x_output_synth_get - get synth connected to given output
+ * @zldev: pointer to zl3073x device
+ * @index: output index
+ *
+ * Return: index of synth connected to given output.
+ */
+static inline
+u8 zl3073x_output_synth_get(struct zl3073x_dev *zldev, u8 index)
+{
+ return zldev->output[index].synth;
+}
+
+/**
+ * zl3073x_synth_dpll_get - get DPLL ID the synth is driven by
+ * @zldev: pointer to zl3073x device
+ * @index: synth index
+ *
+ * Return: ID of DPLL the given synthetizer is driven by
+ */
+static inline
+u64 zl3073x_synth_dpll_get(struct zl3073x_dev *zldev, u8 index)
+{
+ return zldev->synth[index].dpll;
+}
+
+/**
+ * zl3073x_synth_freq_get - get synth current freq
+ * @zldev: pointer to zl3073x device
+ * @index: synth index
+ *
+ * Return: frequency of given synthetizer
+ */
+static inline
+u64 zl3073x_synth_freq_get(struct zl3073x_dev *zldev, u8 index)
+{
+ return zldev->synth[index].freq;
+}
+
#endif /* __LINUX_MFD_ZL3073X_H */
diff --git a/include/linux/mfd/zl3073x_regs.h b/include/linux/mfd/zl3073x_regs.h
index d155650349c97..ff5b2e6fae9d0 100644
--- a/include/linux/mfd/zl3073x_regs.h
+++ b/include/linux/mfd/zl3073x_regs.h
@@ -106,6 +106,63 @@ zl3073x_read_custom_config_ver(struct zl3073x_dev *zldev, u32 *value)
return rc;
}
+/***********************************
+ * Register Page 9, Synth and Output
+ ***********************************/
+
+/*
+ * Register array 'synth_ctrl'
+ * Page: 9, Offset: 0x00, Size: 8 bits, Items: 5, Stride: 1
+ */
+#define ZL_REG_SYNTH_CTRL ZL_REG_ADDR(9, 0x00)
+#define ZL_REG_SYNTH_CTRL_ITEMS 5
+#define ZL_REG_SYNTH_CTRL_STRIDE 1
+#define ZL_SYNTH_CTRL_EN BIT(0)
+#define ZL_SYNTH_CTRL_DPLL_SEL GENMASK(6, 4)
+
+static inline __maybe_unused int
+zl3073x_read_synth_ctrl(struct zl3073x_dev *zldev, unsigned int idx, u8 *value)
+{
+ unsigned int addr, v;
+ int rc;
+
+ if (idx >= ZL_REG_SYNTH_CTRL_ITEMS)
+ return -EINVAL;
+
+ addr = ZL_REG_SYNTH_CTRL + idx * ZL_REG_SYNTH_CTRL_STRIDE;
+ rc = regmap_read(zldev->regmap, addr, &v);
+ *value = v;
+ return rc;
+}
+
+/*
+ * Register array 'output_ctrl'
+ * Page: 9, Offset: 0x28, Size: 8 bits, Items: 10, Stride: 1
+ */
+#define ZL_REG_OUTPUT_CTRL ZL_REG_ADDR(9, 0x28)
+#define ZL_REG_OUTPUT_CTRL_ITEMS 10
+#define ZL_REG_OUTPUT_CTRL_STRIDE 1
+#define ZL_OUTPUT_CTRL_EN BIT(0)
+#define ZL_OUTPUT_CTRL_STOP BIT(1)
+#define ZL_OUTPUT_CTRL_STOP_HIGH BIT(2)
+#define ZL_OUTPUT_CTRL_STOP_HZ BIT(3)
+#define ZL_OUTPUT_CTRL_SYNTH_SEL GENMASK(6, 4)
+
+static inline __maybe_unused int
+zl3073x_read_output_ctrl(struct zl3073x_dev *zldev, unsigned int idx, u8 *value)
+{
+ unsigned int addr, v;
+ int rc;
+
+ if (idx >= ZL_REG_OUTPUT_CTRL_ITEMS)
+ return -EINVAL;
+
+ addr = ZL_REG_OUTPUT_CTRL + idx * ZL_REG_OUTPUT_CTRL_STRIDE;
+ rc = regmap_read(zldev->regmap, addr, &v);
+ *value = v;
+ return rc;
+}
+
/*******************************
* Register Page 10, Ref Mailbox
*******************************/
@@ -181,6 +238,26 @@ zl3073x_mb_poll_ref_mb_sem(struct zl3073x_dev *zldev, u8 bitmask)
ZL_POLL_TIMEOUT_US);
}
+/*
+ * Register 'ref_config'
+ * Page: 10, Offset: 0x0d, Size: 8 bits
+ */
+#define ZL_REG_REF_CONFIG ZL_REG_ADDR(10, 0x0d)
+#define ZL_REF_CONFIG_ENABLE BIT(0)
+#define ZL_REF_CONFIG_DIFF_EN BIT(2)
+
+static inline __maybe_unused int
+zl3073x_mb_read_ref_config(struct zl3073x_dev *zldev, u8 *value)
+{
+ unsigned int v;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_read(zldev->regmap, ZL_REG_REF_CONFIG, &v);
+ *value = v;
+ return rc;
+}
+
/********************************
* Register Page 12, DPLL Mailbox
********************************/
@@ -331,6 +408,94 @@ zl3073x_mb_poll_synth_mb_sem(struct zl3073x_dev *zldev, u8 bitmask)
ZL_POLL_TIMEOUT_US);
}
+/*
+ * Register 'synth_freq_base'
+ * Page: 13, Offset: 0x06, Size: 16 bits
+ */
+#define ZL_REG_SYNTH_FREQ_BASE ZL_REG_ADDR(13, 0x06)
+
+static inline __maybe_unused int
+zl3073x_mb_read_synth_freq_base(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_SYNTH_FREQ_BASE, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+/*
+ * Register 'synth_freq_mult'
+ * Page: 13, Offset: 0x08, Size: 32 bits
+ */
+#define ZL_REG_SYNTH_FREQ_MULT ZL_REG_ADDR(13, 0x08)
+
+static inline __maybe_unused int
+zl3073x_mb_read_synth_freq_mult(struct zl3073x_dev *zldev, u32 *value)
+{
+ __be32 temp;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_SYNTH_FREQ_MULT, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be32_to_cpu(temp);
+ return rc;
+}
+
+/*
+ * Register 'synth_freq_m'
+ * Page: 13, Offset: 0x0c, Size: 16 bits
+ */
+#define ZL_REG_SYNTH_FREQ_M ZL_REG_ADDR(13, 0x0c)
+
+static inline __maybe_unused int
+zl3073x_mb_read_synth_freq_m(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_SYNTH_FREQ_M, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
+/*
+ * Register 'synth_freq_n'
+ * Page: 13, Offset: 0x0e, Size: 16 bits
+ */
+#define ZL_REG_SYNTH_FREQ_N ZL_REG_ADDR(13, 0x0e)
+
+static inline __maybe_unused int
+zl3073x_mb_read_synth_freq_n(struct zl3073x_dev *zldev, u16 *value)
+{
+ __be16 temp;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_bulk_read(zldev->regmap, ZL_REG_SYNTH_FREQ_N, &temp,
+ sizeof(temp));
+ if (rc)
+ return rc;
+
+ *value = be16_to_cpu(temp);
+ return rc;
+}
+
/**********************************
* Register Page 14, Output Mailbox
**********************************/
@@ -406,4 +571,23 @@ zl3073x_mb_poll_output_mb_sem(struct zl3073x_dev *zldev, u8 bitmask)
ZL_POLL_TIMEOUT_US);
}
+/*
+ * Register 'output_mode'
+ * Page: 14, Offset: 0x05, Size: 8 bits
+ */
+#define ZL_REG_OUTPUT_MODE ZL_REG_ADDR(14, 0x05)
+#define ZL_OUTPUT_MODE_SIGNAL_FORMAT GENMASK(7, 4)
+
+static inline __maybe_unused int
+zl3073x_mb_read_output_mode(struct zl3073x_dev *zldev, u8 *value)
+{
+ unsigned int v;
+ int rc;
+
+ lockdep_assert_held(&zldev->mailbox_lock);
+ rc = regmap_read(zldev->regmap, ZL_REG_OUTPUT_MODE, &v);
+ *value = v;
+ return rc;
+}
+
#endif /* __LINUX_MFD_ZL3073X_REGS_H */
--
2.48.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH v3 net-next 8/8] mfd: zl3073x: Register DPLL sub-device during init
2025-04-16 16:21 [PATCH v3 net-next 00/10] Add Microchip ZL3073x support (part 1) Ivan Vecera
` (6 preceding siblings ...)
2025-04-16 16:21 ` [PATCH v3 net-next 7/8] mfd: zl3073x: Fetch invariants during probe Ivan Vecera
@ 2025-04-16 16:21 ` Ivan Vecera
2025-04-17 16:20 ` Lee Jones
7 siblings, 1 reply; 39+ messages in thread
From: Ivan Vecera @ 2025-04-16 16:21 UTC (permalink / raw)
To: netdev
Cc: Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Prathosh Satish, Lee Jones,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
Register DPLL sub-devices to expose this functionality provided
by ZL3073x chip family. Each sub-device represents one of the provided
DPLL channels.
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
drivers/mfd/zl3073x-core.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
index 0bd31591245a2..fda77724a8452 100644
--- a/drivers/mfd/zl3073x-core.c
+++ b/drivers/mfd/zl3073x-core.c
@@ -6,6 +6,7 @@
#include <linux/device.h>
#include <linux/export.h>
#include <linux/math64.h>
+#include <linux/mfd/core.h>
#include <linux/mfd/zl3073x.h>
#include <linux/mfd/zl3073x_regs.h>
#include <linux/module.h>
@@ -774,6 +775,20 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
if (rc)
return rc;
+ /* Add DPLL sub-device cell for each DPLL channel */
+ for (i = 0; i < chip_info->num_channels; i++) {
+ struct mfd_cell dpll_dev = MFD_CELL_BASIC("zl3073x-dpll", NULL,
+ NULL, 0, i);
+
+ rc = devm_mfd_add_devices(zldev->dev, PLATFORM_DEVID_AUTO,
+ &dpll_dev, 1, NULL, 0, NULL);
+ if (rc) {
+ dev_err_probe(zldev->dev, rc,
+ "Failed to add DPLL sub-device\n");
+ return rc;
+ }
+ }
+
/* Register the device as devlink device */
devlink = priv_to_devlink(zldev);
devlink_register(devlink);
--
2.48.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 8/8] mfd: zl3073x: Register DPLL sub-device during init
2025-04-16 16:21 ` [PATCH v3 net-next 8/8] mfd: zl3073x: Register DPLL sub-device during init Ivan Vecera
@ 2025-04-17 16:20 ` Lee Jones
2025-04-17 16:40 ` Ivan Vecera
0 siblings, 1 reply; 39+ messages in thread
From: Lee Jones @ 2025-04-17 16:20 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
On Wed, 16 Apr 2025, Ivan Vecera wrote:
> Register DPLL sub-devices to expose this functionality provided
> by ZL3073x chip family. Each sub-device represents one of the provided
> DPLL channels.
>
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---
> drivers/mfd/zl3073x-core.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
> index 0bd31591245a2..fda77724a8452 100644
> --- a/drivers/mfd/zl3073x-core.c
> +++ b/drivers/mfd/zl3073x-core.c
> @@ -6,6 +6,7 @@
> #include <linux/device.h>
> #include <linux/export.h>
> #include <linux/math64.h>
> +#include <linux/mfd/core.h>
> #include <linux/mfd/zl3073x.h>
> #include <linux/mfd/zl3073x_regs.h>
> #include <linux/module.h>
> @@ -774,6 +775,20 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
> if (rc)
> return rc;
>
> + /* Add DPLL sub-device cell for each DPLL channel */
> + for (i = 0; i < chip_info->num_channels; i++) {
> + struct mfd_cell dpll_dev = MFD_CELL_BASIC("zl3073x-dpll", NULL,
> + NULL, 0, i);
Create a static one of these with the maximum amount of channels.
> +
> + rc = devm_mfd_add_devices(zldev->dev, PLATFORM_DEVID_AUTO,
> + &dpll_dev, 1, NULL, 0, NULL);
Then pass chip_info->num_channels as the 4th argument.
> + if (rc) {
> + dev_err_probe(zldev->dev, rc,
> + "Failed to add DPLL sub-device\n");
> + return rc;
> + }
> + }
> +
> /* Register the device as devlink device */
> devlink = priv_to_devlink(zldev);
> devlink_register(devlink);
> --
> 2.48.1
>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 8/8] mfd: zl3073x: Register DPLL sub-device during init
2025-04-17 16:20 ` Lee Jones
@ 2025-04-17 16:40 ` Ivan Vecera
2025-04-24 15:34 ` Lee Jones
0 siblings, 1 reply; 39+ messages in thread
From: Ivan Vecera @ 2025-04-17 16:40 UTC (permalink / raw)
To: Lee Jones
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
On 17. 04. 25 6:20 odp., Lee Jones wrote:
> On Wed, 16 Apr 2025, Ivan Vecera wrote:
>
>> Register DPLL sub-devices to expose this functionality provided
>> by ZL3073x chip family. Each sub-device represents one of the provided
>> DPLL channels.
>>
>> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
>> ---
>> drivers/mfd/zl3073x-core.c | 15 +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
>> index 0bd31591245a2..fda77724a8452 100644
>> --- a/drivers/mfd/zl3073x-core.c
>> +++ b/drivers/mfd/zl3073x-core.c
>> @@ -6,6 +6,7 @@
>> #include <linux/device.h>
>> #include <linux/export.h>
>> #include <linux/math64.h>
>> +#include <linux/mfd/core.h>
>> #include <linux/mfd/zl3073x.h>
>> #include <linux/mfd/zl3073x_regs.h>
>> #include <linux/module.h>
>> @@ -774,6 +775,20 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
>> if (rc)
>> return rc;
>>
>> + /* Add DPLL sub-device cell for each DPLL channel */
>> + for (i = 0; i < chip_info->num_channels; i++) {
>> + struct mfd_cell dpll_dev = MFD_CELL_BASIC("zl3073x-dpll", NULL,
>> + NULL, 0, i);
>
> Create a static one of these with the maximum amount of channels.
Like this?
static const struct mfd_cell dpll_cells[] = {
MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 5),
};
rc = devm_mfd_add_devices(zldev->dev, PLATFORM_DEVID_AUTO, dpll_cells,
chip_info->num_channels, NULL, 0, NULL);
Ivan
>
>> +
>> + rc = devm_mfd_add_devices(zldev->dev, PLATFORM_DEVID_AUTO,
>> + &dpll_dev, 1, NULL, 0, NULL);
>
> Then pass chip_info->num_channels as the 4th argument.
>
>> + if (rc) {
>> + dev_err_probe(zldev->dev, rc,
>> + "Failed to add DPLL sub-device\n");
>> + return rc;
>> + }
>> + }
>> +
>> /* Register the device as devlink device */
>> devlink = priv_to_devlink(zldev);
>> devlink_register(devlink);
>> --
>> 2.48.1
>>
>
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 8/8] mfd: zl3073x: Register DPLL sub-device during init
2025-04-17 16:40 ` Ivan Vecera
@ 2025-04-24 15:34 ` Lee Jones
2025-04-24 15:36 ` Lee Jones
0 siblings, 1 reply; 39+ messages in thread
From: Lee Jones @ 2025-04-24 15:34 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
On Thu, 17 Apr 2025, Ivan Vecera wrote:
>
>
> On 17. 04. 25 6:20 odp., Lee Jones wrote:
> > On Wed, 16 Apr 2025, Ivan Vecera wrote:
> >
> > > Register DPLL sub-devices to expose this functionality provided
> > > by ZL3073x chip family. Each sub-device represents one of the provided
> > > DPLL channels.
> > >
> > > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > > ---
> > > drivers/mfd/zl3073x-core.c | 15 +++++++++++++++
> > > 1 file changed, 15 insertions(+)
> > >
> > > diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
> > > index 0bd31591245a2..fda77724a8452 100644
> > > --- a/drivers/mfd/zl3073x-core.c
> > > +++ b/drivers/mfd/zl3073x-core.c
> > > @@ -6,6 +6,7 @@
> > > #include <linux/device.h>
> > > #include <linux/export.h>
> > > #include <linux/math64.h>
> > > +#include <linux/mfd/core.h>
> > > #include <linux/mfd/zl3073x.h>
> > > #include <linux/mfd/zl3073x_regs.h>
> > > #include <linux/module.h>
> > > @@ -774,6 +775,20 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
> > > if (rc)
> > > return rc;
> > > + /* Add DPLL sub-device cell for each DPLL channel */
> > > + for (i = 0; i < chip_info->num_channels; i++) {
> > > + struct mfd_cell dpll_dev = MFD_CELL_BASIC("zl3073x-dpll", NULL,
> > > + NULL, 0, i);
> >
> > Create a static one of these with the maximum amount of channels.
>
> Like this?
>
> static const struct mfd_cell dpll_cells[] = {
> MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
> MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
> MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
> MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
> MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 5),
> };
>
> rc = devm_mfd_add_devices(zldev->dev, PLATFORM_DEVID_AUTO, dpll_cells,
> chip_info->num_channels, NULL, 0, NULL);
Yes, looks better, thank you.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH v3 net-next 8/8] mfd: zl3073x: Register DPLL sub-device during init
2025-04-24 15:34 ` Lee Jones
@ 2025-04-24 15:36 ` Lee Jones
0 siblings, 0 replies; 39+ messages in thread
From: Lee Jones @ 2025-04-24 15:36 UTC (permalink / raw)
To: Ivan Vecera
Cc: netdev, Vadim Fedorenko, Arkadiusz Kubalewski, Jiri Pirko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Prathosh Satish,
Kees Cook, Andy Shevchenko, Andrew Morton, Michal Schmidt,
devicetree, linux-kernel, linux-hardening
On Thu, 24 Apr 2025, Lee Jones wrote:
> On Thu, 17 Apr 2025, Ivan Vecera wrote:
>
> >
> >
> > On 17. 04. 25 6:20 odp., Lee Jones wrote:
> > > On Wed, 16 Apr 2025, Ivan Vecera wrote:
> > >
> > > > Register DPLL sub-devices to expose this functionality provided
> > > > by ZL3073x chip family. Each sub-device represents one of the provided
> > > > DPLL channels.
> > > >
> > > > Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> > > > ---
> > > > drivers/mfd/zl3073x-core.c | 15 +++++++++++++++
> > > > 1 file changed, 15 insertions(+)
> > > >
> > > > diff --git a/drivers/mfd/zl3073x-core.c b/drivers/mfd/zl3073x-core.c
> > > > index 0bd31591245a2..fda77724a8452 100644
> > > > --- a/drivers/mfd/zl3073x-core.c
> > > > +++ b/drivers/mfd/zl3073x-core.c
> > > > @@ -6,6 +6,7 @@
> > > > #include <linux/device.h>
> > > > #include <linux/export.h>
> > > > #include <linux/math64.h>
> > > > +#include <linux/mfd/core.h>
> > > > #include <linux/mfd/zl3073x.h>
> > > > #include <linux/mfd/zl3073x_regs.h>
> > > > #include <linux/module.h>
> > > > @@ -774,6 +775,20 @@ int zl3073x_dev_probe(struct zl3073x_dev *zldev,
> > > > if (rc)
> > > > return rc;
> > > > + /* Add DPLL sub-device cell for each DPLL channel */
> > > > + for (i = 0; i < chip_info->num_channels; i++) {
> > > > + struct mfd_cell dpll_dev = MFD_CELL_BASIC("zl3073x-dpll", NULL,
> > > > + NULL, 0, i);
> > >
> > > Create a static one of these with the maximum amount of channels.
> >
> > Like this?
> >
> > static const struct mfd_cell dpll_cells[] = {
> > MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 1),
> > MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 2),
> > MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 3),
> > MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 4),
> > MFD_CELL_BASIC("zl3073x-dpll", NULL, NULL, 0, 5),
> > };
> >
> > rc = devm_mfd_add_devices(zldev->dev, PLATFORM_DEVID_AUTO, dpll_cells,
> > chip_info->num_channels, NULL, 0, NULL);
>
> Yes, looks better, thank you.
Sorry, just a thought. Since this is non-standard, please make it easy
on the reader by providing a comment to say that this call will register
a variable number of devices depending on the value of num_channels.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 39+ messages in thread