On 19-10-02 22:34:21, Dmitry Osipenko wrote:
> 02.10.2019 15:15, Peter Geis пишет:
> > On Wed, Oct 2, 2019 at 7:35 AM Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
> >>
> >> On Tue, Oct 01, 2019 at 09:41:53PM -0400, Peter Geis wrote:
> >>> Add the functions to the chipidea host driver to enable tegra specific
> >>> dma alignment and reset handlers.
> >>>
> >>> Signed-off-by: Peter Geis <pgwipeout@xxxxxxxxx>
> >>> ---
> >>> drivers/usb/chipidea/ci_hdrc_tegra.c | 7 +++++++
> >>> drivers/usb/chipidea/host.c | 13 +++++++++++++
> >>> 2 files changed, 20 insertions(+)
> >>>
> >>> diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c
> >>> index 29415c3a2f48..2f7d542d2273 100644
> >>> --- a/drivers/usb/chipidea/ci_hdrc_tegra.c
> >>> +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
> >>> @@ -118,6 +118,13 @@ static int tegra_udc_probe(struct platform_device *pdev)
> >>> udc->data.usb_phy = udc->phy;
> >>> udc->data.capoffset = DEF_CAPOFFSET;
> >>>
> >>> + /* check the double reset flag */
> >>> + if (of_property_read_bool(pdev->dev.of_node,
> >>> + "nvidia,needs-double-reset")) {
> >>> + dev_dbg(&pdev->dev, "setting double reset flag\n");
> >>> + udc->data.flags |= CI_HDRC_TEGRA_DOUBLE_RESET;
> >>> + }
> >>
> >> Like I said, I think it'd be better to put this into the same patch that
> >> adds the flag.
> >>
> >>> +
> >>> udc->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> >>> pdev->num_resources, &udc->data);
> >>> if (IS_ERR(udc->dev)) {
> >>> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> >>> index b45ceb91c735..e95b7cb8c54d 100644
> >>> --- a/drivers/usb/chipidea/host.c
> >>> +++ b/drivers/usb/chipidea/host.c
> >>> @@ -20,6 +20,7 @@
> >>> #include "ci.h"
> >>> #include "bits.h"
> >>> #include "host.h"
> >>> +#include "tegra.h"
> >>>
> >>> static struct hc_driver __read_mostly ci_ehci_hc_driver;
> >>> static int (*orig_bus_suspend)(struct usb_hcd *hcd);
> >>> @@ -275,6 +276,13 @@ static int ci_ehci_hub_control(
> >>> goto done;
> >>> }
> >>>
> >>> + /* For Tegra USB1 port we need to issue Port Reset twice internally */
> >>> + if (ci->platdata->flags & CI_HDRC_TEGRA_DOUBLE_RESET &&
> >>> + (typeReq == SetPortFeature && wValue == USB_PORT_FEAT_RESET)) {
> >>> + spin_unlock_irqrestore(&ehci->lock, flags);
> >>> + return tegra_ehci_internal_port_reset(ehci, status_reg);
> >>> + }
> >>> +
> >>> /*
> >>> * After resume has finished, it needs do some post resume
> >>> * operation for some SoCs.
> >>> @@ -364,6 +372,11 @@ int ci_hdrc_host_init(struct ci_hdrc *ci)
> >>> rdrv->name = "host";
> >>> ci->roles[CI_ROLE_HOST] = rdrv;
> >>>
> >>> + if (ci->platdata->flags & CI_HDRC_TEGRA_HOST) {
> >>> + ci_ehci_hc_driver.map_urb_for_dma = tegra_ehci_map_urb_for_dma;
> >>> + ci_ehci_hc_driver.unmap_urb_for_dma = tegra_ehci_unmap_urb_for_dma;
> >>> + }
> >>
> >> Same here.
> >>
> >> That said, there are a few other bits in ehci-tegra.c that we may need.
> >> For example, the tegra_ehci_reset() function sets different values for
> >> the TX FIFO threshold, which we don't do for ChipIdea as far as I can
> >> tell. We also differentiate between Tegra20 and later generations with
> >> respect to whether or not they have the HOSTPC registers.
> >>
> >> tegra_ehci_hub_control() also seems to have a number of other work-
> >> arounds that are not yet ported as part of this series. And then there
> >> is the matter of tegra_reset_usb_controller(). I recall that this has
> >> caused severe headaches in the past, so we need to be very careful when
> >> changing to the ChipIdea driver that we don't reintroduce old bugs
> >> again.
> >>
> >> Thierry
> >
> > I saw the patch around Tegra20's FIFO pipeline, I have a Tegra20
> > device to test on so I'll look if that's still necessary.
> > The tegra_ehci driver appeared to implement only what was necessary to
> > make the controller work, as there's a lot of overlap with the
> > chipidea driver.
> > Since the tegra-udc driver worked with very little code, I figured the
> > chipidea driver handled most everything correctly already.
> >
> > As such I looked mostly at the workarounds that were tegra specific.
> >
> > This is also why I requested multiple eyes, as I don't have the
> > benefit of historical context beyond the commit messages.
>
> Ha! Host port works on my Tegra20 using CI driver and this series! At least mouse,
> keyboard and WiFi dongle are working fine. Well done, looking forward to v2 :) We are
> getting closer to fold tegra-ehci and move to the CI driver uniformly.
Great, would you please CC linux-usb ML for v2?
--
Thanks,
Peter Chen
