[PATCH 0/3] ARM: bcm2835: add support for rpi power domain driver

[PATCH 0/3] ARM: bcm2835: add support for rpi power domain driver

[Alexander Aring]

Hi,

this patch series contains at first a patch for the power domain subsystem
which offers to uninit generic power domains when init was called before.

The RPi Power-Domains need to be enabled/disabled by interacting with the
RPi firmware which can fail. To cleanup the probing we need to undo the
power domains again which was registered before.

- Alex

changes since PATCH (was RFCv2 before):
 - add WARN_ON_ONCE if there are still references from generic power domain
   inside the power domain subsystem when running pm_genpd_uninit.
 - add mutex_destroy when running pm_genpd_uninit.
 - split devicetree binding for rpi power domain driver into a separate patch.
 - rename config RASPBERRY_POWER to RASPBERRYPI_POWER
 - order list of <linux/...> includes alphabetical.
 - use rpi_ prefix than raspberrypi_ prefix.
 - rename _DT_BINDINGS_ARM_BCM2835_MBOX_POWER_H to _DT_BINDINGS_ARM_BCM2835_RPI_POWER_H

changes since RFCv2:
 - add pm_genpd_uninit to handle probing failure.
 - move power domain drive to his own driver in arch/arm/mach-bcm/
   Also add own devicetree node for this driver, "raspberrypi,bcm2835-power".
 - Removing all power domains which might exists for the firmware API but
   we currently have no use-case for it. I tried to keep the same domain
   numbering in generic power domains subsystem like they are offered from
   the firmware API. This works, all power_domains which are NULL inside
   the array of genpd_onecell_data.domains[#] will be ignored.
 - Adding Eric Anholt and me to the authors.
 - Creating devicetree documentation for the power domain driver.
 - fix error handling in raspberrypi_firmware_set_power.
 - Remove comment about mapping between power domains array, this is not
   necessary anymore. I add a "enabled" attribute to raspberrypi_power_domain
   which indicates if a domain should be registered or not (zeroed values
   does not indicate such handling, but enabled is false then).
 - remove "goto mbox" not necessary anymore because an own driver
   implementation.
 - Update devicetrees for changes in v2.

Alexander Aring (3):
  power: domain: add pm_genpd_uninit
  ARM: bcm2835: add rpi power domain driver
  devicetree: add rpi power domain driver bindings

 .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt |  25 +++
 arch/arm/boot/dts/bcm2835-rpi.dtsi                 |  11 ++
 arch/arm/boot/dts/bcm2835.dtsi                     |   2 +-
 arch/arm/mach-bcm/Kconfig                          |  10 ++
 arch/arm/mach-bcm/Makefile                         |   1 +
 arch/arm/mach-bcm/raspberrypi-power.c              | 180 +++++++++++++++++++++
 drivers/base/power/domain.c                        |  22 +++
 include/dt-bindings/arm/raspberrypi-power.h        |  14 ++
 include/linux/pm_domain.h                          |   4 +
 9 files changed, 268 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
 create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c
 create mode 100644 include/dt-bindings/arm/raspberrypi-power.h

-- 
2.6.1

--
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

[PATCH 0/3] ARM: bcm2835: add support for rpi power domain driver

[Alexander Aring]

Hi,

this patch series contains at first a patch for the power domain subsystem
which offers to uninit generic power domains when init was called before.

The RPi Power-Domains need to be enabled/disabled by interacting with the
RPi firmware which can fail. To cleanup the probing we need to undo the
power domains again which was registered before.

- Alex

changes since PATCH (was RFCv2 before):
 - add WARN_ON_ONCE if there are still references from generic power domain
   inside the power domain subsystem when running pm_genpd_uninit.
 - add mutex_destroy when running pm_genpd_uninit.
 - split devicetree binding for rpi power domain driver into a separate patch.
 - rename config RASPBERRY_POWER to RASPBERRYPI_POWER
 - order list of <linux/...> includes alphabetical.
 - use rpi_ prefix than raspberrypi_ prefix.
 - rename _DT_BINDINGS_ARM_BCM2835_MBOX_POWER_H to _DT_BINDINGS_ARM_BCM2835_RPI_POWER_H

changes since RFCv2:
 - add pm_genpd_uninit to handle probing failure.
 - move power domain drive to his own driver in arch/arm/mach-bcm/
   Also add own devicetree node for this driver, "raspberrypi,bcm2835-power".
 - Removing all power domains which might exists for the firmware API but
   we currently have no use-case for it. I tried to keep the same domain
   numbering in generic power domains subsystem like they are offered from
   the firmware API. This works, all power_domains which are NULL inside
   the array of genpd_onecell_data.domains[#] will be ignored.
 - Adding Eric Anholt and me to the authors.
 - Creating devicetree documentation for the power domain driver.
 - fix error handling in raspberrypi_firmware_set_power.
 - Remove comment about mapping between power domains array, this is not
   necessary anymore. I add a "enabled" attribute to raspberrypi_power_domain
   which indicates if a domain should be registered or not (zeroed values
   does not indicate such handling, but enabled is false then).
 - remove "goto mbox" not necessary anymore because an own driver
   implementation.
 - Update devicetrees for changes in v2.

Alexander Aring (3):
  power: domain: add pm_genpd_uninit
  ARM: bcm2835: add rpi power domain driver
  devicetree: add rpi power domain driver bindings

 .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt |  25 +++
 arch/arm/boot/dts/bcm2835-rpi.dtsi                 |  11 ++
 arch/arm/boot/dts/bcm2835.dtsi                     |   2 +-
 arch/arm/mach-bcm/Kconfig                          |  10 ++
 arch/arm/mach-bcm/Makefile                         |   1 +
 arch/arm/mach-bcm/raspberrypi-power.c              | 180 +++++++++++++++++++++
 drivers/base/power/domain.c                        |  22 +++
 include/dt-bindings/arm/raspberrypi-power.h        |  14 ++
 include/linux/pm_domain.h                          |   4 +
 9 files changed, 268 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
 create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c
 create mode 100644 include/dt-bindings/arm/raspberrypi-power.h

-- 
2.6.1

[PATCH 1/3] power: domain: add pm_genpd_uninit

[Alexander Aring]

This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This
is useful for multiple power domains while probing. If the probing fails
after one pm_genpd_init was called we need to undo all previous
registrations of generic pm domains inside the gpd_list list.

There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again
registered power domains and not registered domains, the driver can use
this mechanism to have an array with registered and non-registered power
domains, where non-registered power domains are NULL.

Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/base/power/domain.c | 22 ++++++++++++++++++++++
 include/linux/pm_domain.h   |  4 ++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e03b1ad..24f54b8 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1509,6 +1509,28 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 }
 EXPORT_SYMBOL_GPL(pm_genpd_init);
 
+/**
+ * pm_genpd_uninit - Uninitialize a generic I/O PM domain object.
+ * @genpd: PM domain object to initialize.
+ */
+void pm_genpd_uninit(struct generic_pm_domain *genpd)
+{
+	if (IS_ERR_OR_NULL(genpd))
+		return;
+
+	/* check if domain is still in registered inside the pm subsystem */
+	WARN_ON_ONCE(!list_empty(&genpd->master_links) ||
+		     !list_empty(&genpd->slave_links) ||
+		     !list_empty(&genpd->dev_list));
+
+	mutex_lock(&gpd_list_lock);
+	list_del(&genpd->gpd_list_node);
+	mutex_unlock(&gpd_list_lock);
+
+	mutex_destroy(&genpd->lock);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_uninit);
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index ba4ced3..df84a45 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -123,6 +123,7 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 				     struct generic_pm_domain *target);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
+extern void pm_genpd_uninit(struct generic_pm_domain *genpd);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
@@ -161,6 +162,9 @@ static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off)
 {
 }
+static inline void pm_genpd_uninit(struct generic_pm_domain *genpd)
+{
+}
 #endif
 
 static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
-- 
2.6.1

[PATCH 1/3] power: domain: add pm_genpd_uninit

[Alexander Aring]

This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This
is useful for multiple power domains while probing. If the probing fails
after one pm_genpd_init was called we need to undo all previous
registrations of generic pm domains inside the gpd_list list.

There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again
registered power domains and not registered domains, the driver can use
this mechanism to have an array with registered and non-registered power
domains, where non-registered power domains are NULL.

Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Kevin Hilman <khilman@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 drivers/base/power/domain.c | 22 ++++++++++++++++++++++
 include/linux/pm_domain.h   |  4 ++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e03b1ad..24f54b8 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1509,6 +1509,28 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 }
 EXPORT_SYMBOL_GPL(pm_genpd_init);
 
+/**
+ * pm_genpd_uninit - Uninitialize a generic I/O PM domain object.
+ * @genpd: PM domain object to initialize.
+ */
+void pm_genpd_uninit(struct generic_pm_domain *genpd)
+{
+	if (IS_ERR_OR_NULL(genpd))
+		return;
+
+	/* check if domain is still in registered inside the pm subsystem */
+	WARN_ON_ONCE(!list_empty(&genpd->master_links) ||
+		     !list_empty(&genpd->slave_links) ||
+		     !list_empty(&genpd->dev_list));
+
+	mutex_lock(&gpd_list_lock);
+	list_del(&genpd->gpd_list_node);
+	mutex_unlock(&gpd_list_lock);
+
+	mutex_destroy(&genpd->lock);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_uninit);
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index ba4ced3..df84a45 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -123,6 +123,7 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 				     struct generic_pm_domain *target);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
+extern void pm_genpd_uninit(struct generic_pm_domain *genpd);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
@@ -161,6 +162,9 @@ static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off)
 {
 }
+static inline void pm_genpd_uninit(struct generic_pm_domain *genpd)
+{
+}
 #endif
 
 static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
-- 
2.6.1

Re: [PATCH 1/3] power: domain: add pm_genpd_uninit

[Ulf Hansson]

