Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
On Fri, Oct 31, 2014 at 04:51:43PM +0100, Daniel Vetter wrote:
> On Wed, Oct 29, 2014 at 10:14:37AM +0100, Thierry Reding wrote:
> > On Wed, Oct 29, 2014 at 09:57:02AM +0100, Daniel Vetter wrote:
> > > That makes the entire thing a bit non-trivial, which is why I think it
> > > would be better as some generic helper. Which then gets embedded or
> > > instantiated for specific cases, like dt&drm_panel or dt&drm_bridge.
> > > Or maybe even acpi&drm_bridge, who knows ;-)
> >
> > I worry a little about type safety. How will this generic helper know
> > what registry to look in for? Or conversely, if all these resources are
> > added to a single registry how do you know that they're of the correct
> > type? Failing to ensure this could cause situations where you're asking
> > for a panel and get a bridge in return because you've wrongly wired it
> > up in device tree for example.
> >
> > But perhaps if both the registry and the device parts are turned into
> > helpers we could still have a single core implementation and then
> > instantiate that for each type, something roughly like this:
> >
> > struct registry {
> > struct list_head list;
> > struct mutex lock;
> > };
> >
> > struct registry_record {
> > struct list_head list;
> > struct module *owner;
> > struct kref *ref;
> >
> > struct device *dev;
> > };
> >
> > int registry_add(struct registry *registry, struct registry_record
> > *record)
> > {
> > ...
> > try_module_get(record->owner);
> > ...
> > }
> >
> > struct registry_record *registry_find_by_of_node(struct registry
> > *registry,
> > struct device_node *np)
> > {
> > ...
> > kref_get(...);
> > ...
> > }
> >
> > That way it should be possible to embed these into other structures,
> > like so:
> >
> > struct drm_panel {
> > struct registry_record record;
> > ...
> > };
> >
> > static struct registry drm_panels;
> >
> > int drm_panel_add(struct drm_panel *panel)
> > {
> > return registry_add(&drm_panels, &panel->record);
> > }
> >
> > struct drm_panel *of_drm_panel_find(struct device_node *np)
> > {
> > struct registry_record *record;
> >
> > record = registry_find_by_of_node(&drm_panels, np);
> >
> > return container_of(record, struct drm_panel, record);
> > }
> >
> > Is that what you had in mind?
>
> Yeah I've thought that we should instantiate using macros even, so that we
> have per-type registries. So you'd smash the usual set of
> DECLARE/DEFINE_AUX_DEV_REGISTRY into headers/source files, and they'd take
> a (name, key, value) tripled. For the example here(of_drm_panel, struct
> device_node *, struct drm_panel *) or similar. I might be hand-waving over
> a few too many details though ;-)
Okay, I'll take a stab at this and see if I can convert DRM panel to it.
Thierry
pgpUw0Hut_eXQ.pgp
Description: PGP signature
Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
On Fri, Oct 31, 2014 at 04:59:40PM +0100, Daniel Vetter wrote:
> On Wed, Oct 29, 2014 at 10:16:49AM +0100, Thierry Reding wrote:
> > On Wed, Oct 29, 2014 at 08:51:27AM +0100, Daniel Vetter wrote:
> > > On Tue, Oct 28, 2014 at 03:29:47PM +0100, Thierry Reding wrote:
> > > > On Mon, Oct 27, 2014 at 11:20:31PM +0100, Daniel Vetter wrote:
> > > > > On Mon, Oct 27, 2014 at 8:58 PM, Sean Paul
> > > > > wrote:
> > > > > >>> @@ -660,8 +662,11 @@ struct drm_bridge_funcs {
> > > > > >>> * @driver_private: pointer to the bridge driver's internal
> > > > > >>> context
> > > > > >>> */
> > > > > >>> struct drm_bridge {
> > > > > >>> - struct drm_device *dev;
> > > > > >>> + struct device *dev;
> > > > > >>
> > > > > >> Please don't rename the ->dev pointer into drm. Because _all_ the
> > > > > >> other
> > > > > >> drm structures still call it ->dev. Also, can't we use struct
> > > > > >> device_node
> > > > > >> here like we do in the of helpers Russell added? See 7e435aad38083
> > > > > >>
> > > > > >
> > > > > > I think this is modeled after the naming in drm_panel, FWIW.
> > > > > > However,
> > > > > > seems reasonable to keep the device_node instead.
> > > > >
> > > > > Hm, indeed. Tbh I vote to rename drm_panel->drm to ->dev and like with
> > > > > drm_crtc drop the struct device and go directly to a struct
> > > > > device_node. Since we don't really need the sturct device, the only
> > > > > thing we care about is the of_node. For added bonus wrap an #ifdef
> > > > > CONFIG_OF around all the various struct device_node in drm_foo.h.
> > > > > Should be all fairly simple to pull off with cocci.
> > > > >
> > > > > Thierry?
> > > >
> > > > The struct device * is in DRM panel because there's nothing device tree
> > > > specific about the concept. Having a struct device_node * instead would
> > > > indicate that it can only be used with a device tree, whereas the
> > > > framework doesn't care the tiniest bit what type of device we have.
> > > >
> > > > While the trend clearly is to use more device tree, I don't think we
> > > > should make it impossible for anybody else to use these frameworks.
> > > >
> > > > There are other advantages to keeping a struct device *, like having
> > > > access to the proper device and its name. Also you get access to the
> > > > device_node * via dev->of_node anyway. I don't see any advantage in
> > > > switching to just a struct device_node *, only disadvantages.
> > >
> > > Well the idea is to make the lookup key specific, and conditional on
> > > #CONFIG_OF. If there's going to be another neat way to enumerate platform
> > > devices then I think we should add that, too. Or maybe we should have a
> > > void *platform_data or so.
> > >
> > > The reason I really don't want a struct device * in core drm structures is
> > > that two releases down the road people will have found tons of really
> > > great ways to abuse them and re-create a midlayer. DRM core really should
> > > only care about the sw objects and not be hw specific at all. Heck there's
> > > not even an requirement to have any piece of actual hw, you could write a
> > > completely fake drm driver (for e.g. testing like the new v4l driver).
> > >
> > > Tbh I wonder a bit why we even have this registery embedded into the core
> > > drm objects. Essentially the only thing you're doing is a list that maps
> > > some platform specific key onto some subsystem specific driver structure
> > > or fails the lookup. So instead of putting all these low-level details
> > > into drm core structures can't we just have a generic hashtable/list for
> > > this, plus some static inline helpers that cast the void * you get into
> > > the one you want?
> > >
> > > I also get the feeling that this really should be in the driver core (like
> > > the component helpers), and that we should think a lot harder about
> > > lifetimes and refcounting (see my other reply on that).
> >
> > Yes, that sounds very useful indeed. Also see my reply to yours. =)
>
> Just replying here with some of the irc discussions we've had. Since
> drm_bridge/panel isn't a core drm interface object exposed to userspace
> it's less critical. I still think that wasting a few brain cycles to have
> a clear separation between the abstract interface object and how to bind
> and unbind the pieces together is worthwhile, even though the cost when
> getting it wrong is much less severe than in the case of a mandatory piece
> of core infrastructure.
>
> I think in general the recent armsoc motivated drm infrastructure lacks a
> bit in attention to these details. E.g. the cma helpers are built into the
> drm.ko module, but clearly are auxilliary library code. So they should be
> pulled out and the headers clean up, so that we have a clear separation
> between core and helpers. Otherwise someone will sooner or later screw up
> and insert a helper depency into the core, and then we've started with the
> midlayer mess. Same goes with drm_bridge/panel,
Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
On Mon, Nov 03, 2014 at 09:06:04AM +0100, Thierry Reding wrote:
> On Fri, Oct 31, 2014 at 04:59:40PM +0100, Daniel Vetter wrote:
> > On Wed, Oct 29, 2014 at 10:16:49AM +0100, Thierry Reding wrote:
> > > On Wed, Oct 29, 2014 at 08:51:27AM +0100, Daniel Vetter wrote:
> > > > On Tue, Oct 28, 2014 at 03:29:47PM +0100, Thierry Reding wrote:
> > > > > On Mon, Oct 27, 2014 at 11:20:31PM +0100, Daniel Vetter wrote:
> > > > > > On Mon, Oct 27, 2014 at 8:58 PM, Sean Paul
> > > > > > wrote:
> > > > > > >>> @@ -660,8 +662,11 @@ struct drm_bridge_funcs {
> > > > > > >>> * @driver_private: pointer to the bridge driver's internal
> > > > > > >>> context
> > > > > > >>> */
> > > > > > >>> struct drm_bridge {
> > > > > > >>> - struct drm_device *dev;
> > > > > > >>> + struct device *dev;
> > > > > > >>
> > > > > > >> Please don't rename the ->dev pointer into drm. Because _all_
> > > > > > >> the other
> > > > > > >> drm structures still call it ->dev. Also, can't we use struct
> > > > > > >> device_node
> > > > > > >> here like we do in the of helpers Russell added? See
> > > > > > >> 7e435aad38083
> > > > > > >>
> > > > > > >
> > > > > > > I think this is modeled after the naming in drm_panel, FWIW.
> > > > > > > However,
> > > > > > > seems reasonable to keep the device_node instead.
> > > > > >
> > > > > > Hm, indeed. Tbh I vote to rename drm_panel->drm to ->dev and like
> > > > > > with
> > > > > > drm_crtc drop the struct device and go directly to a struct
> > > > > > device_node. Since we don't really need the sturct device, the only
> > > > > > thing we care about is the of_node. For added bonus wrap an #ifdef
> > > > > > CONFIG_OF around all the various struct device_node in drm_foo.h.
> > > > > > Should be all fairly simple to pull off with cocci.
> > > > > >
> > > > > > Thierry?
> > > > >
> > > > > The struct device * is in DRM panel because there's nothing device
> > > > > tree
> > > > > specific about the concept. Having a struct device_node * instead
> > > > > would
> > > > > indicate that it can only be used with a device tree, whereas the
> > > > > framework doesn't care the tiniest bit what type of device we have.
> > > > >
> > > > > While the trend clearly is to use more device tree, I don't think we
> > > > > should make it impossible for anybody else to use these frameworks.
> > > > >
> > > > > There are other advantages to keeping a struct device *, like having
> > > > > access to the proper device and its name. Also you get access to the
> > > > > device_node * via dev->of_node anyway. I don't see any advantage in
> > > > > switching to just a struct device_node *, only disadvantages.
> > > >
> > > > Well the idea is to make the lookup key specific, and conditional on
> > > > #CONFIG_OF. If there's going to be another neat way to enumerate
> > > > platform
> > > > devices then I think we should add that, too. Or maybe we should have a
> > > > void *platform_data or so.
> > > >
> > > > The reason I really don't want a struct device * in core drm structures
> > > > is
> > > > that two releases down the road people will have found tons of really
> > > > great ways to abuse them and re-create a midlayer. DRM core really
> > > > should
> > > > only care about the sw objects and not be hw specific at all. Heck
> > > > there's
> > > > not even an requirement to have any piece of actual hw, you could write
> > > > a
> > > > completely fake drm driver (for e.g. testing like the new v4l driver).
> > > >
> > > > Tbh I wonder a bit why we even have this registery embedded into the
> > > > core
> > > > drm objects. Essentially the only thing you're doing is a list that maps
> > > > some platform specific key onto some subsystem specific driver structure
> > > > or fails the lookup. So instead of putting all these low-level details
> > > > into drm core structures can't we just have a generic hashtable/list for
> > > > this, plus some static inline helpers that cast the void * you get into
> > > > the one you want?
> > > >
> > > > I also get the feeling that this really should be in the driver core
> > > > (like
> > > > the component helpers), and that we should think a lot harder about
> > > > lifetimes and refcounting (see my other reply on that).
> > >
> > > Yes, that sounds very useful indeed. Also see my reply to yours. =)
> >
> > Just replying here with some of the irc discussions we've had. Since
> > drm_bridge/panel isn't a core drm interface object exposed to userspace
> > it's less critical. I still think that wasting a few brain cycles to have
> > a clear separation between the abstract interface object and how to bind
> > and unbind the pieces together is worthwhile, even though the cost when
> > getting it wrong is much less severe than in the case of a mandatory piece
> > of core infrastructure.
> >
> > I think in general the recent armsoc motivated drm infrastructure lacks a
> > bit in attention to these details. E.g. the cma helpers are built into the
> > dr
Re: [PATCH v2 2/2] tty: serial: samsung: Clean-up selection of number of available UARTs
Hello Kukjin,
On Fri, Oct 31, 2014 at 8:06 AM, Abhilash Kesavan
wrote:
> Hi Kukjin,
>
> On Tue, Oct 28, 2014 at 5:56 PM, Abhilash Kesavan
> wrote:
>> Hi Kukjin
>>
>> On Tue, Oct 28, 2014 at 4:01 PM, Kukjin Kim wrote:
>>> Abhilash Kesavan wrote:
>>> Hi,
>>>
>>> Sorry for late response.
>>>
Remove symbols SERIAL_SAMSUNG_UARTS_4 and SERIAL_SAMSUNG_UARTS which
select the number of UART ports available on the SoC. Replace the usage
of SERIAL_SAMSUNG_UARTS in the serial driver with the maximum number of
>>>
>>> Well, as you know the number of uart ports are different on each Samsung
>>> SoCs
>>> so I don't think just using maximum number of uart ports are possible for
>>> new
>>> exynos7 SoC at this moment.
>>
>> Thanks for the review.
>> The main reason for me sending this patch was so that we may be able
>> to re-use the serial driver on arm64 based Exynos7 too. The two
>> symbols mentioned above which depend on PLAT_SAMSUNG prevent this. I
>> initially sent a patch which changed the dependency to SERIAL_SAMSUNG
>> for these 2 symbols. However, Tomasz suggested that a clean-up of
>> these two symbols would be a better option.
>>
>> Please see the discussion of the previous version here:
>> https://lkml.org/lkml/2014/9/29/702
>>
>> Can you please let me know if the previous version is acceptable ?
>
> Kukjin, can you please indicate the approach you would like me to
> take. Without this serial support is blocked on Exynos7.
Gentle reminder on this.
Abhilash
>
> Thanks,
> Abhilash
>>
>>>
UART ports possible. Removal of these symbols also helps in Exynos7
serial enablement.
Signed-off-by: Abhilash Kesavan
Reviewed-by: Tomasz Figa
Cc: Greg Kroah-Hartman
---
drivers/tty/serial/Kconfig | 16
drivers/tty/serial/samsung.c | 11 +++
drivers/tty/serial/samsung.h |5 -
3 files changed, 7 insertions(+), 25 deletions(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 81f6ee7..9fc9092 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -247,22 +247,6 @@ config SERIAL_SAMSUNG
provide all of these ports, depending on how the serial port
pins are configured.
-config SERIAL_SAMSUNG_UARTS_4
- bool
- depends on PLAT_SAMSUNG
- default y if !(CPU_S3C2410 || CPU_S3C2412 || CPU_S3C2440 ||
CPU_S3C2442)
- help
- Internal node for the common case of 4 Samsung compatible UARTs
-
-config SERIAL_SAMSUNG_UARTS
- int
- depends on PLAT_SAMSUNG
- default 4 if SERIAL_SAMSUNG_UARTS_4 || CPU_S3C2416
- default 3
- help
- Select the number of available UART ports for the Samsung S3C
- serial driver
-
config SERIAL_SAMSUNG_DEBUG
bool "Samsung SoC serial debug"
depends on SERIAL_SAMSUNG && DEBUG_LL
diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
index c78f43a..ba04c6d 100644
--- a/drivers/tty/serial/samsung.c
+++ b/drivers/tty/serial/samsung.c
@@ -962,14 +962,14 @@ static struct uart_ops s3c24xx_serial_ops = {
static struct uart_driver s3c24xx_uart_drv = {
.owner = THIS_MODULE,
.driver_name= "s3c2410_serial",
- .nr = CONFIG_SERIAL_SAMSUNG_UARTS,
+ .nr = MAX_SAMSUNG_UARTS,
.cons = S3C24XX_SERIAL_CONSOLE,
.dev_name = S3C24XX_SERIAL_NAME,
.major = S3C24XX_SERIAL_MAJOR,
.minor = S3C24XX_SERIAL_MINOR,
};
-static struct s3c24xx_uart_port
s3c24xx_serial_ports[CONFIG_SERIAL_SAMSUNG_UARTS] = {
+static struct s3c24xx_uart_port s3c24xx_serial_ports[MAX_SAMSUNG_UARTS] =
{
[0] = {
.port = {
.lock =
__SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[0].port.lock),
@@ -992,8 +992,6 @@ static struct s3c24xx_uart_port
s3c24xx_serial_ports[CONFIG_SERIAL_SAMSUNG_UARTS
.line = 1,
}
},
-#if CONFIG_SERIAL_SAMSUNG_UARTS > 2
-
[2] = {
.port = {
.lock =
__SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[2].port.lock),
@@ -1005,8 +1003,6 @@ static struct s3c24xx_uart_port
s3c24xx_serial_ports[CONFIG_SERIAL_SAMSUNG_UARTS
.line = 2,
}
},
-#endif
-#if CONFIG_SERIAL_SAMSUNG_UARTS > 3
[3] = {
.port = {
.lock =
__SPIN_LOCK_UNLOCKED(s3c24xx_serial_ports[3].port.lock),
@@ -1018,7 +1014,6 @@ static struct s3c24xx_uart_port
s3c24xx_serial_ports[CONFIG_SERIAL_SAMSUNG_U
Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
On Mon, Nov 3, 2014 at 1:31 PM, Thierry Reding wrote:
> On Fri, Oct 31, 2014 at 04:51:43PM +0100, Daniel Vetter wrote:
>> On Wed, Oct 29, 2014 at 10:14:37AM +0100, Thierry Reding wrote:
>> > On Wed, Oct 29, 2014 at 09:57:02AM +0100, Daniel Vetter wrote:
>> > > That makes the entire thing a bit non-trivial, which is why I think it
>> > > would be better as some generic helper. Which then gets embedded or
>> > > instantiated for specific cases, like dt&drm_panel or dt&drm_bridge.
>> > > Or maybe even acpi&drm_bridge, who knows ;-)
>> >
>> > I worry a little about type safety. How will this generic helper know
>> > what registry to look in for? Or conversely, if all these resources are
>> > added to a single registry how do you know that they're of the correct
>> > type? Failing to ensure this could cause situations where you're asking
>> > for a panel and get a bridge in return because you've wrongly wired it
>> > up in device tree for example.
>> >
>> > But perhaps if both the registry and the device parts are turned into
>> > helpers we could still have a single core implementation and then
>> > instantiate that for each type, something roughly like this:
>> >
>> > struct registry {
>> > struct list_head list;
>> > struct mutex lock;
>> > };
>> >
>> > struct registry_record {
>> > struct list_head list;
>> > struct module *owner;
>> > struct kref *ref;
>> >
>> > struct device *dev;
>> > };
>> >
>> > int registry_add(struct registry *registry, struct registry_record
>> > *record)
>> > {
>> > ...
>> > try_module_get(record->owner);
>> > ...
>> > }
>> >
>> > struct registry_record *registry_find_by_of_node(struct registry
>> > *registry,
>> > struct device_node
>> > *np)
>> > {
>> > ...
>> > kref_get(...);
>> > ...
>> > }
>> >
>> > That way it should be possible to embed these into other structures,
>> > like so:
>> >
>> > struct drm_panel {
>> > struct registry_record record;
>> > ...
>> > };
>> >
>> > static struct registry drm_panels;
>> >
>> > int drm_panel_add(struct drm_panel *panel)
>> > {
>> > return registry_add(&drm_panels, &panel->record);
>> > }
>> >
>> > struct drm_panel *of_drm_panel_find(struct device_node *np)
>> > {
>> > struct registry_record *record;
>> >
>> > record = registry_find_by_of_node(&drm_panels, np);
>> >
>> > return container_of(record, struct drm_panel, record);
>> > }
>> >
>> > Is that what you had in mind?
>>
>> Yeah I've thought that we should instantiate using macros even, so that we
>> have per-type registries. So you'd smash the usual set of
>> DECLARE/DEFINE_AUX_DEV_REGISTRY into headers/source files, and they'd take
>> a (name, key, value) tripled. For the example here(of_drm_panel, struct
>> device_node *, struct drm_panel *) or similar. I might be hand-waving over
>> a few too many details though ;-)
>
> Okay, I'll take a stab at this and see if I can convert DRM panel to it.
It would be great if you can do this soon. I would anyhow need a reference
for converting bridge framework as per Daniel's requirement :)
Ajay
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] drm/exynos: Add DECON driver
Hi, Fortunately, I could get the user manual for Exynos7420. Below are my comments. Thanks, Inki Dae On 2014년 10월 23일 01:34, Ajay kumar wrote: > On Wed, Oct 22, 2014 at 8:26 PM, Inki Dae wrote: >> >> Thanks for contribution. >> >> It seems reasonable that you separate device drivers into FIMD and DECON >> because many registers of them have many different offsets and fields. >> However, there may be a good solution that we can combine common sets of >> these drivers later. > Yes, this is the main reason behind sending this as RFC patch. > I want to know what's the best way to do this. > FIMD, 5433 DECON and Exynos7 DECON - all are different. > Also, in Exynos7 DECON-INT is same as DECON-EXT(Mixer). > So, even I am not sure how the driver layouts should be! Please, make sure Exynos SoC name, Exynos7410 or Exynos7420. In my understanding, Exynos7 doesn't mean one real SoC. > >> Below are my comments. >> >> Thanks, >> Inki Dae >> >> On 2014년 10월 10일 21:48, Ajay Kumar wrote: >>> This series is based on exynos-drm-next branch of Inki Dae's tree at: >>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >>> >>> DECON(Display and Enhancement Controller) is the new IP >>> in exynos7 SOC for generating video signals using pixel data. >> >> DECON was used since Exynos5430. And is Exynos5433 different from >> Exynos7? If so, could I get the Exynos7 user manual (TRM) for review? > Yes, Exynos5433 DECON is very much different than Exynos7 DECON. Do not use Exynos7 word and use Exynos7410 or Exynos7420 instead. > I will see how manual can be arranged. > >>> >>> DECON driver can be used to drive 2 different interfaces on Exynos7: >>> DECON-INT(video controller) and DECON-EXT(Mixer for HDMI) >>> >>> The existing FIMD driver code was used as a template to create >>> DECON driver. Only DECON-INT is supported as of now, and >>> DECON-EXT support will be added later. >>> >>> Signed-off-by: Akshu Agrawal >>> Signed-off-by: Ajay Kumar >>> --- >>> .../devicetree/bindings/video/exynos-decon.txt | 68 ++ >>> drivers/gpu/drm/exynos/Kconfig | 11 +- >>> drivers/gpu/drm/exynos/Makefile|1 + >>> drivers/gpu/drm/exynos/exynos_drm_decon.c | 1086 >> >>> drivers/gpu/drm/exynos/exynos_drm_drv.c| 17 +- >>> drivers/gpu/drm/exynos/exynos_drm_drv.h| 11 + >>> include/video/samsung_decon.h | 346 +++ >>> 7 files changed, 1537 insertions(+), 3 deletions(-) >>> create mode 100644 >> Documentation/devicetree/bindings/video/exynos-decon.txt >>> create mode 100644 drivers/gpu/drm/exynos/exynos_drm_decon.c >>> create mode 100644 include/video/samsung_decon.h >>> >>> diff --git a/Documentation/devicetree/bindings/video/exynos-decon.txt >> b/Documentation/devicetree/bindings/video/exynos-decon.txt >>> new file mode 100644 >>> index 000..e865650 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/video/exynos-decon.txt >>> @@ -0,0 +1,68 @@ >>> +Device-Tree bindings for Samsung Exynos7 SoC display controller (DECON) >>> + >>> +DECON (Display and Enhancement Controller) is the Display Controller >> for the >>> +Exynos7 series of SoCs which transfers the image data from a video memory >>> +buffer to an external LCD interface. >>> + >>> +Required properties: >>> +- compatible: value should be "samsung,exynos7-decon"; >> >> If exynos5433 was just renamed to exynos7 then, it should be one of the >> following: >> (a) "samsung,exynos5430-decon" for Display and enhancement controller >> IP for Exynos5430 >> (b) "samsung,exynos7" for Display and enhancement controller IP for >> Exynos7 >> >> Or, >> (a) "samsung,exynos5430-decon" for Display and enhancement controller >> IP for Exynos5430 >> >> (b) "samsung,exynos5433-decon" for Display and enhancement controller >> IP for Exynos5433 >> (c) "samsung,exynos7" for Display and enhancement controller IP for >> Exynos7 > Eventually, we will end up here. > >> >>> + >>> +- reg: physical base address and length of the DECON registers set. >>> + >>> +- interrupt-parent: should be the phandle of the decon controller's >>> + parent interrupt controller. >>> + >>> +- interrupts: should contain a list of all DECON IP block interrupts >> in the >>> + order: FIFO Level, VSYNC, LCD_SYSTEM. The interrupt specifier >>> + format depends on the interrupt controller used. >>> + >>> +- interrupt-names: should contain the interrupt names: "fifo", "vsync", >>> + "lcd_sys", in the same order as they were listed in the interrupts >>> +property. >>> + >>> +- pinctrl-0: pin control group to be used for this controller. >>> + >>> +- pinctrl-names: must contain a "default" entry. >>> + >>> +- clocks: must include clock specifiers corresponding to entries in the >>> + clock-names property. >>> + >>> +- clock-names: list of clock names sorted in the same order as
Re: [PATCH 6/8] regulator: max77686: Add external GPIO control
On pią, 2014-10-31 at 10:32 +, Mark Brown wrote: > On Fri, Oct 31, 2014 at 08:51:38AM +0100, Krzysztof Kozlowski wrote: > > > However new DT style parsing (regulator_of_get_init_data()) does the > > basic parsing stuff and this removes a lot of code from driver. The > > driver no longer parses all regulaotrs but the regulator core does it. > > Unfortunately regulator core does not parse custom bindings (like such > > GPIOs) so I would have to iterate once again through all regulators just > > to find "gpios" property. > > We could always add a callback for the driver to handle any custom > properties... one of the advantages of an OS like Linux is that we can > improve the core code. I thought about this - adding a callback, called on each child in regulator_of_get_init_data(). However the reason behind this callback is to parse GPIO and set config.ena_gpio. However in that context the regulator_config is const so the callback cannot change it. Unless it casts it to non-const... which isn't what we want I think. So now I wonder whether adding generic bindings for ena_gpio make sense. These would look like bindings for fixed-regulator (with "ena-" prefix). Unfortunately this would duplicate a little the ena_gpio in regulator_config... but to me it seems more appropriate. What do you think about adding generic bindings for ena_gpio? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
nv u-boot for spring (was: building nv-uboot for nyan-big.)
Dear Stephan, Am 03.11.2014 um 11:42 schrieb S.J.R. van Schaik: > [...] I have done some research to figure out how to build > a working nv u-boot binary, and working instructions can be found on the > linux-exynos wiki [1]. However, when I tried to apply them to the snow > and spring boards, by selecting a different branch and a different > u-boot configuration, those attempts were of no success. [...] [...] > The u-boot.bin binary seems to build fine. However, the problem is similar > to the snow and spring boards, where I can't seem to build the DTB-files. You didn't ask me. ;) My Spring branch with patches is located here: https://github.com/afaerber/u-boot/commits/spring For my convenience I changed the sources to default to the right device tree, and I manually inlined exynos-periph-id.dtsi. If you need further info on my build steps, I can look them up in my scripts (on another machine) - I mainly derived them from the official instructions for Snow on the Chromium Wiki. I'm pretty sure I did not mess with the EC firmware at all. But whatever workarounds exist, the Chromium folks should really be working on upstreaming U-Boot support for all those Chromebooks, that usually sanitizes the build process as a side-effect, resulting in less user questions. :) For Snow I believe that to be done. Regards, Andreas > 1. > http://linux-exynos.org/wiki/ARM_Chromebook/Building_u-boot#Building_U-Boot_for_Peach_Pi.2FPit -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] regulator: max77686: Add external GPIO control
On Mon, Nov 03, 2014 at 01:07:02PM +0100, Krzysztof Kozlowski wrote: > On pią, 2014-10-31 at 10:32 +, Mark Brown wrote: > > We could always add a callback for the driver to handle any custom > > properties... one of the advantages of an OS like Linux is that we can > > improve the core code. > I thought about this - adding a callback, called on each child in > regulator_of_get_init_data(). However the reason behind this callback is > to parse GPIO and set config.ena_gpio. However in that context the > regulator_config is const so the callback cannot change it. Unless it > casts it to non-const... which isn't what we want I think. > So now I wonder whether adding generic bindings for ena_gpio make sense. > These would look like bindings for fixed-regulator (with "ena-" prefix). > Unfortunately this would duplicate a little the ena_gpio in > regulator_config... but to me it seems more appropriate. > What do you think about adding generic bindings for ena_gpio? Well, if you only want this for enable GPIO control (sorry, I'm really not reading a lot of these long threads when it looks like there'll be a resubmit anyway) we can always add a way for drivers to specify the name of a property to look at easily enough. signature.asc Description: Digital signature
Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
On 1 November 2014 01:20, Rafael J. Wysocki wrote: > On Friday, October 31, 2014 10:16:14 AM Ulf Hansson wrote: >> On 24 October 2014 18:12, Kevin Hilman wrote: >> > Ulf Hansson writes: >> > >> >> Changes in v3: >> >> -Rework the entire intermediate step which was suggested in v2. >> >>That means solving the race condition, but also cope with PM >> >> domains >> >>that are initialized in powered off state. >> >> >> >> Changes in v2: >> >> -Added some acks. >> >> -Updated commit messages. >> >> -Included a wider audience of the patchset. V1 was lacking SoC >> >>maintainers. >> >> >> >> Here are link to the first patchset, were some discussion started. >> >> http://marc.info/?l=linux-pm&m=141208104729597&w=2 >> >> >> >> There may be more than one device in a PM domain which then will be >> >> probed at different points in time. >> >> >> >> Depending on timing and runtime PM support, in for the device related >> >> driver/subsystem, a PM domain may be advised to power off after a >> >> successful probe sequence. >> >> >> >> A general requirement for a device within a PM domain, is that the >> >> PM domain must stay powered during the probe sequence. To cope with >> >> such requirement, let's add two new APIs, dev_pm_domain_get|put(). >> >> >> >> These APIs are intended to be invoked from subsystem-level code and the >> >> calls between get/put needs to be balanced. >> >> >> >> dev_pm_domain_get(), tells the PM domain that it needs to increase a >> >> usage count and to keep supplying power. dev_pm_domain_put(), does the >> >> opposite. >> > >> > I'm confused. Why arent' pm_runtime_get*() and pm_runtime_put*() working? >> >> See, below. >> >> > >> > What's not explained here (or what I'm not understanding) is why a PM >> > domain is powering off if it has active devices. >> >> It doesn't. The problem is that using pm_runtime_get_sync() in this >> path is not working. >> >> Now, I failed to include some of the important information from >> previous discussions around this patchset. Let me iterate the patchset >> with better commit messages, but let's first continue this thread. >> >> Here are some of the previous discussion: >> >> http://marc.info/?l=linux-pm&m=141270897014653&w=2 >> http://marc.info/?l=linux-pm&m=141208104729597&w=2 >> >> Below is a summary of why I think "pm_runtime_get_sync()" isn't working for >> us. >> >> 1) >> It's bad practice to use pm_runtime_get_sync() in the ->probe() path, > > Honestly, I'm no longer amused. > >> to bring your resources to full power. The consequence would be a >> driver that requires CONFIG_PM_RUNTIME to be even functional, which >> just isn't acceptable. > > Sorry, but this is utter nonsense. I admit, I was too vague while stating this. Looking at the big picture you are obviously right. I should have referred to those SOCs/buses/drivers that I am working/looking at. > > CONFIG_PM_RUNTIME unset means "no runtime PM at all", so all drivers can > expect > everything they need to be always on. If that is not the case, then someone > is > doing runtime PM behind the scenes and therefore cheating. Or in different > words, for CONFIG_PM_RUNTIME unset bus types, platforms etc must ensure that > everything is on from the drivers' perspective. I don't think I have stated anything that isn't in agreement with the above? While I am struggling in making my points clearer, it seems like we look a bit differently upon how runtime PM are being deployed. For those drivers I am working on, it's common that these handles runtime PM resources, like for example clocks. The clocks needs to be enabled for the driver to handle I/O, but those may also be gated at request inactivity to save power. That means, the clocks may be considered as both functional clocks and runtime PM resources. Therefore, the driver must enable its clocks during ->probe() and without relying on CONFIG_PM_RUNTIME to be set. Similar to what you stated for the buses above. To also cope with the scenario where CONFIG_PM_RUNTIME is set, drivers must update the device's runtime PM status using pm_runtime_set_active(), to synchronize the state with the runtime PM core. Otherwise we will get clock unbalance issues while gating/ungating the clocks from the runtime PM callbacks. The AMBA bus and some of its drivers a good example of how this has been implemented: driver/amba/bus.c drivers/mmc/host/mmci.c drivers/spi/spi-pl022.c This conclusion I have made from this is: - Using pm_runtime_get_sync() during the ->probe() path to explicitly power up a PM domain, is not suitable as the _common_ solution to solve the race condition. It certainly may work for some scenarios, but not for those I am looking at. > > If that is the case, then calling pm_runtime_get_sync() from ->probe > for CONFIG_PM_RUNTIME unset simply doesn't matter. > > Now, for CONFIG_PM_RUNTIME enabled, if power domains are in use, doing > pm_runtime_get_sync() from ->probe is the only way t
Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
CC: Kevin, Ulf, Geert.
On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
> These power domain transition notifiers will assist in carrying
> out some activity associated with domain power on/off such as
> some registers which may lose its contents and need save/restore
> across domain power off/on.
>
> 4 type of notifications are added,
> GPD_OFF_PRE - GPD state before power off
> GPD_OFF_POST- GPD state after power off
> GPD_ON_PRE - GPD state before power off
> GPD_ON_POST - GPD state after power off
>
> 3 notfication API's are exported.
> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> enum gpd_on_off_state state);
>
> In this approach the notifiers are registered/unregistered with pd name.
> The actual invoking of the notfiers will be done by the platform implementing
> power domain enable/disable low level handlers according to the above
> defined notification types. This approach will considerably reduce the
> number of call to notifiers as compared to calling always from core
> powerdomain subsystem.
>
> Also the registered domain's will be managed inside a cache list and not
> part of the genpd structure. This will help in registration of notifiers from
> subsystems (such as clock) even when the PD subsystem is still not
> initialised.
> This list also filters out the unregistered pd's requesting notification.
>
> Cc: Rafael J. Wysocki
> Reviewed-by: Pankaj Dubey
> Signed-off-by: Amit Daniel Kachhap
> ---
> drivers/base/power/domain.c | 112
> ++-
> include/linux/pm_domain.h | 31
> 2 files changed, 142 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4..e05045e 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -46,10 +46,19 @@
> __retval;
> \
> })
>
> +struct cache_notify_domains {
> + char *name;
> + /* Node in the cache pm domain name list */
> + struct list_head cache_list_node;
> +};
> +
> static LIST_HEAD(gpd_list);
> static DEFINE_MUTEX(gpd_list_lock);
> +static LIST_HEAD(domain_notify_list);
> +static DEFINE_MUTEX(domain_notify_list_lock);
> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>
> -static struct generic_pm_domain *pm_genpd_lookup_name(const char
> *domain_name)
> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
> {
> struct generic_pm_domain *genpd = NULL, *gpd;
>
> @@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const
> char *domain_name)
> mutex_unlock(&gpd_list_lock);
> return genpd;
> }
> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>
> struct generic_pm_domain *dev_to_genpd(struct device *dev)
> {
> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
> mutex_unlock(&gpd_list_lock);
> }
>
> +/**
> + * genpd_register_notifier - Register a PM domain for future notification.
> + * @nb: notification block containing notification handle.
> + * @pd_name: PM domain name.
> + */
> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
> +{
> + int ret;
> + struct cache_notify_domains *entry;
> +
> + if (!pd_name)
> + return -EINVAL;
> +
> + /* Search if the pd is already registered */
> + mutex_lock(&domain_notify_list_lock);
> + list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> + if (!strcmp(entry->name, pd_name))
> + break;
> + }
> + mutex_unlock(&domain_notify_list_lock);
> +
> + if (entry) {
> + pr_err("%s: pd already exists\n", __func__);
> + return -EINVAL;
> + }
> +
> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> + if (!entry)
> + return -ENOMEM;
> +
> + entry->name = pd_name;
> +
> + mutex_lock(&domain_notify_list_lock);
> + list_add(&entry->cache_list_node, &domain_notify_list);
> + mutex_unlock(&domain_notify_list_lock);
> + ret = blocking_notifier_chain_register(
> + &genpd_transition_notifier_list, nb);
> + return ret;
> +}
> +
> +/**
> + * genpd_unregister_notifier - Un-register a PM domain for future
> notification.
> + * @nb: notification block containing notification handle.
> + * @pd_name: PM domain name.
> + */
> +int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
> +{
> + int ret;
> + struct cache_notify_domains *entry;
> +
> + mutex_lock(&domain_notify_list_lock);
> + list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> + if (!strcmp(entry->name, pd_name))
> +
[PATCH v4 00/14] Add max77802 regulator operating mode support
Hello Mark, This is the fourth version of the series that adds operating modes support for the regulators in the max77802 PMIC. This version uses the standard suspend states bindings and the opmodes are parsed by the regulator core while drivers only define a translation function to map between hardware specific to standard modes as you suggested. The series adds a "regulator-initial-mode" property to configure at startup the operating mode for the regulators that support changing its mode during normal operation and a "regulator-mode" property for the regulators that supports changing its operating mode when the system enters in a suspend state. These properties were originally part of Chanwoo Choi's regulator suspend state series [0] but were removed since there wasn't a way to define the operating modes in a generic way. In this series, the generic regulator DT binding doc explains that each device has to document what their valid operating modes are. Drivers must provide a translation function so the core can map the modes. Since parsing the modes in the core is a very different approach, most of the patches are new but those that remains have a changelog. This series depend on [0] and also v2 of patch: "ARM: EXYNOS: Call regulator core suspend prepare and finish functions" [1]. Javier Martinez Canillas (14): regulator: Document binding for initial and suspend modes regulator: Add function to map modes to struct regulator_desc regulator: of: Add regulator desc param to of_get_regulator_init_data() regulator: of: Pass the regulator description in the match table regulator: max1586: zero-initialize regulator match table array regulator: max77686: zero-initialize regulator match table regulator: max77802: zero-initialize regulator match table regulator: max8660: zero-initialize regulator match table array regulator: s2mpa01: zero-initialize regulator match table array regulator: of: Add support for parsing initial and suspend modes regulator: max77802: Document binding for regulator operating modes regulator: max77802: Use unsigned int for modes in max77802_map_mode() regulator: max77802: Set regulator modes translation callback ARM: dts: Configure regulators for suspend on exynos Peach boards Patch #1 extends the regulator DT binding to document the initial and suspend modes properties. Patch #2 adds a function pointer to the static regulator description so drivers can define a callback that does the modes translation. Patch #3 does some refactoring to pass the regulator descriptor to the function extracting the regulator initial data from DT. Patch #4 adds a pointer to the regulator descriptor in the match table so users extracting the init_data from of_regulator_match can also map the modes. Patch #5-#9 are fixes to be sure that all callers are passing an initialised match table. Patch #10 modifies the function that extracts the regulator data from DT to parse the initial and suspend modes. Patch #11 extends the max77802 DT binding to document the valid opmodes for the regulators on this device. Patch #12 change the signature of the function doing the modes mapping for the max77802 regulators mode. Patch #13 set the function handler for the max77802 modes translation. Patch #14 configure the regulators for the Peach Pit and Pi Chromebooks. Best regards, Javier [0]: https://lkml.org/lkml/2014/10/10/161 [1]: http://www.spinics.net/lists/arm-kernel/msg369923.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: dts: Add pwm-fan node to the Odroid-U3 board
Add pwm-fan node to the Odroid-U3 board file to enable PWM control of the
cooling fan.
Signed-off-by: Kamil Debski
---
arch/arm/boot/dts/exynos4412-odroidu3.dts | 13 +
1 file changed, 13 insertions(+)
diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts
b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index c8a64be..764bda4 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -31,6 +31,19 @@
linux,default-trigger = "heartbeat";
};
};
+
+ pwm: pwm@139D {
+ pinctrl-0 = <&pwm0_out>;
+ pinctrl-names = "default";
+ samsung,pwm-outputs = <0>;
+ status = "okay";
+ };
+
+ pwm-fan {
+ compatible = "pwm-fan";
+ status = "okay";
+ pwms = <&pwm 0 1 0>;
+ };
};
&usb3503 {
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 13/14] regulator: max77802: Set regulator modes translation callback
The max77802 PMIC regulators output can be configured in one of two
modes: Output ON (normal) and Output ON in Low Power Mode. Some of
the regulators support their operating mode to be changed on startup
or by consumers when the system is running while others only support
their operating mode to be changed while the system has entered in a
suspend state.
Use the max77802_map_mode() function to translate the device specific
modes to the standard operating modes as used by the regulator core.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/max77802.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index 1d1f7b4..09276c2 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -375,6 +375,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
.vsel_mask = MAX77802_VSEL_MASK, \
.enable_reg = MAX77802_REG_LDO1CTRL1 + num - 1, \
.enable_mask= MAX77802_OPMODE_MASK << MAX77802_OPMODE_SHIFT_LDO, \
+ .map_modes = max77802_map_mode,\
}
/* LDOs 1, 2, 8, 15, 17, 27, 30, 35 */
@@ -393,6 +394,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
.vsel_mask = MAX77802_VSEL_MASK, \
.enable_reg = MAX77802_REG_LDO1CTRL1 + num - 1, \
.enable_mask= MAX77802_OPMODE_MASK << MAX77802_OPMODE_SHIFT_LDO, \
+ .map_modes = max77802_map_mode,\
}
/* BUCKs 1, 6 */
@@ -411,6 +413,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
.vsel_mask = MAX77802_DVS_VSEL_MASK, \
.enable_reg = MAX77802_REG_BUCK ## num ## CTRL, \
.enable_mask= MAX77802_OPMODE_MASK, \
+ .map_modes = max77802_map_mode,\
}
/* BUCKS 2-4 */
@@ -430,6 +433,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
.enable_reg = MAX77802_REG_BUCK ## num ## CTRL1,\
.enable_mask= MAX77802_OPMODE_MASK << \
MAX77802_OPMODE_BUCK234_SHIFT, \
+ .map_modes = max77802_map_mode,\
}
/* BUCK 5 */
@@ -448,6 +452,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
.vsel_mask = MAX77802_VSEL_MASK, \
.enable_reg = MAX77802_REG_BUCK5CTRL, \
.enable_mask= MAX77802_OPMODE_MASK, \
+ .map_modes = max77802_map_mode,\
}
/* BUCKs 7-10 */
@@ -466,6 +471,7 @@ static struct regulator_ops max77802_buck_dvs_ops = {
.vsel_mask = MAX77802_VSEL_MASK, \
.enable_reg = MAX77802_REG_BUCK7CTRL + (num - 7) * 3, \
.enable_mask= MAX77802_OPMODE_MASK, \
+ .map_modes = max77802_map_mode,\
}
static const struct regulator_desc regulators[] = {
@@ -540,6 +546,7 @@ static int max77802_pmic_dt_parse_pdata(struct
platform_device *pdev,
for (i = 0; i < pdata->num_regulators; i++) {
rmatch.name = regulators[i].name;
+ rmatch.desc = ®ulators[i];
rmatch.init_data = NULL;
rmatch.of_node = NULL;
if (of_regulator_match(&pdev->dev, regulators_np, &rmatch,
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 06/14] regulator: max77686: zero-initialize regulator match table
The struct of_regulator_match is declared as a non-static local variable
so the structure members are not auto-initialized.
Initialize the struct at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/max77686.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
index 09b0d8c..281eb4b 100644
--- a/drivers/regulator/max77686.c
+++ b/drivers/regulator/max77686.c
@@ -434,7 +434,7 @@ static int max77686_pmic_dt_parse_pdata(struct
platform_device *pdev,
struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
struct device_node *pmic_np, *regulators_np;
struct max77686_regulator_data *rdata;
- struct of_regulator_match rmatch;
+ struct of_regulator_match rmatch = { };
unsigned int i;
pmic_np = iodev->dev->of_node;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 14/14] ARM: dts: Configure regulators for suspend on exynos Peach boards
The regulator core now has support to choose if a regulator
has to be enabled or disabled during system suspend and also
the supports changing the regulator operating mode during
runtime and when the system enters into sleep mode.
To lower power during suspend, configure the regulators state
using the same configuration found in the ChromeOS 3.8 kernel.
Signed-off-by: Javier Martinez Canillas
---
Changes in v4: None
Changes in v3:
- Use the standard suspend state binding as suggested by Mark Brown.
arch/arm/boot/dts/exynos5420-peach-pit.dts | 81 ++
arch/arm/boot/dts/exynos5800-peach-pi.dts | 81 ++
2 files changed, 162 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts
b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 9a050e1..8b744c7 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include "exynos5420.dtsi"
/ {
@@ -192,6 +193,9 @@
regulator-always-on;
regulator-boot-on;
regulator-ramp-delay = <12500>;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck2_reg: BUCK2 {
@@ -201,6 +205,9 @@
regulator-always-on;
regulator-boot-on;
regulator-ramp-delay = <12500>;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck3_reg: BUCK3 {
@@ -210,6 +217,9 @@
regulator-always-on;
regulator-boot-on;
regulator-ramp-delay = <12500>;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck4_reg: BUCK4 {
@@ -219,6 +229,9 @@
regulator-always-on;
regulator-boot-on;
regulator-ramp-delay = <12500>;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck5_reg: BUCK5 {
@@ -227,6 +240,9 @@
regulator-max-microvolt = <120>;
regulator-always-on;
regulator-boot-on;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck6_reg: BUCK6 {
@@ -236,6 +252,9 @@
regulator-always-on;
regulator-boot-on;
regulator-ramp-delay = <12500>;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck7_reg: BUCK7 {
@@ -244,6 +263,9 @@
regulator-max-microvolt = <135>;
regulator-always-on;
regulator-boot-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
buck8_reg: BUCK8 {
@@ -252,6 +274,9 @@
regulator-max-microvolt = <285>;
regulator-always-on;
regulator-boot-on;
+ regulator-state-mem {
+ regulator-off-in-suspend;
+ };
};
buck9_reg: BUCK9 {
@@ -260,6 +285,9 @@
regulator-max-microvolt = <200>;
regulator-always-on;
regulator-boot-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
};
buck10_reg: BUCK10 {
@@ -268,6 +296,9 @@
regulator-max-microvolt = <180>;
regulator-always-on;
regulator-boot-on;
+
[PATCH v4 12/14] regulator: max77802: Use unsigned int for modes in max77802_map_mode()
All function dealing with operating modes use unsigned int for modes
so change max77802_map_mode() function signature for consistency.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/max77802.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index d10c605..1d1f7b4 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -73,7 +73,7 @@ struct max77802_regulator_prv {
unsigned int opmode[MAX77802_REG_MAX];
};
-static inline int max77802_map_mode(int mode)
+static inline unsigned int max77802_map_mode(unsigned int mode)
{
return mode == MAX77802_OPMODE_NORMAL ?
REGULATOR_MODE_NORMAL : REGULATOR_MODE_STANDBY;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 10/14] regulator: of: Add support for parsing initial and suspend modes
Some regulators support their operating mode to be changed on startup
or by consumers when the system is running while others only support
their operating mode to be changed while the system has entered in a
suspend state.
The regulator Device Tree binding documents a set of properties to
configure the regulators operating modes from a FDT. This patch builds
on (40e20d6 regulator: of: Add support for parsing regulator_state for
suspend state) and adds support to parse those properties and fill the
regulator constraints so the regulator core can call the right suspend
handlers when the system enters into sleep.
The modes are defined in the Device Tree using the hardware specific
modes supported by the regulators. Regulator drivers have to define a
translation function that is used to map the hardware specific modes
to the standard ones.
Signed-off-by: Javier Martinez Canillas
---
Changes in v4:
- Parse the properties in the core and map using driver provided functions.
Suggested by Mark Brown
Changes in v3:
- Use the standard suspend states binding instead of custom properties.
Suggested by Mark Brown
drivers/regulator/of_regulator.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index cbc1d71..cd65885 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -25,7 +25,8 @@ static const char *const regulator_states[PM_SUSPEND_MAX + 1]
= {
};
static void of_get_regulation_constraints(struct device_node *np,
- struct regulator_init_data **init_data)
+ struct regulator_init_data **init_data,
+ const struct regulator_desc *desc)
{
const __be32 *min_uV, *max_uV;
struct regulation_constraints *constraints = &(*init_data)->constraints;
@@ -81,6 +82,14 @@ static void of_get_regulation_constraints(struct device_node
*np,
if (!ret)
constraints->enable_time = pval;
+ if (!of_property_read_u32(np, "regulator-initial-mode", &pval)) {
+ if (desc && desc->map_modes)
+ constraints->initial_mode = desc->map_modes(pval);
+ else
+ pr_warn("%s: failed to parse regulator-initial-mode\n",
+ np->name);
+ }
+
for (i = 0; i < ARRAY_SIZE(regulator_states); i++) {
switch (i) {
case PM_SUSPEND_MEM:
@@ -100,6 +109,15 @@ static void of_get_regulation_constraints(struct
device_node *np,
if (!suspend_np || !suspend_state)
continue;
+ if (!of_property_read_u32(suspend_np, "regulator-mode",
+ &pval)) {
+ if (desc && desc->map_modes)
+ suspend_state->mode = desc->map_modes(pval);
+ else
+ pr_warn("%s: failed to parse regulator-mode\n",
+ np->name);
+ }
+
if (of_property_read_bool(suspend_np,
"regulator-on-in-suspend"))
suspend_state->enabled = true;
@@ -140,7 +158,7 @@ struct regulator_init_data
*of_get_regulator_init_data(struct device *dev,
if (!init_data)
return NULL; /* Out of memory? */
- of_get_regulation_constraints(node, &init_data);
+ of_get_regulation_constraints(node, &init_data, desc);
return init_data;
}
EXPORT_SYMBOL_GPL(of_get_regulator_init_data);
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 11/14] regulator: max77802: Document binding for regulator operating modes
Some regulators from the max77802 PMIC support to be configured in one
of two operating mode: Output ON (normal) and Output On Low Power Mode.
Not all regulators support these two modes and for some of them, the
mode can be changed while the system is running in normal operation
while others only support their mode to be changed on system suspend.
Extend the max77802 PMIC bindin by documenting the possible operating
modes values so the regulators modes can be correctly configured.
Signed-off-by: Javier Martinez Canillas
---
Changes in v4: None
Changes in v3:
- Use the standard suspend states bindings as suggested by Mark Brown.
.../devicetree/bindings/regulator/max77802.txt | 33 ++
1 file changed, 33 insertions(+)
diff --git a/Documentation/devicetree/bindings/regulator/max77802.txt
b/Documentation/devicetree/bindings/regulator/max77802.txt
index 5aeaffc..1a78ec2 100644
--- a/Documentation/devicetree/bindings/regulator/max77802.txt
+++ b/Documentation/devicetree/bindings/regulator/max77802.txt
@@ -25,6 +25,27 @@ with their hardware counterparts as follow. The valid names
are:
example: LDO1, LDO2, LDO35.
-BUCKn : for BUCKs, where n can lie in range 1 to 10.
example: BUCK1, BUCK5, BUCK10.
+
+The max77802 regulator supports two different operating modes: Normal and Low
+Power Mode. Some regulators support the modes to be changed at startup or by
+the consumers during normal operation while others only support to change the
+mode during system suspend. The standard regulator suspend states binding can
+be used to configure the regulator operating mode.
+
+The regulators that support the standard "regulator-initial-mode" property,
+changing their mode during normal operation are: LDOs 1, 3, 20 and 21.
+
+The possible values for "regulator-initial-mode" and "regulator-mode" are:
+ 1: Normal regulator voltage output mode.
+ 3: Low Power which reduces the quiescent current down to only 1uA
+
+The list of valid modes are defined in the dt-bindings/regulator/regulator.h
+header and can be included by device tree source files.
+
+The standard "regulator-mode" property can only be used for regulators that
+support changing their mode to Low Power Mode during suspend. These regulators
+are: BUCKs 2-4 and LDOs 1-35.
+
Example:
max77802@09 {
@@ -36,11 +57,23 @@ Example:
#size-cells = <0>;
regulators {
+ ldo1_reg: LDO1 {
+ regulator-name = "vdd_1v0";
+ regulator-min-microvolt = <100>;
+ regulator-max-microvolt = <100>;
+ regulator-always-on;
+ regulator-initial-mode = ;
+ };
+
ldo11_reg: LDO11 {
regulator-name = "vdd_ldo11";
regulator-min-microvolt = <190>;
regulator-max-microvolt = <190>;
regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = ;
+ };
};
buck1_reg: BUCK1 {
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 09/14] regulator: s2mpa01: zero-initialize regulator match table array
The struct of_regulator_match rmatch[] is declared as a non-static local
variable so the structure members are not auto-initialized.
Initialize the array at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/s2mpa01.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/s2mpa01.c b/drivers/regulator/s2mpa01.c
index 2263071..5db4e12 100644
--- a/drivers/regulator/s2mpa01.c
+++ b/drivers/regulator/s2mpa01.c
@@ -341,7 +341,7 @@ static int s2mpa01_pmic_probe(struct platform_device *pdev)
{
struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
struct sec_platform_data *pdata = dev_get_platdata(iodev->dev);
- struct of_regulator_match rdata[S2MPA01_REGULATOR_MAX];
+ struct of_regulator_match rdata[S2MPA01_REGULATOR_MAX] = { };
struct device_node *reg_np = NULL;
struct regulator_config config = { };
struct s2mpa01_info *s2mpa01;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 08/14] regulator: max8660: zero-initialize regulator match table array
The struct of_regulator_match rmatch[] is declared as a non-static local
variable so the structure members are not auto-initialized.
Initialize the array at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/max8660.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/max8660.c b/drivers/regulator/max8660.c
index 2fc4111..7eee2ca 100644
--- a/drivers/regulator/max8660.c
+++ b/drivers/regulator/max8660.c
@@ -335,7 +335,7 @@ static int max8660_pdata_from_dt(struct device *dev,
int matched, i;
struct device_node *np;
struct max8660_subdev_data *sub;
- struct of_regulator_match rmatch[ARRAY_SIZE(max8660_reg)];
+ struct of_regulator_match rmatch[ARRAY_SIZE(max8660_reg)] = { };
np = of_get_child_by_name(dev->of_node, "regulators");
if (!np) {
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 02/14] regulator: Add function to map modes to struct regulator_desc
The regulator-initial-mode and regulator-mode DT properties allows to
configure the regulator operating modes at startup or when a system
enters into a susend state.
But these properties use as valid values the operating modes supported
by each device while the core deals with the standard operating modes.
So a mapping function is needed to translate from the hardware specific
modes to the standard ones.
This mapping is a non-varying configuration for each regulator, so add
a function pointer to struct regulator_desc that will allow drivers to
define their callback to do the modes translation.
Signed-off-by: Javier Martinez Canillas
---
include/linux/regulator/driver.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 28da08e..b54d037 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -243,6 +243,8 @@ enum regulator_type {
*
* @enable_time: Time taken for initial enable of regulator (in uS).
* @off_on_delay: guard time (in uS), before re-enabling a regulator
+ *
+ * @map_modes: Callback invoked to translate between hardware to standard
modes.
*/
struct regulator_desc {
const char *name;
@@ -285,6 +287,8 @@ struct regulator_desc {
unsigned int enable_time;
unsigned int off_on_delay;
+
+ unsigned int (*map_modes)(unsigned int mode);
};
/**
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 07/14] regulator: max77802: zero-initialize regulator match table
The struct of_regulator_match is declared as a non-static local variable
so the structure members are not auto-initialized.
Initialize the struct at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/max77802.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/max77802.c b/drivers/regulator/max77802.c
index a0d1462..d10c605 100644
--- a/drivers/regulator/max77802.c
+++ b/drivers/regulator/max77802.c
@@ -520,7 +520,7 @@ static int max77802_pmic_dt_parse_pdata(struct
platform_device *pdev,
struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
struct device_node *pmic_np, *regulators_np;
struct max77686_regulator_data *rdata;
- struct of_regulator_match rmatch;
+ struct of_regulator_match rmatch = { };
unsigned int i;
pmic_np = iodev->dev->of_node;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()
The of_get_regulator_init_data() function is used to extract the regulator
init_data but information on how to extract certain data is defined in the
static regulator descriptor (e.g: how to map the hardware operating modes).
Add a const struct regulator_desc * parameter to the function signature so
the parsing logic could use the information in the struct regulator_desc.
of_get_regulator_init_data() relies on of_get_regulation_constraints() to
actually extract the init_data so it has to pass the struct regulator_desc
but that is changed in a following patch.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/88pm8607.c | 3 ++-
drivers/regulator/anatop-regulator.c | 2 +-
drivers/regulator/arizona-ldo1.c | 3 ++-
drivers/regulator/arizona-micsupp.c| 2 +-
drivers/regulator/da9052-regulator.c | 2 +-
drivers/regulator/da9210-regulator.c | 2 +-
drivers/regulator/fan53555.c | 2 +-
drivers/regulator/fixed.c | 2 +-
drivers/regulator/gpio-regulator.c | 2 +-
drivers/regulator/max8952.c| 2 +-
drivers/regulator/max8973-regulator.c | 3 ++-
drivers/regulator/max8997.c| 2 +-
drivers/regulator/max8998.c| 4 ++--
drivers/regulator/mc13xxx-regulator-core.c | 3 ++-
drivers/regulator/of_regulator.c | 9 ++---
drivers/regulator/pwm-regulator.c | 2 +-
drivers/regulator/qcom_rpm-regulator.c | 3 ++-
drivers/regulator/s5m8767.c| 2 +-
drivers/regulator/sky81452-regulator.c | 2 +-
drivers/regulator/stw481x-vmmc.c | 2 +-
drivers/regulator/ti-abb-regulator.c | 2 +-
drivers/regulator/tps51632-regulator.c | 3 ++-
drivers/regulator/tps62360-regulator.c | 3 ++-
drivers/regulator/tps65218-regulator.c | 3 ++-
drivers/regulator/twl-regulator.c | 2 +-
drivers/regulator/vexpress.c | 3 ++-
include/linux/regulator/of_regulator.h | 8 ++--
27 files changed, 47 insertions(+), 31 deletions(-)
diff --git a/drivers/regulator/88pm8607.c b/drivers/regulator/88pm8607.c
index 6d77dcd..ce0343a 100644
--- a/drivers/regulator/88pm8607.c
+++ b/drivers/regulator/88pm8607.c
@@ -330,7 +330,8 @@ static int pm8607_regulator_dt_init(struct platform_device
*pdev,
for_each_child_of_node(nproot, np) {
if (!of_node_cmp(np->name, info->desc.name)) {
config->init_data =
- of_get_regulator_init_data(&pdev->dev, np);
+ of_get_regulator_init_data(&pdev->dev, np,
+ NULL);
config->of_node = np;
break;
}
diff --git a/drivers/regulator/anatop-regulator.c
b/drivers/regulator/anatop-regulator.c
index 542d14e..5cdce49 100644
--- a/drivers/regulator/anatop-regulator.c
+++ b/drivers/regulator/anatop-regulator.c
@@ -189,7 +189,7 @@ static int anatop_regulator_probe(struct platform_device
*pdev)
int ret = 0;
u32 val;
- initdata = of_get_regulator_init_data(dev, np);
+ initdata = of_get_regulator_init_data(dev, np, NULL);
sreg = devm_kzalloc(dev, sizeof(*sreg), GFP_KERNEL);
if (!sreg)
return -ENOMEM;
diff --git a/drivers/regulator/arizona-ldo1.c b/drivers/regulator/arizona-ldo1.c
index 4c9db58..fb393c5 100644
--- a/drivers/regulator/arizona-ldo1.c
+++ b/drivers/regulator/arizona-ldo1.c
@@ -194,7 +194,8 @@ static int arizona_ldo1_of_get_pdata(struct arizona
*arizona,
if (init_node) {
config->of_node = init_node;
- init_data = of_get_regulator_init_data(arizona->dev, init_node);
+ init_data = of_get_regulator_init_data(arizona->dev, init_node,
+ NULL);
if (init_data) {
init_data->consumer_supplies = &ldo1->supply;
diff --git a/drivers/regulator/arizona-micsupp.c
b/drivers/regulator/arizona-micsupp.c
index ce9aca5..71290bf 100644
--- a/drivers/regulator/arizona-micsupp.c
+++ b/drivers/regulator/arizona-micsupp.c
@@ -210,7 +210,7 @@ static int arizona_micsupp_of_get_pdata(struct arizona
*arizona,
if (np) {
config->of_node = np;
- init_data = of_get_regulator_init_data(arizona->dev, np);
+ init_data = of_get_regulator_init_data(arizona->dev, np, NULL);
if (init_data) {
init_data->consumer_supplies = &micsupp->supply;
diff --git a/drivers/regulator/da9052-regulator.c
b/drivers/regulator/da9052-regulator.c
index 0003362..178343c 100644
--- a/drivers/regulator/da9052-regulator.c
+++ b/drivers/regulator/da9052-regulator.c
@@ -436,7 +436,7 @@ static int da9052_regulator_probe(struct platform_device
*pdev)
if (!of_node_cmp(np->name,
[PATCH v4 04/14] regulator: of: Pass the regulator description in the match table
Drivers can use the of_regulator_match() function to parse the regulator
init_data from DT. A match table is used to specify the name of the node
containing the regulators, the device node and to return the init_data
to the caller.
But also the static regulator descriptor is needed to correctly extract
some DT properties like the regulator initial and suspend modes. Use the
match table to pass that information.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/of_regulator.c | 3 ++-
include/linux/regulator/of_regulator.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 945486f..cbc1d71 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -221,7 +221,8 @@ int of_regulator_match(struct device *dev, struct
device_node *node,
continue;
match->init_data =
- of_get_regulator_init_data(dev, child, NULL);
+ of_get_regulator_init_data(dev, child,
+ match->desc);
if (!match->init_data) {
dev_err(dev,
"failed to parse DT for regulator %s\n",
diff --git a/include/linux/regulator/of_regulator.h
b/include/linux/regulator/of_regulator.h
index 3bbfb1b..ce0877d 100644
--- a/include/linux/regulator/of_regulator.h
+++ b/include/linux/regulator/of_regulator.h
@@ -13,6 +13,7 @@ struct of_regulator_match {
void *driver_data;
struct regulator_init_data *init_data;
struct device_node *of_node;
+ const struct regulator_desc *desc;
};
#if defined(CONFIG_OF)
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 05/14] regulator: max1586: zero-initialize regulator match table array
The struct of_regulator_match rmatch[] is declared as a non-static local
variable so the structure members are not auto-initialized.
Initialize the array at declaration time to avoid the structure members
values to be indeterminate and have sane defaults instead.
Signed-off-by: Javier Martinez Canillas
---
drivers/regulator/max1586.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/regulator/max1586.c b/drivers/regulator/max1586.c
index 86db310..d2a8c64 100644
--- a/drivers/regulator/max1586.c
+++ b/drivers/regulator/max1586.c
@@ -163,7 +163,7 @@ static int of_get_max1586_platform_data(struct device *dev,
struct max1586_platform_data *pdata)
{
struct max1586_subdev_data *sub;
- struct of_regulator_match rmatch[ARRAY_SIZE(max1586_reg)];
+ struct of_regulator_match rmatch[ARRAY_SIZE(max1586_reg)] = { };
struct device_node *np = dev->of_node;
int i, matched;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 01/14] regulator: Document binding for initial and suspend modes
Some regulators can run on different operating modes (opmodes). This allows systems to choose the most efficient opmode for each regulator. This patch builds on top of (291d761 regulator: Document binding for regulator suspend state for PM state) adding a regulator-initial-mode DT property to configure at startup the operating mode for regulators that support changing its mode during normal operation and a property regulator-mode to be used in the regulator-state-[mem/disk] nodes for regulators that supports changing its operating mode when the system enters in a suspend state. The set of possible modes that a regulator can operate depends on the hardware capabilities so a list of generic operating modes can't be provided. Instead, each hardware binding should define the list of valid operating modes for the regulators found on that device. Signed-off-by: Javier Martinez Canillas --- Changes in v3: - Rebased on top of regulator suspend voltage series Documentation/devicetree/bindings/regulator/regulator.txt | 14 ++ 1 file changed, 14 insertions(+) diff --git a/Documentation/devicetree/bindings/regulator/regulator.txt b/Documentation/devicetree/bindings/regulator/regulator.txt index 4e7ed76..3fffa3b 100644 --- a/Documentation/devicetree/bindings/regulator/regulator.txt +++ b/Documentation/devicetree/bindings/regulator/regulator.txt @@ -30,6 +30,20 @@ Optional properties: - regulator-off-in-suspend: regulator should be off in suspend state. - regulator-suspend-microvolt: regulator should be set to this voltage in suspend. + - regulator-mode: operating mode in the given suspend state. + The set of possible operating modes depends on the capabilities of + every hardware so the valid modes are documented on each regulator + device tree binding document. + The "regulator-mode" property only takes effect if the regulator is + enabled for the given suspend state using "regulator-on-in-suspend". + If the regulator has not been explicitly disabled for the given state + with "regulator-off-in-suspend", then setting the operating mode + will also have no effect. +- regulator-initial-mode: initial operating mode. The set of possible operating + modes is the same used for the regulator-mode property and the device binding + documentation explains which property each regulator supports. +If no mode is defined, then the OS will not manage the modes and the hardware +default values will be used instead. Deprecated properties: - regulator-compatible: If a regulator chip contains multiple -- 2.1.0 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
On 3 November 2014 15:54, Rafael J. Wysocki wrote:
> CC: Kevin, Ulf, Geert.
>
> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>> These power domain transition notifiers will assist in carrying
>> out some activity associated with domain power on/off such as
>> some registers which may lose its contents and need save/restore
>> across domain power off/on.
Hi Amit,
This mechanism seems to be needed - somehow.
A similar approach has been posted earlier, I am not sure that
discussion was fully settled though.
http://marc.info/?l=linux-pm&m=136448756308203&w=2
Anyway, I plan to review this shortly.
Kind regards
Uffe
>>
>> 4 type of notifications are added,
>> GPD_OFF_PRE - GPD state before power off
>> GPD_OFF_POST- GPD state after power off
>> GPD_ON_PRE - GPD state before power off
>> GPD_ON_POST - GPD state after power off
>>
>> 3 notfication API's are exported.
>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> enum gpd_on_off_state state);
>>
>> In this approach the notifiers are registered/unregistered with pd name.
>> The actual invoking of the notfiers will be done by the platform implementing
>> power domain enable/disable low level handlers according to the above
>> defined notification types. This approach will considerably reduce the
>> number of call to notifiers as compared to calling always from core
>> powerdomain subsystem.
>>
>> Also the registered domain's will be managed inside a cache list and not
>> part of the genpd structure. This will help in registration of notifiers from
>> subsystems (such as clock) even when the PD subsystem is still not
>> initialised.
>> This list also filters out the unregistered pd's requesting notification.
>>
>> Cc: Rafael J. Wysocki
>> Reviewed-by: Pankaj Dubey
>> Signed-off-by: Amit Daniel Kachhap
>> ---
>> drivers/base/power/domain.c | 112
>> ++-
>> include/linux/pm_domain.h | 31
>> 2 files changed, 142 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 40bc2f4..e05045e 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -46,10 +46,19 @@
>> __retval;
>> \
>> })
>>
>> +struct cache_notify_domains {
>> + char *name;
>> + /* Node in the cache pm domain name list */
>> + struct list_head cache_list_node;
>> +};
>> +
>> static LIST_HEAD(gpd_list);
>> static DEFINE_MUTEX(gpd_list_lock);
>> +static LIST_HEAD(domain_notify_list);
>> +static DEFINE_MUTEX(domain_notify_list_lock);
>> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>>
>> -static struct generic_pm_domain *pm_genpd_lookup_name(const char
>> *domain_name)
>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>> {
>> struct generic_pm_domain *genpd = NULL, *gpd;
>>
>> @@ -66,6 +75,7 @@ static struct generic_pm_domain
>> *pm_genpd_lookup_name(const char *domain_name)
>> mutex_unlock(&gpd_list_lock);
>> return genpd;
>> }
>> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>>
>> struct generic_pm_domain *dev_to_genpd(struct device *dev)
>> {
>> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>> mutex_unlock(&gpd_list_lock);
>> }
>>
>> +/**
>> + * genpd_register_notifier - Register a PM domain for future notification.
>> + * @nb: notification block containing notification handle.
>> + * @pd_name: PM domain name.
>> + */
>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> + int ret;
>> + struct cache_notify_domains *entry;
>> +
>> + if (!pd_name)
>> + return -EINVAL;
>> +
>> + /* Search if the pd is already registered */
>> + mutex_lock(&domain_notify_list_lock);
>> + list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>> + if (!strcmp(entry->name, pd_name))
>> + break;
>> + }
>> + mutex_unlock(&domain_notify_list_lock);
>> +
>> + if (entry) {
>> + pr_err("%s: pd already exists\n", __func__);
>> + return -EINVAL;
>> + }
>> +
>> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>> + if (!entry)
>> + return -ENOMEM;
>> +
>> + entry->name = pd_name;
>> +
>> + mutex_lock(&domain_notify_list_lock);
>> + list_add(&entry->cache_list_node, &domain_notify_list);
>> + mutex_unlock(&domain_notify_list_lock);
>> + ret = blocking_notifier_chain_register(
>> + &genpd_transition_notifier_list, nb);
>> + return ret;
>> +}
>> +
>> +/**
>> + * genpd_unregister_notifier - Un-register a PM domain for future
>> notification.
>> +
Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver
On Mon, 03 Nov 2014, Amit Daniel Kachhap wrote: > From: Pankaj Dubey > > This patch moves Exynos PMU driver implementation from "arm/mach-exynos" > to "drivers/mfd". This driver is mainly used for setting misc bits of > register from PMU IP of Exynos SoC which will be required to configure > before Suspend/Resume. Currently all these settings are done in > "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC support, > there is a need of DT based implementation of PMU driver. > > This driver uses already existing DT binding information. > > CC: Sangbeom Kim > CC: Samuel Ortiz > CC: Lee Jones > Signed-off-by: Pankaj Dubey > Signed-off-by: Amit Daniel Kachhap > --- > arch/arm/mach-exynos/Kconfig |1 + > arch/arm/mach-exynos/Makefile |2 +- > drivers/mfd/Kconfig|9 + > drivers/mfd/Makefile |1 + > .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c |0 > 5 files changed, 12 insertions(+), 1 deletion(-) > rename arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%) I thought we spoke about this already [1]? What's changed? [1] https://lkml.org/lkml/2014/4/29/193 -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()
On Mon, Nov 03, 2014 at 03:40:38PM +0100, Javier Martinez Canillas wrote:
> for_each_child_of_node(nproot, np) {
> if (!of_node_cmp(np->name, info->desc.name)) {
> config->init_data =
> - of_get_regulator_init_data(&pdev->dev, np);
> + of_get_regulator_init_data(&pdev->dev, np,
> +NULL);
This looks buggy, we're not passing in a descriptor.
signature.asc
Description: Digital signature
Re: [PATCH v4 05/14] regulator: max1586: zero-initialize regulator match table array
On Mon, Nov 03, 2014 at 03:40:40PM +0100, Javier Martinez Canillas wrote: > The struct of_regulator_match rmatch[] is declared as a non-static local > variable so the structure members are not auto-initialized. Applied, thanks. This is a bug fix not *that* closely related to the rest of the series, if it's being included in a series like this it should've been at the start of the series not after substantial new feature additions so that the fixes don't get ignored due to problems with the features and so that the fixes can be sent to Linus without dependencies. signature.asc Description: Digital signature
Re: [PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()
On 11/03/2014 04:33 PM, Mark Brown wrote:
> On Mon, Nov 03, 2014 at 03:40:38PM +0100, Javier Martinez Canillas wrote:
>
>> for_each_child_of_node(nproot, np) {
>> if (!of_node_cmp(np->name, info->desc.name)) {
>> config->init_data =
>> -of_get_regulator_init_data(&pdev->dev, np);
>> +of_get_regulator_init_data(&pdev->dev, np,
>> + NULL);
>
> This looks buggy, we're not passing in a descriptor.
>
The descriptor is only used when extracting the init_data to map the
modes and since it was not a parameter before, some drivers needs to
be refactored to pass that information.
I thought that instead of adding intrusive changes in drivers that I
don't have hw to test, that parameter could be optional so the patch
that use that information check if the descriptor and the map_modes
function pointer are not NULL.
Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 05/14] regulator: max1586: zero-initialize regulator match table array
On 11/03/2014 04:41 PM, Mark Brown wrote: > On Mon, Nov 03, 2014 at 03:40:40PM +0100, Javier Martinez Canillas wrote: >> The struct of_regulator_match rmatch[] is declared as a non-static local >> variable so the structure members are not auto-initialized. > > Applied, thanks. > > This is a bug fix not *that* closely related to the rest of the series, > if it's being included in a series like this it should've been at the > start of the series not after substantial new feature additions so that > the fixes don't get ignored due to problems with the features and so > that the fixes can be sent to Linus without dependencies. > True, I added in the series since of_get_regulation_constraints() checks if the struct regulator_desc * pointer is not NULL and so drivers that don't initialize the struct of_regulator_match containing that pointer will make that check to fail and the pointer be wrongly dereferenced. Sorry, next time I'll post fixes separately and make the series depend on that instead. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/14] Add max77802 regulator operating mode support
On Mon, Nov 03, 2014 at 03:40:35PM +0100, Javier Martinez Canillas wrote: > Hello Mark, > > This is the fourth version of the series that adds operating modes > support for the regulators in the max77802 PMIC. This version uses No, it's not. This is a a patch series doing a whole bunch of different things, there's at least bug fixes to existing drivers, new features and also this new driver in what I've glanced at so far. These things shouldn't just be being thrown together into a single patch series, and the patch series shouldn't then be described as just being what is in the end a minor part of the collection. Doing this makes things more manageable from the review side, avoids pointless dependencies and avoids setting off alarm bells. My first thought when seeing this was "how can a regulator driver be so complex as to need 14 patches, there must be something seriously wrong here". The whole thing about making sure that what you're doing makes sense beyond just giving the correct test results also applies to sending the patches - think if what's being said to the people reviewing the patches is sensible. signature.asc Description: Digital signature
Re: [PATCH v4 06/14] regulator: max77686: zero-initialize regulator match table
On Mon, Nov 03, 2014 at 03:40:41PM +0100, Javier Martinez Canillas wrote: > The struct of_regulator_match is declared as a non-static local variable > so the structure members are not auto-initialized. Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH v4 07/14] regulator: max77802: zero-initialize regulator match table
On Mon, Nov 03, 2014 at 03:40:42PM +0100, Javier Martinez Canillas wrote: > The struct of_regulator_match is declared as a non-static local variable > so the structure members are not auto-initialized. Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH v4 08/14] regulator: max8660: zero-initialize regulator match table array
On Mon, Nov 03, 2014 at 03:40:43PM +0100, Javier Martinez Canillas wrote: > The struct of_regulator_match rmatch[] is declared as a non-static local > variable so the structure members are not auto-initialized. Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH v4 09/14] regulator: s2mpa01: zero-initialize regulator match table array
On Mon, Nov 03, 2014 at 03:40:44PM +0100, Javier Martinez Canillas wrote: > The struct of_regulator_match rmatch[] is declared as a non-static local > variable so the structure members are not auto-initialized. Applied, thanks. signature.asc Description: Digital signature
Re: [PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()
On Mon, Nov 03, 2014 at 04:48:34PM +0100, Javier Martinez Canillas wrote:
> On 11/03/2014 04:33 PM, Mark Brown wrote:
> > On Mon, Nov 03, 2014 at 03:40:38PM +0100, Javier Martinez Canillas wrote:
> >>if (!of_node_cmp(np->name, info->desc.name)) {
> >>config->init_data =
> >> - of_get_regulator_init_data(&pdev->dev, np);
> >> + of_get_regulator_init_data(&pdev->dev, np,
> >> + NULL);
> > This looks buggy, we're not passing in a descriptor.
> The descriptor is only used when extracting the init_data to map the
> modes and since it was not a parameter before, some drivers needs to
> be refactored to pass that information.
No, it's only *currently* used for that. If we don't bother passing the
descriptor in then future additions which make use of it (including
adding mode operations to existing drivers) won't work and it might not
be obvious why.
> I thought that instead of adding intrusive changes in drivers that I
> don't have hw to test, that parameter could be optional so the patch
> that use that information check if the descriptor and the map_modes
> function pointer are not NULL.
You're already going through and modifying every single driver and all
of those I looked at already had references to the descriptor in
adjacent code or a global descriptor for the one regulator supported by
the driver.
signature.asc
Description: Digital signature
Re: [PATCH v4 00/14] Add max77802 regulator operating mode support
On 11/03/2014 04:54 PM, Mark Brown wrote: > > No, it's not. This is a a patch series doing a whole bunch of different > things, there's at least bug fixes to existing drivers, new features and > also this new driver in what I've glanced at so far. These things > shouldn't just be being thrown together into a single patch series, and > the patch series shouldn't then be described as just being what is in > the end a minor part of the collection. > Ok, sorry. Next time I'll take care to split fixes from new features and also better explain what the series really does. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/14] Add max77802 regulator operating mode support
On Mon, Nov 03, 2014 at 03:54:43PM +, Mark Brown wrote: > On Mon, Nov 03, 2014 at 03:40:35PM +0100, Javier Martinez Canillas wrote: > > Hello Mark, > > > > This is the fourth version of the series that adds operating modes > > support for the regulators in the max77802 PMIC. This version uses > No, it's not. This is a a patch series doing a whole bunch of different > things, there's at least bug fixes to existing drivers, new features and > also this new driver in what I've glanced at so far. These things s/new driver/new driver support/ sorry Point being that operating mode support in the driver is mostly orthogonal to DT bindings is mostly orthogonal to random bug fixes. signature.asc Description: Digital signature
Re: [PATCH v4 12/14] regulator: max77802: Use unsigned int for modes in max77802_map_mode()
On Mon, Nov 03, 2014 at 03:40:47PM +0100, Javier Martinez Canillas wrote: > All function dealing with operating modes use unsigned int for modes > so change max77802_map_mode() function signature for consistency. Applied, thanks. signature.asc Description: Digital signature
Re: [RFC 1/2] PM / Domains: Power on domain early during system resume
Andrzej Hajda writes:
> On 10/30/2014 08:36 AM, Krzysztof Kozlowski wrote:
>> On śro, 2014-10-29 at 10:46 -0700, Kevin Hilman wrote:
>>> Krzysztof Kozlowski writes:
>>>
When resuming the system the power domain has to be powered on early so
any runtime PM aware devices could resume.
This fixes following scenario reproduced on Exynos DRM:
1. Power domain is off before suspending the system.
2. System is suspended to RAM.
3. Resuming starts. The Exynos DRM driver resume callback is called.
4. The Exynos DRM driver calls drm_helper_resume_force_mode which turns
the screen on by calling exynos_dsi_dpms with DRM_MODE_DPMS_ON.
>>> Dumb Q: if the device (and power domain) were off before (and during)
>>> suspend, why are they being resumed?
>>>
>>> Shouldn't the resume path restore things to the same state they were
>>> before suspend?
>> One could expect that... but the Exynos DRM driver behaves differently
>> (and some other drivers also). In resume method it calls
>> drm_helper_resume_force_mode() which forces restoring mode setting
>> configuration. Apparently setting a mode needs DPMS on:
>> static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
>> {
>> ...
>> exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
>> ...
>>
>> The previous DPMS status (status during suspend) is completely ignored
>> here.
>
> Suspend callback switches off all connectors (thus all other devs in
> their pipeline) by calling dpms_off,
> in restore callback all devs are restored to their previous state by
> calling appropriate dpms.
> So I guess drm_helper_resume_force_mode() call at the end of resume is
> incorrect.
Though I'm not terribly familiar with DRM, it seems incorrect because I
expect resume to restore the state of things when suspend happened, not
forcibly resume everything.
> On the other side it is present in many other drivers, so I am also
> little bit confused.
Many other DRM drivers? or other drivers too?
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 03/14] regulator: of: Add regulator desc param to of_get_regulator_init_data()
On 11/03/2014 04:59 PM, Mark Brown wrote: > No, it's only *currently* used for that. If we don't bother passing the > descriptor in then future additions which make use of it (including > adding mode operations to existing drivers) won't work and it might not > be obvious why. > fair enough. > You're already going through and modifying every single driver and all > of those I looked at already had references to the descriptor in > adjacent code or a global descriptor for the one regulator supported by > the driver. > Ok, I'll make sure that all drivers passes the descriptor. Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/14] mfd/regulator: dt-bindings: max77686: Document of_compatible for regulator
On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote:
> Document new required property for regulator driver: of_compatible. The
> property allows MFD core to bind the driver to node with regulators and
> this simplifies parsing regulators init data from DTS
>
> Signed-off-by: Krzysztof Kozlowski
> ---
> Documentation/devicetree/bindings/mfd/max77686.txt | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
I want a thumbs-up from Mark on this before applying.
> diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt
> b/Documentation/devicetree/bindings/mfd/max77686.txt
> index 75fdfaf41831..f4010a9f66eb 100644
> --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> @@ -18,8 +18,11 @@ Required properties:
>
> Optional node:
> - voltage-regulators : The regulators of max77686 have to be instantiated
> - under subnode named "voltage-regulators" using the following format.
> + under subnode named "voltage-regulators".
> + Required property:
> + - compatible : Must be "maxim,max77686-pmic"
>
> + Optional properties: For each regulator use following format:
> regulator_name {
> regulator-compatible = LDOn/BUCKn
> standard regulator constraints
> @@ -49,6 +52,8 @@ Example:
> reg = <0x09>;
>
> voltage-regulators {
> + compatible = "maxim,max77686-pmic";
> +
> ldo11_reg {
> regulator-compatible = "LDO11";
> regulator-name = "vdd_ldo11";
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pm_runtime_enable() in ->probe()
"Rafael J. Wysocki" writes: > On Saturday, November 01, 2014 02:08:57 AM Rafael J. Wysocki wrote: >> On Saturday, November 01, 2014 01:20:38 AM Rafael J. Wysocki wrote: >> > On Friday, October 31, 2014 10:16:14 AM Ulf Hansson wrote: >> > > On 24 October 2014 18:12, Kevin Hilman wrote: >> >> [cut] >> >> > > >> > > 1) >> > > It's bad practice to use pm_runtime_get_sync() in the ->probe() path, >> > >> > Honestly, I'm no longer amused. >> > >> > > to bring your resources to full power. The consequence would be a >> > > driver that requires CONFIG_PM_RUNTIME to be even functional, which >> > > just isn't acceptable. >> > >> > Sorry, but this is utter nonsense. >> > >> > CONFIG_PM_RUNTIME unset means "no runtime PM at all", so all drivers can >> > expect >> > everything they need to be always on. If that is not the case, then >> > someone is >> > doing runtime PM behind the scenes and therefore cheating. Or in different >> > words, for CONFIG_PM_RUNTIME unset bus types, platforms etc must ensure >> > that >> > everything is on from the drivers' perspective. >> > >> > If that is the case, then calling pm_runtime_get_sync() from ->probe >> > for CONFIG_PM_RUNTIME unset simply doesn't matter. >> > >> > Now, for CONFIG_PM_RUNTIME enabled, if power domains are in use, doing >> > pm_runtime_get_sync() from ->probe is the only way the driver can ensure >> > in a non-racy way that the device will be accessible going forward. >> > >> > Why? Simply because the probing need not happen during system >> > initialization. >> > It very well may take places when the other devices in the same domain have >> > beein in use for quite a while and have been using runtime PM (in which >> > case the domain may go off at any time unless it is explicityly prevented >> > from >> > doing that). >> > >> > One thing that you may be missing is that, for CONFIG_PM_RUNTIME set, >> > runtime PM has to be either enabled or disabled for all devices in one >> > domain (and if it is disabled, then the domain needs to be always on for >> > all practical purposes). Otherwise you can't just make all of them happy >> > at the same time. The documentation doesn't cover this, because it had >> > been >> > written before we even started to consider power domains. >> > >> > > Drivers that behaves well within this context, follows the runtime PM >> > > documentation/recommendation. >> > >> > So please go to Documentation/power/runtime_pm.txt and read it again. >> > Quote: >> > >> > "If the default initial runtime PM status of the device (i.e. 'suspended') >> > reflects the actual state of the device, its bus type's or its driver's >> > ->probe() callback will likely need to wake it up using one of the PM >> > core's >> > helper functions described in Section 4. In that case, pm_runtime_resume() >> > should be used. Of course, for this purpose the device's runtime PM has >> > to be >> > enabled earlier by calling pm_runtime_enable()." >> >> All that said there is a logical error related to calling pm_runtime_enable() >> and its derivatives from ->probe() that I've been overlooking pretty much >> from the start. >> >> Namely, really_probe() sets dev->driver to drv before calling ->probe() >> for either the bus type or the driver itself, so if the driver's probe >> calls pm_runtime_enable(), it will execute the driver's own runtime PM >> resume callback before the driver can check whether or not it wants to handle >> the device in the first place. That doesn't sound quite right to me. >> >> This means we need a different mechanism to ensure that the device will >> be accessible to the driver and/or the bus type in ->probe(). >> >> At one point we had pm_runtime_get_sync() before really_probe() in >> driver_proble_device() IIRC, but people complained about it, so we dropped it >> and put a barrier in there instead, but that's not sufficient. > > So we actually had pm_runtime_get_noresume() before the barrier, but then > we dropped it due to complaints. > >> We need to explicitly ensure that the device won't be powered off while >> ->probe() is in progress *but* we need to avoid calling the driver's runtime >> PM resume callback before ->probe() returns. > > I wonder, then, if we just need to do pm_runtime_get_sync() instead of the > barrier and then pm_runtime_put() instead of pm_request_idle() in > driver_probe_device()? Won't we still have the same problems in the case of ->probe failure that were fixed by removing those calls[1]? IOW, after the driver's ->probe returns failure, it's no longer safe to call the driver's runtime PM callbacks, since they may be accessing resources that no longer exits. Hmm, thinking a little more, maybe this kind of failure is a good time for the driver to use pm_runtime_force_suspend(), then later when the PM core does the _put(), it will see the device aleady suspended and there shouldn't be any problems. So I think I'm OK with this approach, in theory. Kevin [1] commit eed5d215075
Re: [PATCH v3 08/14] mfd: max77686/802: Remove support for board files
On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote:
> The driver is used only on Exynos based boards with DTS support.
> Convert the driver to DTS-only version. This simplifies a little the
> code:
> 1. No dead (unused) entries in platform_data structure.
> 2. More code removed.
> 3. Regulator driver does not depend on allocated memory
>from MFD driver.
> 4. It makes also easier extending the regulator driver.
>
> Add to the max77686 MFD driver dependency on CONFIG_OF because without
> DTS the regulator drivers won't bind.
>
> Signed-off-by: Krzysztof Kozlowski
> Reviewed-by: Javier Martinez Canillas
> ---
> drivers/mfd/Kconfig | 1 +
> drivers/mfd/max77686.c | 23 ---
> include/linux/mfd/max77686-private.h | 1 -
> include/linux/mfd/max77686.h | 28
> 4 files changed, 1 insertion(+), 52 deletions(-)
Applied, thanks.
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index cbdb10918af1..fd9d19ccf8c0 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -401,6 +401,7 @@ config MFD_MAX14577
> config MFD_MAX77686
> bool "Maxim Semiconductor MAX77686/802 PMIC Support"
> depends on I2C=y
> + depends on OF
> select MFD_CORE
> select REGMAP_I2C
> select REGMAP_IRQ
> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index 9e1046bdef90..418b08b6c066 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -211,24 +211,10 @@ static const struct of_device_id
> max77686_pmic_dt_match[] = {
> { },
> };
>
> -static struct max77686_platform_data *max77686_i2c_parse_dt_pdata(struct
> device
> - *dev)
> -{
> - struct max77686_platform_data *pd;
> -
> - pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
> - if (!pd)
> - return NULL;
> -
> - dev->platform_data = pd;
> - return pd;
> -}
> -
> static int max77686_i2c_probe(struct i2c_client *i2c,
> const struct i2c_device_id *id)
> {
> struct max77686_dev *max77686 = NULL;
> - struct max77686_platform_data *pdata = dev_get_platdata(&i2c->dev);
> const struct of_device_id *match;
> unsigned int data;
> int ret = 0;
> @@ -239,14 +225,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
> const struct mfd_cell *cells;
> int n_devs;
>
> - if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node && !pdata)
> - pdata = max77686_i2c_parse_dt_pdata(&i2c->dev);
> -
> - if (!pdata) {
> - dev_err(&i2c->dev, "No platform data found.\n");
> - return -EINVAL;
> - }
> -
> max77686 = devm_kzalloc(&i2c->dev,
> sizeof(struct max77686_dev), GFP_KERNEL);
> if (!max77686)
> @@ -265,7 +243,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
> max77686->dev = &i2c->dev;
> max77686->i2c = i2c;
>
> - max77686->wakeup = pdata->wakeup;
> max77686->irq = i2c->irq;
>
> if (max77686->type == TYPE_MAX77686) {
> diff --git a/include/linux/mfd/max77686-private.h
> b/include/linux/mfd/max77686-private.h
> index 960b92ad450d..f5043490d67c 100644
> --- a/include/linux/mfd/max77686-private.h
> +++ b/include/linux/mfd/max77686-private.h
> @@ -447,7 +447,6 @@ struct max77686_dev {
> struct regmap_irq_chip_data *rtc_irq_data;
>
> int irq;
> - bool wakeup;
> struct mutex irqlock;
> int irq_masks_cur[MAX77686_IRQ_GROUP_NR];
> int irq_masks_cache[MAX77686_IRQ_GROUP_NR];
> diff --git a/include/linux/mfd/max77686.h b/include/linux/mfd/max77686.h
> index 553f7d09258a..bb995ab9a575 100644
> --- a/include/linux/mfd/max77686.h
> +++ b/include/linux/mfd/max77686.h
> @@ -119,12 +119,6 @@ enum max77802_regulators {
> MAX77802_REG_MAX,
> };
>
> -struct max77686_regulator_data {
> - int id;
> - struct regulator_init_data *initdata;
> - struct device_node *of_node;
> -};
> -
> enum max77686_opmode {
> MAX77686_OPMODE_NORMAL,
> MAX77686_OPMODE_LP,
> @@ -136,26 +130,4 @@ struct max77686_opmode_data {
> int mode;
> };
>
> -struct max77686_platform_data {
> - int ono;
> - int wakeup;
> -
> - /* PMIC */
> - struct max77686_regulator_data *regulators;
> - int num_regulators;
> -
> - struct max77686_opmode_data *opmode_data;
> -
> - /*
> - * GPIO-DVS feature is not enabled with the current version of
> - * MAX77686 driver. Buck2/3/4_voltages[0] is used as the default
> - * voltage at probe. DVS/SELB gpios are set as OUTPUT-LOW.
> - */
> - int buck234_gpio_dvs[3]; /* GPIO of [0]DVS1, [1]DVS2, [2]DVS3 */
> - int buck234_gpio_selb[3]; /* [0]SELB2, [1]SELB3, [2]SELB4 */
> - unsigned int buck2_voltage[8]; /* buckx_voltage in uV */
> - unsigned int buck3_voltage[8];
> - unsigned int buck4_voltage[8];
> -};
> -
>
Re: [PATCH] ARM: dts: Add pwm-fan node to the Odroid-U3 board
Hi,
Am 03.11.2014 um 15:43 schrieb Kamil Debski:
> Add pwm-fan node to the Odroid-U3 board file to enable PWM control of the
> cooling fan.
>
> Signed-off-by: Kamil Debski
> ---
> arch/arm/boot/dts/exynos4412-odroidu3.dts | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts
> b/arch/arm/boot/dts/exynos4412-odroidu3.dts
> index c8a64be..764bda4 100644
> --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
> +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
> @@ -31,6 +31,19 @@
> linux,default-trigger = "heartbeat";
> };
> };
> +
> + pwm: pwm@139D {
> + pinctrl-0 = <&pwm0_out>;
> + pinctrl-names = "default";
> + samsung,pwm-outputs = <0>;
> + status = "okay";
> + };
Please add the pwm label in the .dtsi if necessary and then just do &pwm
{...}; before &usb3503 below.
> +
> + pwm-fan {
> + compatible = "pwm-fan";
> + status = "okay";
> + pwms = <&pwm 0 1 0>;
> + };
> };
>
> &usb3503 {
Regards,
Andreas
--
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
Hi,
Cc: Ulf, Kevin, Geert.
On 03/11/14 04:53, Amit Daniel Kachhap wrote:
> These power domain transition notifiers will assist in carrying
> out some activity associated with domain power on/off such as
> some registers which may lose its contents and need save/restore
> across domain power off/on.
Other specific use cases I could add here is gating selected clocks
to ensure proper signal propagation for devices attached to a power
domain, e.g. ungating selected clocks before the power domain on and
restoring them to previous state after the power domain was switched
on.
> 4 type of notifications are added,
> GPD_OFF_PRE - GPD state before power off
> GPD_OFF_POST- GPD state after power off
> GPD_ON_PRE - GPD state before power off
> GPD_ON_POST - GPD state after power off
>
> 3 notfication API's are exported.
> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
> enum gpd_on_off_state state);
>
> In this approach the notifiers are registered/unregistered with pd name.
> The actual invoking of the notfiers will be done by the platform implementing
> power domain enable/disable low level handlers according to the above
> defined notification types. This approach will considerably reduce the
> number of call to notifiers as compared to calling always from core
> powerdomain subsystem.
>
> Also the registered domain's will be managed inside a cache list and not
> part of the genpd structure. This will help in registration of notifiers from
> subsystems (such as clock) even when the PD subsystem is still not
> initialised.
> This list also filters out the unregistered pd's requesting notification.
This patch is somewhat similar my patch adding power domain state change
notifications [1]. I have already a reworked version of that patch, with the
notifier list moved to struct generic_pm_domain and using genpd->lock instead
of dev->power.lock. Anyway, while I'd leave the decision about the location
from where the notifier chains are supposed to be called to the subsystem's
maintainers preference, I'm rather reluctant to having one global notifiers
list for all possible power domains and all the notification clients.
The possibly long list needs to be traversed at each notifier call and it
seems in your implementation any registered user of the notification gets
notifications for all possible power domains.
[1] https://lkml.org/lkml/2014/8/5/182
> Cc: Rafael J. Wysocki
> Reviewed-by: Pankaj Dubey
> Signed-off-by: Amit Daniel Kachhap
> ---
> drivers/base/power/domain.c | 112
> ++-
> include/linux/pm_domain.h | 31
> 2 files changed, 142 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 40bc2f4..e05045e 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -46,10 +46,19 @@
> __retval;
> \
> })
>
> +struct cache_notify_domains {
> + char *name;
> + /* Node in the cache pm domain name list */
> + struct list_head cache_list_node;
> +};
> +
> static LIST_HEAD(gpd_list);
> static DEFINE_MUTEX(gpd_list_lock);
> +static LIST_HEAD(domain_notify_list);
> +static DEFINE_MUTEX(domain_notify_list_lock);
> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>
> -static struct generic_pm_domain *pm_genpd_lookup_name(const char
> *domain_name)
> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
> {
> struct generic_pm_domain *genpd = NULL, *gpd;
>
> @@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const
> char *domain_name)
> mutex_unlock(&gpd_list_lock);
> return genpd;
> }
> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>
> struct generic_pm_domain *dev_to_genpd(struct device *dev)
> {
> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
> mutex_unlock(&gpd_list_lock);
> }
>
> +/**
> + * genpd_register_notifier - Register a PM domain for future notification.
> + * @nb: notification block containing notification handle.
> + * @pd_name: PM domain name.
> + */
> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
> +{
> + int ret;
> + struct cache_notify_domains *entry;
> +
> + if (!pd_name)
> + return -EINVAL;
> +
> + /* Search if the pd is already registered */
> + mutex_lock(&domain_notify_list_lock);
> + list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
> + if (!strcmp(entry->name, pd_name))
> + break;
> + }
> + mutex_unlock(&domain_notify_list_lock);
> +
> + if (entry) {
> + pr_err("%s: pd already exists\n", __func__);
>
Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
+Mike Turquette Hi Amit, "Rafael J. Wysocki" writes: > CC: Kevin, Ulf, Geert. > > On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote: >> These power domain transition notifiers will assist in carrying >> out some activity associated with domain power on/off such as >> some registers which may lose its contents and need save/restore >> across domain power off/on. The runtime PM framework already provides callbacks that are useful for context save/restore for devices. Could you please describe in more detail which registers in which kind of devices need to be saved/restored, and why they cannot be saved/restored using existing mechanisms. Personally, I'm uncomfortable with notifiers like this because it suggests that underlying frameworks are not doing the right thing, or are not being used. (I also don't like the implementation here where a single global notifier list is maintained by the core, but the notifiers are actually triggered by SoC specific code.) IIUC, the usage in this series seems to be that certain clock related registers need to be saved/restored across a power domain transition. Wouldn't an alternative solution be to add a feature to the clock driver such that the state of each clock is saved when the clock is disabled, and restored when the clock is enabled? That would allow any clock context to survive any power domain transtion also, correct? I have some issues with the implementaion as well, but I think we need to first sort out the real need for this before going into those details. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
Now really filling in the CC list, apologies for duplicate
post.
On 03/11/14 19:21, Sylwester Nawrocki wrote:
> Hi,
>
> Cc: Ulf, Kevin, Geert.
>
> On 03/11/14 04:53, Amit Daniel Kachhap wrote:
>> These power domain transition notifiers will assist in carrying
>> out some activity associated with domain power on/off such as
>> some registers which may lose its contents and need save/restore
>> across domain power off/on.
>
> Other specific use cases I could add here is gating selected clocks
> to ensure proper signal propagation for devices attached to a power
> domain, e.g. ungating selected clocks before the power domain on and
> restoring them to previous state after the power domain was switched
> on.
>
>> 4 type of notifications are added,
>> GPD_OFF_PRE - GPD state before power off
>> GPD_OFF_POST- GPD state after power off
>> GPD_ON_PRE - GPD state before power off
>> GPD_ON_POST - GPD state after power off
>>
>> 3 notfication API's are exported.
>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> enum gpd_on_off_state state);
>>
>> In this approach the notifiers are registered/unregistered with pd name.
>> The actual invoking of the notfiers will be done by the platform implementing
>> power domain enable/disable low level handlers according to the above
>> defined notification types. This approach will considerably reduce the
>> number of call to notifiers as compared to calling always from core
>> powerdomain subsystem.
>>
>> Also the registered domain's will be managed inside a cache list and not
>> part of the genpd structure. This will help in registration of notifiers from
>> subsystems (such as clock) even when the PD subsystem is still not
>> initialised.
>> This list also filters out the unregistered pd's requesting notification.
>
> This patch is somewhat similar my patch adding power domain state change
> notifications [1]. I have already a reworked version of that patch, with the
> notifier list moved to struct generic_pm_domain and using genpd->lock instead
> of dev->power.lock. Anyway, while I'd leave the decision about the location
> from where the notifier chains are supposed to be called to the subsystem's
> maintainers preference, I'm rather reluctant to having one global notifiers
> list for all possible power domains and all the notification clients.
> The possibly long list needs to be traversed at each notifier call and it
> seems in your implementation any registered user of the notification gets
> notifications for all possible power domains.
>
> [1] https://lkml.org/lkml/2014/8/5/182
>
>> Cc: Rafael J. Wysocki
>> Reviewed-by: Pankaj Dubey
>> Signed-off-by: Amit Daniel Kachhap
>> ---
>> drivers/base/power/domain.c | 112
>> ++-
>> include/linux/pm_domain.h | 31
>> 2 files changed, 142 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 40bc2f4..e05045e 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -46,10 +46,19 @@
>> __retval;
>> \
>> })
>>
>> +struct cache_notify_domains {
>> +char *name;
>> +/* Node in the cache pm domain name list */
>> +struct list_head cache_list_node;
>> +};
>> +
>> static LIST_HEAD(gpd_list);
>> static DEFINE_MUTEX(gpd_list_lock);
>> +static LIST_HEAD(domain_notify_list);
>> +static DEFINE_MUTEX(domain_notify_list_lock);
>> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>>
>> -static struct generic_pm_domain *pm_genpd_lookup_name(const char
>> *domain_name)
>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>> {
>> struct generic_pm_domain *genpd = NULL, *gpd;
>>
>> @@ -66,6 +75,7 @@ static struct generic_pm_domain
>> *pm_genpd_lookup_name(const char *domain_name)
>> mutex_unlock(&gpd_list_lock);
>> return genpd;
>> }
>> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>>
>> struct generic_pm_domain *dev_to_genpd(struct device *dev)
>> {
>> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>> mutex_unlock(&gpd_list_lock);
>> }
>>
>> +/**
>> + * genpd_register_notifier - Register a PM domain for future notification.
>> + * @nb: notification block containing notification handle.
>> + * @pd_name: PM domain name.
>> + */
>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>> +{
>> +int ret;
>> +struct cache_notify_domains *entry;
>> +
>> +if (!pd_name)
>> +return -EINVAL;
>> +
>> +/* Search if the pd is already registered */
>> +mutex_lock(&domain_notify_list_lock);
>> +list_for_each_entry(entry, &domain_notify_list, cache_list
Re: [PATCH v3] usb: Remove references to non-existent PLAT_S5P symbol
On Tue, Oct 07, 2014 at 11:12:07AM +0200, Sylwester Nawrocki wrote:
> The PLAT_S5P Kconfig symbol was removed in commit d78c16ccde96
> ("ARM: SAMSUNG: Remove remaining legacy code"). There are still
> some references left, fix that by replacing them with ARCH_S5PV210.
>
> Fixes: d78c16ccde96 ("ARM: SAMSUNG: Remove remaining legacy code")
> Reported-by: Paul Bolle
> Acked-by: Jingoo Han
> Cc: [3.17+]
This isn't a stable issue...
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pm_runtime_enable() in ->probe()
On Monday, November 03, 2014 09:00:29 AM Kevin Hilman wrote: > "Rafael J. Wysocki" writes: > > > On Saturday, November 01, 2014 02:08:57 AM Rafael J. Wysocki wrote: > >> On Saturday, November 01, 2014 01:20:38 AM Rafael J. Wysocki wrote: > >> > On Friday, October 31, 2014 10:16:14 AM Ulf Hansson wrote: > >> > > On 24 October 2014 18:12, Kevin Hilman wrote: > >> > >> [cut] > >> > >> > > > >> > > 1) > >> > > It's bad practice to use pm_runtime_get_sync() in the ->probe() path, > >> > > >> > Honestly, I'm no longer amused. > >> > > >> > > to bring your resources to full power. The consequence would be a > >> > > driver that requires CONFIG_PM_RUNTIME to be even functional, which > >> > > just isn't acceptable. > >> > > >> > Sorry, but this is utter nonsense. > >> > > >> > CONFIG_PM_RUNTIME unset means "no runtime PM at all", so all drivers can > >> > expect > >> > everything they need to be always on. If that is not the case, then > >> > someone is > >> > doing runtime PM behind the scenes and therefore cheating. Or in > >> > different > >> > words, for CONFIG_PM_RUNTIME unset bus types, platforms etc must ensure > >> > that > >> > everything is on from the drivers' perspective. > >> > > >> > If that is the case, then calling pm_runtime_get_sync() from ->probe > >> > for CONFIG_PM_RUNTIME unset simply doesn't matter. > >> > > >> > Now, for CONFIG_PM_RUNTIME enabled, if power domains are in use, doing > >> > pm_runtime_get_sync() from ->probe is the only way the driver can ensure > >> > in a non-racy way that the device will be accessible going forward. > >> > > >> > Why? Simply because the probing need not happen during system > >> > initialization. > >> > It very well may take places when the other devices in the same domain > >> > have > >> > beein in use for quite a while and have been using runtime PM (in which > >> > case the domain may go off at any time unless it is explicityly > >> > prevented from > >> > doing that). > >> > > >> > One thing that you may be missing is that, for CONFIG_PM_RUNTIME set, > >> > runtime PM has to be either enabled or disabled for all devices in one > >> > domain (and if it is disabled, then the domain needs to be always on for > >> > all practical purposes). Otherwise you can't just make all of them happy > >> > at the same time. The documentation doesn't cover this, because it had > >> > been > >> > written before we even started to consider power domains. > >> > > >> > > Drivers that behaves well within this context, follows the runtime PM > >> > > documentation/recommendation. > >> > > >> > So please go to Documentation/power/runtime_pm.txt and read it again. > >> > Quote: > >> > > >> > "If the default initial runtime PM status of the device (i.e. > >> > 'suspended') > >> > reflects the actual state of the device, its bus type's or its driver's > >> > ->probe() callback will likely need to wake it up using one of the PM > >> > core's > >> > helper functions described in Section 4. In that case, > >> > pm_runtime_resume() > >> > should be used. Of course, for this purpose the device's runtime PM has > >> > to be > >> > enabled earlier by calling pm_runtime_enable()." > >> > >> All that said there is a logical error related to calling > >> pm_runtime_enable() > >> and its derivatives from ->probe() that I've been overlooking pretty much > >> from the start. > >> > >> Namely, really_probe() sets dev->driver to drv before calling ->probe() > >> for either the bus type or the driver itself, so if the driver's probe > >> calls pm_runtime_enable(), it will execute the driver's own runtime PM > >> resume callback before the driver can check whether or not it wants to > >> handle > >> the device in the first place. That doesn't sound quite right to me. > >> > >> This means we need a different mechanism to ensure that the device will > >> be accessible to the driver and/or the bus type in ->probe(). > >> > >> At one point we had pm_runtime_get_sync() before really_probe() in > >> driver_proble_device() IIRC, but people complained about it, so we dropped > >> it > >> and put a barrier in there instead, but that's not sufficient. > > > > So we actually had pm_runtime_get_noresume() before the barrier, but then > > we dropped it due to complaints. > > > >> We need to explicitly ensure that the device won't be powered off while > >> ->probe() is in progress *but* we need to avoid calling the driver's > >> runtime > >> PM resume callback before ->probe() returns. > > > > I wonder, then, if we just need to do pm_runtime_get_sync() instead of the > > barrier and then pm_runtime_put() instead of pm_request_idle() in > > driver_probe_device()? > > Won't we still have the same problems in the case of ->probe failure > that were fixed by removing those calls[1]? Well, it looks like that might not be the best way to fix the problem. First of all, I think that drivers can reasonably expect devices to be accessible when ->probe() runs. This me
Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
On Monday, November 03, 2014 03:03:46 PM Ulf Hansson wrote: > On 1 November 2014 01:20, Rafael J. Wysocki wrote: > > On Friday, October 31, 2014 10:16:14 AM Ulf Hansson wrote: > >> On 24 October 2014 18:12, Kevin Hilman wrote: > >> > Ulf Hansson writes: > >> > > >> >> Changes in v3: > >> >> -Rework the entire intermediate step which was suggested in v2. > >> >>That means solving the race condition, but also cope with PM > >> >> domains > >> >>that are initialized in powered off state. > >> >> > >> >> Changes in v2: > >> >> -Added some acks. > >> >> -Updated commit messages. > >> >> -Included a wider audience of the patchset. V1 was lacking SoC > >> >>maintainers. > >> >> > >> >> Here are link to the first patchset, were some discussion started. > >> >> http://marc.info/?l=linux-pm&m=141208104729597&w=2 > >> >> > >> >> There may be more than one device in a PM domain which then will be > >> >> probed at different points in time. > >> >> > >> >> Depending on timing and runtime PM support, in for the device related > >> >> driver/subsystem, a PM domain may be advised to power off after a > >> >> successful probe sequence. > >> >> > >> >> A general requirement for a device within a PM domain, is that the > >> >> PM domain must stay powered during the probe sequence. To cope with > >> >> such requirement, let's add two new APIs, dev_pm_domain_get|put(). > >> >> > >> >> These APIs are intended to be invoked from subsystem-level code and the > >> >> calls between get/put needs to be balanced. > >> >> > >> >> dev_pm_domain_get(), tells the PM domain that it needs to increase a > >> >> usage count and to keep supplying power. dev_pm_domain_put(), does the > >> >> opposite. > >> > > >> > I'm confused. Why arent' pm_runtime_get*() and pm_runtime_put*() working? > >> > >> See, below. > >> > >> > > >> > What's not explained here (or what I'm not understanding) is why a PM > >> > domain is powering off if it has active devices. > >> > >> It doesn't. The problem is that using pm_runtime_get_sync() in this > >> path is not working. > >> > >> Now, I failed to include some of the important information from > >> previous discussions around this patchset. Let me iterate the patchset > >> with better commit messages, but let's first continue this thread. > >> > >> Here are some of the previous discussion: > >> > >> http://marc.info/?l=linux-pm&m=141270897014653&w=2 > >> http://marc.info/?l=linux-pm&m=141208104729597&w=2 > >> > >> Below is a summary of why I think "pm_runtime_get_sync()" isn't working > >> for us. > >> > >> 1) > >> It's bad practice to use pm_runtime_get_sync() in the ->probe() path, > > > > Honestly, I'm no longer amused. > > > >> to bring your resources to full power. The consequence would be a > >> driver that requires CONFIG_PM_RUNTIME to be even functional, which > >> just isn't acceptable. > > > > Sorry, but this is utter nonsense. > > I admit, I was too vague while stating this. Looking at the big > picture you are obviously right. > > I should have referred to those SOCs/buses/drivers that I am working/looking > at. > > > > > CONFIG_PM_RUNTIME unset means "no runtime PM at all", so all drivers can > > expect > > everything they need to be always on. If that is not the case, then > > someone is > > doing runtime PM behind the scenes and therefore cheating. Or in different > > words, for CONFIG_PM_RUNTIME unset bus types, platforms etc must ensure that > > everything is on from the drivers' perspective. > > I don't think I have stated anything that isn't in agreement with the above? > > While I am struggling in making my points clearer, it seems like we > look a bit differently upon how runtime PM are being deployed. > > For those drivers I am working on, it's common that these handles > runtime PM resources, like for example clocks. The clocks needs to be > enabled for the driver to handle I/O, but those may also be gated at > request inactivity to save power. That means, the clocks may be > considered as both functional clocks and runtime PM resources. > > Therefore, the driver must enable its clocks during ->probe() and > without relying on CONFIG_PM_RUNTIME to be set. Similar to what you > stated for the buses above. > > To also cope with the scenario where CONFIG_PM_RUNTIME is set, drivers > must update the device's runtime PM status using > pm_runtime_set_active(), to synchronize the state with the runtime PM > core. Otherwise we will get clock unbalance issues while > gating/ungating the clocks from the runtime PM callbacks. Generally, there are two or even three levels of runtime PM handling, driver, (possibly) bus type and (possibly) PM domain (and multiple levels of these are possible in principle). All of them have to be initialized at different times. Quite arguably, the PM domain and/or bus type runtime PM handling should be initialized even before registerind the device or during device registration. Doing that later may b
RE: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver
Hi Lee, On Monday, November 03, 2014 8:56 PM, Lee Jones wrote, > Subject: Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver > > On Mon, 03 Nov 2014, Amit Daniel Kachhap wrote: > > > From: Pankaj Dubey > > > > This patch moves Exynos PMU driver implementation from "arm/mach-exynos" > > to "drivers/mfd". This driver is mainly used for setting misc bits of > > register from PMU IP of Exynos SoC which will be required to configure > > before Suspend/Resume. Currently all these settings are done in > > "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC > > support, there is a need of DT based implementation of PMU driver. > > > > This driver uses already existing DT binding information. > > > > CC: Sangbeom Kim > > CC: Samuel Ortiz > > CC: Lee Jones > > Signed-off-by: Pankaj Dubey > > Signed-off-by: Amit Daniel Kachhap > > --- > > arch/arm/mach-exynos/Kconfig |1 + > > arch/arm/mach-exynos/Makefile |2 +- > > drivers/mfd/Kconfig|9 + > > drivers/mfd/Makefile |1 + > > .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c |0 > > 5 files changed, 12 insertions(+), 1 deletion(-) rename > > arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%) > > I thought we spoke about this already [1]? What's changed? > > [1] https://lkml.org/lkml/2014/4/29/193 > Yes correct, we had discussion about this in above post. But at that time I was not having enough points to put forward for keeping Exynos PMU driver in MFD folder. At that point there was only one idea behind keeping Exynos PMU that in future it will be reused for ARM64 based SoC for PM related functionalities. But in this series we have implemented Exynos Power Domain for Samsung's ARM64 based SoCs. As far as I can see this will in following ways - 1: Moving out PM domain specific code from mach-exynos and reuse of same code for ARM64 based SoC also. 2: Since PM domain relies on PMU registers and does not have its own DT binding, MFD client and MFD device is most suitable for making this kind of platform drivers. > -- > Lee Jones > Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software > for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
Hi,
On Tuesday, November 04, 2014 12:11 AM, Sylwester Nawrocki wrote,
> To: linux-arm-ker...@lists.infradead.org
> Cc: Amit Daniel Kachhap; linux-samsung-soc@vger.kernel.org; linux-
> p...@vger.kernel.org; kgene@samsung.com; pankaj.du...@samsung.com;
Rafael
> J. Wysocki; Beata Michalska; ge...@linux-m68k.org; Kevin Hilman; Ulf
Hansson
> Subject: Re: [PATCH 03/12] PM / Domains: Add notifier support for power
domain
> transitions
>
>
> Now really filling in the CC list, apologies for duplicate post.
>
> On 03/11/14 19:21, Sylwester Nawrocki wrote:
> > Hi,
> >
> > Cc: Ulf, Kevin, Geert.
> >
> > On 03/11/14 04:53, Amit Daniel Kachhap wrote:
> >> These power domain transition notifiers will assist in carrying out
> >> some activity associated with domain power on/off such as some
> >> registers which may lose its contents and need save/restore across
> >> domain power off/on.
> >
> > Other specific use cases I could add here is gating selected clocks to
> > ensure proper signal propagation for devices attached to a power
> > domain, e.g. ungating selected clocks before the power domain on and
> > restoring them to previous state after the power domain was switched
> > on.
> >
> >> 4 type of notifications are added,
> >> GPD_OFF_PRE - GPD state before power off
> >> GPD_OFF_POST- GPD state after power off
> >> GPD_ON_PRE - GPD state before power off
> >> GPD_ON_POST - GPD state after power off
> >>
> >> 3 notfication API's are exported.
> >> 1) int genpd_register_notifier(struct notifier_block *nb, char
> >> *pd_name);
> >> 2) int genpd_unregister_notifier(struct notifier_block *nb, char
> >> *pd_name);
> >> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain
*genpd,
> >>enum gpd_on_off_state state);
> >>
> >> In this approach the notifiers are registered/unregistered with pd
name.
> >> The actual invoking of the notfiers will be done by the platform
> >> implementing power domain enable/disable low level handlers according
> >> to the above defined notification types. This approach will
> >> considerably reduce the number of call to notifiers as compared to
> >> calling always from core powerdomain subsystem.
> >>
> >> Also the registered domain's will be managed inside a cache list and
> >> not part of the genpd structure. This will help in registration of
> >> notifiers from subsystems (such as clock) even when the PD subsystem is
still not
> initialised.
> >> This list also filters out the unregistered pd's requesting
notification.
> >
> > This patch is somewhat similar my patch adding power domain state
> > change notifications [1]. I have already a reworked version of that
> > patch, with the notifier list moved to struct generic_pm_domain and
> > using genpd->lock instead of dev->power.lock. Anyway, while I'd leave
> > the decision about the location from where the notifier chains are
> > supposed to be called to the subsystem's maintainers preference, I'm
> > rather reluctant to having one global notifiers list for all possible
power domains
> and all the notification clients.
> > The possibly long list needs to be traversed at each notifier call and
> > it seems in your implementation any registered user of the
> > notification gets notifications for all possible power domains.
> >
> > [1] https://lkml.org/lkml/2014/8/5/182
> >
> >> Cc: Rafael J. Wysocki
> >> Reviewed-by: Pankaj Dubey
> >> Signed-off-by: Amit Daniel Kachhap
> >> ---
> >> drivers/base/power/domain.c | 112
> ++-
> >> include/linux/pm_domain.h | 31
> >> 2 files changed, 142 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/base/power/domain.c
> >> b/drivers/base/power/domain.c index 40bc2f4..e05045e 100644
> >> --- a/drivers/base/power/domain.c
> >> +++ b/drivers/base/power/domain.c
> >> @@ -46,10 +46,19 @@
> >>__retval;
\
> >> })
> >>
> >> +struct cache_notify_domains {
> >> + char *name;
> >> + /* Node in the cache pm domain name list */
> >> + struct list_head cache_list_node;
> >> +};
> >> +
> >> static LIST_HEAD(gpd_list);
> >> static DEFINE_MUTEX(gpd_list_lock);
> >> +static LIST_HEAD(domain_notify_list); static
> >> +DEFINE_MUTEX(domain_notify_list_lock);
> >> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
> >>
> >> -static struct generic_pm_domain *pm_genpd_lookup_name(const char
> >> *domain_name)
> >> +struct generic_pm_domain *pm_genpd_lookup_name(const char
> >> +*domain_name)
> >> {
> >>struct generic_pm_domain *genpd = NULL, *gpd;
> >>
> >> @@ -66,6 +75,7 @@ static struct generic_pm_domain
> *pm_genpd_lookup_name(const char *domain_name)
> >>mutex_unlock(&gpd_list_lock);
> >>return genpd;
> >> }
> >> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
> >>
> >> struct generic_pm_domain *dev_to_genpd(struct device *dev) { @@
> >> -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain
*genpd,
> >>mutex_unlock(&gpd_list_lock);
> >> }
> >>
> >> +/**
Re: [GIT PULL 1/3] 1st Round of Samsung cleanup for v3.19
On Tue, Oct 21, 2014 at 12:54:14AM +0900, Kukjin Kim wrote: > Hi, > > Here is 1st round of Samsung updates for v3.19 and actually it has > been sent out to arm-soc for v3.18 before but as you know missed. > I've just rebased the branch on top of v3.18-rc1, please pull. > > Thanks, > Kukjin > > > > The following changes since commit f114040e3ea6e07372334ade75d1ee0775c355e1: > > Linux 3.18-rc1 (2014-10-19 18:08:38 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > tags/samsung-cleanup > > for you to fetch changes up to 13cfa6c4f7facfc690ba9e99ec382c151fddaced: > > ARM: EXYNOS: Fix CPU idle clock down after CPU off (2014-10-21 > 00:06:22 +0900) Merged. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL 3/3] 1st Round of Samsung DT updates for v3.19
On Tue, Oct 21, 2014 at 12:54:41AM +0900, Kukjin Kim wrote: > Hi, > > Here is Samsung DT updates for v3.19, please pull. > > Thanks, > Kukjin > > --- > > The following changes since commit f114040e3ea6e07372334ade75d1ee0775c355e1: > > Linux 3.18-rc1 (2014-10-19 18:08:38 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git > tags/samsung-dt > > for you to fetch changes up to e540920cf21c2764047b8a6ddaeaa0683624e46e: > > ARM: dts: add CPU nodes for Exynos4 SoCs (2014-10-21 00:12:42 +0900) Merged, thanks. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
On Mon, Nov 3, 2014 at 11:51 PM, Sylwester Nawrocki
wrote:
> Hi,
>
> Cc: Ulf, Kevin, Geert.
>
> On 03/11/14 04:53, Amit Daniel Kachhap wrote:
>> These power domain transition notifiers will assist in carrying
>> out some activity associated with domain power on/off such as
>> some registers which may lose its contents and need save/restore
>> across domain power off/on.
>
> Other specific use cases I could add here is gating selected clocks
> to ensure proper signal propagation for devices attached to a power
> domain, e.g. ungating selected clocks before the power domain on and
> restoring them to previous state after the power domain was switched
> on.
yes correct.
>
>> 4 type of notifications are added,
>> GPD_OFF_PRE - GPD state before power off
>> GPD_OFF_POST- GPD state after power off
>> GPD_ON_PRE - GPD state before power off
>> GPD_ON_POST - GPD state after power off
>>
>> 3 notfication API's are exported.
>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>> enum gpd_on_off_state state);
>>
>> In this approach the notifiers are registered/unregistered with pd name.
>> The actual invoking of the notfiers will be done by the platform implementing
>> power domain enable/disable low level handlers according to the above
>> defined notification types. This approach will considerably reduce the
>> number of call to notifiers as compared to calling always from core
>> powerdomain subsystem.
>>
>> Also the registered domain's will be managed inside a cache list and not
>> part of the genpd structure. This will help in registration of notifiers from
>> subsystems (such as clock) even when the PD subsystem is still not
>> initialised.
>> This list also filters out the unregistered pd's requesting notification.
>
> This patch is somewhat similar my patch adding power domain state change
> notifications [1]. I have already a reworked version of that patch, with the
> notifier list moved to struct generic_pm_domain and using genpd->lock instead
Yes this will be correct as others also suggested to make per genpd
notifier block.
> of dev->power.lock. Anyway, while I'd leave the decision about the location
> from where the notifier chains are supposed to be called to the subsystem's
> maintainers preference, I'm rather reluctant to having one global notifiers
> list for all possible power domains and all the notification clients.
> The possibly long list needs to be traversed at each notifier call and it
> seems in your implementation any registered user of the notification gets
> notifications for all possible power domains.
>
> [1] https://lkml.org/lkml/2014/8/5/182
My fault, I somehow missed this link earlier. After going through
this, I found it registers genpd from the platform driver, so the
function signature is
int pm_genpd_register_notifier(struct device *dev, struct notifier_block *nb);
I suggest to make the function signature to be like,
int pm_genpd_register_notifier(struct device_node *np, struct
notifier_block *nb)
In this way this function should should be able to support both
platform devices and non platform devices like clk.
The function may work like,
pdev = of_find_device_by_node(np);
if (pdev) {
// get genpd from device and go ahead with notfier registration.
// blocking_notifier_chain_register(genpd->pd_notifier, nb)
} else
{
// get pd_handle from np
// get the pd_name from phandle and try registering this gen pd
// if the genpd subsystem is not initialised then add this in
a temporary list and register the notifier later
}
Can you post your implementation with 1st part ? Later I can post the
else part with my changes.
>
>> Cc: Rafael J. Wysocki
>> Reviewed-by: Pankaj Dubey
>> Signed-off-by: Amit Daniel Kachhap
>> ---
>> drivers/base/power/domain.c | 112
>> ++-
>> include/linux/pm_domain.h | 31
>> 2 files changed, 142 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 40bc2f4..e05045e 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -46,10 +46,19 @@
>> __retval;
>> \
>> })
>>
>> +struct cache_notify_domains {
>> + char *name;
>> + /* Node in the cache pm domain name list */
>> + struct list_head cache_list_node;
>> +};
>> +
>> static LIST_HEAD(gpd_list);
>> static DEFINE_MUTEX(gpd_list_lock);
>> +static LIST_HEAD(domain_notify_list);
>> +static DEFINE_MUTEX(domain_notify_list_lock);
>> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>>
>> -static struct generic_pm_domain *pm_genpd_lookup_name(const char
>> *domain_name)
>> +struct generic_pm_domain *pm_genpd
Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
On Mon, Nov 3, 2014 at 8:22 PM, Ulf Hansson wrote:
> On 3 November 2014 15:54, Rafael J. Wysocki wrote:
>> CC: Kevin, Ulf, Geert.
>>
>> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:
>>> These power domain transition notifiers will assist in carrying
>>> out some activity associated with domain power on/off such as
>>> some registers which may lose its contents and need save/restore
>>> across domain power off/on.
>
> Hi Amit,
>
> This mechanism seems to be needed - somehow.
> A similar approach has been posted earlier, I am not sure that
> discussion was fully settled though.
>
> http://marc.info/?l=linux-pm&m=136448756308203&w=2
Missed looking into it. Thanks for the pointer.
>
> Anyway, I plan to review this shortly.
Sure.
>
> Kind regards
> Uffe
>
>>>
>>> 4 type of notifications are added,
>>> GPD_OFF_PRE - GPD state before power off
>>> GPD_OFF_POST- GPD state after power off
>>> GPD_ON_PRE - GPD state before power off
>>> GPD_ON_POST - GPD state after power off
>>>
>>> 3 notfication API's are exported.
>>> 1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
>>> 2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
>>> 3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
>>> enum gpd_on_off_state state);
>>>
>>> In this approach the notifiers are registered/unregistered with pd name.
>>> The actual invoking of the notfiers will be done by the platform
>>> implementing
>>> power domain enable/disable low level handlers according to the above
>>> defined notification types. This approach will considerably reduce the
>>> number of call to notifiers as compared to calling always from core
>>> powerdomain subsystem.
>>>
>>> Also the registered domain's will be managed inside a cache list and not
>>> part of the genpd structure. This will help in registration of notifiers
>>> from
>>> subsystems (such as clock) even when the PD subsystem is still not
>>> initialised.
>>> This list also filters out the unregistered pd's requesting notification.
>>>
>>> Cc: Rafael J. Wysocki
>>> Reviewed-by: Pankaj Dubey
>>> Signed-off-by: Amit Daniel Kachhap
>>> ---
>>> drivers/base/power/domain.c | 112
>>> ++-
>>> include/linux/pm_domain.h | 31
>>> 2 files changed, 142 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>>> index 40bc2f4..e05045e 100644
>>> --- a/drivers/base/power/domain.c
>>> +++ b/drivers/base/power/domain.c
>>> @@ -46,10 +46,19 @@
>>> __retval;
>>> \
>>> })
>>>
>>> +struct cache_notify_domains {
>>> + char *name;
>>> + /* Node in the cache pm domain name list */
>>> + struct list_head cache_list_node;
>>> +};
>>> +
>>> static LIST_HEAD(gpd_list);
>>> static DEFINE_MUTEX(gpd_list_lock);
>>> +static LIST_HEAD(domain_notify_list);
>>> +static DEFINE_MUTEX(domain_notify_list_lock);
>>> +static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
>>>
>>> -static struct generic_pm_domain *pm_genpd_lookup_name(const char
>>> *domain_name)
>>> +struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
>>> {
>>> struct generic_pm_domain *genpd = NULL, *gpd;
>>>
>>> @@ -66,6 +75,7 @@ static struct generic_pm_domain
>>> *pm_genpd_lookup_name(const char *domain_name)
>>> mutex_unlock(&gpd_list_lock);
>>> return genpd;
>>> }
>>> +EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
>>>
>>> struct generic_pm_domain *dev_to_genpd(struct device *dev)
>>> {
>>> @@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
>>> mutex_unlock(&gpd_list_lock);
>>> }
>>>
>>> +/**
>>> + * genpd_register_notifier - Register a PM domain for future notification.
>>> + * @nb: notification block containing notification handle.
>>> + * @pd_name: PM domain name.
>>> + */
>>> +int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
>>> +{
>>> + int ret;
>>> + struct cache_notify_domains *entry;
>>> +
>>> + if (!pd_name)
>>> + return -EINVAL;
>>> +
>>> + /* Search if the pd is already registered */
>>> + mutex_lock(&domain_notify_list_lock);
>>> + list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
>>> + if (!strcmp(entry->name, pd_name))
>>> + break;
>>> + }
>>> + mutex_unlock(&domain_notify_list_lock);
>>> +
>>> + if (entry) {
>>> + pr_err("%s: pd already exists\n", __func__);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
>>> + if (!entry)
>>> + return -ENOMEM;
>>> +
>>> + entry->name = pd_name;
>>> +
>>> + mutex_lock(&domain_notify_list_lock);
>>> + list_add(&entry->cache_list_node, &domain_notify_list);
>>> + mutex_unlock(&domain_notify_list_lo
Re: [PATCH 03/12] PM / Domains: Add notifier support for power domain transitions
On Mon, Nov 3, 2014 at 11:53 PM, Kevin Hilman wrote: > +Mike Turquette > > Hi Amit, > > "Rafael J. Wysocki" writes: > >> CC: Kevin, Ulf, Geert. >> >> On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote: >>> These power domain transition notifiers will assist in carrying >>> out some activity associated with domain power on/off such as >>> some registers which may lose its contents and need save/restore >>> across domain power off/on. > > The runtime PM framework already provides callbacks that are useful for > context save/restore for devices. Could you please describe in more > detail which registers in which kind of devices need to be > saved/restored, and why they cannot be saved/restored using existing > mechanisms. Basically the requirement is mandated by exynos7 manual. It tells that before turning off the power domain, some clock registers need to saved and should be restored just after turning the power domain. These clock registers are not necessarily gate clocks but could be mux clocks etc. The driver may not have all information of these clocks also. I suppose these are Soc specific changes but drivers should work across Socs. This behavior is almost similar to suspend/resume case where a whole list of clock registers are saved/restored. Even earlier post by Sylwester (https://lkml.org/lkml/2014/8/5/182) also points to the need of this feature. > > Personally, I'm uncomfortable with notifiers like this because it > suggests that underlying frameworks are not doing the right thing, or > are not being used. (I also don't like the implementation here where a > single global notifier list is maintained by the core, but the notifiers > are actually triggered by SoC specific code.) Yes right the global notifier block can be moved to per genpd structure. Also SoC trigger can be moved to core files. > > IIUC, the usage in this series seems to be that certain clock related > registers need to be saved/restored across a power domain transition. > > Wouldn't an alternative solution be to add a feature to the clock driver > such that the state of each clock is saved when the clock is disabled, > and restored when the clock is enabled? That would allow any clock > context to survive any power domain transtion also, correct? I also thought about same. But the trigger point for this would be driver calling clk disable/enable and not the power domain. so this will lead to lot of save/restore for each power domain child. > > I have some issues with the implementaion as well, but I think we need > to first sort out the real need for this before going into those > details. right :( may be I should have posted this as RFC Thanks for the review. Regards, Amit D > > Kevin > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"asix: Don't reset PHY on if_up for ASIX 88772" breaks net on arndale platform
Hi, With 3.18-rc3, asix on arndale (samsung exynos 5250 based board), fails to work. Interface is initialized but network traffic seem not to pass through. With kernel IP config the result looks like: [3.323275] usb 3-3.2.4: new high-speed USB device number 4 using exynos-ehci [3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95, idProduct=772a [3.424735] usb 3-3.2.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [3.432196] usb 3-3.2.4: Product: AX88772 [3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp. [3.441486] usb 3-3.2.4: SerialNumber: 01 [3.447530] asix 3-3.2.4:1.0 (unnamed net_device) (uninitialized): invalid hw address, using random [3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at usb-1211.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, de:a2:66:bf:ca:4f [4.488773] asix 3-3.2.4:1.0 eth0: link down [5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1 [5.712947] Sending DHCP requests .. timed out! [ 83.165303] IP-Config: Retrying forever (NFS root)... [ 83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1 [ 83.192944] Sending DHCP requests . Similar results also with dhclient. Git bisect identified the breaking commit as: commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31 Author: Michel Stam Date: Thu Oct 2 10:22:02 2014 +0200 asix: Don't reset PHY on if_up for ASIX 88772 Taking 3.18-rc3 and that commit reverted, network works again: [3.303500] usb 3-3.2.4: new high-speed USB device number 4 using exynos-ehci [3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95, idProduct=772a [3.404963] usb 3-3.2.4: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [3.412424] usb 3-3.2.4: Product: AX88772 [3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp. [3.421715] usb 3-3.2.4: SerialNumber: 01 [3.427755] asix 3-3.2.4:1.0 (unnamed net_device) (uninitialized): invalid hw address, using random [3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at usb-1211.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, 12:59:f1:a8:43:90 [7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1 [7.118258] Sending DHCP requests ., OK [9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my address is 192.168.1.111 There might something wrong on the samsung platform code (I understand the USB on arndale is "funny"), but this is still an regression from 3.17. Riku -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 08/14] mfd: max77686/802: Remove support for board files
On pon, 2014-11-03 at 17:01 +, Lee Jones wrote: > On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote: > > > The driver is used only on Exynos based boards with DTS support. > > Convert the driver to DTS-only version. This simplifies a little the > > code: > > 1. No dead (unused) entries in platform_data structure. > > 2. More code removed. > > 3. Regulator driver does not depend on allocated memory > >from MFD driver. > > 4. It makes also easier extending the regulator driver. > > > > Add to the max77686 MFD driver dependency on CONFIG_OF because without > > DTS the regulator drivers won't bind. > > > > Signed-off-by: Krzysztof Kozlowski > > Reviewed-by: Javier Martinez Canillas > > --- > > drivers/mfd/Kconfig | 1 + > > drivers/mfd/max77686.c | 23 --- > > include/linux/mfd/max77686-private.h | 1 - > > include/linux/mfd/max77686.h | 28 > > 4 files changed, 1 insertion(+), 52 deletions(-) > > Applied, thanks. N, don't apply it. It depends on previous patches removing the board file support from regulators. If the patch looks OK, please only ack it. I would like to push everything through regulator tree. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/14] mfd/regulator: dt-bindings: max77686: Document of_compatible for regulator
On pon, 2014-11-03 at 17:00 +, Lee Jones wrote:
> On Thu, 30 Oct 2014, Krzysztof Kozlowski wrote:
>
> > Document new required property for regulator driver: of_compatible. The
> > property allows MFD core to bind the driver to node with regulators and
> > this simplifies parsing regulators init data from DTS
> >
> > Signed-off-by: Krzysztof Kozlowski
> > ---
> > Documentation/devicetree/bindings/mfd/max77686.txt | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
>
> I want a thumbs-up from Mark on this before applying.
I'll be resending new version, without the compatible property.
Best regards,
Krzysztof
>
> > diff --git a/Documentation/devicetree/bindings/mfd/max77686.txt
> > b/Documentation/devicetree/bindings/mfd/max77686.txt
> > index 75fdfaf41831..f4010a9f66eb 100644
> > --- a/Documentation/devicetree/bindings/mfd/max77686.txt
> > +++ b/Documentation/devicetree/bindings/mfd/max77686.txt
> > @@ -18,8 +18,11 @@ Required properties:
> >
> > Optional node:
> > - voltage-regulators : The regulators of max77686 have to be instantiated
> > - under subnode named "voltage-regulators" using the following format.
> > + under subnode named "voltage-regulators".
> > + Required property:
> > + - compatible : Must be "maxim,max77686-pmic"
> >
> > + Optional properties: For each regulator use following format:
> > regulator_name {
> > regulator-compatible = LDOn/BUCKn
> > standard regulator constraints
> > @@ -49,6 +52,8 @@ Example:
> > reg = <0x09>;
> >
> > voltage-regulators {
> > + compatible = "maxim,max77686-pmic";
> > +
> > ldo11_reg {
> > regulator-compatible = "LDO11";
> > regulator-name = "vdd_ldo11";
>
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
