power: mxs_power: add driver for mxs power subsystem

RFC PATCH New Stefan Wahren 5 participants 23 messages Nov 20, 2014 – Feb 26, 2015

Subsystems (2)

Device tree bindings Arm i.MX and Layerscape

lore

Share & tools

lore

Stefan Wahren

author

2014-11-20 20:14 UTC #2 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

In reply to Stefan Wahren

This patch adds a minimal driver for the Freescale i.MX23, i.MX28 power subsystem. It's required to trigger the probing of the underlying drivers like on-chip regulators. Additionally the drivers supports the configuration of the DC-DC clock frequency to avoid possible interferences.

Signed-off-by Stefan Wahren <[email protected]>

---
 drivers/power/Kconfig     |    8 ++
 drivers/power/Makefile    |    1 +
 drivers/power/mxs_power.c |  226 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 235 insertions(+)
 create mode 100644 drivers/power/mxs_power.c

drivers/power/Kconfig +8 -0

index 0108c2a..8a3eaa4 100644
@@ -43,6 +43,14 @@ config MAX8925_POWER
 	  Say Y here to enable support for the battery charger in the Maxim
 	  MAX8925 PMIC.
 
+config MXS_POWER
+	tristate "Freescale MXS power subsystem support"
+	depends on ARCH_MXS
+	help
+	  Say Y here to enable support for the Freescale i.MX23/i.MX28
+	  power subsystem. This is a requirement to get access to on-chip
+	  regulators, battery charger and many more.
+
 config WM831X_BACKUP
 	tristate "WM831X backup battery charger support"
 	depends on MFD_WM831X

drivers/power/Makefile +1 -0

index dfa8942..91c0a62 100644
@@ -10,6 +10,7 @@ obj-$(CONFIG_GENERIC_ADC_BATTERY)	+= generic-adc-battery.o
 obj-$(CONFIG_PDA_POWER)		+= pda_power.o
 obj-$(CONFIG_APM_POWER)		+= apm_power.o
 obj-$(CONFIG_MAX8925_POWER)	+= max8925_power.o
+obj-$(CONFIG_MXS_POWER)		+= mxs_power.o
 obj-$(CONFIG_WM831X_BACKUP)	+= wm831x_backup.o
 obj-$(CONFIG_WM831X_POWER)	+= wm831x_power.o
 obj-$(CONFIG_WM8350_POWER)	+= wm8350_power.o

drivers/power/mxs_power.c new file +226 -0

226 lines not on this page · raw message

--
1.7.9.5

Sebastian Reichel

2015-01-21 23:01 UTC #3 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hi Stefan,

Sorry for the big delay.

Quoted 5 lines · Stefan Wahren

This patch adds a minimal driver for the Freescale i.MX23, i.MX28 power subsystem. It's required to trigger the probing of the underlying drivers like on-chip regulators. Additionally the drivers supports the configuration of the DC-DC clock frequency to avoid possible interferences.

I would expect PLL to be board specific and part of DT. Why is it specified as parameter?

Apart from that the driver/patch looks fine to me.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150122/672303ef/attachment.sig>

Stefan Wahren

author

2015-01-22 08:08 UTC #4 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hi Sebastian,

[add Marek]

Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:

Quoted 3 lines

Hi Stefan,
Sorry for the big delay.

a late answer is better than no answer :)

Quoted 9 lines · Stefan Wahren

Quoted 5 lines
This patch adds a minimal driver for the Freescale i.MX23, i.MX28 power subsystem. It's required to trigger the probing of the underlying drivers like on-chip regulators. Additionally the drivers supports the configuration of the DC-DC clock frequency to avoid possible interferences.
I would expect PLL to be board specific and part of DT. Why is it specified as parameter?

I think the switching frequency of the DC-DC belongs to the configuration and don't describe how the hardware is connected. But i don't have a problem to change it into a DT property.

Does a common property name exists for the switching frequency or would it be vendor specific?

Quoted 1 line

Apart from that the driver/patch looks fine to me.

In the meantime i made some bugfixes on these patches. Now i think it would be better to integrate the on-chip regulator driver into the patch series. That would clarify the function of the power driver.