On 19 November 2015 at 19:08, Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This
> is useful for multiple power domains while probing. If the probing fails
> after one pm_genpd_init was called we need to undo all previous
> registrations of generic pm domains inside the gpd_list list.
>
> There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again
> registered power domains and not registered domains, the driver can use
> this mechanism to have an array with registered and non-registered power
> domains, where non-registered power domains are NULL.
>
> Cc: Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>
> Cc: Kevin Hilman <khilman-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> Cc: Len Brown <len.brown-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/base/power/domain.c | 22 ++++++++++++++++++++++
>  include/linux/pm_domain.h   |  4 ++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e03b1ad..24f54b8 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1509,6 +1509,28 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>  }
>  EXPORT_SYMBOL_GPL(pm_genpd_init);
>
> +/**
> + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object.
> + * @genpd: PM domain object to initialize.
> + */
> +void pm_genpd_uninit(struct generic_pm_domain *genpd)
> +{
> +       if (IS_ERR_OR_NULL(genpd))
> +               return;
> +
> +       /* check if domain is still in registered inside the pm subsystem */
> +       WARN_ON_ONCE(!list_empty(&genpd->master_links) ||
> +                    !list_empty(&genpd->slave_links) ||
> +                    !list_empty(&genpd->dev_list));
> +

We did discuss about verifying that the genpd mustn't have a
corresponding DT provider. I do realize that it becomes a bit complex
to verify that, unless we decide to add some handle to it within the
genpd struct.

Anyway, perhaps this minimal effort is good enough as is. Especially
since we won't be able to handle the error cases, besides giving a
WARN.

> +       mutex_lock(&gpd_list_lock);
> +       list_del(&genpd->gpd_list_node);
> +       mutex_unlock(&gpd_list_lock);
> +
> +       mutex_destroy(&genpd->lock);
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_uninit);
> +
>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>  /*
>   * Device Tree based PM domain providers.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index ba4ced3..df84a45 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -123,6 +123,7 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
>                                      struct generic_pm_domain *target);
>  extern void pm_genpd_init(struct generic_pm_domain *genpd,
>                           struct dev_power_governor *gov, bool is_off);
> +extern void pm_genpd_uninit(struct generic_pm_domain *genpd);
>
>  extern struct dev_power_governor simple_qos_governor;
>  extern struct dev_power_governor pm_domain_always_on_gov;
> @@ -161,6 +162,9 @@ static inline void pm_genpd_init(struct generic_pm_domain *genpd,
>                                  struct dev_power_governor *gov, bool is_off)
>  {
>  }
> +static inline void pm_genpd_uninit(struct generic_pm_domain *genpd)
> +{
> +}
>  #endif
>
>  static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
> --
> 2.6.1
>

So, I am fine with this, but let's see if other people have objections.

Acked-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Kind regards
Uffe
--
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

[PATCH 1/3] power: domain: add pm_genpd_uninit

[Ulf Hansson]

On 19 November 2015 at 19:08, Alexander Aring <alex.aring@gmail.com> wrote:
> This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This
> is useful for multiple power domains while probing. If the probing fails
> after one pm_genpd_init was called we need to undo all previous
> registrations of generic pm domains inside the gpd_list list.
>
> There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again
> registered power domains and not registered domains, the driver can use
> this mechanism to have an array with registered and non-registered power
> domains, where non-registered power domains are NULL.
>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  drivers/base/power/domain.c | 22 ++++++++++++++++++++++
>  include/linux/pm_domain.h   |  4 ++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e03b1ad..24f54b8 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1509,6 +1509,28 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>  }
>  EXPORT_SYMBOL_GPL(pm_genpd_init);
>
> +/**
> + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object.
> + * @genpd: PM domain object to initialize.
> + */
> +void pm_genpd_uninit(struct generic_pm_domain *genpd)
> +{
> +       if (IS_ERR_OR_NULL(genpd))
> +               return;
> +
> +       /* check if domain is still in registered inside the pm subsystem */
> +       WARN_ON_ONCE(!list_empty(&genpd->master_links) ||
> +                    !list_empty(&genpd->slave_links) ||
> +                    !list_empty(&genpd->dev_list));
> +

We did discuss about verifying that the genpd mustn't have a
corresponding DT provider. I do realize that it becomes a bit complex
to verify that, unless we decide to add some handle to it within the
genpd struct.

Anyway, perhaps this minimal effort is good enough as is. Especially
since we won't be able to handle the error cases, besides giving a
WARN.

> +       mutex_lock(&gpd_list_lock);
> +       list_del(&genpd->gpd_list_node);
> +       mutex_unlock(&gpd_list_lock);
> +
> +       mutex_destroy(&genpd->lock);
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_uninit);
> +
>  #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>  /*
>   * Device Tree based PM domain providers.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index ba4ced3..df84a45 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -123,6 +123,7 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
>                                      struct generic_pm_domain *target);
>  extern void pm_genpd_init(struct generic_pm_domain *genpd,
>                           struct dev_power_governor *gov, bool is_off);
> +extern void pm_genpd_uninit(struct generic_pm_domain *genpd);
>
>  extern struct dev_power_governor simple_qos_governor;
>  extern struct dev_power_governor pm_domain_always_on_gov;
> @@ -161,6 +162,9 @@ static inline void pm_genpd_init(struct generic_pm_domain *genpd,
>                                  struct dev_power_governor *gov, bool is_off)
>  {
>  }
> +static inline void pm_genpd_uninit(struct generic_pm_domain *genpd)
> +{
> +}
>  #endif
>
>  static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
> --
> 2.6.1
>

So, I am fine with this, but let's see if other people have objections.

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

Re: [PATCH 1/3] power: domain: add pm_genpd_uninit

[Kevin Hilman]

Alexander Aring <alex.aring@gmail.com> writes:

> This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This
> is useful for multiple power domains while probing. If the probing fails
> after one pm_genpd_init was called we need to undo all previous
> registrations of generic pm domains inside the gpd_list list.
>
> There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again
> registered power domains and not registered domains, the driver can use
> this mechanism to have an array with registered and non-registered power
> domains, where non-registered power domains are NULL.
>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  drivers/base/power/domain.c | 22 ++++++++++++++++++++++
>  include/linux/pm_domain.h   |  4 ++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e03b1ad..24f54b8 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1509,6 +1509,28 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>  }
>  EXPORT_SYMBOL_GPL(pm_genpd_init);
>  
> +/**
> + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object.
> + * @genpd: PM domain object to initialize.
> + */
> +void pm_genpd_uninit(struct generic_pm_domain *genpd)
> +{
> +	if (IS_ERR_OR_NULL(genpd))
> +		return;
> +
> +	/* check if domain is still in registered inside the pm subsystem */
> +	WARN_ON_ONCE(!list_empty(&genpd->master_links) ||
> +		     !list_empty(&genpd->slave_links) ||
> +		     !list_empty(&genpd->dev_list));

So we WARN if there are parents/children/devices, but shouldn't this
return an error instead of continuing to remove/uninit the genpd?

The caller of _uninit should probably also be removing any
parents/children and removing devices before calling _uninit, no?

> +	mutex_lock(&gpd_list_lock);
> +	list_del(&genpd->gpd_list_node);
> +	mutex_unlock(&gpd_list_lock);
> +
> +	mutex_destroy(&genpd->lock);
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_uninit);

Kevin

[PATCH 1/3] power: domain: add pm_genpd_uninit

[Kevin Hilman]

Alexander Aring <alex.aring@gmail.com> writes:

> This patch adds function pm_genpd_uninit for undo a pm_genpd_init. This
> is useful for multiple power domains while probing. If the probing fails
> after one pm_genpd_init was called we need to undo all previous
> registrations of generic pm domains inside the gpd_list list.
>
> There is a check on IS_ERR_OR_NULL(genpd) which is useful to check again
> registered power domains and not registered domains, the driver can use
> this mechanism to have an array with registered and non-registered power
> domains, where non-registered power domains are NULL.
>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Kevin Hilman <khilman@kernel.org>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> ---
>  drivers/base/power/domain.c | 22 ++++++++++++++++++++++
>  include/linux/pm_domain.h   |  4 ++++
>  2 files changed, 26 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index e03b1ad..24f54b8 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1509,6 +1509,28 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>  }
>  EXPORT_SYMBOL_GPL(pm_genpd_init);
>  
> +/**
> + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object.
> + * @genpd: PM domain object to initialize.
> + */
> +void pm_genpd_uninit(struct generic_pm_domain *genpd)
> +{
> +	if (IS_ERR_OR_NULL(genpd))
> +		return;
> +
> +	/* check if domain is still in registered inside the pm subsystem */
> +	WARN_ON_ONCE(!list_empty(&genpd->master_links) ||
> +		     !list_empty(&genpd->slave_links) ||
> +		     !list_empty(&genpd->dev_list));

So we WARN if there are parents/children/devices, but shouldn't this
return an error instead of continuing to remove/uninit the genpd?

The caller of _uninit should probably also be removing any
parents/children and removing devices before calling _uninit, no?

> +	mutex_lock(&gpd_list_lock);
> +	list_del(&genpd->gpd_list_node);
> +	mutex_unlock(&gpd_list_lock);
> +
> +	mutex_destroy(&genpd->lock);
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_uninit);

Kevin

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Alexander Aring]

This patch adds support for RPi several Power Domains and enable support
to enable the USB Power Domain when it's not enabled before.

This patch based on Eric Anholt's patch to support Power Domains. He had
an issue about -EPROBE_DEFER inside the power domain subsystem, this
issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
if we fail to init or turn-on domain").

It was tested with barebox and the following scripts before booting
linux:

/env/a_off:

 # cat /env/a_off
 #turn off which are enabled by default
 regulator -n bcm2835_mci0 -s disable
 regulator -n uart0-pl0110 -s disable

/env/a_on:

 # cat /env/a_on
 #turn off which are enabled by default
 regulator -n bcm2835_mci0 -s disable
 regulator -n uart0-pl0110 -s disable

 regulator -n bcm2835_mci0 -s enable
 regulator -n uart0-pl0110 -s enable
 regulator -n uart0-pl0111 -s enable
 regulator -n bcm2835_usb -s enable
 regulator -n bcm2835_i2c0 -s enable
 regulator -n bcm2835_i2c1 -s enable
 regulator -n bcm2835_i2c2 -s enable
 regulator -n bcm2835_spi -s enable
 regulator -n bcm2835_ccp2tx -s enable
 regulator -n bcm2835_dsi -s enable

/env/b:

 # cat /env/b
 sh /env/a_on

 regulator -n bcm2835_mci0 -s disable
 regulator -n uart0-pl0110 -s disable
 regulator -n uart0-pl0111 -s disable
 regulator -n bcm2835_usb -s disable
 regulator -n bcm2835_i2c0 -s disable
 regulator -n bcm2835_i2c1 -s disable
 regulator -n bcm2835_i2c2 -s disable
 regulator -n bcm2835_spi -s disable
 regulator -n bcm2835_ccp2tx -s disable
 regulator -n bcm2835_dsi -s disable

/env/c:

 # cat /env/c
 sh ./env/b

 regulator -n bcm2835_mci0 -s enable
 regulator -n uart0-pl0110 -s enable
 regulator -n uart0-pl0111 -s enable
 regulator -n bcm2835_usb -s enable
 regulator -n bcm2835_i2c0 -s enable
 regulator -n bcm2835_i2c1 -s enable
 regulator -n bcm2835_i2c2 -s enable
 regulator -n bcm2835_spi -s enable
 regulator -n bcm2835_ccp2tx -s enable
 regulator -n bcm2835_dsi -s enable

These scripts enables/disable all regulators inside the bootloader. It
was running with a "hard" and "soft" reset without any issues. These
testcases should fit to Stephen Warren suggestions:

