Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
provides DPLL and PTP functionality. This series bring first part
that adds the common MFD driver that provides an access to the bus
that can be either I2C or SPI.

The next series will bring the DPLL driver that will covers DPLL
functionality. And another ones will bring PTP driver and flashing
capability via devlink.

Testing was done by myself and by Prathosh Satish on Microchip EDS2
development board with ZL30732 DPLL chip connected over I2C bus.

Patch breakdown
===============
Patch 1 - Common DT schema for DPLL device and pin
Patch 3 - Basic support for I2C, SPI and regmap
Patch 4 - Devlink registration
Patches 5-6 - Helpers for accessing device registers
Patches 7-8 - Component versions reporting via devlink dev info
Patches 9-10 - Helpers for accessing register mailboxes
Patch 11 - Clock ID generation for DPLL driver
Patch 12 - Export strnchrnul function for modules
           (used by next patch)
Patch 13 - Support for MFG config initialization file
Patch 14 - Fetch invariant register values used by DPLL and later by
           PTP driver

Ivan Vecera (14):
  dt-bindings: dpll: Add device tree bindings for DPLL device and pin
  dt-bindings: dpll: Add support for Microchip Azurite chip family
  mfd: Add Microchip ZL3073x support
  mfd: zl3073x: Register itself as devlink device
  mfd: zl3073x: Add register access helpers
  mfd: zl3073x: Add macros for device registers access
  mfd: zl3073x: Add components versions register defs
  mfd: zl3073x: Implement devlink device info
  mfd: zl3073x: Add macro to wait for register value bits to be cleared
  mfd: zl3073x: Add functions to work with register mailboxes
  mfd: zl3073x: Add clock_id field
  lib: Allow modules to use strnchrnul
  mfd: zl3073x: Load mfg file into HW if it is present
  mfd: zl3073x: Fetch invariants during probe

 .../devicetree/bindings/dpll/dpll-device.yaml |  76 ++
 .../devicetree/bindings/dpll/dpll-pin.yaml    |  44 +
 .../bindings/dpll/microchip,zl3073x-i2c.yaml  |  74 ++
 .../bindings/dpll/microchip,zl3073x-spi.yaml  |  77 ++
 MAINTAINERS                                   |  11 +
 drivers/mfd/Kconfig                           |  32 +
 drivers/mfd/Makefile                          |   5 +
 drivers/mfd/zl3073x-core.c                    | 883 ++++++++++++++++++
 drivers/mfd/zl3073x-i2c.c                     |  59 ++
 drivers/mfd/zl3073x-spi.c                     |  59 ++
 drivers/mfd/zl3073x.h                         |  14 +
 include/linux/mfd/zl3073x.h                   | 363 +++++++
 lib/string.c                                  |   1 +
 13 files changed, 1698 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml
 create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml
 create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x-i2c.yaml
 create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x-spi.yaml
 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

-- 
2.48.1

On Wed, 09 Apr 2025, Ivan Vecera wrote:

> Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> provides DPLL and PTP functionality. This series bring first part
> that adds the common MFD driver that provides an access to the bus
> that can be either I2C or SPI.
> 
> The next series will bring the DPLL driver that will covers DPLL
> functionality. And another ones will bring PTP driver and flashing
> capability via devlink.
> 
> Testing was done by myself and by Prathosh Satish on Microchip EDS2
> development board with ZL30732 DPLL chip connected over I2C bus.
> 
> Patch breakdown
> ===============
> Patch 1 - Common DT schema for DPLL device and pin
> Patch 3 - Basic support for I2C, SPI and regmap
> Patch 4 - Devlink registration
> Patches 5-6 - Helpers for accessing device registers
> Patches 7-8 - Component versions reporting via devlink dev info
> Patches 9-10 - Helpers for accessing register mailboxes
> Patch 11 - Clock ID generation for DPLL driver
> Patch 12 - Export strnchrnul function for modules
>            (used by next patch)
> Patch 13 - Support for MFG config initialization file
> Patch 14 - Fetch invariant register values used by DPLL and later by
>            PTP driver
> 
> Ivan Vecera (14):
>   dt-bindings: dpll: Add device tree bindings for DPLL device and pin
>   dt-bindings: dpll: Add support for Microchip Azurite chip family
>   mfd: Add Microchip ZL3073x support
>   mfd: zl3073x: Register itself as devlink device
>   mfd: zl3073x: Add register access helpers
>   mfd: zl3073x: Add macros for device registers access
>   mfd: zl3073x: Add components versions register defs
>   mfd: zl3073x: Implement devlink device info
>   mfd: zl3073x: Add macro to wait for register value bits to be cleared
>   mfd: zl3073x: Add functions to work with register mailboxes
>   mfd: zl3073x: Add clock_id field
>   lib: Allow modules to use strnchrnul
>   mfd: zl3073x: Load mfg file into HW if it is present
>   mfd: zl3073x: Fetch invariants during probe
> 
>  .../devicetree/bindings/dpll/dpll-device.yaml |  76 ++
>  .../devicetree/bindings/dpll/dpll-pin.yaml    |  44 +
>  .../bindings/dpll/microchip,zl3073x-i2c.yaml  |  74 ++
>  .../bindings/dpll/microchip,zl3073x-spi.yaml  |  77 ++
>  MAINTAINERS                                   |  11 +
>  drivers/mfd/Kconfig                           |  32 +
>  drivers/mfd/Makefile                          |   5 +
>  drivers/mfd/zl3073x-core.c                    | 883 ++++++++++++++++++
>  drivers/mfd/zl3073x-i2c.c                     |  59 ++
>  drivers/mfd/zl3073x-spi.c                     |  59 ++
>  drivers/mfd/zl3073x.h                         |  14 +
>  include/linux/mfd/zl3073x.h                   | 363 +++++++
>  lib/string.c                                  |   1 +
>  13 files changed, 1698 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml
>  create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml
>  create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x-i2c.yaml
>  create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x-spi.yaml
>  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

Not only are all of the added abstractions and ugly MACROs hard to read
and troublesome to maintain, they're also completely unnecessary at this
(driver) level.  Nicely authored, easy to read / maintain code wins over
clever code 95% of the time.  Exporting functions to pass around
pointers to private structures is also a non-starter.

After looking at the code, there doesn't appear to be any inclusion of
the MFD core header.  How can this be an MFD if you do not use the MFD
API?  MFD is not a dumping area for common code and call-backs.  It's a
subsystem used to neatly separate out and share resources between
functionality that just happens to share the same hardware chip.

-- 
Lee Jones [李琼斯]

On Fri, Apr 11, 2025 at 9:26 AM Lee Jones <lee@kernel.org> wrote:
> On Wed, 09 Apr 2025, Ivan Vecera wrote:
> > Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> > provides DPLL and PTP functionality. This series bring first part
> > that adds the common MFD driver that provides an access to the bus
> > that can be either I2C or SPI.
> > [...]
>
> Not only are all of the added abstractions and ugly MACROs hard to read
> and troublesome to maintain, they're also completely unnecessary at this
> (driver) level.  Nicely authored, easy to read / maintain code wins over
> clever code 95% of the time.

Hello Lee,

IMHO defining the registers with the ZL3073X_REG*_DEF macros is both
clever and easy to read / maintain. On one line I can see the register
name, size and address. For the indexed registers also their count and
the stride. It's almost like looking at a datasheet. And the
type-checking for accessing the registers using the correct size is
nice. I even liked the paranoid WARN_ON for checking the index
overflows.

The weak point is the non-obvious usage in call sites. Seeing:
  rc = zl3073x_read_id(zldev, &id);
can be confusing. One will not find the function with cscope or grep.
Nothing immediately suggests that there's macro magic behind it.
What if usage had to be just slightly more explicit?:
  rc = ZL3073X_READ(id, zldev, &id);

