2024-02-15 16:12:04

by Kory Maincent

[permalink] [raw]
Subject: [PATCH net-next v4 00/17] net: Add support for Power over Ethernet (PoE)

This patch series aims at adding support for PoE (Power over Ethernet),
based on the already existing support for PoDL (Power over Data Line)
implementation. In addition, it adds support for two specific PoE
controller, the Microchip PD692x0 and the TI TPS23881.

This patch series is sponsored by Dent Project
<[email protected]>.

In detail:
- Patch 1 to 13 prepare net to support PoE devices.
- Patch 14 and 15 add PD692x0 PoE PSE controller driver and its binding.
- Patch 16 and 17 add TI TPS23881 PSE controller driver and its binding.

Changes in v2:
- Extract "firmware_loader: Expand Firmware upload error codes patches" to
send it alone and get it merge in an immutable branch.
- Add "c33" prefix for PoE variables and enums.
- Enhance few comments.
- Add PSE Documentation.
- Make several changes in pd692x0 driver, mainly for readibility.
- Link to v1: https://lore.kernel.org/r/[email protected]

Changes in v3:
- Add patches to add Oleksij and myself to PSE MAINTAINERS.
- Add patches to add pse devlink.
- Add TI TPS23881 PSE controller driver with its binding.
- Replace pse_get_types helper by pse_has_podl and pse_has_c33
- Changed the PSE core bindings.
- Add a setup_pi_matrix callback.
- Register regulator for each PSE PI (Power Interface).
- Changed the PD692x0 bindings.
- Updated PD692x0 drivers to new bindings and PSE PI description.
- Updated PD692x0 drivers according to the reviews and made fixes.
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v4:
- Replaced sponsored-by tag by a simple sentence.
- Fix pse_pi node bindings.
- Add pse pi documentation written by Oleksij.
- Link to v3: https://lore.kernel.org/r/[email protected]

Signed-off-by: Kory Maincent <[email protected]>
---
Kory Maincent (17):
MAINTAINERS: net: Add Oleksij to pse-pd maintainers
of: property: Add fw_devlink support for pse parent
net: pse-pd: Rectify and adapt the naming of admin_cotrol member of struct pse_control_config
ethtool: Expand Ethernet Power Equipment with c33 (PoE) alongside PoDL
net: pse-pd: Introduce PSE types enumeration
net: ethtool: pse-pd: Expand pse commands with the PSE PoE interface
netlink: specs: Modify pse attribute prefix
netlink: specs: Expand the pse netlink command with PoE interface
MAINTAINERS: Add myself to pse networking maintainer
net: pse-pd: Add support for PSE PIs
dt-bindings: net: pse-pd: Add another way of describing several PSE PIs
net: pse-pd: Add support for setup_pi_matrix callback
net: pse-pd: Use regulator framework within PSE framework
dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller
net: pse-pd: Add PD692x0 PSE controller driver
dt-bindings: net: pse-pd: Add bindings for TPS23881 PSE controller
net: pse-pd: Add TI TPS23881 PSE controller driver

.../bindings/net/pse-pd/microchip,pd692x0.yaml | 157 +++
.../bindings/net/pse-pd/pse-controller.yaml | 84 +-
.../bindings/net/pse-pd/ti,tps23881.yaml | 111 ++
Documentation/netlink/specs/ethtool.yaml | 33 +-
Documentation/networking/ethtool-netlink.rst | 20 +
Documentation/networking/index.rst | 1 +
Documentation/networking/pse-pd/index.rst | 10 +
Documentation/networking/pse-pd/introduction.rst | 73 ++
Documentation/networking/pse-pd/pse-pi.rst | 275 +++++
MAINTAINERS | 8 +
drivers/net/mdio/fwnode_mdio.c | 29 +-
drivers/net/pse-pd/Kconfig | 20 +
drivers/net/pse-pd/Makefile | 2 +
drivers/net/pse-pd/pd692x0.c | 1223 ++++++++++++++++++++
drivers/net/pse-pd/pse_core.c | 491 +++++++-
drivers/net/pse-pd/pse_regulator.c | 49 +-
drivers/net/pse-pd/tps23881.c | 818 +++++++++++++
drivers/of/property.c | 2 +
include/linux/pse-pd/pse.h | 89 +-
include/uapi/linux/ethtool.h | 43 +
include/uapi/linux/ethtool_netlink.h | 3 +
include/uapi/linux/pse.h | 23 +
net/ethtool/pse-pd.c | 60 +-
23 files changed, 3504 insertions(+), 120 deletions(-)
---
base-commit: 069f62e4e393fc9ffa2f0da501155fbdce7e2605
change-id: 20231024-feature_poe-139490e73403

Best regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com



2024-02-15 16:12:26

by Kory Maincent

[permalink] [raw]
Subject: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

Add the PD692x0 I2C Power Sourcing Equipment controller device tree
bindings documentation.

This patch is sponsored by Dent Project <[email protected]>.

Signed-off-by: Kory Maincent <[email protected]>
---

Changes in v2:
- Enhance ports-matrix description.
- Replace additionalProperties by unevaluatedProperties.
- Drop i2c suffix.

Changes in v3:
- Remove ports-matrix parameter.
- Add description of all physical ports and managers.
- Add pse_pis subnode moving to the API of pse-controller binding.
- Remove the MAINTAINERS section for this driver as I will be maintaining
all pse-pd subsystem.
---
.../bindings/net/pse-pd/microchip,pd692x0.yaml | 157 +++++++++++++++++++++
1 file changed, 157 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml
new file mode 100644
index 000000000000..57ba5365157c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml
@@ -0,0 +1,157 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pse-pd/microchip,pd692x0.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PD692x0 Power Sourcing Equipment controller
+
+maintainers:
+ - Kory Maincent <[email protected]>
+
+allOf:
+ - $ref: pse-controller.yaml#
+
+properties:
+ compatible:
+ enum:
+ - microchip,pd69200
+ - microchip,pd69210
+ - microchip,pd69220
+
+ reg:
+ maxItems: 1
+
+ managers:
+ $ref: "#/$defs/managers"
+ description:
+ List of the PD69208T4/PD69204T4/PD69208M PSE managers. Each manager
+ have 4 or 8 physical ports according to the chip version. No need to
+ specify the SPI chip select as it is automatically detected by the
+ PD692x0 PSE controller. The PSE managers have to be described from
+ the lowest chip select to the greatest one, which is the detection
+ behavior of the PD692x0 PSE controller. The PD692x0 support up to
+ 12 PSE managers which can expose up to 96 physical ports. All
+ physical ports available on a manager have to be described in the
+ incremental order even if they are not used.
+
+required:
+ - compatible
+ - reg
+ - pse_pis
+
+$defs:
+ manager:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ reg:
+ maxItems: 1
+
+ patternProperties:
+ '^port@[0-7]$':
+ type: object
+ required:
+ - reg
+
+ required:
+ - reg
+
+ managers:
+ type: object
+
+ properties:
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ patternProperties:
+ "^manager@0[0-9]|1[0-2]$":
+ $ref: "#/$defs/manager"
+
+ required:
+ - "#address-cells"
+ - "#size-cells"
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ethernet-pse@3c {
+ compatible = "microchip,pd69200";
+ reg = <0x3c>;
+
+ managers {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ manager@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phys0: port@0 {
+ reg = <0>;
+ };
+
+ phys1: port@1 {
+ reg = <1>;
+ };
+
+ phys2: port@2 {
+ reg = <2>;
+ };
+
+ phys3: port@3 {
+ reg = <3>;
+ };
+ };
+
+ manager@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phys4: port@0 {
+ reg = <0>;
+ };
+
+ phys5: port@1 {
+ reg = <1>;
+ };
+
+ phys6: port@2 {
+ reg = <2>;
+ };
+
+ phys7: port@3 {
+ reg = <3>;
+ };
+ };
+ };
+
+ pse_pis {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pse_pi0: pse_pi@0 {
+ reg = <0>;
+ #pse-cells = <0>;
+ pairset-names = "alternative-a", "alternative-b";
+ pairsets = <&phys0>, <&phys1>;
+ };
+ pse_pi1: pse_pi@1 {
+ reg = <1>;
+ #pse-cells = <0>;
+ pairset-names = "alternative-a";
+ pairsets = <&phys2>;
+ };
+ };
+ };
+ };