"(a) before having explicitly turned the power domain on or off at all (b)
after having turned it on (c) after having turned it off, and for all
power domains."

Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Lee Jones <lee@kernel.org>
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 arch/arm/boot/dts/bcm2835-rpi.dtsi          |  11 ++
 arch/arm/boot/dts/bcm2835.dtsi              |   2 +-
 arch/arm/mach-bcm/Kconfig                   |  10 ++
 arch/arm/mach-bcm/Makefile                  |   1 +
 arch/arm/mach-bcm/raspberrypi-power.c       | 180 ++++++++++++++++++++++++++++
 include/dt-bindings/arm/raspberrypi-power.h |  14 +++
 6 files changed, 217 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c
 create mode 100644 include/dt-bindings/arm/raspberrypi-power.h

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 3572f03..f828202 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -1,3 +1,4 @@
+#include <dt-bindings/arm/raspberrypi-power.h>
 #include "bcm2835.dtsi"
 
 / {
@@ -20,6 +21,12 @@
 			compatible = "raspberrypi,bcm2835-firmware";
 			mboxes = <&mailbox>;
 		};
+
+		power: power {
+			compatible = "raspberrypi,bcm2835-power";
+			firmware = <&firmware>;
+			#power-domain-cells = <1>;
+		};
 	};
 };
 
@@ -60,3 +67,7 @@
 	status = "okay";
 	bus-width = <4>;
 };
+
+&usb {
+	power-domains = <&power RPI_POWER_DOMAIN_USB>;
+};
diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index aef64de..6d62af0 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -177,7 +177,7 @@
 			status = "disabled";
 		};
 
-		usb@7e980000 {
+		usb: usb@7e980000 {
 			compatible = "brcm,bcm2835-usb";
 			reg = <0x7e980000 0x10000>;
 			interrupts = <1 9>;
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 8c53c55..20479d7 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -134,6 +134,16 @@ config ARCH_BCM2835
 	  This enables support for the Broadcom BCM2835 SoC. This SoC is
 	  used in the Raspberry Pi and Roku 2 devices.
 
+config RASPBERRYPI_POWER
+	bool "Raspberry Pi power domain driver"
+	depends on ARCH_BCM2835
+	depends on RASPBERRYPI_FIRMWARE
+	select PM_GENERIC_DOMAINS if PM
+	select PM_GENERIC_DOMAINS_OF if PM
+	help
+	  This enables support for the RPi power domains which can be enabled
+	  or disabled via the RPi firmware.
+
 config ARCH_BCM_63XX
 	bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7
 	depends on MMU
diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 892261f..fec2d6b 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -36,6 +36,7 @@ endif
 
 # BCM2835
 obj-$(CONFIG_ARCH_BCM2835)	+= board_bcm2835.o
+obj-$(CONFIG_RASPBERRYPI_POWER)	+= raspberrypi-power.o
 
 # BCM5301X
 obj-$(CONFIG_ARCH_BCM_5301X)	+= bcm_5301x.o
diff --git a/arch/arm/mach-bcm/raspberrypi-power.c b/arch/arm/mach-bcm/raspberrypi-power.c
new file mode 100644
index 0000000..ee77e45
--- /dev/null
+++ b/arch/arm/mach-bcm/raspberrypi-power.c
@@ -0,0 +1,180 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Authors:
+ * (C) 2015 Pengutronix, Alexander Aring <aar@pengutronix.de>
+ * Eric Anholt <eric@anholt.net>
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <dt-bindings/arm/raspberrypi-power.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define RPI_POWER_DOMAIN(_domain, _name)			\
+	[_domain] = {						\
+		.domain = _domain,				\
+		.enabled = true,				\
+		.base = {					\
+			.name = _name,				\
+			.power_off = rpi_domain_off,		\
+			.power_on = rpi_domain_on,		\
+		},						\
+	}
+
+struct rpi_power_domain {
+	u32 domain;
+	bool enabled;
+	struct generic_pm_domain base;
+};
+
+struct rpi_power_domain_packet {
+	u32 domain;
+	u32 on;
+} __packet;
+
+static struct rpi_firmware *fw;
+static struct genpd_onecell_data rpi_genpd_xlate;
+
+/*
+ * Asks the firmware to enable or disable power on a specific power
+ * domain.
+ */
+static int rpi_firmware_set_power(u32 domain, bool on)
+{
+	struct rpi_power_domain_packet packet;
+
+	packet.domain = domain;
+	packet.on = on;
+	return rpi_firmware_property(fw, RPI_FIRMWARE_SET_POWER_STATE, &packet,
+				     sizeof(packet));
+}
+
+/* Asks the firmware to if power is on for a specific power domain. */
+static int rpi_firmware_power_is_on(u32 domain)
+{
+	struct rpi_power_domain_packet packet;
+	int ret;
+
+	packet.domain = domain;
+	ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_POWER_STATE, &packet,
+				    sizeof(packet));
+	if (ret < 0)
+		return ret;
+
+	return packet.on & BIT(0);
+}
+
+static int rpi_domain_off(struct generic_pm_domain *domain)
+{
+	struct rpi_power_domain *rpi_domain =
+		container_of(domain, struct rpi_power_domain, base);
+
+	return rpi_firmware_set_power(rpi_domain->domain, false);
+}
+
+static int rpi_domain_on(struct generic_pm_domain *domain)
+{
+	struct rpi_power_domain *rpi_domain =
+		container_of(domain, struct rpi_power_domain, base);
+
+	return rpi_firmware_set_power(rpi_domain->domain, true);
+}
+
+static struct rpi_power_domain rpi_power_domains[] = {
+	RPI_POWER_DOMAIN(RPI_POWER_DOMAIN_USB, "USB"),
+};
+
+static int rpi_power_probe(struct platform_device *pdev)
+{
+	struct device_node *fw_np;
+	struct device *dev = &pdev->dev;
+	struct generic_pm_domain **power_domains;
+	int i, ret, num_domains = ARRAY_SIZE(rpi_power_domains);
+
+	fw_np = of_parse_phandle(pdev->dev.of_node, "firmware", 0);
+	if (!fw_np) {
+		dev_err(&pdev->dev, "no firmware node\n");
+		return -ENODEV;
+	}
+
+	fw = rpi_firmware_get(fw_np);
+	if (!fw)
+		return -EPROBE_DEFER;
+
+	power_domains = devm_kzalloc(dev, sizeof(*power_domains) * num_domains,
+				     GFP_KERNEL);
+	if (!power_domains)
+		return -ENOMEM;
+
+	rpi_genpd_xlate.domains = power_domains;
+	rpi_genpd_xlate.num_domains = num_domains;
+
+	for (i = 0; i < num_domains; i++) {
+		bool is_off;
+
+		if (!rpi_power_domains[i].enabled)
+			continue;
+
+		/* get the initial state */
+		ret = rpi_firmware_power_is_on(rpi_power_domains[i].domain);
+		if (ret < 0)
+			goto uninit_pm;
+
+		/* pm_genpd_init needs is_off, invert the logic here */
+		is_off = !ret;
+		pm_genpd_init(&rpi_power_domains[i].base, NULL, is_off);
+		/* let power_domains array know about the registered pm */
+		power_domains[i] = &rpi_power_domains[i].base;
+	}
+
+	ret = of_genpd_add_provider_onecell(dev->of_node, &rpi_genpd_xlate);
+	if (ret < 0)
+		goto uninit_pm;
+
+	return 0;
+
+uninit_pm:
+	for (i = 0; i < num_domains; i++)
+		pm_genpd_uninit(power_domains[i]);
+
+	return ret;
+}
+
+static int rpi_power_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	int i;
+
+	for (i = 0; i < rpi_genpd_xlate.num_domains; i++)
+		pm_genpd_uninit(rpi_genpd_xlate.domains[i]);
+
+	of_genpd_del_provider(dev->of_node);
+
+	return 0;
+}
+
+static const struct of_device_id rpi_power_of_match[] = {
+	{ .compatible = "raspberrypi,bcm2835-power", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rpi_power_of_match);
+
+static struct platform_driver rpi_power_driver = {
+	.driver = {
+		.name = "raspberrypi-power",
+		.of_match_table = rpi_power_of_match,
+	},
+	.probe		= rpi_power_probe,
+	.remove		= rpi_power_remove,
+};
+module_platform_driver(rpi_power_driver);
+
+MODULE_AUTHOR("Alexander Aring <aar@pengutronix.de>");
+MODULE_AUTHOR("Eric Anholt <eric@anholt.net>");
+MODULE_DESCRIPTION("Raspberry Pi power domain driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/arm/raspberrypi-power.h b/include/dt-bindings/arm/raspberrypi-power.h
new file mode 100644
index 0000000..c2ffbebc
--- /dev/null
+++ b/include/dt-bindings/arm/raspberrypi-power.h
@@ -0,0 +1,14 @@
+/*
+ *  Copyright © 2015 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _DT_BINDINGS_ARM_BCM2835_RPI_POWER_H
+#define _DT_BINDINGS_ARM_BCM2835_RPI_POWER_H
+
+#define RPI_POWER_DOMAIN_USB	3
+
+#endif /* _DT_BINDINGS_ARM_BCM2835_RPI_POWER_H */
-- 
2.6.1

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Alexander Aring]

This patch adds support for RPi several Power Domains and enable support
to enable the USB Power Domain when it's not enabled before.

This patch based on Eric Anholt's patch to support Power Domains. He had
an issue about -EPROBE_DEFER inside the power domain subsystem, this
issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
if we fail to init or turn-on domain").

It was tested with barebox and the following scripts before booting
linux:

/env/a_off:

 # cat /env/a_off
 #turn off which are enabled by default
 regulator -n bcm2835_mci0 -s disable
 regulator -n uart0-pl0110 -s disable

/env/a_on:

 # cat /env/a_on
 #turn off which are enabled by default
 regulator -n bcm2835_mci0 -s disable
 regulator -n uart0-pl0110 -s disable

 regulator -n bcm2835_mci0 -s enable
 regulator -n uart0-pl0110 -s enable
 regulator -n uart0-pl0111 -s enable
 regulator -n bcm2835_usb -s enable
 regulator -n bcm2835_i2c0 -s enable
 regulator -n bcm2835_i2c1 -s enable
 regulator -n bcm2835_i2c2 -s enable
 regulator -n bcm2835_spi -s enable
 regulator -n bcm2835_ccp2tx -s enable
 regulator -n bcm2835_dsi -s enable

/env/b:

 # cat /env/b
 sh /env/a_on

 regulator -n bcm2835_mci0 -s disable
 regulator -n uart0-pl0110 -s disable
 regulator -n uart0-pl0111 -s disable
 regulator -n bcm2835_usb -s disable
 regulator -n bcm2835_i2c0 -s disable
 regulator -n bcm2835_i2c1 -s disable
 regulator -n bcm2835_i2c2 -s disable
 regulator -n bcm2835_spi -s disable
 regulator -n bcm2835_ccp2tx -s disable
 regulator -n bcm2835_dsi -s disable

/env/c:

 # cat /env/c
 sh ./env/b

 regulator -n bcm2835_mci0 -s enable
 regulator -n uart0-pl0110 -s enable
 regulator -n uart0-pl0111 -s enable
 regulator -n bcm2835_usb -s enable
 regulator -n bcm2835_i2c0 -s enable
 regulator -n bcm2835_i2c1 -s enable
 regulator -n bcm2835_i2c2 -s enable
 regulator -n bcm2835_spi -s enable
 regulator -n bcm2835_ccp2tx -s enable
 regulator -n bcm2835_dsi -s enable

These scripts enables/disable all regulators inside the bootloader. It
was running with a "hard" and "soft" reset without any issues. These
testcases should fit to Stephen Warren suggestions:

"(a) before having explicitly turned the power domain on or off at all (b)
after having turned it on (c) after having turned it off, and for all
power domains."

Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Lee Jones <lee@kernel.org>
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 arch/arm/boot/dts/bcm2835-rpi.dtsi          |  11 ++
 arch/arm/boot/dts/bcm2835.dtsi              |   2 +-
 arch/arm/mach-bcm/Kconfig                   |  10 ++
 arch/arm/mach-bcm/Makefile                  |   1 +
 arch/arm/mach-bcm/raspberrypi-power.c       | 180 ++++++++++++++++++++++++++++
 include/dt-bindings/arm/raspberrypi-power.h |  14 +++
 6 files changed, 217 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c
 create mode 100644 include/dt-bindings/arm/raspberrypi-power.h

diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 3572f03..f828202 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -1,3 +1,4 @@
+#include <dt-bindings/arm/raspberrypi-power.h>
 #include "bcm2835.dtsi"
 
 / {
@@ -20,6 +21,12 @@
 			compatible = "raspberrypi,bcm2835-firmware";
 			mboxes = <&mailbox>;
 		};
+
+		power: power {
+			compatible = "raspberrypi,bcm2835-power";
+			firmware = <&firmware>;
+			#power-domain-cells = <1>;
+		};
 	};
 };
 
