From mboxrd@z Thu Jan 1 00:00:00 1970 From: Quentin Schulz Subject: Re: [PATCH v3 12/12] ARM: dtsi: axp81x: set pinmux for GPIO0/1 when used as LDOs Date: Wed, 4 Oct 2017 09:35:06 +0200 Message-ID: <8be22e47-8071-be4c-0bd4-38fa51cbe3ff@free-electrons.com> References: <20171002120854.5212-1-quentin.schulz@free-electrons.com> <20171002120854.5212-13-quentin.schulz@free-electrons.com> <20171003144737.rhgtp3uwckhvoqdw@flea> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: In-Reply-To: Content-Language: en-US Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Chen-Yu Tsai , Maxime Ripard Cc: Linus Walleij , Mark Brown , Rob Herring , Mark Rutland , Russell King , Lee Jones , "linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , Thomas Petazzoni , linux-sunxi List-Id: linux-gpio@vger.kernel.org Hi Chen-Yu, Linus, On 03/10/2017 17:08, Chen-Yu Tsai wrote: > On Tue, Oct 3, 2017 at 10:47 PM, Maxime Ripard > wrote: >> Hi Linus, >> >> On Tue, Oct 03, 2017 at 09:27:17AM +0000, Linus Walleij wrote: >>> On Mon, Oct 2, 2017 at 2:08 PM, Quentin Schulz >>> wrote: >>> >>>> On AXP813/818, GPIO0 and GPIO1 can be used as LDO as (respectively) >>>> ldo_io0 and ldo_io1. >>> (...) >>>> + gpio0_ldo: gpio0_ldo { >>>> + pins = "GPIO0"; >>>> + function = "ldo"; >>>> + }; >>> (...) >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&gpio0_ldo>; >>>> /* Disable by default to avoid conflicts with GPIO */ >>>> status = "disabled"; >>> >>> So this is still by default disabled, but you make the default >>> mode something called "ldo". >>> >>> And I think that is to be understood as a low-dropout regulator? >>> >>> So is the idea that this should be represented as a regulator >>> in the end? >>> >>> Then I think the state name should not be "default" rather >>> something like "regulator" and "default" should be the GPIO >>> mode, as I guess something like that exists. >>> >>> Activating a regulator using pin control "default" mode is >>> not very pretty. It would probably be unintuitive and end >>> up wasting power because people will get confused about >>> what is going on. >> >> That's not really it. The PMIC has pins that can be muxed either to >> (regular) GPIOs, an ADC or to an LDO regulator. >> >> This is just muxing, the regulator will be enabled and disabled >> separately through another register. If it wasn't the case, it would >> indeed be very messy. > > No. Actually they are controlled in the same register, so it is > very messy. The muxing options are: > > - 0: drive low > - 1: drive high > - 2: input with interrupt triggering > - 3: LDO on > - 4: LDO off > - 5~7: floating (or ADC) > Just to be a little more precise, - 0: drive low - 1: drive high - 2: input with interrupt triggering - 3: LDO on - 4: LDO off - 5~7: floating (or ADC) for AXP813, and - 0: drive low - 1: drive high - 2: input with interrupt triggering - 3: LDO on - 4: ADC - 5~7: floating for AXP209. So I think what you suggested Linus is not really relevant here as the regulator framework will take care of disabling the regulator when needed (for AXP813 via the ldo_off "muxing" selected by the regulator framework). However, there is no LDO off bit for AXP209 and the LDO can't be set to 0V in any other register. What's done now in the regulator driver for AXP209 is to select the floating "muxing" for the pin when wanting to disable the regulator. So I guess that's a way to handle it. Should we do it another way? Thanks for raising the issue, I frankly haven't thought about that at all. I have to send a v4 to update the support for AXP813 (basically setting ADC muxing to 0x5 instead of 0x4, for AXP813) as I misread the muxing register description when adding support for it. Thanks, Quentin -- Quentin Schulz, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751486AbdJDHfL (ORCPT ); Wed, 4 Oct 2017 03:35:11 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:53553 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751170AbdJDHfJ (ORCPT ); Wed, 4 Oct 2017 03:35:09 -0400 Subject: Re: [PATCH v3 12/12] ARM: dtsi: axp81x: set pinmux for GPIO0/1 when used as LDOs To: Chen-Yu Tsai , Maxime Ripard Cc: Linus Walleij , Mark Brown , Rob Herring , Mark Rutland , Russell King , Lee Jones , "linux-gpio@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Thomas Petazzoni , linux-sunxi References: <20171002120854.5212-1-quentin.schulz@free-electrons.com> <20171002120854.5212-13-quentin.schulz@free-electrons.com> <20171003144737.rhgtp3uwckhvoqdw@flea> From: Quentin Schulz Message-ID: <8be22e47-8071-be4c-0bd4-38fa51cbe3ff@free-electrons.com> Date: Wed, 4 Oct 2017 09:35:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Chen-Yu, Linus, On 03/10/2017 17:08, Chen-Yu Tsai wrote: > On Tue, Oct 3, 2017 at 10:47 PM, Maxime Ripard > wrote: >> Hi Linus, >> >> On Tue, Oct 03, 2017 at 09:27:17AM +0000, Linus Walleij wrote: >>> On Mon, Oct 2, 2017 at 2:08 PM, Quentin Schulz >>> wrote: >>> >>>> On AXP813/818, GPIO0 and GPIO1 can be used as LDO as (respectively) >>>> ldo_io0 and ldo_io1. >>> (...) >>>> + gpio0_ldo: gpio0_ldo { >>>> + pins = "GPIO0"; >>>> + function = "ldo"; >>>> + }; >>> (...) >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&gpio0_ldo>; >>>> /* Disable by default to avoid conflicts with GPIO */ >>>> status = "disabled"; >>> >>> So this is still by default disabled, but you make the default >>> mode something called "ldo". >>> >>> And I think that is to be understood as a low-dropout regulator? >>> >>> So is the idea that this should be represented as a regulator >>> in the end? >>> >>> Then I think the state name should not be "default" rather >>> something like "regulator" and "default" should be the GPIO >>> mode, as I guess something like that exists. >>> >>> Activating a regulator using pin control "default" mode is >>> not very pretty. It would probably be unintuitive and end >>> up wasting power because people will get confused about >>> what is going on. >> >> That's not really it. The PMIC has pins that can be muxed either to >> (regular) GPIOs, an ADC or to an LDO regulator. >> >> This is just muxing, the regulator will be enabled and disabled >> separately through another register. If it wasn't the case, it would >> indeed be very messy. > > No. Actually they are controlled in the same register, so it is > very messy. The muxing options are: > > - 0: drive low > - 1: drive high > - 2: input with interrupt triggering > - 3: LDO on > - 4: LDO off > - 5~7: floating (or ADC) > Just to be a little more precise, - 0: drive low - 1: drive high - 2: input with interrupt triggering - 3: LDO on - 4: LDO off - 5~7: floating (or ADC) for AXP813, and - 0: drive low - 1: drive high - 2: input with interrupt triggering - 3: LDO on - 4: ADC - 5~7: floating for AXP209. So I think what you suggested Linus is not really relevant here as the regulator framework will take care of disabling the regulator when needed (for AXP813 via the ldo_off "muxing" selected by the regulator framework). However, there is no LDO off bit for AXP209 and the LDO can't be set to 0V in any other register. What's done now in the regulator driver for AXP209 is to select the floating "muxing" for the pin when wanting to disable the regulator. So I guess that's a way to handle it. Should we do it another way? Thanks for raising the issue, I frankly haven't thought about that at all. I have to send a v4 to update the support for AXP813 (basically setting ADC muxing to 0x5 instead of 0x4, for AXP813) as I misread the muxing register description when adding support for it. Thanks, Quentin -- Quentin Schulz, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com From mboxrd@z Thu Jan 1 00:00:00 1970 From: quentin.schulz@free-electrons.com (Quentin Schulz) Date: Wed, 4 Oct 2017 09:35:06 +0200 Subject: [PATCH v3 12/12] ARM: dtsi: axp81x: set pinmux for GPIO0/1 when used as LDOs In-Reply-To: References: <20171002120854.5212-1-quentin.schulz@free-electrons.com> <20171002120854.5212-13-quentin.schulz@free-electrons.com> <20171003144737.rhgtp3uwckhvoqdw@flea> Message-ID: <8be22e47-8071-be4c-0bd4-38fa51cbe3ff@free-electrons.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Chen-Yu, Linus, On 03/10/2017 17:08, Chen-Yu Tsai wrote: > On Tue, Oct 3, 2017 at 10:47 PM, Maxime Ripard > wrote: >> Hi Linus, >> >> On Tue, Oct 03, 2017 at 09:27:17AM +0000, Linus Walleij wrote: >>> On Mon, Oct 2, 2017 at 2:08 PM, Quentin Schulz >>> wrote: >>> >>>> On AXP813/818, GPIO0 and GPIO1 can be used as LDO as (respectively) >>>> ldo_io0 and ldo_io1. >>> (...) >>>> + gpio0_ldo: gpio0_ldo { >>>> + pins = "GPIO0"; >>>> + function = "ldo"; >>>> + }; >>> (...) >>>> + pinctrl-names = "default"; >>>> + pinctrl-0 = <&gpio0_ldo>; >>>> /* Disable by default to avoid conflicts with GPIO */ >>>> status = "disabled"; >>> >>> So this is still by default disabled, but you make the default >>> mode something called "ldo". >>> >>> And I think that is to be understood as a low-dropout regulator? >>> >>> So is the idea that this should be represented as a regulator >>> in the end? >>> >>> Then I think the state name should not be "default" rather >>> something like "regulator" and "default" should be the GPIO >>> mode, as I guess something like that exists. >>> >>> Activating a regulator using pin control "default" mode is >>> not very pretty. It would probably be unintuitive and end >>> up wasting power because people will get confused about >>> what is going on. >> >> That's not really it. The PMIC has pins that can be muxed either to >> (regular) GPIOs, an ADC or to an LDO regulator. >> >> This is just muxing, the regulator will be enabled and disabled >> separately through another register. If it wasn't the case, it would >> indeed be very messy. > > No. Actually they are controlled in the same register, so it is > very messy. The muxing options are: > > - 0: drive low > - 1: drive high > - 2: input with interrupt triggering > - 3: LDO on > - 4: LDO off > - 5~7: floating (or ADC) > Just to be a little more precise, - 0: drive low - 1: drive high - 2: input with interrupt triggering - 3: LDO on - 4: LDO off - 5~7: floating (or ADC) for AXP813, and - 0: drive low - 1: drive high - 2: input with interrupt triggering - 3: LDO on - 4: ADC - 5~7: floating for AXP209. So I think what you suggested Linus is not really relevant here as the regulator framework will take care of disabling the regulator when needed (for AXP813 via the ldo_off "muxing" selected by the regulator framework). However, there is no LDO off bit for AXP209 and the LDO can't be set to 0V in any other register. What's done now in the regulator driver for AXP209 is to select the floating "muxing" for the pin when wanting to disable the regulator. So I guess that's a way to handle it. Should we do it another way? Thanks for raising the issue, I frankly haven't thought about that at all. I have to send a v4 to update the support for AXP813 (basically setting ADC muxing to 0x5 instead of 0x4, for AXP813) as I misread the muxing register description when adding support for it. Thanks, Quentin -- Quentin Schulz, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com