Re: [PATCH] hwmon: (tps23861) add class restrictions and semi-auto mode support

[Krzysztof Kozlowski <krzk@kernel.org>]

On 07/08/2025 23:50, Gregory Fuchedgi wrote:
> This commit adds optional individual per-port device tree nodes, in which ports
> can be:
>   - restricted by class. For example, class = <2> for a port would only enable
>     power if class 1 or class 2 were negotiated. Requires interrupt handler to
>     be configured if class != 4 (max supported). This is because hardware cannot
>     be set with acceptable classes in advance. So the driver would enable
>     Semi-Auto mode and in the interrupt handler would check negotiated class
>     against device tree config and enable power only if it is acceptable class.
>   - fully disabled. For boards that do not use all 4 ports. This would put
>     disabled ports in Off state and would hide corresponding hwmon files.
>   - off by default. The port is kept disabled, until enabled via corresponding
>     in_enable in sysfs.
> 

Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.
</form letter>


> The driver keeps current behavior of using Auto mode for all ports if no
> per-port device tree nodes given.
> 
> This commit also adds optional reset and shutdown pin support:
>   - if reset pin is configured the chip will be reset in probe.
>   - if both reset and shutdown pins are configured the driver would keep ports
>     shut down while configuring the tps23861 over i2c. Having both shutdown and
>     reset pins ensures no PoE activity happens while i2c configuration is
>     happening.
> 
> Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
> ---
>  .../bindings/hwmon/ti,tps23861.yaml           |  78 +++++-
>  Documentation/hwmon/tps23861.rst              |   6 +-
>  drivers/hwmon/tps23861.c                      | 253 +++++++++++++++++-
>  3 files changed, 325 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> index ee7de53e1918..328050656ab8 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> @@ -24,12 +24,34 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
>    shunt-resistor-micro-ohms:
>      description: The value of current sense resistor in microohms.
>      default: 255000
>      minimum: 250000
>      maximum: 255000
>  
> +  reset-gpios:
> +    description: Optional GPIO for the reset pin.
> +    maxItems: 1
> +
> +  shutdown-gpios:
> +    description: |
> +      Optional GPIO for the shutdown pin. Used to prevent PoE activity before
> +      the driver had a chance to configure the chip.
> +    maxItems: 1
> +
> +  interrupts:
> +    description: |
> +      The interrupt specifier. Only required if PoE class is restricted to less
> +      than class 4 in the device tree.
> +    maxItems: 1
> +
>  required:
>    - compatible
>    - reg
> @@ -37,7 +59,27 @@ required:
>  allOf:
>    - $ref: hwmon-common.yaml#
>  
> -unevaluatedProperties: false
> +additionalProperties:
> +  type: object
> +  description: Port specific nodes.

Yuo should rather use patternProperties.

...

		IRQF_TRIGGER_FALLING,
> +						"tps23861_irq", data);
> +		if (ret) {
> +			dev_err(dev, "failed to request irq %d\n", data->irq);
> +			return ret;
> +		}
> +		dev_info(dev, "irq successfully requested\n");

Not useful. Drop.



Best regards,
Krzysztof