Quoted 1 line

-- Sebastian

Best regards
Stefan

Sébastien SZYMANSKI

2015-01-22 09:37 UTC #5 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hello,

Quoted 5 lines · Stefan Wahren

In the meantime i made some bugfixes on these patches. Now i think it would be better to integrate the on-chip regulator driver into the patch series. That would clarify the function of the power driver.

I was about to resend this series with a poweroff driver for the imx28 SoCs. Should I wait this series get merged?

I thought adding a poweroff child node like this:

power: power at 80044000 {
	...
	poweroff: poweroff at 80044100 {
		compatible = "fsl,imx28-poweroff";
		reg = <0x80044100 0x10>;
	};
	...
};

Here is the driver:

/*
* MXS SoCs reset code.
*
* Copyright (c) 2015 Armadeus Systems.
*
* Author: S?bastien Szymanski <[email protected]> *
* based on imx-snvs-poweroff.c
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 and
* only version 2 as published by the Free Software Foundation. *
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
*/

#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/platform_device.h>

#define MXS_SET_ADDR 0x4

#define POWER_RESET_UNLOCK__KEY (0x3E77 << 16)
#define POWER_RESET_PWD 0x1

static void __iomem *power_reset;

static void do_mxs_poweroff(void)
{
writel(POWER_RESET_UNLOCK__KEY | POWER_RESET_PWD,
power_reset + MXS_SET_ADDR);
}

static int mxs_poweroff_probe(struct platform_device *pdev)

{
        power_reset = of_iomap(pdev->dev.of_node, 0);
        if (!power_reset) {
                dev_err(&pdev->dev, "failed to get memory\n");
                return -ENODEV;
        }

        pm_power_off = do_mxs_poweroff;

        dev_info(&pdev->dev, "power off function set!\n");

        return 0;
}

static const struct of_device_id of_mxs_poweroff_match[] = {
        { .compatible = "fsl,imx28-poweroff", },
        {},
};

MODULE_DEVICE_TABLE(of, of_mxs_poweroff_match);

static struct platform_driver mxs_poweroff_driver = {
        .probe = mxs_poweroff_probe,
        .driver = {
                .name = "mxs-poweroff",
                .of_match_table = of_match_ptr(of_mxs_poweroff_match),
        },
};

static int __init mxs_poweroff_init(void)

{
        return platform_driver_register(&mxs_poweroff_driver);
}

device_initcall(mxs_poweroff_init);

Best Regards,

Signature · 6 lines

S?bastien SZYMANSKI
Software Engineer
Armadeus Systems - A new vision of the embedded world
sebastien.szymanski at armadeus.com
Tel: +33 (0)9 72 29 41 44
Fax: +33 (0)9 72 28 79 26

Stefan Wahren

author

2015-01-22 09:48 UTC #6 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hello,

Am 22.01.2015 um 10:37 schrieb S?bastien SZYMANSKI:

Quoted 9 lines

Hello,
Quoted 4 lines · Stefan Wahren
In the meantime i made some bugfixes on these patches. Now i think it would be better to integrate the on-chip regulator driver into the patch series. That would clarify the function of the power driver.
I was about to resend this series with a poweroff driver for the imx28 SoCs.

sounds great.

Quoted 1 line

Should I wait this series get merged?

Yes, please. In the meantime you can use my repo at github (based on pm+acpi-3.19-rc4):

https://github.com/lategoodbye/linux-mxs-power

Quoted 11 lines

I thought adding a poweroff child node like this:

power: power at 80044000 {
	...
	poweroff: poweroff at 80044100 {
		compatible = "fsl,imx28-poweroff";
		reg = <0x80044100 0x10>;
	};
	...
};

Just a note to your driver it would be nice if it's handles i.MX23 and i.MX28.

Quoted 1 line

[...]

Regards Stefan

Sebastian Reichel

2015-01-25 15:04 UTC #7 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

In reply to Stefan Wahren

Hi,

[add maintainers from regulator framework]

