From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751705AbcFBEn0 (ORCPT ); Thu, 2 Jun 2016 00:43:26 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:13874 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750744AbcFBEnY (ORCPT ); Thu, 2 Jun 2016 00:43:24 -0400 X-AuditID: cbfec7f4-f796c6d000001486-0a-574fb969ea3d Subject: Re: [PATCH v7 1/6] mfd: max8997: Use regmap to access registers To: Jacek Anaszewski References: <1464773339-756-1-git-send-email-k.kozlowski@samsung.com> <1464774841-1439-1-git-send-email-k.kozlowski@samsung.com> <574EC8EE.5000509@samsung.com> Cc: Kukjin Kim , MyungJoo Ham , Chanwoo Choi , Richard Purdie , 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 From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <574FB967.7090201@samsung.com> Date: Thu, 02 Jun 2016 06:43:19 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-version: 1.0 In-reply-to: <574EC8EE.5000509@samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupkkeLIzCtJLcpLzFFi42I5/e/4Fd3Mnf7hBs39nBYbZ6xntbj+5Tmr Re/V54wWr18YWvQ/fs1ssenxNVaLy7vmsFlsfbOO0eJz7xFGixnn9zFZ3G5cwWZx998nRovd u56yOvB6HNl5jM1j06pONo/NS+o99sz/werRt2UVo8fnTXIBbFFcNimpOZllqUX6dglcGX// /2ctaOet+HDyPlMD43auLkZODgkBE4krp/4wQ9hiEhfurWfrYuTiEBJYyiix+stiRgjnGaPE 7w23WUCqhAU8JPra7zOC2CIC+hINDX1QRYsZJb71r2EFcZgFupglupbdAJvLJmAssXn5EjaI HXISvd2TwCbxCmhJvFnZAxZnEVCVmDOpE6xeVCBCYtb2H0wQNYISPybfA6vnFNCWmNPwHGgb B9ACPYn7F7VAwswC8hKb17xlnsAoOAtJxyyEqllIqhYwMq9iFE0tTS4oTkrPNdQrTswtLs1L 10vOz93ECImgLzsYFx+zOsQowMGoxMO7QtM/XIg1say4MvcQowQHs5IIr/R2oBBvSmJlVWpR fnxRaU5q8SFGaQ4WJXHeubvehwgJpCeWpGanphakFsFkmTg4pRoYPf32Huk6JqbDNnVfObvb iV2NW7ZY/Tlke+dA+KcPk/cKvZp5/0C1t8aZtE2C8Ytmn1d4avzkn0Ud8+ZJt/at/bXi2s/Z +a2FOxPdbU7N+/5IYqKv2v2eRX3T10xsOuGamMD/ssSbyUmzqGnhQ/kPNatevL2mKNbhEMcZ mne+m2liQaLp2Z9MzUosxRmJhlrMRcWJAJSJJ06cAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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