[RFC 00/18] Versatile Express config rework

[RFC 00/18] Versatile Express config rework

[Pawel Moll]

Greetings,

When I promised to rework vexpress' MFD & co. components,
I did't expect it would take almost half a year to post
some patches... The worst thing is that I got most of this
stuff done months ago, but not until the usual holiday
period came I got a chance to post it.

Anyway, the following series retires custom "function"
interface, replacing it with regmap. For a price of
a single WARN_ON removed (05/18 "driver core: Do not WARN 
when devres list is not empty at probe time"), I've managed
to hide all possible VE-specific ways of talking to the
peripherals behind regmap, creating platform devices with
platform-specific regmap config added as devm.

I also had to solve a problem of early initialization
required for the core components, where early doesn't mean
"very early" but simply "earlier than the other ones".
The main issue was of_platform_populate() instantiating
devices for nodes that already has been serviced. There
were some attempts to fix it in the past (including a
struct device pointer in struct device_node). My proposal
is just an extra "already populated" flag in device_node,
set by generic device functions, which makes the
of_platform_populate() to skip such node.

In the end the vexpress-sysreg is almost exclusively
a "pure" MFD device, creating mfd_cells for other drivers.
The system config interface has been split into a "misc"
driver (may become a "soc" driver if the directory
materializes) and the config bus core (using standard
device "class" now) went of course into "bus".

There is also a bunch of other, less important patches
all around the place. I would appreciate the respective
maintainers having a look at them. If anyone wants to
take the small changes in for 3.16 that's cool, if not
I'd more than happy to recieve an ack and merge it with
the whole series in the future.

This series is based on 3.15-rc5 and my hope is to get it
polished enough in time for 3.17 merge window.

Have a great time whatever your plans are for the next
2 weeks!

Pawel

Pawel Moll (17):
  mfd: syscon: Consider platform data a regmap config name
  power/reset: vexpress: Use sched_clock as the time source
  GPIO: gpio-generic: Add label to platform data
  driver core & of: Mark of_nodes of added device as populated
  driver core: Do not WARN when devres list is not empty at probe time
  regmap: Formalise use of non-bus context
  regmap: debugfs: Always create "registers" & "access" files
  hwmon: vexpress: Use regmap instead of custom interface
  power/reset: vexpress: Use regmap instead of custom interface
  regulator: vexpress: Use regmap instead of custom interface
  clocksource: Sched clock source for Versatile Express
  clk: versatile: Split config options for sp810 and vexpress_osc
  clk: versatile: Use regmap instead of custom interface
  ARM: vexpress: Simplify SMP operations for DT-powered system
  bus: Versatile Express configuration bus
  misc: Versatile Express System Config interface driver
  mfd: vexpress: Split sysreg functions into MFD cells

Sudeep KarkadaNagesha (1):
  ARM: vexpress: remove redundant vexpress_dt_cpus_num to get cpu count

 .../devicetree/bindings/arm/vexpress-sysreg.txt    |  37 +-
 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi            |  76 ++-
 arch/arm/boot/dts/vexpress-v2m.dtsi                |  76 ++-
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts         |   5 +-
 arch/arm/mach-vexpress/core.h                      |   3 +-
 arch/arm/mach-vexpress/ct-ca9x4.c                  |   8 +-
 arch/arm/mach-vexpress/platsmp.c                   | 187 ++-----
 arch/arm/mach-vexpress/v2m.c                       |  74 +--
 arch/arm64/boot/dts/rtsm_ve-motherboard.dtsi       |   2 +-
 drivers/base/core.c                                |   4 +
 drivers/base/dd.c                                  |   1 -
 drivers/base/regmap/internal.h                     |   3 +-
 drivers/base/regmap/regmap-debugfs.c               |  10 +-
 drivers/base/regmap/regmap.c                       |  34 +-
 drivers/bus/Kconfig                                |   8 +
 drivers/bus/Makefile                               |   2 +
 drivers/bus/vexpress-config.c                      | 178 +++++++
 drivers/clk/Kconfig                                |   9 +-
 drivers/clk/versatile/Kconfig                      |  26 +
 drivers/clk/versatile/Makefile                     |   5 +-
 drivers/clk/versatile/clk-vexpress-osc.c           |  70 +--
 drivers/clocksource/Kconfig                        |   9 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/vexpress.c                     |  40 ++
 drivers/gpio/gpio-generic.c                        |   2 +
 drivers/hwmon/Kconfig                              |   3 +-
 drivers/hwmon/vexpress.c                           |  29 +-
 drivers/mfd/Kconfig                                |  15 +-
 drivers/mfd/Makefile                               |   2 +-
 drivers/mfd/syscon.c                               |   1 +
 drivers/mfd/vexpress-config.c                      | 288 -----------
 drivers/mfd/vexpress-sysreg.c                      | 563 ++++++---------------
 drivers/misc/Kconfig                               |   9 +
 drivers/misc/Makefile                              |   1 +
 drivers/misc/vexpress-syscfg.c                     | 322 ++++++++++++
 drivers/of/device.c                                |  16 +
 drivers/of/platform.c                              |   6 +-
 drivers/power/reset/Kconfig                        |   2 +-
 drivers/power/reset/vexpress-poweroff.c            |  15 +-
 drivers/regulator/Kconfig                          |   3 +-
 drivers/regulator/vexpress.c                       |  52 +-
 include/linux/basic_mmio_gpio.h                    |   1 +
 include/linux/of.h                                 |   6 +
 include/linux/of_device.h                          |  11 +
 include/linux/regmap.h                             |   6 +-
 include/linux/vexpress.h                           | 101 +---
 46 files changed, 1168 insertions(+), 1154 deletions(-)
 create mode 100644 drivers/bus/vexpress-config.c
 create mode 100644 drivers/clk/versatile/Kconfig
 create mode 100644 drivers/clocksource/vexpress.c
 delete mode 100644 drivers/mfd/vexpress-config.c
 create mode 100644 drivers/misc/vexpress-syscfg.c

-- 
1.8.3.2

[RFC 05/18] driver core: Do not WARN when devres list is not empty at probe time

[Pawel Moll]

regmap_init() adds the initialised map to the device resources,
which can be then obtained in the driver->probe function in a
generic way with dev_reg_regmap(), which makes it independent
from underlying hardware interface. This is useful when
platform_devices are not simply memory mapped but must use
custom way of accessing the registers.

Unfortunately the device core WARNs in a situation when probed
devices has a non-empty resources list. This patch simply
removes this check and doesn't seem to have any side effects.

Signed-off-by: Pawel Moll <pawel.moll@arm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/dd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0605176..07b8419 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -269,7 +269,6 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 	atomic_inc(&probe_count);
 	pr_debug("bus: '%s': %s: probing driver %s with device %s\n",
 		 drv->bus->name, __func__, drv->name, dev_name(dev));
-	WARN_ON(!list_empty(&dev->devres_head));
 
 	dev->driver = drv;
 
-- 
1.8.3.2

[RFC 03/18] GPIO: gpio-generic: Add label to platform data

[Linus Walleij]

On Mon, Dec 23, 2013 at 5:23 PM, Pawel Moll <pawel.moll@arm.com> wrote:

> When registering more than one platform device, it is
> useful to set the gpio chip label in the platform data.
>
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Alexandre Courbot <gnurou@gmail.com>
> Cc: Anton Vorontsov <anton@enomsg.org>

Good point. But what about also adding device tree support for
naming the chips while you're at it?

I imagine a generic gpiochip property in
Documentation/devicetree/bindings/gpio/gpio.txt

Yours,
Linus Walleij

[RFC 08/18] hwmon: vexpress: Use regmap instead of custom interface

[Guenter Roeck]

On Mon, Dec 23, 2013 at 04:23:40PM +0000, Pawel Moll wrote:
> This patch makes the Versatile Express hwmon driver
> use regmap interface, instead of custom vexpress config
> one. It will request the regmap resource associated
> with the device, which makes it pretty much hardware
> agnostic.
> 
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Jean Delvare <khali@linux-fr.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: lm-sensors at lm-sensors.org
> ---
>  drivers/hwmon/Kconfig    |  3 ++-
>  drivers/hwmon/vexpress.c | 29 +++++++++++------------------
>  2 files changed, 13 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 52d548f..7747a47 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1324,7 +1324,8 @@ config SENSORS_TWL4030_MADC
>  
>  config SENSORS_VEXPRESS
>  	tristate "Versatile Express"
> -	depends on VEXPRESS_CONFIG
> +	depends on ARM || ARM64
> +	depends on REGMAP
>  	help
>  	  This driver provides support for hardware sensors available on
>  	  the ARM Ltd's Versatile Express platform. It can provide wide
> diff --git a/drivers/hwmon/vexpress.c b/drivers/hwmon/vexpress.c
> index d867e6b..b58cf1c 100644
> --- a/drivers/hwmon/vexpress.c
> +++ b/drivers/hwmon/vexpress.c
> @@ -22,11 +22,11 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> -#include <linux/vexpress.h>
> +#include <linux/regmap.h>
>  
>  struct vexpress_hwmon_data {
>  	struct device *hwmon_dev;
> -	struct vexpress_config_func *func;
> +	struct regmap *reg;
>  };
>  
>  static ssize_t vexpress_hwmon_name_show(struct device *dev,
> @@ -56,7 +56,7 @@ static ssize_t vexpress_hwmon_u32_show(struct device *dev,
>  	int err;
>  	u32 value;
>  
> -	err = vexpress_config_read(data->func, 0, &value);
> +	err = regmap_read(data->reg, 0, &value);
>  	if (err)
>  		return err;
>  
> @@ -69,13 +69,13 @@ static ssize_t vexpress_hwmon_u64_show(struct device *dev,
>  {
>  	struct vexpress_hwmon_data *data = dev_get_drvdata(dev);
>  	int err;
> -	u32 value_hi, value_lo;
> +	unsigned int value_hi, value_lo;
>  
> -	err = vexpress_config_read(data->func, 0, &value_lo);
> +	err = regmap_read(data->reg, 0, &value_lo);
>  	if (err)
>  		return err;
>  
> -	err = vexpress_config_read(data->func, 1, &value_hi);
> +	err = regmap_read(data->reg, 1, &value_hi);
>  	if (err)
>  		return err;
>  
> @@ -175,26 +175,21 @@ static int vexpress_hwmon_probe(struct platform_device *pdev)
>  	if (!match)
>  		return -ENODEV;
>  
> -	data->func = vexpress_config_func_get_by_dev(&pdev->dev);
> -	if (!data->func)
> +	data->reg = dev_get_regmap(&pdev->dev, NULL);
> +	if (!data->reg)
>  		return -ENODEV;
>  
>  	err = sysfs_create_group(&pdev->dev.kobj, match->data);
>  	if (err)
> -		goto error;
> +		return err;
>  
>  	data->hwmon_dev = hwmon_device_register(&pdev->dev);
>  	if (IS_ERR(data->hwmon_dev)) {
> -		err = PTR_ERR(data->hwmon_dev);
> -		goto error;
> +		sysfs_remove_group(&pdev->dev.kobj, match->data);
> +		return PTR_ERR(data->hwmon_dev);

This change is unrelated and violates coding style. If the goto disturbs you,
I would suggest to convert the driver to use devm_hwmon_device_register_with_groups() 
in a separate patch and get rid of the error handling and the entire remove() function.

Thanks,
Guenter

>  	}
>  
>  	return 0;
> -
> -error:
> -	sysfs_remove_group(&pdev->dev.kobj, match->data);
> -	vexpress_config_func_put(data->func);
> -	return err;
>  }
>  
>  static int vexpress_hwmon_remove(struct platform_device *pdev)
> @@ -207,8 +202,6 @@ static int vexpress_hwmon_remove(struct platform_device *pdev)
>  	match = of_match_device(vexpress_hwmon_of_match, &pdev->dev);
>  	sysfs_remove_group(&pdev->dev.kobj, match->data);
>  
> -	vexpress_config_func_put(data->func);
> -
>  	return 0;
>  }
>  
> -- 
> 1.8.3.2
> 
> 
> 

[RFC 02/18] power/reset: vexpress: Use sched_clock as the time source

[John Stultz]

On Mon, Dec 23, 2013 at 8:23 AM, Pawel Moll <pawel.moll@arm.com> wrote:
> At this stage of system shutdown procedure the jiffies may
> not be updated anymore, so have to base on raw sched_clock
> values.
>
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Anton Vorontsov <anton@enomsg.org>
> Cc: David Woodhouse <dwmw2@infradead.org>
> ---
>  drivers/power/reset/vexpress-poweroff.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/reset/vexpress-poweroff.c b/drivers/power/reset/vexpress-poweroff.c
> index 476aa49..d752233 100644
> --- a/drivers/power/reset/vexpress-poweroff.c
> +++ b/drivers/power/reset/vexpress-poweroff.c
> @@ -15,6 +15,7 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
> +#include <linux/sched.h>
>  #include <linux/stat.h>
>  #include <linux/vexpress.h>
>
> @@ -27,12 +28,12 @@ static void vexpress_reset_do(struct device *dev, const char *what)
>                         vexpress_config_func_get_by_dev(dev);
>
>         if (func) {
> -               unsigned long timeout;
> +               unsigned long long timeout_ns;
>
>                 err = vexpress_config_write(func, 0, 0);
>
> -               timeout = jiffies + HZ;
> -               while (time_before(jiffies, timeout))
> +               timeout_ns = sched_clock() + 50000000;
> +               while (!err && time_before64(sched_clock(), timeout_ns))
>                         cpu_relax();
>         }

So this may not be a problem in this particular case, but sched_clock
could be backed by jiffies on some hardware, causing the same problem
to appear.

Might udelay/mdelay be a better fit for this sort of case (since
udelay may be counter backed, but may also be loop backed on hardware
without continuous counters)?

thanks
-john

[RFC 12/18] clk: versatile: Split config options for sp810 and vexpress_osc

[Mike Turquette]

Quoting Pawel Moll (2013-12-23 08:23:44)
> Move the Kconfig entry for Versatile (& Express) clock drivers
> into a separate file and add individual options for sp810
> and vexpress_osc drivers, as they are optional in some
> configurations and may have separate dependencies.
> 
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Mike Turquette <mturquette@linaro.org>

Is there a reason to continue to expose these as user-visible config
options? I guess that with Device Tree we can just include clock nodes
(or not) for boards that use these clocks (or do not use them).

And it seems for many configurations COMMON_CLK_VERSATILE is always
selected, so what is the point of the other two sub-options?

Thanks,
Mike

> ---
>  drivers/clk/Kconfig            |  9 +--------
>  drivers/clk/versatile/Kconfig  | 26 ++++++++++++++++++++++++++
>  drivers/clk/versatile/Makefile |  5 +++--
>  3 files changed, 30 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/clk/versatile/Kconfig
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 5c51115..f5486a4 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -40,14 +40,7 @@ config COMMON_CLK_WM831X
>            Supports the clocking subsystem of the WM831x/2x series of
>           PMICs from Wolfson Microlectronics.
>  
> -config COMMON_CLK_VERSATILE
> -       bool "Clock driver for ARM Reference designs"
> -       depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
> -       ---help---
> -          Supports clocking on ARM Reference designs:
> -         - Integrator/AP and Integrator/CP
> -         - RealView PB1176, EB, PB11MP and PBX
> -         - Versatile Express
> +source "drivers/clk/versatile/Kconfig"
>  
>  config COMMON_CLK_MAX77686
>         tristate "Clock driver for Maxim 77686 MFD"
> diff --git a/drivers/clk/versatile/Kconfig b/drivers/clk/versatile/Kconfig
> new file mode 100644
> index 0000000..1530c93
> --- /dev/null
> +++ b/drivers/clk/versatile/Kconfig
> @@ -0,0 +1,26 @@
> +config COMMON_CLK_VERSATILE
> +       bool "Clock driver for ARM Reference designs"
> +       depends on ARCH_INTEGRATOR || ARCH_REALVIEW || ARCH_VEXPRESS || ARM64
> +       ---help---
> +          Supports clocking on ARM Reference designs:
> +         - Integrator/AP and Integrator/CP
> +         - RealView PB1176, EB, PB11MP and PBX
> +         - Versatile Express
> +
> +config CLK_SP810
> +       bool "Clock driver for ARM SP810 System Controller"
> +       depends on COMMON_CLK_VERSATILE
> +       default y if ARCH_VEXPRESS
> +       ---help---
> +         Supports clock muxing (REFCLK/TIMCLK to TIMERCLKEN0-3) capabilities
> +         of the ARM SP810 System Controller cell.
> +
> +config CLK_VEXPRESS_OSC
> +       bool "Clock driver for Versatile Express OSC clock generators"
> +       depends on COMMON_CLK_VERSATILE
> +       depends on VEXPRESS_CONFIG
> +       default y if ARCH_VEXPRESS
> +       ---help---
> +         Simple regmap-based driver driving clock generators on Versatile
> +         Express platforms hidden behind its configuration infrastructure,
> +         commonly known as OSCs.
> diff --git a/drivers/clk/versatile/Makefile b/drivers/clk/versatile/Makefile
> index c16ca78..fd449f9 100644
> --- a/drivers/clk/versatile/Makefile
> +++ b/drivers/clk/versatile/Makefile
> @@ -3,5 +3,6 @@ obj-$(CONFIG_ICST)              += clk-icst.o
>  obj-$(CONFIG_ARCH_INTEGRATOR)  += clk-integrator.o
>  obj-$(CONFIG_INTEGRATOR_IMPD1) += clk-impd1.o
>  obj-$(CONFIG_ARCH_REALVIEW)    += clk-realview.o
> -obj-$(CONFIG_ARCH_VEXPRESS)    += clk-vexpress.o clk-sp810.o
> -obj-$(CONFIG_VEXPRESS_CONFIG)  += clk-vexpress-osc.o
> +obj-$(CONFIG_ARCH_VEXPRESS)    += clk-vexpress.o
> +obj-$(CONFIG_CLK_SP810)                += clk-sp810.o
> +obj-$(CONFIG_CLK_VEXPRESS_OSC) += clk-vexpress-osc.o
> -- 
> 1.8.3.2
> 
> 

[RFC 07/18] regmap: debugfs: Always create "registers" & "access" files

[Mark Brown]

On Mon, Dec 23, 2013 at 04:23:39PM +0000, Pawel Moll wrote:
> When a map covers a single register, max_register is equal
> to 0, so the "registers" & "access" files were not created.
> 
> This patch is removing this restriction. It should be save,
> as the maps not without register 0 should return false
> for regmap_readable(map, 0).

No, we need to find a better way of doing this such as checking to see
if register zero is accessible in some way.  We want to avoid having
those files if there's no way of populating them since this is useful
information for tooling.

[RFC 10/18] regulator: vexpress: Use regmap instead of custom interface

[Mark Brown]

On Mon, Dec 23, 2013 at 04:23:42PM +0000, Pawel Moll wrote:

> This patch makes the Versatile Express regulator driver
> use regmap interface, instead of custom vexpress config
> one. It will request the regmap resource associated
> with the device, which makes it pretty much hardware
> agnostic.

If this change is making the operations hardware agnostic regmap based
ones then they should be being either replaced with standard operations
or standard operations being added.  It looks like you should be able to
use the existing linear map operations.

[RFC 06/18] regmap: Formalise use of non-bus context

[Mark Brown]

On Mon, Dec 23, 2013 at 04:23:38PM +0000, Pawel Moll wrote:

I'd note that I wasn't CCed on most of this series so I'm not entirely
sure what it's trying to do.

> Bus-less maps (ones with reg_read and reg_write functions
> defined in regmap_config) were given the context passed
> in regmap_init(), but it was still called "bus_context".
> 
> This patch formalises this aspect by renaming it to simple
> "context" and adds the missing link, free_context function
> in regmap_config, which allows bus-less maps to use the
> context in classic way.

This should be two patches, one patch to do the rename and one to add
the operation.  The obvious question here is why is this callback useful
- what is being allocated in a regmap specific context that needs to be
lifetime managed separately to the thing doing the creation?  I can't
see any obvious reason why this would ever get used.

[RFC 01/18] mfd: syscon: Consider platform data a regmap config name

[Lee Jones]

> Use the device platform data as a regmap config
> name. This is particularly useful in the regmap
> debugfs when there is more than one syscon device
> registered, to distinguish the register blocks.
> 
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mfd/syscon.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 71841f9..ea1770b 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -143,6 +143,7 @@ static int syscon_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	syscon_regmap_config.max_register = res->end - res->start - 3;
> +	syscon_regmap_config.name = dev_get_platdata(&pdev->dev);
>  	syscon->regmap = devm_regmap_init_mmio(dev, base,
>  					&syscon_regmap_config);
>  	if (IS_ERR(syscon->regmap)) {

This smells a bit fishy to me. Can you point me to the piece of code
or patch where you set the platform_data pointer as a string?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[RFC 01/18] mfd: syscon: Consider platform data a regmap config name

[Lee Jones]

On Mon, 06 Jan 2014, Lee Jones wrote:

> > Use the device platform data as a regmap config
> > name. This is particularly useful in the regmap
> > debugfs when there is more than one syscon device
> > registered, to distinguish the register blocks.
> > 
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/mfd/syscon.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> > index 71841f9..ea1770b 100644
> > --- a/drivers/mfd/syscon.c
> > +++ b/drivers/mfd/syscon.c
> > @@ -143,6 +143,7 @@ static int syscon_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  
> >  	syscon_regmap_config.max_register = res->end - res->start - 3;
> > +	syscon_regmap_config.name = dev_get_platdata(&pdev->dev);
> >  	syscon->regmap = devm_regmap_init_mmio(dev, base,
> >  					&syscon_regmap_config);
> >  	if (IS_ERR(syscon->regmap)) {
> 
> This smells a bit fishy to me. Can you point me to the piece of code
> or patch where you set the platform_data pointer as a string?

Scrap that, I see it.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[RFC 18/18] mfd: vexpress: Split sysreg functions into MFD cells

[Lee Jones]

> This patch splits individual sysreg functions into
> separate MFD cells, which then become individual
> platform devices with their own drivers.
> 
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> ---
>  .../devicetree/bindings/arm/vexpress-sysreg.txt    |  37 ++-
>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi            |  76 ++++++-
>  arch/arm/boot/dts/vexpress-v2m.dtsi                |  76 ++++++-
>  arch/arm/mach-vexpress/v2m.c                       |  23 +-
>  drivers/mfd/Kconfig                                |  11 +
>  drivers/mfd/Makefile                               |   2 +-
>  drivers/mfd/vexpress-sysreg.c                      | 247 +++++++++++++++------
>  include/linux/vexpress.h                           |  16 +-
>  8 files changed, 377 insertions(+), 111 deletions(-)

<snip>

> +static struct mfd_cell vexpress_sysreg_cells[] = {
> +	{
> +		.name = "syscon",
> +		.of_compatible = "arm,vexpress-sysreg,sys_id",
> +		.num_resources = 1,
> +		.resources = (struct resource []) {
> +			DEFINE_RES_MEM(SYS_ID, 0x4),
> +		},
> +		.platform_data = &vexpress_sysreg_sys_id_pdata,
> +		.pdata_size = sizeof(vexpress_sysreg_sys_id_pdata),

Not sure how comfortable I am with using Device Tree and populating
platform_data with information which by the looks of it you're
planning to make persistent. What's stopping you from using a DT
property to name the block, or better yet, just use the compatible
string? Also, won't naming these blocks in DT also aid other OSes?

<snip>

> +static int vexpress_sysreg_probe(struct platform_device *pdev)
>  {
> -	vexpress_sysreg_base = base;
> -	vexpress_sysreg_setup(NULL);
> +	struct resource *mem;
> +	void __iomem *base;
> +	struct bgpio_chip *mmc_gpio_chip;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem)
> +		return -EINVAL;
> +
> +	base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
> +	if (!base)
> +		return -ENOMEM;
> +
> +	vexpress_config_set_master(vexpress_sysreg_get_master(base));
> +
> +	/*
> +	 * Duplicated SYS_MCI pseudo-GPIO controller for compatibility with
> +	 * older trees using sysreg node for MMC control lines.
> +	 */
> +	mmc_gpio_chip = devm_kzalloc(&pdev->dev, sizeof(*mmc_gpio_chip),
> +			GFP_KERNEL);
> +	if (!mmc_gpio_chip)
> +		return -ENOMEM;
> +	bgpio_init(mmc_gpio_chip, &pdev->dev, 0x4, base + SYS_MCI, NULL,
> +			NULL, NULL, NULL, 0);
> +	mmc_gpio_chip->gc.ngpio = 2;
> +	gpiochip_add(&mmc_gpio_chip->gc);
> +
> +	return mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
> +			vexpress_sysreg_cells,
> +			ARRAY_SIZE(vexpress_sysreg_cells), mem, 0, 0);

Don't use 0 as NULL, you will cause a sparse error.

<snip>

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[RFC 03/18] GPIO: gpio-generic: Add label to platform data

[Pawel Moll]

Greetings (and the usual: Happy New Year!)

On Mon, 2013-12-23 at 17:26 +0000, Linus Walleij wrote:
> On Mon, Dec 23, 2013 at 5:23 PM, Pawel Moll <pawel.moll@arm.com> wrote:
> 
> > When registering more than one platform device, it is
> > useful to set the gpio chip label in the platform data.
> >
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > Cc: Linus Walleij <linus.walleij@linaro.org>
> > Cc: Alexandre Courbot <gnurou@gmail.com>
> > Cc: Anton Vorontsov <anton@enomsg.org>
> 
> Good point. But what about also adding device tree support for
> naming the chips while you're at it?
> 
> I imagine a generic gpiochip property in
> Documentation/devicetree/bindings/gpio/gpio.txt

Well, this has been discussed almost to death already (as you may
remember ;-) in the thread:

http://www.spinics.net/lists/devicetree/msg00072.html

(only portions survived)

Although I was in favour of the generic binding (as were you if I
remember correctly), the final non-conclusion was not to open the
"generic door". Fair enough with me, I went the defined-in-code way
here...

Pawe?

[RFC 08/18] hwmon: vexpress: Use regmap instead of custom interface

[Pawel Moll]

On Mon, 2013-12-23 at 17:31 +0000, Guenter Roeck wrote:
> On Mon, Dec 23, 2013 at 04:23:40PM +0000, Pawel Moll wrote:
> > This patch makes the Versatile Express hwmon driver
> > use regmap interface, instead of custom vexpress config
> > one. It will request the regmap resource associated
> > with the device, which makes it pretty much hardware
> > agnostic.
> > 
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > Cc: Jean Delvare <khali@linux-fr.org>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Cc: lm-sensors at lm-sensors.org
> > ---
> >  drivers/hwmon/Kconfig    |  3 ++-
> >  drivers/hwmon/vexpress.c | 29 +++++++++++------------------
> >  2 files changed, 13 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 52d548f..7747a47 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -1324,7 +1324,8 @@ config SENSORS_TWL4030_MADC
> >  
> >  config SENSORS_VEXPRESS
> >  	tristate "Versatile Express"
> > -	depends on VEXPRESS_CONFIG
> > +	depends on ARM || ARM64
> > +	depends on REGMAP
> >  	help
> >  	  This driver provides support for hardware sensors available on
> >  	  the ARM Ltd's Versatile Express platform. It can provide wide
> > diff --git a/drivers/hwmon/vexpress.c b/drivers/hwmon/vexpress.c
> > index d867e6b..b58cf1c 100644
> > --- a/drivers/hwmon/vexpress.c
> > +++ b/drivers/hwmon/vexpress.c
> > @@ -22,11 +22,11 @@
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> > -#include <linux/vexpress.h>
> > +#include <linux/regmap.h>
> >  
> >  struct vexpress_hwmon_data {
> >  	struct device *hwmon_dev;
> > -	struct vexpress_config_func *func;
> > +	struct regmap *reg;
> >  };
> >  
> >  static ssize_t vexpress_hwmon_name_show(struct device *dev,
> > @@ -56,7 +56,7 @@ static ssize_t vexpress_hwmon_u32_show(struct device *dev,
> >  	int err;
> >  	u32 value;
> >  
> > -	err = vexpress_config_read(data->func, 0, &value);
> > +	err = regmap_read(data->reg, 0, &value);
> >  	if (err)
> >  		return err;
> >  
> > @@ -69,13 +69,13 @@ static ssize_t vexpress_hwmon_u64_show(struct device *dev,
> >  {
> >  	struct vexpress_hwmon_data *data = dev_get_drvdata(dev);
> >  	int err;
> > -	u32 value_hi, value_lo;
> > +	unsigned int value_hi, value_lo;
> >  
> > -	err = vexpress_config_read(data->func, 0, &value_lo);
> > +	err = regmap_read(data->reg, 0, &value_lo);
> >  	if (err)
> >  		return err;
> >  
> > -	err = vexpress_config_read(data->func, 1, &value_hi);
> > +	err = regmap_read(data->reg, 1, &value_hi);
> >  	if (err)
> >  		return err;
> >  
> > @@ -175,26 +175,21 @@ static int vexpress_hwmon_probe(struct platform_device *pdev)
> >  	if (!match)
> >  		return -ENODEV;
> >  
> > -	data->func = vexpress_config_func_get_by_dev(&pdev->dev);
> > -	if (!data->func)
> > +	data->reg = dev_get_regmap(&pdev->dev, NULL);
> > +	if (!data->reg)
> >  		return -ENODEV;
> >  
> >  	err = sysfs_create_group(&pdev->dev.kobj, match->data);
> >  	if (err)
> > -		goto error;
> > +		return err;
> >  
> >  	data->hwmon_dev = hwmon_device_register(&pdev->dev);
> >  	if (IS_ERR(data->hwmon_dev)) {
> > -		err = PTR_ERR(data->hwmon_dev);
> > -		goto error;
> > +		sysfs_remove_group(&pdev->dev.kobj, match->data);
> > +		return PTR_ERR(data->hwmon_dev);
> 
> This change is unrelated and violates coding style. If the goto disturbs you,
> I would suggest to convert the driver to use devm_hwmon_device_register_with_groups() 
> in a separate patch and get rid of the error handling and the entire remove() function.

Ok, will do, thanks!

Pawe?

[RFC 02/18] power/reset: vexpress: Use sched_clock as the time source

[Pawel Moll]

On Mon, 2013-12-23 at 19:28 +0000, John Stultz wrote:
> On Mon, Dec 23, 2013 at 8:23 AM, Pawel Moll <pawel.moll@arm.com> wrote:
> > At this stage of system shutdown procedure the jiffies may
> > not be updated anymore, so have to base on raw sched_clock
> > values.
> >
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > Cc: Anton Vorontsov <anton@enomsg.org>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > ---
> >  drivers/power/reset/vexpress-poweroff.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/power/reset/vexpress-poweroff.c b/drivers/power/reset/vexpress-poweroff.c
> > index 476aa49..d752233 100644
> > --- a/drivers/power/reset/vexpress-poweroff.c
> > +++ b/drivers/power/reset/vexpress-poweroff.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/sched.h>
> >  #include <linux/stat.h>
> >  #include <linux/vexpress.h>
> >
> > @@ -27,12 +28,12 @@ static void vexpress_reset_do(struct device *dev, const char *what)
> >                         vexpress_config_func_get_by_dev(dev);
> >
> >         if (func) {
> > -               unsigned long timeout;
> > +               unsigned long long timeout_ns;
> >
> >                 err = vexpress_config_write(func, 0, 0);
> >
> > -               timeout = jiffies + HZ;
> > -               while (time_before(jiffies, timeout))
> > +               timeout_ns = sched_clock() + 50000000;
> > +               while (!err && time_before64(sched_clock(), timeout_ns))
> >                         cpu_relax();
> >         }
> 
> So this may not be a problem in this particular case, but sched_clock
> could be backed by jiffies on some hardware, causing the same problem
> to appear.

Uh, right. As you guessed, on vexpress sched_clock, once registered, is
always available. But of course I shouldn't fully rely on this.

> Might udelay/mdelay be a better fit for this sort of case (since
> udelay may be counter backed, but may also be loop backed on hardware
> without continuous counters)?

I'm sure I though about udelay, but for some reason decided against it.
The reason may be already invalid, so I'll check it again :-)

Thanks!

Pawe?

[RFC 12/18] clk: versatile: Split config options for sp810 and vexpress_osc

[Pawel Moll]

On Mon, 2013-12-23 at 20:05 +0000, Mike Turquette wrote:
> Quoting Pawel Moll (2013-12-23 08:23:44)
> > Move the Kconfig entry for Versatile (& Express) clock drivers
> > into a separate file and add individual options for sp810
> > and vexpress_osc drivers, as they are optional in some
> > configurations and may have separate dependencies.
> > 
> > Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mike Turquette <mturquette@linaro.org>
> 
> Is there a reason to continue to expose these as user-visible config
> options? I guess that with Device Tree we can just include clock nodes
> (or not) for boards that use these clocks (or do not use them).
>
> And it seems for many configurations COMMON_CLK_VERSATILE is always
> selected, so what is the point of the other two sub-options?

It's mostly for arm64, where all the bits and pieces are individually
selected (and SP810 is not present in some vexpress configurations).

Pawe?

[RFC 18/18] mfd: vexpress: Split sysreg functions into MFD cells

[Pawel Moll]

On Mon, 2014-01-06 at 10:40 +0000, Lee Jones wrote:
> > +static struct mfd_cell vexpress_sysreg_cells[] = {
> > +	{
> > +		.name = "syscon",
> > +		.of_compatible = "arm,vexpress-sysreg,sys_id",
> > +		.num_resources = 1,
> > +		.resources = (struct resource []) {
> > +			DEFINE_RES_MEM(SYS_ID, 0x4),
> > +		},
> > +		.platform_data = &vexpress_sysreg_sys_id_pdata,
> > +		.pdata_size = sizeof(vexpress_sysreg_sys_id_pdata),
> 
> Not sure how comfortable I am with using Device Tree and populating
> platform_data with information which by the looks of it you're
> planning to make persistent. What's stopping you from using a DT
> property to name the block, or better yet, just use the compatible
> string? Also, won't naming these blocks in DT also aid other OSes?

Right. This particular of_compatible string in the *_cells is a leftover
from previous versions of the patch. Only the gpio-related ones, as
mentioned in the example:

>  Example:
>         v2m_sysreg: sysreg at 10000000 {
>                 compatible = "arm,vexpress-sysreg";
>                 reg = <0x10000000 0x1000>;
> -               gpio-controller;
> -               #gpio-cells = <2>;
> +
> +               v2m_led_gpios: sys_led at 08 {
> +                       compatible = "arm,vexpress-sysreg,sys_led";
> +                       gpio-controller;
> +                       #gpio-cells = <2>;
> +               };
> +
> +               v2m_mmc_gpios: sys_mci at 48 {
> +                       compatible = "arm,vexpress-sysreg,sys_mci";
> +                       gpio-controller;
> +                       #gpio-cells = <2>;
> +               };
> +
> +               v2m_flash_gpios: sys_flash at 4c {
> +                       compatible = "arm,vexpress-sysreg,sys_flash";
> +                       gpio-controller;
> +                       #gpio-cells = <2>;
> +               };
>         };

are supposed for the sake the gpio users (and some of them don't exist
in some version of the sysregs).

> > +	return mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO,
> > +			vexpress_sysreg_cells,
> > +			ARRAY_SIZE(vexpress_sysreg_cells), mem, 0, 0);
> 
> Don't use 0 as NULL, you will cause a sparse error.

... and rightly so! ;-) Well spotted, thanks!

Pawe?

[RFC 04/18] driver core & of: Mark of_nodes of added device as populated

[Rob Herring]

On Mon, Dec 23, 2013 at 10:23 AM, Pawel Moll <pawel.moll@arm.com> wrote:
> In "Device Tree powered" systems, platform devices are usually
> massively populated with of_platform_populate() call, executed
> at some level of initcalls, either by generic architecture
> or by platform-specific code.
>
> There are situations though where certain devices must be
> created (and bound with drivers) before all the others.
> This presents small challenge in DT-driven systems, as
> devices explicitly created in early code would be created
> again by of_platform_populate().

Isn't this already at least partially solved with the aux data
support? I'm guessing the difference here is how the early device is
created.

> This patch tries to solve that issue in a generic way,
> adding a "populated" flag which is set in the device_node
> structure when a device is being created in the core.
> Later, of_platform_populate() skips such nodes (and
> its children) in a similar way to the non-available ones.

Couldn't you store a struct device ptr in struct device_node instead?

In any case I'd like to see this contained within the DT code. I don't
see why the driver core needs to be modified for a DT specific
problem.

Rob

[RFC 07/18] regmap: debugfs: Always create "registers" & "access" files

[Pawel Moll]

On Tue, 2013-12-24 at 12:19 +0000, Mark Brown wrote:
> On Mon, Dec 23, 2013 at 04:23:39PM +0000, Pawel Moll wrote:
> > When a map covers a single register, max_register is equal
> > to 0, so the "registers" & "access" files were not created.
> > 
> > This patch is removing this restriction. It should be save,
> > as the maps not without register 0 should return false
> > for regmap_readable(map, 0).
> 
> No, we need to find a better way of doing this such as checking to see
> if register zero is accessible in some way.  We want to avoid having
> those files if there's no way of populating them since this is useful
> information for tooling.

Something like this then?

diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
index c5471cd..45d812c 100644
--- a/drivers/base/regmap/regmap-debugfs.c
+++ b/drivers/base/regmap/regmap-debugfs.c
@@ -511,7 +511,7 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
 	debugfs_create_file("range", 0400, map->debugfs,
 			    map, &regmap_reg_ranges_fops);
 
-	if (map->max_register) {
+	if (map->max_register || regmap_readable(map, 0)) {
 		debugfs_create_file("registers", 0400, map->debugfs,
 				    map, &regmap_map_fops);
 		debugfs_create_file("access", 0400, map->debugfs,

Other possibility would be replacing max_register with registers_num,
but it looks a massive job...

Pawe?

[RFC 07/18] regmap: debugfs: Always create "registers" & "access" files

[Mark Brown]

On Wed, Jan 08, 2014 at 05:31:37PM +0000, Pawel Moll wrote:
> On Tue, 2013-12-24 at 12:19 +0000, Mark Brown wrote:

> > No, we need to find a better way of doing this such as checking to see
> > if register zero is accessible in some way.  We want to avoid having
> > those files if there's no way of populating them since this is useful
> > information for tooling.

> Something like this then?

> diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c
> index c5471cd..45d812c 100644
> --- a/drivers/base/regmap/regmap-debugfs.c
> +++ b/drivers/base/regmap/regmap-debugfs.c
> @@ -511,7 +511,7 @@ void regmap_debugfs_init(struct regmap *map, const char *name)
>  	debugfs_create_file("range", 0400, map->debugfs,
>  			    map, &regmap_reg_ranges_fops);
>  
> -	if (map->max_register) {
> +	if (map->max_register || regmap_readable(map, 0)) {

Yes.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140108/92af00ff/attachment.sig>

[RFC 10/18] regulator: vexpress: Use regmap instead of custom interface

[Pawel Moll]

On Tue, 2013-12-24 at 12:24 +0000, Mark Brown wrote:
> On Mon, Dec 23, 2013 at 04:23:42PM +0000, Pawel Moll wrote:
> 
> > This patch makes the Versatile Express regulator driver
> > use regmap interface, instead of custom vexpress config
> > one. It will request the regmap resource associated
> > with the device, which makes it pretty much hardware
> > agnostic.
> 
> If this change is making the operations hardware agnostic regmap based
> ones then they should be being either replaced with standard operations
> or standard operations being added.  It looks like you should be able to
> use the existing linear map operations.

Bad wording on my side. What I meant was: it doesn't matter what wacky
VE-specific interface is hidden behind the regmap, not it's a generic
regulator driver.

Having said that, I could create helpers for continuous (linear won't
cut - if you remember the continuous_voltage_range was introduced for
VE's sake) regulator, which would simply read/write value to a specified
register. Not sure how useful would that be, though. I doubt that such
"regulator" will appear anywhere outside VE world. And if so, this would
probably call for a "generic continuous regulator" driver and a relevant
DT binding, which I don't think anyone really wants. If your opinion
differs, I'm happy to botch something up.

Cheers!

Pawe?

[RFC 06/18] regmap: Formalise use of non-bus context

[Pawel Moll]

On Tue, 2013-12-24 at 12:45 +0000, Mark Brown wrote:
> I'd note that I wasn't CCed on most of this series so I'm not entirely
> sure what it's trying to do.

Apologies. The series is quite long and I didn't want to bother too many
people with mostly irrelevant changes. Will copy you on the whole thing
next time.

> > Bus-less maps (ones with reg_read and reg_write functions
> > defined in regmap_config) were given the context passed
> > in regmap_init(), but it was still called "bus_context".
> > 
> > This patch formalises this aspect by renaming it to simple
> > "context" and adds the missing link, free_context function
> > in regmap_config, which allows bus-less maps to use the
> > context in classic way.
> 
> This should be two patches, one patch to do the rename and one to add
> the operation.  

Sure, will do.

> The obvious question here is why is this callback useful
> - what is being allocated in a regmap specific context that needs to be
> lifetime managed separately to the thing doing the creation?  I can't
> see any obvious reason why this would ever get used.

First of all, it's just a generalization of the free_context already
existing in regmap_bus (and used by regmap-mmio). And in case of this
series it is being used to release extra resource added allocated for a
"busless" regmap_config. Briefly, I'm using devm_regmap_init() to
"attach" a custom regmap configuration to a device when it is being
created (which is then dev_get_regmap()-ed in the driver, as you saw in
the regulator patch) and its context is a pointer to kzallocated data.
free_context is used to release it when devm resource is being removed.

Does it make any sense?

Pawe?

[RFC 06/18] regmap: Formalise use of non-bus context

[Mark Brown]

On Thu, Jan 09, 2014 at 01:08:31PM +0000, Pawel Moll wrote:
> On Tue, 2013-12-24 at 12:45 +0000, Mark Brown wrote:

> > The obvious question here is why is this callback useful
> > - what is being allocated in a regmap specific context that needs to be
> > lifetime managed separately to the thing doing the creation?  I can't
> > see any obvious reason why this would ever get used.

> First of all, it's just a generalization of the free_context already
> existing in regmap_bus (and used by regmap-mmio). And in case of this
> series it is being used to release extra resource added allocated for a
> "busless" regmap_config. Briefly, I'm using devm_regmap_init() to
> "attach" a custom regmap configuration to a device when it is being
> created (which is then dev_get_regmap()-ed in the driver, as you saw in
> the regulator patch) and its context is a pointer to kzallocated data.
> free_context is used to release it when devm resource is being removed.

> Does it make any sense?

To be honest not really - the above sounds like you should've allocated
the memory using devm_kzalloc() or just embedding the allocated data in
the driver data for the parent.  Internal things need to clean up after
themselves but users should already have a larger context of some kind.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140109/dd74d83e/attachment.sig>

[RFC 10/18] regulator: vexpress: Use regmap instead of custom interface

[Mark Brown]

On Wed, Jan 08, 2014 at 06:25:31PM +0000, Pawel Moll wrote:
> On Tue, 2013-12-24 at 12:24 +0000, Mark Brown wrote:

> > If this change is making the operations hardware agnostic regmap based
> > ones then they should be being either replaced with standard operations
> > or standard operations being added.  It looks like you should be able to
> > use the existing linear map operations.

> Bad wording on my side. What I meant was: it doesn't matter what wacky
> VE-specific interface is hidden behind the regmap, not it's a generic
> regulator driver.

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

[RFC 06/18] regmap: Formalise use of non-bus context

[Pawel Moll]

On Thu, 2014-01-09 at 13:34 +0000, Mark Brown wrote:
> > First of all, it's just a generalization of the free_context already
> > existing in regmap_bus (and used by regmap-mmio). And in case of this
> > series it is being used to release extra resource added allocated for a
> > "busless" regmap_config. Briefly, I'm using devm_regmap_init() to
> > "attach" a custom regmap configuration to a device when it is being
> > created (which is then dev_get_regmap()-ed in the driver, as you saw in
> > the regulator patch) and its context is a pointer to kzallocated data.
> > free_context is used to release it when devm resource is being removed.
> 
> > Does it make any sense?
> 
> To be honest not really - the above sounds like you should've allocated
> the memory using devm_kzalloc() or just embedding the allocated data in
> the driver data for the parent.  Internal things need to clean up after
> themselves but users should already have a larger context of some kind.

I've started with regmap_bus so the free_context did fit well like in
regmap-mmio, but you're right - with the current approach I should be
able to simply use devm_kzalloc(). I'll give it a try and if it works,
I'll drop this patch completely.

Pawe?

[RFC 03/18] GPIO: gpio-generic: Add label to platform data

[Linus Walleij]

On Wed, Jan 8, 2014 at 4:57 PM, Pawel Moll <pawel.moll@arm.com> wrote:
> On Mon, 2013-12-23 at 17:26 +0000, Linus Walleij wrote:

>> Good point. But what about also adding device tree support for
>> naming the chips while you're at it?
>>
>> I imagine a generic gpiochip property in
>> Documentation/devicetree/bindings/gpio/gpio.txt
>
> Well, this has been discussed almost to death already (as you may
> remember ;-) in the thread:
>
> http://www.spinics.net/lists/devicetree/msg00072.html
>
> (only portions survived)
>
> Although I was in favour of the generic binding (as were you if I
> remember correctly), the final non-conclusion was not to open the
> "generic door". Fair enough with me, I went the defined-in-code way
> here...

OK I buy this. I am willing to apply this patch. Just a problem:

Content-Type: text/plain; charset=WINDOWS-1252
Content-Transfer-Encoding: quoted-printable

I can't apply this patch :-)

Please consult your fellow upstream developers on how to get
the mail out of the ARM offices in a form that I can apply...

Yours,
Linus Walleij

[RFC 03/18] GPIO: gpio-generic: Add label to platform data

[Pawel Moll]

On Tue, 2014-01-14 at 10:30 +0000, Linus Walleij wrote:
> Please consult your fellow upstream developers on how to get
> the mail out of the ARM offices in a form that I can apply...

They were exactly in the same position :-)

Fortunately we've got a Christmas gift of a normal SMTP server, so when
I re-post the series, it should be already fine.

Thanks!

Pawe?

[RFC 04/18] driver core & of: Mark of_nodes of added device as populated

[Grant Likely]

On Wed, 8 Jan 2014 11:28:01 -0600, Rob Herring <robherring2@gmail.com> wrote:
> On Mon, Dec 23, 2013 at 10:23 AM, Pawel Moll <pawel.moll@arm.com> wrote:
> > In "Device Tree powered" systems, platform devices are usually
> > massively populated with of_platform_populate() call, executed
> > at some level of initcalls, either by generic architecture
> > or by platform-specific code.
> >
> > There are situations though where certain devices must be
> > created (and bound with drivers) before all the others.
> > This presents small challenge in DT-driven systems, as
> > devices explicitly created in early code would be created
> > again by of_platform_populate().
> 
> Isn't this already at least partially solved with the aux data
> support? I'm guessing the difference here is how the early device is
> created.

Still, creating a device early and then trying to remember that it has
been done does nothing but add complexity for a very small number of use
cases. I still would much rather see things that need really early setup
to avoid the device model entirely and do the bare minimum needed to
allow the kernel to get to initcall time. In the cases where a subsystem
API requires a struct device, I would consider using a dummy throwaway
struct device for the early bits.

g.

[RFC 06/18] regmap: Formalise use of non-bus context

[Grant Likely]

On Thu, 09 Jan 2014 13:08:31 +0000, Pawel Moll <pawel.moll@arm.com> wrote:
> On Tue, 2013-12-24 at 12:45 +0000, Mark Brown wrote:
> > I'd note that I wasn't CCed on most of this series so I'm not entirely
> > sure what it's trying to do.
> 
> Apologies. The series is quite long and I didn't want to bother too many
> people with mostly irrelevant changes. Will copy you on the whole thing
> next time.
> 
> > > Bus-less maps (ones with reg_read and reg_write functions
> > > defined in regmap_config) were given the context passed
> > > in regmap_init(), but it was still called "bus_context".
> > > 
> > > This patch formalises this aspect by renaming it to simple
> > > "context" and adds the missing link, free_context function
> > > in regmap_config, which allows bus-less maps to use the
> > > context in classic way.
> > 
> > This should be two patches, one patch to do the rename and one to add
> > the operation.  
> 
> Sure, will do.
> 
> > The obvious question here is why is this callback useful
> > - what is being allocated in a regmap specific context that needs to be
> > lifetime managed separately to the thing doing the creation?  I can't
> > see any obvious reason why this would ever get used.
> 
> First of all, it's just a generalization of the free_context already
> existing in regmap_bus (and used by regmap-mmio). And in case of this
> series it is being used to release extra resource added allocated for a
> "busless" regmap_config. Briefly, I'm using devm_regmap_init() to
> "attach" a custom regmap configuration to a device when it is being
> created (which is then dev_get_regmap()-ed in the driver, as you saw in
> the regulator patch) and its context is a pointer to kzallocated data.
> free_context is used to release it when devm resource is being removed.

Have you thought through all the implications here? What you've
described effectively changes the devm model. devm operates under the
assumption that devm data only exists between probe() and remove() time.
If you 'preload' devm data then the preloaded data will get discarded at
remove() time which breaks if the driver is remove and probed again at
runtime.

g.

[RFC 06/18] regmap: Formalise use of non-bus context

[Pawel Moll]

On Thu, 2014-01-16 at 17:09 +0000, Grant Likely wrote:
> > First of all, it's just a generalization of the free_context already
> > existing in regmap_bus (and used by regmap-mmio). And in case of this
> > series it is being used to release extra resource added allocated for a
> > "busless" regmap_config. Briefly, I'm using devm_regmap_init() to
> > "attach" a custom regmap configuration to a device when it is being
> > created (which is then dev_get_regmap()-ed in the driver, as you saw in
> > the regulator patch) and its context is a pointer to kzallocated data.
> > free_context is used to release it when devm resource is being removed.
> 
> Have you thought through all the implications here? What you've
> described effectively changes the devm model. devm operates under the
> assumption that devm data only exists between probe() and remove() time.
> If you 'preload' devm data then the preloaded data will get discarded at
> remove() time which breaks if the driver is remove and probed again at
> runtime.

Uh. Right, you're correct, I've missed that (obvious) fact :-(

I will get the drivers back to "vexpress_regmap_init()" model or try to
find a way of "attaching" a regmap pointer(s?) to a struct device. If it
makes sense at all...

Pawe?