| 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: <6ee047d4-f3de-4c25-aaae-721221dc3003@kernel.org>
Date: Wed, 16 Apr 2025 12:54:51 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Piotr Kubik <piotr.kubik@...ran.com>,
Oleksij Rempel <o.rempel@...gutronix.de>,
Kory Maincent <kory.maincent@...tlin.com>,
Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] net: pse-pd: Add Si3474 PSE controller driver
On 16/04/2025 12:47, Piotr Kubik wrote:
> From: Piotr Kubik <piotr.kubik@...ran.com>
>
> Add a driver for the Skyworks Si3474 I2C Power Sourcing Equipment
> controller.
>
> Based on the TPS23881 driver code.
>
> Driver supports basic features of Si3474 IC:
> - get port status,
> - get port power,
> - get port voltage,
> - enable/disable port power.
>
> Only 4p configurations are supported at this moment.
>
> Signed-off-by: Piotr Kubik <piotr.kubik@...ran.com>
> ---
> drivers/net/pse-pd/Kconfig | 10 +
> drivers/net/pse-pd/Makefile | 1 +
> drivers/net/pse-pd/si3474.c | 477 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 488 insertions(+)
> create mode 100644 drivers/net/pse-pd/si3474.c
Please put bindings before their user (see DT submitting patches)
>
> diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
> index 7fab916a7f46..6d2fef6c2602 100644
> --- a/drivers/net/pse-pd/Kconfig
> +++ b/drivers/net/pse-pd/Kconfig
> @@ -41,4 +41,14 @@ config PSE_TPS23881
>
> To compile this driver as a module, choose M here: the
> module will be called tps23881.
> +
> +config PSE_SI3474
> + tristate "Si3474 PSE controller"
> + depends on I2C
> + help
> + This module provides support for Si3474 regulator based Ethernet
> + Power Sourcing Equipment.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called si3474.
> endif
> diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
> index 9d2898b36737..b33b4d905cd5 100644
> --- a/drivers/net/pse-pd/Makefile
> +++ b/drivers/net/pse-pd/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_PSE_CONTROLLER) += pse_core.o
> obj-$(CONFIG_PSE_REGULATOR) += pse_regulator.o
> obj-$(CONFIG_PSE_PD692X0) += pd692x0.o
> obj-$(CONFIG_PSE_TPS23881) += tps23881.o
> +obj-$(CONFIG_PSE_SI3474) += si3474.o
> \ No newline at end of file
1. Warnin ghere
2. Don't add your entries to the end but in more-or-less alphabetically
sorted place.
> diff --git a/drivers/net/pse-pd/si3474.c b/drivers/net/pse-pd/si3474.c
> new file mode 100644
> index 000000000000..a2b4b8bff393
> --- /dev/null
> +++ b/drivers/net/pse-pd/si3474.c
> @@ -0,0 +1,477 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Driver for the Skyworks Si3474 PoE PSE Controller
> + *
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pse-pd/pse.h>
> +
> +#define SI3474_MAX_CHANS 8
> +
> +#define MANUFACTURER_ID 0x08
> +#define IC_ID 0x05
> +#define SI3474_DEVICE_ID (MANUFACTURER_ID << 3 | IC_ID)
> +
> +/* Misc registers */
> +#define VENDOR_IC_ID_REG 0x1B
> +#define TEMPERATURE_REG 0x2C
> +#define FIRMWARE_REVISION_REG 0x41
> +#define CHIP_REVISION_REG 0x43
> +
> +/* Main status registers */
> +#define POWER_STATUS_REG 0x10
> +#define PB_POWER_ENABLE_REG 0x19
> +
> +/* PORTn Current */
> +#define PORT1_CURRENT_LSB_REG 0x30
> +
> +/* PORTn Current [mA], return in [nA] */
> +/* 1000 * ((PORTn_CURRENT_MSB << 8) + PORTn_CURRENT_LSB) / 16384 */
> +#define SI3474_NA_STEP (1000 * 1000 * 1000 / 16384)
> +
> +/* VPWR Voltage */
> +#define VPWR_LSB_REG 0x2E
> +#define VPWR_MSB_REG 0x2F
> +
> +/* PORTn Voltage */
> +#define PORT1_VOLTAGE_LSB_REG 0x32
> +
> +/* VPWR Voltage [V], return in [uV] */
> +/* 60 * (( VPWR_MSB << 8) + VPWR_LSB) / 16384 */
> +#define SI3474_UV_STEP (1000 * 1000 * 60 / 16384)
> +
> +struct si3474_port_desc {
> + u8 chan[2];
> + bool is_4p;
> +};
> +
> +struct si3474_priv {
> + struct i2c_client *client;
> + struct pse_controller_dev pcdev;
> + struct device_node *np;
> + struct si3474_port_desc port[SI3474_MAX_CHANS];
> +};
> +
> +static struct si3474_priv *to_si3474_priv(struct pse_controller_dev *pcdev)
> +{
> + return container_of(pcdev, struct si3474_priv, pcdev);
> +}
> +
> +static int si3474_pi_get_admin_state(struct pse_controller_dev *pcdev,
> int id,
Your patchset is corrupted
> + struct pse_admin_state *admin_state)
> +{
> + struct si3474_priv *priv = to_si3474_priv(pcdev);
> + struct i2c_client *client = priv->client;
> + bool enabled = FALSE;
I believe it is "false", not FALSE.
...
> +
> + if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
> + dev_err(dev, "i2c check functionality failed\n");
> + return -ENXIO;
> + }
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + ret = i2c_smbus_read_byte_data(client, VENDOR_IC_ID_REG);
> + if (ret < 0)
> + return ret;
> +
> + if (ret != SI3474_DEVICE_ID) {
> + dev_err(dev, "Wrong device ID: 0x%x\n", ret);
> + return -ENXIO;
> + }
> +
> + ret = i2c_smbus_read_byte_data(client, FIRMWARE_REVISION_REG);
> + if (ret < 0)
> + return ret;
> + fw_version = ret;
> +
> + ret = i2c_smbus_read_byte_data(client, CHIP_REVISION_REG);
> + if (ret < 0)
> + return ret;
> +
> + dev_info(&client->dev, "Chip revision: 0x%x, firmware version: 0x%x\n",
> + ret, fw_version);
dev_dbg, don't pollute dmesg on success.
> +
> + priv->client = client;
> + i2c_set_clientdata(client, priv);
> + priv->np = dev->of_node;
> +
> + priv->pcdev.owner = THIS_MODULE;
> + priv->pcdev.ops = &si3474_ops;
> + priv->pcdev.dev = dev;
> + priv->pcdev.types = ETHTOOL_PSE_C33;
> + priv->pcdev.nr_lines = SI3474_MAX_CHANS;
> + ret = devm_pse_controller_register(dev, &priv->pcdev);
> + if (ret) {
> + return dev_err_probe(dev, ret,
> + "Failed to register PSE controller\n");
> + }
> +
> + return ret;
> +}
> +
> +static const struct i2c_device_id si3474_id[] = {{"si3474"}, {}};
Use existing kernel style for such arrays.
Best regards,
Krzysztof
Powered by blists - more mailing lists