Re: [PATCH net-next 2/2] net: pse-pd: pd692x0: support disabling disable ports GPIO

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver requests the 'disable-ports' GPIO with `GPIOD_OUT_HIGH`, which logically asserts the disable signal, contradicting the goal of enabling PoE.
--

commit bef0e9b8a1213b027391963a1e52c45fae73be60
Author: Robert Marko <robert.marko@sartura.hr>

net: pse-pd: pd692x0: support disabling disable ports GPIO

This commit adds support for parsing a disable-ports GPIO in the
Microchip PSE controller driver. It attempts to set this GPIO to output
high by default to enable PoE on all ports during probe.

> diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
> --- a/drivers/net/pse-pd/pd692x0.c
> +++ b/drivers/net/pse-pd/pd692x0.c

[ ... ]

> @@ -1780,6 +1782,11 @@ static int pd692x0_i2c_probe(struct i2c_client *client)
>  	priv->client = client;
>  	i2c_set_clientdata(client, priv);
>  
> +	disable_ports = devm_gpiod_get_optional(dev, "disable-ports", GPIOD_OUT_HIGH);

Will requesting this GPIO with GPIOD_OUT_HIGH actually disable the ports
instead of enabling them?

The gpiod API operates on logical states rather than physical voltage levels.
By requesting a GPIO named disable-ports with GPIOD_OUT_HIGH, the disable
signal is logically asserted.

This activates the disable condition and turns off PoE on the ports.

To de-assert the disable signal and enable PoE as intended, should this be
requested with GPIOD_OUT_LOW instead?

> +	if (IS_ERR(disable_ports))
> +		return dev_err_probe(&client->dev, PTR_ERR(disable_ports),
> +				     "Failed to get disable ports GPIO\n");
> +
>  	ret = i2c_master_recv(client, (u8 *)&buf, sizeof(buf));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507104720.262641-1-robert.marko@sartura.hr?part=2