Hi Krzysztof, On 20 January 2016 at 04:59, Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx> wrote: > On 20.01.2016 00:04, Anand Moon wrote: >> Hi Krzysztof, >> >> On 18 January 2016 at 09:58, Krzysztof Kozlowski >>>> Already within function pwm_samsung_set_invert is protected by >>>> spin_lock_irqsave(&samsung_pwm_lock, flags); >>>> >>>> So no need to introduce another lock to control pwm_samsung_set_polarity. >>>> >>>> Best Regards. >>>> -Anand Moon >>> >>> I don't have any clue what is your point here. I don't get what >>> pwm_samsung_set_polarity has to do with main pwm core... >>> >>> Sorry, you need to be more specific. >>> >>> Best regards, >>> Krzysztof >>> >>> >> >> Below is the mapping of calls from pwm driver. >> I have tried to map the functionality and I am trying to understand >> the flow of the driver. >> >> Also looking in document >> >> https://www.kernel.org/doc/Documentation/pwm.txt >> >> pwm-samsung driver controls the LEDS, fans...etc >> >> Form the dts modes pwmleds >> >> pwmleds { >> compatible = "pwm-leds"; >> >> blueled { >> label = "blue:heartbeat"; >> pwms = <&pwm 2 2000000 0>; >> pwm-names = "pwm2"; >> max_brightness = <255>; >> linux,default-trigger = "heartbeat"; >> }; >> }; >> >> Following is the map out from the device tree. >> >> pwms = <&pwm 2 2000000 0>; >> >> &pwm -> pwm: pwm@12dd0000 --->samsung,exynos4210-pwm >> 2 -> period >> 2000000 -> duty_cycle >> 0 -> polarity > > I do not see any relations between DTS and the problem. > >> >> And here is the mapping of the call of function >> Note: This function call are as per my understanding of the flow in >> the driver. I might be wrong. >> >> pwm_samsung_set_polarity(struct pwm_chip *chip, struct pwm_device >> *pwm, enum pwm_polarity polarity) >> \ >> pwm_samsung_set_invert(our_chip, pwm->hwpwm, invert); >> \ >> pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity) > > No, pwm_samsung_set_invert does not call pwm_set_polarity(). This would > result in a circular call - back to pwm_samsung_set_polarity(). > >> \ >> pwm->chip->ops->set_polarity(pwm->chip, pwm, polarity); >> \ >> pwm_enable(struct pwm_device *pwm) or pwm_disable(struct pwm_device *pwm) >> >> pwm_enable or pwm_disable will be triggered on change in pwm->flags by >> the pwm core. >> before pwm_set_polarity is called form the Samsung driver it hold with >> following locks >> >> Here is the locking >> >> pwm_samsung_set_polarity(struct pwm_chip *chip, struct pwm_device >> *pwm, enum pwm_polarity polarity) >> \ >> pwm_samsung_set_invert(struct samsung_pwm_chip *chip, unsigned int >> channel, bool invert) >> \ >> spin_lock_irqsave(&samsung_pwm_lock, flags); >> \ >> pwm_set_polarity(struct pwm_device *pwm, enum pwm_polarity polarity) >> \ >> mutex_lock(&pwm->lock) >> >> pwm_enable(struct pwm_device *pwm) or pwm_disable(struct >> pwm_device *pwm) >> \ >> mutex_lock(&pwm->lock); >> >> Problem I see that we are holding the lock in interrupt context. >> I don't know how the this triggers this bug. >> >> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97 > > So leave it. If your flow of calls was correct, you would spot the > problem. But actually it does not matter - I think the flow is not correct. > > Best regards, > Krzysztof Yep the flow might be wrong. Ok thanks for your input. Best Regards. -Anand Moon -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |