Hi,
On 10.05.2014 08:56, Pankaj Dubey wrote:
> From: Young-Gun Jang <[email protected]>
>
> Add support for mapping Samsung Power Management Unit (PMU)
> base address from device tree. This patch also adds helper
> function as "get_exynos_pmuregmap". This function can be used
> by other machine files such as "pm.c", "hotplug.c" for accessing
> PMU regmap handle.
>
I don't think there is a need to use regmap to provide access to PMU to
such low level code such as pm.c or hotplug.c. Moreover, I believe that
it might be undesirable in some cases, e.g. very low level code running
at early resume or late suspend.
IMHO, based on what we now have for SYSRAM, you could simply map PMU
from device tree one time, before SMP init, and keep the address in some
globally accessible variable, like those for SYSRAM we have right now
(sysram_base_addr, sysram_ns_base_addr -> pmu_base_addr).
Then, registration of the normal syscon would happen through standard
platform driver mechanisms and no special early handling would be necessary.
Best regards,
Tomasz
Hi Tomasz,
> Hi,
>
> On 10.05.2014 08:56, Pankaj Dubey wrote:
> > From: Young-Gun Jang <[email protected]>
> >
> > Add support for mapping Samsung Power Management Unit (PMU) base
> > address from device tree. This patch also adds helper function as
> > "get_exynos_pmuregmap". This function can be used by other machine
> > files such as "pm.c", "hotplug.c" for accessing PMU regmap handle.
> >
>
> I don't think there is a need to use regmap to provide access to PMU to
such low
> level code such as pm.c or hotplug.c. Moreover, I believe that it might be
undesirable
> in some cases, e.g. very low level code running at early resume or late
suspend.
>
> IMHO, based on what we now have for SYSRAM, you could simply map PMU from
> device tree one time, before SMP init, and keep the address in some
globally
> accessible variable, like those for SYSRAM we have right now
(sysram_base_addr,
> sysram_ns_base_addr -> pmu_base_addr).
>
Thanks for review.
Well I adopted same approach in V1 of this patch series.
V1: https://lkml.org/lkml/2014/4/2/48
So, if we do not have issues with that approach, I think we can map PMU
address
one time and use it for all machine files including pmu.c.
Also I can see that early_syscon patch [1] is not progressing anymore,
so in next version of this series better I remove dependency of early syscon
and usage
of regmap.
1: https://lkml.org/lkml/2014/4/8/239
Tomasz, It will be good if you can review remaining patches under this
series, specially patch [2].
So that, I can update this series after addressing all comments.
2: https://lkml.org/lkml/2014/5/10/26
> Then, registration of the normal syscon would happen through standard
platform
> driver mechanisms and no special early handling would be necessary.
>
> Best regards,
> Tomasz
Thanks,
Pankaj Dubey
Hi Pankaj,
On 17.06.2014 08:43, Pankaj Dubey wrote:
> Hi Tomasz,
>
>> Hi,
>>
>> On 10.05.2014 08:56, Pankaj Dubey wrote:
>>> From: Young-Gun Jang <[email protected]>
>>>
>>> Add support for mapping Samsung Power Management Unit (PMU) base
>>> address from device tree. This patch also adds helper function as
>>> "get_exynos_pmuregmap". This function can be used by other machine
>>> files such as "pm.c", "hotplug.c" for accessing PMU regmap handle.
>>>
>>
>> I don't think there is a need to use regmap to provide access to PMU to
> such low
>> level code such as pm.c or hotplug.c. Moreover, I believe that it might be
> undesirable
>> in some cases, e.g. very low level code running at early resume or late
> suspend.
>>
>> IMHO, based on what we now have for SYSRAM, you could simply map PMU from
>> device tree one time, before SMP init, and keep the address in some
> globally
>> accessible variable, like those for SYSRAM we have right now
> (sysram_base_addr,
>> sysram_ns_base_addr -> pmu_base_addr).
>>
>
> Thanks for review.
>
> Well I adopted same approach in V1 of this patch series.
>
> V1: https://lkml.org/lkml/2014/4/2/48
>
> So, if we do not have issues with that approach, I think we can map PMU
> address
> one time and use it for all machine files including pmu.c.
The approach itself is fine, but I believe there is no reason to use fdt
there. My recommendation is to follow the method used to map SYSRAMs in
patch "b3205dea8f ARM: EXYNOS: Map SYSRAM through generic DT bindings"
and taking into account patch "b87abf7deb ARM: exynos: move sysram info
to exynos.c", which moves things around source files.
> Also I can see that early_syscon patch [1] is not progressing anymore,
> so in next version of this series better I remove dependency of early syscon
> and usage
> of regmap.
I have another proposal, basically something I already proposed in
review of one of previous versions of this series. I will send a patch
as a reply to this message.
>
> 1: https://lkml.org/lkml/2014/4/8/239
>
> Tomasz, It will be good if you can review remaining patches under this
> series, specially patch [2].
> So that, I can update this series after addressing all comments.
>
> 2: https://lkml.org/lkml/2014/5/10/26
>
Most of the patches have already received my reviewed-by tag. I'm
generally hesitating to review remaining ones, because the general
architecture will be quite different after changing things mentioned
above. However let me see and try to point issues I can find.
Best regards,
Tomasz
Currently a syscon entity can be only registered directly through a
platform device that binds to a dedicated driver. However in certain use
cases it is desirable to make a device used with another driver a syscon
interface provider. For example, certain SoCs (e.g. Exynos) contain
system controller blocks which perform various functions such as power
domain control, CPU power management, low power mode control, but in
addition contain certain IP integration glue, such as various signal
masks, coprocessor power control, etc. In such case, there is a need to
have a dedicated driver for such system controller but also share
registers with other drivers. The latter is where the syscon interface
is helpful.
This patch decouples syscon object from syscon driver, so that it can be
registered from any driver in addition to the original "syscon" platform
driver.
Signed-off-by: Tomasz Figa <[email protected]>
---
drivers/mfd/syscon.c | 102 +++++++++++++++++++++++++++++----------------
include/linux/mfd/syscon.h | 9 ++++
2 files changed, 75 insertions(+), 36 deletions(-)
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c
index ca15878..11baaae 100644
--- a/drivers/mfd/syscon.c
+++ b/drivers/mfd/syscon.c
@@ -14,6 +14,7 @@
#include <linux/err.h>
#include <linux/io.h>
+#include <linux/list.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_address.h>
@@ -23,30 +24,28 @@
#include <linux/regmap.h>
#include <linux/mfd/syscon.h>
-static struct platform_driver syscon_driver;
+static DEFINE_SPINLOCK(syscon_list_slock);
+static LIST_HEAD(syscon_list);
struct syscon {
struct regmap *regmap;
+ struct device *dev;
+ struct list_head list;
};
-static int syscon_match_node(struct device *dev, void *data)
-{
- struct device_node *dn = data;
-
- return (dev->of_node == dn) ? 1 : 0;
-}
-
struct regmap *syscon_node_to_regmap(struct device_node *np)
{
- struct syscon *syscon;
- struct device *dev;
+ struct syscon *entry, *syscon = ERR_PTR(-EPROBE_DEFER);
+
+ spin_lock(&syscon_list_slock);
- dev = driver_find_device(&syscon_driver.driver, NULL, np,
- syscon_match_node);
- if (!dev)
- return ERR_PTR(-EPROBE_DEFER);
+ list_for_each_entry(entry, &syscon_list, list)
+ if (entry->dev->of_node == np) {
+ syscon = entry;
+ break;
+ }
- syscon = dev_get_drvdata(dev);
+ spin_unlock(&syscon_list_slock);
return syscon->regmap;
}
@@ -68,22 +67,19 @@ struct regmap *syscon_regmap_lookup_by_compatible(const char *s)
}
EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_compatible);
-static int syscon_match_pdevname(struct device *dev, void *data)
-{
- return !strcmp(dev_name(dev), (const char *)data);
-}
-
struct regmap *syscon_regmap_lookup_by_pdevname(const char *s)
{
- struct device *dev;
- struct syscon *syscon;
+ struct syscon *entry, *syscon = ERR_PTR(-EPROBE_DEFER);
+
+ spin_lock(&syscon_list_slock);
- dev = driver_find_device(&syscon_driver.driver, NULL, (void *)s,
- syscon_match_pdevname);
- if (!dev)
- return ERR_PTR(-EPROBE_DEFER);
+ list_for_each_entry(entry, &syscon_list, list)
+ if (!strcmp(dev_name(entry->dev), s)) {
+ syscon = entry;
+ break;
+ }
- syscon = dev_get_drvdata(dev);
+ spin_unlock(&syscon_list_slock);
return syscon->regmap;
}
@@ -121,17 +117,49 @@ static struct regmap_config syscon_regmap_config = {
.reg_stride = 4,
};
+static void devm_syscon_unregister(struct device *dev, void *res)
+{
+ struct syscon *syscon = *(struct syscon **)res;
+
+ spin_lock(&syscon_list_slock);
+ list_del(&syscon->list);
+ spin_unlock(&syscon_list_slock);
+}
+
+int devm_syscon_register(struct device *dev, struct regmap *regmap)
+{
+ struct syscon **ptr, *syscon;
+
+ syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
+ if (!syscon)
+ return -ENOMEM;
+
+ syscon->regmap = regmap;
+ syscon->dev = dev;
+
+ ptr = devres_alloc(devm_syscon_unregister, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return -ENOMEM;
+
+ spin_lock(&syscon_list_slock);
+ list_add_tail(&syscon->list, &syscon_list);
+ spin_unlock(&syscon_list_slock);
+
+ *ptr = syscon;
+ devres_add(dev, ptr);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(devm_syscon_register);
+
static int syscon_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct syscon_platform_data *pdata = dev_get_platdata(dev);
- struct syscon *syscon;
+ struct regmap *regmap;
struct resource *res;
void __iomem *base;
-
- syscon = devm_kzalloc(dev, sizeof(*syscon), GFP_KERNEL);
- if (!syscon)
- return -ENOMEM;
+ int ret;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
@@ -144,14 +172,16 @@ static int syscon_probe(struct platform_device *pdev)
syscon_regmap_config.max_register = res->end - res->start - 3;
if (pdata)
syscon_regmap_config.name = pdata->label;
- syscon->regmap = devm_regmap_init_mmio(dev, base,
+ regmap = devm_regmap_init_mmio(dev, base,
&syscon_regmap_config);
- if (IS_ERR(syscon->regmap)) {
+ if (IS_ERR(regmap)) {
dev_err(dev, "regmap init failed\n");
- return PTR_ERR(syscon->regmap);
+ return PTR_ERR(regmap);
}
- platform_set_drvdata(pdev, syscon);
+ ret = devm_syscon_register(dev, regmap);
+ if (ret)
+ return ret;
dev_dbg(dev, "regmap %pR registered\n", res);
diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h
index 75e543b..6a1cadf 100644
--- a/include/linux/mfd/syscon.h
+++ b/include/linux/mfd/syscon.h
@@ -18,8 +18,11 @@
#include <linux/err.h>
struct device_node;
+struct regmap;
#ifdef CONFIG_MFD_SYSCON
+extern int devm_syscon_register(struct device *dev, struct regmap *regmap);
+
extern struct regmap *syscon_node_to_regmap(struct device_node *np);
extern struct regmap *syscon_regmap_lookup_by_compatible(const char *s);
extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s);
@@ -27,6 +30,12 @@ extern struct regmap *syscon_regmap_lookup_by_phandle(
struct device_node *np,
const char *property);
#else
+static inline int devm_syscon_register(struct device *dev,
+ struct regmap *regmap)
+{
+ return -ENOSYS;
+}
+
static inline struct regmap *syscon_node_to_regmap(struct device_node *np)
{
return ERR_PTR(-ENOSYS);
--
1.9.3
On Tuesday 17 June 2014 17:32:44 Tomasz Figa wrote:
> Currently a syscon entity can be only registered directly through a
> platform device that binds to a dedicated driver. However in certain use
> cases it is desirable to make a device used with another driver a syscon
> interface provider. For example, certain SoCs (e.g. Exynos) contain
> system controller blocks which perform various functions such as power
> domain control, CPU power management, low power mode control, but in
> addition contain certain IP integration glue, such as various signal
> masks, coprocessor power control, etc. In such case, there is a need to
> have a dedicated driver for such system controller but also share
> registers with other drivers. The latter is where the syscon interface
> is helpful.
>
> This patch decouples syscon object from syscon driver, so that it can be
> registered from any driver in addition to the original "syscon" platform
> driver.
>
> Signed-off-by: Tomasz Figa <[email protected]>
Hi Tomasz,
This seems like a reasonable way of solving the problem, but I think
there is an even better one that we have about in the past: if we
promote syscon from a platform driver into a a drivers/base/ helper
that is independent of the platform device matching, we can use
call syscon_regmap_lookup_* for any device node, whether it's already
bound to a driver or not, which do what you need. It would also make
it easier to call the syscon code before the platform_device
infrastructure gets initialized, which is something a number of
people have asked for, e.g. for using regmap to do SMP bringup
or for clock registration.
Arnd
Hi Arnd,
On 17.06.2014 17:42, Arnd Bergmann wrote:
> On Tuesday 17 June 2014 17:32:44 Tomasz Figa wrote:
>> Currently a syscon entity can be only registered directly through a
>> platform device that binds to a dedicated driver. However in certain use
>> cases it is desirable to make a device used with another driver a syscon
>> interface provider. For example, certain SoCs (e.g. Exynos) contain
>> system controller blocks which perform various functions such as power
>> domain control, CPU power management, low power mode control, but in
>> addition contain certain IP integration glue, such as various signal
>> masks, coprocessor power control, etc. In such case, there is a need to
>> have a dedicated driver for such system controller but also share
>> registers with other drivers. The latter is where the syscon interface
>> is helpful.
>>
>> This patch decouples syscon object from syscon driver, so that it can be
>> registered from any driver in addition to the original "syscon" platform
>> driver.
>>
>> Signed-off-by: Tomasz Figa <[email protected]>
>
> Hi Tomasz,
>
> This seems like a reasonable way of solving the problem, but I think
> there is an even better one that we have about in the past: if we
> promote syscon from a platform driver into a a drivers/base/ helper
> that is independent of the platform device matching, we can use
> call syscon_regmap_lookup_* for any device node, whether it's already
> bound to a driver or not, which do what you need. It would also make
> it easier to call the syscon code before the platform_device
> infrastructure gets initialized, which is something a number of
> people have asked for, e.g. for using regmap to do SMP bringup
> or for clock registration.
Basically, unless I'm missing your point, this is what my patch does,
except that I don't move it to drivers/base/ and the registration
function I added require a pointer to struct device. Indeed, decoupling
it further from the driver model, by adding of_syscon_register() should
be useful for early users.
Should I move this to drivers/base/, even though from current location
it can be used outside the platform driver anyway?
Best regards,
Tomasz
On Tue, 17 Jun 2014, Tomasz Figa wrote:
> On 17.06.2014 17:42, Arnd Bergmann wrote:
> > On Tuesday 17 June 2014 17:32:44 Tomasz Figa wrote:
> >> Currently a syscon entity can be only registered directly through a
> >> platform device that binds to a dedicated driver. However in certain use
> >> cases it is desirable to make a device used with another driver a syscon
> >> interface provider. For example, certain SoCs (e.g. Exynos) contain
> >> system controller blocks which perform various functions such as power
> >> domain control, CPU power management, low power mode control, but in
> >> addition contain certain IP integration glue, such as various signal
> >> masks, coprocessor power control, etc. In such case, there is a need to
> >> have a dedicated driver for such system controller but also share
> >> registers with other drivers. The latter is where the syscon interface
> >> is helpful.
> >>
> >> This patch decouples syscon object from syscon driver, so that it can be
> >> registered from any driver in addition to the original "syscon" platform
> >> driver.
+1 for this approach.
Michal, Pankay,
Does it also suit your needs?
> >> Signed-off-by: Tomasz Figa <[email protected]>
> >
> > Hi Tomasz,
> >
> > This seems like a reasonable way of solving the problem, but I think
> > there is an even better one that we have about in the past: if we
> > promote syscon from a platform driver into a a drivers/base/ helper
> > that is independent of the platform device matching, we can use
> > call syscon_regmap_lookup_* for any device node, whether it's already
> > bound to a driver or not, which do what you need. It would also make
> > it easier to call the syscon code before the platform_device
> > infrastructure gets initialized, which is something a number of
> > people have asked for, e.g. for using regmap to do SMP bringup
> > or for clock registration.
>
> Basically, unless I'm missing your point, this is what my patch does,
> except that I don't move it to drivers/base/ and the registration
> function I added require a pointer to struct device. Indeed, decoupling
> it further from the driver model, by adding of_syscon_register() should
> be useful for early users.
If agreed by Arnd, I think this work can be completed as a subsequent
patch.
> Should I move this to drivers/base/, even though from current location
> it can be used outside the platform driver anyway?
If I'm being honest with myself, I'd say that Syscon wasn't really an
MFD driver. I'm happy to keep it in there any continue maintaining
it, but wouldn't block a move either.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Tuesday 17 June 2014 23:26:22 Tomasz Figa wrote:
> On 17.06.2014 17:42, Arnd Bergmann wrote:
> > This seems like a reasonable way of solving the problem, but I think
> > there is an even better one that we have about in the past: if we
> > promote syscon from a platform driver into a a drivers/base/ helper
> > that is independent of the platform device matching, we can use
> > call syscon_regmap_lookup_* for any device node, whether it's already
> > bound to a driver or not, which do what you need. It would also make
> > it easier to call the syscon code before the platform_device
> > infrastructure gets initialized, which is something a number of
> > people have asked for, e.g. for using regmap to do SMP bringup
> > or for clock registration.
>
> Basically, unless I'm missing your point, this is what my patch does,
> except that I don't move it to drivers/base/ and the registration
> function I added require a pointer to struct device. Indeed, decoupling
> it further from the driver model, by adding of_syscon_register() should
> be useful for early users.
I believe the part you are missing is that with the approach I suggested,
there would be no registration function at all. You can easily do the
lookup from the client drivers using the DT data structures, with no
need for the device at all. The only exception today is the clps711x
platform using syscon_regmap_lookup_by_pdevname(), but that should be
solved in 3.17 when clps711x becomes DT-only.
> Should I move this to drivers/base/, even though from current location
> it can be used outside the platform driver anyway?
Thinking about it some more, drivers/of might be better than drivers/base.
It depends a bit where we are heading with this, in particular if we
expect to see non-DT users in the future.
Arnd
Hi,
On 06/18/2014 02:57 PM, Arnd Bergmann wrote:
> On Tuesday 17 June 2014 23:26:22 Tomasz Figa wrote:
>> On 17.06.2014 17:42, Arnd Bergmann wrote:
>>> This seems like a reasonable way of solving the problem, but I think
>>> there is an even better one that we have about in the past: if we
>>> promote syscon from a platform driver into a a drivers/base/ helper
>>> that is independent of the platform device matching, we can use
>>> call syscon_regmap_lookup_* for any device node, whether it's already
>>> bound to a driver or not, which do what you need. It would also make
>>> it easier to call the syscon code before the platform_device
>>> infrastructure gets initialized, which is something a number of
>>> people have asked for, e.g. for using regmap to do SMP bringup
>>> or for clock registration.
>>
>> Basically, unless I'm missing your point, this is what my patch does,
>> except that I don't move it to drivers/base/ and the registration
>> function I added require a pointer to struct device. Indeed, decoupling
>> it further from the driver model, by adding of_syscon_register() should
>> be useful for early users.
>
> I believe the part you are missing is that with the approach I suggested,
> there would be no registration function at all. You can easily do the
> lookup from the client drivers using the DT data structures, with no
> need for the device at all. The only exception today is the clps711x
> platform using syscon_regmap_lookup_by_pdevname(), but that should be
> solved in 3.17 when clps711x becomes DT-only.
I agree with Arnd. This patch is doing the part of what we wanted to do with it.
It means create a list of registered devices.
But you are working with struct device which you can't use in early phase
which is the feature we wanted to use. But currently we have workaround
in the kernel and clk subsystem is not able to work with regmap anyway.
Moving to proper location is just the last step.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: http://www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
Hi,
On Wednesday, June 18 2014, Lee Jones wrote:
> On Tue, 17 Jun 2014, Tomasz Figa wrote:
> > On 17.06.2014 17:42, Arnd Bergmann wrote:
> > > On Tuesday 17 June 2014 17:32:44 Tomasz Figa wrote:
> > >> Currently a syscon entity can be only registered directly through a
> > >> platform device that binds to a dedicated driver. However in
> > >> certain use cases it is desirable to make a device used with
> > >> another driver a syscon interface provider. For example, certain
> > >> SoCs (e.g. Exynos) contain system controller blocks which perform
> > >> various functions such as power domain control, CPU power
> > >> management, low power mode control, but in addition contain certain
> > >> IP integration glue, such as various signal masks, coprocessor
> > >> power control, etc. In such case, there is a need to have a
> > >> dedicated driver for such system controller but also share
> > >> registers with other drivers. The latter is where the syscon interface is helpful.
> > >>
> > >> This patch decouples syscon object from syscon driver, so that it
> > >> can be registered from any driver in addition to the original
> > >> "syscon" platform driver.
>
> +1 for this approach.
>
> Michal, Pankay,
> Does it also suit your needs?
>
Sorry for late reply.
I tested this patch after changing exynos PMU to be a syscon provider and it's working well.
So if we can address Arnd's comments, this patch will be helpful in making exynos PMU a
complete platform driver.
Thanks,
Pankaj Dubey
> > >> Signed-off-by: Tomasz Figa <[email protected]>
> > >
> > > Hi Tomasz,
> > >
> > > This seems like a reasonable way of solving the problem, but I think
> > > there is an even better one that we have about in the past: if we
> > > promote syscon from a platform driver into a a drivers/base/ helper
> > > that is independent of the platform device matching, we can use call
> > > syscon_regmap_lookup_* for any device node, whether it's already
> > > bound to a driver or not, which do what you need. It would also make
> > > it easier to call the syscon code before the platform_device
> > > infrastructure gets initialized, which is something a number of
> > > people have asked for, e.g. for using regmap to do SMP bringup or
> > > for clock registration.
> >
> > Basically, unless I'm missing your point, this is what my patch does,
> > except that I don't move it to drivers/base/ and the registration
> > function I added require a pointer to struct device. Indeed,
> > decoupling it further from the driver model, by adding
> > of_syscon_register() should be useful for early users.
>
> If agreed by Arnd, I think this work can be completed as a subsequent patch.
>
> > Should I move this to drivers/base/, even though from current location
> > it can be used outside the platform driver anyway?
>
> If I'm being honest with myself, I'd say that Syscon wasn't really an MFD driver. I'm
> happy to keep it in there any continue maintaining it, but wouldn't block a move
> either.
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software
> for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Hi Arnd,
On Wednesday, June 18, 2014 Arnd wrote:
> To: Tomasz Figa
> Cc: [email protected]; [email protected]; Tomasz
Figa;
> [email protected]; Kukjin Kim; Russell King - ARM Linux;
Samuel
> Ortiz; Pankaj Dubey; [email protected]; [email protected];
> [email protected]; [email protected]; Lee Jones
> Subject: Re: [PATCH RFC] mfd: syscon: Decouple syscon interface from
syscon
> devices
>
> On Tuesday 17 June 2014 23:26:22 Tomasz Figa wrote:
> > On 17.06.2014 17:42, Arnd Bergmann wrote:
> > > This seems like a reasonable way of solving the problem, but I think
> > > there is an even better one that we have about in the past: if we
> > > promote syscon from a platform driver into a a drivers/base/ helper
> > > that is independent of the platform device matching, we can use call
> > > syscon_regmap_lookup_* for any device node, whether it's already
> > > bound to a driver or not, which do what you need. It would also make
> > > it easier to call the syscon code before the platform_device
> > > infrastructure gets initialized, which is something a number of
> > > people have asked for, e.g. for using regmap to do SMP bringup or
> > > for clock registration.
> >
> > Basically, unless I'm missing your point, this is what my patch does,
> > except that I don't move it to drivers/base/ and the registration
> > function I added require a pointer to struct device. Indeed,
> > decoupling it further from the driver model, by adding
> > of_syscon_register() should be useful for early users.
>
> I believe the part you are missing is that with the approach I suggested,
there would
> be no registration function at all. You can easily do the lookup from the
client drivers
> using the DT data structures, with no need for the device at all.
Will you please elaborate more on this?
The only exception
> today is the clps711x platform using syscon_regmap_lookup_by_pdevname(),
but
> that should be solved in 3.17 when clps711x becomes DT-only.
>
Does it mean that this may become a blocking issue for this patch to go
before 3.17?
If yes, isn't it will be good to accept this patch as it is if it's not
causing issue or breaking
anything with existing users of syscon. At least for our use of converting
Exynos PMU and
PM related consolidation this change is working fine and we have verified
it.
> > Should I move this to drivers/base/, even though from current location
> > it can be used outside the platform driver anyway?
>
> Thinking about it some more, drivers/of might be better than drivers/base.
> It depends a bit where we are heading with this, in particular if we
expect to see non-
> DT users in the future.
>
> Arnd
Thanks,
Pankaj Dubey