lkml.org 
[lkml]   [2016]   [Sep]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v4 1/3] usb: chipidea: imx: Change switch order
From
Date
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
>>
>

\
 
 \ /
  Last update: 2016-09-22 23:58    [W:1.566 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site