I could immediately see that ZL3073X_READ is a macro. Its definition
would be near the definitions of the ZL3073X_REG*_DEF macros, so I
could correctly guess these things are related.
The 1st argument of the ZL3073X_READ macro is the register name.
(There would be a ZL3073X_READ_IDX with one more argument for indexed
registers.)
In vim, having the cursor on the 1st argument (id) and pressing gD
takes me to the corresponding ZL3073X_REG16_DEF line.

Would it still be too ugly in your view?

Michal

On Fri, Apr 11, 2025 at 04:27:08PM +0200, Michal Schmidt wrote:
> On Fri, Apr 11, 2025 at 9:26 AM Lee Jones <lee@kernel.org> wrote:
> > On Wed, 09 Apr 2025, Ivan Vecera wrote:
> > > Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> > > provides DPLL and PTP functionality. This series bring first part
> > > that adds the common MFD driver that provides an access to the bus
> > > that can be either I2C or SPI.
> > > [...]
> >
> > Not only are all of the added abstractions and ugly MACROs hard to read
> > and troublesome to maintain, they're also completely unnecessary at this
> > (driver) level.  Nicely authored, easy to read / maintain code wins over
> > clever code 95% of the time.
> 
> Hello Lee,
> 
> IMHO defining the registers with the ZL3073X_REG*_DEF macros is both
> clever and easy to read / maintain. On one line I can see the register
> name, size and address. For the indexed registers also their count and
> the stride. It's almost like looking at a datasheet. And the
> type-checking for accessing the registers using the correct size is
> nice. I even liked the paranoid WARN_ON for checking the index
> overflows.

If this is much better, define (and upstream) something for everyone to 
use rather than a one-off in some driver. It doesn't matter how great it 
is if it is different from everyone else. The last thing I want to do is 
figure out how register accesses work when looking at an unfamilar 
driver.

> The weak point is the non-obvious usage in call sites. Seeing:
>   rc = zl3073x_read_id(zldev, &id);
> can be confusing. One will not find the function with cscope or grep.

Exactly.

> Nothing immediately suggests that there's macro magic behind it.
> What if usage had to be just slightly more explicit?:
>   rc = ZL3073X_READ(id, zldev, &id);
> 
> I could immediately see that ZL3073X_READ is a macro. Its definition
> would be near the definitions of the ZL3073X_REG*_DEF macros, so I
> could correctly guess these things are related.
> The 1st argument of the ZL3073X_READ macro is the register name.
> (There would be a ZL3073X_READ_IDX with one more argument for indexed
> registers.)
> In vim, having the cursor on the 1st argument (id) and pressing gD
> takes me to the corresponding ZL3073X_REG16_DEF line.
> 
> Would it still be too ugly in your view?

If you have opinions on how register accesses should look, how to do 
them in rust is being defined now.

Rob

On Fri, 11 Apr 2025, Rob Herring wrote:

> On Fri, Apr 11, 2025 at 04:27:08PM +0200, Michal Schmidt wrote:
> > On Fri, Apr 11, 2025 at 9:26 AM Lee Jones <lee@kernel.org> wrote:
> > > On Wed, 09 Apr 2025, Ivan Vecera wrote:
> > > > Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> > > > provides DPLL and PTP functionality. This series bring first part
> > > > that adds the common MFD driver that provides an access to the bus
> > > > that can be either I2C or SPI.
> > > > [...]
> > >
> > > Not only are all of the added abstractions and ugly MACROs hard to read
> > > and troublesome to maintain, they're also completely unnecessary at this
> > > (driver) level.  Nicely authored, easy to read / maintain code wins over
> > > clever code 95% of the time.
> > 
> > Hello Lee,
> > 
> > IMHO defining the registers with the ZL3073X_REG*_DEF macros is both
> > clever and easy to read / maintain. On one line I can see the register
> > name, size and address. For the indexed registers also their count and
> > the stride. It's almost like looking at a datasheet. And the
> > type-checking for accessing the registers using the correct size is
> > nice. I even liked the paranoid WARN_ON for checking the index
> > overflows.
> 
> If this is much better, define (and upstream) something for everyone to 
> use rather than a one-off in some driver. It doesn't matter how great it 
> is if it is different from everyone else. The last thing I want to do is 
> figure out how register accesses work when looking at an unfamilar 
> driver.

