All of lore.kernel.org
 help / color / mirror / Atom feed
From: hdegoede@redhat.com (Hans de Goede)
To: linux-arm-kernel@lists.infradead.org
Subject: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi
Date: Mon, 27 Jul 2015 10:07:55 +0200	[thread overview]
Message-ID: <55B5E6DB.8020009@redhat.com> (raw)
In-Reply-To: <1437960486-2809-1-git-send-email-public_timo.s@silentcreek.de>

Hi,

On 27-07-15 03:28, Timo Sigurdsson wrote:
> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
> driver, so add them to allow for voltage-scaling with cpufreq-dt. With the
> regulators enabled, we can define board-specific operating points. The defined
> CPU voltages are more conservative (based on the values used by the vendor),
> so they should be more failsafe across all boards of this kind out there.
>
> I'm posting this as RFC as I would like to make a few more remarks and raise
> questions along the way (plus, I'm anything but an experienced developer, so a
> a critical review might help).
>
> I checked the regulator definitions against the schematics released by LeMaker.
> I also compared that to the DT and schematics of Cubiboard 2 and Cubietruck. Of
> course, I also tested the patch on the actual hardware and it works fine for me.
> The CPU voltages are slightly higher than those set in sun7i-a20.dtsi and even
> though they work well for me, I thought it might be safer to use the more
> conservative values used by LeMaker in their old fex file. Would you agree?
> Besides, it it ok to have this in one patch or should it be splitted in two
> (one for the regulators and one for the opp)? Did I miss something important?
>
> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>

I've a simular patch here:

https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1

I did not submit this upstream yet as it is part of a series to enable the otg
controller on the bananapi which needs axp-usb-power-supply support for which
the actual powersupply driver changes are still pending.

As you can see other then you adding the cpu operating points are patches are
identical, which is good :)

IMHO we should just stick with the standard operating points unless we know
that there are stability issues with them (such as e.g. on the A10 OlinuxIno Lime).

Regards,

Hans



> ---
>   arch/arm/boot/dts/sun7i-a20-bananapi.dts | 47 +++++++++++++++++++++++++++++---
>   1 file changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> index 9f7b472..2bcbb0e 100644
> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> @@ -92,6 +92,22 @@
>   	status = "okay";
>   };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +	operating-points = <
> +		/* kHz	  uV */
> +		1008000	1450000
> +		960000	1425000
> +		912000	1425000
> +		864000	1350000
> +		720000	1250000
> +		528000	1150000
> +		312000	1100000
> +		144000	1050000
> +		>;
> +	cooling-max-level = <7>;
> +};
> +
>   &ehci0 {
>   	status = "okay";
>   };
> @@ -119,13 +135,9 @@
>   	status = "okay";
>
>   	axp209: pmic at 34 {
> -		compatible = "x-powers,axp209";
>   		reg = <0x34>;
>   		interrupt-parent = <&nmi_intc>;
>   		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -
> -		interrupt-controller;
> -		#interrupt-cells = <1>;
>   	};
>   };
>
> @@ -182,6 +194,33 @@
>   	};
>   };
>
> +#include "axp209.dtsi"
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1050000>;
> +	regulator-max-microvolt = <1450000>;
> +	regulator-name = "vdd-cpu";
> +};
> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-int-dll";
> +};
> +
> +&reg_ldo1 {
> +	regulator-name = "vdd-rtc";
> +};
> +
> +&reg_ldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3000000>;
> +	regulator-max-microvolt = <3000000>;
> +	regulator-name = "avcc";
> +};
> +
>   &reg_usb1_vbus {
>   	status = "okay";
>   };
>

WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: public_timo.s-fWgRPtSzPNU3WX+qO2AYSQ@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
Subject: Re: [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi
Date: Mon, 27 Jul 2015 10:07:55 +0200	[thread overview]
Message-ID: <55B5E6DB.8020009@redhat.com> (raw)
In-Reply-To: <1437960486-2809-1-git-send-email-public_timo.s-fWgRPtSzPNU3WX+qO2AYSQ@public.gmane.org>

Hi,

On 27-07-15 03:28, Timo Sigurdsson wrote:
> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
> driver, so add them to allow for voltage-scaling with cpufreq-dt. With the
> regulators enabled, we can define board-specific operating points. The defined
> CPU voltages are more conservative (based on the values used by the vendor),
> so they should be more failsafe across all boards of this kind out there.
>
> I'm posting this as RFC as I would like to make a few more remarks and raise
> questions along the way (plus, I'm anything but an experienced developer, so a
> a critical review might help).
>
> I checked the regulator definitions against the schematics released by LeMaker.
> I also compared that to the DT and schematics of Cubiboard 2 and Cubietruck. Of
> course, I also tested the patch on the actual hardware and it works fine for me.
> The CPU voltages are slightly higher than those set in sun7i-a20.dtsi and even
> though they work well for me, I thought it might be safer to use the more
> conservative values used by LeMaker in their old fex file. Would you agree?
> Besides, it it ok to have this in one patch or should it be splitted in two
> (one for the regulators and one for the opp)? Did I miss something important?
>
> Signed-off-by: Timo Sigurdsson <public_timo.s-fWgRPtSzPNU3WX+qO2AYSQ@public.gmane.org>

I've a simular patch here:

https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1

I did not submit this upstream yet as it is part of a series to enable the otg
controller on the bananapi which needs axp-usb-power-supply support for which
the actual powersupply driver changes are still pending.

As you can see other then you adding the cpu operating points are patches are
identical, which is good :)

IMHO we should just stick with the standard operating points unless we know
that there are stability issues with them (such as e.g. on the A10 OlinuxIno Lime).

Regards,

Hans



> ---
>   arch/arm/boot/dts/sun7i-a20-bananapi.dts | 47 +++++++++++++++++++++++++++++---
>   1 file changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> index 9f7b472..2bcbb0e 100644
> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> @@ -92,6 +92,22 @@
>   	status = "okay";
>   };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +	operating-points = <
> +		/* kHz	  uV */
> +		1008000	1450000
> +		960000	1425000
> +		912000	1425000
> +		864000	1350000
> +		720000	1250000
> +		528000	1150000
> +		312000	1100000
> +		144000	1050000
> +		>;
> +	cooling-max-level = <7>;
> +};
> +
>   &ehci0 {
>   	status = "okay";
>   };
> @@ -119,13 +135,9 @@
>   	status = "okay";
>
>   	axp209: pmic@34 {
> -		compatible = "x-powers,axp209";
>   		reg = <0x34>;
>   		interrupt-parent = <&nmi_intc>;
>   		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -
> -		interrupt-controller;
> -		#interrupt-cells = <1>;
>   	};
>   };
>
> @@ -182,6 +194,33 @@
>   	};
>   };
>
> +#include "axp209.dtsi"
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1050000>;
> +	regulator-max-microvolt = <1450000>;
> +	regulator-name = "vdd-cpu";
> +};
> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-int-dll";
> +};
> +
> +&reg_ldo1 {
> +	regulator-name = "vdd-rtc";
> +};
> +
> +&reg_ldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3000000>;
> +	regulator-max-microvolt = <3000000>;
> +	regulator-name = "avcc";
> +};
> +
>   &reg_usb1_vbus {
>   	status = "okay";
>   };
>

WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com>
To: public_timo.s@silentcreek.de, robh+dt@kernel.org,
	pawel.moll@arm.com, mark.rutland@arm.com,
	ijc+devicetree@hellion.org.uk, galak@codeaurora.org,
	linux@arm.linux.org.uk, maxime.ripard@free-electrons.com,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com