Quoted 10 lines · Stefan Wahren

Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:
Quoted 8 lines · Stefan Wahren
Quoted 5 lines
This patch adds a minimal driver for the Freescale i.MX23, i.MX28 power subsystem. It's required to trigger the probing of the underlying drivers like on-chip regulators. Additionally the drivers supports the configuration of the DC-DC clock frequency to avoid possible interferences.
I would expect PLL to be board specific and part of DT. Why is it specified as parameter?

Quoted 6 lines

I think the switching frequency of the DC-DC belongs to the configuration and don't describe how the hardware is connected. But i don't have a problem to change it into a DT property.
Does a common property name exists for the switching frequency or would it be vendor specific?

As far as I know most regulators have fixed switching frequencies, so there is no common property name so far. I added regulator framework people, since this property should be regulator specific.

Apart from introducing a custom/common switching-frequency property the dts could also expose the PLL like this and use the common clock-frequency property:

power: power at 80044000 {
    compatible = "fsl,imx28-power";
    #address-cells = <1>;
    #size-cells = <1>;
    reg = <0x80044000 0x2000>;
    interrupts = <6>;
    ranges;

    powerpll {
        compatible = "fsl,imx28-power-pll"
        #clock-cells = <0>;
        clock-frequency = <12345>;
    }
}

Stefan Wahren

author

2015-01-26 19:46 UTC #8 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hi,

Quoted 42 lines

Sebastian Reichel <[email protected]> hat am 25. Januar 2015 um 16:04 geschrieben:
Hi,
[add maintainers from regulator framework]
Quoted 10 lines · Stefan Wahren
Am 22.01.2015 um 00:01 schrieb Sebastian Reichel:
Quoted 8 lines · Stefan Wahren
Quoted 5 lines
This patch adds a minimal driver for the Freescale i.MX23, i.MX28 power subsystem. It's required to trigger the probing of the underlying drivers like on-chip regulators. Additionally the drivers supports the configuration of the DC-DC clock frequency to avoid possible interferences.
I would expect PLL to be board specific and part of DT. Why is it specified as parameter?
Quoted 6 lines
I think the switching frequency of the DC-DC belongs to the configuration and don't describe how the hardware is connected. But i don't have a problem to change it into a DT property.
Does a common property name exists for the switching frequency or would it be vendor specific?
As far as I know most regulators have fixed switching frequencies, so there is no common property name so far. I added regulator framework people, since this property should be regulator specific.
Apart from introducing a custom/common switching-frequency property the dts could also expose the PLL like this and use the common clock-frequency property:
power: power at 80044000 {
compatible = "fsl,imx28-power";
#address-cells = <1>;
#size-cells = <1>;
reg = <0x80044000 0x2000>;
interrupts = <6>;
ranges;

Quoted 6 lines

powerpll {
compatible = "fsl,imx28-power-pll"
#clock-cells = <0>;
clock-frequency = <12345>;
}
}

okay i understand. But doesn't it need a extra driver to set the switching frequency because of the new compatible string?

At this point i think about moving this feature into the bootloader.

@Fabio, @Marek:

What's your opinion about that?

Quoted 2 lines

-- Sebastian

Stefan

Mark Brown

2015-01-27 00:16 UTC #9 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Quoted 6 lines · Stefan Wahren

Quoted 6 lines
Quoted 2 lines
Does a common property name exists for the switching frequency or would it be vendor specific?
As far as I know most regulators have fixed switching frequencies, so there is no common property name so far. I added regulator framework people, since this property should be regulator specific.

No, it's pretty common for there to be a couple of options - there's normally a performance tradeoff (power for regulation accuracy and/or cost) which can be selected. IIRC it's selected at design time as it affect the choice of passives on the board.

Quoted 2 lines

okay i understand. But doesn't it need a extra driver to set the switching frequency because of the new compatible string?

I don't understand this bit at all, sorry.

Quoted 12 lines

At this point i think about moving this feature into the bootloader.
@Fabio, @Marek:
What's your opinion about that?
Quoted 2 lines
-- Sebastian
Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150127/224c59a4/attachment.sig>

Stefan Wahren

author

2015-01-27 18:35 UTC #10 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hi Mark,

Quoted 17 lines

