From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Jacek Anaszewski <j.anaszewski@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>,
MyungJoo Ham <myungjoo.ham@samsung.com>,
Chanwoo Choi <cw00.choi@samsung.com>,
Richard Purdie <rpurdie@rpsys.net>,
linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-leds@vger.kernel.org, linux-pm@vger.kernel.org,
r.baldyga@hackerion.com,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH v7 1/6] mfd: max8997: Use regmap to access registers
Date: Thu, 02 Jun 2016 06:43:19 +0200 [thread overview]
Message-ID: <574FB967.7090201@samsung.com> (raw)
In-Reply-To: <574EC8EE.5000509@samsung.com>
On 06/01/2016 01:37 PM, Jacek Anaszewski wrote:
> Hi Krzysztof,
>
> One thing drew my attention while reviewing this again:
> max8997_led_brightness_set() can sleep, but the brightness_set
> op it is assigned to must not sleep. At the time when this driver was
> merged we were delegating brightness setting to workqueues task
> in LED class drivers that can sleep during this call.
> This must have been overlooked, which is even more likely, taking into
> account that the initial patch doesn't have LED maintainer's ack.
>
> The non-sleeping requirement is motivated by the fact that brightness
> can be set from softirq context, e.g. when timer trigger is enabled.
>
> Currently LED class drivers don't have to use workqueue on their own,
> but are required to use brightness_set_blocking op instead of
> brightness_set if they can sleep while setting brightness.
>
> Apart of that, I think that operations in max8997_led_brightness_set()
> should be protected with mutex to assure leaving the device in
> a consistent state in case of concurrent calls.
>
> I am aware that this is out of this patch scope, but I'd be grateful
> if you could apply those changes and test them on hardware if you have
> an access to.
The problem you mention existed before the patch. It was using sleeping
primitives (mutex) before adding regmap so I understand you don't have
anything against this patch, right?
I can fix the issue but it will be a little bit trickier because I don't
have the hardware. Other guys in the team tested the patchset for me so
I rely on them in that matter. Anyway I'll work on it.
Best regards,
Krzysztof
WARNING: multiple messages have this Message-ID (diff)
From: k.kozlowski@samsung.com (Krzysztof Kozlowski)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v7 1/6] mfd: max8997: Use regmap to access registers
Date: Thu, 02 Jun 2016 06:43:19 +0200 [thread overview]
Message-ID: <574FB967.7090201@samsung.com> (raw)
In-Reply-To: <574EC8EE.5000509@samsung.com>
On 06/01/2016 01:37 PM, Jacek Anaszewski wrote:
> Hi Krzysztof,
>
> One thing drew my attention while reviewing this again:
> max8997_led_brightness_set() can sleep, but the brightness_set
> op it is assigned to must not sleep. At the time when this driver was
> merged we were delegating brightness setting to workqueues task
> in LED class drivers that can sleep during this call.
> This must have been overlooked, which is even more likely, taking into
> account that the initial patch doesn't have LED maintainer's ack.
>
> The non-sleeping requirement is motivated by the fact that brightness
> can be set from softirq context, e.g. when timer trigger is enabled.
>
> Currently LED class drivers don't have to use workqueue on their own,
> but are required to use brightness_set_blocking op instead of
> brightness_set if they can sleep while setting brightness.
>
> Apart of that, I think that operations in max8997_led_brightness_set()
> should be protected with mutex to assure leaving the device in
> a consistent state in case of concurrent calls.
>
> I am aware that this is out of this patch scope, but I'd be grateful
> if you could apply those changes and test them on hardware if you have
> an access to.
The problem you mention existed before the patch. It was using sleeping
primitives (mutex) before adding regmap so I understand you don't have
anything against this patch, right?
I can fix the issue but it will be a little bit trickier because I don't
have the hardware. Other guys in the team tested the patchset for me so
I rely on them in that matter. Anyway I'll work on it.
Best regards,
Krzysztof
next prev parent reply other threads:[~2016-06-02 4:43 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-01 9:28 [PATCH v7 0/6] mfd: (multiple trees): max8997: Add regmap Krzysztof Kozlowski
2016-06-01 9:28 ` Krzysztof Kozlowski
2016-06-01 9:28 ` [rtc-linux] " Krzysztof Kozlowski
2016-06-01 9:53 ` [PATCH v7 1/6] mfd: max8997: Use regmap to access registers Krzysztof Kozlowski
2016-06-01 9:53 ` Krzysztof Kozlowski
2016-06-01 9:53 ` [rtc-linux] " Krzysztof Kozlowski
2016-06-01 11:37 ` Jacek Anaszewski
2016-06-01 11:37 ` Jacek Anaszewski
2016-06-02 4:43 ` Krzysztof Kozlowski [this message]
2016-06-02 4:43 ` Krzysztof Kozlowski
2016-06-02 7:08 ` Jacek Anaszewski
2016-06-02 7:08 ` Jacek Anaszewski
[not found] ` <1464774841-1439-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
[not found] ` <1464773339-756-1-git-send-email-k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2016-06-01 9:53 ` [PATCH v7 2/6] mfd: max8997: handle IRQs using regmap Krzysztof Kozlowski
2016-06-01 9:53 ` Krzysztof Kozlowski
2016-06-01 9:53 ` Krzysztof Kozlowski
2016-06-01 9:53 ` [rtc-linux] " Krzysztof Kozlowski
2016-06-01 9:53 ` [PATCH v7 3/6] mfd: max8997: Change irq names to upper case Krzysztof Kozlowski
2016-06-01 9:53 ` Krzysztof Kozlowski
2016-06-01 9:53 ` Krzysztof Kozlowski
2016-06-01 9:53 ` [rtc-linux] " Krzysztof Kozlowski
2016-06-01 9:53 ` [PATCH v7 4/6] extcon: max8997: Fix handling error code of regmap_irq_get_virq() Krzysztof Kozlowski
2016-06-01 9:53 ` Krzysztof Kozlowski
2016-06-01 9:53 ` Krzysztof Kozlowski
2016-06-01 9:53 ` [rtc-linux] " Krzysztof Kozlowski
2016-06-01 9:54 ` [PATCH v7 5/6] rtc: max8997: Check for ERRNO " Krzysztof Kozlowski
2016-06-01 9:54 ` Krzysztof Kozlowski
2016-06-01 9:54 ` Krzysztof Kozlowski
2016-06-01 9:54 ` [rtc-linux] " Krzysztof Kozlowski
2016-06-01 9:54 ` [PATCH v7 6/6] extcon: max8997: Fix inconsistent indenting Krzysztof Kozlowski
2016-06-01 9:54 ` Krzysztof Kozlowski
2016-06-01 9:54 ` Krzysztof Kozlowski
2016-06-01 9:54 ` [rtc-linux] " Krzysztof Kozlowski
2016-06-08 14:26 ` [PATCH v7 1/6] mfd: max8997: Use regmap to access registers Lee Jones
2016-06-08 14:26 ` Lee Jones
2016-06-08 14:26 ` Lee Jones
2016-06-08 14:26 ` [rtc-linux] " Lee Jones
2016-06-08 14:30 ` Krzysztof Kozlowski
2016-06-08 14:30 ` Krzysztof Kozlowski
2016-06-08 14:30 ` Krzysztof Kozlowski
2016-06-08 14:30 ` [rtc-linux] " Krzysztof Kozlowski
[not found] ` <57582C1F.5040306-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2016-06-16 15:07 ` Lee Jones
2016-06-16 15:07 ` Lee Jones
2016-06-16 15:07 ` Lee Jones
2016-06-16 15:07 ` [rtc-linux] " Lee Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=574FB967.7090201@samsung.com \
--to=k.kozlowski@samsung.com \
--cc=b.zolnierkie@samsung.com \
--cc=cw00.choi@samsung.com \
--cc=j.anaszewski@samsung.com \
--cc=kgene@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=myungjoo.ham@samsung.com \
--cc=r.baldyga@hackerion.com \
--cc=rpurdie@rpsys.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.