@@ -60,3 +67,7 @@
 	status = "okay";
 	bus-width = <4>;
 };
+
+&usb {
+	power-domains = <&power RPI_POWER_DOMAIN_USB>;
+};
diff --git a/arch/arm/boot/dts/bcm2835.dtsi b/arch/arm/boot/dts/bcm2835.dtsi
index aef64de..6d62af0 100644
--- a/arch/arm/boot/dts/bcm2835.dtsi
+++ b/arch/arm/boot/dts/bcm2835.dtsi
@@ -177,7 +177,7 @@
 			status = "disabled";
 		};
 
-		usb at 7e980000 {
+		usb: usb at 7e980000 {
 			compatible = "brcm,bcm2835-usb";
 			reg = <0x7e980000 0x10000>;
 			interrupts = <1 9>;
diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index 8c53c55..20479d7 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -134,6 +134,16 @@ config ARCH_BCM2835
 	  This enables support for the Broadcom BCM2835 SoC. This SoC is
 	  used in the Raspberry Pi and Roku 2 devices.
 
+config RASPBERRYPI_POWER
+	bool "Raspberry Pi power domain driver"
+	depends on ARCH_BCM2835
+	depends on RASPBERRYPI_FIRMWARE
+	select PM_GENERIC_DOMAINS if PM
+	select PM_GENERIC_DOMAINS_OF if PM
+	help
+	  This enables support for the RPi power domains which can be enabled
+	  or disabled via the RPi firmware.
+
 config ARCH_BCM_63XX
 	bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7
 	depends on MMU
diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index 892261f..fec2d6b 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -36,6 +36,7 @@ endif
 
 # BCM2835
 obj-$(CONFIG_ARCH_BCM2835)	+= board_bcm2835.o
+obj-$(CONFIG_RASPBERRYPI_POWER)	+= raspberrypi-power.o
 
 # BCM5301X
 obj-$(CONFIG_ARCH_BCM_5301X)	+= bcm_5301x.o
diff --git a/arch/arm/mach-bcm/raspberrypi-power.c b/arch/arm/mach-bcm/raspberrypi-power.c
new file mode 100644
index 0000000..ee77e45
--- /dev/null
+++ b/arch/arm/mach-bcm/raspberrypi-power.c
@@ -0,0 +1,180 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Authors:
+ * (C) 2015 Pengutronix, Alexander Aring <aar@pengutronix.de>
+ * Eric Anholt <eric@anholt.net>
+ */
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <dt-bindings/arm/raspberrypi-power.h>
+#include <soc/bcm2835/raspberrypi-firmware.h>
+
+#define RPI_POWER_DOMAIN(_domain, _name)			\
+	[_domain] = {						\
+		.domain = _domain,				\
+		.enabled = true,				\
+		.base = {					\
+			.name = _name,				\
+			.power_off = rpi_domain_off,		\
+			.power_on = rpi_domain_on,		\
+		},						\
+	}
+
+struct rpi_power_domain {
+	u32 domain;
+	bool enabled;
+	struct generic_pm_domain base;
+};
+
+struct rpi_power_domain_packet {
+	u32 domain;
+	u32 on;
+} __packet;
+
+static struct rpi_firmware *fw;
+static struct genpd_onecell_data rpi_genpd_xlate;
+
+/*
+ * Asks the firmware to enable or disable power on a specific power
+ * domain.
+ */
+static int rpi_firmware_set_power(u32 domain, bool on)
+{
+	struct rpi_power_domain_packet packet;
+
+	packet.domain = domain;
+	packet.on = on;
+	return rpi_firmware_property(fw, RPI_FIRMWARE_SET_POWER_STATE, &packet,
+				     sizeof(packet));
+}
+
+/* Asks the firmware to if power is on for a specific power domain. */
+static int rpi_firmware_power_is_on(u32 domain)
+{
+	struct rpi_power_domain_packet packet;
+	int ret;
+
+	packet.domain = domain;
+	ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_POWER_STATE, &packet,
+				    sizeof(packet));
+	if (ret < 0)
+		return ret;
+
+	return packet.on & BIT(0);
+}
+
+static int rpi_domain_off(struct generic_pm_domain *domain)
+{
+	struct rpi_power_domain *rpi_domain =
+		container_of(domain, struct rpi_power_domain, base);
+
+	return rpi_firmware_set_power(rpi_domain->domain, false);
+}
+
+static int rpi_domain_on(struct generic_pm_domain *domain)
+{
+	struct rpi_power_domain *rpi_domain =
+		container_of(domain, struct rpi_power_domain, base);
+
+	return rpi_firmware_set_power(rpi_domain->domain, true);
+}
+
+static struct rpi_power_domain rpi_power_domains[] = {
+	RPI_POWER_DOMAIN(RPI_POWER_DOMAIN_USB, "USB"),
+};
+
+static int rpi_power_probe(struct platform_device *pdev)
+{
+	struct device_node *fw_np;
+	struct device *dev = &pdev->dev;
+	struct generic_pm_domain **power_domains;
+	int i, ret, num_domains = ARRAY_SIZE(rpi_power_domains);
+
+	fw_np = of_parse_phandle(pdev->dev.of_node, "firmware", 0);
+	if (!fw_np) {
+		dev_err(&pdev->dev, "no firmware node\n");
+		return -ENODEV;
+	}
+
+	fw = rpi_firmware_get(fw_np);
+	if (!fw)
+		return -EPROBE_DEFER;
+
+	power_domains = devm_kzalloc(dev, sizeof(*power_domains) * num_domains,
+				     GFP_KERNEL);
+	if (!power_domains)
+		return -ENOMEM;
+
+	rpi_genpd_xlate.domains = power_domains;
+	rpi_genpd_xlate.num_domains = num_domains;
+
+	for (i = 0; i < num_domains; i++) {
+		bool is_off;
+
+		if (!rpi_power_domains[i].enabled)
+			continue;
+
+		/* get the initial state */
+		ret = rpi_firmware_power_is_on(rpi_power_domains[i].domain);
+		if (ret < 0)
+			goto uninit_pm;
+
+		/* pm_genpd_init needs is_off, invert the logic here */
+		is_off = !ret;
+		pm_genpd_init(&rpi_power_domains[i].base, NULL, is_off);
+		/* let power_domains array know about the registered pm */
+		power_domains[i] = &rpi_power_domains[i].base;
+	}
+
+	ret = of_genpd_add_provider_onecell(dev->of_node, &rpi_genpd_xlate);
+	if (ret < 0)
+		goto uninit_pm;
+
+	return 0;
+
+uninit_pm:
+	for (i = 0; i < num_domains; i++)
+		pm_genpd_uninit(power_domains[i]);
+
+	return ret;
+}
+
+static int rpi_power_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	int i;
+
+	for (i = 0; i < rpi_genpd_xlate.num_domains; i++)
+		pm_genpd_uninit(rpi_genpd_xlate.domains[i]);
+
+	of_genpd_del_provider(dev->of_node);
+
+	return 0;
+}
+
+static const struct of_device_id rpi_power_of_match[] = {
+	{ .compatible = "raspberrypi,bcm2835-power", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rpi_power_of_match);
+
+static struct platform_driver rpi_power_driver = {
+	.driver = {
+		.name = "raspberrypi-power",
+		.of_match_table = rpi_power_of_match,
+	},
+	.probe		= rpi_power_probe,
+	.remove		= rpi_power_remove,
+};
+module_platform_driver(rpi_power_driver);
+
+MODULE_AUTHOR("Alexander Aring <aar@pengutronix.de>");
+MODULE_AUTHOR("Eric Anholt <eric@anholt.net>");
+MODULE_DESCRIPTION("Raspberry Pi power domain driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/arm/raspberrypi-power.h b/include/dt-bindings/arm/raspberrypi-power.h
new file mode 100644
index 0000000..c2ffbebc
--- /dev/null
+++ b/include/dt-bindings/arm/raspberrypi-power.h
@@ -0,0 +1,14 @@
+/*
+ *  Copyright ? 2015 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _DT_BINDINGS_ARM_BCM2835_RPI_POWER_H
+#define _DT_BINDINGS_ARM_BCM2835_RPI_POWER_H
+
+#define RPI_POWER_DOMAIN_USB	3
+
+#endif /* _DT_BINDINGS_ARM_BCM2835_RPI_POWER_H */
-- 
2.6.1

Re: [PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Ulf Hansson]

[...]

> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index 8c53c55..20479d7 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -134,6 +134,16 @@ config ARCH_BCM2835
>           This enables support for the Broadcom BCM2835 SoC. This SoC is
>           used in the Raspberry Pi and Roku 2 devices.
>
> +config RASPBERRYPI_POWER

You don't need a new Kconfig option I think. If you fold in the below
"select" under ARCH_BCM2835, that should work as well, right?

select PM_GENERIC_DOMAINS if (RASPBERRYPI_FIRMWARE && PM && OF)

> +       bool "Raspberry Pi power domain driver"
> +       depends on ARCH_BCM2835
> +       depends on RASPBERRYPI_FIRMWARE
> +       select PM_GENERIC_DOMAINS if PM
> +       select PM_GENERIC_DOMAINS_OF if PM
> +       help
> +         This enables support for the RPi power domains which can be enabled
> +         or disabled via the RPi firmware.
> +
>  config ARCH_BCM_63XX
>         bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7
>         depends on MMU
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index 892261f..fec2d6b 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -36,6 +36,7 @@ endif
>
>  # BCM2835
>  obj-$(CONFIG_ARCH_BCM2835)     += board_bcm2835.o
> +obj-$(CONFIG_RASPBERRYPI_POWER)        += raspberrypi-power.o

According to above, then this should become:

obj-$(CONFIG_PM_GENERIC_DOMAINS) += raspberrypi-power.o

[...]

Kind regards
Uffe

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Ulf Hansson]

[...]

> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index 8c53c55..20479d7 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -134,6 +134,16 @@ config ARCH_BCM2835
>           This enables support for the Broadcom BCM2835 SoC. This SoC is
>           used in the Raspberry Pi and Roku 2 devices.
>
> +config RASPBERRYPI_POWER

You don't need a new Kconfig option I think. If you fold in the below
"select" under ARCH_BCM2835, that should work as well, right?

select PM_GENERIC_DOMAINS if (RASPBERRYPI_FIRMWARE && PM && OF)

> +       bool "Raspberry Pi power domain driver"
> +       depends on ARCH_BCM2835
> +       depends on RASPBERRYPI_FIRMWARE
> +       select PM_GENERIC_DOMAINS if PM
> +       select PM_GENERIC_DOMAINS_OF if PM
> +       help
> +         This enables support for the RPi power domains which can be enabled
> +         or disabled via the RPi firmware.
> +
>  config ARCH_BCM_63XX
>         bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7
>         depends on MMU
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index 892261f..fec2d6b 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -36,6 +36,7 @@ endif
>
>  # BCM2835
>  obj-$(CONFIG_ARCH_BCM2835)     += board_bcm2835.o
> +obj-$(CONFIG_RASPBERRYPI_POWER)        += raspberrypi-power.o

According to above, then this should become:

obj-$(CONFIG_PM_GENERIC_DOMAINS) += raspberrypi-power.o

[...]

Kind regards
Uffe

Re: [PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Alexander Aring]

Hi,

On Tue, Nov 24, 2015 at 09:44:59PM +0100, Ulf Hansson wrote:
> [...]
> 
> > diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> > index 8c53c55..20479d7 100644
> > --- a/arch/arm/mach-bcm/Kconfig
> > +++ b/arch/arm/mach-bcm/Kconfig
> > @@ -134,6 +134,16 @@ config ARCH_BCM2835
> >           This enables support for the Broadcom BCM2835 SoC. This SoC is
> >           used in the Raspberry Pi and Roku 2 devices.
> >
> > +config RASPBERRYPI_POWER
> 
> You don't need a new Kconfig option I think. If you fold in the below
> "select" under ARCH_BCM2835, that should work as well, right?
> 
> select PM_GENERIC_DOMAINS if (RASPBERRYPI_FIRMWARE && PM && OF)
> 

I think this depends on what the maintainers like to have here.

The raspberrypi firmware isn't BCM2835 specific, when some SoC which is
BCM2835 and enabled the RASPBERRYPI_FIRMWARE (for what reason ever) it will
enable also the power domain driver for the RPi.

When some BCM2835 enable it, then it will do nothing because the
devicetree entries should not match then.

> > +       bool "Raspberry Pi power domain driver"
> > +       depends on ARCH_BCM2835
> > +       depends on RASPBERRYPI_FIRMWARE
> > +       select PM_GENERIC_DOMAINS if PM
> > +       select PM_GENERIC_DOMAINS_OF if PM
> > +       help
> > +         This enables support for the RPi power domains which can be enabled
> > +         or disabled via the RPi firmware.
> > +
> >  config ARCH_BCM_63XX
> >         bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7
> >         depends on MMU
> > diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> > index 892261f..fec2d6b 100644
> > --- a/arch/arm/mach-bcm/Makefile
> > +++ b/arch/arm/mach-bcm/Makefile
> > @@ -36,6 +36,7 @@ endif
> >
> >  # BCM2835
> >  obj-$(CONFIG_ARCH_BCM2835)     += board_bcm2835.o
> > +obj-$(CONFIG_RASPBERRYPI_POWER)        += raspberrypi-power.o
> 
> According to above, then this should become:
> 
> obj-$(CONFIG_PM_GENERIC_DOMAINS) += raspberrypi-power.o
> 

What about other BCM$FOOBAR arch's which might select/enables
CONFIG_PM_GENERIC_DOMAINS somehow?

Like ARCH_BCM_63XX.

Possible other solution would be to make the CONFIG_RASPBERRYPI_POWER as
a hidden entry inside Kconfig without a prompt.

Then it should look like this:

config RASPBERRYPI_POWER
	bool
	select PM_GENERIC_DOMAINS
	select PM_GENERIC_DOMAINS_OF

config ARCH_BCM2835
	...
	select RASPBERRYPI_POWER if (RASPBERRYPI_FIRMWARE && PM && OF)
	...

and leave the Makefile as it is.

- Alex
--
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

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Alexander Aring]

