The tps23861 driver does not initialize the chip and relies on it being
in auto-mode by default. On some devices, these controllers default to
OFF-Mode and hence cannot be used at all.
This brings minimal support for initializing the controller in a user-
defined mode.
Tested on a TP-Link TL-SG2452P with 12x TI TPS23861 controllers.
Signed-off-by: Andreas Böhler <[email protected]>
---
.../bindings/hwmon/ti,tps23861.yaml | 76 +++++++++++++++++
drivers/hwmon/tps23861.c | 81 +++++++++++++++++++
2 files changed, 157 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
index 3bc8e73dfbf0..ed3a703478fb 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
@@ -35,6 +35,50 @@ required:
- compatible
- reg
+patternProperties:
+ "^port@([0-3])$":
+ type: object
+ description: Represents ports of the device and their specific configuration.
+
+ properties:
+ reg:
+ description: The port number
+ items:
+ minimum: 0
+ maximum: 3
+
+ mode:
+ description: The operating mode the device should be initialized with
+ items:
+ - enum:
+ - auto
+ - semiauto
+ - manual
+ - off
+
+ enable:
+ description: Whether the port should be enabled
+ items:
+ minimum: 0
+ maximum: 1
+
+ power:
+ description: Whether the port should be powered on
+ items:
+ minimum: 0
+ maximum: 1
+
+ poe_plus:
+ description: Whether the port should support PoE+
+ items:
+ minimum: 0
+ maximum: 1
+
+ required:
+ - reg
+
+ additionalProperties: false
+
additionalProperties: false
examples:
@@ -47,5 +91,37 @@ examples:
compatible = "ti,tps23861";
reg = <0x30>;
shunt-resistor-micro-ohms = <255000>;
+
+ port@0 {
+ reg = <0>;
+ mode = "auto";
+ enable = <1>;
+ power = <1>;
+ poe_plus = <1>;
+ };
+
+ port@1 {
+ reg = <1>;
+ mode = "semiauto";
+ enable = <1>;
+ power = <0>;
+ poe_plus = <1>;
+ };
+
+ port@2 {
+ reg = <2>;
+ mode = "manual";
+ enable = <0>;
+ power = <0>;
+ poe_plus = <0>;
+ };
+
+ port@3 {
+ reg = <3>;
+ mode = "off";
+ enable = <0>;
+ power = <0>;
+ poe_plus = <1>;
+ };
};
};
diff --git a/drivers/hwmon/tps23861.c b/drivers/hwmon/tps23861.c
index 42762e87b014..27bf8716cf12 100644
--- a/drivers/hwmon/tps23861.c
+++ b/drivers/hwmon/tps23861.c
@@ -85,6 +85,8 @@
#define PORT_DETECT_CAPACITANCE_INVALID_DELTA 11
#define PORT_DETECT_CAPACITANCE_OUT_OF_RANGE 12
#define POE_PLUS 0x40
+#define POE_PLUS_MASK(_port) \
+ GENMASK(_port + 4, _port + 4)
#define OPERATING_MODE 0x12
#define OPERATING_MODE_OFF 0
#define OPERATING_MODE_MANUAL 1
@@ -94,9 +96,22 @@
#define OPERATING_MODE_PORT_2_MASK GENMASK(3, 2)
#define OPERATING_MODE_PORT_3_MASK GENMASK(5, 4)
#define OPERATING_MODE_PORT_4_MASK GENMASK(7, 6)
+#define OPERATING_MODE_PORT(_mode, _port) \
+ (_mode << (_port * 2))
+#define DISCONNECT_ENABLE 0x13
+#define DISCONNECT_ENABLE_MASK(_port) \
+ GENMASK(_port, _port)
+#define DISCONNECT_MASK(_port) \
+ (GENMASK(_port, _port) | GENMASK(_port + 4, _port + 4))
+
+#define DETECT_CLASS_ENABLE 0x14
#define DETECT_CLASS_RESTART 0x18
#define POWER_ENABLE 0x19
+#define POWER_ENABLE_ON_MASK(_port) \
+ GENMASK(_port, _port)
+#define POWER_ENABLE_OFF_MASK(_port) \
+ GENMASK(_port + 4, _port + 4)
#define TPS23861_NUM_PORTS 4
#define TPS23861_GENERAL_MASK_1 0x17
@@ -548,7 +563,16 @@ static int tps23861_probe(struct i2c_client *client)
struct device *dev = &client->dev;
struct tps23861_data *data;
struct device *hwmon_dev;
+ struct device_node *child;
u32 shunt_resistor;
+ u32 reg;
+ u32 temp;
+ const char *mode;
+ unsigned int poe_plusval;
+ unsigned int mode_val;
+ unsigned int power_val;
+ unsigned int enable_val;
+ unsigned int disconnect_enable_val;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -577,6 +601,63 @@ static int tps23861_probe(struct i2c_client *client)
TPS23861_GENERAL_MASK_1,
TPS23861_CURRENT_SHUNT_MASK);
+ regmap_read(data->regmap, POE_PLUS, &poe_plusval);
+ regmap_read(data->regmap, POWER_ENABLE, &power_val);
+ regmap_read(data->regmap, OPERATING_MODE, &mode_val);
+ regmap_read(data->regmap, DETECT_CLASS_ENABLE, &enable_val);
+ regmap_read(data->regmap, DISCONNECT_ENABLE, &disconnect_enable_val);
+
+ for_each_child_of_node(dev->of_node, child) {
+ if (of_property_read_u32(child, "reg", ®))
+ continue;
+
+ if (reg > (TPS23861_NUM_PORTS - 1) || reg < 0)
+ continue;
+
+ if (!of_property_read_string(child, "mode", &mode)) {
+ if (!strncmp(mode, "manual", 6)) {
+ mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
+ mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_MANUAL, reg);
+ } else if (!strncmp(mode, "semiauto", 8)) {
+ mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
+ mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_SEMI, reg);
+ } else if (!strncmp(mode, "auto", 4))
+ mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
+ else
+ mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
+ }
+
+ if (!of_property_read_u32(child, "enable", &temp)) {
+ if (temp) {
+ enable_val |= DISCONNECT_MASK(reg);
+ disconnect_enable_val |= DISCONNECT_ENABLE_MASK(reg);
+ } else {
+ enable_val &= ~DISCONNECT_MASK(reg);
+ disconnect_enable_val &= ~DISCONNECT_ENABLE_MASK(reg);
+ }
+ }
+
+ if (!of_property_read_u32(child, "power", &temp)) {
+ if (temp)
+ power_val |= POWER_ENABLE_ON_MASK(reg);
+ else
+ power_val |= POWER_ENABLE_OFF_MASK(reg);
+ }
+
+ if (!of_property_read_u32(child, "poe_plus", &temp)) {
+ if (temp)
+ poe_plusval |= POE_PLUS_MASK(reg);
+ else
+ poe_plusval &= ~POE_PLUS_MASK(reg);
+ }
+ }
+
+ regmap_write(data->regmap, POE_PLUS, poe_plusval);
+ regmap_write(data->regmap, POWER_ENABLE, power_val);
+ regmap_write(data->regmap, OPERATING_MODE, mode_val);
+ regmap_write(data->regmap, DETECT_CLASS_ENABLE, enable_val);
+ regmap_write(data->regmap, DISCONNECT_ENABLE, disconnect_enable_val);
+
hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
data, &tps23861_chip_info,
NULL);
--
2.37.2
On 25/08/2022 17:10, Andreas Böhler wrote:
> The tps23861 driver does not initialize the chip and relies on it being
> in auto-mode by default. On some devices, these controllers default to
> OFF-Mode and hence cannot be used at all.
>
> This brings minimal support for initializing the controller in a user-
> defined mode.
>
> Tested on a TP-Link TL-SG2452P with 12x TI TPS23861 controllers.
>
> Signed-off-by: Andreas Böhler <[email protected]>
> ---
> .../bindings/hwmon/ti,tps23861.yaml | 76 +++++++++++++++++
Please run scripts/checkpatch.pl and fix the warnings.
Best regards,
Krzysztof
On Thu, Aug 25, 2022 at 04:10:42PM +0200, Andreas B?hler wrote:
> The tps23861 driver does not initialize the chip and relies on it being
> in auto-mode by default. On some devices, these controllers default to
> OFF-Mode and hence cannot be used at all.
>
> This brings minimal support for initializing the controller in a user-
> defined mode.
>
> Tested on a TP-Link TL-SG2452P with 12x TI TPS23861 controllers.
>
> Signed-off-by: Andreas B?hler <[email protected]>
> ---
> .../bindings/hwmon/ti,tps23861.yaml | 76 +++++++++++++++++
> drivers/hwmon/tps23861.c | 81 +++++++++++++++++++
> 2 files changed, 157 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> index 3bc8e73dfbf0..ed3a703478fb 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
Devicetree bindings need to be a separate patch.
> @@ -35,6 +35,50 @@ required:
> - compatible
> - reg
>
> +patternProperties:
> + "^port@([0-3])$":
> + type: object
> + description: Represents ports of the device and their specific configuration.
> +
> + properties:
> + reg:
> + description: The port number
> + items:
> + minimum: 0
> + maximum: 3
> +
> + mode:
> + description: The operating mode the device should be initialized with
> + items:
> + - enum:
> + - auto
> + - semiauto
> + - manual
> + - off
> +
> + enable:
> + description: Whether the port should be enabled
> + items:
> + minimum: 0
> + maximum: 1
> +
> + power:
> + description: Whether the port should be powered on
> + items:
> + minimum: 0
> + maximum: 1
> +
> + poe_plus:
> + description: Whether the port should support PoE+
> + items:
> + minimum: 0
> + maximum: 1
> +
The above properties are out of scope for a hardware monitoring driver.
A hardware monitoring driver should only affect hardware monitoring
functionality, not chip operation. Port control functionality should
be implemented in a phy driver.
Having said this, I notice that the driver's 'enable' attribute is
misused (abused). It should enable or disable port monitoring, not
port functionality. That attribute should be removed from the driver.
Guenter
> + required:
> + - reg
> +
> + additionalProperties: false
> +
> additionalProperties: false
>
> examples:
> @@ -47,5 +91,37 @@ examples:
> compatible = "ti,tps23861";
> reg = <0x30>;
> shunt-resistor-micro-ohms = <255000>;
> +
> + port@0 {
> + reg = <0>;
> + mode = "auto";
> + enable = <1>;
> + power = <1>;
> + poe_plus = <1>;
> + };
> +
> + port@1 {
> + reg = <1>;
> + mode = "semiauto";
> + enable = <1>;
> + power = <0>;
> + poe_plus = <1>;
> + };
> +
> + port@2 {
> + reg = <2>;
> + mode = "manual";
> + enable = <0>;
> + power = <0>;
> + poe_plus = <0>;
> + };
> +
> + port@3 {
> + reg = <3>;
> + mode = "off";
> + enable = <0>;
> + power = <0>;
> + poe_plus = <1>;
> + };
> };
> };
> diff --git a/drivers/hwmon/tps23861.c b/drivers/hwmon/tps23861.c
> index 42762e87b014..27bf8716cf12 100644
> --- a/drivers/hwmon/tps23861.c
> +++ b/drivers/hwmon/tps23861.c
> @@ -85,6 +85,8 @@
> #define PORT_DETECT_CAPACITANCE_INVALID_DELTA 11
> #define PORT_DETECT_CAPACITANCE_OUT_OF_RANGE 12
> #define POE_PLUS 0x40
> +#define POE_PLUS_MASK(_port) \
> + GENMASK(_port + 4, _port + 4)
> #define OPERATING_MODE 0x12
> #define OPERATING_MODE_OFF 0
> #define OPERATING_MODE_MANUAL 1
> @@ -94,9 +96,22 @@
> #define OPERATING_MODE_PORT_2_MASK GENMASK(3, 2)
> #define OPERATING_MODE_PORT_3_MASK GENMASK(5, 4)
> #define OPERATING_MODE_PORT_4_MASK GENMASK(7, 6)
> +#define OPERATING_MODE_PORT(_mode, _port) \
> + (_mode << (_port * 2))
>
> +#define DISCONNECT_ENABLE 0x13
> +#define DISCONNECT_ENABLE_MASK(_port) \
> + GENMASK(_port, _port)
> +#define DISCONNECT_MASK(_port) \
> + (GENMASK(_port, _port) | GENMASK(_port + 4, _port + 4))
> +
> +#define DETECT_CLASS_ENABLE 0x14
> #define DETECT_CLASS_RESTART 0x18
> #define POWER_ENABLE 0x19
> +#define POWER_ENABLE_ON_MASK(_port) \
> + GENMASK(_port, _port)
> +#define POWER_ENABLE_OFF_MASK(_port) \
> + GENMASK(_port + 4, _port + 4)
> #define TPS23861_NUM_PORTS 4
>
> #define TPS23861_GENERAL_MASK_1 0x17
> @@ -548,7 +563,16 @@ static int tps23861_probe(struct i2c_client *client)
> struct device *dev = &client->dev;
> struct tps23861_data *data;
> struct device *hwmon_dev;
> + struct device_node *child;
> u32 shunt_resistor;
> + u32 reg;
> + u32 temp;
> + const char *mode;
> + unsigned int poe_plusval;
> + unsigned int mode_val;
> + unsigned int power_val;
> + unsigned int enable_val;
> + unsigned int disconnect_enable_val;
>
> data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> if (!data)
> @@ -577,6 +601,63 @@ static int tps23861_probe(struct i2c_client *client)
> TPS23861_GENERAL_MASK_1,
> TPS23861_CURRENT_SHUNT_MASK);
>
> + regmap_read(data->regmap, POE_PLUS, &poe_plusval);
> + regmap_read(data->regmap, POWER_ENABLE, &power_val);
> + regmap_read(data->regmap, OPERATING_MODE, &mode_val);
> + regmap_read(data->regmap, DETECT_CLASS_ENABLE, &enable_val);
> + regmap_read(data->regmap, DISCONNECT_ENABLE, &disconnect_enable_val);
> +
> + for_each_child_of_node(dev->of_node, child) {
> + if (of_property_read_u32(child, "reg", ®))
> + continue;
> +
> + if (reg > (TPS23861_NUM_PORTS - 1) || reg < 0)
> + continue;
> +
> + if (!of_property_read_string(child, "mode", &mode)) {
> + if (!strncmp(mode, "manual", 6)) {
> + mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
> + mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_MANUAL, reg);
> + } else if (!strncmp(mode, "semiauto", 8)) {
> + mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
> + mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_SEMI, reg);
> + } else if (!strncmp(mode, "auto", 4))
> + mode_val |= OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
> + else
> + mode_val &= ~OPERATING_MODE_PORT(OPERATING_MODE_AUTO, reg);
> + }
> +
> + if (!of_property_read_u32(child, "enable", &temp)) {
> + if (temp) {
> + enable_val |= DISCONNECT_MASK(reg);
> + disconnect_enable_val |= DISCONNECT_ENABLE_MASK(reg);
> + } else {
> + enable_val &= ~DISCONNECT_MASK(reg);
> + disconnect_enable_val &= ~DISCONNECT_ENABLE_MASK(reg);
> + }
> + }
> +
> + if (!of_property_read_u32(child, "power", &temp)) {
> + if (temp)
> + power_val |= POWER_ENABLE_ON_MASK(reg);
> + else
> + power_val |= POWER_ENABLE_OFF_MASK(reg);
> + }
> +
> + if (!of_property_read_u32(child, "poe_plus", &temp)) {
> + if (temp)
> + poe_plusval |= POE_PLUS_MASK(reg);
> + else
> + poe_plusval &= ~POE_PLUS_MASK(reg);
> + }
> + }
> +
> + regmap_write(data->regmap, POE_PLUS, poe_plusval);
> + regmap_write(data->regmap, POWER_ENABLE, power_val);
> + regmap_write(data->regmap, OPERATING_MODE, mode_val);
> + regmap_write(data->regmap, DETECT_CLASS_ENABLE, enable_val);
> + regmap_write(data->regmap, DISCONNECT_ENABLE, disconnect_enable_val);
> +
> hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
> data, &tps23861_chip_info,
> NULL);
> --
> 2.37.2
>
On Thu, 25 Aug 2022 16:10:42 +0200, Andreas Böhler wrote:
> The tps23861 driver does not initialize the chip and relies on it being
> in auto-mode by default. On some devices, these controllers default to
> OFF-Mode and hence cannot be used at all.
>
> This brings minimal support for initializing the controller in a user-
> defined mode.
>
> Tested on a TP-Link TL-SG2452P with 12x TI TPS23861 controllers.
>
> Signed-off-by: Andreas Böhler <[email protected]>
> ---
> .../bindings/hwmon/ti,tps23861.yaml | 76 +++++++++++++++++
> drivers/hwmon/tps23861.c | 81 +++++++++++++++++++
> 2 files changed, 157 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:28.23-33: Warning (reg_format): /example-0/i2c/tps23861@30/port@0:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:36.23-33: Warning (reg_format): /example-0/i2c/tps23861@30/port@1:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:44.23-33: Warning (reg_format): /example-0/i2c/tps23861@30/port@2:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:52.23-33: Warning (reg_format): /example-0/i2c/tps23861@30/port@3:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:27.26-33.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@0: Relying on default #address-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:27.26-33.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@0: Relying on default #size-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:35.26-41.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@1: Relying on default #address-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:35.26-41.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@1: Relying on default #size-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:43.26-49.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@2: Relying on default #address-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:43.26-49.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@2: Relying on default #size-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:51.26-57.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@3: Relying on default #address-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dts:51.26-57.21: Warning (avoid_default_addr_size): /example-0/i2c/tps23861@30/port@3: Relying on default #size-cells value
Documentation/devicetree/bindings/hwmon/ti,tps23861.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.