| 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: <be0eba3d-4142-4ca8-a74a-0ebb22186d69@adtran.com>
Date: Wed, 21 May 2025 08:04:38 +0000
From: Piotr Kubik <piotr.kubik@...ran.com>
To: ALOK TIWARI <alok.a.tiwari@...cle.com>
CC: Oleksij Rempel <o.rempel@...gutronix.de>, Kory Maincent
<kory.maincent@...tlin.com>, Jakub Kicinski <kuba@...nel.org>, Eric Dumazet
<edumazet@...gle.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
<davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "devicetree@...r.kernel.org"
<devicetree@...r.kernel.org>
Subject: Re: [PATCH net-next v3 2/2] net: pse-pd: Add Si3474 PSE controller
driver
On 5/16/25 23:38, ALOK TIWARI wrote:
>
> On 16-05-2025 18:37, 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 | 649 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 660 insertions(+)
>> create mode 100644 drivers/net/pse-pd/si3474.c
>>
>> diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
>> index 7fab916a7f46..d1b100eb8c52 100644
>> --- a/drivers/net/pse-pd/Kconfig
>> +++ b/drivers/net/pse-pd/Kconfig
>> @@ -32,6 +32,16 @@ config PSE_PD692X0
>> To compile this driver as a module, choose M here: the
>> module will be called pd692x0.
>>
>> +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.
>> +
>> config PSE_TPS23881
>> tristate "TPS23881 PSE controller"
>> depends on I2C
>> diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
>> index 9d2898b36737..cc78f7ea7f5f 100644
>> --- a/drivers/net/pse-pd/Makefile
>> +++ b/drivers/net/pse-pd/Makefile
>> @@ -5,4 +5,5 @@ obj-$(CONFIG_PSE_CONTROLLER) += pse_core.o
>>
>> obj-$(CONFIG_PSE_REGULATOR) += pse_regulator.o
>> obj-$(CONFIG_PSE_PD692X0) += pd692x0.o
>> +obj-$(CONFIG_PSE_SI3474) += si3474.o
>> obj-$(CONFIG_PSE_TPS23881) += tps23881.o
>> diff --git a/drivers/net/pse-pd/si3474.c b/drivers/net/pse-pd/si3474.c
>> new file mode 100644
>> index 000000000000..7c21b475ca1a
>> --- /dev/null
>> +++ b/drivers/net/pse-pd/si3474.c
>> @@ -0,0 +1,649 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Driver for the Skyworks Si3474 PoE PSE Controller
>> + *
>> + * Chip Architecture & Terminology:
>> + *
>> + * The Si3474 is a single-chip PoE PSE controller managing 8 physical power
>> + * delivery channels. Internally, it's structured into two logical "Quads".
>> + *
>> + * Quad 0: Manages physical channels ('ports' in datasheet) 0, 1, 2, 3
>> + * Quad 1: Manages physical channels ('ports' in datasheet) 4, 5, 6, 7
>> + *
>> + * Each Quad is accessed via a separate I2C address. The base address range is
>> + * set by hardware pins A1-A4, and the specific address selects Quad 0 (usually
>> + * the lower/even address) or Quad 1 (usually the higher/odd address).
>> + * See datasheet Table 2.2 for the address mapping.
>> + *
>> + * While the Quads manage channel-specific operations, the Si3474 package has
>> + * several resources shared across the entire chip:
>> + * - Single RESETb input pin.
>> + * - Single INTb output pin (signals interrupts from *either* Quad).
>> + * - Single OSS input pin (Emergency Shutdown).
>> + * - Global I2C Address (0x7F) used for firmware updates.
>> + * - Global status monitoring (Temperature, VDD/VPWR Undervoltage Lockout).
>> + *
>> + * Driver Architecture:
>> + *
>> + * To handle the mix of per-Quad access and shared resources correctly, this
>> + * driver treats the entire Si3474 package as one logical device. The driver
>> + * instance associated with the primary I2C address (Quad 0) takes ownership.
>> + * It discovers and manages the I2C client for the secondary address (Quad 1).
>> + * This primary instance handles shared resources like IRQ management and
>> + * registers a single PSE controller device representing all logical PIs.
>> + * Internal functions route I2C commands to the appropriate Quad's i2c_client
>> + * based on the target channel or PI.
>> + *
>> + * Terminology Mapping:
>> + *
>> + * - "PI" (Power Interface): Refers to the logical PSE port as defined by
>> + * IEEE 802.3 (typically corresponds to an RJ45 connector). This is the
>> + * `id` (0-7) used in the pse_controller_ops.
>> + * - "Channel": Refers to one of the 8 physical power control paths within
>> + * the Si3474 chip itself (hardware channels 0-7). This terminology is
>> + * used internally within the driver to avoid confusion with 'ports'.
>> + * - "Quad": One of the two internal 4-channel management units within the
>> + * Si3474, each accessed via its own I2C address.
>> + *
>> + * Relationship:
>> + * - A 2-Pair PoE PI uses 1 Channel.
>> + * - A 4-Pair PoE PI uses 2 Channels.
>> + *
>> + * ASCII Schematic:
>> + *
>> + * +-----------------------------------------------------+
>> + * | Si3474 Chip |
>> + * | |
>> + * | +---------------------+ +---------------------+ |
>> + * | | Quad 0 | | Quad 1 | |
>> + * | | Channels 0, 1, 2, 3 | | Channels 4, 5, 6, 7 | |
>> + * | +----------^----------+ +-------^-------------+ |
>> + * | I2C Addr 0 | | I2C Addr 1 |
>> + * | +------------------------+ |
>> + * | (Primary Driver Instance) (Managed by Primary) |
>> + * | |
>> + * | Shared Resources (affect whole chip): |
>> + * | - Single INTb Output -> Handled by Primary |
>> + * | - Single RESETb Input |
>> + * | - Single OSS Input -> Handled by Primary |
>> + * | - Global I2C Addr (0x7F) for Firmware Update |
>> + * | - Global Status (Temp, VDD/VPWR UVLO) |
>> + * +-----------------------------------------------------+
>> + * | | | | | | | |
>> + * Ch0 Ch1 Ch2 Ch3 Ch4 Ch5 Ch6 Ch7 (Physical Channels)
>> + *
>> + * Example Mapping (Logical PI to Physical Channel(s)):
>> + * * 2-Pair Mode (8 PIs):
>> + * PI 0 -> Ch 0
>> + * PI 1 -> Ch 1
>> + * ...
>> + * PI 7 -> Ch 7
>> + * * 4-Pair Mode (4 PIs):
>> + * PI 0 -> Ch 0 + Ch 1 (Managed via Quad 0 Addr)
>> + * PI 1 -> Ch 2 + Ch 3 (Managed via Quad 0 Addr)
>> + * PI 2 -> Ch 4 + Ch 5 (Managed via Quad 1 Addr)
>> + * PI 3 -> Ch 6 + Ch 7 (Managed via Quad 1 Addr)
>> + * (Note: Actual mapping depends on Device Tree and PORT_REMAP config)
>> + */
>> +
>> +#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
>
> SI3474_MAX_CHANS, this could be sound like changes or chance
>
Well, for me it sounds good.
Moreover, this shortcut is widely used in kernel code for many different types of channels.
>> +
>> +#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 PORT_MODE_REG 0x12
>> +#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_pi_desc {
>> + u8 chan[2];
>
> here chan does not sound smooth -> channels[] ?
> especially for an array or list
>
Same as above.
>> + bool is_4p;
>> +};
>> +
>> +struct si3474_priv {
>> + struct i2c_client *client[2];
>> + struct pse_controller_dev pcdev;
>> + struct device_node *np;
>> + struct si3474_pi_desc pi[SI3474_MAX_CHANS];
>> +};
>
> Thanks,
> Alok
Thanks
/Piotr
Powered by blists - more mailing lists