Message ID | 20250409144250.206590-4-ivecera@redhat.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Add Microchip ZL3073x support (part 1) | expand |
On Wed, Apr 09, 2025 at 04:42: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 (later in this series) and by the PTP driver (will be > added later). > > The chip family is characterized by following properties: > * 2 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 ... > +/* > + * Regmap ranges > + */ > +#define ZL3073x_PAGE_SIZE 128 > +#define ZL3073x_NUM_PAGES 16 > +#define ZL3073x_PAGE_SEL 0x7F > + > +/* > + * Regmap range configuration > + * > + * The device uses 7-bit addressing and has 16 register pages with > + * range 0x00-0x7f. The register 0x7f in each page acts as page > + * selector where bits 0-3 contains currently selected page. > + */ > +static const struct regmap_range_cfg zl3073x_regmap_ranges[] = { > + { > + .range_min = 0, This still has the same issue, you haven't given a chance to me to reply in v1 thread. I'm not going to review this as it's not settled down yet. Let's first discuss the questions you have in v1. > + .range_max = ZL3073x_NUM_PAGES * ZL3073x_PAGE_SIZE, > + .selector_reg = ZL3073x_PAGE_SEL, > + .selector_mask = GENMASK(3, 0), > + .selector_shift = 0, > + .window_start = 0, > + .window_len = ZL3073x_PAGE_SIZE, > + }, > +};
On 09/04/2025 17:43, Andy Shevchenko wrote: >> +/* >> + * Regmap range configuration >> + * >> + * The device uses 7-bit addressing and has 16 register pages with >> + * range 0x00-0x7f. The register 0x7f in each page acts as page >> + * selector where bits 0-3 contains currently selected page. >> + */ >> +static const struct regmap_range_cfg zl3073x_regmap_ranges[] = { >> + { >> + .range_min = 0, > > This still has the same issue, you haven't given a chance to me to reply > in v1 thread. I'm not going to review this as it's not settled down yet. > Let's first discuss the questions you have in v1. > I already started reviewing v2, so now we have simultaneous discussions in v1 and v2... Best regards, Krzysztof
On 10. 04. 25 9:19 dop., Krzysztof Kozlowski wrote: > On 09/04/2025 17:43, Andy Shevchenko wrote: >>> +/* >>> + * Regmap range configuration >>> + * >>> + * The device uses 7-bit addressing and has 16 register pages with >>> + * range 0x00-0x7f. The register 0x7f in each page acts as page >>> + * selector where bits 0-3 contains currently selected page. >>> + */ >>> +static const struct regmap_range_cfg zl3073x_regmap_ranges[] = { >>> + { >>> + .range_min = 0, >> >> This still has the same issue, you haven't given a chance to me to reply >> in v1 thread. I'm not going to review this as it's not settled down yet. >> Let's first discuss the questions you have in v1. >> Sorry for that but I don't understand where the issue is... Many drivers uses this the same way. E.g. drivers/leds/leds-aw200xx.c drivers/mfd/rsmu_i2c.c sound/soc/codecs/tas2562.c ...and many others All of them uses selector register that is present on all pages, wide range for register access <0, num_pages*window_size> and window <0, window_size> Do they also do incorrectly or am I missing something? I. > I already started reviewing v2, so now we have simultaneous discussions > in v1 and v2... > > Best regards, > Krzysztof >
On Thu, Apr 10, 2025 at 09:52:39AM +0200, Ivan Vecera wrote: > > > On 10. 04. 25 9:19 dop., Krzysztof Kozlowski wrote: > > On 09/04/2025 17:43, Andy Shevchenko wrote: > > > > +/* > > > > + * Regmap range configuration > > > > + * > > > > + * The device uses 7-bit addressing and has 16 register pages with > > > > + * range 0x00-0x7f. The register 0x7f in each page acts as page > > > > + * selector where bits 0-3 contains currently selected page. > > > > + */ > > > > +static const struct regmap_range_cfg zl3073x_regmap_ranges[] = { > > > > + { > > > > + .range_min = 0, > > > > > > This still has the same issue, you haven't given a chance to me to reply > > > in v1 thread. I'm not going to review this as it's not settled down yet. > > > Let's first discuss the questions you have in v1. > > > > > Sorry for that but I don't understand where the issue is... Many drivers > uses this the same way. > E.g. > drivers/leds/leds-aw200xx.c > drivers/mfd/rsmu_i2c.c > sound/soc/codecs/tas2562.c > ...and many others > > All of them uses selector register that is present on all pages, wide range > for register access <0, num_pages*window_size> and window <0, window_size> > > Do they also do incorrectly or am I missing something? The bigger point is, you should of asked this as part of the discussion on the previous version. You should not post a new version until all discussion has come to an end, you understand all the comments, or you have persuaded the commentor that the code is in fact correct. Posting more versions without having that discussion just wastes reviewers/Maintainers time, and that is not what you want to do if you want to get your patch merged. Andrew
On 10. 04. 25 7:50 odp., Andrew Lunn wrote: > On Thu, Apr 10, 2025 at 09:52:39AM +0200, Ivan Vecera wrote: >> >> >> On 10. 04. 25 9:19 dop., Krzysztof Kozlowski wrote: >>> On 09/04/2025 17:43, Andy Shevchenko wrote: >>>>> +/* >>>>> + * Regmap range configuration >>>>> + * >>>>> + * The device uses 7-bit addressing and has 16 register pages with >>>>> + * range 0x00-0x7f. The register 0x7f in each page acts as page >>>>> + * selector where bits 0-3 contains currently selected page. >>>>> + */ >>>>> +static const struct regmap_range_cfg zl3073x_regmap_ranges[] = { >>>>> + { >>>>> + .range_min = 0, >>>> >>>> This still has the same issue, you haven't given a chance to me to reply >>>> in v1 thread. I'm not going to review this as it's not settled down yet. >>>> Let's first discuss the questions you have in v1. >>>> >> >> Sorry for that but I don't understand where the issue is... Many drivers >> uses this the same way. >> E.g. >> drivers/leds/leds-aw200xx.c >> drivers/mfd/rsmu_i2c.c >> sound/soc/codecs/tas2562.c >> ...and many others >> >> All of them uses selector register that is present on all pages, wide range >> for register access <0, num_pages*window_size> and window <0, window_size> >> >> Do they also do incorrectly or am I missing something? > > The bigger point is, you should of asked this as part of the > discussion on the previous version. You should not post a new version > until all discussion has come to an end, you understand all the > comments, or you have persuaded the commentor that the code is in fact > correct. > > Posting more versions without having that discussion just wastes > reviewers/Maintainers time, and that is not what you want to do if you > want to get your patch merged. > > Andrew Yeah, excuse me. I'm very sorry for this impatience. I.
diff --git a/MAINTAINERS b/MAINTAINERS index 0742a10e87c88..5c086e945b148 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15996,6 +15996,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..30b36e3ee8f7f 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 + Support for Microchip Azurite DPLL/PTP/SyncE chip family. This option + supports I2C as the control interface. + + This driver provides common support for accessing the device. + Additional drivers must be enabled in order to use the functionality + of the device. + +config MFD_ZL3073X_SPI + tristate "Microchip Azurite DPLL/PTP/SyncE with SPI" + depends on SPI + select MFD_ZL3073X_CORE + select REGMAP_SPI + help + Support for Microchip Azurite DPLL/PTP/SyncE chip family. This option + supports SPI as the control interface. + + This driver provides common support for accessing the device. + Additional drivers must be enabled in order to use the functionality + of the device. + 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..ccb6987d04a20 --- /dev/null +++ b/drivers/mfd/zl3073x-core.c @@ -0,0 +1,101 @@ +// 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/module.h> +#include <linux/regmap.h> +#include "zl3073x.h" + +/* + * Regmap ranges + */ +#define ZL3073x_PAGE_SIZE 128 +#define ZL3073x_NUM_PAGES 16 +#define ZL3073x_PAGE_SEL 0x7F + +/* + * Regmap range configuration + * + * The device uses 7-bit addressing and has 16 register pages with + * range 0x00-0x7f. The register 0x7f in each page acts as page + * selector where bits 0-3 contains currently selected page. + */ +static const struct regmap_range_cfg zl3073x_regmap_ranges[] = { + { + .range_min = 0, + .range_max = ZL3073x_NUM_PAGES * ZL3073x_PAGE_SIZE, + .selector_reg = ZL3073x_PAGE_SEL, + .selector_mask = GENMASK(3, 0), + .selector_shift = 0, + .window_start = 0, + .window_len = ZL3073x_PAGE_SIZE, + }, +}; + +/* + * Regmap config + */ +const struct regmap_config zl3073x_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = ZL3073x_NUM_PAGES * ZL3073x_PAGE_SIZE, + .ranges = zl3073x_regmap_ranges, + .num_ranges = ARRAY_SIZE(zl3073x_regmap_ranges), +}; + +/** + * zl3073x_get_regmap_config - return pointer to regmap config + * + * Return: pointer to regmap config + */ +const struct regmap_config *zl3073x_get_regmap_config(void) +{ + return &zl3073x_regmap_config; +} +EXPORT_SYMBOL_NS_GPL(zl3073x_get_regmap_config, "ZL3073X"); + +/** + * zl3073x_devm_alloc - allocates zl3073x device structure + * @dev: pointer to device structure + * + * Allocates zl3073x device structure as device resource. + * + * Return: pointer to zl3073x device structure + */ +struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev) +{ + struct zl3073x_dev *zldev; + + return devm_kzalloc(dev, sizeof(*zldev), GFP_KERNEL); +} +EXPORT_SYMBOL_NS_GPL(zl3073x_devm_alloc, "ZL3073X"); + +/** + * zl3073x_dev_init - initialize zl3073x device + * @zldev: pointer to zl3073x device + * + * Common initialization of zl3073x device structure. + * + * Returns: 0 on success, <0 on error + */ +int zl3073x_dev_init(struct zl3073x_dev *zldev) +{ + int rc; + + rc = devm_mutex_init(zldev->dev, &zldev->lock); + if (rc) { + dev_err_probe(zldev->dev, rc, "Failed to initialize mutex\n"); + return rc; + } + + return 0; +} +EXPORT_SYMBOL_NS_GPL(zl3073x_dev_init, "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..461b583e536b7 --- /dev/null +++ b/drivers/mfd/zl3073x-i2c.c @@ -0,0 +1,58 @@ +// 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 device *dev = &client->dev; + struct zl3073x_dev *zldev; + + zldev = zl3073x_devm_alloc(dev); + if (!zldev) + return -ENOMEM; + + zldev->dev = dev; + zldev->regmap = devm_regmap_init_i2c(client, zl3073x_get_regmap_config()); + if (IS_ERR(zldev->regmap)) { + dev_err_probe(dev, PTR_ERR(zldev->regmap), + "Failed to initialize register map\n"); + return PTR_ERR(zldev->regmap); + } + + i2c_set_clientdata(client, zldev); + + return zl3073x_dev_init(zldev); +} + +static const struct i2c_device_id zl3073x_i2c_id[] = { + { "zl3073x-i2c" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(i2c, zl3073x_i2c_id); + +static const struct of_device_id zl3073x_i2c_of_match[] = { + { .compatible = "microchip,zl3073x-i2c" }, + { /* 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..db976aef74917 --- /dev/null +++ b/drivers/mfd/zl3073x-spi.c @@ -0,0 +1,58 @@ +// 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 device *dev = &spi->dev; + struct zl3073x_dev *zldev; + + zldev = zl3073x_devm_alloc(dev); + if (!zldev) + return -ENOMEM; + + zldev->dev = dev; + zldev->regmap = devm_regmap_init_spi(spi, zl3073x_get_regmap_config()); + if (IS_ERR(zldev->regmap)) { + dev_err_probe(dev, PTR_ERR(zldev->regmap), + "Failed to initialize register map\n"); + return PTR_ERR(zldev->regmap); + } + + spi_set_drvdata(spi, zldev); + + return zl3073x_dev_init(zldev); +} + +static const struct spi_device_id zl3073x_spi_id[] = { + { "zl3073x-spi" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(spi, zl3073x_spi_id); + +static const struct of_device_id zl3073x_spi_of_match[] = { + { .compatible = "microchip,zl3073x-spi" }, + { /* 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..8e8ffa961e4ca --- /dev/null +++ b/drivers/mfd/zl3073x.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __ZL3073X_CORE_H +#define __ZL3073X_CORE_H + +struct device; +struct regmap_config; +struct zl3073x_dev; + +struct zl3073x_dev *zl3073x_devm_alloc(struct device *dev); +int zl3073x_dev_init(struct zl3073x_dev *zldev); +const struct regmap_config *zl3073x_get_regmap_config(void); + +#endif /* __ZL3073X_CORE_H */ diff --git a/include/linux/mfd/zl3073x.h b/include/linux/mfd/zl3073x.h new file mode 100644 index 0000000000000..f3f33ef8bfa18 --- /dev/null +++ b/include/linux/mfd/zl3073x.h @@ -0,0 +1,23 @@ +/* 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 HW registers + * @lock: lock to be held during access to HW registers + */ +struct zl3073x_dev { + struct device *dev; + struct regmap *regmap; + struct mutex lock; +}; + +#endif /* __LINUX_MFD_ZL3073X_H */
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 (later in this series) and by the PTP driver (will be added later). The chip family is characterized by following properties: * 2 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 Signed-off-by: Ivan Vecera <ivecera@redhat.com> 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 | 101 ++++++++++++++++++++++++++++++++++++ drivers/mfd/zl3073x-i2c.c | 58 +++++++++++++++++++++ drivers/mfd/zl3073x-spi.c | 58 +++++++++++++++++++++ drivers/mfd/zl3073x.h | 14 +++++ include/linux/mfd/zl3073x.h | 23 ++++++++ 8 files changed, 298 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