Hi,

On Tue, Nov 24, 2015 at 09:44:59PM +0100, Ulf Hansson wrote:
> [...]
> 
> > diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> > index 8c53c55..20479d7 100644
> > --- a/arch/arm/mach-bcm/Kconfig
> > +++ b/arch/arm/mach-bcm/Kconfig
> > @@ -134,6 +134,16 @@ config ARCH_BCM2835
> >           This enables support for the Broadcom BCM2835 SoC. This SoC is
> >           used in the Raspberry Pi and Roku 2 devices.
> >
> > +config RASPBERRYPI_POWER
> 
> You don't need a new Kconfig option I think. If you fold in the below
> "select" under ARCH_BCM2835, that should work as well, right?
> 
> select PM_GENERIC_DOMAINS if (RASPBERRYPI_FIRMWARE && PM && OF)
> 

I think this depends on what the maintainers like to have here.

The raspberrypi firmware isn't BCM2835 specific, when some SoC which is
BCM2835 and enabled the RASPBERRYPI_FIRMWARE (for what reason ever) it will
enable also the power domain driver for the RPi.

When some BCM2835 enable it, then it will do nothing because the
devicetree entries should not match then.

> > +       bool "Raspberry Pi power domain driver"
> > +       depends on ARCH_BCM2835
> > +       depends on RASPBERRYPI_FIRMWARE
> > +       select PM_GENERIC_DOMAINS if PM
> > +       select PM_GENERIC_DOMAINS_OF if PM
> > +       help
> > +         This enables support for the RPi power domains which can be enabled
> > +         or disabled via the RPi firmware.
> > +
> >  config ARCH_BCM_63XX
> >         bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7
> >         depends on MMU
> > diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> > index 892261f..fec2d6b 100644
> > --- a/arch/arm/mach-bcm/Makefile
> > +++ b/arch/arm/mach-bcm/Makefile
> > @@ -36,6 +36,7 @@ endif
> >
> >  # BCM2835
> >  obj-$(CONFIG_ARCH_BCM2835)     += board_bcm2835.o
> > +obj-$(CONFIG_RASPBERRYPI_POWER)        += raspberrypi-power.o
> 
> According to above, then this should become:
> 
> obj-$(CONFIG_PM_GENERIC_DOMAINS) += raspberrypi-power.o
> 

What about other BCM$FOOBAR arch's which might select/enables
CONFIG_PM_GENERIC_DOMAINS somehow?

Like ARCH_BCM_63XX.

Possible other solution would be to make the CONFIG_RASPBERRYPI_POWER as
a hidden entry inside Kconfig without a prompt.

Then it should look like this:

config RASPBERRYPI_POWER
	bool
	select PM_GENERIC_DOMAINS
	select PM_GENERIC_DOMAINS_OF

config ARCH_BCM2835
	...
	select RASPBERRYPI_POWER if (RASPBERRYPI_FIRMWARE && PM && OF)
	...

and leave the Makefile as it is.

- Alex

Re: [PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

Alexander Aring <alex.aring@gmail.com> writes:

> Hi,
>
> On Tue, Nov 24, 2015 at 09:44:59PM +0100, Ulf Hansson wrote:
>> [...]
>> 
>> > diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
>> > index 8c53c55..20479d7 100644
>> > --- a/arch/arm/mach-bcm/Kconfig
>> > +++ b/arch/arm/mach-bcm/Kconfig
>> > @@ -134,6 +134,16 @@ config ARCH_BCM2835
>> >           This enables support for the Broadcom BCM2835 SoC. This SoC is
>> >           used in the Raspberry Pi and Roku 2 devices.
>> >
>> > +config RASPBERRYPI_POWER
>> 
>> You don't need a new Kconfig option I think. If you fold in the below
>> "select" under ARCH_BCM2835, that should work as well, right?
>> 
>> select PM_GENERIC_DOMAINS if (RASPBERRYPI_FIRMWARE && PM && OF)
>> 
>
> I think this depends on what the maintainers like to have here.
>
> The raspberrypi firmware isn't BCM2835 specific, when some SoC which is
> BCM2835 and enabled the RASPBERRYPI_FIRMWARE (for what reason ever) it will
> enable also the power domain driver for the RPi.
>
> When some BCM2835 enable it, then it will do nothing because the
> devicetree entries should not match then.

As far as I'm concerned, using the firmware is a stopgap to get other
drivers working, until we can get a native power domain driver written.
I think it makes sense for this code to be under its own menu entry, so
it can be flipped back off when we get the real thing in place.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Eric Anholt]

Alexander Aring <alex.aring@gmail.com> writes:

> Hi,
>
> On Tue, Nov 24, 2015 at 09:44:59PM +0100, Ulf Hansson wrote:
>> [...]
>> 
>> > diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
>> > index 8c53c55..20479d7 100644
>> > --- a/arch/arm/mach-bcm/Kconfig
>> > +++ b/arch/arm/mach-bcm/Kconfig
>> > @@ -134,6 +134,16 @@ config ARCH_BCM2835
>> >           This enables support for the Broadcom BCM2835 SoC. This SoC is
>> >           used in the Raspberry Pi and Roku 2 devices.
>> >
>> > +config RASPBERRYPI_POWER
>> 
>> You don't need a new Kconfig option I think. If you fold in the below
>> "select" under ARCH_BCM2835, that should work as well, right?
>> 
>> select PM_GENERIC_DOMAINS if (RASPBERRYPI_FIRMWARE && PM && OF)
>> 
>
> I think this depends on what the maintainers like to have here.
>
> The raspberrypi firmware isn't BCM2835 specific, when some SoC which is
> BCM2835 and enabled the RASPBERRYPI_FIRMWARE (for what reason ever) it will
> enable also the power domain driver for the RPi.
>
> When some BCM2835 enable it, then it will do nothing because the
> devicetree entries should not match then.

As far as I'm concerned, using the firmware is a stopgap to get other
drivers working, until we can get a native power domain driver written.
I think it makes sense for this code to be under its own menu entry, so
it can be flipped back off when we get the real thing in place.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151125/5a5f0a62/attachment.sig>

Re: [PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 5196 bytes --]

Alexander Aring <alex.aring@gmail.com> writes:

> This patch adds support for RPi several Power Domains and enable support
> to enable the USB Power Domain when it's not enabled before.
>
> This patch based on Eric Anholt's patch to support Power Domains. He had
> an issue about -EPROBE_DEFER inside the power domain subsystem, this
> issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
> if we fail to init or turn-on domain").

