| Message ID | 20200701062004.29908-7-pawell@cadence.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: cdns3: Improvements for cdns3 DRD code | expand |
On 20-07-01 08:20:01, Pawel Laszczak wrote: > Patch adds 2 definitions that make it easier to understand the code. > > Signed-off-by: Pawel Laszczak <pawell@cadence.com> > --- > drivers/usb/cdns3/drd.c | 4 ++-- > drivers/usb/cdns3/drd.h | 3 +++ > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c > index 6fe092c828b3..8e7673da905e 100644 > --- a/drivers/usb/cdns3/drd.c > +++ b/drivers/usb/cdns3/drd.c > @@ -87,7 +87,7 @@ bool cdns3_is_host(struct cdns3 *cdns) > { > if (cdns->dr_mode == USB_DR_MODE_HOST) > return true; > - else if (!cdns3_get_id(cdns)) > + else if (cdns3_get_id(cdns) == CDNS3_ID_HOST) > return true; > > return false; > @@ -98,7 +98,7 @@ bool cdns3_is_device(struct cdns3 *cdns) > if (cdns->dr_mode == USB_DR_MODE_PERIPHERAL) > return true; > else if (cdns->dr_mode == USB_DR_MODE_OTG) > - if (cdns3_get_id(cdns)) > + if (cdns3_get_id(cdns) == CDNS3_ID_PERIPHERAL) > return true; > > return false; > diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h > index 35b6d459ee58..3889fead9df1 100644 > --- a/drivers/usb/cdns3/drd.h > +++ b/drivers/usb/cdns3/drd.h > @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs { > /* Only for CDNS3_CONTROLLER_V0 version */ > #define OVERRIDE_IDPULLUP_V0 BIT(24) > > +#define CDNS3_ID_PERIPHERAL 1 > +#define CDNS3_ID_HOST 0 > + Instead of adding MACRO, I prefer adding comments at the code to indicate "ID=0" means it is host mode, "ID=1" means it is device mode.
On Tue, Jul 07, 2020 at 06:30:50AM +0000, Peter Chen wrote: > On 20-07-01 08:20:01, Pawel Laszczak wrote: > > Patch adds 2 definitions that make it easier to understand the code. > > > > Signed-off-by: Pawel Laszczak <pawell@cadence.com> > > --- > > drivers/usb/cdns3/drd.c | 4 ++-- > > drivers/usb/cdns3/drd.h | 3 +++ > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c > > index 6fe092c828b3..8e7673da905e 100644 > > --- a/drivers/usb/cdns3/drd.c > > +++ b/drivers/usb/cdns3/drd.c > > @@ -87,7 +87,7 @@ bool cdns3_is_host(struct cdns3 *cdns) > > { > > if (cdns->dr_mode == USB_DR_MODE_HOST) > > return true; > > - else if (!cdns3_get_id(cdns)) > > + else if (cdns3_get_id(cdns) == CDNS3_ID_HOST) > > return true; > > > > return false; > > @@ -98,7 +98,7 @@ bool cdns3_is_device(struct cdns3 *cdns) > > if (cdns->dr_mode == USB_DR_MODE_PERIPHERAL) > > return true; > > else if (cdns->dr_mode == USB_DR_MODE_OTG) > > - if (cdns3_get_id(cdns)) > > + if (cdns3_get_id(cdns) == CDNS3_ID_PERIPHERAL) > > return true; > > > > return false; > > diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h > > index 35b6d459ee58..3889fead9df1 100644 > > --- a/drivers/usb/cdns3/drd.h > > +++ b/drivers/usb/cdns3/drd.h > > @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs { > > /* Only for CDNS3_CONTROLLER_V0 version */ > > #define OVERRIDE_IDPULLUP_V0 BIT(24) > > > > +#define CDNS3_ID_PERIPHERAL 1 > > +#define CDNS3_ID_HOST 0 > > + > > Instead of adding MACRO, I prefer adding comments at the code to indicate > "ID=0" means it is host mode, "ID=1" means it is device mode. The comment can only be in one place but the macro can be used everywhere and is immediately readable. I suggested this patch, but now that I see it I'm still surprised how much I like it. regards, dan carpenter
diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c index 6fe092c828b3..8e7673da905e 100644 --- a/drivers/usb/cdns3/drd.c +++ b/drivers/usb/cdns3/drd.c @@ -87,7 +87,7 @@ bool cdns3_is_host(struct cdns3 *cdns) { if (cdns->dr_mode == USB_DR_MODE_HOST) return true; - else if (!cdns3_get_id(cdns)) + else if (cdns3_get_id(cdns) == CDNS3_ID_HOST) return true; return false; @@ -98,7 +98,7 @@ bool cdns3_is_device(struct cdns3 *cdns) if (cdns->dr_mode == USB_DR_MODE_PERIPHERAL) return true; else if (cdns->dr_mode == USB_DR_MODE_OTG) - if (cdns3_get_id(cdns)) + if (cdns3_get_id(cdns) == CDNS3_ID_PERIPHERAL) return true; return false; diff --git a/drivers/usb/cdns3/drd.h b/drivers/usb/cdns3/drd.h index 35b6d459ee58..3889fead9df1 100644 --- a/drivers/usb/cdns3/drd.h +++ b/drivers/usb/cdns3/drd.h @@ -153,6 +153,9 @@ struct cdns3_otg_common_regs { /* Only for CDNS3_CONTROLLER_V0 version */ #define OVERRIDE_IDPULLUP_V0 BIT(24) +#define CDNS3_ID_PERIPHERAL 1 +#define CDNS3_ID_HOST 0 + bool cdns3_is_host(struct cdns3 *cdns); bool cdns3_is_device(struct cdns3 *cdns); int cdns3_get_id(struct cdns3 *cdns);
Patch adds 2 definitions that make it easier to understand the code. Signed-off-by: Pawel Laszczak <pawell@cadence.com> --- drivers/usb/cdns3/drd.c | 4 ++-- drivers/usb/cdns3/drd.h | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-)