Mark Brown <[email protected]> hat am 27. Januar 2015 um 01:16 geschrieben:
Quoted 6 lines · Stefan Wahren
Quoted 6 lines
Quoted 2 lines
Does a common property name exists for the switching frequency or would it be vendor specific?
As far as I know most regulators have fixed switching frequencies, so there is no common property name so far. I added regulator framework people, since this property should be regulator specific.
No, it's pretty common for there to be a couple of options - there's normally a performance tradeoff (power for regulation accuracy and/or cost) which can be selected. IIRC it's selected at design time as it affect the choice of passives on the board.

Quoted 4 lines

Quoted 2 lines
okay i understand. But doesn't it need a extra driver to set the switching frequency because of the new compatible string?
I don't understand this bit at all, sorry.

Sebastian suggested a new sub-node in the devicetree:

    powerpll {
        compatible = "fsl,imx28-power-pll"
        #clock-cells = <0>;
        clock-frequency = <12345>;
    }

and i think that the new compatible string needs a separate driver to take care of the switching frequency.

Or is it okay to leave the handling of the switching frequency in the mxs-power driver?

Btw if we add a new node to set switching frequency, i think it would be better to describe the dc-dc convertor and not the pll:

    dcdc {
       compatible = "fsl,imx28-dcdc"
       frequency = <12345>;
    }

What do you think?

Best regards
Stefan

Quoted 13 lines

Quoted 13 lines
At this point i think about moving this feature into the bootloader.
@Fabio, @Marek:
What's your opinion about that?
Quoted 2 lines
-- Sebastian
Stefan

Mark Brown

2015-01-27 19:43 UTC #11 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Quoted 9 lines · Stefan Wahren

Quoted 6 lines
Mark Brown <[email protected]> hat am 27. Januar 2015 um 01:16 geschrieben:
Quoted 2 lines
okay i understand. But doesn't it need a extra driver to set the switching frequency because of the new compatible string?
I don't understand this bit at all, sorry.
Sebastian suggested a new sub-node in the devicetree:

Quoted 8 lines

    powerpll {
        compatible = "fsl,imx28-power-pll"
        #clock-cells = <0>;
        clock-frequency = <12345>;
    }
and i think that the new compatible string needs a separate driver to take care of the switching frequency.

I can see that this has been suggested, what I can't understand is why we would wish to do this but I am missing some context here which may make it all perfectly clear.

Quoted 2 lines

Or is it okay to leave the handling of the switching frequency in the mxs-power driver?

Unless the clock is exposed externally to the regulator either as an input or an output I'm not sure I see any benefit but I'm possibly missing something here. It does sound like there might be a separate clock IP or something?

Quoted 9 lines

Btw if we add a new node to set switching frequency, i think it would be better to describe the dc-dc convertor and not the pll:
    dcdc {
       compatible = "fsl,imx28-dcdc"
       frequency = <12345>;
    }
What do you think?

Yes, just adding a new property on the regulator node with all the other
properties seems sensible (we've just gained some helpers which make it
fairly straightforward to do this).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150127/3e3dfca8/attachment.sig>

Stefan Wahren

author

2015-01-28 22:22 UTC #12 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hi Mark,

Quoted 21 lines

Mark Brown <[email protected]> hat am 27. Januar 2015 um 20:43 geschrieben:
Quoted 16 lines · Stefan Wahren
Quoted 8 lines
Mark Brown <[email protected]> hat am 27. Januar 2015 um 01:16 geschrieben:
Quoted 3 lines
okay i understand. But doesn't it need a extra driver to set the switching
frequency because of the new compatible string?
I don't understand this bit at all, sorry.
Sebastian suggested a new sub-node in the devicetree:
powerpll {
compatible = "fsl,imx28-power-pll"
#clock-cells = <0>;
clock-frequency = <12345>;
}

Quoted 7 lines

Quoted 3 lines
and i think that the new compatible string needs a separate driver to take care
of the switching frequency.
I can see that this has been suggested, what I can't understand is why we would wish to do this but I am missing some context here which may make it all perfectly clear.

changing the switching frequency is not a common feature for this IC. In most cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal). Unfortunatelly this could cause interferences for example in UHF band. So switching to ref_pll and changing clock divider is a option to avoid such interferences without changing hardware.

The relevant register DC-DC Miscellaneous Register (HW_POWER_MISC) is described in chapter 11.12.10 of the reference manual [1].

I decided to implement it as a module parameter, because i see it's a configuration option.

[1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf

Quoted 28 lines

Quoted 4 lines
Or is it okay to leave the handling of the switching frequency in the mxs-power
driver?
Unless the clock is exposed externally to the regulator either as an input or an output I'm not sure I see any benefit but I'm possibly missing something here. It does sound like there might be a separate clock IP or something?
Quoted 10 lines
Btw if we add a new node to set switching frequency, i think it would be better
to describe the dc-dc convertor and not the pll:
dcdc {
compatible = "fsl,imx28-dcdc"
frequency = <12345>;
}
What do you think?
Yes, just adding a new property on the regulator node with all the other properties seems sensible (we've just gained some helpers which make it fairly straightforward to do this).
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Sebastian Reichel

2015-01-28 22:59 UTC #13 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hi,

Quoted 23 lines · Stefan Wahren

Quoted 23 lines
Mark Brown <[email protected]> hat am 27. Januar 2015 um 20:43 geschrieben:
Quoted 20 lines · Stefan Wahren
Quoted 8 lines
Mark Brown <[email protected]> hat am 27. Januar 2015 um 01:16 geschrieben:
Quoted 3 lines
okay i understand. But doesn't it need a extra driver to set the switching
frequency because of the new compatible string?
I don't understand this bit at all, sorry.
Sebastian suggested a new sub-node in the devicetree:
powerpll {
compatible = "fsl,imx28-power-pll"
#clock-cells = <0>;
clock-frequency = <12345>;
}
and i think that the new compatible string needs a separate driver to take care
of the switching frequency.

Quoted 8 lines

Quoted 3 lines
I can see that this has been suggested, what I can't understand is why we would wish to do this but I am missing some context here which may make it all perfectly clear.
The relevant register DC-DC Miscellaneous Register (HW_POWER_MISC) is described in chapter 11.12.10 of the reference manual [1].
[1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf

Having a look at the datasheet my suggestion for the sub-node doesn't make sense. I missunderstood how the hardware architecture looks like, sorry for the noise :(

Quoted 8 lines

changing the switching frequency is not a common feature for this IC. In most cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal). Unfortunatelly this could cause interferences for example in UHF band. So switching to ref_pll and changing clock divider is a option to avoid such interferences without changing hardware.
I decided to implement it as a module parameter, because i see it's a configuration option.

I think its best to use a DT property then. That way it can be set appropiatly for boards known to use a band, which could have interference problems.

So just add a simple property (without a subnode). If I understand Mark correctly, there's a high chance more drivers will need to describe the switching frequency, so I suggest to use a vendor independent property name:

switching-frequency = <12345>;

Stefan Wahren

author

2015-02-03 20:41 UTC #14 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hi Sebastian,

Quoted 25 lines

Sebastian Reichel <[email protected]> hat am 28. Januar 2015 um 23:59 geschrieben:
Hi,
Quoted 11 lines · Stefan Wahren
Quoted 4 lines
Mark Brown <[email protected]> hat am 27. Januar 2015 um 20:43 geschrieben:
On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
changing the switching frequency is not a common feature for this IC. In most
cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal). Unfortunatelly this could cause interferences for example in UHF band. So switching to ref_pll and changing clock divider is a option to avoid such interferences without changing hardware.
Quoted 2 lines
I decided to implement it as a module parameter, because i see it's a configuration option.
I think its best to use a DT property then. That way it can be set appropiatly for boards known to use a band, which could have interference problems.

Quoted 6 lines

So just add a simple property (without a subnode). If I understand Mark correctly, there's a high chance more drivers will need to describe the switching frequency, so I suggest to use a vendor independent property name:
switching-frequency = <12345>;

i'm not sure. So here is the complete current state (power and regulator binding from imx28.dtsi) including my suggestion for the new property:

	power: power at 80044000 {
		compatible = "fsl,imx28-power"; /* handled by mxs_power.c */
		#address-cells = <1>;
		#size-cells = <1>;
		reg = <0x80044000 0x2000>;
		interrupts = <6>;
		switching-frequency = <24000000>; /* new property */
		ranges;

		reg_vddd: regulator at 80044040 {
			reg = <0x80044040 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */
			regulator-name = "vddd";
			regulator-min-microvolt = <1350000>;
			regulator-max-microvolt = <1550000>;
			vddd-supply = <&reg_vdda>;
		};

		reg_vdda: regulator at 80044050 {
			reg = <0x80044050 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */
			regulator-name = "vdda";
			regulator-min-microvolt = <1725000>;
			regulator-max-microvolt = <1950000>;
			vdda-supply = <&reg_vddio>;
		};

		reg_vddio: regulator at 80044060 {
			reg = <0x80044060 0x10>,
			      <0x80044010 0x10>,
			      <0x800440c0 0x10>;
			reg-names = "base-address",
				    "v5ctrl-address",
				    "status-address";
			compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */
			regulator-name = "vddio";
			regulator-min-microvolt = <3000000>;
			regulator-max-microvolt = <3575000>;
			regulator-microvolt-offset = <80000>;
		};
	};

Please correct me if i'm wrong.

Stefan

Quoted 6 lines

-- Sebastian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Stefan Wahren

author

2015-02-20 10:57 UTC #15 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Quoted 95 lines

Stefan Wahren <[email protected]> hat am 3. Februar 2015 um 21:41 geschrieben:
Hi Sebastian,
Quoted 26 lines
Sebastian Reichel <[email protected]> hat am 28. Januar 2015 um 23:59 geschrieben:
Hi,
Quoted 12 lines · Stefan Wahren
Quoted 4 lines
Mark Brown <[email protected]> hat am 27. Januar 2015 um 20:43 geschrieben:
On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
changing the switching frequency is not a common feature for this IC. In most
cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal).
Unfortunatelly this could cause interferences for example in UHF band. So switching to ref_pll and changing clock divider is a option to avoid such interferences without changing hardware.
Quoted 2 lines
I decided to implement it as a module parameter, because i see it's a configuration option.
I think its best to use a DT property then. That way it can be set appropiatly for boards known to use a band, which could have interference problems.
Quoted 6 lines
So just add a simple property (without a subnode). If I understand Mark correctly, there's a high chance more drivers will need to describe the switching frequency, so I suggest to use a vendor independent property name:
switching-frequency = <12345>;
i'm not sure. So here is the complete current state (power and regulator binding
from imx28.dtsi) including my suggestion for the new property:
power: power at 80044000 {
compatible = "fsl,imx28-power"; /* handled by mxs_power.c */ #address-cells = <1>;
#size-cells = <1>;
reg = <0x80044000 0x2000>;
interrupts = <6>;
switching-frequency = <24000000>; /* new property */ ranges;
reg_vddd: regulator at 80044040 {
reg = <0x80044040 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */ regulator-name = "vddd";
regulator-min-microvolt = <1350000>; regulator-max-microvolt = <1550000>;
vddd-supply = <&reg_vdda>;
};
reg_vdda: regulator at 80044050 {
reg = <0x80044050 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */ regulator-name = "vdda";
regulator-min-microvolt = <1725000>; regulator-max-microvolt = <1950000>;
vdda-supply = <&reg_vddio>;
};
reg_vddio: regulator at 80044060 {
reg = <0x80044060 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */ regulator-name = "vddio";
regulator-min-microvolt = <3000000>; regulator-max-microvolt = <3575000>; regulator-microvolt-offset = <80000>;
};
};

