Messages in this thread | | | Subject | Re: [PATCH v4 1/3] usb: chipidea: imx: Change switch order | From | Fabien Lahoudere <> | Date | Wed, 21 Sep 2016 09:51:16 +0200 |
| |
Hi,
On 21/09/16 08:57, Peter Chen wrote: > On Mon, Sep 19, 2016 at 01:45:38PM +0200, Fabien Lahoudere wrote: >> Each USB controller have different behaviour, so in order to avoid to have >> several "swicth(data->index)" and lock/unlock, we prefer to get the index >> switch and then test for features if they exist for this index. >> This patch also remove useless test of reg and val. Those two values cannot >> be NULL. > > Sorry, I can't see the benefits of this change. > Considering certain controller doesn't have oc feature, with current > code it only needs one comparison (flag disable_oc), but with your > changes, it needs two comparisons.
The benefits are visible with next patches only but you ask me to split it, so I do.
Thanks
Fabien
> > Peter >> >> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk> >> --- >> drivers/usb/chipidea/usbmisc_imx.c | 38 ++++++++++++++++++++++++-------------- >> 1 file changed, 24 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c >> index 20d02a5..9549821 100644 >> --- a/drivers/usb/chipidea/usbmisc_imx.c >> +++ b/drivers/usb/chipidea/usbmisc_imx.c >> @@ -199,31 +199,41 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data) >> val |= MX53_USB_PLL_DIV_24_MHZ; >> writel(val, usbmisc->base + MX53_USB_OTG_PHY_CTRL_1_OFFSET); >> >> - if (data->disable_oc) { >> - spin_lock_irqsave(&usbmisc->lock, flags); >> - switch (data->index) { >> - case 0: >> + spin_lock_irqsave(&usbmisc->lock, flags); >> + >> + switch (data->index) { >> + case 0: >> + if (data->disable_oc) { >> reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; >> val = readl(reg) | MX53_BM_OVER_CUR_DIS_OTG; >> - break; >> - case 1: >> + writel(val, reg); >> + } >> + break; >> + case 1: >> + if (data->disable_oc) { >> reg = usbmisc->base + MX53_USB_OTG_PHY_CTRL_0_OFFSET; >> val = readl(reg) | MX53_BM_OVER_CUR_DIS_H1; >> - break; >> - case 2: >> + writel(val, reg); >> + } >> + break; >> + case 2: >> + if (data->disable_oc) { >> reg = usbmisc->base + MX53_USB_UH2_CTRL_OFFSET; >> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; >> - break; >> - case 3: >> + writel(val, reg); >> + } >> + break; >> + case 3: >> + if (data->disable_oc) { >> reg = usbmisc->base + MX53_USB_UH3_CTRL_OFFSET; >> val = readl(reg) | MX53_BM_OVER_CUR_DIS_UHx; >> - break; >> - } >> - if (reg && val) >> writel(val, reg); >> - spin_unlock_irqrestore(&usbmisc->lock, flags); >> + } >> + break; >> } >> >> + spin_unlock_irqrestore(&usbmisc->lock, flags); >> + >> return 0; >> } >> >> -- >> 2.1.4 >> >
| |