Dear Igor Grinberg,
Hi Lucas, Tom,
I'm sorry for the late reply. I understand, that Tom has already applied this to tegra/next, but as the changes/follow up patches are required, may be we can do this in another fashion...
- Thanks for the patch and working on extending the generic framework!
- This patch has no dependencies on tegra specific patches, so I think, it should go through Marex usb tree, but doing this will require the right merge order, so bisectability will not suffer. So, Marek, Tom, you should decide which way is fine with you both.
_ALWAYS_ CC the right custodians. That is, me. Seeing this patch bypassed me completely, I'm really unhappy.
Tom, Yesterday, I was wondering if the patch was already applied, and I had no clue what's its status. Also, the patchwork says "New". So, if it is not hard for you in the future, I'd like a short reply to the list, saying something like: "Applied, thanks.", like most custodians do. Thanks!
+1
On 08/21/12 23:18, 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
After the below comments are fixed: Acked-by: Igor Grinberg grinberg@compulab.co.il
drivers/usb/ulpi/ulpi.c | 26 ++++++++++++++++++++++---- include/usb/ulpi.h | 13 +++++++++++-- 2 Dateien geändert, 33 Zeilen hinzugefügt(+), 6 Zeilen entfernt(-)
diff --git a/drivers/usb/ulpi/ulpi.c b/drivers/usb/ulpi/ulpi.c index dde2585..f358bde 100644 --- a/drivers/usb/ulpi/ulpi.c +++ b/drivers/usb/ulpi/ulpi.c @@ -106,20 +106,38 @@ 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)+{
- u8 *reg;
- int ret;
- reg = external ? &ulpi->otg_ctrl_set : &ulpi->otg_ctrl_clear;
- if ((ret = ulpi_write(ulpi_vp, reg, ULPI_OTG_EXTVBUSIND)))
return ret;- reg = passthu ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear;
- if ((ret = ulpi_write(ulpi_vp, reg, ULPI_IFACE_PASSTHRU)))
return ret;- reg = complement ? &ulpi->iface_ctrl_set : &ulpi->iface_ctrl_clear;
- if ((ret = ulpi_write(ulpi_vp, reg, ULPI_IFACE_EXTVBUS_COMPLEMENT)))
return ret;These are fine, two requests though:
- As Tom already pointed in the private email:
ERROR: do not use assignment in if condition #361: FILE: drivers/usb/ulpi/ulpi.c:127:
- if ((ret = ulpi_write(ulpi_vp, reg, ULPI_OTG_EXTVBUSIND)))
ERROR: do not use assignment in if condition #365: FILE: drivers/usb/ulpi/ulpi.c:131:
- if ((ret = ulpi_write(ulpi_vp, reg, ULPI_IFACE_PASSTHRU)))
ERROR: do not use assignment in if condition #369: FILE: drivers/usb/ulpi/ulpi.c:135:
- if ((ret = ulpi_write(ulpi_vp, reg, ULPI_IFACE_EXTVBUS_COMPLEMENT)))
those must be fixed.
Agreed, _ALWAYS_ run checkpatch.pl before submitting. It's even better idea to add a git precommit hook for that.
- Can you make only one access for each register? Use flags/val variable (like in other places) and do only one access per register. Can you?
- 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.