Exactly right.  The issue isn't that defining register accesses using
MACROs is a bad idea generally.  I've seen it done before in downstream
BSPs and the like.  It's that this solution isn't following the
conventions already in carved for such things in the Mainline kernel.
To engineers already used to the current conventions, this is much
harder to follow and interact with.

As Rob says, if this is truly better, discuss the idea with a much
wider audience and have it applied broadly across all areas.  We shall
not be bucking the trend or trend setting here in MFD.

-- 
Lee Jones [李琼斯]

On Fri, Apr 11, 2025 at 4:27 PM Michal Schmidt <mschmidt@redhat.com> wrote:
> On Fri, Apr 11, 2025 at 9:26 AM Lee Jones <lee@kernel.org> wrote:
> > On Wed, 09 Apr 2025, Ivan Vecera wrote:
> > > Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> > > provides DPLL and PTP functionality. This series bring first part
> > > that adds the common MFD driver that provides an access to the bus
> > > that can be either I2C or SPI.
> > > [...]
> >
> > Not only are all of the added abstractions and ugly MACROs hard to read
> > and troublesome to maintain, they're also completely unnecessary at this
> > (driver) level.  Nicely authored, easy to read / maintain code wins over
> > clever code 95% of the time.
>
> Hello Lee,
>
> IMHO defining the registers with the ZL3073X_REG*_DEF macros is both
> clever and easy to read / maintain. On one line I can see the register
> name, size and address. For the indexed registers also their count and
> the stride. It's almost like looking at a datasheet. And the
> type-checking for accessing the registers using the correct size is
> nice. I even liked the paranoid WARN_ON for checking the index
> overflows.
>
> The weak point is the non-obvious usage in call sites. Seeing:
>   rc = zl3073x_read_id(zldev, &id);
> can be confusing. One will not find the function with cscope or grep.
> Nothing immediately suggests that there's macro magic behind it.
> What if usage had to be just slightly more explicit?:
>   rc = ZL3073X_READ(id, zldev, &id);
>
> I could immediately see that ZL3073X_READ is a macro. Its definition
> would be near the definitions of the ZL3073X_REG*_DEF macros, so I
> could correctly guess these things are related.
> The 1st argument of the ZL3073X_READ macro is the register name.
> (There would be a ZL3073X_READ_IDX with one more argument for indexed
> registers.)
> In vim, having the cursor on the 1st argument (id) and pressing gD
> takes me to the corresponding ZL3073X_REG16_DEF line.
>
> Would it still be too ugly in your view?

And if having "id" as both the register name and the local variable
name is irritating, the registers can get a prefix, e.g. the register
name could be defined as REG_id.

Michal

