From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751733AbcFBHJD (ORCPT ); Thu, 2 Jun 2016 03:09:03 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:18924 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750740AbcFBHJA (ORCPT ); Thu, 2 Jun 2016 03:09:00 -0400 X-AuditID: cbfec7f4-f796c6d000001486-a8-574fdb878acc Message-id: <574FDB86.2070107@samsung.com> Date: Thu, 02 Jun 2016 09:08:54 +0200 From: Jacek Anaszewski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804 Thunderbird/17.0.8 MIME-version: 1.0 To: Krzysztof Kozlowski 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 Subject: Re: [PATCH v7 1/6] mfd: max8997: Use regmap to access registers 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> <574FB967.7090201@samsung.com> In-reply-to: <574FB967.7090201@samsung.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrBLMWRmVeSWpSXmKPExsVy+t/xK7rtt/3DDU5c0bDYOGM9q8X1L89Z LV6/MLTof/ya2WLT42usFpd3zWGz2PpmHaPF594jjBYzzu9jsrjduILN4u6/T4wWu3c9ZXXg 8Tiy8xibx6ZVnWwem5fUe+yZ/4PVo2/LKkaPz5vkAtiiuGxSUnMyy1KL9O0SuDIOfp7AXrCN r2Le9JnMDYwnuLsYOTkkBEwkZt+dwgRhi0lcuLeerYuRi0NIYCmjxOWpp5lBEkICzxgl7r9U A7F5BbQkPhy6xgZiswioSlxbdYgFxGYTMJT4+eI12CBRgQiJP6f3sULUC0r8mHwPrEYEqObg 7u1MIAuYBbqYJbqW3QBbICzgIdHXfp8RYvMhoM1dR4E2cHBwCmhL/NypClLDLGArseD9OhYI W15i85q3zBMYBWYh2TELSdksJGULGJlXMYqmliYXFCel5xrqFSfmFpfmpesl5+duYoTEx5cd jIuPWR1iFOBgVOLhXaHpHy7EmlhWXJkLdAwHs5IIr94NoBBvSmJlVWpRfnxRaU5q8SFGaQ4W JXHeubvehwgJpCeWpGanphakFsFkmTg4pRoYZ2s3JDNue/qt84RktOvFzy+rzMI+rCluqH6Q xv6TVSxG8Leox9O1S7yF2TZO1twhEMlSWHmsbsm57v9Ge7ymCCnuv8MmKXTUrp/dZ5a383ud ltPbZ92YXugSt+LLxpCEWhuD5rcv2oKfdbYbzkt1f+qnYvtYYlJcML9i/7aJYl9atevP+Jco sRRnJBpqMRcVJwIAbgH4fYsCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/02/2016 06:43 AM, Krzysztof Kozlowski wrote: > 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? Right, I just wanted to indicate the problem. The patch itself is ok. > 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. Ack. -- Best regards, Jacek Anaszewski