| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1f1e5566-a9a0-4d72-80be-81eddfe95fa3@redhat.com>
Date: Sun, 29 Jun 2025 20:59:54 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
Cc: netdev@...r.kernel.org, Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
Jiri Pirko <jiri@...nulli.us>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Prathosh Satish <Prathosh.Satish@...rochip.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>,
Jason Gunthorpe <jgg@...pe.ca>, Shannon Nelson <shannon.nelson@....com>,
Dave Jiang <dave.jiang@...el.com>, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
Michal Schmidt <mschmidt@...hat.com>, Petr Oros <poros@...hat.com>
Subject: Re: [PATCH net-next v11 03/14] dpll: Add basic Microchip ZL3073x
support
On 18. 06. 25 10:56 dop., Jonathan Cameron wrote:
> On Mon, 16 Jun 2025 22:13:53 +0200
> Ivan Vecera <ivecera@...hat.com> wrote:
>
>> Microchip Azurite ZL3073x represents chip family providing DPLL
>> and optionally PHC (PTP) functionality. The chips can be connected
>> be connected over I2C or SPI bus.
>>
>> They have the following characteristics:
>> * 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 operate in either differential or
>> single-ended mode (differential mode uses 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 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 128 bytes, with each
>> page containing a page selector register at address 0x7F.
>> For reading/writing multi-byte registers, the device supports bulk
>> transfers.
>>
>> Add basic functionality to access device registers and probe
>> functionality for both I2C and SPI cases.
>>
>> Signed-off-by: Ivan Vecera <ivecera@...hat.com>
> A few trivial drive by comments.
>
>> diff --git a/drivers/dpll/zl3073x/i2c.c b/drivers/dpll/zl3073x/i2c.c
>> new file mode 100644
>> index 0000000000000..bca1cd729895c
>> --- /dev/null
>> +++ b/drivers/dpll/zl3073x/i2c.c
>> @@ -0,0 +1,93 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#include <linux/dev_printk.h>
>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "core.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 (IS_ERR(zldev))
>> + return PTR_ERR(zldev);
>> +
>> + zldev->regmap = devm_regmap_init_i2c(client, &zl3073x_regmap_config);
>> + if (IS_ERR(zldev->regmap)) {
>> + dev_err_probe(dev, PTR_ERR(zldev->regmap),
>> + "Failed to initialize regmap\n");
>> + return PTR_ERR(zldev->regmap);
> As below.
>
>> + }
>
>> diff --git a/drivers/dpll/zl3073x/spi.c b/drivers/dpll/zl3073x/spi.c
>> new file mode 100644
>> index 0000000000000..219676da71b78
>> --- /dev/null
>> +++ b/drivers/dpll/zl3073x/spi.c
>> @@ -0,0 +1,93 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#include <linux/dev_printk.h>
>> +#include <linux/err.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#include "core.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 (IS_ERR(zldev))
>> + return PTR_ERR(zldev);
>> +
>> + zldev->regmap = devm_regmap_init_spi(spi, &zl3073x_regmap_config);
>> + if (IS_ERR(zldev->regmap)) {
>> + dev_err_probe(dev, PTR_ERR(zldev->regmap),
>> + "Failed to initialize regmap\n");
>> + return PTR_ERR(zldev->regmap);
>
> return dev_err_probe();
> One of it's biggest advantages is that dev_err_probe() returns the
> ret value passed in avoiding duplication like this and saving
> a few lines of code each time.
Will fix.
>> + }
>> +
>> + return zl3073x_dev_probe(zldev, spi_get_device_match_data(spi));
>> +}
>> +
>> +static const struct spi_device_id zl3073x_spi_id[] = {
>> + {
>> + .name = "zl30731",
>> + .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30731],
>
> Not my subsystem so up to you, but in general over time we've found that
> an enum + array tends to bring few benefits over appropriately named
> zl30731_chip_info separate structures.
Will update according this.
>> + },
>> + {
>> + .name = "zl30732",
>> + .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30732],
>> + },
>> + {
>> + .name = "zl30733",
>> + .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30733],
>> + },
>> + {
>> + .name = "zl30734",
>> + .driver_data = (kernel_ulong_t)&zl3073x_chip_info[ZL30734],
>> + },
>> + {
>> + .name = "zl30735",
>> + .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@...hat.com>");
>> +MODULE_DESCRIPTION("Microchip ZL3073x SPI driver");
>> +MODULE_IMPORT_NS("ZL3073X");
>> +MODULE_LICENSE("GPL");
>
Thanks for advice.
Ivan
Powered by blists - more mailing lists