On 11. 04. 25 9:26 dop., Lee Jones wrote:
> On Wed, 09 Apr 2025, Ivan Vecera wrote:
> 
>> Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
>> provides DPLL and PTP functionality. This series bring first part
>> that adds the common MFD driver that provides an access to the bus
>> that can be either I2C or SPI.
>>
>> The next series will bring the DPLL driver that will covers DPLL
>> functionality. And another ones will bring PTP driver and flashing
>> capability via devlink.
>>
>> Testing was done by myself and by Prathosh Satish on Microchip EDS2
>> development board with ZL30732 DPLL chip connected over I2C bus.
>>
>> Patch breakdown
>> ===============
>> Patch 1 - Common DT schema for DPLL device and pin
>> Patch 3 - Basic support for I2C, SPI and regmap
>> Patch 4 - Devlink registration
>> Patches 5-6 - Helpers for accessing device registers
>> Patches 7-8 - Component versions reporting via devlink dev info
>> Patches 9-10 - Helpers for accessing register mailboxes
>> Patch 11 - Clock ID generation for DPLL driver
>> Patch 12 - Export strnchrnul function for modules
>>             (used by next patch)
>> Patch 13 - Support for MFG config initialization file
>> Patch 14 - Fetch invariant register values used by DPLL and later by
>>             PTP driver
>>
>> Ivan Vecera (14):
>>    dt-bindings: dpll: Add device tree bindings for DPLL device and pin
>>    dt-bindings: dpll: Add support for Microchip Azurite chip family
>>    mfd: Add Microchip ZL3073x support
>>    mfd: zl3073x: Register itself as devlink device
>>    mfd: zl3073x: Add register access helpers
>>    mfd: zl3073x: Add macros for device registers access
>>    mfd: zl3073x: Add components versions register defs
>>    mfd: zl3073x: Implement devlink device info
>>    mfd: zl3073x: Add macro to wait for register value bits to be cleared
>>    mfd: zl3073x: Add functions to work with register mailboxes
>>    mfd: zl3073x: Add clock_id field
>>    lib: Allow modules to use strnchrnul
>>    mfd: zl3073x: Load mfg file into HW if it is present
>>    mfd: zl3073x: Fetch invariants during probe
>>
>>   .../devicetree/bindings/dpll/dpll-device.yaml |  76 ++
>>   .../devicetree/bindings/dpll/dpll-pin.yaml    |  44 +
>>   .../bindings/dpll/microchip,zl3073x-i2c.yaml  |  74 ++
>>   .../bindings/dpll/microchip,zl3073x-spi.yaml  |  77 ++
>>   MAINTAINERS                                   |  11 +
>>   drivers/mfd/Kconfig                           |  32 +
>>   drivers/mfd/Makefile                          |   5 +
>>   drivers/mfd/zl3073x-core.c                    | 883 ++++++++++++++++++
>>   drivers/mfd/zl3073x-i2c.c                     |  59 ++
>>   drivers/mfd/zl3073x-spi.c                     |  59 ++
>>   drivers/mfd/zl3073x.h                         |  14 +
>>   include/linux/mfd/zl3073x.h                   | 363 +++++++
>>   lib/string.c                                  |   1 +
>>   13 files changed, 1698 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml
>>   create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml
>>   create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x-i2c.yaml
>>   create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x-spi.yaml
>>   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
> 
> Not only are all of the added abstractions and ugly MACROs hard to read
> and troublesome to maintain, they're also completely unnecessary at this
> (driver) level.  Nicely authored, easy to read / maintain code wins over
> clever code 95% of the time.  Exporting functions to pass around
> pointers to private structures is also a non-starter.

If you mean regmap_config exported to zl3073x-i2c/spi modules then these 
three modules could be squashed together into single module.

> After looking at the code, there doesn't appear to be any inclusion of
> the MFD core header.  How can this be an MFD if you do not use the MFD
> API?  MFD is not a dumping area for common code and call-backs.  It's a
> subsystem used to neatly separate out and share resources between
> functionality that just happens to share the same hardware chip.

You are right, the v2 series was spliced into 2 separate series as the 
bot warns about too big (>15 commits) series. The real MFD API usage is 
present in the second one (or in patch 14 of the v1 patch-set) when MFD 
cells are created for DPLL functional blocks.

And yes, I chose the MFD for the common part because DPLL and PTP 
functions share the same registers concurrently. And MFD could be the 
right place for exposing these shared resources and providing 
synchronized access to them. The device has also GPIO functionality that 
could be potentially covered in the future.

Or would you prefer to implement all these functional block in one 
monolithic driver? Would it be better for maintenance?

Thanks,
Ivan

On Wed, 09 Apr 2025, Ivan Vecera wrote:

> Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> provides DPLL and PTP functionality. This series bring first part
> that adds the common MFD driver that provides an access to the bus
> that can be either I2C or SPI.
> 
> The next series will bring the DPLL driver that will covers DPLL
> functionality. And another ones will bring PTP driver and flashing
> capability via devlink.
> 
> Testing was done by myself and by Prathosh Satish on Microchip EDS2
> development board with ZL30732 DPLL chip connected over I2C bus.
> 
> Patch breakdown
> ===============
> Patch 1 - Common DT schema for DPLL device and pin
> Patch 3 - Basic support for I2C, SPI and regmap
> Patch 4 - Devlink registration
> Patches 5-6 - Helpers for accessing device registers
> Patches 7-8 - Component versions reporting via devlink dev info
> Patches 9-10 - Helpers for accessing register mailboxes
> Patch 11 - Clock ID generation for DPLL driver
> Patch 12 - Export strnchrnul function for modules
>            (used by next patch)
> Patch 13 - Support for MFG config initialization file
> Patch 14 - Fetch invariant register values used by DPLL and later by
>            PTP driver
> 
> Ivan Vecera (14):
>   dt-bindings: dpll: Add device tree bindings for DPLL device and pin
>   dt-bindings: dpll: Add support for Microchip Azurite chip family
>   mfd: Add Microchip ZL3073x support
>   mfd: zl3073x: Register itself as devlink device
>   mfd: zl3073x: Add register access helpers
>   mfd: zl3073x: Add macros for device registers access
>   mfd: zl3073x: Add components versions register defs
>   mfd: zl3073x: Implement devlink device info
>   mfd: zl3073x: Add macro to wait for register value bits to be cleared
>   mfd: zl3073x: Add functions to work with register mailboxes
>   mfd: zl3073x: Add clock_id field
>   lib: Allow modules to use strnchrnul
>   mfd: zl3073x: Load mfg file into HW if it is present
>   mfd: zl3073x: Fetch invariants during probe
> 
>  .../devicetree/bindings/dpll/dpll-device.yaml |  76 ++
>  .../devicetree/bindings/dpll/dpll-pin.yaml    |  44 +
>  .../bindings/dpll/microchip,zl3073x-i2c.yaml  |  74 ++
>  .../bindings/dpll/microchip,zl3073x-spi.yaml  |  77 ++
>  MAINTAINERS                                   |  11 +
>  drivers/mfd/Kconfig                           |  32 +
>  drivers/mfd/Makefile                          |   5 +
>  drivers/mfd/zl3073x-core.c                    | 883 ++++++++++++++++++
>  drivers/mfd/zl3073x-i2c.c                     |  59 ++
>  drivers/mfd/zl3073x-spi.c                     |  59 ++
>  drivers/mfd/zl3073x.h                         |  14 +
>  include/linux/mfd/zl3073x.h                   | 363 +++++++
>  lib/string.c                                  |   1 +
>  13 files changed, 1698 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dpll/dpll-device.yaml
>  create mode 100644 Documentation/devicetree/bindings/dpll/dpll-pin.yaml
>  create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x-i2c.yaml
>  create mode 100644 Documentation/devicetree/bindings/dpll/microchip,zl3073x-spi.yaml
>  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

FWIW, I'm not planning to even look at this until Andy and Krzysztof are
satisfied.  What I will say is that all of those horrible macros will
have to be removed and no abstractions will be accepted unless they are
accompanied with very good reasons as to why they are required.

-- 
Lee Jones [李琼斯]

On Wed,  9 Apr 2025 16:42:36 +0200 Ivan Vecera wrote:
> Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> provides DPLL and PTP functionality. This series bring first part
> that adds the common MFD driver that provides an access to the bus
> that can be either I2C or SPI.
> 
> The next series will bring the DPLL driver that will covers DPLL
> functionality. And another ones will bring PTP driver and flashing
> capability via devlink.
> 
> Testing was done by myself and by Prathosh Satish on Microchip EDS2
> development board with ZL30732 DPLL chip connected over I2C bus.

