Am Freitag, den 28.09.2012, 10:15 +0200 schrieb Igor Grinberg:
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.
I tend to disagree. The ULPI PHY register set was specifically designed to not need this use-modify-write dance. Why would we like to ignore this?
Yes we are possible doing one unneeded register access here, but what would it buy us to ignore the set/clear registers just to avoid one register access?
- 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.