Subject: Re: [linux-sunxi] [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi
Date: Mon, 27 Jul 2015 10:07:55 +0200	[thread overview]
Message-ID: <55B5E6DB.8020009@redhat.com> (raw)
In-Reply-To: <1437960486-2809-1-git-send-email-public_timo.s@silentcreek.de>

Hi,

On 27-07-15 03:28, Timo Sigurdsson wrote:
> sun7i-a20-bananapi.dts doesn't contain regulator nodes for the AXP209 PMU
> driver, so add them to allow for voltage-scaling with cpufreq-dt. With the
> regulators enabled, we can define board-specific operating points. The defined
> CPU voltages are more conservative (based on the values used by the vendor),
> so they should be more failsafe across all boards of this kind out there.
>
> I'm posting this as RFC as I would like to make a few more remarks and raise
> questions along the way (plus, I'm anything but an experienced developer, so a
> a critical review might help).
>
> I checked the regulator definitions against the schematics released by LeMaker.
> I also compared that to the DT and schematics of Cubiboard 2 and Cubietruck. Of
> course, I also tested the patch on the actual hardware and it works fine for me.
> The CPU voltages are slightly higher than those set in sun7i-a20.dtsi and even
> though they work well for me, I thought it might be safer to use the more
> conservative values used by LeMaker in their old fex file. Would you agree?
> Besides, it it ok to have this in one patch or should it be splitted in two
> (one for the regulators and one for the opp)? Did I miss something important?
>
> Signed-off-by: Timo Sigurdsson <public_timo.s@silentcreek.de>

I've a simular patch here:

https://github.com/jwrdegoede/linux-sunxi/commit/6a30b7d5be6012b81e5e1439a444e41c0ac1afc1

I did not submit this upstream yet as it is part of a series to enable the otg
controller on the bananapi which needs axp-usb-power-supply support for which
the actual powersupply driver changes are still pending.

As you can see other then you adding the cpu operating points are patches are
identical, which is good :)

IMHO we should just stick with the standard operating points unless we know
that there are stability issues with them (such as e.g. on the A10 OlinuxIno Lime).

Regards,

Hans