The DPLL here is for timing, right? Not digital logic?
After a brief glance I'm wondering why mfd, PHC + DPLL 
is a pretty common combo. Am I missing something?

On 10. 04. 25 2:17 dop., Jakub Kicinski wrote:
> On Wed,  9 Apr 2025 16:42:36 +0200 Ivan Vecera wrote:
>> Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
>> provides DPLL and PTP functionality. This series bring first part
>> that adds the common MFD driver that provides an access to the bus
>> that can be either I2C or SPI.
>>
>> The next series will bring the DPLL driver that will covers DPLL
>> functionality. And another ones will bring PTP driver and flashing
>> capability via devlink.
>>
>> Testing was done by myself and by Prathosh Satish on Microchip EDS2
>> development board with ZL30732 DPLL chip connected over I2C bus.
> 
> The DPLL here is for timing, right? Not digital logic?
> After a brief glance I'm wondering why mfd, PHC + DPLL
> is a pretty common combo. Am I missing something?

Well, you are right, this is not pretty common combo right now. But how 
many DPLL implementations we have now in kernel?

There are 3 mlx5, ice and ptp_ocp. The first two are ethernet NIC 
drivers that re-expose (translate) DPLL API provided by their firmwares 
and the 3rd timecard that acts primarily as PTP clock.

Azurite is primarly the DPLL chip with multiple DPLL channels and one of 
its use-case is time synchronization or signal synchronization. Other 
one can be PTP clock and even GPIO controller where some of input or 
output pins can be configured not to receive or send periodic signal but 
can act is GPIO inputs or outputs (depends on wiring and usage).

So I have taken an approach to have common MFD driver that provides a 
synchronized access to device registers and to have another drivers for 
particular functionality in well bounded manner (DPLL sub-device (MFD 
cell) for each DPLL channel, PTP cell for channel that is configured to 
provide PTP clock and potentially GPIO controller cell but this is 
out-of-scope now).

Btw. I would be interesting to see a NIC that just exposes an access to 
I2C bus (implements I2C read/write by NIC firmware) instead of exposing 
complete DPLL API from the firmware. Just an idea.

Thanks,
Ivan

On Thu, 10 Apr 2025 11:18:24 +0200 Ivan Vecera wrote:
> On 10. 04. 25 2:17 dop., Jakub Kicinski wrote:
> > On Wed,  9 Apr 2025 16:42:36 +0200 Ivan Vecera wrote:  
> >> Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
> >> provides DPLL and PTP functionality. This series bring first part
> >> that adds the common MFD driver that provides an access to the bus
> >> that can be either I2C or SPI.
> >>
> >> The next series will bring the DPLL driver that will covers DPLL
> >> functionality. And another ones will bring PTP driver and flashing
> >> capability via devlink.
> >>
> >> Testing was done by myself and by Prathosh Satish on Microchip EDS2
> >> development board with ZL30732 DPLL chip connected over I2C bus.  
> > 
> > The DPLL here is for timing, right? Not digital logic?
> > After a brief glance I'm wondering why mfd, PHC + DPLL
> > is a pretty common combo. Am I missing something?  
> 
> Well, you are right, this is not pretty common combo right now. But how 
> many DPLL implementations we have now in kernel?
> 
> There are 3 mlx5, ice and ptp_ocp. The first two are ethernet NIC 
> drivers that re-expose (translate) DPLL API provided by their firmwares 
> and the 3rd timecard that acts primarily as PTP clock.
> 
> Azurite is primarly the DPLL chip with multiple DPLL channels and one of 
> its use-case is time synchronization or signal synchronization. Other 
> one can be PTP clock and even GPIO controller where some of input or 
> output pins can be configured not to receive or send periodic signal but 
> can act is GPIO inputs or outputs (depends on wiring and usage).
> 
> So I have taken an approach to have common MFD driver that provides a 
> synchronized access to device registers and to have another drivers for 
> particular functionality in well bounded manner (DPLL sub-device (MFD 
> cell) for each DPLL channel, PTP cell for channel that is configured to 
> provide PTP clock and potentially GPIO controller cell but this is 
> out-of-scope now).