--
2.25.1


2024-02-15 16:13:05

by Kory Maincent

[permalink] [raw]
Subject: [PATCH net-next v4 17/17] net: pse-pd: Add TI TPS23881 PSE controller driver

Add a new driver for the TI TPS23881 I2C Power Sourcing Equipment
controller.

This patch is sponsored by Dent Project <[email protected]>.

Signed-off-by: Kory Maincent <[email protected]>

---
Change in v3:
- New patch.
---
drivers/net/pse-pd/Kconfig | 9 +
drivers/net/pse-pd/Makefile | 1 +
drivers/net/pse-pd/tps23881.c | 818 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 828 insertions(+)

diff --git a/drivers/net/pse-pd/Kconfig b/drivers/net/pse-pd/Kconfig
index e3a6ba669f20..80cf373a5a0e 100644
--- a/drivers/net/pse-pd/Kconfig
+++ b/drivers/net/pse-pd/Kconfig
@@ -31,4 +31,13 @@ config PSE_PD692X0
To compile this driver as a module, choose M here: the
module will be called pd692x0.

+config PSE_TPS23881
+ tristate "TPS23881 PSE controller"
+ depends on I2C
+ help
+ This module provides support for TPS23881 regulator based Ethernet
+ Power Sourcing Equipment.
+
+ To compile this driver as a module, choose M here: the
+ module will be called tps23881.
endif
diff --git a/drivers/net/pse-pd/Makefile b/drivers/net/pse-pd/Makefile
index 9c12c4a65730..9d2898b36737 100644
--- a/drivers/net/pse-pd/Makefile
+++ b/drivers/net/pse-pd/Makefile
@@ -5,3 +5,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
diff --git a/drivers/net/pse-pd/tps23881.c b/drivers/net/pse-pd/tps23881.c
new file mode 100644
index 000000000000..432b1686fd9b
--- /dev/null
+++ b/drivers/net/pse-pd/tps23881.c
@@ -0,0 +1,818 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Driver for the TI TPS23881 PoE PSE Controller driver (I2C bus)
+ *
+ * Copyright (c) 2023 Bootlin, Kory Maincent <[email protected]>
+ */
+
+#include <linux/delay.h>
+#include <linux/firmware.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 TPS23881_MAX_CHANS 8
+
+#define TPS23881_REG_PW_STATUS 0x10
+#define TPS23881_REG_OP_MODE 0x12
+#define TPS23881_REG_DIS_EN 0x13
+#define TPS23881_REG_DET_CLA_EN 0x14
+#define TPS23881_REG_GEN_MASK 0x17
+#define TPS23881_REG_NBITACC BIT(5)
+#define TPS23881_REG_PW_EN 0x19
+#define TPS23881_REG_PORT_MAP 0x26
+#define TPS23881_REG_PORT_POWER 0x29
+#define TPS23881_REG_POEPLUS 0x40
+#define TPS23881_REG_TPON BIT(0)
+#define TPS23881_REG_FWREV 0x41
+#define TPS23881_REG_DEVID 0x43
+#define TPS23881_REG_SRAM_CTRL 0x60
+#define TPS23881_REG_SRAM_DATA 0x61
+
+struct tps23881_port_desc {
+ u8 chan[2];
+ bool is_4p;
+};
+
+struct tps23881_priv {
+ struct i2c_client *client;
+ struct pse_controller_dev pcdev;
+ struct device_node *np;
+ struct tps23881_port_desc port[TPS23881_MAX_CHANS];
+};
+
+static struct tps23881_priv *to_tps23881_priv(struct pse_controller_dev *pcdev)
+{
+ return container_of(pcdev, struct tps23881_priv, pcdev);
+}
+
+static int tps23881_pi_enable(struct pse_controller_dev *pcdev, int id)
+{
+ struct tps23881_priv *priv = to_tps23881_priv(pcdev);
+ struct i2c_client *client = priv->client;
+ u8 chan;
+ u16 val;
+ int ret;
+
+ if (id >= TPS23881_MAX_CHANS)
+ return -ERANGE;
+
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_PW_STATUS);
+ if (ret < 0)
+ return ret;
+
+ chan = priv->port[id].chan[0];
+ if (chan < 4)
+ val = (u16)(ret | BIT(chan));
+ else
+ val = (u16)(ret | BIT(chan + 4));
+
+ if (priv->port[id].is_4p) {
+ chan = priv->port[id].chan[1];
+ if (chan < 4)
+ val |= BIT(chan);
+ else
+ val |= BIT(chan + 4);
+ }
+
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_PW_EN, val);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int tps23881_pi_disable(struct pse_controller_dev *pcdev, int id)
+{
+ struct tps23881_priv *priv = to_tps23881_priv(pcdev);
+ struct i2c_client *client = priv->client;
+ u8 chan;
+ u16 val;
+ int ret;
+
+ if (id >= TPS23881_MAX_CHANS)
+ return -ERANGE;
+
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_PW_STATUS);
+ if (ret < 0)
+ return ret;
+
+ chan = priv->port[id].chan[0];
+ if (chan < 4)
+ val = (u16)(ret | BIT(chan + 4));
+ else
+ val = (u16)(ret | BIT(chan + 8));
+
+ if (priv->port[id].is_4p) {
+ chan = priv->port[id].chan[1];
+ if (chan < 4)
+ val |= BIT(chan + 4);
+ else
+ val |= BIT(chan + 8);
+ }
+
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_PW_EN, val);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int tps23881_pi_is_enabled(struct pse_controller_dev *pcdev, int id)
+{
+ struct tps23881_priv *priv = to_tps23881_priv(pcdev);
+ struct i2c_client *client = priv->client;
+ bool enabled;
+ u8 chan;
+ int ret;
+
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_PW_STATUS);
+ if (ret < 0)
+ return ret;
+
+ chan = priv->port[id].chan[0];
+ if (chan < 4)
+ enabled = ret & BIT(chan);
+ else
+ enabled = ret & BIT(chan + 4);
+
+ if (priv->port[id].is_4p) {
+ chan = priv->port[id].chan[1];
+ if (chan < 4)
+ enabled &= !!(ret & BIT(chan));
+ else
+ enabled &= !!(ret & BIT(chan + 4));
+ }
+
+ /* Return enabled status only if both channel are on this state */
+ return enabled;
+}
+
+static int tps23881_ethtool_get_status(struct pse_controller_dev *pcdev,
+ unsigned long id,
+ struct netlink_ext_ack *extack,
+ struct pse_control_status *status)
+{
+ struct tps23881_priv *priv = to_tps23881_priv(pcdev);
+ struct i2c_client *client = priv->client;
+ bool enabled, delivering;
+ u8 chan;
+ int ret;
+
+ ret = i2c_smbus_read_word_data(client, TPS23881_REG_PW_STATUS);
+ if (ret < 0)
+ return ret;
+
+ chan = priv->port[id].chan[0];
+ if (chan < 4) {
+ enabled = ret & BIT(chan);
+ delivering = ret & BIT(chan + 4);
+ } else {
+ enabled = ret & BIT(chan + 4);
+ delivering = ret & BIT(chan + 8);
+ }
+
+ if (priv->port[id].is_4p) {
+ chan = priv->port[id].chan[1];
+ if (chan < 4) {
+ enabled &= !!(ret & BIT(chan));
+ delivering &= !!(ret & BIT(chan + 4));
+ } else {
+ enabled &= !!(ret & BIT(chan + 4));
+ delivering &= !!(ret & BIT(chan + 8));
+ }
+ }
+
+ /* Return delivering status only if both channel are on this state */
+ if (delivering)
+ status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_DELIVERING;
+ else
+ status->c33_pw_status = ETHTOOL_C33_PSE_PW_D_STATUS_DISABLED;
+
+ /* Return enabled status only if both channel are on this state */
+ if (enabled)
+ status->c33_admin_state = ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED;
+ else
+ status->c33_admin_state = ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED;
+
+ return 0;
+}
+
+/* Parse managers subnode into a array of device node */
+static int
+tps23881_get_of_channels(struct tps23881_priv *priv,
+ struct device_node *chan_node[TPS23881_MAX_CHANS])
+{
+ struct device_node *channels_node, *node;
+ int i, ret;
+
+ if (!priv->np)
+ return -EINVAL;
+
+ channels_node = of_find_node_by_name(priv->np, "channels");
+ if (!channels_node)
+ return -EINVAL;
+
+ for_each_child_of_node(channels_node, node) {
+ u32 chan_id;
+
+ if (!of_node_name_eq(node, "channel"))
+ continue;
+
+ ret = of_property_read_u32(node, "reg", &chan_id);
+ if (ret) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (chan_id >= TPS23881_MAX_CHANS || chan_node[chan_id]) {
+ dev_err(&priv->client->dev,
+ "wrong number of port (%d)\n", chan_id);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ of_node_get(node);
+ chan_node[chan_id] = node;
+ }
+
+ of_node_put(channels_node);
+ return 0;
+
+out:
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ of_node_put(chan_node[i]);
+ chan_node[i] = NULL;
+ }
+
+ of_node_put(node);
+ of_node_put(channels_node);
+ return ret;
+}
+
+struct tps23881_port_matrix {
+ u8 pi_id;
+ u8 lgcl_chan[2];
+ u8 hw_chan[2];
+ bool is_4p;
+ bool exist;
+};
+
+static int
+tps23881_match_channel(const struct pse_pi_pairset *pairset,
+ struct device_node *chan_node[TPS23881_MAX_CHANS])
+{
+ int i;
+
+ /* Look on every channels */
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ if (pairset->np == chan_node[i])
+ return i;
+ }
+
+ return -ENODEV;
+}
+
+static bool
+tps23881_is_chan_free(struct tps23881_port_matrix port_matrix[TPS23881_MAX_CHANS],
+ int chan)
+{
+ int i;
+
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ if (port_matrix[i].exist &&
+ (port_matrix[i].hw_chan[0] == chan ||
+ port_matrix[i].hw_chan[1] == chan))
+ return false;
+ }
+
+ return true;
+}
+
+/* Fill port matrix with the matching channels */
+static int
+tps23881_match_port_matrix(struct pse_pi *pi, int pi_id,
+ struct device_node *chan_node[TPS23881_MAX_CHANS],
+ struct tps23881_port_matrix port_matrix[TPS23881_MAX_CHANS])
+{
+ int ret;
+
+ if (!pi->pairset[0].np)
+ return 0;
+
+ ret = tps23881_match_channel(&pi->pairset[0], chan_node);
+ if (ret < 0)
+ return ret;
+
+ if (!tps23881_is_chan_free(port_matrix, ret)) {
+ pr_err("tps23881: channel %d already used\n", ret);
+ return -ENODEV;
+ }
+
+ port_matrix[pi_id].hw_chan[0] = ret;
+ port_matrix[pi_id].exist = true;
+
+ if (!pi->pairset[1].np)
+ return 0;
+
+ ret = tps23881_match_channel(&pi->pairset[1], chan_node);
+ if (ret < 0)
+ return ret;
+
+ if (!tps23881_is_chan_free(port_matrix, ret)) {
+ pr_err("tps23881: channel %d already used\n", ret);
+ return -ENODEV;
+ }
+
+ if (port_matrix[pi_id].hw_chan[0] / 4 != ret / 4) {
+ pr_err("tps23881: 4-pair PSE can only be set within the same 4 ports group");
+ return -ENODEV;
+ }
+
+ port_matrix[pi_id].hw_chan[1] = ret;
+ port_matrix[pi_id].is_4p = true;
+
+ return 0;
+}
+
+static int
+tps23881_get_unused_chan(struct tps23881_port_matrix port_matrix[TPS23881_MAX_CHANS],
+ int port_cnt)
+{
+ bool used;
+ int i, j;
+
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ used = false;
+
+ for (j = 0; j < port_cnt; j++) {
+ if (port_matrix[j].hw_chan[0] == i) {
+ used = true;
+ break;
+ }
+
+ if (port_matrix[j].is_4p &&
+ port_matrix[j].hw_chan[1] == i) {
+ used = true;
+ break;
+ }
+ }
+
+ if (!used)
+ return i;
+ }
+
+ return -1;
+}
+
+/* Sort the port matrix to following particular hardware ports matrix
+ * specification of the tps23881. The device has two 4-ports groups and
+ * each 4-pair powered device has to be configured to use two consecutive
+ * logical channel in each 4 ports group (1 and 2 or 3 and 4). Also the
+ * hardware matrix has to be fully configured even with unused chan to be
+ * valid.
+ */
+static int
+tps23881_sort_port_matrix(struct tps23881_port_matrix port_matrix[TPS23881_MAX_CHANS])
+{
+ struct tps23881_port_matrix tmp_port_matrix[TPS23881_MAX_CHANS] = {0};
+ int i, ret, port_cnt = 0, cnt_4ch_grp1 = 0, cnt_4ch_grp2 = 4;
+
+ /* Configure 4p port matrix */
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ int *cnt;
+
+ if (!port_matrix[i].exist || !port_matrix[i].is_4p)
+ continue;
+
+ if (port_matrix[i].hw_chan[0] < 4)
+ cnt = &cnt_4ch_grp1;
+ else
+ cnt = &cnt_4ch_grp2;
+
+ tmp_port_matrix[port_cnt].exist = true;
+ tmp_port_matrix[port_cnt].is_4p = true;
+ tmp_port_matrix[port_cnt].pi_id = i;
+ tmp_port_matrix[port_cnt].hw_chan[0] = port_matrix[i].hw_chan[0];
+ tmp_port_matrix[port_cnt].hw_chan[1] = port_matrix[i].hw_chan[1];
+
+ /* 4-pair ports have to be configured with consecutive
+ * logical channels 0 and 1, 2 and 3.
+ */
+ tmp_port_matrix[port_cnt].lgcl_chan[0] = (*cnt)++;
+ tmp_port_matrix[port_cnt].lgcl_chan[1] = (*cnt)++;
+
+ port_cnt++;
+ }
+
+ /* Configure 2p port matrix */
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ int *cnt;
+
+ if (!port_matrix[i].exist || port_matrix[i].is_4p)
+ continue;
+
+ if (port_matrix[i].hw_chan[0] < 4)
+ cnt = &cnt_4ch_grp1;
+ else
+ cnt = &cnt_4ch_grp2;
+
+ tmp_port_matrix[port_cnt].exist = true;
+ tmp_port_matrix[port_cnt].pi_id = i;
+ tmp_port_matrix[port_cnt].lgcl_chan[0] = (*cnt)++;
+ tmp_port_matrix[port_cnt].hw_chan[0] = port_matrix[i].hw_chan[0];
+
+ port_cnt++;
+ }
+
+ /* Complete the rest of the first 4 port group matrix even if
+ * channels are unused
+ */
+ while (cnt_4ch_grp1 < 4) {
+ ret = tps23881_get_unused_chan(tmp_port_matrix, port_cnt);
+ if (ret < 0) {
+ pr_err("tps23881: port matrix issue, no chan available\n");
+ return -ENODEV;
+ }
+
+ if (port_cnt >= TPS23881_MAX_CHANS) {
+ pr_err("tps23881: wrong number of channels\n");
+ return -ENODEV;
+ }
+ tmp_port_matrix[port_cnt].lgcl_chan[0] = cnt_4ch_grp1;
+ tmp_port_matrix[port_cnt].hw_chan[0] = ret;
+ cnt_4ch_grp1++;
+ port_cnt++;
+ }
+
+ /* Complete the rest of the second 4 port group matrix even if
+ * channels are unused
+ */
+ while (cnt_4ch_grp2 < 8) {
+ ret = tps23881_get_unused_chan(tmp_port_matrix, port_cnt);
+ if (ret < 0) {
+ pr_err("tps23881: port matrix issue, no chan available\n");
+ return -ENODEV;
+ }
+
+ if (port_cnt >= TPS23881_MAX_CHANS) {
+ pr_err("tps23881: wrong number of channels\n");
+ return -ENODEV;
+ }
+ tmp_port_matrix[port_cnt].lgcl_chan[0] = cnt_4ch_grp2;
+ tmp_port_matrix[port_cnt].hw_chan[0] = ret;
+ cnt_4ch_grp2++;
+ port_cnt++;
+ }
+
+ memcpy(port_matrix, tmp_port_matrix, sizeof(tmp_port_matrix));
+
+ return port_cnt;
+}
+
+/* Write port matrix to the hardware port matrix and the software port
+ * matrix.
+ */
+static int
+tps23881_write_port_matrix(struct tps23881_priv *priv,
+ struct tps23881_port_matrix port_matrix[TPS23881_MAX_CHANS],
+ int port_cnt)
+{
+ struct i2c_client *client = priv->client;
+ u8 pi_id, lgcl_chan, hw_chan;
+ u16 val = 0;
+ int i, ret;
+
+ for (i = 0; i < port_cnt; i++) {
+ pi_id = port_matrix[i].pi_id;
+ lgcl_chan = port_matrix[i].lgcl_chan[0];
+ hw_chan = port_matrix[i].hw_chan[0] % 4;
+
+ /* Set software port matrix for existing ports */
+ if (port_matrix[i].exist)
+ priv->port[pi_id].chan[0] = lgcl_chan;
+
+ /* Set hardware port matrix for all ports */
+ val |= hw_chan << (lgcl_chan * 2);
+
+ if (!port_matrix[i].is_4p)
+ continue;
+
+ lgcl_chan = port_matrix[i].lgcl_chan[1];
+ hw_chan = port_matrix[i].hw_chan[1] % 4;
+
+ /* Set software port matrix for existing ports */
+ if (port_matrix[i].exist) {
+ priv->port[pi_id].is_4p = true;
+ priv->port[pi_id].chan[1] = lgcl_chan;
+ }
+
+ /* Set hardware port matrix for all ports */
+ val |= hw_chan << (lgcl_chan * 2);
+ }
+
+ /* Write hardware ports matrix */
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_PORT_MAP, val);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int
+tps23881_set_ports_conf(struct tps23881_priv *priv,
+ struct tps23881_port_matrix port_matrix[TPS23881_MAX_CHANS])
+{
+ struct i2c_client *client = priv->client;
+ int i, ret;
+ u16 val;
+
+ /* Set operating mode */
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_OP_MODE, 0xaaaa);
+ if (ret < 0)
+ return ret;
+
+ /* Disable DC disconnect */
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_DIS_EN, 0x0);
+ if (ret < 0)
+ return ret;
+
+ /* Set port power allocation */
+ val = 0;
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ if (!port_matrix[i].exist)
+ continue;
+
+ if (port_matrix[i].is_4p)
+ val |= 0xf << ((port_matrix[i].lgcl_chan[0] / 2) * 4);
+ else
+ val |= 0x3 << ((port_matrix[i].lgcl_chan[0] / 2) * 4);
+ }
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_PORT_POWER, val);
+ if (ret < 0)
+ return ret;
+
+ /* Enable detection and classification */
+ val = 0;
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ if (!port_matrix[i].exist)
+ continue;
+
+ val |= BIT(port_matrix[i].lgcl_chan[0]) |
+ BIT(port_matrix[i].lgcl_chan[0] + 4);
+ if (port_matrix[i].is_4p)
+ val |= BIT(port_matrix[i].lgcl_chan[1]) |
+ BIT(port_matrix[i].lgcl_chan[1] + 4);
+ }
+ ret = i2c_smbus_write_word_data(client, TPS23881_REG_DET_CLA_EN, 0xffff);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int
+tps23881_set_ports_matrix(struct tps23881_priv *priv,
+ struct device_node *chan_node[TPS23881_MAX_CHANS])
+{
+ struct tps23881_port_matrix port_matrix[TPS23881_MAX_CHANS] = {0};
+ int i, ret;
+
+ /* Update with values for every PSE PIs */
+ for (i = 0; i < TPS23881_MAX_CHANS; i++) {
+ ret = tps23881_match_port_matrix(&priv->pcdev.pi[i], i,
+ chan_node, port_matrix);
+ if (ret)
+ return ret;
+ }
+
+ ret = tps23881_sort_port_matrix(port_matrix);
+ if (ret < 0)
+ return ret;
+
+ ret = tps23881_write_port_matrix(priv, port_matrix, ret);
+ if (ret)
+ return ret;
+
+ ret = tps23881_set_ports_conf(priv, port_matrix);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int tps23881_setup_pi_matrix(struct pse_controller_dev *pcdev)
+{
+ struct device_node *chan_node[TPS23881_MAX_CHANS] = {NULL};
+ struct tps23881_priv *priv = to_tps23881_priv(pcdev);
+ int ret, i;
+
+ ret = tps23881_get_of_channels(priv, chan_node);
+ if (ret < 0) {
+ dev_warn(&priv->client->dev,
+ "Unable to parse port-matrix, default matrix will be used\n");
+ return 0;
+ }
+
+ ret = tps23881_set_ports_matrix(priv, chan_node);
+
+ for (i = 0; i < TPS23881_MAX_CHANS; i++)
+ of_node_put(chan_node[i]);
+
+ return ret;
+}
+
+static const struct pse_controller_ops tps23881_ops = {
+ .setup_pi_matrix = tps23881_setup_pi_matrix,
+ .pi_enable = tps23881_pi_enable,
+ .pi_disable = tps23881_pi_disable,
+ .pi_is_enabled = tps23881_pi_is_enabled,
+ .ethtool_get_status = tps23881_ethtool_get_status,
+};
+
+static const char fw_parity_name[] = "ti/tps23881/tps23881-parity-14.bin";
+static const char fw_sram_name[] = "ti/tps23881/tps23881-sram-14.bin";
+
+struct tps23881_fw_conf {
+ u8 reg;
+ u8 val;
+};
+
+static const struct tps23881_fw_conf tps23881_parity_flash_conf[] = {
+ {.reg = 0x60, .val = 0x01},
+ {.reg = 0x62, .val = 0x00},
+ {.reg = 0x63, .val = 0x80},
+ {.reg = 0x60, .val = 0xC4},
+ {.reg = 0x1D, .val = 0xBC},
+ {.reg = 0xD7, .val = 0x02},
+ {.reg = 0x91, .val = 0x00},
+ {.reg = 0x90, .val = 0x00},
+ {.reg = 0xD7, .val = 0x00},
+ {.reg = 0x1D, .val = 0x00},
+ { /* sentinel */ }
+};
+
+static const struct tps23881_fw_conf tps23881_sram_flash_conf[] = {
+ {.reg = 0x60, .val = 0xC5},
+ {.reg = 0x62, .val = 0x00},
+ {.reg = 0x63, .val = 0x80},
+ {.reg = 0x60, .val = 0xC0},
+ {.reg = 0x1D, .val = 0xBC},
+ {.reg = 0xD7, .val = 0x02},
+ {.reg = 0x91, .val = 0x00},
+ {.reg = 0x90, .val = 0x00},
+ {.reg = 0xD7, .val = 0x00},
+ {.reg = 0x1D, .val = 0x00},
+ { /* sentinel */ }
+};
+
+static int tps23881_flash_fw_part(struct i2c_client *client,
+ const char *fw_name,
+ const struct tps23881_fw_conf *fw_conf)
+{
+ const struct firmware *fw = NULL;
+ int i, ret;
+
+ ret = request_firmware(&fw, fw_name, &client->dev);
+ if (ret)
+ return ret;
+
+ dev_info(&client->dev, "Flashing %s\n", fw_name);
+
+ /* Prepare device for RAM download */
+ while (fw_conf->reg) {
+ ret = i2c_smbus_write_byte_data(client, fw_conf->reg,
+ fw_conf->val);
+ if (ret < 0)
+ return ret;
+
+ fw_conf++;
+ }
+
+ /* Flash the firmware file */
+ for (i = 0; i < fw->size; i++) {
+ ret = i2c_smbus_write_byte_data(client,
+ TPS23881_REG_SRAM_DATA,
+ fw->data[i]);
+ if (ret < 0)
+ return ret;
+ }
+
+ release_firmware(fw);
+
+ return 0;
+}
+
+static int tps23881_flash_fw(struct i2c_client *client)
+{
+ int ret;
+
+ ret = tps23881_flash_fw_part(client, fw_parity_name,
+ tps23881_parity_flash_conf);
+ if (ret)
+ return ret;
+
+ ret = tps23881_flash_fw_part(client, fw_sram_name,
+ tps23881_sram_flash_conf);
+ if (ret)
+ return ret;
+
+ ret = i2c_smbus_write_byte_data(client, TPS23881_REG_SRAM_CTRL, 0x18);
+ if (ret < 0)
+ return ret;
+
+ mdelay(12);
+
+ return 0;
+}
+
+static int tps23881_i2c_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct tps23881_priv *priv;
+ int ret;
+ u8 val;
+
+ 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, TPS23881_REG_DEVID);
+ if (ret < 0)
+ return ret;
+
+ if (ret != 0x22) {
+ dev_err(dev, "Wrong device ID\n");
+ return -ENXIO;
+ }
+
+ ret = tps23881_flash_fw(client);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_byte_data(client, TPS23881_REG_FWREV);
+ if (ret < 0)
+ return ret;
+
+ dev_info(&client->dev, "Firmware revision 0x%x\n", ret);
+
+ /* Set configuration B, 16 bit access on a single device address */
+ ret = i2c_smbus_read_byte_data(client, TPS23881_REG_GEN_MASK);
+ if (ret < 0)
+ return ret;
+
+ val = ret | TPS23881_REG_NBITACC;
+ ret = i2c_smbus_write_byte_data(client, TPS23881_REG_GEN_MASK, val);
+ if (ret < 0)
+ return ret;
+
+ priv->client = client;
+ i2c_set_clientdata(client, priv);
+ priv->np = dev->of_node;
+
+ priv->pcdev.owner = THIS_MODULE;
+ priv->pcdev.ops = &tps23881_ops;
+ priv->pcdev.dev = dev;
+ priv->pcdev.types = PSE_C33;
+ priv->pcdev.nr_lines = TPS23881_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 tps23881_id[] = {
+ { "tps23881", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, tps23881_id);
+
+static const struct of_device_id tps23881_of_match[] = {
+ { .compatible = "ti,tps23881", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, tps23881_of_match);
+
+static struct i2c_driver tps23881_driver = {
+ .probe = tps23881_i2c_probe,
+ .id_table = tps23881_id,
+ .driver = {
+ .name = "tps23881",
+ .of_match_table = tps23881_of_match,
+ },
+};
+module_i2c_driver(tps23881_driver);
+
+MODULE_AUTHOR("Kory Maincent <[email protected]>");
+MODULE_DESCRIPTION("TI TPS23881 PoE PSE Controller driver");
+MODULE_LICENSE("GPL");

--
2.25.1


2024-02-15 16:17:10

by Kory Maincent

[permalink] [raw]
Subject: [PATCH net-next v4 06/17] net: ethtool: pse-pd: Expand pse commands with the PSE PoE interface

Add PSE PoE interface support in the ethtool pse command.

This patch is sponsored by Dent Project <[email protected]>.

Reviewed-by: Andrew Lunn <[email protected]>
Signed-off-by: Kory Maincent <[email protected]>
---

Changes in v2:
- Follow the "c33" PoE prefix naming change.

Changes in v3:
- Replace the pse_get_types() helper by pse_has_podl() and pse_has_c33().
- Replace PoE to c33 in the netlink error log.
- Fix documentation build warning.
---
Documentation/networking/ethtool-netlink.rst | 20 ++++++++++
net/ethtool/pse-pd.c | 60 +++++++++++++++++++++++-----
2 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index d583d9abf2f8..294187c3a3b0 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1717,6 +1717,10 @@ Kernel response contents:
PSE functions
``ETHTOOL_A_PODL_PSE_PW_D_STATUS`` u32 power detection status of the
PoDL PSE.
+ ``ETHTOOL_A_C33_PSE_ADMIN_STATE`` u32 Operational state of the PoE
+ PSE functions.
+ ``ETHTOOL_A_C33_PSE_PW_D_STATUS`` u32 power detection status of the
+ PoE PSE.
====================================== ====== =============================

When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` attribute identifies
@@ -1728,6 +1732,12 @@ aPoDLPSEAdminState. Possible values are:
.. kernel-doc:: include/uapi/linux/ethtool.h
:identifiers: ethtool_podl_pse_admin_state

+The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_STATE`` implementing
+``IEEE 802.3-2022`` 30.9.1.1.2 aPSEAdminState.
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+ :identifiers: ethtool_c33_pse_admin_state
+
When set, the optional ``ETHTOOL_A_PODL_PSE_PW_D_STATUS`` attribute identifies
the power detection status of the PoDL PSE. The status depend on internal PSE
state machine and automatic PD classification support. This option is
@@ -1737,6 +1747,12 @@ Possible values are:
.. kernel-doc:: include/uapi/linux/ethtool.h
:identifiers: ethtool_podl_pse_pw_d_status

+The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_PW_D_STATUS`` implementing
+``IEEE 802.3-2022`` 30.9.1.1.5 aPSEPowerDetectionStatus.
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+ :identifiers: ethtool_c33_pse_pw_d_status
+
PSE_SET
=======

@@ -1747,6 +1763,7 @@ Request contents:
====================================== ====== =============================
``ETHTOOL_A_PSE_HEADER`` nested request header
``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` u32 Control PoDL PSE Admin state
+ ``ETHTOOL_A_C33_PSE_ADMIN_CONTROL`` u32 Control PSE Admin state
====================================== ====== =============================

When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` attribute is used
@@ -1754,6 +1771,9 @@ to control PoDL PSE Admin functions. This option is implementing
``IEEE 802.3-2018`` 30.15.1.2.1 acPoDLPSEAdminControl. See
``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` for supported values.

+The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_CONTROL`` implementing
+``IEEE 802.3-2022`` 30.9.1.2.1 acPSEAdminControl.
+
RSS_GET
=======

diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index aef57a058f0d..a3bfc3d9644e 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -82,6 +82,10 @@ static int pse_reply_size(const struct ethnl_req_info *req_base,
len += nla_total_size(sizeof(u32)); /* _PODL_PSE_ADMIN_STATE */
if (st->podl_pw_status > 0)
len += nla_total_size(sizeof(u32)); /* _PODL_PSE_PW_D_STATUS */
+ if (st->c33_admin_state > 0)
+ len += nla_total_size(sizeof(u32)); /* _C33_PSE_ADMIN_STATE */
+ if (st->c33_pw_status > 0)
+ len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_D_STATUS */

return len;
}
@@ -103,6 +107,16 @@ static int pse_fill_reply(struct sk_buff *skb,
st->podl_pw_status))
return -EMSGSIZE;

+ if (st->c33_admin_state > 0 &&
+ nla_put_u32(skb, ETHTOOL_A_C33_PSE_ADMIN_STATE,
+ st->c33_admin_state))
+ return -EMSGSIZE;
+
+ if (st->c33_pw_status > 0 &&
+ nla_put_u32(skb, ETHTOOL_A_C33_PSE_PW_D_STATUS,
+ st->c33_pw_status))
+ return -EMSGSIZE;
+
return 0;
}

