Re: [PATCH v13 2/2] usb: typec: ucsi: add support for Cypress CCGx
On Tue, Oct 23, 2018 at 06:56:59PM +, Ajay Gupta wrote:
> > On Wed, Oct 03, 2018 at 11:27:28AM -0700, Ajay Gupta wrote:
> > > Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller over I2C
> > > interface.
> > >
> > > This UCSI I2C driver uses I2C bus driver interface for communicating
> > > with Type-C controller.
> > > + /*
> > > + * Flush CCGx RESPONSE queue by acking interrupts. Above ucsi
> > control
> > > + * register write will push response which must be cleared.
> > > + */
> > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
> > > + if (status < 0)
> > > + return status;
(1)
> > > + do {
> > > + status = ccg_write(uc, CCGX_RAB_INTR_REG, &data,
> > sizeof(data));
> > > + if (status < 0)
> > > + return status;
(2)
> > > +
> > > + usleep_range(1, 11000);
> > > +
> > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, &data,
> > sizeof(data));
> > > + if (status < 0)
> > > + return status;
> > > + } while ((data != 0x00) && count--);
> >
> > What's the significance of that count?
> It is like a retry count to clear interrupt status.
>
> > Shouldn't you return -ETIMEDOUT if count == 0?
> Yes. Good catch. Does the below fix looks ok?
At least for me looks OK (I dunno why I missed that what Heikki found recently).
Nevertheless, I have one more question about (1) and (2) above.
Is it necessary to do one more read before do-while loop?
>
> do {
> status = ccg_write(uc, CCGX_RAB_INTR_REG, &data,
> sizeof(data));
> if (status < 0)
> return status;
>
> usleep_range(1, 11000);
>
> status = ccg_read(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
> if (status < 0)
> return status;
>
> if (!data)
> return 0;
> } while (data && count--);
>
> return -ETIMEDOUT;
--
With Best Regards,
Andy Shevchenko
Re: Query on usb/core/devio.c
On Thu, 18 Oct 2018 13:42:46 -0400 Alan Stern wrote: > On Thu, 18 Oct 2018, Mayuresh Kulkarni wrote: > > > > The only way to make the ioctl work properly is to have it do a > > > runtime-PM put at the start and then a runtime-PM get before it > > > returns. This is true regardless of the reason for returning: normal > > > termination, timeout, signal, whatever. Nothing else would be safe. > > > > > > > Will below steps work safely (sometimes pseudo-code/snippets help to > > align)? - > > > > "new" ioctl - > > > > timeout is parameter to ioctl. > > > > /* attempt suspend of device */ > > usb_autosuspend_device(dev); > > > > usb_unlock_device(dev); > > r = wait_event_interruptible_timeout(ps->resume_wait, > > (ps->resume_done == true), timeout * HZ); > > Not exactly. The condition to test here is whether the device has been > suspended, so it's more like this: > > r = wait_event_interruptible_timeout(ps->suspend_wait, > (ps->suspend_done == true), timeout * HZ); > > where ps->suspend_done is set by the runtime_suspend callback. After > this we will do: > > if (r > 0) /* Device suspended before the timeout expired */ > r = wait_event_interruptible(ps->resume_wait, > (ps->resume_done == true)); > > > usb_lock_device(dev); > > > > /* > > * There are 3 possibilities here: > > * 1. Device did suspend and resume (success) > > * 2. Signal was received (failed suspend) > > * 3. Time-out happened (failed suspend) > > 4. Device did suspend but a signal was received before the device > resumed. > > > * In any of above cases, we need to resume device. > > */ > > usb_autoresume_device(dev); > > > > ps->resume_done = false; > > > > "ps->resume_done = true;" will be done by the runtime resume call-back. > > Exactly. You got it. Will that let you accomplish what you want? > We spend time internally to go over the "new" ioctl proposal. Overall it looks promising. However, we still have an issue as below - Consider a use-case where, user-space calls "new" ioctl, but suspend never happen (for various reasons) && async event happens on USB device side by the end-user. In such a case, since user-space is waiting in "new" ioctl, it is not in position to queue a request to read-out the async event info. It will be able to queue a request when the "new" ioctl returns which will be "time-out" later (in this case). Due to auto-suspend time-out's default's value of 2 sec, the user-space has to choose the time-out to "new" ioctl > 2 sec. Does this use-case make sense? If yes, this probably is not a good UX since user did something and it took > 2 sec for the system to respond and take the appropriate action. Looking into finer details of this use-case, now. > Alan Stern
Re: Query on usb/core/devio.c
On Mon, 22 Oct 2018 10:24:46 -0400 Alan Stern wrote: > On Mon, 22 Oct 2018, Oliver Neukum wrote: > > > On Do, 2018-10-18 at 13:42 -0400, Alan Stern wrote: > > > On Thu, 18 Oct 2018, Mayuresh Kulkarni wrote: > > > > > > > > The only way to make the ioctl work properly is to have it do a > > > > > runtime-PM put at the start and then a runtime-PM get before it > > > > If and only if you want to do this with one ioctl() > > If you separate the runtime-PM put and the get, you can do it without > > the waiting part. > > Sure, but you still need a runtime-PM put at the start and a runtime-PM > get at the end. In fact, if you separate this into two calls then you > run the risk that the user may never perform the second call, and you > would end up with an unbalanced put. > I am tending to agree towards having a single ioctl call here. It is better to give minimal control to user-space w.r.t. runtime PM. The current proposal of user-space giving an hint to USB-FS/core that - it is not using the device, sounds better. > > > > /* > > > > * There are 3 possibilities here: > > > > * 1. Device did suspend and resume (success) > > > > * 2. Signal was received (failed suspend) > > > > * 3. Time-out happened (failed suspend) > > > > > > 4. Device did suspend but a signal was received before the device > > > resumed. > > > > > > > * In any of above cases, we need to resume device. > > > > */ > > > > usb_autoresume_device(dev); > > > > Yes and that is the problem. Why do you want to wait for the result > > of runtime-PM put ? If we need a channel for notifying user space > > about resume of a device, why wait for the result of suspend instead > > of using the same channel? > > This is not meant to be a general-purpose channel for notifying > userspace when a device resumes. Such a channel should be defined in > the runtime-PM layer, not in the USB layer. > > This is instead meant to be a special-purpose mechanism for adding a > runtime-suspend/resume interface to usbfs. > Just to be clear here - the worst case wait-time from user-space perspective will be + , right? If yes then, timeout argument means "wait at-least timeout sec". > > > > > > > > ps->resume_done = false; > > > > > > > > "ps->resume_done = true;" will be done by the runtime resume call-back. > > > > No. You cannot do that in this way. It needs to be a unified device > > state or a sequence of multiple suspends and resumes will have strange > > results. > > They won't, because such a sequence cannot occur. The ioctl thread > will wake up when the first resume occurs and it will do a runtime-PM > get, thus preventing any further suspends. > > Alan Stern
Re: Query on usb/core/devio.c
On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > On Mon, 22 Oct 2018 10:24:46 -0400 > Alan Stern wrote: > > > On Mon, 22 Oct 2018, Oliver Neukum wrote: > > > > > On Do, 2018-10-18 at 13:42 -0400, Alan Stern wrote: > > > > On Thu, 18 Oct 2018, Mayuresh Kulkarni wrote: > > > > > > > > > > The only way to make the ioctl work properly is to have it do a > > > > > > runtime-PM put at the start and then a runtime-PM get before it > > > > > > If and only if you want to do this with one ioctl() > > > If you separate the runtime-PM put and the get, you can do it without > > > the waiting part. > > > > Sure, but you still need a runtime-PM put at the start and a runtime-PM > > get at the end. In fact, if you separate this into two calls then you > > run the risk that the user may never perform the second call, and you > > would end up with an unbalanced put. > > > > I am tending to agree towards having a single ioctl call here. It is better > to give minimal control to user-space w.r.t. runtime PM. The current proposal > of user-space giving an hint to USB-FS/core that - it is not using the > device, sounds better. > > > > > > /* > > > > > * There are 3 possibilities here: > > > > > * 1. Device did suspend and resume (success) > > > > > * 2. Signal was received (failed suspend) > > > > > * 3. Time-out happened (failed suspend) > > > > > > > > 4. Device did suspend but a signal was received before the device > > > > resumed. > > > > > > > > > * In any of above cases, we need to resume device. > > > > > */ > > > > > usb_autoresume_device(dev); > > > > > > Yes and that is the problem. Why do you want to wait for the result > > > of runtime-PM put ? If we need a channel for notifying user space > > > about resume of a device, why wait for the result of suspend instead > > > of using the same channel? > > > > This is not meant to be a general-purpose channel for notifying > > userspace when a device resumes. Such a channel should be defined in > > the runtime-PM layer, not in the USB layer. > > > > This is instead meant to be a special-purpose mechanism for adding a > > runtime-suspend/resume interface to usbfs. > > > > Just to be clear here - the worst case wait-time from user-space > perspective will be + , right? That's right. > If yes then, timeout argument means "wait at-least timeout sec". Yes. Unless for a few unlikely cases, including: A signal is received before the timeout expires; The device suspends and then resumes before the timeout expires. On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > We spend time internally to go over the "new" ioctl proposal. Overall > it looks promising. > > However, we still have an issue as below - > Consider a use-case where, user-space calls "new" ioctl, but suspend > never happen (for various reasons) && async event happens on USB > device side by the end-user. > > In such a case, since user-space is waiting in "new" ioctl, it is not > in position to queue a request to read-out the async event info. It > will be able to queue a request when the "new" ioctl returns which > will be "time-out" later (in this case). Due to auto-suspend > time-out's default's value of 2 sec, the user-space has to choose the > time-out to "new" ioctl > 2 sec. Not so. That "2 second" value can be adjusted by the user; it can be reduced to as little as 1 ms. Alan Stern
Re: Error reading USB camera in BeagleBone with ARM Debian
Hi Joseph, Hi Bin, I am currently investigating a bug with the BeagleBoneBlack (which also uses the AM335x) and a Logitech Webcam, which might be connected. I am using a Logitech C270 and if I set the format to YUYV and the framesize to 544x288 or larger (including specifically 640x480), any ioctl VIDIOC_DQBUF will hang indefinetely. This might be the underlying source of the timeout Joseph encountered. Joseph, have you tried setting the camera to the lowest resolution it can go? If low resolutions work, but higher ones, like 640x480 don't, we might be chasing the same problem. If not, I will look into my problem separately and open a new thread. Bin, my case happened using ISOC transfers, but my preliminary investigation has shown lots of ISOC traffic flowing, apparently successfuly from the camera to the BBB, which makes me suspect a problem closer to the v4l2/musb boundary, but I'm still reading into the code. If you would like to investigate that further, the C270 is rather affordable, but I will gladly provide any details, usbmon captures etc. that you require. Cheers, Joel Pepper On 19.10.2018 20:06, Josep M. Mirats Tur wrote: > Hi Bin, > >> Do you know what is the usb throughput required in your test case? for >> example image format, resolution, frame rate? Is the usb data transfer >> bulk or isoch? > Yes, the image format is 640 x 480 > The camera can run up to 30fps, although I do not need it and set up it to 5 > The Usb data transfer is "bulk" > > Regards > Josep M. > > On Fri, Oct 19, 2018 at 8:01 PM Bin Liu wrote: >> On Fri, Oct 19, 2018 at 07:48:20PM +0200, Josep M. Mirats Tur wrote: >>> Hi, >>> >>> Yes, this would be the link: >>> http://shop-orbbec3d-com.3dcartstores.com/Astra-Mini_p_40.html >> Okay, It is about $160, a bit more than my bugdet... >> >>> In the meanwhile I'll check my application with other board. >>> >>> Do you think there might be a problem with the processor frequency? >>> I've also crossing e-mails with ORBBEC engineers, and their only >>> answer has been to suggest using an ARM processor capable of 1.8GHz >>> Beagle is capable of 1GHz >>> But I can not see why is this so important as long as USB is well >>> synchronized and interrupts and driver work well. (Actually I'm not >>> familiar with ARM USB system, is just intuition) >> I guess the cpu clock requirement is only to ensure the whole system is >> capable to process the usb packets, depending on the image resolution, >> fps, and any other data processing. But lower cpu clock shouldn't cause >> such problem that the usb controller generates rx interrupt but fifo has >> no usb packet. If ARM cannot keep up, which doesn't send IN requests on >> time, the camera should just drop data. >> >> Do you know what is the usb throughput required in your test case? for >> example image format, resolution, frame rate? Is the usb data transfer >> bulk or isoch? >> >> Regards, >> -Bin.
Re: Query on usb/core/devio.c
On Wed, 24 Oct 2018 10:10:32 -0400 Alan Stern wrote: > On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > > > On Mon, 22 Oct 2018 10:24:46 -0400 > > Alan Stern wrote: > > > > > On Mon, 22 Oct 2018, Oliver Neukum wrote: > > > > > > > On Do, 2018-10-18 at 13:42 -0400, Alan Stern wrote: > > > > > On Thu, 18 Oct 2018, Mayuresh Kulkarni wrote: > > > > > > > > > > > > The only way to make the ioctl work properly is to have it do a > > > > > > > runtime-PM put at the start and then a runtime-PM get before it > > > > > > > > If and only if you want to do this with one ioctl() > > > > If you separate the runtime-PM put and the get, you can do it without > > > > the waiting part. > > > > > > Sure, but you still need a runtime-PM put at the start and a runtime-PM > > > get at the end. In fact, if you separate this into two calls then you > > > run the risk that the user may never perform the second call, and you > > > would end up with an unbalanced put. > > > > > > > I am tending to agree towards having a single ioctl call here. It is better > > to give minimal control to user-space w.r.t. runtime PM. The current > > proposal of user-space giving an hint to USB-FS/core that - it is not using > > the device, sounds better. > > > > > > > > /* > > > > > > * There are 3 possibilities here: > > > > > > * 1. Device did suspend and resume (success) > > > > > > * 2. Signal was received (failed suspend) > > > > > > * 3. Time-out happened (failed suspend) > > > > > > > > > > 4. Device did suspend but a signal was received before the device > > > > > resumed. > > > > > > > > > > > * In any of above cases, we need to resume device. > > > > > > */ > > > > > > usb_autoresume_device(dev); > > > > > > > > Yes and that is the problem. Why do you want to wait for the result > > > > of runtime-PM put ? If we need a channel for notifying user space > > > > about resume of a device, why wait for the result of suspend instead > > > > of using the same channel? > > > > > > This is not meant to be a general-purpose channel for notifying > > > userspace when a device resumes. Such a channel should be defined in > > > the runtime-PM layer, not in the USB layer. > > > > > > This is instead meant to be a special-purpose mechanism for adding a > > > runtime-suspend/resume interface to usbfs. > > > > > > > Just to be clear here - the worst case wait-time from user-space > > perspective will be + , right? > > That's right. > > > If yes then, timeout argument means "wait at-least timeout sec". > > Yes. Unless for a few unlikely cases, including: > > A signal is received before the timeout expires; > > The device suspends and then resumes before the timeout > expires. > > > On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > > > We spend time internally to go over the "new" ioctl proposal. Overall > > it looks promising. > > > > However, we still have an issue as below - > > Consider a use-case where, user-space calls "new" ioctl, but suspend > > never happen (for various reasons) && async event happens on USB > > device side by the end-user. > > > > In such a case, since user-space is waiting in "new" ioctl, it is not > > in position to queue a request to read-out the async event info. It > > will be able to queue a request when the "new" ioctl returns which > > will be "time-out" later (in this case). Due to auto-suspend > > time-out's default's value of 2 sec, the user-space has to choose the > > time-out to "new" ioctl > 2 sec. > > Not so. That "2 second" value can be adjusted by the user; it can be > reduced to as little as 1 ms. > Thanks for the tip. But an issue with changing the module-param "usb_autosuspend_delay" is, the new value will be applicable to all USB devices connected after that. This may or may-not be optimum. I am not sure, but need to check if it is possible to set "usb_autosuspend_delay" based on connected device as well as current use-case running in the user-space we are targeting (which is Android, FYI). > Alan Stern
Re: Error reading USB camera in BeagleBone with ARM Debian
Joel, On Wed, Oct 24, 2018 at 05:31:25PM +0200, Joel Pepper wrote: > Hi Joseph, Hi Bin, > > I am currently investigating a bug with the BeagleBoneBlack (which also > uses the AM335x) and a Logitech Webcam, which might be connected. > > I am using a Logitech C270 and if I set the format to YUYV and the > framesize to 544x288 or larger (including specifically 640x480), any > ioctl VIDIOC_DQBUF will hang indefinetely. This might be the underlying > source of the timeout Joseph encountered. > > Joseph, > > have you tried setting the camera to the lowest resolution it can go? If > low resolutions work, but higher ones, like 640x480 don't, we might be > chasing the same problem. If not, I will look into my problem separately > and open a new thread. > > > Bin, > > my case happened using ISOC transfers, but my preliminary investigation > has shown lots of ISOC traffic flowing, apparently successfuly from the > camera to the BBB, which makes me suspect a problem closer to the > v4l2/musb boundary, but I'm still reading into the code. If you would > like to investigate that further, the C270 is rather affordable, but I > will gladly provide any details, usbmon captures etc. that you require. I am aware of an issue for 640x480@30fps with some cameras. I am in full-day meetings this week. Once I get time, I will provide more details for the issue I know of to see if you have face the same. Regards, -Bin.
Re: Query on usb/core/devio.c
On Wed, 24 Oct 2018, Mayuresh Kulkarni wrote: > > > In such a case, since user-space is waiting in "new" ioctl, it is not > > > in position to queue a request to read-out the async event info. It > > > will be able to queue a request when the "new" ioctl returns which > > > will be "time-out" later (in this case). Due to auto-suspend > > > time-out's default's value of 2 sec, the user-space has to choose the > > > time-out to "new" ioctl > 2 sec. > > > > Not so. That "2 second" value can be adjusted by the user; it can be > > reduced to as little as 1 ms. > > > > Thanks for the tip. > > But an issue with changing the module-param "usb_autosuspend_delay" > is, the new value will be applicable to all USB devices connected > after that. This may or may-not be optimum. The usb_autosuspend_delay module parameter is merely a default. Each device has its own autosuspend timeout value in its .../power/autosuspend_delay_ms sysfs file. > I am not sure, but need to check if it is possible to set > "usb_autosuspend_delay" based on connected device as well as current > use-case running in the user-space we are targeting (which is > Android, FYI). It's possible that Android has made some changes to the way autosuspend is handled. Alan Stern
ehci frame index goes backwards?
I am developing a custom USB device that makes use of isochronous IN transfers. It works fine from my laptop (xHCI) however on an embedded target (vortex86dx3, EHCI) I am seeing mysterious EFBIG errors. After adding lots of debug to ehci-sched I discovered that the EHCI register "FRINDEX" which is supposed to increase monotonically, is actually taking a jump backwards: [ 25.576198] ehci-pci :00:0a.1: scan_isoc ehci_read_frame_index(ehci)=12318 [ 25.576523] ehci-pci :00:0a.1: iso_stream_schedule ehci_read_frame_index(ehci)=12292 <- (count goes down!?) [ 25.576550] ehci-pci :00:0a.1: request c302d23e would overflow (4104+288 >= 4096) The backwards jump screws up periodic scheduling and culminates in the EFBIG error I've been seeing. I'm suspecting buggy EHCI hardware, but my question is... could anything else cause this? Other misc background facts: - Host is an Adlink CM1 PC104 module which contains a Vortex86 DX3. USB device is an LPC4323 operating full-speed. - I instigate the error by softbooting the system. EFBIG typically happens within a minute or does not happen at all. - I also get EXDEV errors from "URB was too late" in sitd_complete(). I suspect this is related to the problem above but don't fully understand the mechanism. Thanks, Dan.
Re: Error reading USB camera in BeagleBone with ARM Debian
Hi Joel, The camera I have provides 640x480 as its lowest resolution. Hope the issue can be solved. Regards On Wed, Oct 24, 2018 at 5:31 PM Joel Pepper wrote: > > Hi Joseph, Hi Bin, > > I am currently investigating a bug with the BeagleBoneBlack (which also > uses the AM335x) and a Logitech Webcam, which might be connected. > > I am using a Logitech C270 and if I set the format to YUYV and the > framesize to 544x288 or larger (including specifically 640x480), any > ioctl VIDIOC_DQBUF will hang indefinetely. This might be the underlying > source of the timeout Joseph encountered. > > Joseph, > > have you tried setting the camera to the lowest resolution it can go? If > low resolutions work, but higher ones, like 640x480 don't, we might be > chasing the same problem. If not, I will look into my problem separately > and open a new thread. > > > Bin, > > my case happened using ISOC transfers, but my preliminary investigation > has shown lots of ISOC traffic flowing, apparently successfuly from the > camera to the BBB, which makes me suspect a problem closer to the > v4l2/musb boundary, but I'm still reading into the code. If you would > like to investigate that further, the C270 is rather affordable, but I > will gladly provide any details, usbmon captures etc. that you require. > > > Cheers, > > Joel Pepper > > On 19.10.2018 20:06, Josep M. Mirats Tur wrote: > > Hi Bin, > > > >> Do you know what is the usb throughput required in your test case? for > >> example image format, resolution, frame rate? Is the usb data transfer > >> bulk or isoch? > > Yes, the image format is 640 x 480 > > The camera can run up to 30fps, although I do not need it and set up it to 5 > > The Usb data transfer is "bulk" > > > > Regards > > Josep M. > > > > On Fri, Oct 19, 2018 at 8:01 PM Bin Liu wrote: > >> On Fri, Oct 19, 2018 at 07:48:20PM +0200, Josep M. Mirats Tur wrote: > >>> Hi, > >>> > >>> Yes, this would be the link: > >>> http://shop-orbbec3d-com.3dcartstores.com/Astra-Mini_p_40.html > >> Okay, It is about $160, a bit more than my bugdet... > >> > >>> In the meanwhile I'll check my application with other board. > >>> > >>> Do you think there might be a problem with the processor frequency? > >>> I've also crossing e-mails with ORBBEC engineers, and their only > >>> answer has been to suggest using an ARM processor capable of 1.8GHz > >>> Beagle is capable of 1GHz > >>> But I can not see why is this so important as long as USB is well > >>> synchronized and interrupts and driver work well. (Actually I'm not > >>> familiar with ARM USB system, is just intuition) > >> I guess the cpu clock requirement is only to ensure the whole system is > >> capable to process the usb packets, depending on the image resolution, > >> fps, and any other data processing. But lower cpu clock shouldn't cause > >> such problem that the usb controller generates rx interrupt but fifo has > >> no usb packet. If ARM cannot keep up, which doesn't send IN requests on > >> time, the camera should just drop data. > >> > >> Do you know what is the usb throughput required in your test case? for > >> example image format, resolution, frame rate? Is the usb data transfer > >> bulk or isoch? > >> > >> Regards, > >> -Bin. > >
Re: ehci frame index goes backwards?
On Wed, Oct 24, 2018 at 9:48 AM Daniel Goertzen wrote: > > I am developing a custom USB device that makes use of isochronous IN > transfers. It works fine from my laptop (xHCI) however on an embedded > target (vortex86dx3, EHCI) I am seeing mysterious EFBIG errors. After > adding lots of debug to ehci-sched I discovered that the EHCI register > "FRINDEX" which is supposed to increase monotonically, is actually > taking a jump backwards: It does increment monotonically, but not forever. There is a limit defined by the ehci spec. Frame Index. The value in this register increments at the end of each time frame (e.g. micro-frame). Bits [N:3] are used for the Frame List current index. This means that each location of the frame list is accessed 8 times (frames or micro-frames) before moving to the next index. The following illustrates values of N based on the value of the Frame List Size field in the USBCMD register. 10, 11 or 12 bits is the N size. This becomes the SOF number. It does wrap, maybe you are seeing a reading after a wrap? Regards, Steve > ehci_read_frame_index(ehci)=12318 > [ 25.576523] ehci-pci :00:0a.1: iso_stream_schedule > ehci_read_frame_index(ehci)=12292 <- (count goes down!?) > [ 25.576550] ehci-pci :00:0a.1: request c302d23e would overflow > (4104+288 >= 4096) > > The backwards jump screws up periodic scheduling and culminates in the > EFBIG error I've been seeing. I'm suspecting buggy EHCI hardware, but > my question is... could anything else cause this? > > Other misc background facts: > - Host is an Adlink CM1 PC104 module which contains a Vortex86 DX3. > USB device is an LPC4323 operating full-speed. > - I instigate the error by softbooting the system. EFBIG typically > happens within a minute or does not happen at all. > - I also get EXDEV errors from "URB was too late" in sitd_complete(). > I suspect this is related to the problem above but don't fully > understand the mechanism. > > Thanks, > Dan.
RE: [PATCH v13 2/2] usb: typec: ucsi: add support for Cypress CCGx
Hi Andy
> > > > Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller over
> > > > I2C interface.
> > > >
> > > > This UCSI I2C driver uses I2C bus driver interface for
> > > > communicating with Type-C controller.
>
> > > > + /*
> > > > +* Flush CCGx RESPONSE queue by acking interrupts. Above ucsi
> > > control
> > > > +* register write will push response which must be cleared.
> > > > +*/
> > > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, &data, sizeof(data));
> > > > + if (status < 0)
> > > > + return status;
>
> (1)
>
> > > > + do {
> > > > + status = ccg_write(uc, CCGX_RAB_INTR_REG, &data,
> > > sizeof(data));
> > > > + if (status < 0)
> > > > + return status;
>
> (2)
>
> > > > +
> > > > + usleep_range(1, 11000);
> > > > +
> > > > + status = ccg_read(uc, CCGX_RAB_INTR_REG, &data,
> > > sizeof(data));
> > > > + if (status < 0)
> > > > + return status;
> > > > + } while ((data != 0x00) && count--);
> > >
> > > What's the significance of that count?
> > It is like a retry count to clear interrupt status.
> >
> > > Shouldn't you return -ETIMEDOUT if count == 0?
> > Yes. Good catch. Does the below fix looks ok?
>
> At least for me looks OK (I dunno why I missed that what Heikki found
> recently).
> Nevertheless, I have one more question about (1) and (2) above.
>
> Is it necessary to do one more read before do-while loop?
Yes, we need to read to get interrupt status bits and then write
them back to clear the status.
Thanks
Ajay
--nvpublic
>
> >
> > do {
> > status = ccg_write(uc, CCGX_RAB_INTR_REG, &data,
> > sizeof(data));
> > if (status < 0)
> > return status;
> >
> > usleep_range(1, 11000);
> >
> > status = ccg_read(uc, CCGX_RAB_INTR_REG, &data,
> > sizeof(data));
> > if (status < 0)
> > return status;
> >
> > if (!data)
> > return 0;
> > } while (data && count--);
> >
> > return -ETIMEDOUT;
>
> --
> With Best Regards,
> Andy Shevchenko
>
Re: [PATCH 2/3] net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers
Hello!
On 10/24/2018 04:51 PM, Jarkko Nikula wrote:
> Platform drivers don't need dummy runtime PM callbacks that just return
> success in order to have runtime PM happening. This has changed since
> following commits:
>
> 05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks")
> 543f2503a956 ("PM / platform_bus: Allow runtime PM by default")
> 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
Wow, these are old! :-)
> Signed-off-by: Jarkko Nikula
> ---
> Only build tested.
Seems long overdue...
[...]
Reviewed-by: Sergei Shtylyov
MBR, Sergei
RE: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Hi Wolfram, > > I don't think SMBus is an option in this case since the intended > > client (Cypress something in patch 2/2) does xfers that would need > > 16-bit commands and I think they are always 8-bit in SMBus, no? > > Yes. Command is 8 bit, data can be 16. Thanks for the heads up! Please help review v13 of this patch set at https://marc.info/?l=linux-i2c&m=153859126630601&w=2 https://marc.info/?l=linux-i2c&m=153859127330604&w=2 https://marc.info/?l=linux-i2c&m=153859127830605&w=2 Thanks Ajay --nvpublic
Re: ehci frame index goes backwards?
On Wed, 24 Oct 2018, Daniel Goertzen wrote: > I am developing a custom USB device that makes use of isochronous IN > transfers. It works fine from my laptop (xHCI) however on an embedded > target (vortex86dx3, EHCI) I am seeing mysterious EFBIG errors. After > adding lots of debug to ehci-sched I discovered that the EHCI register > "FRINDEX" which is supposed to increase monotonically, is actually > taking a jump backwards: > > [ 25.576198] ehci-pci :00:0a.1: scan_isoc > ehci_read_frame_index(ehci)=12318 > [ 25.576523] ehci-pci :00:0a.1: iso_stream_schedule > ehci_read_frame_index(ehci)=12292 <- (count goes down!?) Even stranger, the difference between the two timestamps is only 325 us, during which time the frame index value should not increase by more than 3. But here we see it decreasing by 26! > [ 25.576550] ehci-pci :00:0a.1: request c302d23e would overflow > (4104+288 >= 4096) > > The backwards jump screws up periodic scheduling and culminates in the > EFBIG error I've been seeing. I'm suspecting buggy EHCI hardware, but > my question is... could anything else cause this? Not anything else I can think of. Alan Stern > Other misc background facts: > - Host is an Adlink CM1 PC104 module which contains a Vortex86 DX3. > USB device is an LPC4323 operating full-speed. > - I instigate the error by softbooting the system. EFBIG typically > happens within a minute or does not happen at all. > - I also get EXDEV errors from "URB was too late" in sitd_complete(). > I suspect this is related to the problem above but don't fully > understand the mechanism. > > Thanks, > Dan.
Re: ehci frame index goes backwards?
On Wed, 24 Oct 2018, Steve Calfee wrote: > On Wed, Oct 24, 2018 at 9:48 AM Daniel Goertzen > wrote: > > > > I am developing a custom USB device that makes use of isochronous IN > > transfers. It works fine from my laptop (xHCI) however on an embedded > > target (vortex86dx3, EHCI) I am seeing mysterious EFBIG errors. After > > adding lots of debug to ehci-sched I discovered that the EHCI register > > "FRINDEX" which is supposed to increase monotonically, is actually > > taking a jump backwards: > It does increment monotonically, but not forever. There is a limit > defined by the ehci spec. > > > Frame Index. The value in this register increments at the end of each > time frame (e.g. > micro-frame). Bits [N:3] are used for the Frame List current index. > This means that each > location of the frame list is accessed 8 times (frames or > micro-frames) before moving to > the next index. The following illustrates values of N based on the > value of the Frame List > Size field in the USBCMD register. > > > 10, 11 or 12 bits is the N size. This becomes the SOF number. It does > wrap, maybe you are seeing a reading after a wrap? The value won't wrap from 12318 to 12292 (the values reported by Daniel). In fact, it won't wrap until it gets above at least 16383, since values that high are required for calculating the SOF number. Alan Stern
Re: Error reading USB camera in BeagleBone with ARM Debian
Joel, On Wed, Oct 24, 2018 at 07:19:01PM +0200, Josep M. Mirats Tur wrote: > Hi Joel, > > The camera I have provides 640x480 as its lowest resolution. > Hope the issue can be solved. If your issue was same as mine - the uvc layer holds spinlock for too long in urb->complete() which prevents musb to schedule next IN transaction on time, it won't be that easy to solve. :( I had a try last year, but the solution caused other problem, then haven't looked at this issue since then. Regards, -Bin. > Regards > > On Wed, Oct 24, 2018 at 5:31 PM Joel Pepper > wrote: > > > > Hi Joseph, Hi Bin, > > > > I am currently investigating a bug with the BeagleBoneBlack (which also > > uses the AM335x) and a Logitech Webcam, which might be connected. > > > > I am using a Logitech C270 and if I set the format to YUYV and the > > framesize to 544x288 or larger (including specifically 640x480), any > > ioctl VIDIOC_DQBUF will hang indefinetely. This might be the underlying > > source of the timeout Joseph encountered. > > > > Joseph, > > > > have you tried setting the camera to the lowest resolution it can go? If > > low resolutions work, but higher ones, like 640x480 don't, we might be > > chasing the same problem. If not, I will look into my problem separately > > and open a new thread. > > > > > > Bin, > > > > my case happened using ISOC transfers, but my preliminary investigation > > has shown lots of ISOC traffic flowing, apparently successfuly from the > > camera to the BBB, which makes me suspect a problem closer to the > > v4l2/musb boundary, but I'm still reading into the code. If you would > > like to investigate that further, the C270 is rather affordable, but I > > will gladly provide any details, usbmon captures etc. that you require. > > > > > > Cheers, > > > > Joel Pepper > > > > On 19.10.2018 20:06, Josep M. Mirats Tur wrote: > > > Hi Bin, > > > > > >> Do you know what is the usb throughput required in your test case? for > > >> example image format, resolution, frame rate? Is the usb data transfer > > >> bulk or isoch? > > > Yes, the image format is 640 x 480 > > > The camera can run up to 30fps, although I do not need it and set up it > > > to 5 > > > The Usb data transfer is "bulk" > > > > > > Regards > > > Josep M. > > > > > > On Fri, Oct 19, 2018 at 8:01 PM Bin Liu wrote: > > >> On Fri, Oct 19, 2018 at 07:48:20PM +0200, Josep M. Mirats Tur wrote: > > >>> Hi, > > >>> > > >>> Yes, this would be the link: > > >>> http://shop-orbbec3d-com.3dcartstores.com/Astra-Mini_p_40.html > > >> Okay, It is about $160, a bit more than my bugdet... > > >> > > >>> In the meanwhile I'll check my application with other board. > > >>> > > >>> Do you think there might be a problem with the processor frequency? > > >>> I've also crossing e-mails with ORBBEC engineers, and their only > > >>> answer has been to suggest using an ARM processor capable of 1.8GHz > > >>> Beagle is capable of 1GHz > > >>> But I can not see why is this so important as long as USB is well > > >>> synchronized and interrupts and driver work well. (Actually I'm not > > >>> familiar with ARM USB system, is just intuition) > > >> I guess the cpu clock requirement is only to ensure the whole system is > > >> capable to process the usb packets, depending on the image resolution, > > >> fps, and any other data processing. But lower cpu clock shouldn't cause > > >> such problem that the usb controller generates rx interrupt but fifo has > > >> no usb packet. If ARM cannot keep up, which doesn't send IN requests on > > >> time, the camera should just drop data. > > >> > > >> Do you know what is the usb throughput required in your test case? for > > >> example image format, resolution, frame rate? Is the usb data transfer > > >> bulk or isoch? > > >> > > >> Regards, > > >> -Bin. > > > >
Re: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On Wed, Oct 24, 2018 at 05:46:22PM +, Ajay Gupta wrote: > Hi Wolfram, > > > I don't think SMBus is an option in this case since the intended > > > client (Cypress something in patch 2/2) does xfers that would need > > > 16-bit commands and I think they are always 8-bit in SMBus, no? > > > > Yes. Command is 8 bit, data can be 16. Thanks for the heads up! > Please help review v13 of this patch set at > https://marc.info/?l=linux-i2c&m=153859126630601&w=2 > https://marc.info/?l=linux-i2c&m=153859127330604&w=2 > https://marc.info/?l=linux-i2c&m=153859127830605&w=2 It is the middle of the merge window, maintainers are a "bit" busy at the moment :)
EPROTO when USB 3 GbE adapters are under load
Hi, There are multiple reports[1][2][3] (more elsewhere on the internet) of USB 3 GbE adapters throwing EPROTO errors on USB transfer especially when the devices are under load. Both of the two common chipsets (Realtek RTL8153 (r8152[4]) and Asix AX88179 (ax88179_178a[5])) seem to exhibit this behaviour. [1]: https://bugzilla.kernel.org/show_bug.cgi?id=75381 [2]: https://bugzilla.kernel.org/show_bug.cgi?id=196747 [3]: https://bugzilla.kernel.org/show_bug.cgi?id=198931 [4]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/usb/r8152.c [5]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/usb/ax88179_178a.c I'm trying to figure out why this happens (while it doesn't seem to happen on other OSes, but I'm not sure). I think it's unlikely that both drivers are buggy, so perhaps it is something to do with the USB stack instead of the device drivers. It wouldn't be surprising if both devices actually don't adhere to the USB specs properly and other OSes are just more tolerant of that (?) but that is just conjecture on my part. Does anyone have any ideas? Thanks. -- Hao Wei
Re: [PATCH V2 4/6] usb: ohci-platform: Add support for Broadcom STB SoC's
On Wed, Oct 17, 2018 at 11:30 PM Al Cooper wrote:
>
> Add support for Broadcom STB SoC's to the ohci platform driver.
>
> Signed-off-by: Al Cooper
> ---
> drivers/usb/host/ohci-platform.c | 35 +--
> include/linux/usb/ohci_pdriver.h | 1 +
> 2 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-platform.c
> b/drivers/usb/host/ohci-platform.c
> index 65a1c3fdc88c..363d6fa676a5 100644
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -99,12 +100,24 @@ static int ohci_platform_probe(struct platform_device
> *dev)
> if (usb_disabled())
> return -ENODEV;
>
> - /*
> -* Use reasonable defaults so platforms don't have to provide these
> -* with DT probing on ARM.
> -*/
> - if (!pdata)
> - pdata = &ohci_platform_defaults;
> + if (!pdata) {
> + const struct usb_ohci_pdata *match_pdata;
> +
> + match_pdata = of_device_get_match_data(&dev->dev);
> + if (match_pdata) {
> + pdata = devm_kzalloc(&dev->dev, sizeof(*pdata),
> +GFP_KERNEL);
> + if (!pdata)
> + return -ENOMEM;
> + *pdata = *match_pdata;
It looks like you copy the const pdata to get a non-const version.
Have you tried
propagating the 'const' modifier so that users can rely on that here?
> + } else {
> + /*
> +* Use reasonable defaults so platforms don't have
> +* to provide these with DT probing on ARM.
> +*/
> + pdata = &ohci_platform_defaults;
> + }
> + }
That would also allow you to unify it with the else path by listing
the ohci_platform_defaults
in the id table for all other compatible strings.
Arnd
Re: [PATCH V2 5/6] usb: ehci: Add new EHCI driver for Broadcom STB SoC's
On Wed, Oct 17, 2018 at 11:30 PM Al Cooper wrote:
> +
> +static int ehci_brcm_probe(struct platform_device *pdev)
> +{
> + struct usb_hcd *hcd;
> + struct resource *res_mem;
> + struct brcm_priv *priv;
> + int irq;
> + int err;
> +
> + if (usb_disabled())
> + return -ENODEV;
> +
> + err = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> + if (err)
> + return err;
You should never use "dma_coerce_mask_and_coherent" in new code. Make sure that
the DT is correct and call dma_set_mask_and_coherent() instead.
> +#ifdef CONFIG_OF
> +static const struct of_device_id brcm_ehci_of_match[] = {
> + { .compatible = "brcm,bcm7445-ehci", },
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, brcm_ehci_of_match);
> +#endif /* CONFIG_OF */
> +
> +static struct platform_driver ehci_brcm_driver = {
> + .probe = ehci_brcm_probe,
> + .remove = ehci_brcm_remove,
> + .shutdown = usb_hcd_platform_shutdown,
> + .driver = {
> + .owner = THIS_MODULE,
> + .name = "ehci-brcm",
> + .pm = &ehci_brcm_pm_ops,
> + .of_match_table = of_match_ptr(brcm_ehci_of_match),
> + }
> +};
You won't ever use this driver without CONFIG_OF, so just remove the #ifdef and
of_match_ptr() wrapper here. It will still build fine without CONFIG_OF.
Arnd
[PATCH 0/4 v5] Keep rtsx_usb suspended when there's no card
Hi, This is based on Ulf's work [1] [2]. This patch series can keep rtsx_usb suspended, to save ~0.5W on Intel platforms and ~1.5W on AMD platforms. [1] https://patchwork.kernel.org/patch/10440583/ [2] https://patchwork.kernel.org/patch/10445725/ Kai-Heng Feng (4): misc: rtsx_usb: Use USB remote wakeup signaling for card insertion detection memstick: Prevent memstick host from getting runtime suspended during card detection memstick: rtsx_usb_ms: Use ms_dev() helper memstick: rtsx_usb_ms: Support runtime power management v5: Corretly use system suspend/resume and runtime suspend/resume callback. Prevent runtime callbacks get call during system suspend. v4: Use pm_runtime_put() in memstick_check(). v3: Skip parent device check in rtsx_usb_resume_child(). Remove dev_dbg() if it only prints function name. Use ms_dev() helper at more places. Remove const qualifier for UNIVERSAL_DEV_PM_OPS. v2: Resend to linux-usb and LKML. drivers/memstick/core/memstick.c| 4 + drivers/memstick/host/rtsx_usb_ms.c | 173 +--- drivers/misc/cardreader/rtsx_usb.c | 8 ++ 3 files changed, 117 insertions(+), 68 deletions(-) -- 2.17.1
[PATCH 1/4 v5] misc: rtsx_usb: Use USB remote wakeup signaling for card insertion detection
Although rtsx_usb doesn't support card removal detection, card insertion
will resume rtsx_usb by USB remote wakeup signaling.
When rtsx_usb gets resumed, also resumes its child devices,
rtsx_usb_sdmmc and rtsx_usb_ms, to notify them there's a card in its
slot.
Signed-off-by: Kai-Heng Feng
---
drivers/misc/cardreader/rtsx_usb.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/misc/cardreader/rtsx_usb.c
b/drivers/misc/cardreader/rtsx_usb.c
index b97903ff1a72..f7a66f614085 100644
--- a/drivers/misc/cardreader/rtsx_usb.c
+++ b/drivers/misc/cardreader/rtsx_usb.c
@@ -723,8 +723,15 @@ static int rtsx_usb_suspend(struct usb_interface *intf,
pm_message_t message)
return 0;
}
+static int rtsx_usb_resume_child(struct device *dev, void *data)
+{
+ pm_request_resume(dev);
+ return 0;
+}
+
static int rtsx_usb_resume(struct usb_interface *intf)
{
+ device_for_each_child(&intf->dev, NULL, rtsx_usb_resume_child);
return 0;
}
@@ -734,6 +741,7 @@ static int rtsx_usb_reset_resume(struct usb_interface *intf)
(struct rtsx_ucr *)usb_get_intfdata(intf);
rtsx_usb_reset_chip(ucr);
+ device_for_each_child(&intf->dev, NULL, rtsx_usb_resume_child);
return 0;
}
--
2.17.1
[PATCH 2/4 v5] memstick: Prevent memstick host from getting runtime suspended during card detection
We can use MEMSTICK_POWER_{ON,OFF} along with pm_runtime_{get,put}
helpers to let memstick host support runtime pm.
There's a small window between memstick_detect_change() and its queued
work, memstick_check(). In this window the rpm count may go down to zero
before the memstick host powers on, so the host can be inadvertently
suspended.
Increment rpm count before calling memstick_check(), and decrement rpm
count afterward, as now we are sure the memstick host should be
suspended or not.
Signed-off-by: Kai-Heng Feng
---
drivers/memstick/core/memstick.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index 76382c858c35..5f16a8826401 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#define DRIVER_NAME "memstick"
@@ -209,6 +210,7 @@ static int memstick_dummy_check(struct memstick_dev *card)
*/
void memstick_detect_change(struct memstick_host *host)
{
+ pm_runtime_get_noresume(host->dev.parent);
queue_work(workqueue, &host->media_checker);
}
EXPORT_SYMBOL(memstick_detect_change);
@@ -479,6 +481,8 @@ static void memstick_check(struct work_struct *work)
host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_OFF);
mutex_unlock(&host->lock);
+
+ pm_runtime_put(host->dev.parent);
dev_dbg(&host->dev, "memstick_check finished\n");
}
--
2.17.1
[PATCH 4/4 v5] memstick: rtsx_usb_ms: Support runtime power management
In order to let host's parent device, rtsx_usb, to use USB remote wake
up signaling to do card detection, it needs to be suspended. Hence it's
necessary to add runtime PM support for the memstick host.
To keep memstick host stays suspended when it's not in use, convert the
card detection function from kthread to delayed_work, which can be
scheduled when the host is resumed and can be canceled when the host is
suspended.
Put the device to suspend when there's no card and the power mode is
MEMSTICK_POWER_OFF.
Signed-off-by: Kai-Heng Feng
---
drivers/memstick/host/rtsx_usb_ms.c | 167 +---
1 file changed, 102 insertions(+), 65 deletions(-)
diff --git a/drivers/memstick/host/rtsx_usb_ms.c
b/drivers/memstick/host/rtsx_usb_ms.c
index cd12f3d1c088..3800c24b084e 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -40,15 +40,14 @@ struct rtsx_usb_ms {
struct mutexhost_mutex;
struct work_struct handle_req;
-
- struct task_struct *detect_ms;
- struct completion detect_ms_exit;
+ struct delayed_work poll_card;
u8 ssc_depth;
unsigned intclock;
int power_mode;
unsigned char ifmode;
booleject;
+ boolsystem_suspending;
};
static inline struct device *ms_dev(struct rtsx_usb_ms *host)
@@ -545,7 +544,7 @@ static void rtsx_usb_ms_handle_req(struct work_struct *work)
host->req->error);
}
} while (!rc);
- pm_runtime_put(ms_dev(host));
+ pm_runtime_put_sync(ms_dev(host));
}
}
@@ -585,14 +584,14 @@ static int rtsx_usb_ms_set_param(struct memstick_host
*msh,
break;
if (value == MEMSTICK_POWER_ON) {
- pm_runtime_get_sync(ms_dev(host));
+ pm_runtime_get_noresume(ms_dev(host));
err = ms_power_on(host);
+ if (err)
+ pm_runtime_put_noidle(ms_dev(host));
} else if (value == MEMSTICK_POWER_OFF) {
err = ms_power_off(host);
- if (host->msh->card)
+ if (!err)
pm_runtime_put_noidle(ms_dev(host));
- else
- pm_runtime_put(ms_dev(host));
} else
err = -EINVAL;
if (!err)
@@ -638,25 +637,44 @@ static int rtsx_usb_ms_set_param(struct memstick_host
*msh,
}
out:
mutex_unlock(&ucr->dev_mutex);
- pm_runtime_put(ms_dev(host));
+ pm_runtime_put_sync(ms_dev(host));
/* power-on delay */
- if (param == MEMSTICK_POWER && value == MEMSTICK_POWER_ON)
+ if (param == MEMSTICK_POWER && value == MEMSTICK_POWER_ON) {
usleep_range(1, 12000);
+ if (!host->eject)
+ schedule_delayed_work(&host->poll_card, 100);
+ }
+
dev_dbg(ms_dev(host), "%s: return = %d\n", __func__, err);
return err;
}
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
static int rtsx_usb_ms_suspend(struct device *dev)
{
struct rtsx_usb_ms *host = dev_get_drvdata(dev);
struct memstick_host *msh = host->msh;
- dev_dbg(ms_dev(host), "--> %s\n", __func__);
+ /* Since we use rtsx_usb's resume callback to runtime resume its
+* children to implement remote wakeup signaling, this causes
+* rtsx_usb_ms' runtime resume callback runs after its suspend
+* callback:
+* rtsx_usb_ms_suspend()
+* rtsx_usb_resume()
+* -> rtsx_usb_ms_runtime_resume()
+* -> memstick_detect_change()
+*
+* rtsx_usb_suspend()
+*
+* To avoid this, skip runtime resume/suspend if system suspend is
+* underway.
+*/
+ host->system_suspending = true;
memstick_suspend_host(msh);
+
return 0;
}
@@ -665,58 +683,83 @@ static int rtsx_usb_ms_resume(struct device *dev)
struct rtsx_usb_ms *host = dev_get_drvdata(dev);
struct memstick_host *msh = host->msh;
- dev_dbg(ms_dev(host), "--> %s\n", __func__);
-
memstick_resume_host(msh);
+ host->system_suspending = false;
+
return 0;
}
-#endif /* CONFIG_PM_SLEEP */
-/*
- * Thread function of ms card slot detection. The thread starts right after
- * successful host addition. It stops while the driver removal function sets
- * host->eject true.
- */
-static int rtsx_usb_detect_ms_card(void *__host)
+static int rtsx_usb_ms_runtime_suspend(struct device *dev)
+{
+ struct rtsx_usb_ms *host = dev_get_drvdata(dev);
+
+ if (host->system_suspending)
+ return
[PATCH 3/4 v5] memstick: rtsx_usb_ms: Use ms_dev() helper
Use ms_dev() helper for consistency.
Signed-off-by: Kai-Heng Feng
---
drivers/memstick/host/rtsx_usb_ms.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/memstick/host/rtsx_usb_ms.c
b/drivers/memstick/host/rtsx_usb_ms.c
index 4f64563df7de..cd12f3d1c088 100644
--- a/drivers/memstick/host/rtsx_usb_ms.c
+++ b/drivers/memstick/host/rtsx_usb_ms.c
@@ -785,7 +785,7 @@ static int rtsx_usb_ms_drv_remove(struct platform_device
*pdev)
mutex_lock(&host->host_mutex);
if (host->req) {
- dev_dbg(&(pdev->dev),
+ dev_dbg(ms_dev(host),
"%s: Controller removed during transfer\n",
dev_name(&msh->dev));
host->req->error = -ENOMEDIUM;
@@ -807,10 +807,10 @@ static int rtsx_usb_ms_drv_remove(struct platform_device
*pdev)
if (pm_runtime_active(ms_dev(host)))
pm_runtime_put(ms_dev(host));
- pm_runtime_disable(&pdev->dev);
+ pm_runtime_disable(ms_dev(host));
platform_set_drvdata(pdev, NULL);
- dev_dbg(&(pdev->dev),
+ dev_dbg(ms_dev(host),
": Realtek USB Memstick controller has been removed\n");
return 0;
--
2.17.1
[PATCH 0/3] PM: Renesas: Remove dummy runtime PM callbacks
I noticed these independent Renesas drivers have needless dummy runtime PM callbacks. I don't have the HW so only build tested. Patches can be applied independently to their own subsystems. I wanted to send them together if some of them gets Tested-by or sees a regression. Jarkko Nikula (3): i2c: sh_mobile: Remove dummy runtime PM callbacks net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers usb: renesas_usbhs: Remove dummy runtime PM callbacks drivers/i2c/busses/i2c-sh_mobile.c | 18 -- drivers/net/ethernet/renesas/ravb_main.c | 13 - drivers/net/ethernet/renesas/sh_eth.c| 13 - drivers/usb/renesas_usbhs/common.c | 14 -- 4 files changed, 58 deletions(-) -- 2.19.1
[PATCH 2/3] net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers
Platform drivers don't need dummy runtime PM callbacks that just return
success in order to have runtime PM happening. This has changed since
following commits:
05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks")
543f2503a956 ("PM / platform_bus: Allow runtime PM by default")
8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
Signed-off-by: Jarkko Nikula
---
Only build tested.
---
drivers/net/ethernet/renesas/ravb_main.c | 13 -
drivers/net/ethernet/renesas/sh_eth.c| 13 -
2 files changed, 26 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c
b/drivers/net/ethernet/renesas/ravb_main.c
index defed0d0c51d..9498f9d67501 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2335,21 +2335,8 @@ static int __maybe_unused ravb_resume(struct device *dev)
return ret;
}
-static int __maybe_unused ravb_runtime_nop(struct device *dev)
-{
- /* Runtime PM callback shared between ->runtime_suspend()
-* and ->runtime_resume(). Simply returns success.
-*
-* This driver re-initializes all registers after
-* pm_runtime_get_sync() anyway so there is no need
-* to save and restore registers here.
-*/
- return 0;
-}
-
static const struct dev_pm_ops ravb_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(ravb_suspend, ravb_resume)
- SET_RUNTIME_PM_OPS(ravb_runtime_nop, ravb_runtime_nop, NULL)
};
static struct platform_driver ravb_driver = {
diff --git a/drivers/net/ethernet/renesas/sh_eth.c
b/drivers/net/ethernet/renesas/sh_eth.c
index f27a0dc8c563..980310686d45 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3447,21 +3447,8 @@ static int sh_eth_resume(struct device *dev)
}
#endif
-static int sh_eth_runtime_nop(struct device *dev)
-{
- /* Runtime PM callback shared between ->runtime_suspend()
-* and ->runtime_resume(). Simply returns success.
-*
-* This driver re-initializes all registers after
-* pm_runtime_get_sync() anyway so there is no need
-* to save and restore registers here.
-*/
- return 0;
-}
-
static const struct dev_pm_ops sh_eth_dev_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(sh_eth_suspend, sh_eth_resume)
- SET_RUNTIME_PM_OPS(sh_eth_runtime_nop, sh_eth_runtime_nop, NULL)
};
#define SH_ETH_PM_OPS (&sh_eth_dev_pm_ops)
#else
--
2.19.1
[PATCH 1/3] i2c: sh_mobile: Remove dummy runtime PM callbacks
Platform drivers don't need dummy runtime PM callbacks that just return
success and non-NULL pm pointer in their struct device_driver in order
to have runtime PM happening. This has changed since following commits:
05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks")
543f2503a956 ("PM / platform_bus: Allow runtime PM by default")
8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
Signed-off-by: Jarkko Nikula
---
Only build tested.
---
drivers/i2c/busses/i2c-sh_mobile.c | 18 --
1 file changed, 18 deletions(-)
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c
b/drivers/i2c/busses/i2c-sh_mobile.c
index 818cab14e87c..a7a7a9c3bc7c 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -947,27 +947,9 @@ static int sh_mobile_i2c_remove(struct platform_device
*dev)
return 0;
}
-static int sh_mobile_i2c_runtime_nop(struct device *dev)
-{
- /* Runtime PM callback shared between ->runtime_suspend()
-* and ->runtime_resume(). Simply returns success.
-*
-* This driver re-initializes all registers after
-* pm_runtime_get_sync() anyway so there is no need
-* to save and restore registers here.
-*/
- return 0;
-}
-
-static const struct dev_pm_ops sh_mobile_i2c_dev_pm_ops = {
- .runtime_suspend = sh_mobile_i2c_runtime_nop,
- .runtime_resume = sh_mobile_i2c_runtime_nop,
-};
-
static struct platform_driver sh_mobile_i2c_driver = {
.driver = {
.name = "i2c-sh_mobile",
- .pm = &sh_mobile_i2c_dev_pm_ops,
.of_match_table = sh_mobile_i2c_dt_ids,
},
.probe = sh_mobile_i2c_probe,
--
2.19.1
[PATCH 3/3] usb: renesas_usbhs: Remove dummy runtime PM callbacks
Platform drivers don't need dummy runtime PM callbacks that just return
success in order to have runtime PM happening. This has changed since
following commits:
05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks")
543f2503a956 ("PM / platform_bus: Allow runtime PM by default")
8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
Signed-off-by: Jarkko Nikula
---
Only build tested.
---
drivers/usb/renesas_usbhs/common.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/usb/renesas_usbhs/common.c
b/drivers/usb/renesas_usbhs/common.c
index a3e1290d682d..0e760f228dd8 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -874,23 +874,9 @@ static int usbhsc_resume(struct device *dev)
return 0;
}
-static int usbhsc_runtime_nop(struct device *dev)
-{
- /* Runtime PM callback shared between ->runtime_suspend()
-* and ->runtime_resume(). Simply returns success.
-*
-* This driver re-initializes all registers after
-* pm_runtime_get_sync() anyway so there is no need
-* to save and restore registers here.
-*/
- return 0;
-}
-
static const struct dev_pm_ops usbhsc_pm_ops = {
.suspend= usbhsc_suspend,
.resume = usbhsc_resume,
- .runtime_suspend= usbhsc_runtime_nop,
- .runtime_resume = usbhsc_runtime_nop,
};
static struct platform_driver renesas_usbhs_driver = {
--
2.19.1
Re: [PATCH 0/3] PM: Renesas: Remove dummy runtime PM callbacks
On Wed, Oct 24, 2018 at 04:51:31PM +0300, Jarkko Nikula wrote: > I noticed these independent Renesas drivers have needless dummy runtime > PM callbacks. I don't have the HW so only build tested. > > Patches can be applied independently to their own subsystems. I wanted to > send them together if some of them gets Tested-by or sees a regression. At least the I2C driver part of this was on my todo list as well (just a bit lower :/). I wanted to find out why they have been there in the first place. Do you know if such callbacks were needed "back in the days"? Adding Magnus to recipients... > > Jarkko Nikula (3): > i2c: sh_mobile: Remove dummy runtime PM callbacks > net: ethernet: Remove dummy runtime PM callbacks from Renesas drivers > usb: renesas_usbhs: Remove dummy runtime PM callbacks > > drivers/i2c/busses/i2c-sh_mobile.c | 18 -- > drivers/net/ethernet/renesas/ravb_main.c | 13 - > drivers/net/ethernet/renesas/sh_eth.c| 13 - > drivers/usb/renesas_usbhs/common.c | 14 -- > 4 files changed, 58 deletions(-) > > -- > 2.19.1 > signature.asc Description: PGP signature
[RFC PATCH 1/5] driver core: Add fwnode member to struct device_connection
This will prepare the device connection API for connections
described in firmware.
Signed-off-by: Heikki Krogerus
---
include/linux/device.h | 6 ++
1 file changed, 6 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index 90224e75ade4..a964a0d614fa 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -753,11 +753,17 @@ struct device_dma_parameters {
/**
* struct device_connection - Device Connection Descriptor
+ * @fwnode: The device node of the connected device
* @endpoint: The names of the two devices connected together
* @id: Unique identifier for the connection
* @list: List head, private, for internal use only
+ *
+ * NOTE: @fwnode is not used together with @endpoint. @fwnode is used when
+ * platform firmware defines the connection. When the connection is registeded
+ * with device_connection_add() @endpoint is used instead.
*/
struct device_connection {
+ struct fwnode_handle*fwnode;
const char *endpoint[2];
const char *id;
struct list_headlist;
--
2.19.1
[RFC PATCH 3/5] usb: roles: Find the muxes by also matching against the device node
When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.
Signed-off-by: Heikki Krogerus
---
drivers/usb/common/roles.c | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 99116af07f1d..bb52e006d203 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -8,6 +8,7 @@
*/
#include
+#include
#include
#include
#include
@@ -84,7 +85,12 @@ enum usb_role usb_role_switch_get_role(struct
usb_role_switch *sw)
}
EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
-static int __switch_match(struct device *dev, const void *name)
+static int switch_fwnode_match(struct device *dev, const void *fwnode)
+{
+ return dev_fwnode(dev) == fwnode;
+}
+
+static int switch_name_match(struct device *dev, const void *name)
{
return !strcmp((const char *)name, dev_name(dev));
}
@@ -94,8 +100,12 @@ static void *usb_role_switch_match(struct device_connection
*con, int ep,
{
struct device *dev;
- dev = class_find_device(role_class, NULL, con->endpoint[ep],
- __switch_match);
+ if (con->fwnode)
+ dev = class_find_device(role_class, NULL, con->fwnode,
+ switch_fwnode_match);
+ else
+ dev = class_find_device(role_class, NULL, con->endpoint[ep],
+ switch_name_match);
return dev ? to_role_switch(dev) : ERR_PTR(-EPROBE_DEFER);
}
--
2.19.1
[RFC PATCH 2/5] usb: typec: mux: Find the muxes by also matching against the device node
When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/mux.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index d990aa510fab..161a96280296 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
static DEFINE_MUTEX(switch_lock);
@@ -23,9 +24,14 @@ static void *typec_switch_match(struct device_connection
*con, int ep,
{
struct typec_switch *sw;
- list_for_each_entry(sw, &switch_list, entry)
- if (!strcmp(con->endpoint[ep], dev_name(sw->dev)))
+ list_for_each_entry(sw, &switch_list, entry) {
+ if (con->fwnode) {
+ if (dev_fwnode(sw->dev) == con->fwnode)
+ return sw;
+ } else if (!strcmp(con->endpoint[ep], dev_name(sw->dev))) {
return sw;
+ }
+ }
/*
* We only get called if a connection was found, tell the caller to
@@ -114,9 +120,14 @@ static void *typec_mux_match(struct device_connection
*con, int ep, void *data)
{
struct typec_mux *mux;
- list_for_each_entry(mux, &mux_list, entry)
- if (!strcmp(con->endpoint[ep], dev_name(mux->dev)))
+ list_for_each_entry(mux, &mux_list, entry) {
+ if (con->fwnode) {
+ if (dev_fwnode(mux->dev) == con->fwnode)
+ return mux;
+ } else if (!strcmp(con->endpoint[ep], dev_name(mux->dev))) {
return mux;
+ }
+ }
/*
* We only get called if a connection was found, tell the caller to
--
2.19.1
[RFC PATCH 0/5] Adding graph handling to device connection API
Hi, I'm only presenting my idea with these how I think we should be able to deal with graphs in the API, so these are completely untested, and obviously I can't say for certain if the idea works or not. I will try to test these using custom ACPI tables, but of course these should be tested on DT platform as well, so if somebody can do that, I would much appreciate. Thanks, Heikki Krogerus (5): driver core: Add fwnode member to struct device_connection usb: typec: mux: Find the muxes by also matching against the device node usb: roles: Find the muxes by also matching against the device node usb: typec: Find the ports by also matching against the device node drivers core: Find device connections also from device graphs drivers/base/devcon.c | 48 +++--- drivers/usb/common/roles.c | 16 ++--- drivers/usb/typec/class.c | 19 --- drivers/usb/typec/mux.c| 19 +++ include/linux/device.h | 6 + 5 files changed, 95 insertions(+), 13 deletions(-) -- 2.19.1
[RFC PATCH 4/5] usb: typec: Find the ports by also matching against the device node
When the connections are defined in firmware, struct
device_connection will have the fwnode member pointing to
the device node (struct fwnode_handle) of the requested
device, and the endpoint will not be used at all in that
case.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/class.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 5db0593ca0bd..fe6f3a932a88 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -204,15 +204,28 @@ static void typec_altmode_put_partner(struct altmode
*altmode)
put_device(&adev->dev);
}
-static int __typec_port_match(struct device *dev, const void *name)
+static int typec_port_fwnode_match(struct device *dev, const void *fwnode)
+{
+ return dev_name(fwnode) == fwnode;
+}
+
+static int typec_port_name_match(struct device *dev, const void *name)
{
return !strcmp((const char *)name, dev_name(dev));
}
static void *typec_port_match(struct device_connection *con, int ep, void
*data)
{
- return class_find_device(typec_class, NULL, con->endpoint[ep],
-__typec_port_match);
+ struct device *dev;
+
+ if (con->fwnode)
+ dev = class_find_device(typec_class, NULL, con->fwnode,
+ typec_port_fwnode_match);
+ else
+ dev = class_find_device(typec_class, NULL, con->endpoint[ep],
+ typec_port_name_match);
+
+ return dev ? dev : ERR_PTR(-EPROBE_DEFER);
}
struct typec_altmode *
--
2.19.1
[RFC PATCH 5/5] drivers core: Find device connections also from device graphs
If connections between devices are described in OF graph or
ACPI device graph, we can find them by using the
fwnode_graph_*() functions.
Signed-off-by: Heikki Krogerus
---
drivers/base/devcon.c | 48 ---
1 file changed, 45 insertions(+), 3 deletions(-)
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index d427e806cd73..9c446ef84dd1 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -7,10 +7,45 @@
*/
#include
+#include
static DEFINE_MUTEX(devcon_lock);
static LIST_HEAD(devcon_list);
+static void *
+fwnode_graph_devcon_match(struct fwnode_handle *fwnode,
+ const char *con_id, void *data,
+ void *(*match)(struct device_connection *con,
+int ep, void *data))
+{
+ struct device_connection con = { .id = con_id };
+ struct fwnode_handle *ep;
+ const char *val;
+ void *ret;
+ int err;
+
+ fwnode_graph_for_each_endpoint(fwnode, ep) {
+ con.fwnode = fwnode_graph_get_remote_port_parent(ep);
+
+ if (con_id) {
+ err = fwnode_property_read_string(con.fwnode, "name",
+ &val);
+ if (err || strcmp(con_id, val)) {
+ fwnode_handle_put(con.fwnode);
+ continue;
+ }
+ }
+
+ ret = match(&con, -1, data);
+ fwnode_handle_put(con.fwnode);
+ if (ret) {
+ fwnode_handle_put(ep);
+ return ret;
+ }
+ }
+ return NULL;
+}
+
/**
* device_connection_find_match - Find physical connection to a device
* @dev: Device with the connection
@@ -23,10 +58,11 @@ static LIST_HEAD(devcon_list);
* caller is expecting to be returned.
*/
void *device_connection_find_match(struct device *dev, const char *con_id,
- void *data,
- void *(*match)(struct device_connection *con,
- int ep, void *data))
+ void *data,
+ void *(*match)(struct device_connection *con,
+ int ep, void *data))
{
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
const char *devname = dev_name(dev);
struct device_connection *con;
void *ret = NULL;
@@ -35,6 +71,12 @@ void *device_connection_find_match(struct device *dev, const
char *con_id,
if (!match)
return NULL;
+ if (fwnode) {
+ ret = fwnode_graph_devcon_match(fwnode, con_id, data, match);
+ if (ret)
+ return ret;
+ }
+
mutex_lock(&devcon_lock);
list_for_each_entry(con, &devcon_list, list) {
--
2.19.1
Re: [RFC PATCH 1/5] driver core: Add fwnode member to struct device_connection
On 10/24/18 8:05 AM, Heikki Krogerus wrote:
> This will prepare the device connection API for connections
> described in firmware.
>
> Signed-off-by: Heikki Krogerus
> ---
> include/linux/device.h | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 90224e75ade4..a964a0d614fa 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -753,11 +753,17 @@ struct device_dma_parameters {
>
> /**
> * struct device_connection - Device Connection Descriptor
> + * @fwnode: The device node of the connected device
> * @endpoint: The names of the two devices connected together
> * @id: Unique identifier for the connection
> * @list: List head, private, for internal use only
> + *
> + * NOTE: @fwnode is not used together with @endpoint. @fwnode is used when
> + * platform firmware defines the connection. When the connection is
> registeded
for your next version:)
registered
> + * with device_connection_add() @endpoint is used instead.
> */
> struct device_connection {
> + struct fwnode_handle*fwnode;
> const char *endpoint[2];
> const char *id;
> struct list_headlist;
>
cheers.
--
~Randy
Re: [RFC PATCH 4/5] usb: typec: Find the ports by also matching against the device node
Hello!
On 10/24/2018 06:05 PM, Heikki Krogerus wrote:
> When the connections are defined in firmware, struct
> device_connection will have the fwnode member pointing to
> the device node (struct fwnode_handle) of the requested
> device, and the endpoint will not be used at all in that
> case.
>
> Signed-off-by: Heikki Krogerus
> ---
> drivers/usb/typec/class.c | 19 ---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 5db0593ca0bd..fe6f3a932a88 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -204,15 +204,28 @@ static void typec_altmode_put_partner(struct altmode
> *altmode)
> put_device(&adev->dev);
> }
>
> -static int __typec_port_match(struct device *dev, const void *name)
> +static int typec_port_fwnode_match(struct device *dev, const void *fwnode)
> +{
> + return dev_name(fwnode) == fwnode;
dev_name(dev), maybe?
[...]
MBR, Sergei
Re: [PATCH 0/3] PM: Renesas: Remove dummy runtime PM callbacks
> At least the I2C driver part of this was on my todo list as well (just a > bit lower :/). I wanted to find out why they have been there in the > first place. Do you know if such callbacks were needed "back in the > days"? I see now that you referenced the relevant commits in the patch descriptions. Thanks! signature.asc Description: PGP signature
RE: [PATCH 3/3] usb: renesas_usbhs: Remove dummy runtime PM callbacks
Hi Jarkko,
> From: Jarkko Nikula, Sent: Wednesday, October 24, 2018 10:52 PM
>
> Platform drivers don't need dummy runtime PM callbacks that just return
> success in order to have runtime PM happening. This has changed since
> following commits:
>
> 05aa55dddb9e ("PM / Runtime: Lenient generic runtime pm callbacks")
> 543f2503a956 ("PM / platform_bus: Allow runtime PM by default")
> 8b313a38ecff ("PM / Platform: Use generic runtime PM callbacks directly")
>
> Signed-off-by: Jarkko Nikula
> ---
> Only build tested.
Thank you for the patch!
Reviewed-by: Yoshihiro Shimoda
> ---
> drivers/usb/renesas_usbhs/common.c | 14 --
> 1 file changed, 14 deletions(-)
>
> diff --git a/drivers/usb/renesas_usbhs/common.c
> b/drivers/usb/renesas_usbhs/common.c
> index a3e1290d682d..0e760f228dd8 100644
> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -874,23 +874,9 @@ static int usbhsc_resume(struct device *dev)
> return 0;
> }
>
> -static int usbhsc_runtime_nop(struct device *dev)
> -{
> - /* Runtime PM callback shared between ->runtime_suspend()
> - * and ->runtime_resume(). Simply returns success.
> - *
> - * This driver re-initializes all registers after
> - * pm_runtime_get_sync() anyway so there is no need
> - * to save and restore registers here.
> - */
I guess this code came from i2c-sh_mobile.c or sh_eth.c
and we didn't realize this code didn't need at that time (Oct 10 2011).
Best regards,
Yoshihiro Shimoda
> - return 0;
> -}
> -
> static const struct dev_pm_ops usbhsc_pm_ops = {
> .suspend= usbhsc_suspend,
> .resume = usbhsc_resume,
> - .runtime_suspend= usbhsc_runtime_nop,
> - .runtime_resume = usbhsc_runtime_nop,
> };
>
> static struct platform_driver renesas_usbhs_driver = {
> --
> 2.19.1