Glad to see you pick this up!

> It was tested with barebox and the following scripts before booting
> linux:
>
> /env/a_off:
>
>  # cat /env/a_off
>  #turn off which are enabled by default
>  regulator -n bcm2835_mci0 -s disable
>  regulator -n uart0-pl0110 -s disable
>
> /env/a_on:
>
>  # cat /env/a_on
>  #turn off which are enabled by default
>  regulator -n bcm2835_mci0 -s disable
>  regulator -n uart0-pl0110 -s disable
>
>  regulator -n bcm2835_mci0 -s enable
>  regulator -n uart0-pl0110 -s enable
>  regulator -n uart0-pl0111 -s enable
>  regulator -n bcm2835_usb -s enable
>  regulator -n bcm2835_i2c0 -s enable
>  regulator -n bcm2835_i2c1 -s enable
>  regulator -n bcm2835_i2c2 -s enable
>  regulator -n bcm2835_spi -s enable
>  regulator -n bcm2835_ccp2tx -s enable
>  regulator -n bcm2835_dsi -s enable
>
> /env/b:
>
>  # cat /env/b
>  sh /env/a_on
>
>  regulator -n bcm2835_mci0 -s disable
>  regulator -n uart0-pl0110 -s disable
>  regulator -n uart0-pl0111 -s disable
>  regulator -n bcm2835_usb -s disable
>  regulator -n bcm2835_i2c0 -s disable
>  regulator -n bcm2835_i2c1 -s disable
>  regulator -n bcm2835_i2c2 -s disable
>  regulator -n bcm2835_spi -s disable
>  regulator -n bcm2835_ccp2tx -s disable
>  regulator -n bcm2835_dsi -s disable
>
> /env/c:
>
>  # cat /env/c
>  sh ./env/b
>
>  regulator -n bcm2835_mci0 -s enable
>  regulator -n uart0-pl0110 -s enable
>  regulator -n uart0-pl0111 -s enable
>  regulator -n bcm2835_usb -s enable
>  regulator -n bcm2835_i2c0 -s enable
>  regulator -n bcm2835_i2c1 -s enable
>  regulator -n bcm2835_i2c2 -s enable
>  regulator -n bcm2835_spi -s enable
>  regulator -n bcm2835_ccp2tx -s enable
>  regulator -n bcm2835_dsi -s enable
>
> These scripts enables/disable all regulators inside the bootloader. It
> was running with a "hard" and "soft" reset without any issues. These
> testcases should fit to Stephen Warren suggestions:
>
> "(a) before having explicitly turned the power domain on or off at all (b)
> after having turned it on (c) after having turned it off, and for all
> power domains."

I would drop this whole block from the commit message.  It doesn't seem
worth keeping associated with this code (though thanks for testing it!).

> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>

If I'm going to be credited as an author, we should probably keep my:

Signed-off-by: Eric Anholt <eric@anholt.net>

It looks like you've mostly rewritten things, and there's not a whole
lot of meat to this driver so I don't care about getting credit myself,
but best not to give people reasons to be suspicious.

> ---
>  arch/arm/boot/dts/bcm2835-rpi.dtsi          |  11 ++
>  arch/arm/boot/dts/bcm2835.dtsi              |   2 +-
>  arch/arm/mach-bcm/Kconfig                   |  10 ++
>  arch/arm/mach-bcm/Makefile                  |   1 +
>  arch/arm/mach-bcm/raspberrypi-power.c       | 180 ++++++++++++++++++++++++++++
>  include/dt-bindings/arm/raspberrypi-power.h |  14 +++
>  6 files changed, 217 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c
>  create mode 100644 include/dt-bindings/arm/raspberrypi-power.h

To merge the patch, the .dtsi changes need to be in a separate commit
which I would pick up in the dt branch.  I'm hoping Ulf or another PM
domains maintainer would be able to pick up the Kconfig/Makefile/.c
patch in their tree, so it can be queued after the uninit function
change.  If they won't, then it would go through my tree, but still on a
different branch from DT changes.

This is the only thing I see needing to change before I can Ack.

> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index 8c53c55..20479d7 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -134,6 +134,16 @@ config ARCH_BCM2835
>  	  This enables support for the Broadcom BCM2835 SoC. This SoC is
>  	  used in the Raspberry Pi and Roku 2 devices.
>  
> +config RASPBERRYPI_POWER
> +	bool "Raspberry Pi power domain driver"
> +	depends on ARCH_BCM2835
> +	depends on RASPBERRYPI_FIRMWARE
> +	select PM_GENERIC_DOMAINS if PM
> +	select PM_GENERIC_DOMAINS_OF if PM
> +	help
> +	  This enables support for the RPi power domains which can be enabled
> +	  or disabled via the RPi firmware.
> +
>  config ARCH_BCM_63XX
>  	bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7
>  	depends on MMU

I'd love to have this be "depends on ARCH_BCM2835 || COMPILE_TEST" --
that gets us better coverage from automated builders in -next.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Eric Anholt]

Alexander Aring <alex.aring@gmail.com> writes:

> This patch adds support for RPi several Power Domains and enable support
> to enable the USB Power Domain when it's not enabled before.
>
> This patch based on Eric Anholt's patch to support Power Domains. He had
> an issue about -EPROBE_DEFER inside the power domain subsystem, this
> issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
> if we fail to init or turn-on domain").

Glad to see you pick this up!

> It was tested with barebox and the following scripts before booting
> linux:
>
> /env/a_off:
>
>  # cat /env/a_off
>  #turn off which are enabled by default
>  regulator -n bcm2835_mci0 -s disable
>  regulator -n uart0-pl0110 -s disable
>
> /env/a_on:
>
>  # cat /env/a_on
>  #turn off which are enabled by default
>  regulator -n bcm2835_mci0 -s disable
>  regulator -n uart0-pl0110 -s disable
>
>  regulator -n bcm2835_mci0 -s enable
>  regulator -n uart0-pl0110 -s enable
>  regulator -n uart0-pl0111 -s enable
>  regulator -n bcm2835_usb -s enable
>  regulator -n bcm2835_i2c0 -s enable
>  regulator -n bcm2835_i2c1 -s enable
>  regulator -n bcm2835_i2c2 -s enable
>  regulator -n bcm2835_spi -s enable
>  regulator -n bcm2835_ccp2tx -s enable
>  regulator -n bcm2835_dsi -s enable
>
> /env/b:
>
>  # cat /env/b
>  sh /env/a_on
>
>  regulator -n bcm2835_mci0 -s disable
>  regulator -n uart0-pl0110 -s disable
>  regulator -n uart0-pl0111 -s disable
>  regulator -n bcm2835_usb -s disable
>  regulator -n bcm2835_i2c0 -s disable
>  regulator -n bcm2835_i2c1 -s disable
>  regulator -n bcm2835_i2c2 -s disable
>  regulator -n bcm2835_spi -s disable
>  regulator -n bcm2835_ccp2tx -s disable
>  regulator -n bcm2835_dsi -s disable
>
> /env/c:
>
>  # cat /env/c
>  sh ./env/b
>
>  regulator -n bcm2835_mci0 -s enable
>  regulator -n uart0-pl0110 -s enable
>  regulator -n uart0-pl0111 -s enable
>  regulator -n bcm2835_usb -s enable
>  regulator -n bcm2835_i2c0 -s enable
>  regulator -n bcm2835_i2c1 -s enable
>  regulator -n bcm2835_i2c2 -s enable
>  regulator -n bcm2835_spi -s enable
>  regulator -n bcm2835_ccp2tx -s enable
>  regulator -n bcm2835_dsi -s enable
>
> These scripts enables/disable all regulators inside the bootloader. It
> was running with a "hard" and "soft" reset without any issues. These
> testcases should fit to Stephen Warren suggestions:
>
> "(a) before having explicitly turned the power domain on or off at all (b)
> after having turned it on (c) after having turned it off, and for all
> power domains."

I would drop this whole block from the commit message.  It doesn't seem
worth keeping associated with this code (though thanks for testing it!).

> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Lee Jones <lee@kernel.org>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>

If I'm going to be credited as an author, we should probably keep my:

Signed-off-by: Eric Anholt <eric@anholt.net>

It looks like you've mostly rewritten things, and there's not a whole
lot of meat to this driver so I don't care about getting credit myself,
but best not to give people reasons to be suspicious.

> ---
>  arch/arm/boot/dts/bcm2835-rpi.dtsi          |  11 ++
>  arch/arm/boot/dts/bcm2835.dtsi              |   2 +-
>  arch/arm/mach-bcm/Kconfig                   |  10 ++
>  arch/arm/mach-bcm/Makefile                  |   1 +
>  arch/arm/mach-bcm/raspberrypi-power.c       | 180 ++++++++++++++++++++++++++++
>  include/dt-bindings/arm/raspberrypi-power.h |  14 +++
>  6 files changed, 217 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/mach-bcm/raspberrypi-power.c
>  create mode 100644 include/dt-bindings/arm/raspberrypi-power.h

To merge the patch, the .dtsi changes need to be in a separate commit
which I would pick up in the dt branch.  I'm hoping Ulf or another PM
domains maintainer would be able to pick up the Kconfig/Makefile/.c
patch in their tree, so it can be queued after the uninit function
change.  If they won't, then it would go through my tree, but still on a
different branch from DT changes.

This is the only thing I see needing to change before I can Ack.

> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index 8c53c55..20479d7 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -134,6 +134,16 @@ config ARCH_BCM2835
>  	  This enables support for the Broadcom BCM2835 SoC. This SoC is
>  	  used in the Raspberry Pi and Roku 2 devices.
>  
> +config RASPBERRYPI_POWER
> +	bool "Raspberry Pi power domain driver"
> +	depends on ARCH_BCM2835
> +	depends on RASPBERRYPI_FIRMWARE
> +	select PM_GENERIC_DOMAINS if PM
> +	select PM_GENERIC_DOMAINS_OF if PM
> +	help
> +	  This enables support for the RPi power domains which can be enabled
> +	  or disabled via the RPi firmware.
> +
>  config ARCH_BCM_63XX
>  	bool "Broadcom BCM63xx DSL SoC" if ARCH_MULTI_V7
>  	depends on MMU

I'd love to have this be "depends on ARCH_BCM2835 || COMPILE_TEST" --
that gets us better coverage from automated builders in -next.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 818 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151124/eecdb336/attachment.sig>

Re: [PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Kevin Hilman]

Alexander Aring <alex.aring@gmail.com> writes:

> This patch adds support for RPi several Power Domains and enable support
> to enable the USB Power Domain when it's not enabled before.
>
> This patch based on Eric Anholt's patch to support Power Domains. He had
> an issue about -EPROBE_DEFER inside the power domain subsystem, this
> issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
> if we fail to init or turn-on domain").

[...]

