| Message ID | 20200928122741.17884-6-pawell@cadence.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduced new Cadence USBSSP DRD Driver. | expand |
On 20-09-28 14:27:38, Pawel Laszczak wrote: > Patch changes the type for gadget_dev pointer in cdns structure from > pointer to cdns3_device structure to void pointer. > This filed is in reusable code and after this change it will be used to > point to both cdns3_device or cdnsp_device objects. > > Signed-off-by: Pawel Laszczak <pawell@cadence.com> > --- > drivers/usb/cdns3/core.h | 4 ++-- > drivers/usb/cdns3/gadget.c | 6 ++++++ > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h > index 0c6e14683b36..267923904a37 100644 > --- a/drivers/usb/cdns3/core.h > +++ b/drivers/usb/cdns3/core.h > @@ -55,7 +55,7 @@ struct cdns_role_driver { > * @roles: array of supported roles for this controller > * @role: current role > * @host_dev: the child host device pointer for cdns core > - * @gadget_dev: the child gadget device pointer for cdns3 core > + * @gadget_dev: the child gadget device pointer > * @usb2_phy: pointer to USB2 PHY > * @usb3_phy: pointer to USB3 PHY > * @mutex: the mutex for concurrent code at driver > @@ -87,7 +87,7 @@ struct cdns { > struct cdns_role_driver *roles[USB_ROLE_DEVICE + 1]; > enum usb_role role; > struct platform_device *host_dev; > - struct cdns3_device *gadget_dev; > + void *gadget_dev; > struct phy *usb2_phy; > struct phy *usb3_phy; > /* mutext used in workqueue*/ > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > index 26f68182e65e..f9cbc84bbfb8 100644 > --- a/drivers/usb/cdns3/gadget.c > +++ b/drivers/usb/cdns3/gadget.c > @@ -3177,7 +3177,9 @@ static int __cdns3_gadget_init(struct cdns *cdns) > static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) > { > struct cdns3_device *priv_dev = cdns->gadget_dev; > + unsigned long flags; > > + spin_lock_irqsave(&priv_dev->lock, flags); unrelated changes? > cdns3_disconnect_gadget(priv_dev); > > priv_dev->gadget.speed = USB_SPEED_UNKNOWN; > @@ -3186,6 +3188,7 @@ static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) > > /* disable interrupt for device */ > writel(0, &priv_dev->regs->usb_ien); > + spin_unlock_irqrestore(&priv_dev->lock, flags); > > return 0; > } > @@ -3193,11 +3196,14 @@ static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) > static int cdns3_gadget_resume(struct cdns *cdns, bool hibernated) > { > struct cdns3_device *priv_dev = cdns->gadget_dev; > + unsigned long flags; > > if (!priv_dev->gadget_driver) > return 0; > > + spin_lock_irqsave(&priv_dev->lock, flags); ditto > cdns3_gadget_config(priv_dev); > + spin_unlock_irqrestore(&priv_dev->lock, flags); > > return 0; > } > -- > 2.17.1 > -- Thanks, Peter Chen
> >On 20-09-28 14:27:38, Pawel Laszczak wrote: >> Patch changes the type for gadget_dev pointer in cdns structure from >> pointer to cdns3_device structure to void pointer. >> This filed is in reusable code and after this change it will be used to >> point to both cdns3_device or cdnsp_device objects. >> >> Signed-off-by: Pawel Laszczak <pawell@cadence.com> >> --- >> drivers/usb/cdns3/core.h | 4 ++-- >> drivers/usb/cdns3/gadget.c | 6 ++++++ >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h >> index 0c6e14683b36..267923904a37 100644 >> --- a/drivers/usb/cdns3/core.h >> +++ b/drivers/usb/cdns3/core.h >> @@ -55,7 +55,7 @@ struct cdns_role_driver { >> * @roles: array of supported roles for this controller >> * @role: current role >> * @host_dev: the child host device pointer for cdns core >> - * @gadget_dev: the child gadget device pointer for cdns3 core >> + * @gadget_dev: the child gadget device pointer >> * @usb2_phy: pointer to USB2 PHY >> * @usb3_phy: pointer to USB3 PHY >> * @mutex: the mutex for concurrent code at driver >> @@ -87,7 +87,7 @@ struct cdns { >> struct cdns_role_driver *roles[USB_ROLE_DEVICE + 1]; >> enum usb_role role; >> struct platform_device *host_dev; >> - struct cdns3_device *gadget_dev; >> + void *gadget_dev; >> struct phy *usb2_phy; >> struct phy *usb3_phy; >> /* mutext used in workqueue*/ >> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c >> index 26f68182e65e..f9cbc84bbfb8 100644 >> --- a/drivers/usb/cdns3/gadget.c >> +++ b/drivers/usb/cdns3/gadget.c >> @@ -3177,7 +3177,9 @@ static int __cdns3_gadget_init(struct cdns *cdns) >> static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) >> { >> struct cdns3_device *priv_dev = cdns->gadget_dev; >> + unsigned long flags; >> >> + spin_lock_irqsave(&priv_dev->lock, flags); > >unrelated changes? It's needed but this patch has missing removing this line from: cdns3_suspend/cdns3_resume from core.c file. It exists in patch 2 (usb: cdns3: Split core.c into cdns3-plat and core.c file). I will correct this patch. I had to move this spin_lock because in core.c we don't know the cdns->gadget_dev type (void * gadget_dev). Thanks, Pawel Laszczak > >> cdns3_disconnect_gadget(priv_dev); >> >> priv_dev->gadget.speed = USB_SPEED_UNKNOWN; >> @@ -3186,6 +3188,7 @@ static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) >> >> /* disable interrupt for device */ >> writel(0, &priv_dev->regs->usb_ien); >> + spin_unlock_irqrestore(&priv_dev->lock, flags); >> >> return 0; >> } >> @@ -3193,11 +3196,14 @@ static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) >> static int cdns3_gadget_resume(struct cdns *cdns, bool hibernated) >> { >> struct cdns3_device *priv_dev = cdns->gadget_dev; >> + unsigned long flags; >> >> if (!priv_dev->gadget_driver) >> return 0; >> >> + spin_lock_irqsave(&priv_dev->lock, flags); > >ditto > >> cdns3_gadget_config(priv_dev); >> + spin_unlock_irqrestore(&priv_dev->lock, flags); >> >> return 0; >> } >> -- >> 2.17.1 >> > >-- > >Thanks, >Peter Chen
>> >>On 20-09-28 14:27:38, Pawel Laszczak wrote: >>> Patch changes the type for gadget_dev pointer in cdns structure from >>> pointer to cdns3_device structure to void pointer. >>> This filed is in reusable code and after this change it will be used to >>> point to both cdns3_device or cdnsp_device objects. >>> >>> Signed-off-by: Pawel Laszczak <pawell@cadence.com> >>> --- >>> drivers/usb/cdns3/core.h | 4 ++-- >>> drivers/usb/cdns3/gadget.c | 6 ++++++ >>> 2 files changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h >>> index 0c6e14683b36..267923904a37 100644 >>> --- a/drivers/usb/cdns3/core.h >>> +++ b/drivers/usb/cdns3/core.h >>> @@ -55,7 +55,7 @@ struct cdns_role_driver { >>> * @roles: array of supported roles for this controller >>> * @role: current role >>> * @host_dev: the child host device pointer for cdns core >>> - * @gadget_dev: the child gadget device pointer for cdns3 core >>> + * @gadget_dev: the child gadget device pointer >>> * @usb2_phy: pointer to USB2 PHY >>> * @usb3_phy: pointer to USB3 PHY >>> * @mutex: the mutex for concurrent code at driver >>> @@ -87,7 +87,7 @@ struct cdns { >>> struct cdns_role_driver *roles[USB_ROLE_DEVICE + 1]; >>> enum usb_role role; >>> struct platform_device *host_dev; >>> - struct cdns3_device *gadget_dev; >>> + void *gadget_dev; >>> struct phy *usb2_phy; >>> struct phy *usb3_phy; >>> /* mutext used in workqueue*/ >>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c >>> index 26f68182e65e..f9cbc84bbfb8 100644 >>> --- a/drivers/usb/cdns3/gadget.c >>> +++ b/drivers/usb/cdns3/gadget.c >>> @@ -3177,7 +3177,9 @@ static int __cdns3_gadget_init(struct cdns *cdns) >>> static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) >>> { >>> struct cdns3_device *priv_dev = cdns->gadget_dev; >>> + unsigned long flags; >>> >>> + spin_lock_irqsave(&priv_dev->lock, flags); >> >>unrelated changes? > >It's needed but this patch has missing removing this line from: >cdns3_suspend/cdns3_resume from core.c file. >It exists in patch 2 (usb: cdns3: Split core.c into cdns3-plat and core.c file). >I will correct this patch. > >I had to move this spin_lock because in core.c we don't know the >cdns->gadget_dev type (void * gadget_dev). > You have right, after merging the latest patches from Felipe repository this changes are not need. Regards, Pawel, > >> >>> cdns3_disconnect_gadget(priv_dev); >>> >>> priv_dev->gadget.speed = USB_SPEED_UNKNOWN; >>> @@ -3186,6 +3188,7 @@ static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) >>> >>> /* disable interrupt for device */ >>> writel(0, &priv_dev->regs->usb_ien); >>> + spin_unlock_irqrestore(&priv_dev->lock, flags); >>> >>> return 0; >>> } >>> @@ -3193,11 +3196,14 @@ static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) >>> static int cdns3_gadget_resume(struct cdns *cdns, bool hibernated) >>> { >>> struct cdns3_device *priv_dev = cdns->gadget_dev; >>> + unsigned long flags; >>> >>> if (!priv_dev->gadget_driver) >>> return 0; >>> >>> + spin_lock_irqsave(&priv_dev->lock, flags); >> >>ditto >> >>> cdns3_gadget_config(priv_dev); >>> + spin_unlock_irqrestore(&priv_dev->lock, flags); >>> >>> return 0; >>> } >>> -- >>> 2.17.1 >>> >> >>-- >> >>Thanks, >>Peter Chen
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h index 0c6e14683b36..267923904a37 100644 --- a/drivers/usb/cdns3/core.h +++ b/drivers/usb/cdns3/core.h @@ -55,7 +55,7 @@ struct cdns_role_driver { * @roles: array of supported roles for this controller * @role: current role * @host_dev: the child host device pointer for cdns core - * @gadget_dev: the child gadget device pointer for cdns3 core + * @gadget_dev: the child gadget device pointer * @usb2_phy: pointer to USB2 PHY * @usb3_phy: pointer to USB3 PHY * @mutex: the mutex for concurrent code at driver @@ -87,7 +87,7 @@ struct cdns { struct cdns_role_driver *roles[USB_ROLE_DEVICE + 1]; enum usb_role role; struct platform_device *host_dev; - struct cdns3_device *gadget_dev; + void *gadget_dev; struct phy *usb2_phy; struct phy *usb3_phy; /* mutext used in workqueue*/ diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index 26f68182e65e..f9cbc84bbfb8 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -3177,7 +3177,9 @@ static int __cdns3_gadget_init(struct cdns *cdns) static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) { struct cdns3_device *priv_dev = cdns->gadget_dev; + unsigned long flags; + spin_lock_irqsave(&priv_dev->lock, flags); cdns3_disconnect_gadget(priv_dev); priv_dev->gadget.speed = USB_SPEED_UNKNOWN; @@ -3186,6 +3188,7 @@ static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) /* disable interrupt for device */ writel(0, &priv_dev->regs->usb_ien); + spin_unlock_irqrestore(&priv_dev->lock, flags); return 0; } @@ -3193,11 +3196,14 @@ static int cdns3_gadget_suspend(struct cdns *cdns, bool do_wakeup) static int cdns3_gadget_resume(struct cdns *cdns, bool hibernated) { struct cdns3_device *priv_dev = cdns->gadget_dev; + unsigned long flags; if (!priv_dev->gadget_driver) return 0; + spin_lock_irqsave(&priv_dev->lock, flags); cdns3_gadget_config(priv_dev); + spin_unlock_irqrestore(&priv_dev->lock, flags); return 0; }
Patch changes the type for gadget_dev pointer in cdns structure from pointer to cdns3_device structure to void pointer. This filed is in reusable code and after this change it will be used to point to both cdns3_device or cdnsp_device objects. Signed-off-by: Pawel Laszczak <pawell@cadence.com> --- drivers/usb/cdns3/core.h | 4 ++-- drivers/usb/cdns3/gadget.c | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-)