On 01/10/11 16:22, Arnaud Patard (Rtp) wrote: > Igor Grinberg <grinberg@xxxxxxxxxxxxxx> writes: >> Can you, please, send patches inline and not as attachments next time? >> It is very inconvenient to review and comment on attached patches. > This patch a quick rewrite&test. It was more meant to understand if the > direction was right rather than a formal submission. (I use quilt to > send patches and not my mailer). There are some comment in the code > needed and I've not put them. Well, I also have got all other patches in the series as attachments... Why not use git send-email? >> On 01/07/11 11:02, Arnaud Patard (Rtp) wrote: >>> What about the following patch ? >>> ulpi: provide start_srp >>> >>> Add support for setting CHRGVBUS thanks to start_srp and use it >>> to workaround a hardware bug on efika mx/sb boards. >>> See http://lists.infradead.org/pipermail/linux-arm-kernel/2011-January/037341.html >>> >>> Signed-off-by: Arnaud Patard <arnaud.patard@xxxxxxxxxxx> >>> Index: linux-2.6-submit/drivers/usb/otg/ulpi.c >>> =================================================================== >>> --- linux-2.6-submit.orig/drivers/usb/otg/ulpi.c 2011-01-04 17:12:26.000000000 +0100 >>> +++ linux-2.6-submit/drivers/usb/otg/ulpi.c 2011-01-06 14:44:50.000000000 +0100 >>> @@ -247,6 +247,14 @@ >>> return otg_io_write(otg, flags, ULPI_OTG_CTRL); >>> } >>> >>> +static int ulpi_start_srp(struct otg_transceiver *otg, bool on) >>> +{ >>> + unsigned int flags = otg_io_read(otg, ULPI_OTG_CTRL); >>> + >>> + flags |= ULPI_OTG_CTRL_CHRGVBUS; >>> + return otg_io_write(otg, flags, ULPI_OTG_CTRL); >>> +} >>> + >> I went through the OTG spec. and found that there are much more things >> should be done for the start_srp() function to be implemented properly. >> For example, it says that we should make sure that the Vbus is discharged >> (means, the previous session has finished) before attempting to start a new >> session, but that is only a half of the problem. The real problem for you, is that >> the duration of the SRP should be no more then 100ms. > oops. That's really annoying. (fwiw, I think that original code from fsl > doesn't take care of that 100ms delay otherwise I guess I would have > detected it too). >>> struct otg_transceiver * >>> otg_ulpi_create(struct otg_io_access_ops *ops, >>> unsigned int flags) >>> @@ -263,6 +271,7 @@ >>> otg->init = ulpi_init; >>> otg->set_host = ulpi_set_host; >>> otg->set_vbus = ulpi_set_vbus; >>> + otg->start_srp = ulpi_start_srp; >>> >>> return otg; >>> } >>> Index: linux-2.6-submit/drivers/usb/host/ehci-mxc.c >>> =================================================================== >>> --- linux-2.6-submit.orig/drivers/usb/host/ehci-mxc.c 2011-01-06 14:45:14.000000000 +0100 >>> +++ linux-2.6-submit/drivers/usb/host/ehci-mxc.c 2011-01-06 15:48:16.000000000 +0100 >>> @@ -25,6 +25,8 @@ >>> >>> #include <mach/mxc_ehci.h> >>> >>> +#include <asm/mach-types.h> >>> + >>> #define ULPI_VIEWPORT_OFFSET 0x170 >>> >>> struct ehci_mxc_priv { >>> @@ -239,6 +241,13 @@ >>> dev_err(dev, "unable to enable vbus on transceiver\n"); >>> goto err_add; >>> } >>> + if (machine_is_mx51_efikamx() || machine_is_mx51_efikasb()) { >>> + ret = otg_start_srp(pdata->otg); >>> + if (ret) { >>> + dev_err(dev, "unable to start srp\n"); >>> + goto err_add; >>> + } >>> + } >> In light of the problem described above, here you will need to write directly >> to the view port instead of calling the otg_start_srp() function. > ok. I don't think it's a problem. Looks otg_io_(read|write) are > available here. yeah, otg_io_(read|write) should do. >> I'd recommend to move this to the machine (board) specific code. >> May be to platform init (pdata->init) call back? > oh, no. I've put this call in this place for some good reasons. This > trick is working only if it's done _after_ add_hcd. Any other place > won't work. For instance, pdata->init is meant to be called before > mxc_initialise_usb_hw iirc which is _before_ add_hcd. I see. The reason, I'd like to move it to the board code is a possibility that h/w will change (hopefully even fixed) and then you will not have to violate the spec anymore, but to deal with the h/w revision checking. ehci-mxc.c doesn't look like a good place to deal with h/w revision checking of the board, so may be it is worth to introduce some kind of another platform hook, like pdata->late_init? I don't insist, it is up to you and Sascha to decide what's the best choice here. >> IMHO, the explanation of the h/w bug workaround in the commit message >> is not enough - in-code comment regarding the bug workaround should be added. > I know. As I said this patch was not the final submission. It was > probably not clear enough. > > Arnaud -- Regards, Igor. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
