lkml.org 
[lkml]   [2016]   [May]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] usb: ohci-at91: Suspend the ports while USB suspending
Date
Hi Alan,

> -----Original Message-----
> From: Alan Stern [mailto:stern@rowland.harvard.edu]
> Sent: 2016年5月13日 2:11
> To: Yang, Wenyou <Wenyou.Yang@atmel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Ferre, Nicolas
> <Nicolas.FERRE@atmel.com>; linux-usb@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH] usb: ohci-at91: Suspend the ports while USB suspending
>
> On Thu, 12 May 2016, Wenyou Yang wrote:
>
> > In order to get lower consumption, as a workaround, suspend the USB
> > PORTA/B/C via set the SUSPEND_A/B/C bits of OHCI Interrupt
> > Configuration Register while OHCI USB suspending.
>
> What does this mean? What does suspending a port do?

It is a IP workaround for SAMA5D2 USB. By setting corresponding bits of a specific register to get lower consumption.

> Is it the same as a normal USB port suspend?

No, it is not same.

>
> If it is the same, why doesn't the USB_PORT_FEAT_SUSPEND subcase of the
> SetPortFeature case in ohci_hub_control() already take care of this?
>
> > This suspend operation must be done before stopping the USB clock,
> > resume after the USB clock enabled.
> >
> > Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> > ---
>
> > @@ -132,6 +135,17 @@ static void at91_stop_hc(struct platform_device
> > *pdev)
> >
> >
> > /*--------------------------------------------------------------------
> > -----*/
> >
> > +struct regmap *at91_dt_syscon_sfr(void) {
> > + struct regmap *regmap;
> > +
> > + regmap = syscon_regmap_lookup_by_compatible("atmel,sama5d2-sfr");
> > + if (IS_ERR(regmap))
> > + return NULL;
>
> If you get an error, the regmap pointer is set to NULL...
>
> > @@ -197,6 +211,8 @@ static int usb_hcd_at91_probe(const struct hc_driver
> *driver,
> > goto err;
> > }
> >
> > + ohci_at91->sfr_regmap = at91_dt_syscon_sfr();
>
> With no other error checking...

Add error checking in next version.

>
> > +
> > board = hcd->self.controller->platform_data;
> > ohci = hcd_to_ohci(hcd);
> > ohci->num_ports = board->ports;
>
> > +static int ohci_at91_port_ctrl(struct regmap *regmap, bool enable) {
> > + u32 regval;
> > + int ret;
> > +
> > + if (IS_ERR(regmap))
> > + return PTR_ERR(regmap);
> > +
> > + ret = regmap_read(regmap, SFR_OHCIICR, &regval);
>
> And now what happens if regmap is NULL? Hint: It won't be pretty...

Yes, it is not pretty. Will rework in next version.

>
> Alan Stern


Best Regards,
Wenyou Yang

\
 
 \ /
  Last update: 2016-05-13 04:01    [W:0.117 / U:0.680 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site