> +#define RPI_POWER_DOMAIN(_domain, _name)			\
> +	[_domain] = {						\

Using _domain as the array index is going to create a sparsely filled
array here, wasting memory.   I'm not sure what the other domain numbers
are for other domains to know if this is a big waste or not, but it's
still a bit wasteful.

In any case, AFAICT, it doesn't look like you need to have the array
index match the domain number anyways since you're using container_of().

So I suggest just removing this array index part, and just creating them
in arrary order.  Then your _probe function isn't going to try to setup
3 non-enabled domains before it finally hits the USB domain.

Kevin

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Kevin Hilman]

Alexander Aring <alex.aring@gmail.com> writes:

> This patch adds support for RPi several Power Domains and enable support
> to enable the USB Power Domain when it's not enabled before.
>
> This patch based on Eric Anholt's patch to support Power Domains. He had
> an issue about -EPROBE_DEFER inside the power domain subsystem, this
> issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
> if we fail to init or turn-on domain").

[...]

> +#define RPI_POWER_DOMAIN(_domain, _name)			\
> +	[_domain] = {						\

Using _domain as the array index is going to create a sparsely filled
array here, wasting memory.   I'm not sure what the other domain numbers
are for other domains to know if this is a big waste or not, but it's
still a bit wasteful.

In any case, AFAICT, it doesn't look like you need to have the array
index match the domain number anyways since you're using container_of().

So I suggest just removing this array index part, and just creating them
in arrary order.  Then your _probe function isn't going to try to setup
3 non-enabled domains before it finally hits the USB domain.

Kevin

Re: [PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Alexander Aring]

Hi,

On Mon, Nov 30, 2015 at 03:51:56PM -0800, Kevin Hilman wrote:
> Alexander Aring <alex.aring@gmail.com> writes:
> 
> > This patch adds support for RPi several Power Domains and enable support
> > to enable the USB Power Domain when it's not enabled before.
> >
> > This patch based on Eric Anholt's patch to support Power Domains. He had
> > an issue about -EPROBE_DEFER inside the power domain subsystem, this
> > issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
> > if we fail to init or turn-on domain").
> 
> [...]
> 
> > +#define RPI_POWER_DOMAIN(_domain, _name)			\
> > +	[_domain] = {						\
> 
> Using _domain as the array index is going to create a sparsely filled
> array here, wasting memory.   I'm not sure what the other domain numbers
> are for other domains to know if this is a big waste or not, but it's
> still a bit wasteful.
> 
> In any case, AFAICT, it doesn't look like you need to have the array
> index match the domain number anyways since you're using container_of().
> 
> So I suggest just removing this array index part, and just creating them
> in arrary order.  Then your _probe function isn't going to try to setup
> 3 non-enabled domains before it finally hits the USB domain.
> 

The idea is here to keeping the _same_ power domains indexes for
device-tree power domain API like the RPi firmware provides it.

If somebody dumps the devicetree and see the power domain index, if
he/she does then a firmware API power domain index mapping it is wrong.
Because we need then a separate mapping:

$ARRAY_DEFINED_INDEX <-> $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX

With the current solution to make a 1:1 mapping it there is no
confusing anymore, because:

$ARRAY_DEFINED_INDEX == $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX


Also there exists power domains 1-10 (so far I know), 1-2 are currently
not used (and dummy-calls inside the rpi firmware implementation). So
later they should be provided anyway.

There exists a little improvement to let the for (i = 0; i < num_domains
...) start at "i = 1", the entry with index "0" will be a waste of memory
then and it's not provided by the firmware API as a power domain.


These are my arguments to keeping the current way of registering power
domains, if you still want that I should change it then I will do it or
maybe I show here some "good" arguments here to keeping this behaviour.

Please let me know. Thanks.

- Alex

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Alexander Aring]

Hi,

On Mon, Nov 30, 2015 at 03:51:56PM -0800, Kevin Hilman wrote:
> Alexander Aring <alex.aring@gmail.com> writes:
> 
> > This patch adds support for RPi several Power Domains and enable support
> > to enable the USB Power Domain when it's not enabled before.
> >
> > This patch based on Eric Anholt's patch to support Power Domains. He had
> > an issue about -EPROBE_DEFER inside the power domain subsystem, this
> > issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
> > if we fail to init or turn-on domain").
> 
> [...]
> 
> > +#define RPI_POWER_DOMAIN(_domain, _name)			\
> > +	[_domain] = {						\
> 
> Using _domain as the array index is going to create a sparsely filled
> array here, wasting memory.   I'm not sure what the other domain numbers
> are for other domains to know if this is a big waste or not, but it's
> still a bit wasteful.
> 
> In any case, AFAICT, it doesn't look like you need to have the array
> index match the domain number anyways since you're using container_of().
> 
> So I suggest just removing this array index part, and just creating them
> in arrary order.  Then your _probe function isn't going to try to setup
> 3 non-enabled domains before it finally hits the USB domain.
> 

The idea is here to keeping the _same_ power domains indexes for
device-tree power domain API like the RPi firmware provides it.

If somebody dumps the devicetree and see the power domain index, if
he/she does then a firmware API power domain index mapping it is wrong.
Because we need then a separate mapping:

$ARRAY_DEFINED_INDEX <-> $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX

With the current solution to make a 1:1 mapping it there is no
confusing anymore, because:

$ARRAY_DEFINED_INDEX == $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX


Also there exists power domains 1-10 (so far I know), 1-2 are currently
not used (and dummy-calls inside the rpi firmware implementation). So
later they should be provided anyway.

There exists a little improvement to let the for (i = 0; i < num_domains
...) start@"i = 1", the entry with index "0" will be a waste of memory
then and it's not provided by the firmware API as a power domain.


These are my arguments to keeping the current way of registering power
domains, if you still want that I should change it then I will do it or
maybe I show here some "good" arguments here to keeping this behaviour.

Please let me know. Thanks.

- Alex

Re: [PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Kevin Hilman]

Alexander Aring <alex.aring@gmail.com> writes:

> Hi,
>
> On Mon, Nov 30, 2015 at 03:51:56PM -0800, Kevin Hilman wrote:
>> Alexander Aring <alex.aring@gmail.com> writes:
>> 
>> > This patch adds support for RPi several Power Domains and enable support
>> > to enable the USB Power Domain when it's not enabled before.
>> >
>> > This patch based on Eric Anholt's patch to support Power Domains. He had
>> > an issue about -EPROBE_DEFER inside the power domain subsystem, this
>> > issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
>> > if we fail to init or turn-on domain").
>> 
>> [...]
>> 
>> > +#define RPI_POWER_DOMAIN(_domain, _name)			\
>> > +	[_domain] = {						\
>> 
>> Using _domain as the array index is going to create a sparsely filled
>> array here, wasting memory.   I'm not sure what the other domain numbers
>> are for other domains to know if this is a big waste or not, but it's
>> still a bit wasteful.
>> 
>> In any case, AFAICT, it doesn't look like you need to have the array
>> index match the domain number anyways since you're using container_of().
>> 
>> So I suggest just removing this array index part, and just creating them
>> in arrary order.  Then your _probe function isn't going to try to setup
>> 3 non-enabled domains before it finally hits the USB domain.
>> 
>
> The idea is here to keeping the _same_ power domains indexes for
> device-tree power domain API like the RPi firmware provides it.
>
> If somebody dumps the devicetree and see the power domain index, if
> he/she does then a firmware API power domain index mapping it is wrong.
> Because we need then a separate mapping:
>
> $ARRAY_DEFINED_INDEX <-> $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX
>
> With the current solution to make a 1:1 mapping it there is no
> confusing anymore, because:
>
> $ARRAY_DEFINED_INDEX == $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX

I'm not proposing to change the DT numbers or the firmware numbers.  All
I'm proposing is that those numbers don't need to be mapped to the array
index.  IOW, in your structure definition macro:

+#define RPI_POWER_DOMAIN(_domain, _name)			\
+	[_domain] = {						\
+		.domain = _domain,				\
[...]

just drop the " [_domain] = { " line.

> Also there exists power domains 1-10 (so far I know), 1-2 are currently
> not used (and dummy-calls inside the rpi firmware implementation). So
> later they should be provided anyway.

They could then be added in any order in the struture.

> There exists a little improvement to let the for (i = 0; i < num_domains
> ...) start at "i = 1", the entry with index "0" will be a waste of memory
> then and it's not provided by the firmware API as a power domain.

Even index 0 will not be wasted with the above approach.

Kevin

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Kevin Hilman]

Alexander Aring <alex.aring@gmail.com> writes:

> Hi,
>
> On Mon, Nov 30, 2015 at 03:51:56PM -0800, Kevin Hilman wrote:
>> Alexander Aring <alex.aring@gmail.com> writes:
>> 
>> > This patch adds support for RPi several Power Domains and enable support
>> > to enable the USB Power Domain when it's not enabled before.
>> >
>> > This patch based on Eric Anholt's patch to support Power Domains. He had
>> > an issue about -EPROBE_DEFER inside the power domain subsystem, this
>> > issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
>> > if we fail to init or turn-on domain").
>> 
>> [...]
>> 
>> > +#define RPI_POWER_DOMAIN(_domain, _name)			\
>> > +	[_domain] = {						\
>> 
>> Using _domain as the array index is going to create a sparsely filled
>> array here, wasting memory.   I'm not sure what the other domain numbers
>> are for other domains to know if this is a big waste or not, but it's
>> still a bit wasteful.
>> 
>> In any case, AFAICT, it doesn't look like you need to have the array
>> index match the domain number anyways since you're using container_of().
>> 
>> So I suggest just removing this array index part, and just creating them
>> in arrary order.  Then your _probe function isn't going to try to setup
>> 3 non-enabled domains before it finally hits the USB domain.
>> 
>
> The idea is here to keeping the _same_ power domains indexes for
> device-tree power domain API like the RPi firmware provides it.
>
> If somebody dumps the devicetree and see the power domain index, if
> he/she does then a firmware API power domain index mapping it is wrong.
> Because we need then a separate mapping:
>
> $ARRAY_DEFINED_INDEX <-> $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX
>
> With the current solution to make a 1:1 mapping it there is no
> confusing anymore, because:
>
> $ARRAY_DEFINED_INDEX == $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX

I'm not proposing to change the DT numbers or the firmware numbers.  All
I'm proposing is that those numbers don't need to be mapped to the array
index.  IOW, in your structure definition macro:

+#define RPI_POWER_DOMAIN(_domain, _name)			\
+	[_domain] = {						\
+		.domain = _domain,				\
[...]

just drop the " [_domain] = { " line.

> Also there exists power domains 1-10 (so far I know), 1-2 are currently
> not used (and dummy-calls inside the rpi firmware implementation). So
> later they should be provided anyway.

They could then be added in any order in the struture.

> There exists a little improvement to let the for (i = 0; i < num_domains
> ...) start at "i = 1", the entry with index "0" will be a waste of memory
> then and it's not provided by the firmware API as a power domain.

Even index 0 will not be wasted with the above approach.

Kevin

Re: [PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Alexander Aring]

Hi,

Meanwhile there exists a new RPi firmware with a new interface to setup
power domains. Eric Anholt already did some work to access new interface
and old one based on this patch.

The next days he want to sent this new driver mainline, I will stop the work
now.

I will add some review notes anyway, maybe Eric Anholt can pick them up.

On Tue, Dec 01, 2015 at 03:27:57PM -0800, Kevin Hilman wrote:
> Alexander Aring <alex.aring@gmail.com> writes:
> 
> > Hi,
> >
> > On Mon, Nov 30, 2015 at 03:51:56PM -0800, Kevin Hilman wrote:
> >> Alexander Aring <alex.aring@gmail.com> writes:
> >> 
> >> > This patch adds support for RPi several Power Domains and enable support
> >> > to enable the USB Power Domain when it's not enabled before.
> >> >
> >> > This patch based on Eric Anholt's patch to support Power Domains. He had
> >> > an issue about -EPROBE_DEFER inside the power domain subsystem, this
> >> > issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
> >> > if we fail to init or turn-on domain").
> >> 
> >> [...]
> >> 
> >> > +#define RPI_POWER_DOMAIN(_domain, _name)			\
> >> > +	[_domain] = {						\
> >> 
> >> Using _domain as the array index is going to create a sparsely filled
> >> array here, wasting memory.   I'm not sure what the other domain numbers
> >> are for other domains to know if this is a big waste or not, but it's
> >> still a bit wasteful.
> >> 
> >> In any case, AFAICT, it doesn't look like you need to have the array
> >> index match the domain number anyways since you're using container_of().
> >> 
> >> So I suggest just removing this array index part, and just creating them
> >> in arrary order.  Then your _probe function isn't going to try to setup
> >> 3 non-enabled domains before it finally hits the USB domain.
> >> 
> >
> > The idea is here to keeping the _same_ power domains indexes for
> > device-tree power domain API like the RPi firmware provides it.
> >
> > If somebody dumps the devicetree and see the power domain index, if
> > he/she does then a firmware API power domain index mapping it is wrong.
> > Because we need then a separate mapping:
> >
> > $ARRAY_DEFINED_INDEX <-> $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX
> >
> > With the current solution to make a 1:1 mapping it there is no
> > confusing anymore, because:
> >
> > $ARRAY_DEFINED_INDEX == $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX
> 
> I'm not proposing to change the DT numbers or the firmware numbers.  All
> I'm proposing is that those numbers don't need to be mapped to the array
> index.  IOW, in your structure definition macro:
> 
> +#define RPI_POWER_DOMAIN(_domain, _name)			\
> +	[_domain] = {						\
> +		.domain = _domain,				\
> [...]
> 
> just drop the " [_domain] = { " line.
> 

This requires additional changes. We need to know the "maximum"
registered index value of RPi power domain.

Because the dynamic allocating array of (linux) power domains, which we
determine by:

... num_domains = ARRAY_SIZE(rpi_power_domains);

The RPi power domain defines are also used by devicetree. Usually I
would change it to an enum with a MAX_ATTR entry, which is not possible
because devicetree compiler can't deal with enums.


Anyway we could determine the max registered RPi power domain index by
doing something like:

num_domains = 0;

for (i = 0; i < ARRAY_SIZE(rpi_power_domains); i++)
	max(num_domains, array[i].domain);

- Alex

[PATCH 2/3] ARM: bcm2835: add rpi power domain driver

[Alexander Aring]

Hi,

Meanwhile there exists a new RPi firmware with a new interface to setup
power domains. Eric Anholt already did some work to access new interface
and old one based on this patch.

The next days he want to sent this new driver mainline, I will stop the work
now.

I will add some review notes anyway, maybe Eric Anholt can pick them up.

On Tue, Dec 01, 2015 at 03:27:57PM -0800, Kevin Hilman wrote:
> Alexander Aring <alex.aring@gmail.com> writes:
> 
> > Hi,
> >
> > On Mon, Nov 30, 2015 at 03:51:56PM -0800, Kevin Hilman wrote:
> >> Alexander Aring <alex.aring@gmail.com> writes:
> >> 
> >> > This patch adds support for RPi several Power Domains and enable support
> >> > to enable the USB Power Domain when it's not enabled before.
> >> >
> >> > This patch based on Eric Anholt's patch to support Power Domains. He had
> >> > an issue about -EPROBE_DEFER inside the power domain subsystem, this
> >> > issue was solved by commit <311fa6a> ("PM / Domains: Return -EPROBE_DEFER
> >> > if we fail to init or turn-on domain").
> >> 
> >> [...]
> >> 
> >> > +#define RPI_POWER_DOMAIN(_domain, _name)			\
> >> > +	[_domain] = {						\
> >> 
> >> Using _domain as the array index is going to create a sparsely filled
> >> array here, wasting memory.   I'm not sure what the other domain numbers
> >> are for other domains to know if this is a big waste or not, but it's
> >> still a bit wasteful.
> >> 
> >> In any case, AFAICT, it doesn't look like you need to have the array
> >> index match the domain number anyways since you're using container_of().
> >> 
> >> So I suggest just removing this array index part, and just creating them
> >> in arrary order.  Then your _probe function isn't going to try to setup
> >> 3 non-enabled domains before it finally hits the USB domain.
> >> 
> >
> > The idea is here to keeping the _same_ power domains indexes for
> > device-tree power domain API like the RPi firmware provides it.
> >
> > If somebody dumps the devicetree and see the power domain index, if
> > he/she does then a firmware API power domain index mapping it is wrong.
> > Because we need then a separate mapping:
> >
> > $ARRAY_DEFINED_INDEX <-> $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX
> >
> > With the current solution to make a 1:1 mapping it there is no
> > confusing anymore, because:
> >
> > $ARRAY_DEFINED_INDEX == $RPI_FIRMWARE_POWER_DOMAIN_API_INDEX
> 
> I'm not proposing to change the DT numbers or the firmware numbers.  All
> I'm proposing is that those numbers don't need to be mapped to the array
> index.  IOW, in your structure definition macro:
> 
> +#define RPI_POWER_DOMAIN(_domain, _name)			\
> +	[_domain] = {						\
> +		.domain = _domain,				\
> [...]
> 
> just drop the " [_domain] = { " line.
> 

This requires additional changes. We need to know the "maximum"
registered index value of RPi power domain.

Because the dynamic allocating array of (linux) power domains, which we
determine by:

... num_domains = ARRAY_SIZE(rpi_power_domains);

The RPi power domain defines are also used by devicetree. Usually I
would change it to an enum with a MAX_ATTR entry, which is not possible
because devicetree compiler can't deal with enums.


Anyway we could determine the max registered RPi power domain index by
doing something like:

num_domains = 0;

for (i = 0; i < ARRAY_SIZE(rpi_power_domains); i++)
	max(num_domains, array[i].domain);

- Alex

[PATCH 3/3] devicetree: add rpi power domain driver bindings

[Alexander Aring]

This patch adds devicetree tree bindings for the Raspberry Pi power
domain driver.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt

diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
new file mode 100644
index 0000000..c3abc24
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
@@ -0,0 +1,25 @@
+Raspberry Pi power domain driver
+
+Required properties:
+
+- compatible:		Should be "raspberrypi,bcm2835-power".
+- firmware:		Reference to the RPi firmware device node.
+- #power-domain-cells:	Should be <1>, we providing multiple power domains.
+
+The valid defines for power domain are:
+
+ RPI_POWER_DOMAIN_USB
+
+Example:
+
+power: power {
+	compatible = "raspberrypi,bcm2835-power";
+	firmware = <&firmware>;
+	#power-domain-cells = <1>;
+};
+
+Example for using power domain:
+
+&usb {
+       power-domains = <&power RPI_POWER_DOMAIN_USB>;
+};
-- 
2.6.1

[PATCH 3/3] devicetree: add rpi power domain driver bindings

[Alexander Aring]

This patch adds devicetree tree bindings for the Raspberry Pi power
domain driver.

Signed-off-by: Alexander Aring <alex.aring@gmail.com>
---
 .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt

diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
new file mode 100644
index 0000000..c3abc24
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
@@ -0,0 +1,25 @@
+Raspberry Pi power domain driver
+
+Required properties:
+
+- compatible:		Should be "raspberrypi,bcm2835-power".
+- firmware:		Reference to the RPi firmware device node.
+- #power-domain-cells:	Should be <1>, we providing multiple power domains.
+
+The valid defines for power domain are:
+
+ RPI_POWER_DOMAIN_USB
+
+Example:
+
+power: power {
+	compatible = "raspberrypi,bcm2835-power";
+	firmware = <&firmware>;
+	#power-domain-cells = <1>;
+};
+
+Example for using power domain:
+
+&usb {
+       power-domains = <&power RPI_POWER_DOMAIN_USB>;
+};
-- 
2.6.1

Re: [PATCH 3/3] devicetree: add rpi power domain driver bindings

[Rob Herring]

On Thu, Nov 19, 2015 at 07:08:10PM +0100, Alexander Aring wrote:
> This patch adds devicetree tree bindings for the Raspberry Pi power
> domain driver.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
> new file mode 100644
> index 0000000..c3abc24
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
> @@ -0,0 +1,25 @@
> +Raspberry Pi power domain driver
> +
> +Required properties:
> +
> +- compatible:		Should be "raspberrypi,bcm2835-power".
> +- firmware:		Reference to the RPi firmware device node.
> +- #power-domain-cells:	Should be <1>, we providing multiple power domains.
> +
> +The valid defines for power domain are:
> +
> + RPI_POWER_DOMAIN_USB
> +
> +Example:
> +
> +power: power {
> +	compatible = "raspberrypi,bcm2835-power";
> +	firmware = <&firmware>;
> +	#power-domain-cells = <1>;
> +};
> +
> +Example for using power domain:
> +
> +&usb {
> +       power-domains = <&power RPI_POWER_DOMAIN_USB>;
> +};
> -- 
> 2.6.1
> 

[PATCH 3/3] devicetree: add rpi power domain driver bindings

[Rob Herring]

On Thu, Nov 19, 2015 at 07:08:10PM +0100, Alexander Aring wrote:
> This patch adds devicetree tree bindings for the Raspberry Pi power
> domain driver.
> 
> Signed-off-by: Alexander Aring <alex.aring@gmail.com>

Acked-by: Rob Herring <robh@kernel.org>

> ---
>  .../bindings/arm/bcm/raspberrypi,bcm2835-power.txt | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
> new file mode 100644
> index 0000000..c3abc24
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-power.txt
> @@ -0,0 +1,25 @@
> +Raspberry Pi power domain driver
> +
> +Required properties:
> +
> +- compatible:		Should be "raspberrypi,bcm2835-power".
> +- firmware:		Reference to the RPi firmware device node.
> +- #power-domain-cells:	Should be <1>, we providing multiple power domains.
> +
> +The valid defines for power domain are:
> +
> + RPI_POWER_DOMAIN_USB
> +
> +Example:
> +
> +power: power {
> +	compatible = "raspberrypi,bcm2835-power";
> +	firmware = <&firmware>;
> +	#power-domain-cells = <1>;
> +};
> +
> +Example for using power domain:
> +
> +&usb {
> +       power-domains = <&power RPI_POWER_DOMAIN_USB>;
> +};
> -- 
> 2.6.1
>