Quoted 3 lines

Please correct me if i'm wrong.
Stefan

Gentle Ping.

Stefan

Quoted 3 lines

Quoted 3 lines
-- Sebastian

Sebastian Reichel

2015-02-23 15:38 UTC #16 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Quoted 52 lines · Stefan Wahren

Quoted 52 lines
power: power at 80044000 {
compatible = "fsl,imx28-power"; /* handled by mxs_power.c */ #address-cells = <1>;
#size-cells = <1>;
reg = <0x80044000 0x2000>;
interrupts = <6>;
switching-frequency = <24000000>; /* new property */ ranges;
reg_vddd: regulator at 80044040 {
reg = <0x80044040 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */ regulator-name = "vddd";
regulator-min-microvolt = <1350000>; regulator-max-microvolt = <1550000>;
vddd-supply = <&reg_vdda>;
};
reg_vdda: regulator at 80044050 {
reg = <0x80044050 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */ regulator-name = "vdda";
regulator-min-microvolt = <1725000>; regulator-max-microvolt = <1950000>;
vdda-supply = <&reg_vddio>;
};
reg_vddio: regulator at 80044060 {
reg = <0x80044060 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */ regulator-name = "vddio";
regulator-min-microvolt = <3000000>; regulator-max-microvolt = <3575000>; regulator-microvolt-offset = <80000>;
};
};

