Re: [PATCH net-next 2/2] net: dsa: microchip: configure strap pins during reset

[Bastien Curutchet <bastien.curutchet@bootlin.com>]

Hi Miquèl,

On 9/10/25 5:38 PM, Miquel Raynal wrote:
> Hello bastien,
> 
>> +static int ksz_configure_strap(struct ksz_device *dev)
>> +{
>> +	struct pinctrl_state *state = NULL;
>> +	struct pinctrl *pinctrl;
>> +	int ret;
>> +
>> +	if (of_device_is_compatible(dev->dev->of_node, "microchip,ksz8463")) {
>> +		struct gpio_desc *rxd0;
>> +		struct gpio_desc *rxd1;
>> +
>> +		rxd0 = devm_gpiod_get_index_optional(dev->dev, "strap", 0, GPIOD_OUT_LOW);
>> +		if (IS_ERR(rxd0))
>> +			return PTR_ERR(rxd0);
>> +
>> +		rxd1 = devm_gpiod_get_index_optional(dev->dev, "strap", 1, GPIOD_OUT_HIGH);
>> +		if (IS_ERR(rxd1))
>> +			return PTR_ERR(rxd1);
>> +
>> +		/* If at least one strap definition is missing we don't do anything */
>> +		if (!rxd0 || !rxd1)
>> +			return 0;
>> +
>> +		pinctrl = devm_pinctrl_get(dev->dev);
>> +		if (IS_ERR(pinctrl))
>> +			return PTR_ERR(pinctrl);
>> +
>> +		state = pinctrl_lookup_state(pinctrl, "reset");
>> +		if (IS_ERR(state))
>> +			return PTR_ERR(state);
>> +
>> +		ret = pinctrl_select_state(pinctrl, state);
>> +		if (ret)
>> +			return ret;
> 
> In order to simplify the pinctrl handling I would propose to replace
> these three function calls by:
> 
>        devm_pinctrl_get_select(dev->dev, "reset")
> 
> I do not think in this case we actually require the internal
> devm_pinctrl_put() calls from the above helper, but they probably do not
> hurt either.
> 

True, thank you.


Best regards,
Bastien