@@ -113,25 +127,18 @@ const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] =
NLA_POLICY_RANGE(NLA_U32, ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED,
ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED),
+ [ETHTOOL_A_C33_PSE_ADMIN_CONTROL] =
+ NLA_POLICY_RANGE(NLA_U32, ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED,
+ ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED),
};

static int
ethnl_set_pse_validate(struct ethnl_req_info *req_info, struct genl_info *info)
-{
- return !!info->attrs[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL];
-}
-
-static int
-ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
{
struct net_device *dev = req_info->dev;
- struct pse_control_config config = {};
struct nlattr **tb = info->attrs;
struct phy_device *phydev;

- /* this values are already validated by the ethnl_pse_set_policy */
- config.podl_admin_control = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
-
phydev = dev->phydev;
if (!phydev) {
NL_SET_ERR_MSG(info->extack, "No PHY is attached");
@@ -143,6 +150,39 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
return -EOPNOTSUPP;
}

+ if (tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
+ !(pse_has_podl(phydev->psec))) {
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL],
+ "setting PoDL PSE admin control not supported");
+ return -EOPNOTSUPP;
+ }
+ if (tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL] &&
+ !(pse_has_c33(phydev->psec))) {
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL],
+ "setting C33 PSE admin control not supported");
+ return -EOPNOTSUPP;
+ }
+
+ return 1;
+}
+
+static int
+ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
+{
+ struct net_device *dev = req_info->dev;
+ struct pse_control_config config = {};
+ struct nlattr **tb = info->attrs;
+ struct phy_device *phydev;
+
+ phydev = dev->phydev;
+ /* These values are already validated by the ethnl_pse_set_policy */
+ if (pse_has_podl(phydev->psec))
+ config.podl_admin_control = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
+ if (pse_has_c33(phydev->psec))
+ config.c33_admin_control = nla_get_u32(tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL]);
+
/* Return errno directly - PSE has no notification */
return pse_ethtool_set_config(phydev->psec, info->extack, &config);
}

