From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751243AbcFILmN (ORCPT ); Thu, 9 Jun 2016 07:42:13 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:8426 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751112AbcFILmJ (ORCPT ); Thu, 9 Jun 2016 07:42:09 -0400 X-AuditID: cbfec7f5-f792a6d000001302-1a-5759560c55c9 Subject: Re: [RFC v4 01/14] regulator: of: Add helper for getting all supplies To: Mark Brown References: <1465465471-28740-1-git-send-email-k.kozlowski@samsung.com> <1465465471-28740-2-git-send-email-k.kozlowski@samsung.com> <20160609102925.GS7510@sirena.org.uk> Cc: Ulf Hansson , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , Liam Girdwood , Greg Kroah-Hartman , Hans de Goede , Jean-Christophe Plagniol-Villard , Tomi Valkeinen , Heiko Stuebner , linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-pm@vger.kernel.org, linux-usb@vger.kernel.org, linux-fbdev@vger.kernel.org, hzpeterchen@gmail.com, Bartlomiej Zolnierkiewicz From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <5759560A.70107@samsung.com> Date: Thu, 09 Jun 2016 13:42:02 +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: <20160609102925.GS7510@sirena.org.uk> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA03Sa0hTYRgHcN5zzs6O5uo4p71pKSwilVp5gV6koi/RoUgjxGmgtvSgotOx qWhfEkrygtNmeVle0pyXoS2npGlq6WiiecNLJaaYQ0wtM6uNQq05Ir89z//9Pc/z5aVwvplw peISk1l5oiRBSNoTg1vGieMOIWHik0NKD9RcouOgB/MmEqlMaziqNAxz0L2GQhzlr1Zx0O3H OhKtGosxtP1xhYNyJvUkGl5uAGhlyQf9nMjCkH5hioP6lV85aLyjjESG7XyANvIMAJWMdGOo ujYTR5q3YxgqKG4i0JP5JQxldhm4aLAzGekqp0lkbBKfc2MaKxoBM67Mw5jn6g9cpqHuO8m0 1Hszy6MzBKPXZpPMzNQLkukpb+QyLTW3mLXuSZJRtmoB0zZVgTPGd20Ys6F3v7Lvmv3paDYh LpWVnzh73T72TY8Wl5kd0+r7iQxQtDcH2FGQ9ocvVWuYrXaBo7M6MgfYU3xaA2DR5hhuaxYB nFfW4FblRAfCWksxaa0F9GE4ae4ibKgJwPvfVDsNTrdyYWblPGFVJO0HW+pqSNsNd5iXq9rJ ebQnHBjJ/ruVogj6CFwtOmCNnelQqG6zYDbiCC2FszvcjvaFfbouYOU4LYJzY97WGKc9YEvj Z7wAOKp3Taj/K/Uu9QjgWuDMpkTJFDdipL4ihUSqSEmMEUUlSfXA9h1+tAPN64BeQFNA6MDz jA4V8zmSVEW6tBdAChcKeH1Xw8R8XrQk/SYrT4qUpySwil7gRhHC/bzSjrVgPh0jSWbjWVbG yv+9YpSdawbwyMq+NPhJWpRzTP8lYonnV7bgkuagWAiKFHU0DGl/cw+dKV9IPV89PSSICw++ 3LPHSSy7cPGp+E7Ss/cHqyMWgzZNId780hlpbNSGWXtXE2639ZCLe/nXKHvQiOvgXPuvgHY/ qmBdEFglgibjqeKBV61e6yW5TPPR1E5LZ7xFSChiJT7euFwh+QM9K3rbCgMAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/09/2016 12:29 PM, Mark Brown wrote: > On Thu, Jun 09, 2016 at 11:44:18AM +0200, Krzysztof Kozlowski wrote: >> Few drivers have a need of getting regulator supplies without knowing >> their names: >> 1. The Simple Framebuffer driver works on setup provided by bootloader >> (outside of scope of kernel); >> 2. Generic power sequence driver may be attached to any device node. >> >> Add a Device Tree helper for parsing "-supply" properties and returning >> allocated bulk regulator consumers. > > I'm still very concerned that this is just an invitation to people to > write half baked regulator consumers and half baked DTs to go along with > it, making it a standard API that doesn't have big red flags on it that > will flag up when "normal" drivers use it is not good. Right now this > just looks like a standard API and people are going to just start using > it. If we are going to do this perhaps we need a separate header or > something to help flag this up. No problem, I can move it to a special header. Actually, if you dislike this as an API, it does not have to be in header at all. I can just duplicate the simplefb code. > In the case of power sequences I'd expect the sequences to perform > operations on named supplies - the core shouldn't know what the supplies > are but the thing specifying the sequence should. Hm, so maybe passing names like: usb3503@08 { reset-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>; initial-mode = <1>; vdd-supply = <&buck8_reg>; foo-supply = <&buck9_reg>; power-sequence; power-sequence-supplies = "vdd", "foo"; }; but this is getting against initial idea of not adding any power-sequence properties. > >> drivers/regulator/of_regulator.c | 86 ++++++++++++++++++++++++++++++++++ >> include/linux/regulator/of_regulator.h | 13 +++++ >> 2 files changed, 99 insertions(+) > > The external interface shouldn't be DT specific, the Intel people are > busy importing all of DT into ACPI so they'll doubtless want an ACPI > version. Sure, I'll add it if this approach is acceptable. At this moment this is not necessary to show my idea so I prefer to avoid doing work which might be discarded very fast by review. Thanks for feedback! Best regards, Krzysztof From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Subject: Re: [RFC v4 01/14] regulator: of: Add helper for getting all supplies Date: Thu, 09 Jun 2016 13:42:02 +0200 Message-ID: <5759560A.70107@samsung.com> References: <1465465471-28740-1-git-send-email-k.kozlowski@samsung.com> <1465465471-28740-2-git-send-email-k.kozlowski@samsung.com> <20160609102925.GS7510@sirena.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Return-path: In-reply-to: <20160609102925.GS7510@sirena.org.uk> Sender: linux-mmc-owner@vger.kernel.org To: Mark Brown Cc: Ulf Hansson , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , Liam Girdwood , Greg Kroah-Hartman , Hans de Goede , Jean-Christophe Plagniol-Villard , Tomi Valkeinen , Heiko Stuebner , linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-pm@vger.kernel.org, linux-usb@vger.kernel.org, linux-fbdev@vger.kernel.org, hzpeterchen@gmail.com, Bartlomiej List-Id: devicetree@vger.kernel.org On 06/09/2016 12:29 PM, Mark Brown wrote: > On Thu, Jun 09, 2016 at 11:44:18AM +0200, Krzysztof Kozlowski wrote: >> Few drivers have a need of getting regulator supplies without knowing >> their names: >> 1. The Simple Framebuffer driver works on setup provided by bootloader >> (outside of scope of kernel); >> 2. Generic power sequence driver may be attached to any device node. >> >> Add a Device Tree helper for parsing "-supply" properties and returning >> allocated bulk regulator consumers. > > I'm still very concerned that this is just an invitation to people to > write half baked regulator consumers and half baked DTs to go along with > it, making it a standard API that doesn't have big red flags on it that > will flag up when "normal" drivers use it is not good. Right now this > just looks like a standard API and people are going to just start using > it. If we are going to do this perhaps we need a separate header or > something to help flag this up. No problem, I can move it to a special header. Actually, if you dislike this as an API, it does not have to be in header at all. I can just duplicate the simplefb code. > In the case of power sequences I'd expect the sequences to perform > operations on named supplies - the core shouldn't know what the supplies > are but the thing specifying the sequence should. Hm, so maybe passing names like: usb3503@08 { reset-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>; initial-mode = <1>; vdd-supply = <&buck8_reg>; foo-supply = <&buck9_reg>; power-sequence; power-sequence-supplies = "vdd", "foo"; }; but this is getting against initial idea of not adding any power-sequence properties. > >> drivers/regulator/of_regulator.c | 86 ++++++++++++++++++++++++++++++++++ >> include/linux/regulator/of_regulator.h | 13 +++++ >> 2 files changed, 99 insertions(+) > > The external interface shouldn't be DT specific, the Intel people are > busy importing all of DT into ACPI so they'll doubtless want an ACPI > version. Sure, I'll add it if this approach is acceptable. At this moment this is not necessary to show my idea so I prefer to avoid doing work which might be discarded very fast by review. Thanks for feedback! Best regards, Krzysztof From mboxrd@z Thu Jan 1 00:00:00 1970 From: Krzysztof Kozlowski Date: Thu, 09 Jun 2016 11:42:02 +0000 Subject: Re: [RFC v4 01/14] regulator: of: Add helper for getting all supplies Message-Id: <5759560A.70107@samsung.com> List-Id: References: <1465465471-28740-1-git-send-email-k.kozlowski@samsung.com> <1465465471-28740-2-git-send-email-k.kozlowski@samsung.com> <20160609102925.GS7510@sirena.org.uk> In-Reply-To: <20160609102925.GS7510@sirena.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-arm-kernel@lists.infradead.org On 06/09/2016 12:29 PM, Mark Brown wrote: > On Thu, Jun 09, 2016 at 11:44:18AM +0200, Krzysztof Kozlowski wrote: >> Few drivers have a need of getting regulator supplies without knowing >> their names: >> 1. The Simple Framebuffer driver works on setup provided by bootloader >> (outside of scope of kernel); >> 2. Generic power sequence driver may be attached to any device node. >> >> Add a Device Tree helper for parsing "-supply" properties and returning >> allocated bulk regulator consumers. > > I'm still very concerned that this is just an invitation to people to > write half baked regulator consumers and half baked DTs to go along with > it, making it a standard API that doesn't have big red flags on it that > will flag up when "normal" drivers use it is not good. Right now this > just looks like a standard API and people are going to just start using > it. If we are going to do this perhaps we need a separate header or > something to help flag this up. No problem, I can move it to a special header. Actually, if you dislike this as an API, it does not have to be in header at all. I can just duplicate the simplefb code. > In the case of power sequences I'd expect the sequences to perform > operations on named supplies - the core shouldn't know what the supplies > are but the thing specifying the sequence should. Hm, so maybe passing names like: usb3503@08 { reset-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>; initial-mode = <1>; vdd-supply = <&buck8_reg>; foo-supply = <&buck9_reg>; power-sequence; power-sequence-supplies = "vdd", "foo"; }; but this is getting against initial idea of not adding any power-sequence properties. > >> drivers/regulator/of_regulator.c | 86 ++++++++++++++++++++++++++++++++++ >> include/linux/regulator/of_regulator.h | 13 +++++ >> 2 files changed, 99 insertions(+) > > The external interface shouldn't be DT specific, the Intel people are > busy importing all of DT into ACPI so they'll doubtless want an ACPI > version. Sure, I'll add it if this approach is acceptable. At this moment this is not necessary to show my idea so I prefer to avoid doing work which might be discarded very fast by review. Thanks for feedback! Best regards, Krzysztof From mboxrd@z Thu Jan 1 00:00:00 1970 From: k.kozlowski@samsung.com (Krzysztof Kozlowski) Date: Thu, 09 Jun 2016 13:42:02 +0200 Subject: [RFC v4 01/14] regulator: of: Add helper for getting all supplies In-Reply-To: <20160609102925.GS7510@sirena.org.uk> References: <1465465471-28740-1-git-send-email-k.kozlowski@samsung.com> <1465465471-28740-2-git-send-email-k.kozlowski@samsung.com> <20160609102925.GS7510@sirena.org.uk> Message-ID: <5759560A.70107@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 06/09/2016 12:29 PM, Mark Brown wrote: > On Thu, Jun 09, 2016 at 11:44:18AM +0200, Krzysztof Kozlowski wrote: >> Few drivers have a need of getting regulator supplies without knowing >> their names: >> 1. The Simple Framebuffer driver works on setup provided by bootloader >> (outside of scope of kernel); >> 2. Generic power sequence driver may be attached to any device node. >> >> Add a Device Tree helper for parsing "-supply" properties and returning >> allocated bulk regulator consumers. > > I'm still very concerned that this is just an invitation to people to > write half baked regulator consumers and half baked DTs to go along with > it, making it a standard API that doesn't have big red flags on it that > will flag up when "normal" drivers use it is not good. Right now this > just looks like a standard API and people are going to just start using > it. If we are going to do this perhaps we need a separate header or > something to help flag this up. No problem, I can move it to a special header. Actually, if you dislike this as an API, it does not have to be in header at all. I can just duplicate the simplefb code. > In the case of power sequences I'd expect the sequences to perform > operations on named supplies - the core shouldn't know what the supplies > are but the thing specifying the sequence should. Hm, so maybe passing names like: usb3503 at 08 { reset-gpios = <&gpx3 5 GPIO_ACTIVE_HIGH>; initial-mode = <1>; vdd-supply = <&buck8_reg>; foo-supply = <&buck9_reg>; power-sequence; power-sequence-supplies = "vdd", "foo"; }; but this is getting against initial idea of not adding any power-sequence properties. > >> drivers/regulator/of_regulator.c | 86 ++++++++++++++++++++++++++++++++++ >> include/linux/regulator/of_regulator.h | 13 +++++ >> 2 files changed, 99 insertions(+) > > The external interface shouldn't be DT specific, the Intel people are > busy importing all of DT into ACPI so they'll doubtless want an ACPI > version. Sure, I'll add it if this approach is acceptable. At this moment this is not necessary to show my idea so I prefer to avoid doing work which might be discarded very fast by review. Thanks for feedback! Best regards, Krzysztof