From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933375AbcASXaI (ORCPT ); Tue, 19 Jan 2016 18:30:08 -0500 Received: from mailout4.w1.samsung.com ([210.118.77.14]:28303 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932702AbcASXaD (ORCPT ); Tue, 19 Jan 2016 18:30:03 -0500 X-AuditID: cbfec7f4-f79026d00000418a-4b-569ec6f7bc07 Subject: Re: [PATCHv2] pwm: avoid holding mutex in interrupt context To: Anand Moon References: <1453064517-12315-1-git-send-email-linux.amoon@gmail.com> <569C2AE8.6070100@samsung.com> <569C69F4.7000906@samsung.com> Cc: Guenter Roeck , Thierry Reding , Javier Martinez Canillas , linux-pwm@vger.kernel.org, Linux Kernel , "linux-samsung-soc@vger.kernel.org" From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <569EC6F6.1020505@samsung.com> Date: Wed, 20 Jan 2016 08:29:58 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-version: 1.0 In-reply-to: Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrFLMWRmVeSWpSXmKPExsVy+t/xa7rfj80LM5izStLizds1TBavXxha XN41h83i7t1VjBYzzu9jsli38Ra7xZOFZ5gsfu6ax+LA4bFz1l12jy39QGLn9wZ2j74tqxg9 Pm+SC2CN4rJJSc3JLEst0rdL4Mq4dHAPS8FC6YoLU/4xNjD+FO5i5OSQEDCRaGpvYoWwxSQu 3FvP1sXIxSEksJRRomX/d3YI5ymjxIS7k4EcDg5hAReJ+xsTQBpEBNQkrjxdwQpRM5VJYv7E KWAOs8BcJolZbZuYQarYBIwlNi9fwgaxQk6it3sSC4jNK6Al8fnKZSYQm0VAVWJB71swW1Qg QuJwZxc7RI2gxI/J91hAFnMKBEsceVABYjILqEtMmZILUsEsIC+xec1b5gmMgrOQNMxCqJqF pGoBI/MqRtHU0uSC4qT0XEO94sTc4tK8dL3k/NxNjJAY+LKDcfExq0OMAhyMSjy8DM3zwoRY E8uKK3MPMUpwMCuJ8J7dAhTiTUmsrEotyo8vKs1JLT7EKM3BoiTOO3fX+xAhgfTEktTs1NSC 1CKYLBMHp1QDo1pYdndytV3mvyP5myR/fTksmfi5cNks3a+u+kXLg2ZP8PTyPjDHU2bWupke jcdPyTffK7k+u5H9TfTulCyjWHHPw48zt75XefrvqPsX+4u/l6j+v6p3/puWWZvSMdkZK2XX KXXHbc6QfNOa4SnxhqdL23SDY0LRDnPPrqqvcSqbH/m81p1ko8RSnJFoqMVcVJwIAPUelHZ9 AgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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