Quoted 5 lines

Quoted 3 lines
Please correct me if i'm wrong.
Stefan
Gentle Ping.

The binding looks ok to me. It would be nice to get an ACK of a DT binding maintainer, though.

Stefan Wahren

author

2015-02-26 19:20 UTC #17 lore raw

RFC PATCH 2/3 power: mxs_power: add driver for mxs power subsystem

Hi,

Quoted 57 lines

Sebastian Reichel <[email protected]> hat am 23. Februar 2015 um 16:38 geschrieben:
Quoted 52 lines · Stefan Wahren
Quoted 52 lines
power: power at 80044000 {
compatible = "fsl,imx28-power"; /* handled by mxs_power.c */ #address-cells = <1>;
#size-cells = <1>;
reg = <0x80044000 0x2000>;
interrupts = <6>;
switching-frequency = <24000000>; /* new property */ ranges;
reg_vddd: regulator at 80044040 {
reg = <0x80044040 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vddd"; /* handled by mxs-regulator.c */ regulator-name = "vddd";
regulator-min-microvolt = <1350000>; regulator-max-microvolt = <1550000>;
vddd-supply = <&reg_vdda>;
};
reg_vdda: regulator at 80044050 {
reg = <0x80044050 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vdda"; /* handled by mxs-regulator.c */ regulator-name = "vdda";
regulator-min-microvolt = <1725000>; regulator-max-microvolt = <1950000>;
vdda-supply = <&reg_vddio>;
};
reg_vddio: regulator at 80044060 {
reg = <0x80044060 0x10>,
<0x80044010 0x10>,
<0x800440c0 0x10>;
reg-names = "base-address",
"v5ctrl-address",
"status-address";
compatible = "fsl,imx28-vddio"; /* handled by mxs-regulator.c */ regulator-name = "vddio";
regulator-min-microvolt = <3000000>; regulator-max-microvolt = <3575000>; regulator-microvolt-offset = <80000>;
};
};

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Please correct me if i'm wrong.
Stefan
Gentle Ping.
The binding looks ok to me. It would be nice to get an ACK of a DT binding maintainer, though.

any objections about implementing DC-DC switching frequency as a DT property instead of module parameter?

Stefan

Stefan Wahren

author

2014-11-20 20:14 UTC #18 lore raw

RFC PATCH 3/3 ARM: dts: enable power subsystem for i.MX28

In reply to Stefan Wahren

This patch enables the power subsystem for all i.MX28 boards.

Signed-off-by Stefan Wahren <[email protected]>

---
 arch/arm/boot/dts/imx28.dtsi |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

arch/arm/boot/dts/imx28.dtsi +5 -1

