2020-01-26 17:11:03

by Saurav Girepunje

[permalink] [raw]
Subject: [PATCH] usb: musb: Fix external abort on non-linefetch

While accessing the usb mode/vbus from sysfs. Below error came on kernel
version 4.19.

On latest kernel vserion api name changed. Therefore API
name and backtrace API names are different.

[ 821.908066] Backtrace:
[ 821.910695] [<bf078fc0>] (musb_default_readl [musb_hdrc]) from [<bf0af738>] (dsps_musb_set_mode+0x38/0x12c [musb_dsps])
[ 821.922059] [<bf0af700>] (dsps_musb_set_mode [musb_dsps]) from [<bf07899c>] (musb_mode_store+0xc8/0x12c [musb_hdrc])
[ 821.933105] r7:a0010013 r6:0000000b r5:cd79d200 r4:cb634010
[ 821.939096] [<bf0788d4>] (musb_mode_store [musb_hdrc]) from [<c0425184>] (dev_attr_store+0x20/0x2c)
[ 821.948593] r7:cd79d200 r6:c5abbf78 r5:00000000 r4:bf0788d4
[ 821.954549] [<c0425164>] (dev_attr_store) from [<c0285b08>] (sysfs_kf_write+0x48/0x4c)
[ 821.962859] r5:00000000 r4:c0425164
[ 821.966620] [<c0285ac0>] (sysfs_kf_write) from [<c0285274>] (kernfs_fop_write+0xfc/0x1fc)
[ 821.975200] r5:00000000 r4:cd79d080
[ 821.978966] [<c0285178>] (kernfs_fop_write) from [<c020ec00>] (__vfs_write+0x34/0x120)
[ 821.987280] r10:00000000 r9:0000000b r8:00000000 r7:0000000b r6:c5abbf78 r5:c0285178
[ 821.995493] r4:cb67a3c0
[ 821.998160] [<c020ebcc>] (__vfs_write) from [<c020fae8>] (vfs_write+0xa8/0x170)
[ 822.005835] r9:0000000b r8:00000000 r7:c5abbf78 r6:000def80 r5:cb67a3c0 r4:0000000b
[ 822.013969] [<c020fa40>] (vfs_write) from [<c02108d0>] (SyS_write+0x44/0x98)
[ 822.021371] r9:0000000b r8:000def80 r7:00000000 r6:00000000 r5:cb67a3c0 r4:cb67a3c0
[ 822.029517] [<c021088c>] (SyS_write) from [<c010d8a0>] (ret_fast_syscall+0x0/0x3c)
[ 822.037467] r9:c5aba000 r8:c010daa8 r7:00000004 r6:b6f0ad58 r5:000def80 r4:0000000b
[ 822.045599] Code: e1a0c00d e92dd800 e24cb004 e0801001 (e5910000)

Without pm_runtime_{get,put}_sync calls in place,
Similar issue come on "mode_show" and "vbus_store" also.

fix- call pm_runtime_{get,put}_sync before reading/writing
usb mode/vbus from sysfs.

Signed-off-by: Saurav Girepunje <[email protected]>
---
drivers/usb/musb/musb_core.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 5ebf30b..a1ca8380 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1723,9 +1723,11 @@ int musb_mailbox(enum musb_vbus_id_status status)
unsigned long flags;
int ret;

+ pm_runtime_get_sync(dev);
spin_lock_irqsave(&musb->lock, flags);
ret = sprintf(buf, "%s\n", usb_otg_state_string(musb->xceiv->otg->state));
spin_unlock_irqrestore(&musb->lock, flags);
+ pm_runtime_put_sync(dev);

return ret;
}
@@ -1738,6 +1740,7 @@ int musb_mailbox(enum musb_vbus_id_status status)
unsigned long flags;
int status;

+ pm_runtime_get_sync(dev);
spin_lock_irqsave(&musb->lock, flags);
if (sysfs_streq(buf, "host"))
status = musb_platform_set_mode(musb, MUSB_HOST);
@@ -1748,6 +1751,7 @@ int musb_mailbox(enum musb_vbus_id_status status)
else
status = -EINVAL;
spin_unlock_irqrestore(&musb->lock, flags);
+ pm_runtime_put_sync(dev);