> ---
>   arch/arm/boot/dts/sun7i-a20-bananapi.dts | 47 +++++++++++++++++++++++++++++---
>   1 file changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/sun7i-a20-bananapi.dts b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> index 9f7b472..2bcbb0e 100644
> --- a/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> +++ b/arch/arm/boot/dts/sun7i-a20-bananapi.dts
> @@ -92,6 +92,22 @@
>   	status = "okay";
>   };
>
> +&cpu0 {
> +	cpu-supply = <&reg_dcdc2>;
> +	operating-points = <
> +		/* kHz	  uV */
> +		1008000	1450000
> +		960000	1425000
> +		912000	1425000
> +		864000	1350000
> +		720000	1250000
> +		528000	1150000
> +		312000	1100000
> +		144000	1050000
> +		>;
> +	cooling-max-level = <7>;
> +};
> +
>   &ehci0 {
>   	status = "okay";
>   };
> @@ -119,13 +135,9 @@
>   	status = "okay";
>
>   	axp209: pmic@34 {
> -		compatible = "x-powers,axp209";
>   		reg = <0x34>;
>   		interrupt-parent = <&nmi_intc>;
>   		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> -
> -		interrupt-controller;
> -		#interrupt-cells = <1>;
>   	};
>   };
>
> @@ -182,6 +194,33 @@
>   	};
>   };
>
> +#include "axp209.dtsi"
> +
> +&reg_dcdc2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1050000>;
> +	regulator-max-microvolt = <1450000>;
> +	regulator-name = "vdd-cpu";
> +};
> +
> +&reg_dcdc3 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <1000000>;
> +	regulator-max-microvolt = <1400000>;
> +	regulator-name = "vdd-int-dll";
> +};
> +
> +&reg_ldo1 {
> +	regulator-name = "vdd-rtc";
> +};
> +
> +&reg_ldo2 {
> +	regulator-always-on;
> +	regulator-min-microvolt = <3000000>;
> +	regulator-max-microvolt = <3000000>;
> +	regulator-name = "avcc";
> +};
> +
>   &reg_usb1_vbus {
>   	status = "okay";
>   };
>

  reply	other threads:[~2015-07-27  8:07 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27  1:28 [RFC] ARM: dts: sunxi: Add regulators and board-specific operating points for LeMaker BananaPi Timo Sigurdsson
2015-07-27  1:28 ` Timo Sigurdsson
2015-07-27  1:28 ` Timo Sigurdsson
2015-07-27  8:07 ` Hans de Goede [this message]
2015-07-27  8:07   ` [linux-sunxi] " Hans de Goede
2015-07-27  8:07   ` Hans de Goede
2015-07-27 12:09   ` [linux-sunxi] " public_timo.s at silentcreek.de
2015-07-27 12:09     ` public_timo.s
2015-07-27 12:09     ` public_timo.s-fWgRPtSzPNU3WX+qO2AYSQ
2015-07-27 12:43     ` Hans de Goede
2015-07-27 12:43       ` Hans de Goede
2015-07-27 12:43       ` Hans de Goede
2015-07-28  9:02       ` Timo Sigurdsson
2015-07-28  9:02         ` Timo Sigurdsson
2015-07-28  9:02         ` Timo Sigurdsson
2015-07-28 12:55         ` [linux-sunxi] " Maxime Ripard
2015-07-28 12:55           ` Maxime Ripard
2015-07-28 12:55           ` Maxime Ripard
2015-07-28 14:57           ` [linux-sunxi] " Timo Sigurdsson
2015-07-28 14:57             ` Timo Sigurdsson
2015-07-28 14:57             ` Timo Sigurdsson
2015-07-28 12:49       ` [linux-sunxi] " Maxime Ripard
2015-07-28 12:49         ` Maxime Ripard
2015-07-28 12:49         ` Maxime Ripard
2015-07-28 14:24         ` Hans de Goede
2015-07-28 14:24           ` Hans de Goede
2015-07-28 14:24           ` Hans de Goede
2015-07-28 15:09           ` [linux-sunxi] " Timo Sigurdsson
2015-07-28 15:09             ` Timo Sigurdsson
2015-07-28 15:09             ` Timo Sigurdsson
2015-07-28 15:29             ` [linux-sunxi] " Hans de Goede
2015-07-28 15:29               ` Hans de Goede
2015-07-28 15:29               ` Hans de Goede
2015-08-02 22:00               ` Timo Sigurdsson
2015-08-02 22:00                 ` Timo Sigurdsson
2015-08-02 22:00                 ` Timo Sigurdsson
2015-07-28 14:45         ` [linux-sunxi] " Timo Sigurdsson
2015-07-28 14:45           ` Timo Sigurdsson
2015-07-28 14:45           ` Timo Sigurdsson
2015-07-27 12:36   ` [linux-sunxi] " public_timo.s at silentcreek.de
2015-07-27 12:36     ` public_timo.s
2015-07-27 12:36     ` public_timo.s-fWgRPtSzPNU3WX+qO2AYSQ
2015-07-27 12:54     ` [linux-sunxi] " Hans de Goede
2015-07-27 12:54       ` Hans de Goede
2015-07-27 12:54       ` Hans de Goede
     [not found]       ` <55B62A1B.6030601-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-07-27 13:14         ` Chen-Yu Tsai
2015-07-28  9:02           ` [linux-sunxi] " Timo Sigurdsson
2015-07-28  9:02             ` Timo Sigurdsson
2015-07-28  9:02             ` Timo Sigurdsson
2015-07-28 12:55             ` [linux-sunxi] " Maxime Ripard
2015-07-28 12:55               ` Maxime Ripard
2015-07-28 12:55               ` Maxime Ripard
2015-07-28 15:01               ` [linux-sunxi] " Timo Sigurdsson
2015-07-28 15:01                 ` Timo Sigurdsson
2015-07-28 15:01                 ` Timo Sigurdsson
2015-07-28 23:02             ` Stefan Monnier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55B5E6DB.8020009@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.