index 47f68ac..5c163a5 100644
@@ -1022,8 +1022,12 @@
 			};
 
 			power: power at 80044000 {
+				compatible = "fsl,imx28-power";
+				#address-cells = <1>;
+				#size-cells = <1>;
 				reg = <0x80044000 0x2000>;
-				status = "disabled";
+				interrupts = <6>;
+				ranges;
 			};
 
 			saif1: saif at 80046000 {

--
1.7.9.5

Fabio Estevam

reviewer

2014-11-20 21:32 UTC #19 lore raw

RFC PATCH 3/3 ARM: dts: enable power subsystem for i.MX28

Hi Stefan,

Quoted 12 lines · Stefan Wahren

This patch enables the power subsystem for all i.MX28 boards.
Signed-off-by Stefan Wahren <[email protected]>
---
 arch/arm/boot/dts/imx28.dtsi |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/boot/dts/imx28.dtsi +0 -0
index 47f68ac..5c163a5 100644

arch/arm/boot/dts/imx28.dtsi line 1025

Quoted 4 lines

arch/arm/boot/dts/imx28.dtsi +1 -0

@@ -1022,8 +1022,12 @@
                        };
 
                        power: power at 80044000 {
+                               compatible = "fsl,imx28-power";

Should we have 'fsl,imx23-power' and 'fsl,imx28-power' as compatible strings?

Or ''fsl,mxs-power' could be used for both?

Stefan Wahren

author

2014-11-20 22:57 UTC #20 lore raw

RFC PATCH 3/3 ARM: dts: enable power subsystem for i.MX28

Hi Fabio,

Quoted 21 lines

Fabio Estevam <[email protected]> hat am 20. November 2014 um 22:32 geschrieben:
Hi Stefan,
Quoted 14 lines · Stefan Wahren
This patch enables the power subsystem for all i.MX28 boards.
Signed-off-by Stefan Wahren <[email protected]>
---
arch/arm/boot/dts/imx28.dtsi | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm/boot/dts/imx28.dtsi +0 -0
index 47f68ac..5c163a5 100644
@@ -1022,8 +1022,12 @@
 };
 

Quoted 4 lines

arch/arm/boot/dts/imx28.dtsi line 1025
Quoted 2 lines
arch/arm/boot/dts/imx28.dtsi +1 -0
 power: power at 80044000 {
+ compatible = "fsl,imx28-power";
Should we have 'fsl,imx23-power' and 'fsl,imx28-power' as compatible strings?

yes, of course.

Quoted 2 lines

Or ''fsl,mxs-power' could be used for both?

I think that would be a bad idea. I can't believe that all PMU register of i.MX23 and i.MX28 are identical. So in the future we will have to distinguish between them. It's better to do it now like most of the mxs driver does.

Stefan

Fabio Estevam

reviewer

2014-11-21 13:55 UTC #21 lore raw

RFC PATCH 3/3 ARM: dts: enable power subsystem for i.MX28

Quoted 5 lines · Stefan Wahren

Quoted 1 line
Or ''fsl,mxs-power' could be used for both?
I think that would be a bad idea. I can't believe that all PMU register of i.MX23 and i.MX28 are identical. So in the future we will have to distinguish between them. It's better to do it now like most of the mxs driver does.

Ok, sounds good then.

Stefan Wahren

author

2014-11-26 10:26 UTC #22 lore raw

cover power: mxs_power: add driver for mxs power subsystem

In reply to Stefan Wahren

Quoted 24 lines

This patch series adds support for Freescale i.MX23, i.xM28 power subsystem.
The idea for this driver was born in the discussion with Mark Rutland about the mxs-regulator driver [1].
The main use is to trigger driver probing of the underlying DT child nodes like on-chip regulators. But it's also the first step for cpufreq-dt support.
[1] - https://lkml.org/lkml/2014/9/29/283
Stefan Wahren (3):
DT: add binding for mxs power subsystem
power: mxs_power: add driver for mxs power subsystem
ARM: dts: enable power subsystem for i.MX28
 .../devicetree/bindings/power/mxs_power.txt        |   23 ++
 arch/arm/boot/dts/imx28.dtsi                       |    6 +-
 drivers/power/Kconfig                              |    8 +
 drivers/power/Makefile                             |    1 +
 drivers/power/mxs_power.c                          |  226 ++++++++++++++++++++
 5 files changed, 263 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/power/mxs_power.txt
 create mode 100644 drivers/power/mxs_power.c

Quoted 3 lines

--
1.7.9.5

Are there any other questions or suggestions about these patch series?

Stefan