return (status == 0) ? n : status;
}
@@ -1766,6 +1770,7 @@ int musb_mailbox(enum musb_vbus_id_status status)
return -EINVAL;
}

+ pm_runtime_get_sync(dev);
spin_lock_irqsave(&musb->lock, flags);
/* force T(a_wait_bcon) to be zero/unlimited *OR* valid */
musb->a_wait_bcon = val ? max_t(int, val, OTG_TIME_A_WAIT_BCON) : 0 ;
@@ -1773,6 +1778,7 @@ int musb_mailbox(enum musb_vbus_id_status status)
musb->is_active = 0;
musb_platform_try_idle(musb, jiffies + msecs_to_jiffies(val));
spin_unlock_irqrestore(&musb->lock, flags);
+ pm_runtime_put_sync(dev);

return n;
}
--
1.9.1


2020-02-06 19:19:04

by Bin Liu

[permalink] [raw]
Subject: Re: [PATCH] usb: musb: Fix external abort on non-linefetch

Hi Saurav,

On Sun, Jan 19, 2020 at 11:34:04AM +0530, SAURAV GIREPUNJE wrote:
> Hi Bin,
>
> Yes, I really got this kernel dump for mode_show() and vbus_store(),
> While accessing show/store for mode and vbus through sysfs .

Please provide the kernel dump for mode_show() and vbus_store(). The log
below is for mode_store().

> I have submitted earlier also three patches for same bug, In reply to
> one of those patches you
> asked me to merge another two patches and just sent in one.

I don't have issue to merge all the fixes into one patch, since they fix
the same bug. But I need the dump log to see how the dump is triggered
in mode_show() and vbus_store() as I don't see the how it could happen
and I cannot reproduce it. These two function do not access any musb
register.

By the way, please don't top-posting, and reply to my message to
preserve the context.

-Bin.

>
> Regards,
> Saurav Girepunje
>
> On 27/10/19 14:06 +0530, Saurav Girepunje wrote:
> > While setting the usb mode from sysfs. Below error came on kernel
> > version 4.19.
> >
> > On latest kernel vserion api name changed. Therefore API
> > name and backtrace API name are different.
> >
> > [ 821.908066] Backtrace:
> > [ 821.910695] [<bf078fc0>] (musb_default_readl [musb_hdrc]) from [<bf0af738>] (dsps_musb_set_mode+0x38/0x12c [musb_dsps])
> > [ 821.922059] [<bf0af700>] (dsps_musb_set_mode [musb_dsps]) from [<bf07899c>] (musb_mode_store+0xc8/0x12c [musb_hdrc])
> > [ 821.933105] r7:a0010013 r6:0000000b r5:cd79d200 r4:cb634010
> > [ 821.939096] [<bf0788d4>] (musb_mode_store [musb_hdrc]) from [<c0425184>] (dev_attr_store+0x20/0x2c)
> > [ 821.948593] r7:cd79d200 r6:c5abbf78 r5:00000000 r4:bf0788d4
> > [ 821.954549] [<c0425164>] (dev_attr_store) from [<c0285b08>] (sysfs_kf_write+0x48/0x4c)
> > [ 821.962859] r5:00000000 r4:c0425164
> > [ 821.966620] [<c0285ac0>] (sysfs_kf_write) from [<c0285274>] (kernfs_fop_write+0xfc/0x1fc)
> > [ 821.975200] r5:00000000 r4:cd79d080
> > [ 821.978966] [<c0285178>] (kernfs_fop_write) from [<c020ec00>] (__vfs_write+0x34/0x120)
> > [ 821.987280] r10:00000000 r9:0000000b r8:00000000 r7:0000000b r6:c5abbf78 r5:c0285178
> > [ 821.995493] r4:cb67a3c0
> > [ 821.998160] [<c020ebcc>] (__vfs_write) from [<c020fae8>] (vfs_write+0xa8/0x170)
> > [ 822.005835] r9:0000000b r8:00000000 r7:c5abbf78 r6:000def80 r5:cb67a3c0 r4:0000000b
> > [ 822.013969] [<c020fa40>] (vfs_write) from [<c02108d0>] (SyS_write+0x44/0x98)
> > [ 822.021371] r9:0000000b r8:000def80 r7:00000000 r6:00000000 r5:cb67a3c0 r4:cb67a3c0
> > [ 822.029517] [<c021088c>] (SyS_write) from [<c010d8a0>] (ret_fast_syscall+0x0/0x3c)
> > [ 822.037467] r9:c5aba000 r8:c010daa8 r7:00000004 r6:b6f0ad58 r5:000def80 r4:0000000b
> > [ 822.045599] Code: e1a0c00d e92dd800 e24cb004 e0801001 (e5910000)
> >
> > Without pm_runtime_{get,put}_sync calls in place,
> > Similar issue come on "mode_show" and "vbus_store" also.
> >
> > fix- call pm_runtime_{get,put}_sync before reading/writing
> > usb mode/vbus from sysfs.
> >
> > As sugguested on commit '2d15f69ed5c1c33f283e77ec161578badde33eaf'
> > Merged all the fix which reslove same bug in the same file.
> > "
> > commit '2d15f69ed5c1c33f283e77ec161578badde33eaf'
> > Author: Saurav Girepunje <[email protected]>
> > Date: Thu Aug 8 00:05:03 2019 +0530
> >
> > usb: musb: Fix external abort on non-linefetch for vbus_store
> >
> > Without pm_runtime_{get,put}_sync calls in place, writing
> > vbus value via /sys causes the error
> >
> > "Unhandled fault external abort on non-linefetch"
> >
> > On Thu, Aug 08, 2019 at 12:11:46AM +0530, Saurav Girepunje wrote:
> > > Without pm_runtime_{get,put}_sync calls in place, writing
> > > vbus value via /sys causes the error
> > >
> > > "Unhandled fault external abort on non-linefetch"
> > >
> > > Signed-off-by: Saurav Girepunje <saurav.girepunje@xxxxxxxxx>
> >
> > Thanks for the patch. Can you please merge another two similar patches
> > you just sent with this one? They all fix the same bug in the same file
> > so could be just in one patch.
> >
> > -Bin.
> > "
> >
> > Signed-off-by: Saurav Girepunje <[email protected]>
> > ---
> > drivers/usb/musb/musb_core.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> > index bd63450af76a..dc7d786feb58 100644
> > --- a/drivers/usb/musb/musb_core.c
> > +++ b/drivers/usb/musb/musb_core.c
> > @@ -1723,9 +1723,11 @@ mode_show(struct device *dev, struct device_attribute *attr, char *buf)
> > unsigned long flags;
> > int ret;
> >
> > + pm_runtime_get_sync(dev);
> > spin_lock_irqsave(&musb->lock, flags);
> > ret = sprintf(buf, "%s\n", usb_otg_state_string(musb->xceiv->otg->state));
> > spin_unlock_irqrestore(&musb->lock, flags);
> > + pm_runtime_put_sync(dev);
> >
> > return ret;
> > }
> > @@ -1738,6 +1740,7 @@ mode_store(struct device *dev, struct device_attribute *attr,
> > unsigned long flags;
> > int status;
> >
> > + pm_runtime_get_sync(dev);
> > spin_lock_irqsave(&musb->lock, flags);
> > if (sysfs_streq(buf, "host"))
> > status = musb_platform_set_mode(musb, MUSB_HOST);
> > @@ -1748,6 +1751,7 @@ mode_store(struct device *dev, struct device_attribute *attr,
> > else
> > status = -EINVAL;
> > spin_unlock_irqrestore(&musb->lock, flags);
> > + pm_runtime_put_sync(dev);
> >
> > return (status == 0) ? n : status;
> > }
> > @@ -1766,6 +1770,7 @@ vbus_store(struct device *dev, struct device_attribute *attr,
> > return -EINVAL;
> > }
> >
> > + pm_runtime_get_sync(dev);
> > spin_lock_irqsave(&musb->lock, flags);
> > /* force T(a_wait_bcon) to be zero/unlimited *OR* valid */
> > musb->a_wait_bcon = val ? max_t(int, val, OTG_TIME_A_WAIT_BCON) : 0 ;
> > @@ -1773,6 +1778,7 @@ vbus_store(struct device *dev, struct device_attribute *attr,
> > musb->is_active = 0;
> > musb_platform_try_idle(musb, jiffies + msecs_to_jiffies(val));
> > spin_unlock_irqrestore(&musb->lock, flags);
> > + pm_runtime_put_sync(dev);
> >
> > return n;
> > }
> > --
> > 2.20.1
> >