On Fri, Mar 14, 2014 at 03:13:19PM +0800, Li Jun wrote: > From: Li Jun <b47624@xxxxxxxxxxxxx> > > Use a more general way to read and write otgsc register. > > Signed-off-by: Li Jun <b47624@xxxxxxxxxxxxx> > --- > > Changes for v3: > Remove special revert write for bits in OTGSC_INT_STATUS_BITS, and > rename 2 API function names and parameters. > > Changes for v2: > Use one API for otgsc regiter write, and add mask as input parameter > for otgsc register target bits read. > > drivers/usb/chipidea/core.c | 16 ++++++++++------ > drivers/usb/chipidea/otg.c | 33 ++++++++++++++++++++++++--------- > drivers/usb/chipidea/otg.h | 18 ++---------------- > drivers/usb/chipidea/udc.c | 17 ++++++++++------- > 4 files changed, 46 insertions(+), 38 deletions(-) > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > index ca6831c..e5ae3a5 100644 > --- a/drivers/usb/chipidea/core.c > +++ b/drivers/usb/chipidea/core.c > @@ -360,7 +360,7 @@ static irqreturn_t ci_irq(int irq, void *data) > u32 otgsc = 0; > > if (ci->is_otg) > - otgsc = hw_read(ci, OP_OTGSC, ~0); > + otgsc = hw_read_otgsc(ci, ~0); > > /* > * Handle id change interrupt, it indicates device/host function > @@ -368,7 +368,8 @@ static irqreturn_t ci_irq(int irq, void *data) > */ > if (ci->is_otg && (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS)) { > ci->id_event = true; > - ci_clear_otg_interrupt(ci, OTGSC_IDIS); > + /* Clear ID change irq status */ > + hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS); > disable_irq_nosync(ci->irq); > queue_work(ci->wq, &ci->work); > return IRQ_HANDLED; > @@ -380,7 +381,8 @@ static irqreturn_t ci_irq(int irq, void *data) > */ > if (ci->is_otg && (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS)) { > ci->b_sess_valid_event = true; > - ci_clear_otg_interrupt(ci, OTGSC_BSVIS); > + /* Clear BSV irq */ > + hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS); > disable_irq_nosync(ci->irq); > queue_work(ci->wq, &ci->work); > return IRQ_HANDLED; > @@ -502,8 +504,9 @@ static void ci_get_otg_capable(struct ci_hdrc *ci) > == (DCCPARAMS_DC | DCCPARAMS_HC)); > if (ci->is_otg) { > dev_dbg(ci->dev, "It is OTG capable controller\n"); > - ci_disable_otg_interrupt(ci, OTGSC_INT_EN_BITS); > - ci_clear_otg_interrupt(ci, OTGSC_INT_STATUS_BITS); > + /* Disable and clear all OTG irq */ > + hw_write_otgsc(ci, OTGSC_INT_EN_BITS | OTGSC_INT_STATUS_BITS, > + OTGSC_INT_STATUS_BITS); > } > } > > @@ -617,7 +620,8 @@ static int ci_hdrc_probe(struct platform_device *pdev) > */ > mdelay(2); > ci->role = ci_otg_role(ci); > - ci_enable_otg_interrupt(ci, OTGSC_IDIE); > + /* Enable ID change irq */ > + hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE); > } else { > /* > * If the controller is not OTG capable, but support > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c > index 39bd7ec..c694340 100644 > --- a/drivers/usb/chipidea/otg.c > +++ b/drivers/usb/chipidea/otg.c > @@ -24,13 +24,31 @@ > #include "otg.h" > > /** > + * hw_read_otgsc returns otgsc register bits value. > + * @mask: bitfield mask > + */ > +u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask) > +{ > + return hw_read(ci, OP_OTGSC, mask); > +} > + > +/** > + * hw_write_otgsc updates target bits of OTGSC register. > + * @mask: bitfield mask > + * @data: to be written > + */ > +void hw_write_otgsc(struct ci_hdrc *ci, u32 mask, u32 data) > +{ > + hw_write(ci, OP_OTGSC, mask | OTGSC_INT_STATUS_BITS, data); > +} > + > +/** > * ci_otg_role - pick role based on ID pin state > * @ci: the controller > */ > enum ci_role ci_otg_role(struct ci_hdrc *ci) > { > - u32 sts = hw_read(ci, OP_OTGSC, ~0); > - enum ci_role role = sts & OTGSC_ID > + enum ci_role role = hw_read_otgsc(ci, OTGSC_ID) > ? CI_ROLE_GADGET > : CI_ROLE_HOST; > > @@ -39,14 +57,10 @@ enum ci_role ci_otg_role(struct ci_hdrc *ci) > > void ci_handle_vbus_change(struct ci_hdrc *ci) > { > - u32 otgsc; > - > if (!ci->is_otg) > return; > > - otgsc = hw_read(ci, OP_OTGSC, ~0); > - > - if (otgsc & OTGSC_BSV) > + if (hw_read_otgsc(ci, OTGSC_BSV)) > usb_gadget_vbus_connect(&ci->gadget); > else > usb_gadget_vbus_disconnect(&ci->gadget); > @@ -115,6 +129,7 @@ void ci_hdrc_otg_destroy(struct ci_hdrc *ci) > flush_workqueue(ci->wq); > destroy_workqueue(ci->wq); > } > - ci_disable_otg_interrupt(ci, OTGSC_INT_EN_BITS); > - ci_clear_otg_interrupt(ci, OTGSC_INT_STATUS_BITS); > + /* Disable all OTG irq and clear status */ > + hw_write_otgsc(ci, OTGSC_INT_EN_BITS | OTGSC_INT_STATUS_BITS, > + OTGSC_INT_STATUS_BITS); > } > diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h > index 449bee0..7349267 100644 > --- a/drivers/usb/chipidea/otg.h > +++ b/drivers/usb/chipidea/otg.h > @@ -11,22 +11,8 @@ > #ifndef __DRIVERS_USB_CHIPIDEA_OTG_H > #define __DRIVERS_USB_CHIPIDEA_OTG_H > > -static inline void ci_clear_otg_interrupt(struct ci_hdrc *ci, u32 bits) > -{ > - /* Only clear request bits */ > - hw_write(ci, OP_OTGSC, OTGSC_INT_STATUS_BITS, bits); > -} > - > -static inline void ci_enable_otg_interrupt(struct ci_hdrc *ci, u32 bits) > -{ > - hw_write(ci, OP_OTGSC, bits | OTGSC_INT_STATUS_BITS, bits); > -} > - > -static inline void ci_disable_otg_interrupt(struct ci_hdrc *ci, u32 bits) > -{ > - hw_write(ci, OP_OTGSC, bits | OTGSC_INT_STATUS_BITS, 0); > -} > - > +u32 hw_read_otgsc(struct ci_hdrc *ci, u32 mask); > +void hw_write_otgsc(struct ci_hdrc *ci, u32 mask, u32 data); > int ci_hdrc_otg_init(struct ci_hdrc *ci); > void ci_hdrc_otg_destroy(struct ci_hdrc *ci); > enum ci_role ci_otg_role(struct ci_hdrc *ci); > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > index 7739c64..f91ec6a 100644 > --- a/drivers/usb/chipidea/udc.c > +++ b/drivers/usb/chipidea/udc.c > @@ -1844,8 +1844,10 @@ void ci_hdrc_gadget_destroy(struct ci_hdrc *ci) > static int udc_id_switch_for_device(struct ci_hdrc *ci) > { > if (ci->is_otg) { > - ci_clear_otg_interrupt(ci, OTGSC_BSVIS); > - ci_enable_otg_interrupt(ci, OTGSC_BSVIE); > + /* Clear BSV irq */ > + hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS); > + /* Enable BSV irq */ > + hw_write_otgsc(ci, OTGSC_BSVIE, OTGSC_BSVIE); You can use one line of code to write both OTGSC_BSVIS and OTGSC_BSVIE, others are OK. Besides, send all your three patches at v5 patchset, then, I can queue them together. > } > > return 0; > @@ -1853,11 +1855,12 @@ static int udc_id_switch_for_device(struct ci_hdrc *ci) > > static void udc_id_switch_for_host(struct ci_hdrc *ci) > { > - if (ci->is_otg) { > - /* host doesn't care B_SESSION_VALID event */ > - ci_clear_otg_interrupt(ci, OTGSC_BSVIS); > - ci_disable_otg_interrupt(ci, OTGSC_BSVIE); > - } > + /* > + * host doesn't care B_SESSION_VALID event > + * so clear and disbale BSV irq > + */ > + if (ci->is_otg) > + hw_write_otgsc(ci, OTGSC_BSVIE | OTGSC_BSVIS, OTGSC_BSVIS); > } > > /** > -- > 1.7.9.5 > > -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html