lkml.org 
[lkml]   [2014]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] mfd: syscon: Support early initialization
Hi Michal,

First of all, even though not touching DT bindings, this change is DT
related and so devicetree mailing list should be on Cc, as well as some
DT people, especially Grant. Adding them.

Otherwise, please see my comments inline.

On 08.04.2014 17:00, Michal Simek wrote:
> Some platforms need to get system controller
> ready as soon as possible.
> The patch provides early_syscon_initialization
> which create early mapping for all syscon compatible
> devices in early_syscon_probe.
> Regmap is get via syscon_early_regmap_lookup_by_phandle()
>
> Regular device probes attach device to regmap
> via regmap_attach_dev().
>
> For early syscon initialization is necessary to extend
> struct syscon and provide remove function
> which unmap all early init structures.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> Changes in v3:
> - Keep backward compatibility for platform drivers and test it
> - Use only one probe method which is early_syscon_probe
> suggested by Lee Jones. Do not force anybody to call early_syscon_init
> - Add kernel-doc description
>
> Changes in v2:
> - Fix bad logic in early_syscon_probe
> - Fix compilation failure for x86_64 reported by zero day testing system
> - Regmap change available here
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git tags/nodev
>
> drivers/mfd/syscon.c | 159 ++++++++++++++++++++++++++++++++++++++++-----
> include/linux/mfd/syscon.h | 11 ++++
> 2 files changed, 153 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
> index 71841f9..8e2ff88 100644
> --- a/drivers/mfd/syscon.c
> +++ b/drivers/mfd/syscon.c
> @@ -20,12 +20,15 @@
> #include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> +#include <linux/slab.h>
> #include <linux/mfd/syscon.h>
>
> static struct platform_driver syscon_driver;
>
> struct syscon {
> + void __iomem *base;
> struct regmap *regmap;
> + struct resource res;
> };
>
> static int syscon_match_node(struct device *dev, void *data)
> @@ -95,6 +98,30 @@ struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
> }
> EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_pdevname);
>
> +/**
> + * syscon_early_regmap_lookup_by_phandle - Early phandle lookup function
> + * @np: device_node pointer
> + * @property: property name which handle system controller phandle
> + * Return: regmap pointer, an error pointer otherwise
> + */
> +struct regmap *syscon_early_regmap_lookup_by_phandle(struct device_node *np,
> + const char *property)
> +{
> + struct device_node *syscon_np;
> + struct syscon *syscon;
> +
> + syscon_np = of_parse_phandle(np, property, 0);
> + if (!syscon_np)
> + return ERR_PTR(-ENODEV);
> +
> + syscon = syscon_np->data;
> +
> + of_node_put(syscon_np);

As it was mentioned in other review comments, this is rather fishy.

Instead, couldn't you create a local list of system controllers
available in the system and then use that to perform look-up? This could
also let you eliminate the need to have separate functions for early and
normal look-up as the list could be simply used for both cases. Of
course you would have to add of_node and list_head fields to struct
syscon, but I don't think that matters.

As a side effect, the look-up would not need to iterate over all devices
in the system anymore, just over the list of registered system controllers.

Best regards,
Tomasz


\
 
 \ /
  Last update: 2014-04-26 00:01    [W:0.141 / U:7.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site