From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754676AbcBPJNn (ORCPT ); Tue, 16 Feb 2016 04:13:43 -0500 Received: from mout.kundenserver.de ([217.72.192.75]:57046 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754705AbcBPJLG (ORCPT ); Tue, 16 Feb 2016 04:11:06 -0500 From: Arnd Bergmann To: linaro-kernel@lists.linaro.org Cc: Mark Brown , Viresh Kumar , Nishanth Menon , linux-pm@vger.kernel.org, Viresh Kumar , Rafael Wysocki , linux-kernel@vger.kernel.org, Jon Hunter , Stephen Boyd Subject: Re: [PATCH] PM / OPP: Initialize regulator pointer to an error value Date: Tue, 16 Feb 2016 10:10:44 +0100 Message-ID: <2628515.OmmYzoeulx@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20160216015616.GH18327@sirena.org.uk> References: <1455544758-7718-1-git-send-email-jonathanh@nvidia.com> <20160216010059.GH6334@vireshk-i7> <20160216015616.GH18327@sirena.org.uk> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:hAntiFznJQhd0/1wm1zGqeUvMI203ASolw0Sjyv3FULTvr3oXe3 aq84LQ14TZFfBXRJIoDAhrraQMAs3mM2WX3G3lfA+d99VKZR308jjgDRv0kT6VoQmOyfaJc qh1pqgRaq1nVxBA/XvnwKi7LOt/mbE/J5JZos1xN73coHZWJHSO+iuCr3UkBPddKH2x2pDP vi+Ym+OK+hCRZxPBJOlCg== X-UI-Out-Filterresults: notjunk:1;V01:K0:FmSWNTNIZjs=:ouJh8hzFtn+85UUcPUGWpS KyaREiWeZBJUCo/mRVXfnkllznIfmy4D6KfPGD+U4QaCKMW2DKqZYHzImqNoKk/XzJ71dkb6Q mV/ifZiW0VUABh37ggTighKpPnw/jipTYLywcu7O3G3KbmONE4qK2veMCVYMYGAvGrx/4WYGQ SCpiPiMiuIF0T/0+DFvPyNOMfTiKFX8zHgEw1BkGoPQzEz3hD6Ejk+Ku3IRiB/UlytTmagU+a oqa8xxPlpyKVUuFEsvhWu2pErVE+vPbxUWs6rQ42fGpZUJlN++ghRyIF2rrfCTZd4m8zd6zjY UYfODTvyMjUvB6YTBer9/iQPSsxuaj5LlBu1n2EWv9mcmrySYE2PhfDryjq5hxq2S8FVUBM5Z 8FfHvmjYjl9Z2Eh5o+ifn+xptcZnKG0Fz9hR8dV21F6A/fYRDGG+bUp7y/JQiGhhAdOh5lU2j cdRJeVWaCg8hRoBtLUmFstT1cfuc/gS7hacvcJTw8L6iqms1I88qL14eS8e4YszSrAh3QvUI3 j0oHkDEr+HOrXMgq5OLsFCpaBASo8oMh72MZ/UUOoV6j1UpLQzPEoI1R4EaHySega96ys9F5r MXVs9ux0mr0Yt6z/UXehk2cqrQOhxssd3uBfMmZ9rLH174B1XRfe0EIDMCR2v3n6b0zMcpHjz +8rNDD1C1ANTYIiPuFCGY4tr0vkGgAQgAkyaYtsYjV7EIoI3zB71MYIWhW72zXsA5eiKcWd5K AvXsFr5W1xAYe0sS Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 16 February 2016 01:56:16 Mark Brown wrote: > On Tue, Feb 16, 2016 at 06:30:59AM +0530, Viresh Kumar wrote: > > > - And so I left the regulator pointer to NULL in OPP core. > > - But then I realized that its not safe to call many regulator core > > APIs with NULL regulator, as those caused the crashes reported by > > multiple people now. > > - clk APIs guarantee that they return early when NULL clk is passed to > > them. > > - Do we need to do the same for regulator core as well ? > > No, NULL is explicitly not something you can substitute in, > essentially all the users are just not bothering to implement error > checking and we don't want to encourage that. The set of use cases > where we legitimately have optional supplies is very small, much smaller > than clocks, because it makes the electrical engineering a lot harder. > I must have misinterpreted the idea behind that API as well then. >>From this function definition: /* * Make sure client drivers will still build on systems with no software * controllable voltage or current regulators. */ static inline struct regulator *__must_check regulator_get(struct device *dev, const char *id) { /* Nothing except the stubbed out regulator API should be * looking at the value except to check if it is an error * value. Drivers are free to handle NULL specifically by * skipping all regulator API calls, but they don't have to. * Drivers which don't, should make sure they properly handle * corner cases of the API, such as regulator_get_voltage() * returning 0. */ return NULL; } my reading was that the expected behavior in any driver was: * call regulator_get() * if IS_ERR(), fail device probe function, never use invalid pointer other than PTR_ERR() * if NULL, and regulator is required, fail probe so we never use the regulator * if NULL, and regulators are optional, continue with the NULL value. * drivers never look into the regulator pointer, and only pass it into regulator APIs which can cope with the NULL value when CONFIG_REGULATOR is disabled. That would be similar to what we have for clocks. Which part of my interpretation is wrong? Arnd