Re: [PATCH 8/9] ARM: platform_device: pdev_archdata: add omap_device pointer

2011-08-06 Thread Grant Likely
On Sat, Aug 6, 2011 at 1:19 AM, Kevin Hilman khil...@ti.com wrote:
 Add omap_device pointer to the ARM-specific arch data in the
 platform_device.  This will be used to attach OMAP-specific
 device-data to the platform device with device lifetime.

 Suggested-by: Russell King rmk+ker...@arm.linux.org.uk
 Cc: Grant Likely grant.lik...@secretlab.ca
 Signed-off-by: Kevin Hilman khil...@ti.com

Acked-by: Grant Likely grant.lik...@secretlab.ca

 ---
  arch/arm/include/asm/device.h |    5 +
  1 files changed, 5 insertions(+), 0 deletions(-)

 diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
 index 9f390ce..b5c9f5b 100644
 --- a/arch/arm/include/asm/device.h
 +++ b/arch/arm/include/asm/device.h
 @@ -12,7 +12,12 @@ struct dev_archdata {
  #endif
  };

 +struct omap_device;
 +
  struct pdev_archdata {
 +#ifdef CONFIG_ARCH_OMAP
 +       struct omap_device *od;
 +#endif
  };

  #endif
 --
 1.7.6





-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 9/9] OMAP: omap_device: decouple platform_device from omap_device

2011-08-06 Thread Grant Likely
Yes, I think this is the right thing to do.  It will certainly make it
possible to keep omap-specific hooks out of the device tree
of_platform_populate() path by using a notifier to attach hwmod data
instead.

Lightly-glanced-at-by: Grant Likely grant.lik...@secretlab.ca

g.

