Re: [PATCH 3/4] usb: usb251xb: simplify reset helper

[Richard Leitner <richard.leitner@xxxxxxxxxxx>]

On 17/09/2019 16:44, Marco Felsch wrote:
> Currently the reset handler was always called to deassert the reset
> line because assert the line was done during probe. Now if we want to
> support pm by turn of the supply we need to call this routine twice and
> the i2c_lock_bus is done twice too. To simplify that we can drop the
> state and just do a reset in one go. So a future pm operation don't need
> to lock the i2c bus twice.
>
> Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx>

Hi,
tested it on a i.MX6 based platform with an usb2512.
Therefore please feel free to add:

Reviewed-by: Richard Leitner <richard.leitner@xxxxxxxxxxx>

and/or

Tested-by: Richard Leitner <richard.leitner@xxxxxxxxxxx>

regards;Richard.L

> ---
>   drivers/usb/misc/usb251xb.c | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/misc/usb251xb.c b/drivers/usb/misc/usb251xb.c
> index 05819167604d..bc031d33f433 100644
> --- a/drivers/usb/misc/usb251xb.c
> +++ b/drivers/usb/misc/usb251xb.c
> @@ -263,20 +263,19 @@ static int usb251x_check_gpio_chip(struct usb251xb *hub)
>   }
>   #endif
>   
> -static void usb251xb_reset(struct usb251xb *hub, int state)
> +static void usb251xb_reset(struct usb251xb *hub)
>   {
>   	if (!hub->gpio_reset)
>   		return;
>   
>   	i2c_lock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT);
>   
> -	gpiod_set_value_cansleep(hub->gpio_reset, state);
> +	gpiod_set_value_cansleep(hub->gpio_reset, 1);
> +	usleep_range(1, 10);	/* >=1us RESET_N asserted */
> +	gpiod_set_value_cansleep(hub->gpio_reset, 0);
>   
>   	/* wait for hub recovery/stabilization */
> -	if (!state)
> -		usleep_range(500, 750);	/* >=500us at power on */
> -	else
> -		usleep_range(1, 10);	/* >=1us at power down */
> +	usleep_range(500, 750);	/* >=500us after RESET_N deasserted */
>   
>   	i2c_unlock_bus(hub->i2c->adapter, I2C_LOCK_SEGMENT);
>   }
> @@ -294,7 +293,7 @@ static int usb251xb_connect(struct usb251xb *hub)
>   		i2c_wb[0] = 0x01;
>   		i2c_wb[1] = USB251XB_STATUS_COMMAND_ATTACH;
>   
> -		usb251xb_reset(hub, 0);
> +		usb251xb_reset(hub);
>   
>   		err = i2c_smbus_write_i2c_block_data(hub->i2c,
>   				USB251XB_ADDR_STATUS_COMMAND, 2, i2c_wb);
> @@ -344,7 +343,7 @@ static int usb251xb_connect(struct usb251xb *hub)
>   	i2c_wb[USB251XB_ADDR_PORT_MAP_7]        = hub->port_map7;
>   	i2c_wb[USB251XB_ADDR_STATUS_COMMAND] = USB251XB_STATUS_COMMAND_ATTACH;
>   
> -	usb251xb_reset(hub, 0);
> +	usb251xb_reset(hub);
>   
>   	/* write registers */
>   	for (i = 0; i < (USB251XB_I2C_REG_SZ / USB251XB_I2C_WRITE_SZ); i++) {