mbox series

[0/4] bmips: add BCM63xx power domain controller

Message ID 20200609105244.4014823-1-noltari@gmail.com
Headers show
Series bmips: add BCM63xx power domain controller | expand

Message

Álvaro Fernández Rojas June 9, 2020, 10:52 a.m. UTC
BCM6318, BCM6328, BCM6362 and BCM63268 SoCs have a power domain controller
to enable/disable certain components in order to save power.

Álvaro Fernández Rojas (4):
  dt-bindings: soc: brcm: add BCM63xx power domain binding
  soc: bcm: add BCM63xx power domain driver
  mips: bmips: dts: add BCM6328 power domain support
  mips: bmips: dts: add BCM6362 power domain support

 .../devicetree/bindings/mips/brcm/soc.txt     |  17 +
 arch/mips/boot/dts/brcm/bcm6328.dtsi          |   6 +
 arch/mips/boot/dts/brcm/bcm6362.dtsi          |   6 +
 drivers/soc/bcm/Kconfig                       |   8 +
 drivers/soc/bcm/Makefile                      |   1 +
 drivers/soc/bcm/bcm63xx-power.c               | 374 ++++++++++++++++++
 6 files changed, 412 insertions(+)
 create mode 100644 drivers/soc/bcm/bcm63xx-power.c

Comments

Florian Fainelli June 10, 2020, 1:12 a.m. UTC | #1
On 6/9/2020 3:52 AM, Álvaro Fernández Rojas wrote:
> BCM6328 SoCs have a power domain controller to enable/disable certain
> components in order to save power.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Florian Fainelli June 10, 2020, 2:19 a.m. UTC | #2
On 6/9/2020 3:52 AM, Álvaro Fernández Rojas wrote:
> BCM6318, BCM6328, BCM6362 and BCM63268 SoCs have a power domain controller
> to enable/disable certain components in order to save power.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  drivers/soc/bcm/Kconfig         |   8 +
>  drivers/soc/bcm/Makefile        |   1 +
>  drivers/soc/bcm/bcm63xx-power.c | 374 ++++++++++++++++++++++++++++++++

I would create drivers/soc/bcm/bcm63xx because there are likely going to
be more changes for BCM63xx DSL SOCs in the future that would land
there, for instance the BCM63138 and newer SoCs have an entirely
different reset controller using the on-chip micro controller that would
be landing there.

Can you also make sure that the MAINTAINERS file still matches that
location?

With respect to the code, given that you have defined #reset-cells = <1>
in the Device Tree binding, I would expect that you create a header
under include/dt-bindings/ which defines constants for the various SoCs
which you are then using within your power domain provider driver.
bcm2835-power.c is a good example of how this works for instance.
Florian Fainelli June 10, 2020, 2:21 a.m. UTC | #3
On 6/9/2020 3:52 AM, Álvaro Fernández Rojas wrote:
> BCM6362 SoCs have a power domain controller to enable/disable certain
> components in order to save power.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Florian Fainelli June 10, 2020, 2:27 a.m. UTC | #4
On 6/9/2020 3:52 AM, Álvaro Fernández Rojas wrote:
> BCM6362 SoCs have a power domain controller to enable/disable certain
> components in order to save power.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Álvaro Fernández Rojas June 10, 2020, 4:32 p.m. UTC | #5
BCM6318, BCM6328, BCM6362 and BCM63268 SoCs have a power domain controller
to enable/disable certain components in order to save power.

v2: Introduce changes suggested by Florian:
  - Add separate YAML file for dt-bindings.
  - Add bcm63xx folder in drivers/soc/bcm.
  - Update MAINTAINERS.
  - Add dt-bindings header files.
  - Also add BCM63268 support.

Álvaro Fernández Rojas (6):
  dt-bindings: soc: brcm: add BCM63xx power domain binding
  soc: bcm: add BCM63xx power domain driver
  mips: bmips: dts: add BCM6328 power domain support
  mips: bmips: dts: add BCM6362 power domain support
  mips: bmips: dts: add BCM63268 power domain support
  mips: bmips: add BCM6318 power domain definitions

 .../bindings/soc/bcm/brcm,bcm63xx-power.yaml  |  44 +++
 MAINTAINERS                                   |   1 +
 arch/mips/boot/dts/brcm/bcm63268.dtsi         |   6 +
 arch/mips/boot/dts/brcm/bcm6328.dtsi          |   6 +
 arch/mips/boot/dts/brcm/bcm6362.dtsi          |   6 +
 drivers/soc/bcm/Kconfig                       |  10 +
 drivers/soc/bcm/Makefile                      |   1 +
 drivers/soc/bcm/bcm63xx/Kconfig               |  12 +
 drivers/soc/bcm/bcm63xx/Makefile              |   2 +
 drivers/soc/bcm/bcm63xx/bcm63xx-power.c       | 374 ++++++++++++++++++
 include/dt-bindings/soc/bcm6318-pm.h          |  17 +
 include/dt-bindings/soc/bcm63268-pm.h         |  21 +
 include/dt-bindings/soc/bcm6328-pm.h          |  17 +
 include/dt-bindings/soc/bcm6362-pm.h          |  21 +
 14 files changed, 538 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/bcm/brcm,bcm63xx-power.yaml
 create mode 100644 drivers/soc/bcm/bcm63xx/Kconfig
 create mode 100644 drivers/soc/bcm/bcm63xx/Makefile
 create mode 100644 drivers/soc/bcm/bcm63xx/bcm63xx-power.c
 create mode 100644 include/dt-bindings/soc/bcm6318-pm.h
 create mode 100644 include/dt-bindings/soc/bcm63268-pm.h
 create mode 100644 include/dt-bindings/soc/bcm6328-pm.h
 create mode 100644 include/dt-bindings/soc/bcm6362-pm.h
Florian Fainelli June 10, 2020, 4:49 p.m. UTC | #6
On 6/10/2020 9:32 AM, Álvaro Fernández Rojas wrote:
> BCM6318, BCM6328, BCM6362 and BCM63268 SoCs have a power domain controller
> to enable/disable certain components in order to save power.
> 
> v2: Introduce changes suggested by Florian:
>   - Add separate YAML file for dt-bindings.
>   - Add bcm63xx folder in drivers/soc/bcm.
>   - Update MAINTAINERS.
>   - Add dt-bindings header files.
>   - Also add BCM63268 support.

Thomas, since I typically send pull requests to soc@kernel.org for
drivers/soc/bcm/, do you want me to take the full series and updatse to
drivers/soc/bcm/bcm63xx/ in the future as well?
Florian Fainelli June 10, 2020, 4:49 p.m. UTC | #7
On 6/10/2020 9:32 AM, Álvaro Fernández Rojas wrote:
> BCM6318, BCM6328, BCM6362 and BCM63268 SoCs have a power domain controller
> to enable/disable certain components in order to save power.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>

Thanks for addressing the previous comments, this looks good to me,
there is just a single request below:

[snip]

> +static const struct bcm63xx_power_data bcm6318_power_domains[] = {
> +	{
> +		.name = "pcie",
> +		.bit = 0,

All of these bits definition should use the constants that you add in
patches 3 through 6, this means you would have to re-order the patches
to maintain bisectability obviously.

Thanks!