From mboxrd@z Thu Jan  1 00:00:00 1970
From: Igor Grinberg <grinberg@compulab.co.il>
Date: Fri, 28 Sep 2012 10:15:28 +0200
Subject: [U-Boot] [PATCH v3 1/2] usb: ulpi: add indicator configuration
	function
In-Reply-To: <1348612532-24419-2-git-send-email-dev@lynxeye.de>
References: <1348612532-24419-1-git-send-email-dev@lynxeye.de>
	<1348612532-24419-2-git-send-email-dev@lynxeye.de>
Message-ID: <50655CA0.2000005@compulab.co.il>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 09/26/12 00:35, Lucas Stach wrote:
> Allows for easy configuration of the VBUS indicator related ULPI
> config bits.
> 
> Also move the external indicator setup from ulpi_set_vbus() to
> the new function.
> 
> Signed-off-by: Lucas Stach <dev@lynxeye.de>
> Acked-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
> v3: Only touch each register once. Now checkpatch clean.
> ---
>  drivers/usb/ulpi/ulpi.c | 32 ++++++++++++++++++++++++++++----
>  include/usb/ulpi.h      | 13 +++++++++++--
>  2 Dateien ge?ndert, 39 Zeilen hinzugef?gt(+), 6 Zeilen entfernt(-)
> 
> diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c
> index dde2585..98dd23c 100644
> --- a/drivers/usb/ulpi/ulpi.c
> +++ b/drivers/usb/ulpi/ulpi.c
> @@ -106,20 +106,44 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed)
>  	return ulpi_write(ulpi_vp, &ulpi->function_ctrl, val);
>  }
>  
> -int ulpi_set_vbus(struct ulpi_viewport *ulpi_vp, int on, int ext_power,
> -			int ext_ind)
> +int ulpi_set_vbus(struct ulpi_viewport *ulpi_vp, int on, int ext_power)
>  {
>  	u32 flags = ULPI_OTG_DRVVBUS;
>  	u8 *reg = on ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
>  
>  	if (ext_power)
>  		flags |= ULPI_OTG_DRVVBUS_EXT;
> -	if (ext_ind)
> -		flags |= ULPI_OTG_EXTVBUSIND;
>  
>  	return ulpi_write(ulpi_vp, reg, flags);
>  }
>  
> +int ulpi_set_vbus_indicator(struct ulpi_viewport *ulpi_vp, int external,
> +			int passthu, int complement)
> +{
> +	u32 flags;
> +	int ret;
> +
> +	ret = ulpi_write(ulpi_vp,
> +			external ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear,
> +			ULPI_OTG_EXTVBUSIND);

I think the below would be clearer and also will look as the rest of the file does:

reg = external ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
ret = ulpi_write(ulpi_vp, reg, ULPI_OTG_EXTVBUSIND);

> +	if (ret)
> +		return ret;
> +
> +	flags = passthu ? ULPI_IFACE_PASSTHRU : 0;
> +	flags |= complement ? ULPI_IFACE_EXTVBUS_COMPLEMENT : 0;
> +	ret = ulpi_write(ulpi_vp, &ulpi->iface_ctrl_set, flags);
> +	if (ret)
> +		return ret;
> +
> +	flags = passthu ? 0 : ULPI_IFACE_PASSTHRU;
> +	flags |= complement ? 0 : ULPI_IFACE_EXTVBUS_COMPLEMENT;
> +	ret = ulpi_write(ulpi_vp, &ulpi->iface_ctrl_clear, flags);

Errr..., that is not what I meant... sorry for confusion.
What I meant is something like:

u32 pthrough = passthu ? ULPI_IFACE_PASSTHRU : 0;
u32 extcompl |= complement ? ULPI_IFACE_EXTVBUS_COMPLEMENT : 0;

val = ulpi_read(ulpi_vp, &ulpi->iface_ctrl);
if (val == ULPI_ERROR)
	return val;

val = (val & ~ULPI_IFACE_PASSTHRU) | pthrough;
val = (val & ~ULPI_IFACE_EXTVBUS_COMPLEMENT) | extcompl;
ret = ulpi_write(ulpi_vp, &ulpi->iface_ctrl, val);

That way you write only once to each register and the code also look uniform.

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  int ulpi_set_pd(struct ulpi_viewport *ulpi_vp, int enable)
>  {
>  	u32 val = ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN;
> diff --git a/include/usb/ulpi.h b/include/usb/ulpi.h
> index 9a75c24..99166c4 100644
> --- a/include/usb/ulpi.h
> +++ b/include/usb/ulpi.h
> @@ -61,8 +61,17 @@ int ulpi_select_transceiver(struct ulpi_viewport *ulpi_vp, unsigned speed);
>   *
>   * returns 0 on success, ULPI_ERROR on failure.
>   */
> -int ulpi_set_vbus(struct ulpi_viewport *ulpi_vp,
> -			int on, int ext_power, int ext_ind);
> +int ulpi_set_vbus(struct ulpi_viewport *ulpi_vp, int on, int ext_power);
> +
> +/*
> + * Configure VBUS indicator
> + * @external		- external VBUS over-current indicator is used
> + * @passthru		- disables ANDing of internal VBUS comparator
> + *                    with external VBUS input
> + * @complement		- inverts the external VBUS input
> + */
> +int ulpi_set_vbus_indicator(struct ulpi_viewport *ulpi_vp, int external,
> +			int passthru, int complement);
>  
>  /*
>   * Enable/disable pull-down resistors on D+ and D- USB lines.

-- 
Regards,
Igor.