On Sat, Aug 6, 2011 at 1:19 AM, Kevin Hilman khil...@ti.com wrote:
 Rather than embedding a struct platform_device inside a struct
 omap_device, decouple them, leaving only a pointer to the
 platform_device inside the omap_device.

 Use the arch-specific data field of the platform_device (pdev_archdata)
 to add an omap_device pointer after the platform_device has been created.

 Signed-off-by: Kevin Hilman khil...@ti.com
 ---
  arch/arm/mach-omap2/opp.c                     |    2 +-
  arch/arm/plat-omap/include/plat/omap_device.h |    7 +-
  arch/arm/plat-omap/omap_device.c              |  100 
 +
  3 files changed, 58 insertions(+), 51 deletions(-)

 diff --git a/arch/arm/mach-omap2/opp.c b/arch/arm/mach-omap2/opp.c
 index ab8b35b..9262a6b 100644
 --- a/arch/arm/mach-omap2/opp.c
 +++ b/arch/arm/mach-omap2/opp.c
 @@ -69,7 +69,7 @@ int __init omap_init_opp_table(struct omap_opp_def *opp_def,
                                opp_def-hwmod_name, i);
                        return -EINVAL;
                }
 -               dev = oh-od-pdev.dev;
 +               dev = oh-od-pdev-dev;

                r = opp_add(dev, opp_def-freq, opp_def-u_volt);
                if (r) {
 diff --git a/arch/arm/plat-omap/include/plat/omap_device.h 
 b/arch/arm/plat-omap/include/plat/omap_device.h
 index 4f98770..d4d9b96 100644
 --- a/arch/arm/plat-omap/include/plat/omap_device.h
 +++ b/arch/arm/plat-omap/include/plat/omap_device.h
 @@ -68,7 +68,7 @@ extern struct device omap_device_parent;
  *
  */
  struct omap_device {
 -       struct platform_device          pdev;
 +       struct platform_device          *pdev;
        struct omap_hwmod               **hwmods;
        struct omap_device_pm_latency   *pm_lats;
        u32                             dev_wakeup_lat;
 @@ -146,7 +146,10 @@ struct omap_device_pm_latency {
  #define OMAP_DEVICE_LATENCY_AUTO_ADJUST BIT(1)

  /* Get omap_device pointer from platform_device pointer */
 -#define to_omap_device(x) container_of((x), struct omap_device, pdev)
 +static inline struct omap_device *to_omap_device(struct platform_device 
 *pdev)
 +{
 +       return pdev ? pdev-archdata.od : NULL;
 +}

  static inline
  void omap_device_disable_idle_on_suspend(struct platform_device *pdev)
 diff --git a/arch/arm/plat-omap/omap_device.c 
 b/arch/arm/plat-omap/omap_device.c
 index 351df31..d8f2299 100644
 --- a/arch/arm/plat-omap/omap_device.c
 +++ b/arch/arm/plat-omap/omap_device.c
 @@ -117,7 +117,7 @@ static int _omap_device_activate(struct omap_device *od, 
 u8 ignore_lat)
  {
        struct timespec a, b, c;

 -       dev_dbg(od-pdev.dev, omap_device: activating\n);
 +       dev_dbg(od-pdev-dev, omap_device: activating\n);

        while (od-pm_lat_level  0) {
                struct omap_device_pm_latency *odpl;
 @@ -141,7 +141,7 @@ static int _omap_device_activate(struct omap_device *od, 
 u8 ignore_lat)
                c = timespec_sub(b, a);
                act_lat = timespec_to_ns(c);

 -               dev_dbg(od-pdev.dev,
 +               dev_dbg(od-pdev-dev,
                        omap_device: pm_lat %d: activate: elapsed time 
                        %llu nsec\n, od-pm_lat_level, act_lat);

 @@ -149,12 +149,12 @@ static int _omap_device_activate(struct omap_device 
 *od, u8 ignore_lat)
                        odpl-activate_lat_worst = act_lat;
                        if (odpl-flags  OMAP_DEVICE_LATENCY_AUTO_ADJUST) {
                                odpl-activate_lat = act_lat;
 -                               dev_dbg(od-pdev.dev,
 +                               dev_dbg(od-pdev-dev,
                                        new worst case activate latency 
                                        %d: %llu\n,
                                        od-pm_lat_level, act_lat);
                        } else
 -                               dev_warn(od-pdev.dev,
 +                               dev_warn(od-pdev-dev,
                                         activate latency %d 
                                         higher than exptected. (%llu  
 %d)\n,
                                         od-pm_lat_level, act_lat,
 @@ -185,7 +185,7 @@ static int _omap_device_deactivate(struct omap_device 
 *od, u8 ignore_lat)
  {
        struct timespec a, b, c;

 -       dev_dbg(od-pdev.dev, omap_device: deactivating\n);
 +       dev_dbg(od-pdev-dev, omap_device: deactivating\n);

        while (od-pm_lat_level  od-pm_lats_cnt) {
                struct omap_device_pm_latency *odpl;
 @@ -208,7 +208,7 @@ static int _omap_device_deactivate(struct omap_device 
 *od, u8 ignore_lat)
                c = timespec_sub(b, a);
                deact_lat = timespec_to_ns(c);

 -               dev_dbg(od-pdev.dev,
 + 

[PATCH] arm: mach-omap2: mux: use kstrdup()

2011-08-06 Thread Thomas Meyer
From: Thomas Meyer tho...@m3y3r.de

 Use kstrdup rather than duplicating its implementation

 The semantic patch that makes this output is available
 in scripts/coccinelle/api/kstrdup.cocci.

 More information about semantic patching is available at
 http://coccinelle.lip6.fr/

Signed-off-by: Thomas Meyer tho...@m3y3r.de
---

diff -u -p a/arch/arm/mach-omap2/mux.c b/arch/arm/mach-omap2/mux.c
--- a/arch/arm/mach-omap2/mux.c 2011-06-10 21:39:11.837703903 +0200
+++ b/arch/arm/mach-omap2/mux.c 2011-08-01 21:25:41.888603428 +0200
@@ -821,11 +821,10 @@ static void __init omap_mux_set_cmdline_
if (!omap_mux_options)
return;
 
-   options = kmalloc(strlen(omap_mux_options) + 1, GFP_KERNEL);
+   options = kstrdup(omap_mux_options, GFP_KERNEL);
if (!options)
return;
 
-   strcpy(options, omap_mux_options);
next_opt = options;
 
while ((token = strsep(next_opt, ,)) != NULL) {
@@ -855,24 +854,19 @@ static int __init omap_mux_copy_names(st
 
for (i = 0; i  OMAP_MUX_NR_MODES; i++) {
if (src-muxnames[i]) {
-   dst-muxnames[i] =
-   kmalloc(strlen(src-muxnames[i]) + 1,
-   GFP_KERNEL);
+   dst-muxnames[i] = kstrdup(src-muxnames[i],
+  GFP_KERNEL);
if (!dst-muxnames[i])
goto free;
-   strcpy(dst-muxnames[i], src-muxnames[i]);
}
}
 
 #ifdef CONFIG_DEBUG_FS
for (i = 0; i  OMAP_MUX_NR_SIDES; i++) {
if (src-balls[i]) {
-   dst-balls[i] =
-   kmalloc(strlen(src-balls[i]) + 1,
-   GFP_KERNEL);
+   dst-balls[i] = kstrdup(src-balls[i], GFP_KERNEL);
if (!dst-balls[i])
goto free;
-   strcpy(dst-balls[i], src-balls[i]);
}
}
 #endif


--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

2011-08-06 Thread Rafael J. Wysocki
On Saturday, August 06, 2011, Mark Brown wrote:
 On Fri, Aug 05, 2011 at 09:37:36PM +0200, Rafael J. Wysocki wrote:
  On Friday, August 05, 2011, Mark Brown wrote:
 
   Do you have any examples of this that aren't better expressed in device
   specific terms?
 
  I'm not sure what you mean exactly, but if you take two PC-like systems
  with similar hardware configurations, but different BIOS-es and motherboard
  layouts, it's very likely that on one of them PCI PME won't be routed
  correctly, for example.  In that case the kernel has no way to figure out
  that that system is broken, the problem can only be worked around from user
  space by diabling runtime PM on the affected PCI devices.  I expect similar
  problems to appear for the PM QoS settings.
 
 I wouldn't say we've got to rely on userspace here - it seems like we
 ought to be able to use DMI or other system data to identify the
 affected systems and activate the workarounds.

That's only practical on systems where the kernel can be rebuilt.
Moreover, if that affects individual devices, using DMI-based blacklists
is not really practical at all.

   It's not that users don't know what they're doing, it's
   that working around system integration and stability issues in userspace
   isn't really progressing things well or helping with maintainability.
 
  No, it's not, but sometimes we simply don't have the choice.  Besides,
  in the particular case of PM QoS, the constraints set by user space will
  simply be added to the constraints set by kernel subsystems.  Thus they
  won't prevent any kernel subsystem from specifying its own constraints,
  but they will give user space the option to override the constraints
  originating from the kernel.
 
 You're right that it doesn't stop the kernel doing anything, the concern
 is that people just won't bother making the kernel work properly and
 will just do their power management in userspace and not bother fixing
 the kernel.

I wouldn't call it fixing the kernel.  I'd rather say putting workarounds
into the kernel, which I'm not sure is the right thing to at least in some
cases.

 Punting to userspace seems like it is creating the
 expectation that we can't make the kernel work and isn't great from a
 usability perspective since users shouldn't really be worrying about bus
 performance or so on, it's not something that's visible at the level
 applications work at.

However, platform builders may want to fine tuned things and I'm not sure
we should require them to patch the kernel for this purpose.

   Generally if the user has sufficient access to be able to do anything
   with this stuff they've got just as much access to the kernel as to
   userspace.
 
  Do you mean they may rebuild the kernel?  That isn't always possible.
 
 I'm not sure I can see a lot of cases where you'd have root access and
 not be able to do kernel updates if required?  Having stuff in debugfs
 for diagnostics doesn't strike me as a problem if that's the issue.

Root access doesn't necessarily mean you have all of the requisite
tools (like compilers etc.) and installing them isn't always trivial
(think of systems like phones etc.), let alone building the kernel from
sources (where you don't necessarily know the original .config used for
building your device's kernel).

IOW, I don't buy the you can always rebuild the kernel if necessary
argument.  It simply is not true in general.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-pm] [PATCH 02/11] PM: extend PM QoS with per-device wake-up constraints

2011-08-06 Thread Mark Brown
On Sat, Aug 06, 2011 at 09:46:20PM +0200, Rafael J. Wysocki wrote:
 On Saturday, August 06, 2011, Mark Brown wrote:

  I wouldn't say we've got to rely on userspace here - it seems like we
  ought to be able to use DMI or other system data to identify the
  affected systems and activate the workarounds.

 That's only practical on systems where the kernel can be rebuilt.
 Moreover, if that affects individual devices, using DMI-based blacklists
 is not really practical at all.

OK, so this does sound like there's probably a genuine issue on PCs due
to limitations in the environment.  Is it possible to expose these
things to userspace in a way that's limited to the affected platforms?

  You're right that it doesn't stop the kernel doing anything, the concern
  is that people just won't bother making the kernel work properly and
  will just do their power management in userspace and not bother fixing
  the kernel.

 I wouldn't call it fixing the kernel.  I'd rather say putting workarounds
 into the kernel, which I'm not sure is the right thing to at least in some
 cases.

That does sound like a fair characterization for PCs.  For embedded
systems where we have a *much* better knowledge of the hardware we're
running on you're just working with the basics of what the hardware
needs to run - the hardware needs whatever it needs and no matter what
you think of the quality of the hardware there's an expectation that the
kernel is ging to be able to work.

  Punting to userspace seems like it is creating the
  expectation that we can't make the kernel work and isn't great from a
  usability perspective since users shouldn't really be worrying about bus
  performance or so on, it's not something that's visible at the level
  applications work at.

 However, platform builders may want to fine tuned things and I'm not sure
 we should require them to patch the kernel for this purpose.

As I've said it's not the fine tuning that I'm worried about, it's the
specific mechanism that's being suggested.  Being able to tune things in
a way that's relevant to the device being tuned seems entirely sensible.

  I'm not sure I can see a lot of cases where you'd have root access and
  not be able to do kernel updates if required?  Having stuff in debugfs
  for diagnostics doesn't strike me as a problem if that's the issue.

 Root access doesn't necessarily mean you have all of the requisite
 tools (like compilers etc.) and installing them isn't always trivial
 (think of systems like phones etc.), let alone building the kernel from
 sources (where you don't necessarily know the original .config used for
 building your device's kernel).

Phones are exactly the sort of case I'm primarily concerned with here.

Realistically if you're in a position where you need to work at this
very low level on an embedded device you can replace the entire firmware
on the device.  We don't want to end up in the situation where we've got
kernel support for a device and the only way to get it to actually run
sensibly is to install the silicon vendor's proprietary userspace, and
we especially don't want to end up in the situation where that userspace
is using standard and supported kernel interfaces to do its thing.

 IOW, I don't buy the you can always rebuild the kernel if necessary
 argument.  It simply is not true in general.

You can push that argument to extremes, of course.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Shouldn't DT preserve pdev name and id to allow platform_match to work?

2011-08-06 Thread Grant Likely
On Fri, Aug 05, 2011 at 02:31:16PM +0200, Cousson, Benoit wrote:
 On 8/5/2011 12:02 PM, Barry Song wrote:
 auxdata passes platform_data and overrides the device name when there
 is no way easy way to make the driver work without it.  It handles the
 the current implementation of clocks and regulators which aren't yet
 populated from the device tree.  It will go away when clock
 regulator bindings are implemented.
 Yes. As OF_DEV_AUXDATA_ID still requires hardware information like
 0x48000100 as below, it seems it is not
 consistent with the origin purpose of ARM DT.
 OF_DEV_AUXDATA_ID(ti,omap-i2c, 0x48000100, omap-i2c.1, 1,i2c_pdata)
 And the information 0x48000100 is something that doesn't want to be in
 kernel codes.it should be only in dts.
 
 FWIW, I do not care about the physical address at all. This is just
 used by the of_dev_lookup function to get the proper instance for a
 device compatible type.
 That's the only way for the OF_DEV_AUXDATA to work, but in theory
 you could do the same as soon as you provide the id. So
 OF_DEV_AUXDATA_ID could avoid it.
 
 Since OF_DEV_AUXDATA_ID seems to be already dead before it even
 reaches the mainline, let's forget about that.

Actually, I'm planning to merge OF_DEV_AUXDATA_ID().  I was hoping to
avoid it but there was enough hard-to-solve-quickly problems that I've
relented.

Adding the match table is not hard-to-solve though, so I'm not
relenting on that point.  :-)

g.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html