diff mbox

[v2,2/3] musb: sunxi: Remove custom babble handling

Message ID 20160922111901.15337-2-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hans de Goede Sept. 22, 2016, 11:19 a.m. UTC
The musb-core now a days always treats babble errors in host mode
as disconnects, so there is no need for the sunxi specific handling
of this anymore.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
-This is a new patch in v2 of this patch series
---
 drivers/usb/musb/sunxi.c | 10 ----------
 1 file changed, 10 deletions(-)

Comments

Bin Liu Sept. 22, 2016, 1:54 p.m. UTC | #1
Hi,

On Thu, Sep 22, 2016 at 02:19:00PM +0300, Hans de Goede wrote:
> The musb-core now a days always treats babble errors in host mode

I don't think this statement is accurate. You might want to change it to
"The musb core already handles babble interrupt" or something else.

Regards,
-Bin.

> as disconnects, so there is no need for the sunxi specific handling
> of this anymore.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> Changes in v2:
> -This is a new patch in v2 of this patch series
> ---
>  drivers/usb/musb/sunxi.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index 1408245..82eba92 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -186,16 +186,6 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
>  	if (musb->int_usb)
>  		writeb(musb->int_usb, musb->mregs + SUNXI_MUSB_INTRUSB);
>  
> -	/*
> -	 * sunxi musb often signals babble on low / full speed device
> -	 * disconnect, without ever raising MUSB_INTR_DISCONNECT, since
> -	 * normally babble never happens treat it as disconnect.
> -	 */
> -	if ((musb->int_usb & MUSB_INTR_BABBLE) && is_host_active(musb)) {
> -		musb->int_usb &= ~MUSB_INTR_BABBLE;
> -		musb->int_usb |= MUSB_INTR_DISCONNECT;
> -	}
> -
>  	if ((musb->int_usb & MUSB_INTR_RESET) && !is_host_active(musb)) {
>  		/* ep0 FADDR must be 0 when (re)entering peripheral mode */
>  		musb_ep_select(musb->mregs, 0);
> -- 
> 2.9.3
>
Hans de Goede Sept. 22, 2016, 2:03 p.m. UTC | #2
Hi,

On 09/22/2016 04:54 PM, Bin Liu wrote:
> Hi,
>
> On Thu, Sep 22, 2016 at 02:19:00PM +0300, Hans de Goede wrote:
>> The musb-core now a days always treats babble errors in host mode
>
> I don't think this statement is accurate. You might want to change it to
> "The musb core already handles babble interrupt" or something else.

It is accurate if you look in the history at drivers/usb/musb
commits around 15-03-10 you will see 2 relevant commits:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/musb?id=b4dc38fd45b63e3da2bc98db5d283a15a637a2fa

"usb: musb: core: simplify musb_recover_work()"

This commits introduces calling musb_root_disconnect(musb)
on babble errors, that was not happening before which is why
I added the custom babble error handling. to the sunxi glue.

And:

"usb: musb: core: always try to recover from babble"

Where the title says it all.

Take these together and I believe that my commit msg:

"The musb-core now a days always treats babble errors in host mode
as disconnects, so there is no need for the sunxi specific handling
of this anymore."

Is quite accurate.

Regards,

Hans


>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> Changes in v2:
>> -This is a new patch in v2 of this patch series
>> ---
>>  drivers/usb/musb/sunxi.c | 10 ----------
>>  1 file changed, 10 deletions(-)
>>
>> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
>> index 1408245..82eba92 100644
>> --- a/drivers/usb/musb/sunxi.c
>> +++ b/drivers/usb/musb/sunxi.c
>> @@ -186,16 +186,6 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
>>  	if (musb->int_usb)
>>  		writeb(musb->int_usb, musb->mregs + SUNXI_MUSB_INTRUSB);
>>
>> -	/*
>> -	 * sunxi musb often signals babble on low / full speed device
>> -	 * disconnect, without ever raising MUSB_INTR_DISCONNECT, since
>> -	 * normally babble never happens treat it as disconnect.
>> -	 */
>> -	if ((musb->int_usb & MUSB_INTR_BABBLE) && is_host_active(musb)) {
>> -		musb->int_usb &= ~MUSB_INTR_BABBLE;
>> -		musb->int_usb |= MUSB_INTR_DISCONNECT;
>> -	}
>> -
>>  	if ((musb->int_usb & MUSB_INTR_RESET) && !is_host_active(musb)) {
>>  		/* ep0 FADDR must be 0 when (re)entering peripheral mode */
>>  		musb_ep_select(musb->mregs, 0);
>> --
>> 2.9.3
>>
Bin Liu Sept. 22, 2016, 2:30 p.m. UTC | #3
Hi,

On Thu, Sep 22, 2016 at 05:03:39PM +0300, Hans de Goede wrote:
> Hi,
> 
> On 09/22/2016 04:54 PM, Bin Liu wrote:
> >Hi,
> >
> >On Thu, Sep 22, 2016 at 02:19:00PM +0300, Hans de Goede wrote:
> >>The musb-core now a days always treats babble errors in host mode
> >
> >I don't think this statement is accurate. You might want to change it to
> >"The musb core already handles babble interrupt" or something else.
> 
> It is accurate if you look in the history at drivers/usb/musb
> commits around 15-03-10 you will see 2 relevant commits:
> 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/musb?id=b4dc38fd45b63e3da2bc98db5d283a15a637a2fa
> 
> "usb: musb: core: simplify musb_recover_work()"
> 
> This commits introduces calling musb_root_disconnect(musb)
> on babble errors, that was not happening before which is why

That is true, but calling musb_root_disconnect() is just one step of the
recovery in musb core, not all.

The statement of "treats babble errors in host mode as disconnects"
implies all the babble handling is just disconnect, which is not
accurate.

BTY, "babble errors in host mode" is also redundant. babble implies
host mode.

Regards,
-Bin.
> I added the custom babble error handling. to the sunxi glue.
> 
> And:
> 
> "usb: musb: core: always try to recover from babble"
> 
> Where the title says it all.
> 
> Take these together and I believe that my commit msg:
> 
> "The musb-core now a days always treats babble errors in host mode
> as disconnects, so there is no need for the sunxi specific handling
> of this anymore."
> 
> Is quite accurate.
> 
> Regards,
> 
> Hans
> 
> 
> >>
> >>Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >>---
> >>Changes in v2:
> >>-This is a new patch in v2 of this patch series
> >>---
> >> drivers/usb/musb/sunxi.c | 10 ----------
> >> 1 file changed, 10 deletions(-)
> >>
> >>diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> >>index 1408245..82eba92 100644
> >>--- a/drivers/usb/musb/sunxi.c
> >>+++ b/drivers/usb/musb/sunxi.c
> >>@@ -186,16 +186,6 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
> >> 	if (musb->int_usb)
> >> 		writeb(musb->int_usb, musb->mregs + SUNXI_MUSB_INTRUSB);
> >>
> >>-	/*
> >>-	 * sunxi musb often signals babble on low / full speed device
> >>-	 * disconnect, without ever raising MUSB_INTR_DISCONNECT, since
> >>-	 * normally babble never happens treat it as disconnect.
> >>-	 */
> >>-	if ((musb->int_usb & MUSB_INTR_BABBLE) && is_host_active(musb)) {
> >>-		musb->int_usb &= ~MUSB_INTR_BABBLE;
> >>-		musb->int_usb |= MUSB_INTR_DISCONNECT;
> >>-	}
> >>-
> >> 	if ((musb->int_usb & MUSB_INTR_RESET) && !is_host_active(musb)) {
> >> 		/* ep0 FADDR must be 0 when (re)entering peripheral mode */
> >> 		musb_ep_select(musb->mregs, 0);
> >>--
> >>2.9.3
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede Sept. 23, 2016, 1:42 p.m. UTC | #4
Hi,

On 09/22/2016 05:30 PM, Bin Liu wrote:
> Hi,
>
> On Thu, Sep 22, 2016 at 05:03:39PM +0300, Hans de Goede wrote:
>> Hi,
>>
>> On 09/22/2016 04:54 PM, Bin Liu wrote:
>>> Hi,
>>>
>>> On Thu, Sep 22, 2016 at 02:19:00PM +0300, Hans de Goede wrote:
>>>> The musb-core now a days always treats babble errors in host mode
>>>
>>> I don't think this statement is accurate. You might want to change it to
>>> "The musb core already handles babble interrupt" or something else.
>>
>> It is accurate if you look in the history at drivers/usb/musb
>> commits around 15-03-10 you will see 2 relevant commits:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/musb?id=b4dc38fd45b63e3da2bc98db5d283a15a637a2fa
>>
>> "usb: musb: core: simplify musb_recover_work()"
>>
>> This commits introduces calling musb_root_disconnect(musb)
>> on babble errors, that was not happening before which is why
>
> That is true, but calling musb_root_disconnect() is just one step of the
> recovery in musb core, not all.
>
> The statement of "treats babble errors in host mode as disconnects"
> implies all the babble handling is just disconnect, which is not
> accurate.

Ok, I'll send out a v3 with an improved commit msg.

> BTY, "babble errors in host mode" is also redundant. babble implies
> host mode.

Regards,

Hans




>
> Regards,
> -Bin.
>> I added the custom babble error handling. to the sunxi glue.
>>
>> And:
>>
>> "usb: musb: core: always try to recover from babble"
>>
>> Where the title says it all.
>>
>> Take these together and I believe that my commit msg:
>>
>> "The musb-core now a days always treats babble errors in host mode
>> as disconnects, so there is no need for the sunxi specific handling
>> of this anymore."
>>
>> Is quite accurate.
>>
>> Regards,
>>
>> Hans
>>
>>
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>> Changes in v2:
>>>> -This is a new patch in v2 of this patch series
>>>> ---
>>>> drivers/usb/musb/sunxi.c | 10 ----------
>>>> 1 file changed, 10 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
>>>> index 1408245..82eba92 100644
>>>> --- a/drivers/usb/musb/sunxi.c
>>>> +++ b/drivers/usb/musb/sunxi.c
>>>> @@ -186,16 +186,6 @@ static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
>>>> 	if (musb->int_usb)
>>>> 		writeb(musb->int_usb, musb->mregs + SUNXI_MUSB_INTRUSB);
>>>>
>>>> -	/*
>>>> -	 * sunxi musb often signals babble on low / full speed device
>>>> -	 * disconnect, without ever raising MUSB_INTR_DISCONNECT, since
>>>> -	 * normally babble never happens treat it as disconnect.
>>>> -	 */
>>>> -	if ((musb->int_usb & MUSB_INTR_BABBLE) && is_host_active(musb)) {
>>>> -		musb->int_usb &= ~MUSB_INTR_BABBLE;
>>>> -		musb->int_usb |= MUSB_INTR_DISCONNECT;
>>>> -	}
>>>> -
>>>> 	if ((musb->int_usb & MUSB_INTR_RESET) && !is_host_active(musb)) {
>>>> 		/* ep0 FADDR must be 0 when (re)entering peripheral mode */
>>>> 		musb_ep_select(musb->mregs, 0);
>>>> --
>>>> 2.9.3
>>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bin Liu Sept. 23, 2016, 1:47 p.m. UTC | #5
On Fri, Sep 23, 2016 at 04:42:26PM +0300, Hans de Goede wrote:
> Hi,
> 
> On 09/22/2016 05:30 PM, Bin Liu wrote:
> >Hi,
> >
> >On Thu, Sep 22, 2016 at 05:03:39PM +0300, Hans de Goede wrote:
> >>Hi,
> >>
> >>On 09/22/2016 04:54 PM, Bin Liu wrote:
> >>>Hi,
> >>>
> >>>On Thu, Sep 22, 2016 at 02:19:00PM +0300, Hans de Goede wrote:
> >>>>The musb-core now a days always treats babble errors in host mode
> >>>
> >>>I don't think this statement is accurate. You might want to change it to
> >>>"The musb core already handles babble interrupt" or something else.
> >>
> >>It is accurate if you look in the history at drivers/usb/musb
> >>commits around 15-03-10 you will see 2 relevant commits:
> >>
> >>https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/musb?id=b4dc38fd45b63e3da2bc98db5d283a15a637a2fa
> >>
> >>"usb: musb: core: simplify musb_recover_work()"
> >>
> >>This commits introduces calling musb_root_disconnect(musb)
> >>on babble errors, that was not happening before which is why
> >
> >That is true, but calling musb_root_disconnect() is just one step of the
> >recovery in musb core, not all.
> >
> >The statement of "treats babble errors in host mode as disconnects"
> >implies all the babble handling is just disconnect, which is not
> >accurate.
> 
> Ok, I'll send out a v3 with an improved commit msg.

Thanks.

If Kishon gives his Acked-by, I will take all the 3 patches. Or
I can just take the 2 musb patches into my tree.

Regards,
-Bin.
diff mbox

Patch

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 1408245..82eba92 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -186,16 +186,6 @@  static irqreturn_t sunxi_musb_interrupt(int irq, void *__hci)
 	if (musb->int_usb)
 		writeb(musb->int_usb, musb->mregs + SUNXI_MUSB_INTRUSB);
 
-	/*
-	 * sunxi musb often signals babble on low / full speed device
-	 * disconnect, without ever raising MUSB_INTR_DISCONNECT, since
-	 * normally babble never happens treat it as disconnect.
-	 */
-	if ((musb->int_usb & MUSB_INTR_BABBLE) && is_host_active(musb)) {
-		musb->int_usb &= ~MUSB_INTR_BABBLE;
-		musb->int_usb |= MUSB_INTR_DISCONNECT;
-	}
-
 	if ((musb->int_usb & MUSB_INTR_RESET) && !is_host_active(musb)) {
 		/* ep0 FADDR must be 0 when (re)entering peripheral mode */
 		musb_ep_select(musb->mregs, 0);