--
2.25.1


2024-02-17 12:15:13

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote:
> Add the PD692x0 I2C Power Sourcing Equipment controller device tree
> bindings documentation.
>
> This patch is sponsored by Dent Project <[email protected]>.
>
> Signed-off-by: Kory Maincent <[email protected]>
> ---
..
> + pse_pis {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pse_pi0: pse_pi@0 {
> + reg = <0>;
> + #pse-cells = <0>;
> + pairset-names = "alternative-a", "alternative-b";
> + pairsets = <&phys0>, <&phys1>;
> + };
> + pse_pi1: pse_pi@1 {
> + reg = <1>;
> + #pse-cells = <0>;
> + pairset-names = "alternative-a";
> + pairsets = <&phys2>;

According to latest discussions, PSE PI nodes will need some
additional, board specific, information:
- this controller do not implements polarity switching, we need to know
what polarity is implemented on this board. The 802.3 spec provide not
really consistent names for polarity configurations:
- Alternative A MDI-X
- Alternative A MDI
- Alternative B X
- Alternative B S
The board may implement one of polarity configurations per alternative
or have additional helpers to switch them without using PSE
controller.
Even if specification explicitly say:
"The PD shall be implemented to be insensitive to the polarity of the power
supply and shall be able to operate per the PD Mode A column and the PD
Mode B column in Table 33–13"
it is possible to find reports like this:
https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070

Probably this kind of property is a good fit:
polarity-supported = "MDI-X", "MDI", "X", "S";

- Except of polarity, we have alternative-b variant with direct or
phantom feeding (No idea if it is proper description). Theoretically, this
difference would affect electrical rating specifications.
For example direct path for alternate-b (10/100Mbit only), would have
higher rating as the path over coils/magnetics. Practically, vendors do not
make different ratings for this paths, so no need to care about it for now
until someone will be able to provide good reason.
Here is example of RJ45 connector with integrated magnetics with PoE support
where alternative-a feed over magnetics and alternative-b is feed directly:
https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=5-2337992-4&DocType=Customer+Drawing&DocLang=English&PartCntxt=5-2337992-4&DocFormat=pdf

(the last topic is more an answer to my self and for archive :))

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2024-02-17 14:04:36

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

On 15/02/2024 17:02, Kory Maincent wrote:
> Add the PD692x0 I2C Power Sourcing Equipment controller device tree
> bindings documentation.
>
> This patch is sponsored by Dent Project <[email protected]>.
>
> Signed-off-by: Kory Maincent <[email protected]>
> ---
>
> Changes in v2:
> - Enhance ports-matrix description.
> - Replace additionalProperties by unevaluatedProperties.
> - Drop i2c suffix.
>
> Changes in v3:
> - Remove ports-matrix parameter.
> - Add description of all physical ports and managers.
> - Add pse_pis subnode moving to the API of pse-controller binding.
> - Remove the MAINTAINERS section for this driver as I will be maintaining
> all pse-pd subsystem.
> ---
> .../bindings/net/pse-pd/microchip,pd692x0.yaml | 157 +++++++++++++++++++++
> 1 file changed, 157 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml
> new file mode 100644
> index 000000000000..57ba5365157c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml
> @@ -0,0 +1,157 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pse-pd/microchip,pd692x0.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip PD692x0 Power Sourcing Equipment controller
> +
> +maintainers:
> + - Kory Maincent <[email protected]>
> +
> +allOf:
> + - $ref: pse-controller.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - microchip,pd69200
> + - microchip,pd69210
> + - microchip,pd69220
> +
> + reg:
> + maxItems: 1
> +
> + managers:
> + $ref: "#/$defs/managers"
> + description:
> + List of the PD69208T4/PD69204T4/PD69208M PSE managers. Each manager
> + have 4 or 8 physical ports according to the chip version. No need to
> + specify the SPI chip select as it is automatically detected by the
> + PD692x0 PSE controller. The PSE managers have to be described from
> + the lowest chip select to the greatest one, which is the detection
> + behavior of the PD692x0 PSE controller. The PD692x0 support up to
> + 12 PSE managers which can expose up to 96 physical ports. All
> + physical ports available on a manager have to be described in the
> + incremental order even if they are not used.
> +
> +required:
> + - compatible
> + - reg
> + - pse_pis
> +
> +$defs:

Why do you need defs for it? You use it only in one place.

..

> +
> + pse_pis {

Again... no underscores in node names.

> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pse_pi0: pse_pi@0 {

Just compile your DTS with W=2.


Best regards,
Krzysztof


2024-02-19 14:46:42

by Kory Maincent

[permalink] [raw]
Subject: Re: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

On Sat, 17 Feb 2024 13:14:29 +0100
Oleksij Rempel <[email protected]> wrote:

> On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote:
> > Add the PD692x0 I2C Power Sourcing Equipment controller device tree
> > bindings documentation.
> >
> > This patch is sponsored by Dent Project <[email protected]>.
> >
> > Signed-off-by: Kory Maincent <[email protected]>
> > ---
> ...
> > + pse_pis {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + pse_pi0: pse_pi@0 {
> > + reg = <0>;
> > + #pse-cells = <0>;
> > + pairset-names = "alternative-a", "alternative-b";
> > + pairsets = <&phys0>, <&phys1>;
> > + };
> > + pse_pi1: pse_pi@1 {
> > + reg = <1>;
> > + #pse-cells = <0>;
> > + pairset-names = "alternative-a";
> > + pairsets = <&phys2>;
>
> According to latest discussions, PSE PI nodes will need some
> additional, board specific, information:
> - this controller do not implements polarity switching, we need to know
> what polarity is implemented on this board. The 802.3 spec provide not
> really consistent names for polarity configurations:
> - Alternative A MDI-X
> - Alternative A MDI
> - Alternative B X
> - Alternative B S
> The board may implement one of polarity configurations per alternative
> or have additional helpers to switch them without using PSE
> controller.
> Even if specification explicitly say:
> "The PD shall be implemented to be insensitive to the polarity of the power
> supply and shall be able to operate per the PD Mode A column and the PD
> Mode B column in Table 33–13"
> it is possible to find reports like this:
> https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070

Mmh not sure we want to support broken cases that does not follow the spec.
Should we?

Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

2024-02-19 14:54:27

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

> Mmh not sure we want to support broken cases that does not follow the spec.
> Should we?

We should specify the properties a device following the spec should
use. These are the common properties we expect all devices should be
using.

Broken devices can however have additional properties, defined in
their own binding. And if we see a pattern for broken properties, we
could pull them out into a -broken.yaml binding, which the broken
devices could share.

Andrew

2024-02-20 10:40:54

by Kory Maincent

[permalink] [raw]
Subject: Re: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

On Sat, 17 Feb 2024 13:14:29 +0100
Oleksij Rempel <[email protected]> wrote:

> On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote:
> > Add the PD692x0 I2C Power Sourcing Equipment controller device tree
> > bindings documentation.
> >
> > This patch is sponsored by Dent Project <[email protected]>.
> >
> > Signed-off-by: Kory Maincent <[email protected]>
> > ---
> ...
> > + pse_pis {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + pse_pi0: pse_pi@0 {
> > + reg = <0>;
> > + #pse-cells = <0>;
> > + pairset-names = "alternative-a", "alternative-b";
> > + pairsets = <&phys0>, <&phys1>;
> > + };
> > + pse_pi1: pse_pi@1 {
> > + reg = <1>;
> > + #pse-cells = <0>;
> > + pairset-names = "alternative-a";
> > + pairsets = <&phys2>;
>
> According to latest discussions, PSE PI nodes will need some
> additional, board specific, information:
> - this controller do not implements polarity switching, we need to know
> what polarity is implemented on this board. The 802.3 spec provide not
> really consistent names for polarity configurations:
> - Alternative A MDI-X
> - Alternative A MDI
> - Alternative B X
> - Alternative B S
> The board may implement one of polarity configurations per alternative
> or have additional helpers to switch them without using PSE
> controller.
> Even if specification explicitly say:
> "The PD shall be implemented to be insensitive to the polarity of the power
> supply and shall be able to operate per the PD Mode A column and the PD
> Mode B column in Table 33–13"
> it is possible to find reports like this:
> https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070
>
> Probably this kind of property is a good fit:
> polarity-supported = "MDI-X", "MDI", "X", "S";

This property should be on the PD side.
Isn't it better to name it "polarity-provided" for each PSE PIs binding? What
do you think?
We agreed that it is mainly for ethtool to show the polarity of a PI, right?

Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

2024-02-20 11:06:28

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

On Tue, Feb 20, 2024 at 11:40:29AM +0100, Köry Maincent wrote:
> On Sat, 17 Feb 2024 13:14:29 +0100
> Oleksij Rempel <[email protected]> wrote:
>
> > On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote:
> > > Add the PD692x0 I2C Power Sourcing Equipment controller device tree
> > > bindings documentation.
> > >
> > > This patch is sponsored by Dent Project <[email protected]>.
> > >
> > > Signed-off-by: Kory Maincent <[email protected]>
> > > ---
> > ...
> > > + pse_pis {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + pse_pi0: pse_pi@0 {
> > > + reg = <0>;
> > > + #pse-cells = <0>;
> > > + pairset-names = "alternative-a", "alternative-b";
> > > + pairsets = <&phys0>, <&phys1>;
> > > + };
> > > + pse_pi1: pse_pi@1 {
> > > + reg = <1>;
> > > + #pse-cells = <0>;
> > > + pairset-names = "alternative-a";
> > > + pairsets = <&phys2>;
> >
> > According to latest discussions, PSE PI nodes will need some
> > additional, board specific, information:
> > - this controller do not implements polarity switching, we need to know
> > what polarity is implemented on this board. The 802.3 spec provide not
> > really consistent names for polarity configurations:
> > - Alternative A MDI-X
> > - Alternative A MDI
> > - Alternative B X
> > - Alternative B S
> > The board may implement one of polarity configurations per alternative
> > or have additional helpers to switch them without using PSE
> > controller.
> > Even if specification explicitly say:
> > "The PD shall be implemented to be insensitive to the polarity of the power
> > supply and shall be able to operate per the PD Mode A column and the PD
> > Mode B column in Table 33–13"
> > it is possible to find reports like this:
> > https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070
> >
> > Probably this kind of property is a good fit:
> > polarity-supported = "MDI-X", "MDI", "X", "S";
>
> This property should be on the PD side.

Probably. Right now we are on PSE side.

> Isn't it better to name it "polarity-provided" for each PSE PIs binding? What
> do you think?

Yes, this suggestion was directed for PSE PI nodes.

In the PHY world, we use "supported" capabilities for what HW can
actually do and "advertised" for how the HW is configured.

If we use word "provided", i would interpret it as subset of
"supported", which at the end is a user space policy.

Since I'm not native englisch speaker, my feeling can be wrong. So, any
one with stronger opinion may have here other preferences.

> We agreed that it is mainly for ethtool to show the polarity of a PI, right?

We have two kind of information here:
- polarity supported by HW. PSE PI may support more then one.
- actually configured polarity.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |

2024-02-21 14:42:47

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

On Sat, Feb 17, 2024 at 01:14:29PM +0100, Oleksij Rempel wrote:
> On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote:
> > Add the PD692x0 I2C Power Sourcing Equipment controller device tree
> > bindings documentation.
> >
> > This patch is sponsored by Dent Project <[email protected]>.
> >
> > Signed-off-by: Kory Maincent <[email protected]>
> > ---
> ...
> > + pse_pis {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + pse_pi0: pse_pi@0 {
> > + reg = <0>;
> > + #pse-cells = <0>;
> > + pairset-names = "alternative-a", "alternative-b";
> > + pairsets = <&phys0>, <&phys1>;
> > + };
> > + pse_pi1: pse_pi@1 {
> > + reg = <1>;
> > + #pse-cells = <0>;
> > + pairset-names = "alternative-a";
> > + pairsets = <&phys2>;
>
> According to latest discussions, PSE PI nodes will need some
> additional, board specific, information:
> - this controller do not implements polarity switching, we need to know
> what polarity is implemented on this board. The 802.3 spec provide not
> really consistent names for polarity configurations:
> - Alternative A MDI-X
> - Alternative A MDI
> - Alternative B X
> - Alternative B S
> The board may implement one of polarity configurations per alternative
> or have additional helpers to switch them without using PSE
> controller.
> Even if specification explicitly say:
> "The PD shall be implemented to be insensitive to the polarity of the power
> supply and shall be able to operate per the PD Mode A column and the PD
> Mode B column in Table 33–13"
> it is possible to find reports like this:
> https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070
>
> Probably this kind of property is a good fit:
> polarity-supported = "MDI-X", "MDI", "X", "S";

Where does that live? Looks like a property of the consumers defined in
the provider. Generally, that's not the right way for DT. I'll say it
again, I think you should be expanding #pse-cells (>1), not getting rid
of them (==0).

Rob

2024-02-21 15:08:16

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net-next v4 14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

On Wed, Feb 21, 2024 at 07:41:35AM -0700, Rob Herring wrote:
> On Sat, Feb 17, 2024 at 01:14:29PM +0100, Oleksij Rempel wrote:
> > On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote:
> > > Add the PD692x0 I2C Power Sourcing Equipment controller device tree
> > > bindings documentation.
> > >
> > > This patch is sponsored by Dent Project <[email protected]>.
> > >
> > > Signed-off-by: Kory Maincent <[email protected]>
> > > ---
> > ...
> > > + pse_pis {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + pse_pi0: pse_pi@0 {
> > > + reg = <0>;
> > > + #pse-cells = <0>;
> > > + pairset-names = "alternative-a", "alternative-b";
> > > + pairsets = <&phys0>, <&phys1>;
> > > + };
> > > + pse_pi1: pse_pi@1 {
> > > + reg = <1>;
> > > + #pse-cells = <0>;
> > > + pairset-names = "alternative-a";
> > > + pairsets = <&phys2>;
> >
> > According to latest discussions, PSE PI nodes will need some
> > additional, board specific, information:
> > - this controller do not implements polarity switching, we need to know
> > what polarity is implemented on this board. The 802.3 spec provide not
> > really consistent names for polarity configurations:
> > - Alternative A MDI-X
> > - Alternative A MDI
> > - Alternative B X
> > - Alternative B S
> > The board may implement one of polarity configurations per alternative
> > or have additional helpers to switch them without using PSE
> > controller.
> > Even if specification explicitly say:
> > "The PD shall be implemented to be insensitive to the polarity of the power
> > supply and shall be able to operate per the PD Mode A column and the PD
> > Mode B column in Table 33–13"
> > it is possible to find reports like this:
> > https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070
> >
> > Probably this kind of property is a good fit:
> > polarity-supported = "MDI-X", "MDI", "X", "S";
>
> Where does that live? Looks like a property of the consumers defined in
> the provider. Generally, that's not the right way for DT.

This is property of PSE PI (Power Interface)

Ethernet PHY --\
PSE (provider) ----> PSE PI (consumer of multiple PSE's) ----> Physial port


PSE - provides power lines.
PSE PI - switches (or not) power lines in different configurations. This
is different part of the board/system. PSE PI can have combination or
one of following configurations: "MDI-X", "MDI", "X", "S";
This is not something what PSE actually do. PSE PI and PSE are described
in IEEE802.3 specification.

> I'll say it
> again, I think you should be expanding #pse-cells (>1), not getting rid
> of them (==0).

Did you took time to read my last explanation? Sorry for making it long
description, this topic is a bit complex.

Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |