Hi Thierry, > From: Thierry Reding, Sent: Tuesday, March 12, 2019 9:46 PM > On Tue, Mar 12, 2019 at 12:16:34PM +0900, Yoshihiro Shimoda wrote: <snip> > > @@ -368,10 +370,12 @@ int pwmchip_remove(struct pwm_chip *chip) > > > > free_pwms(chip); > > > > - pwmchip_sysfs_unexport(chip); > > - > > out: > > mutex_unlock(&pwm_lock); > > + > > + if (!ret) > > + pwmchip_sysfs_unexport(chip); > > + > > I don't exactly remember why he sysfs unexport happens this late. It's > completely asymmetric to what we do in pwmchip_add_with_polarity() and > commit 0733424c9ba9 ("pwm: Unexport children before chip removal") is > a strong indication that this was wrong to begin with. Maybe we should > just move pwmchip_sysfs_unexport() where it belongs, which is right > after pwmchip_sysfs_unexport_children(). In that case, we probably do > not need separate functions anymore either.
I think so. So, I'll merge them on v2 patch. > We also really want to remove sysfs irrespective of whether or not the > chip will be removed as a result of pwmchip_remove(). We can only assume > that the driver will be gone after that, so we shouldn't leave any > dangling sysfs files around. I think so. > Yoshihiro, does it work if you move pwmchip_sysfs_unexport() to the top > of pwmchip_remove(), right below pwmchip_sysfs_unexport_children(), > instead? Does that get rid of the lockdep warning as well? That way it > is also outside of the pwm_lock section, which indeed doesn't seem to be > needed. It works. That gets rid of the lockdep warning as well. So, I'll submit v2 patch later. > Moving the pwmchip_sysfs_export() call outside of that section also > seems fine and it'd be perfectly symmetric with pwmchip_remove() again. I think so. Best regards, Yoshihiro Shimoda > Thierry