Okay, my understanding was that if you need to reuse the component
drivers across multiple different SoCs or devices, and there is no
"natural" bus then MFD is the go to. OTOH using MFD as a software
abstraction/to organize your code is a pointless complication.
(We're going to merge the MFD parts via Lee's tree and the all actual
drivers via netdev?) Admittedly that's just my feeling and not based 
on any real info or experience. I defer to Lee and others to pass
judgment.

On 11. 04. 25 12:57 dop., Jakub Kicinski wrote:
> On Thu, 10 Apr 2025 11:18:24 +0200 Ivan Vecera wrote:
>> On 10. 04. 25 2:17 dop., Jakub Kicinski wrote:
>>> On Wed,  9 Apr 2025 16:42:36 +0200 Ivan Vecera wrote:
>>>> Add support for Microchip Azurite DPLL/PTP/SyncE chip family that
>>>> provides DPLL and PTP functionality. This series bring first part
>>>> that adds the common MFD driver that provides an access to the bus
>>>> that can be either I2C or SPI.
>>>>
>>>> The next series will bring the DPLL driver that will covers DPLL
>>>> functionality. And another ones will bring PTP driver and flashing
>>>> capability via devlink.
>>>>
>>>> Testing was done by myself and by Prathosh Satish on Microchip EDS2
>>>> development board with ZL30732 DPLL chip connected over I2C bus.
>>>
>>> The DPLL here is for timing, right? Not digital logic?
>>> After a brief glance I'm wondering why mfd, PHC + DPLL
>>> is a pretty common combo. Am I missing something?
>>
>> Well, you are right, this is not pretty common combo right now. But how
>> many DPLL implementations we have now in kernel?
>>
>> There are 3 mlx5, ice and ptp_ocp. The first two are ethernet NIC
>> drivers that re-expose (translate) DPLL API provided by their firmwares
>> and the 3rd timecard that acts primarily as PTP clock.
>>
>> Azurite is primarly the DPLL chip with multiple DPLL channels and one of
>> its use-case is time synchronization or signal synchronization. Other
>> one can be PTP clock and even GPIO controller where some of input or
>> output pins can be configured not to receive or send periodic signal but
>> can act is GPIO inputs or outputs (depends on wiring and usage).
>>
>> So I have taken an approach to have common MFD driver that provides a
>> synchronized access to device registers and to have another drivers for
>> particular functionality in well bounded manner (DPLL sub-device (MFD
>> cell) for each DPLL channel, PTP cell for channel that is configured to
>> provide PTP clock and potentially GPIO controller cell but this is
>> out-of-scope now).
> 
> Okay, my understanding was that if you need to reuse the component
> drivers across multiple different SoCs or devices, and there is no
> "natural" bus then MFD is the go to. OTOH using MFD as a software
> abstraction/to organize your code is a pointless complication.
> (We're going to merge the MFD parts via Lee's tree and the all actual
> drivers via netdev?) Admittedly that's just my feeling and not based
> on any real info or experience. I defer to Lee and others to pass
> judgment.

I followed an example of rsmu mfd driver that provides an access to the 
bus (i2c/spi) via regmap and ptp_clockmatrix platform driver for the PTP 
functionality of the RSMU chip. The ptp_clockmatrix device is also 
instantiated only from rsmu mfd and it is not shared by multiple mfd 
drivers.

I.

> Btw. I would be interesting to see a NIC that just exposes an access to I2C
> bus (implements I2C read/write by NIC firmware) instead of exposing complete
> DPLL API from the firmware. Just an idea.

The mellanox driver does expose an standard Linux I2C bus, with the
firmware wiggling the bits on the wire. But i've no idea what devices
are hanging off the bus.

	Andrew