Message ID | 1416514477-19190-3-git-send-email-stefan.wahren@i2se.com (mailing list archive) |
---|---|
State | RFC, archived |
Headers | show |
Hi Stefan, Sorry for the big delay. On Thu, Nov 20, 2014 at 08:14:36PM +0000, Stefan Wahren wrote: > 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
Hi Sebastian, [add Marek] Am 22.01.2015 um 00:01 schrieb Sebastian Reichel: > Hi Stefan, > > Sorry for the big delay. a late answer is better than no answer :) > > On Thu, Nov 20, 2014 at 08:14:36PM +0000, Stefan Wahren wrote: >> 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? > 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. > -- Sebastian Best regards Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 01/22/2015 09:08 AM, Stefan Wahren wrote: > > 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@80044000 { ... poweroff: poweroff@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 <sebastien.szymanski@armadeus.com> * * 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,
Hello, Am 22.01.2015 um 10:37 schrieb Sébastien SZYMANSKI: > Hello, > > On 01/22/2015 09:08 AM, Stefan Wahren wrote: >> 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. > 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 > > I thought adding a poweroff child node like this: > > power: power@80044000 { > ... > poweroff: poweroff@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. > [...] Regards Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, [add maintainers from regulator framework] On Thu, Jan 22, 2015 at 09:08:04AM +0100, Stefan Wahren wrote: > Am 22.01.2015 um 00:01 schrieb Sebastian Reichel: > > On Thu, Nov 20, 2014 at 08:14:36PM +0000, Stefan Wahren wrote: > >> 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? 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@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>; } } -- Sebastian
Hi, > Sebastian Reichel <sre@kernel.org> hat am 25. Januar 2015 um 16:04 > geschrieben: > > > Hi, > > [add maintainers from regulator framework] > > On Thu, Jan 22, 2015 at 09:08:04AM +0100, Stefan Wahren wrote: > > Am 22.01.2015 um 00:01 schrieb Sebastian Reichel: > > > On Thu, Nov 20, 2014 at 08:14:36PM +0000, Stefan Wahren wrote: > > >> 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? > > 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@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>; > } > } 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? > > -- Sebastian Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 26, 2015 at 08:46:45PM +0100, Stefan Wahren wrote: > > > 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. > 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. > > At this point i think about moving this feature into the bootloader. > > @Fabio, @Marek: > > What's your opinion about that? > > > > > -- Sebastian > > Stefan >
Hi Mark, > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16 geschrieben: > > > On Mon, Jan 26, 2015 at 08:46:45PM +0100, Stefan Wahren wrote: > > > > > 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. > > > 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 > > > > > At this point i think about moving this feature into the bootloader. > > > > @Fabio, @Marek: > > > > What's your opinion about that? > > > > > > > > -- Sebastian > > > > Stefan > > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote: > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16 geschrieben: > > > 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. 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. > 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? > 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).
Hi Mark, > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43 geschrieben: > > > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote: > > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16 > > > geschrieben: > > > > > 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. > > 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 > > > 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? > > > 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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote: > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43 geschrieben: > > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote: > > > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16 > > > > geschrieben: > > > > > > > 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. > > > > 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 :( > 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>; -- Sebastian
Hi Sebastian, > Sebastian Reichel <sre@kernel.org> hat am 28. Januar 2015 um 23:59 > geschrieben: > > > Hi, > > On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote: > > > Mark Brown <broonie@kernel.org> 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. > > > > 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>; 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@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@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 = <®_vdda>; }; reg_vdda: regulator@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 = <®_vddio>; }; reg_vddio: regulator@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 > > -- Sebastian > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Stefan Wahren <stefan.wahren@i2se.com> hat am 3. Februar 2015 um 21:41 > geschrieben: > > > Hi Sebastian, > > > Sebastian Reichel <sre@kernel.org> hat am 28. Januar 2015 um 23:59 > > geschrieben: > > > > > > Hi, > > > > On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote: > > > > Mark Brown <broonie@kernel.org> 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. > > > > > > 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>; > > 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@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@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 = <®_vdda>; > }; > > reg_vdda: regulator@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 = <®_vddio>; > }; > > reg_vddio: regulator@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 Gentle Ping. Stefan > > > > > -- Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 20, 2015 at 11:57:41AM +0100, Stefan Wahren wrote: > > power: power@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@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 = <®_vdda>; > > }; > > > > reg_vdda: regulator@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 = <®_vddio>; > > }; > > > > reg_vddio: regulator@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 > > Gentle Ping. The binding looks ok to me. It would be nice to get an ACK of a DT binding maintainer, though. -- Sebastian
Hi, > Sebastian Reichel <sre@kernel.org> hat am 23. Februar 2015 um 16:38 > geschrieben: > > > On Fri, Feb 20, 2015 at 11:57:41AM +0100, Stefan Wahren wrote: > > > power: power@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@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 = <®_vdda>; > > > }; > > > > > > reg_vdda: regulator@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 = <®_vddio>; > > > }; > > > > > > reg_vddio: regulator@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 > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 0108c2a..8a3eaa4 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -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 diff --git a/drivers/power/Makefile b/drivers/power/Makefile index dfa8942..91c0a62 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -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 diff --git a/drivers/power/mxs_power.c b/drivers/power/mxs_power.c new file mode 100644 index 0000000..6b68064 --- /dev/null +++ b/drivers/power/mxs_power.c @@ -0,0 +1,226 @@ +/* + * Freescale MXS power subsystem + * + * Copyright (C) 2014 Stefan Wahren + * + * Inspired by imx-bootlets + */ + +/* + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <linux/err.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> +#include <linux/power_supply.h> +#include <linux/types.h> + +#define BM_POWER_CTRL_POLARITY_VBUSVALID (1 << 5) +#define BM_POWER_CTRL_VBUSVALID_IRQ (1 << 4) +#define BM_POWER_CTRL_ENIRQ_VBUS_VALID (1 << 3) + +#define HW_POWER_5VCTRL_OFFSET 0x10 +#define HW_POWER_MISC_OFFSET 0x90 + +#define BM_POWER_5VCTRL_VBUSVALID_THRESH (7 << 8) +#define BM_POWER_5VCTRL_PWDN_5VBRNOUT (1 << 7) +#define BM_POWER_5VCTRL_VBUSVALID_5VDETECT (1 << 4) + +#define HW_POWER_5VCTRL_VBUSVALID_THRESH_4_40V (5 << 8) + +#define SHIFT_FREQSEL 4 + +#define BM_POWER_MISC_FREQSEL (7 << SHIFT_FREQSEL) + +#define HW_POWER_MISC_FREQSEL_20000_KHZ 1 +#define HW_POWER_MISC_FREQSEL_24000_KHZ 2 +#define HW_POWER_MISC_FREQSEL_19200_KHZ 3 + +#define HW_POWER_MISC_SEL_PLLCLK (1 << 0) + +static int dcdc_pll; +module_param(dcdc_pll, int, 0); +MODULE_PARM_DESC(dcdc_pll, + "DC-DC PLL frequency (kHz). Use 19200, 20000 or 24000"); + +struct mxs_power_data { + void __iomem *base_addr; + struct power_supply dc; +}; + +int get_dcdc_clk_freq(struct mxs_power_data *pdata) +{ + void __iomem *base = pdata->base_addr; + int ret = -EINVAL; + u32 val; + + val = readl(base + HW_POWER_MISC_OFFSET); + + /* XTAL source */ + if ((val & HW_POWER_MISC_SEL_PLLCLK) == 0) + return 24000; + + switch ((val & BM_POWER_MISC_FREQSEL) >> SHIFT_FREQSEL) { + case HW_POWER_MISC_FREQSEL_20000_KHZ: + ret = 20000; + break; + case HW_POWER_MISC_FREQSEL_24000_KHZ: + ret = 24000; + break; + case HW_POWER_MISC_FREQSEL_19200_KHZ: + ret = 19200; + break; + } + + return ret; +} + +int set_dcdc_clk_freq(struct mxs_power_data *pdata, int kHz) +{ + void __iomem *misc = pdata->base_addr + HW_POWER_MISC_OFFSET; + u32 val; + int ret = 0; + + val = readl(misc); + + val &= ~BM_POWER_MISC_FREQSEL; + val &= ~HW_POWER_MISC_SEL_PLLCLK; + + /* Accept only values recommend by Freescale */ + switch (kHz) { + case 19200: + val |= HW_POWER_MISC_FREQSEL_19200_KHZ << SHIFT_FREQSEL; + break; + case 20000: + val |= HW_POWER_MISC_FREQSEL_20000_KHZ << SHIFT_FREQSEL; + break; + case 24000: + val |= HW_POWER_MISC_FREQSEL_24000_KHZ << SHIFT_FREQSEL; + break; + default: + ret = -EINVAL; + break; + } + + if (ret) + return ret; + + /* First program FREQSEL */ + writel(val, misc); + + /* then set PLL as clk for DC-DC converter */ + writel(val | HW_POWER_MISC_SEL_PLLCLK, misc); + + return 0; +} + +static enum power_supply_property mxs_power_dc_props[] = { + POWER_SUPPLY_PROP_ONLINE, +}; + +static int mxs_power_dc_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct mxs_power_data *data = container_of(psy, + struct mxs_power_data, dc); + int ret = 0; + + switch (psp) { + case POWER_SUPPLY_PROP_ONLINE: + val->intval = 1; + break; + default: + ret = -EINVAL; + break; + } + return ret; +} + +static const struct of_device_id of_mxs_power_match[] = { + { .compatible = "fsl,imx23-power" }, + { .compatible = "fsl,imx28-power" }, + { /* end */ } +}; +MODULE_DEVICE_TABLE(of, of_mxs_power_match); + +static int mxs_power_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + struct resource *res; + struct mxs_power_data *data; + int ret; + int dcdc_clk_freq; + + if (!np) { + dev_err(dev, "missing device tree\n"); + return -EINVAL; + } + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (data == NULL) + return -ENOMEM; + + data->dc.properties = mxs_power_dc_props; + data->dc.num_properties = ARRAY_SIZE(mxs_power_dc_props); + data->dc.get_property = mxs_power_dc_get_property; + data->dc.name = "dc"; + data->dc.type = POWER_SUPPLY_TYPE_MAINS; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + data->base_addr = devm_ioremap_resource(dev, res); + if (IS_ERR(data->base_addr)) + return PTR_ERR(data->base_addr); + + ret = power_supply_register(dev, &data->dc); + if (ret) + return ret; + + platform_set_drvdata(pdev, data); + + if (dcdc_pll) + set_dcdc_clk_freq(data, dcdc_pll); + + dcdc_clk_freq = get_dcdc_clk_freq(data); + + dev_info(dev, "DCDC clock freq: %d kHz\n", dcdc_clk_freq); + + return of_platform_populate(np, NULL, NULL, dev); +} + +static int mxs_power_remove(struct platform_device *pdev) +{ + struct mxs_power_data *data = platform_get_drvdata(pdev); + + of_platform_depopulate(&pdev->dev); + power_supply_unregister(&data->dc); + + return 0; +} + +static struct platform_driver mxs_power_driver = { + .driver = { + .name = "mxs_power", + .owner = THIS_MODULE, + .of_match_table = of_mxs_power_match, + }, + .probe = mxs_power_probe, + .remove = mxs_power_remove, +}; + +module_platform_driver(mxs_power_driver); + +MODULE_AUTHOR("Stefan Wahren <stefan.wahren@i2se.com>"); +MODULE_DESCRIPTION("Freescale MXS power subsystem"); +MODULE_LICENSE("GPL v2");
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 <stefan.wahren@i2se.com> --- 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 -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html