Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] arm64: untag user pointers passed to the kernel
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel

arm64 has a feature called Top Byte Ignore, which allows to embed pointer
tags into the top byte of each pointer. Userspace programs (such as
HWASan, a memory debugging tool [1]) might use this feature and pass
tagged user pointers to the kernel through syscalls or other interfaces.

This patch makes a few of the kernel interfaces accept tagged user
pointers. The kernel is already able to handle user faults with tagged
pointers and has the untagged_addr macro, which this patchset reuses.

We're not trying to cover all possible ways the kernel accepts user
pointers in one patchset, so this one should be considered as a start.

Thanks!

[1] http://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html

Changes in v3:
- Rebased onto e5c51f30 (4.17-rc6+).
- Added linux-arch@ to the list of recipients.

Changes in v2:
- Rebased onto 2d618bdf (4.17-rc3+).
- Removed excessive untagging in gup.c.
- Removed untagging pointers returned from __uaccess_mask_ptr.

Changes in v1:
- Rebased onto 4.17-rc1.

Changes in RFC v2:
- Added "#ifndef untagged_addr..." fallback in linux/uaccess.h instead of
  defining it for each arch individually.
- Updated Documentation/arm64/tagged-pointers.txt.
- Dropped ?mm, arm64: untag user addresses in memory syscalls?.
- Rebased onto 3eb2ce82 (4.16-rc7).

Andrey Konovalov (6):
  arm64: add type casts to untagged_addr macro
  uaccess: add untagged_addr definition for other arches
  arm64: untag user addresses in access_ok and __uaccess_mask_ptr
  mm, arm64: untag user addresses in mm/gup.c
  lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
  arm64: update Documentation/arm64/tagged-pointers.txt

 Documentation/arm64/tagged-pointers.txt |  5 +++--
 arch/arm64/include/asm/uaccess.h        | 14 +++++++++-----
 include/linux/uaccess.h                 |  4 ++++
 lib/strncpy_from_user.c                 |  2 ++
 lib/strnlen_user.c                      |  2 ++
 mm/gup.c                                |  4 ++++
 6 files changed, 24 insertions(+), 7 deletions(-)

-- 
2.17.0.921.gf22659ad46-goog

^ permalink raw reply

* [PATCH v3 1/6] arm64: add type casts to untagged_addr macro
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

This patch makes the untagged_addr macro accept all kinds of address types
(void *, unsigned long, etc.) and allows not to specify type casts in each
place where it is used. This is done by using __typeof__.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 arch/arm64/include/asm/uaccess.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index e66b0fca99c2..2d6451cbaa86 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -102,7 +102,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
  * up with a tagged userland pointer. Clear the tag to get a sane pointer to
  * pass on to access_ok(), for instance.
  */
-#define untagged_addr(addr)		sign_extend64(addr, 55)
+#define untagged_addr(addr)		\
+	((__typeof__(addr))sign_extend64((__u64)(addr), 55))
 
 #define access_ok(type, addr, size)	__range_ok(addr, size)
 #define user_addr_max			get_fs
-- 
2.17.0.921.gf22659ad46-goog

^ permalink raw reply related

* [PATCH v3 2/6] uaccess: add untagged_addr definition for other arches
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

To allow arm64 syscalls accept tagged pointers from userspace, we must
untag them when they are passed to the kernel. Since untagging is done in
generic parts of the kernel (like the mm subsystem), the untagged_addr
macro should be defined for all architectures.

Define it as a noop for other architectures besides arm64.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 include/linux/uaccess.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index efe79c1cdd47..c045b4eff95e 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -13,6 +13,10 @@
 
 #include <asm/uaccess.h>
 
+#ifndef untagged_addr
+#define untagged_addr(addr) addr
+#endif
+
 /*
  * Architectures should provide two primitives (raw_copy_{to,from}_user())
  * and get rid of their private instances of copy_{to,from}_user() and
-- 
2.17.0.921.gf22659ad46-goog

^ permalink raw reply related

* [PATCH v3 3/6] arm64: untag user addresses in access_ok and __uaccess_mask_ptr
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

copy_from_user (and a few other similar functions) are used to copy data
from user memory into the kernel memory or vice versa. Since a user can
provided a tagged pointer to one of the syscalls that use copy_from_user,
we need to correctly handle such pointers.

Do this by untagging user pointers in access_ok and in __uaccess_mask_ptr,
before performing access validity checks.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 arch/arm64/include/asm/uaccess.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 2d6451cbaa86..fa7318d3d7d5 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -105,7 +105,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si
 #define untagged_addr(addr)		\
 	((__typeof__(addr))sign_extend64((__u64)(addr), 55))
 
-#define access_ok(type, addr, size)	__range_ok(addr, size)
+#define access_ok(type, addr, size)	\
+	__range_ok(untagged_addr(addr), size)
 #define user_addr_max			get_fs
 
 #define _ASM_EXTABLE(from, to)						\
@@ -237,7 +238,8 @@ static inline void uaccess_enable_not_uao(void)
 
 /*
  * Sanitise a uaccess pointer such that it becomes NULL if above the
- * current addr_limit.
+ * current addr_limit. In case the pointer is tagged (has the top byte set),
+ * untag the pointer before checking.
  */
 #define uaccess_mask_ptr(ptr) (__typeof__(ptr))__uaccess_mask_ptr(ptr)
 static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
@@ -245,10 +247,11 @@ static inline void __user *__uaccess_mask_ptr(const void __user *ptr)
 	void __user *safe_ptr;
 
 	asm volatile(
-	"	bics	xzr, %1, %2\n"
+	"	bics	xzr, %3, %2\n"
 	"	csel	%0, %1, xzr, eq\n"
 	: "=&r" (safe_ptr)
-	: "r" (ptr), "r" (current_thread_info()->addr_limit)
+	: "r" (ptr), "r" (current_thread_info()->addr_limit),
+	  "r" (untagged_addr(ptr))
 	: "cc");
 
 	csdb();
-- 
2.17.0.921.gf22659ad46-goog

^ permalink raw reply related

* [PATCH v3 4/6] mm, arm64: untag user addresses in mm/gup.c
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

mm/gup.c provides a kernel interface that accepts user addresses and
manipulates user pages directly (for example get_user_pages, that is used
by the futex syscall). Here we also need to handle the case of tagged user
pointers.

Add untagging to gup.c functions that use user pointers for vma lookup.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 mm/gup.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/gup.c b/mm/gup.c
index 541904a7c60f..5d0e9715bab7 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -650,6 +650,8 @@ static long __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
 	if (!nr_pages)
 		return 0;
 
+	start = untagged_addr(start);
+
 	VM_BUG_ON(!!pages != !!(gup_flags & FOLL_GET));
 
 	/*
@@ -804,6 +806,8 @@ int fixup_user_fault(struct task_struct *tsk, struct mm_struct *mm,
 	struct vm_area_struct *vma;
 	int ret, major = 0;
 
+	address = untagged_addr(address);
+
 	if (unlocked)
 		fault_flags |= FAULT_FLAG_ALLOW_RETRY;
 
-- 
2.17.0.921.gf22659ad46-goog

^ permalink raw reply related

* [PATCH v3 5/6] lib, arm64: untag addrs passed to strncpy_from_user and strnlen_user
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

strncpy_from_user and strnlen_user accept user addresses as arguments, and
do not go through the same path as copy_from_user and others, so here we
need to handle the case of tagged user addresses separately.

Untag user pointers passed to these functions.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 lib/strncpy_from_user.c | 2 ++
 lib/strnlen_user.c      | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index b53e1b5d80f4..97467cd2bc59 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -106,6 +106,8 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
 	if (unlikely(count <= 0))
 		return 0;
 
+	src = untagged_addr(src);
+
 	max_addr = user_addr_max();
 	src_addr = (unsigned long)src;
 	if (likely(src_addr < max_addr)) {
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 60d0bbda8f5e..8b5f56466e00 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -108,6 +108,8 @@ long strnlen_user(const char __user *str, long count)
 	if (unlikely(count <= 0))
 		return 0;
 
+	str = untagged_addr(str);
+
 	max_addr = user_addr_max();
 	src_addr = (unsigned long)str;
 	if (likely(src_addr < max_addr)) {
-- 
2.17.0.921.gf22659ad46-goog

^ permalink raw reply related

* [PATCH v3 6/6] arm64: update Documentation/arm64/tagged-pointers.txt
From: Andrey Konovalov @ 2018-05-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <cover.1527268727.git.andreyknvl@google.com>

Add a note that work on passing tagged user pointers to the kernel via
syscalls has started, but might not be complete yet.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 Documentation/arm64/tagged-pointers.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/arm64/tagged-pointers.txt b/Documentation/arm64/tagged-pointers.txt
index a25a99e82bb1..361481283f00 100644
--- a/Documentation/arm64/tagged-pointers.txt
+++ b/Documentation/arm64/tagged-pointers.txt
@@ -35,8 +35,9 @@ Using non-zero address tags in any of these locations may result in an
 error code being returned, a (fatal) signal being raised, or other modes
 of failure.
 
-For these reasons, passing non-zero address tags to the kernel via
-system calls is forbidden, and using a non-zero address tag for sp is
+Some initial work for supporting non-zero address tags passed to the
+kernel via system calls has been done, but the kernel doesn't provide
+any guarantees at this point. Using a non-zero address tag for sp is
 strongly discouraged.
 
 Programs maintaining a frame pointer and frame records that use non-zero
-- 
2.17.0.921.gf22659ad46-goog

^ permalink raw reply related

* [PATCH 6/6] coresight: allow to build as modules
From: Mathieu Poirier @ 2018-05-25 17:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524184908.3648cf67456510261a1afe16@arm.com>

On 24 May 2018 at 17:49, Kim Phillips <kim.phillips@arm.com> wrote:
> On Tue, 22 May 2018 15:39:06 -0600
> Mathieu Poirier <mathieu.poirier@linaro.org> wrote:
>
>> On Thu, May 17, 2018 at 08:20:24PM -0500, Kim Phillips wrote:
>> > Allow to build coresight as modules.  This greatly enhances developer
>> > efficiency by allowing the development to take place exclusively on the
>> > target, and without needing to reboot in between changes.
>> >
>> > - Kconfig bools become tristates, to allow =m
>> >
>> > - use -objs to denote merge object directives in Makefile, adds a
>> >   coresight-core nomenclature for the base module.
>> >
>> > - Export core functions so as to be able to be used by
>> >   non-core modules.
>> >
>> > - add a coresight_exit() that unregisters the coresight bus, add
>> >   remove fns for most others.
>> >
>> > - fix up modules with ID tables for autoloading on boot
>> >
>> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> > Cc: Randy Dunlap <rdunlap@infradead.org>
>> > Signed-off-by: Kim Phillips <kim.phillips@arm.com>
>> > ---
>> >  drivers/hwtracing/coresight/Kconfig           | 48 +++++++++++++++----
>> >  drivers/hwtracing/coresight/Makefile          | 28 +++++++----
>> >  .../hwtracing/coresight/coresight-cpu-debug.c |  2 +
>> >  .../coresight/coresight-dynamic-replicator.c  | 26 ++++++++--
>> >  drivers/hwtracing/coresight/coresight-etb10.c | 27 +++++++++--
>> >  .../hwtracing/coresight/coresight-etm-perf.c  |  9 +++-
>> >  .../coresight/coresight-etm3x-sysfs.c         |  1 +
>> >  drivers/hwtracing/coresight/coresight-etm3x.c | 32 +++++++++++--
>> >  .../coresight/coresight-etm4x-sysfs.c         |  1 +
>> >  drivers/hwtracing/coresight/coresight-etm4x.c | 33 +++++++++++--
>> >  .../hwtracing/coresight/coresight-funnel.c    | 26 ++++++++--
>> >  drivers/hwtracing/coresight/coresight-priv.h  |  1 -
>> >  .../coresight/coresight-replicator.c          | 28 +++++++++--
>> >  drivers/hwtracing/coresight/coresight-stm.c   | 23 ++++++++-
>> >  drivers/hwtracing/coresight/coresight-tmc.c   | 18 ++++++-
>> >  drivers/hwtracing/coresight/coresight-tpiu.c  | 26 ++++++++--
>> >  drivers/hwtracing/coresight/coresight.c       | 14 ++++++
>> >  17 files changed, 299 insertions(+), 44 deletions(-)
>>
>> For the next revision please split the work based on files.
>
> If I read that literally, one file-by-one would break build
> bisectability.  Do you mean split by source files depending on the
> logical modules they belong to, e.g., etm3x, etm4x, etb10, etc.?  If

I meant to introduce all the needed changes - including the module
author, description and licences - per module.  That will break up
this patch considerably and allow us to concentrate on individual
component.  As a final step do the changes to Kconfig and the
Makefile.


> so, I think it would look like the coresight-core would be first,
> followed by the rest, but I also think there are cross-dependencies.
> Hmm, OK, I'll have a look, but there's also one more thing:  I think
> the Makefile  obj '-core' nomenclature was to change the name of the
> module to not be the same as the core source file, so what do you think
> about renaming the core source file instead of the module name?  e.g.:
>
> Instead of this:
>
> obj-$(CONFIG_CORESIGHT) += coresight-core.o
> coresight-core-objs := coresight.o \
>                        of_coresight.o
>
> we have this:
>
> obj-$(CONFIG_CORESIGHT) += coresight.o
> coresight-objs := coresight-core.o \
>                   of_coresight.o
>
> and e.g., instead of this:
>
> obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x-core.o
> coresight-etm3x-core-objs := coresight-etm3x.o \
>                              coresight-etm-cp14.o \
>                              coresight-etm3x-sysfs.o
>
> we have this:
>
> obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o
> coresight-etm3x-objs := coresight-etm3x-core.o \
>                         coresight-etm-cp14.o \
>                         coresight-etm3x-sysfs.o
>
> ?

I think that is much better and avoid carrying the heave "-core"
appendage.  Renaming needs to be a patch on its own (but still part of
this set).

>
>> > +static int __exit tmc_remove(struct amba_device *adev)
>> > +{
>> > +   struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
>> > +
>> > +   /* free ETB/ETF or ETR memory */
>> > +   tmc_read_unprepare(drvdata);
>> > +
>> > +   misc_deregister(&drvdata->miscdev);
>> > +   coresight_unregister(drvdata->csdev);
>> > +
>> > +   return 0;
>> > +}
>> > +
>>
>> Right now I can remove the module for a TMC link or sink when part of an active
>> session, something I pointed out during an earlier revision.
>
> Right, I missed that :(
>
> So would the first thing tmc_remove does is this:
>
> if (drvdata->reading)
>         return -EBUSY;
>
> work, or would we need to introduce a new sentinel?

The ->reading flag is to prevent concurrent access to the buffer from
sysFS and to avoid clobbering said buffer with new trace data.  The
TMC driver shouldn't be different from the other ones with regards to
the usage of the try_module_get()/module_put() functions.

>
>> I also think we need to deal with driver removal cases when the TMC buffer
>> (ETR or ETF) is being read from sysFS.
>
> OK, I thought the:
>
> struct file_operations tmc_fops = {
>         .owner = THIS_MODULE,
>
> would prevent module unload whilst sysfs access was being performed,
> but I'll double check.

Right, just check that it works as advertised.

>
> Thanks,
>
> Kim

^ permalink raw reply

* [PATCH 6/6] coresight: allow to build as modules
From: Mathieu Poirier @ 2018-05-25 17:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4986a636-d5a5-387c-abf1-d68afe063f06@arm.com>

On 25 May 2018 at 11:12, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 18/05/18 02:20, Kim Phillips wrote:
>>
>> Allow to build coresight as modules.  This greatly enhances developer
>> efficiency by allowing the development to take place exclusively on the
>> target, and without needing to reboot in between changes.
>>
>> - Kconfig bools become tristates, to allow =m
>>
>> - use -objs to denote merge object directives in Makefile, adds a
>>    coresight-core nomenclature for the base module.
>>
>> - Export core functions so as to be able to be used by
>>    non-core modules.
>>
>> - add a coresight_exit() that unregisters the coresight bus, add
>>    remove fns for most others.
>>
>> - fix up modules with ID tables for autoloading on boot
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Randy Dunlap <rdunlap@infradead.org>
>> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
>
>
> Kim,
>
> I see that you have addressed my comments on a previous version
> of this series posted in April. But I don't see the version number
> increased for this new version. Please add versioning to make it
> easier to make it more obvious.
>
> Also, generally it is a good idea to keep the people who reviewed
> and commented on your previous versions in the newer versions.
>
> Some comments below :
>
>> diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
>> b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
>> index fc742215ab05..bc42b8022556 100644
>> --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
>> +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
>> @@ -37,7 +37,12 @@ struct replicator_state {
>>   static int replicator_enable(struct coresight_device *csdev, int inport,
>>                               int outport)
>>   {
>> -       struct replicator_state *drvdata =
>> dev_get_drvdata(csdev->dev.parent);
>> +       struct device *parent_dev = csdev->dev.parent;
>> +       struct replicator_state *drvdata = dev_get_drvdata(parent_dev);
>> +       struct module *module = parent_dev->driver->owner;
>> +
>> +       if (!try_module_get(module))
>> +               return -ENODEV;
>>         CS_UNLOCK(drvdata->base);
>
>
> What is the guarantee that the "csdev" is still available when we reach
> here ?
>
> A module could be unloaded "after the component was added to the path"
> (via coresight_build_path) and before we invoke the "enable" on each
> component in the path ?

Very good point - this is invariably racy.

>
> Also, it  is tedious to do module_get in "enable" and module_put in the
> disable call backs for each component.
>
> Instead, if we do a module_get() in build_path and module_put() in
> release path, we could solve all these problems and keep it the module
> refcount in a central place.

Good idea, it does streamline things a lot.

>
>> +MODULE_DEVICE_TABLE(amba, replicator_ids);
>> +
>>   static struct amba_driver replicator_driver = {
>>         .drv = {
>>                 .name   = "coresight-dynamic-replicator",
>> @@ -207,9 +226,10 @@ static struct amba_driver replicator_driver = {
>>                 .suppress_bind_attrs = true,
>>         },
>>         .probe          = replicator_probe,
>> +       .remove         = replicator_remove,
>>         .id_table       = replicator_ids,
>>   };
>
>
> Do we have the owner field set here for this driver ? I see that you
> added it for some components and not others. e.g, you have added it for
> etm4x, while not for replicator and others.
>
>
>> +MODULE_DEVICE_TABLE(amba, etm4_ids);
>> +
>>   static struct amba_driver etm4x_driver = {
>>         .drv = {
>>                 .name   = "coresight-etm4x",
>> +               .owner  = THIS_MODULE,
>>                 .suppress_bind_attrs = true,
>>         },
>>         .probe          = etm4_probe,
>> +       .remove         = etm4_remove,
>>         .id_table       = etm4_ids,
>>   };
>> -builtin_amba_driver(etm4x_driver);
>> +module_amba_driver(etm4x_driver);
>
>
>
> Suzuki

^ permalink raw reply

* i2c-gpio and boards conversions to GPIO descriptors
From: Simon Guinot @ 2018-05-25 17:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525154144.GC19100@kw.sim.vm.gnt>

On Fri, May 25, 2018 at 05:41:44PM +0200, Simon Guinot wrote:
> Hi Linus,

Forwarding to some mailing lists I missed in a first place.

> 
> I think your patch b2e63555592f "i2c: gpio: Convert to use descriptors"
> may have broken i2c-gpio support for some boards using old fashion
> platform_device declarations.
> 
> Indeed when an "i2c-gpio" platform_device is registered with a fixed id
> e.g. 0, then I think the device name becomes "i2c-gpio.0". And then this
> won't match a lookup table registered with an "i2c-gpio" dev_id.
> 
> Please double check this :)
> 
> Simon
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180525/4bcae35d/attachment.sig>

^ permalink raw reply

* [PATCH v2 1/8] driver core: make deferring probe after init optional
From: Rob Herring @ 2018-05-25 17:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <92e3be10-e65a-99e9-6ef7-f11ded6a35f9@arm.com>

On Fri, May 25, 2018 at 7:20 AM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 24/05/18 21:57, Rob Herring wrote:
>>
>> On Thu, May 24, 2018 at 2:00 PM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>>
>>> On Thu, May 24, 2018 at 12:50:17PM -0500, Rob Herring wrote:
>>>>
>>>> Deferred probe will currently wait forever on dependent devices to
>>>> probe,
>>>> but sometimes a driver will never exist. It's also not always critical
>>>> for
>>>> a driver to exist. Platforms can rely on default configuration from the
>>>> bootloader or reset defaults for things such as pinctrl and power
>>>> domains.
>>>> This is often the case with initial platform support until various
>>>> drivers
>>>> get enabled. There's at least 2 scenarios where deferred probe can
>>>> render
>>>> a platform broken. Both involve using a DT which has more devices and
>>>> dependencies than the kernel supports. The 1st case is a driver may be
>>>> disabled in the kernel config. The 2nd case is the kernel version may
>>>> simply not have the dependent driver. This can happen if using a newer
>>>> DT
>>>> (provided by firmware perhaps) with a stable kernel version.
>>>>
>>>> Subsystems or drivers may opt-in to this behavior by calling
>>>> driver_deferred_probe_check_init_done() instead of just returning
>>>> -EPROBE_DEFER. They may use additional information from DT or kernel's
>>>> config to decide whether to continue to defer probe or not.
>>>>
>>>> Cc: Alexander Graf <agraf@suse.de>
>>>> Signed-off-by: Rob Herring <robh@kernel.org>
>>>> ---
>>>>   drivers/base/dd.c      | 17 +++++++++++++++++
>>>>   include/linux/device.h |  2 ++
>>>>   2 files changed, 19 insertions(+)
>>>>
>>>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>>>> index c9f54089429b..d6034718da6f 100644
>>>> --- a/drivers/base/dd.c
>>>> +++ b/drivers/base/dd.c
>>>> @@ -226,6 +226,16 @@ void device_unblock_probing(void)
>>>>        driver_deferred_probe_trigger();
>>>>   }
>>>>
>>>> +int driver_deferred_probe_check_init_done(struct device *dev, bool
>>>> optional)
>>>> +{
>>>> +     if (optional && initcalls_done) {
>>>
>>>
>>> Wait, what's the "optional" mess here?
>>
>>
>> My intent was that subsystems just always call this function and never
>> return EPROBE_DEFER themselves. Then the driver core can make
>> decisions as to what to do (such as the timeout added in the next
>> patch). Or it can print common error/debug messages. So optional is a
>> hint to allow subsystems per device control.
>
>
> Maybe just driver_defer_probe() might be a more descriptive name? To me,
> calling "foo_check_x()" with a parameter that says "I don't actually care
> about x" is the really unintuitive bit.

All the other (though static or internal to driver core) functions are
prefixed driver_deferred_probe_* so I was trying to remain consistent
there. You're right though, with the timeout it's not just whether
initcalls are done. It's really "get the return value depending on the
core's deferred probe state". So perhaps one of these:

driver_deferred_probe_get_return_val()
driver_deferred_probe_handle_return()

The other option would be a more straight-forward functions that just
returns a bool on whether to continue deferring and leave the return
code handling to the caller:

if (driver_deferred_probe_enabled_for_builtin(dev))
  return -EPROBE_DEFER;
else
  return -ENODEV;

The pinctrl case would look like this:

builtin_only = of_property_read_bool(np_pctldev, "pinctrl-use-default");
if (builtin_only && driver_deferred_probe_enabled_for_builtin(dev))
  return -EPROBE_DEFER;
else if (!builtin_only && driver_deferred_probe_enabled(dev))
  return -EPROBE_DEFER;
else
  return -ENODEV;

I still prefer the former, picking the bike shed color is easier with
the latter.

>>>
>>> The caller knows this value, so why do you need to even pass it in here?
>>
>>
>> Because regardless of the value, we always stop deferring when/if we
>> hit the timeout and the caller doesn't know about the timeout. If we
>> get rid of it, we'd need functions for both init done and for deferred
>> timeout.
>>
>>> And bool values that are not obvious are horrid.  I had to go look this
>>> up when reading the later patches that just passed "true" in this
>>> variable as I had no idea what that meant.
>>
>>
>> Perhaps inverting it and calling it "keep_deferring" would be better.
>> However, the flag is ignored if we have timed out.
>
>
> Perhaps an enum (or bitmask of named flags) then? That would allow the most
> readability at callsites, plus it seems quite likely that we may want
> intermediate degrees of "deferral strictness" eventually.

A bitmask is just 32 booleans stuffed into one parameter which I can
guess Greg's opinion on.

I can't really think of other flags we might need here. If we added
some userspace trigger saying module loading is done, I don't think
we'd need that to be per caller.

Rob

^ permalink raw reply

* [PATCH] PCI: Try to clean up resources via remove if shutdown doesn't exist
From: Sinan Kaya @ 2018-05-25 17:36 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1527257063-15843-1-git-send-email-okaya@codeaurora.org>

[+cc kexec]

On 5/25/2018 7:04 AM, Sinan Kaya wrote:
> It is up to a driver to implement shutdown() callback. If shutdown()
> callback is not implemented, PCI device can have pending interrupt and
> even do DMA transactions while the system is going down.
> 
> If kexec is in use, this can damage the newly booting kexec kernel
> or even prevent it from booting altogether. Fallback to calling the
> remove() callback if shutdown() isn't implemented for a given driver.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/pci/pci-driver.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 6ace470..4ac72e3 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -475,8 +475,17 @@ static void pci_device_shutdown(struct device *dev)
>  
>  	pm_runtime_resume(dev);
>  
> +	/*
> +	 * Try shutdown callback if it exists, otherwise fallback to remove
> +	 * callback. PCI drivers can do DMA and have pending interrupts.
> +	 * Leaving the DMA and interrupts pending could damage the newly
> +	 * booting kexec kernel as well as prevent it from booting altogether
> +	 * if the pending interrupt is level.
> +	 */
>  	if (drv && drv->shutdown)
>  		drv->shutdown(pci_dev);
> +	else if (drv && drv->remove)
> +		drv->remove(pci_dev);
>  
>  	/*
>  	 * If this is a kexec reboot, turn off Bus Master bit on the
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply

* [PATCH 0/9] clk: davinci: outstanding fixes
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel

This is a resend of all of the outstanding DaVinci clock patches plus one new
patch. All of the patches (except the new one) have been reviewed and tested
by someone other than me.

The new patch ("clk: davinci: Fix link errors when not all SoCs are enabled")
should be fairly trivial.

I am resending them all in one series to hopefully make it easier to get them
picked up by having them in the correct order to avoid merge conflicts. This
series is based on the clk/clk-davinci-psc-da830 branch.

David Lechner (7):
  clk: davinci: pll-dm355: drop pll2_sysclk2
  clk: davinci: pll-dm355: fix SYSCLKn parent names
  clk: davinci: psc-dm355: fix ASP0/1 clkdev lookups
  clk: davinci: pll: allow dev == NULL
  clk: davinci: da850-pll: change PLL0 to CLK_OF_DECLARE
  clk: davinci: psc: allow for dev == NULL
  clk: davinci: Fix link errors when not all SoCs are enabled

Sekhar Nori (2):
  clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled
  clk: davinci: psc-dm365: fix few clocks

 drivers/clk/davinci/pll-da830.c  |   5 +-
 drivers/clk/davinci/pll-da850.c  |  37 ++--
 drivers/clk/davinci/pll-dm355.c  |  22 ++-
 drivers/clk/davinci/pll-dm365.c  |   9 +-
 drivers/clk/davinci/pll-dm644x.c |   9 +-
 drivers/clk/davinci/pll-dm646x.c |  11 +-
 drivers/clk/davinci/pll.c        | 299 +++++++++++++++++++++----------
 drivers/clk/davinci/pll.h        |  41 +++--
 drivers/clk/davinci/psc-dm355.c  |   7 +-
 drivers/clk/davinci/psc-dm365.c  |  22 ++-
 drivers/clk/davinci/psc-dm644x.c |   3 +-
 drivers/clk/davinci/psc-dm646x.c |   3 +-
 drivers/clk/davinci/psc.c        |  72 ++++++--
 drivers/clk/davinci/psc.h        |  12 ++
 include/linux/clk/davinci.h      |  40 +++++
 15 files changed, 418 insertions(+), 174 deletions(-)
 create mode 100644 include/linux/clk/davinci.h

-- 
2.17.0

^ permalink raw reply

* [PATCH 1/9] clk: davinci: pll-dm355: drop pll2_sysclk2
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

This removes pll2_sysclk2 from the TI DM355 clock driver. This SoC
doesn't have such a clock. Also, SYSCLK_ALWAYS_ENABLED is transferred
to pll2_sysclk1 since it drives the DDR and doesn't have another
mechanism to keep it on.

Reported-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: David Lechner <david@lechnology.com>
Acked-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/clk/davinci/pll-dm355.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/clk/davinci/pll-dm355.c b/drivers/clk/davinci/pll-dm355.c
index 5345f8286c50..718d9bbbf30d 100644
--- a/drivers/clk/davinci/pll-dm355.c
+++ b/drivers/clk/davinci/pll-dm355.c
@@ -62,8 +62,7 @@ static const struct davinci_pll_clk_info dm355_pll2_info = {
 		 PLL_POSTDIV_ALWAYS_ENABLED | PLL_POSTDIV_FIXED_DIV,
 };
 
-SYSCLK(1, pll2_sysclk1, pll2, 5, SYSCLK_FIXED_DIV);
-SYSCLK(2, pll2_sysclk2, pll2, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
+SYSCLK(1, pll2_sysclk1, pll2, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
 
 int dm355_pll2_init(struct device *dev, void __iomem *base)
 {
@@ -71,8 +70,6 @@ int dm355_pll2_init(struct device *dev, void __iomem *base)
 
 	davinci_pll_sysclk_register(dev, &pll2_sysclk1, base);
 
-	davinci_pll_sysclk_register(dev, &pll2_sysclk2, base);
-
 	davinci_pll_sysclkbp_clk_register(dev, "pll2_sysclkbp", base);
 
 	return 0;
-- 
2.17.0

^ permalink raw reply related

* [PATCH 2/9] clk: davinci: pll-dm355: fix SYSCLKn parent names
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

This fixes the parent clock names of the SYSCLKn clocks for the DM355
SoC in the TI DaVinici PLL clock driver.

It appears that this name just didn't get updated to the correct name
like the other SoCs during the driver's development.

Reported-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: David Lechner <david@lechnology.com>
Acked-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/clk/davinci/pll-dm355.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/davinci/pll-dm355.c b/drivers/clk/davinci/pll-dm355.c
index 718d9bbbf30d..93f4a53d6b44 100644
--- a/drivers/clk/davinci/pll-dm355.c
+++ b/drivers/clk/davinci/pll-dm355.c
@@ -22,10 +22,10 @@ static const struct davinci_pll_clk_info dm355_pll1_info = {
 		 PLL_POSTDIV_ALWAYS_ENABLED | PLL_POSTDIV_FIXED_DIV,
 };
 
-SYSCLK(1, pll1_sysclk1, pll1, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
-SYSCLK(2, pll1_sysclk2, pll1, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
-SYSCLK(3, pll1_sysclk3, pll1, 5, SYSCLK_ALWAYS_ENABLED);
-SYSCLK(4, pll1_sysclk4, pll1, 5, SYSCLK_ALWAYS_ENABLED);
+SYSCLK(1, pll1_sysclk1, pll1_pllen, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
+SYSCLK(2, pll1_sysclk2, pll1_pllen, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
+SYSCLK(3, pll1_sysclk3, pll1_pllen, 5, SYSCLK_ALWAYS_ENABLED);
+SYSCLK(4, pll1_sysclk4, pll1_pllen, 5, SYSCLK_ALWAYS_ENABLED);
 
 int dm355_pll1_init(struct device *dev, void __iomem *base)
 {
@@ -62,7 +62,7 @@ static const struct davinci_pll_clk_info dm355_pll2_info = {
 		 PLL_POSTDIV_ALWAYS_ENABLED | PLL_POSTDIV_FIXED_DIV,
 };
 
-SYSCLK(1, pll2_sysclk1, pll2, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
+SYSCLK(1, pll2_sysclk1, pll2_pllen, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
 
 int dm355_pll2_init(struct device *dev, void __iomem *base)
 {
-- 
2.17.0

^ permalink raw reply related

* [PATCH 3/9] clk: davinci: psc-dm355: fix ASP0/1 clkdev lookups
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

The clkdev lookups for the ASP0/1 devices on TI DM355 were declared, but
not assigned to any LPSC. This assigns the clkdev lookups to the
correct LPSCs.

Reported-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: David Lechner <david@lechnology.com>
Reviewed-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/clk/davinci/psc-dm355.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/davinci/psc-dm355.c b/drivers/clk/davinci/psc-dm355.c
index 6995ecea2677..128e7345b20c 100644
--- a/drivers/clk/davinci/psc-dm355.c
+++ b/drivers/clk/davinci/psc-dm355.c
@@ -41,14 +41,14 @@ static const struct davinci_lpsc_clk_info dm355_psc_info[] = {
 	LPSC(5,  0, timer3,      pll1_auxclk,  NULL,               0),
 	LPSC(6,  0, spi1,        pll1_sysclk2, spi1_clkdev,        0),
 	LPSC(7,  0, mmcsd1,      pll1_sysclk2, mmcsd1_clkdev,      0),
-	LPSC(8,  0, asp1,        pll1_sysclk2, NULL,               0),
+	LPSC(8,  0, asp1,        pll1_sysclk2, mcbsp1_clkdev,      0),
 	LPSC(9,  0, usb,         pll1_sysclk2, usb_clkdev,         0),
 	LPSC(10, 0, pwm3,        pll1_auxclk,  NULL,               0),
 	LPSC(11, 0, spi2,        pll1_sysclk2, spi2_clkdev,        0),
 	LPSC(12, 0, rto,         pll1_auxclk,  NULL,               0),
 	LPSC(14, 0, aemif,       pll1_sysclk2, aemif_clkdev,       0),
 	LPSC(15, 0, mmcsd0,      pll1_sysclk2, mmcsd0_clkdev,      0),
-	LPSC(17, 0, asp0,        pll1_sysclk2, NULL,               0),
+	LPSC(17, 0, asp0,        pll1_sysclk2, mcbsp0_clkdev,      0),
 	LPSC(18, 0, i2c,         pll1_auxclk,  i2c_clkdev,         0),
 	LPSC(19, 0, uart0,       pll1_auxclk,  uart0_clkdev,       0),
 	LPSC(20, 0, uart1,       pll1_auxclk,  uart1_clkdev,       0),
-- 
2.17.0

^ permalink raw reply related

* [PATCH 4/9] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

From: Sekhar Nori <nsekhar@ti.com>

PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot
be disabled. Mark it so to prevent unused clock disable
infrastructure from disabling it.

Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 drivers/clk/davinci/pll-dm646x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c
index a61cc3256418..0ae827e3ce80 100644
--- a/drivers/clk/davinci/pll-dm646x.c
+++ b/drivers/clk/davinci/pll-dm646x.c
@@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = {
 	.flags = 0,
 };
 
-SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0);
+SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED);
 
 int dm646x_pll2_init(struct device *dev, void __iomem *base)
 {
-- 
2.17.0

^ permalink raw reply related

* [PATCH 5/9] clk: davinci: psc-dm365: fix few clocks
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

From: Sekhar Nori <nsekhar@ti.com>

Fix parent of EMAC and voice codec PSC clocks. Documentation is clear
on EMAC clock parent, but its not fully clear on parent of voice codec
clock. The implementation chosen is matches arch/arm/mach-davinci/dm365.c.
Add a comment explaining this for posterity.

There is only one power domain on DM365. Fix the power domain of voice
codec and vpss dac modules.

While at it, add a comment explaining how the parent of vpss dac clock was
derived. Note that this patch does not touch the parent of vpss dac clock.

Signed-off-by: Sekhar Nori <nsekhar@ti.com>
Reviewed-by: David Lechner <david@lechnology.com>
---
 drivers/clk/davinci/psc-dm365.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/davinci/psc-dm365.c b/drivers/clk/davinci/psc-dm365.c
index 3ad915f37376..289af3913fb0 100644
--- a/drivers/clk/davinci/psc-dm365.c
+++ b/drivers/clk/davinci/psc-dm365.c
@@ -65,9 +65,22 @@ static const struct davinci_lpsc_clk_info dm365_psc_info[] = {
 	LPSC(31, 0, arm,         pll2_sysclk2, NULL,               LPSC_ALWAYS_ENABLED),
 	LPSC(38, 0, spi3,        pll1_sysclk4, spi3_clkdev,        0),
 	LPSC(39, 0, spi4,        pll1_auxclk,  spi4_clkdev,        0),
-	LPSC(40, 0, emac,        pll2_sysclk4, emac_clkdev,        0),
-	LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0),
-	LPSC(46, 1, vpss_dac,    pll1_sysclk3, vpss_dac_clkdev,    0),
+	LPSC(40, 0, emac,        pll1_sysclk4, emac_clkdev,        0),
+	/*
+	 * The TRM (ARM Subsystem User's Guide) shows two clocks input into
+	 * voice codec module (PLL2 SYSCLK4 with a DIV2 and PLL1 SYSCLK4). Its
+	 * not fully clear from documentation which clock should be considered
+	 * as parent for PSC. The clock chosen here is to maintain
+	 * compatibility with existing code in arch/arm/mach-davinci/dm365.c
+	 */
+	LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0),
+	/*
+	 * Its not fully clear from TRM (ARM Subsystem User's Guide) as to what
+	 * the parent of VPSS DAC LPSC should actually be. PLL1 SYSCLK3 feeds
+	 * into HDVICP and MJCP. The clock chosen here is to remain compatible
+	 * with code existing in arch/arm/mach-davinci/dm365.c
+	 */
+	LPSC(46, 0, vpss_dac,    pll1_sysclk3, vpss_dac_clkdev,    0),
 	LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0),
 	LPSC(50, 0, mjcp,        pll1_sysclk3, NULL,               0),
 	{ }
-- 
2.17.0

^ permalink raw reply related

* [PATCH 6/9] clk: davinci: pll: allow dev == NULL
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

This modifies the TI Davinci PLL clock driver to allow for the case
when dev == NULL. On some (most) SoCs that use this driver, the PLL
clock needs to be registered during early boot because it is used
for clocksource/clkevent and there will be no platform device available.

Signed-off-by: David Lechner <david@lechnology.com>
Reviewed-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/clk/davinci/pll-da830.c  |   5 +-
 drivers/clk/davinci/pll-da850.c  |  22 +--
 drivers/clk/davinci/pll-dm355.c  |   9 +-
 drivers/clk/davinci/pll-dm365.c  |   9 +-
 drivers/clk/davinci/pll-dm644x.c |   9 +-
 drivers/clk/davinci/pll-dm646x.c |   9 +-
 drivers/clk/davinci/pll.c        | 279 +++++++++++++++++++++----------
 drivers/clk/davinci/pll.h        |  30 ++--
 include/linux/clk/davinci.h      |  24 +++
 9 files changed, 259 insertions(+), 137 deletions(-)
 create mode 100644 include/linux/clk/davinci.h

diff --git a/drivers/clk/davinci/pll-da830.c b/drivers/clk/davinci/pll-da830.c
index 929a3d3a9adb..0a0d06fb25fd 100644
--- a/drivers/clk/davinci/pll-da830.c
+++ b/drivers/clk/davinci/pll-da830.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clkdev.h>
+#include <linux/clk/davinci.h>
 #include <linux/bitops.h>
 #include <linux/init.h>
 #include <linux/types.h>
@@ -36,11 +37,11 @@ SYSCLK(5, pll0_sysclk5, pll0_pllen, 5, 0);
 SYSCLK(6, pll0_sysclk6, pll0_pllen, 5, SYSCLK_FIXED_DIV);
 SYSCLK(7, pll0_sysclk7, pll0_pllen, 5, 0);
 
-int da830_pll_init(struct device *dev, void __iomem *base)
+int da830_pll_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
 	struct clk *clk;
 
-	davinci_pll_clk_register(dev, &da830_pll_info, "ref_clk", base);
+	davinci_pll_clk_register(dev, &da830_pll_info, "ref_clk", base, cfgchip);
 
 	clk = davinci_pll_sysclk_register(dev, &pll0_sysclk2, base);
 	clk_register_clkdev(clk, "pll0_sysclk2", "da830-psc0");
diff --git a/drivers/clk/davinci/pll-da850.c b/drivers/clk/davinci/pll-da850.c
index 2a038b7908cc..59cc2e3733f9 100644
--- a/drivers/clk/davinci/pll-da850.c
+++ b/drivers/clk/davinci/pll-da850.c
@@ -7,7 +7,9 @@
 
 #include <linux/bitops.h>
 #include <linux/clk-provider.h>
+#include <linux/clk/davinci.h>
 #include <linux/clkdev.h>
+#include <linux/device.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/mfd/da8xx-cfgchip.h>
@@ -81,11 +83,11 @@ static const struct davinci_pll_obsclk_info da850_pll0_obsclk_info = {
 	.ocsrc_mask = GENMASK(4, 0),
 };
 
-int da850_pll0_init(struct device *dev, void __iomem *base)
+int da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
 	struct clk *clk;
 
-	davinci_pll_clk_register(dev, &da850_pll0_info, "ref_clk", base);
+	davinci_pll_clk_register(dev, &da850_pll0_info, "ref_clk", base, cfgchip);
 
 	clk = davinci_pll_sysclk_register(dev, &pll0_sysclk1, base);
 	clk_register_clkdev(clk, "pll0_sysclk1", "da850-psc0");
@@ -134,11 +136,11 @@ static const struct davinci_pll_sysclk_info *da850_pll0_sysclk_info[] = {
 	NULL
 };
 
-int of_da850_pll0_init(struct device *dev, void __iomem *base)
+int of_da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
-	return of_davinci_pll_init(dev, &da850_pll0_info,
+	return of_davinci_pll_init(dev, dev->of_node, &da850_pll0_info,
 				   &da850_pll0_obsclk_info,
-				   da850_pll0_sysclk_info, 7, base);
+				   da850_pll0_sysclk_info, 7, base, cfgchip);
 }
 
 static const struct davinci_pll_clk_info da850_pll1_info = {
@@ -179,11 +181,11 @@ static const struct davinci_pll_obsclk_info da850_pll1_obsclk_info = {
 	.ocsrc_mask = GENMASK(4, 0),
 };
 
-int da850_pll1_init(struct device *dev, void __iomem *base)
+int da850_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
 	struct clk *clk;
 
-	davinci_pll_clk_register(dev, &da850_pll1_info, "oscin", base);
+	davinci_pll_clk_register(dev, &da850_pll1_info, "oscin", base, cfgchip);
 
 	davinci_pll_sysclk_register(dev, &pll1_sysclk1, base);
 
@@ -204,9 +206,9 @@ static const struct davinci_pll_sysclk_info *da850_pll1_sysclk_info[] = {
 	NULL
 };
 
-int of_da850_pll1_init(struct device *dev, void __iomem *base)
+int of_da850_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
-	return of_davinci_pll_init(dev, &da850_pll1_info,
+	return of_davinci_pll_init(dev, dev->of_node, &da850_pll1_info,
 				   &da850_pll1_obsclk_info,
-				   da850_pll1_sysclk_info, 3, base);
+				   da850_pll1_sysclk_info, 3, base, cfgchip);
 }
diff --git a/drivers/clk/davinci/pll-dm355.c b/drivers/clk/davinci/pll-dm355.c
index 93f4a53d6b44..505aed80be9a 100644
--- a/drivers/clk/davinci/pll-dm355.c
+++ b/drivers/clk/davinci/pll-dm355.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/clk/davinci.h>
 #include <linux/clkdev.h>
 #include <linux/init.h>
 #include <linux/types.h>
@@ -27,11 +28,11 @@ SYSCLK(2, pll1_sysclk2, pll1_pllen, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED)
 SYSCLK(3, pll1_sysclk3, pll1_pllen, 5, SYSCLK_ALWAYS_ENABLED);
 SYSCLK(4, pll1_sysclk4, pll1_pllen, 5, SYSCLK_ALWAYS_ENABLED);
 
-int dm355_pll1_init(struct device *dev, void __iomem *base)
+int dm355_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
 	struct clk *clk;
 
-	davinci_pll_clk_register(dev, &dm355_pll1_info, "ref_clk", base);
+	davinci_pll_clk_register(dev, &dm355_pll1_info, "ref_clk", base, cfgchip);
 
 	clk = davinci_pll_sysclk_register(dev, &pll1_sysclk1, base);
 	clk_register_clkdev(clk, "pll1_sysclk1", "dm355-psc");
@@ -64,9 +65,9 @@ static const struct davinci_pll_clk_info dm355_pll2_info = {
 
 SYSCLK(1, pll2_sysclk1, pll2_pllen, 5, SYSCLK_FIXED_DIV | SYSCLK_ALWAYS_ENABLED);
 
-int dm355_pll2_init(struct device *dev, void __iomem *base)
+int dm355_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
-	davinci_pll_clk_register(dev, &dm355_pll2_info, "oscin", base);
+	davinci_pll_clk_register(dev, &dm355_pll2_info, "oscin", base, cfgchip);
 
 	davinci_pll_sysclk_register(dev, &pll2_sysclk1, base);
 
diff --git a/drivers/clk/davinci/pll-dm365.c b/drivers/clk/davinci/pll-dm365.c
index 5f8d9f42d0f3..2d29712753a3 100644
--- a/drivers/clk/davinci/pll-dm365.c
+++ b/drivers/clk/davinci/pll-dm365.c
@@ -7,6 +7,7 @@
 
 #include <linux/bitops.h>
 #include <linux/clkdev.h>
+#include <linux/clk/davinci.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
@@ -56,11 +57,11 @@ static const struct davinci_pll_obsclk_info dm365_pll1_obsclk_info = {
 	.ocsrc_mask = BIT(4),
 };
 
-int dm365_pll1_init(struct device *dev, void __iomem *base)
+int dm365_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
 	struct clk *clk;
 
-	davinci_pll_clk_register(dev, &dm365_pll1_info, "ref_clk", base);
+	davinci_pll_clk_register(dev, &dm365_pll1_info, "ref_clk", base, cfgchip);
 
 	clk = davinci_pll_sysclk_register(dev, &pll1_sysclk1, base);
 	clk_register_clkdev(clk, "pll1_sysclk1", "dm365-psc");
@@ -119,11 +120,11 @@ static const struct davinci_pll_obsclk_info dm365_pll2_obsclk_info = {
 	.ocsrc_mask = BIT(4),
 };
 
-int dm365_pll2_init(struct device *dev, void __iomem *base)
+int dm365_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
 	struct clk *clk;
 
-	davinci_pll_clk_register(dev, &dm365_pll2_info, "oscin", base);
+	davinci_pll_clk_register(dev, &dm365_pll2_info, "oscin", base, cfgchip);
 
 	davinci_pll_sysclk_register(dev, &pll2_sysclk1, base);
 
diff --git a/drivers/clk/davinci/pll-dm644x.c b/drivers/clk/davinci/pll-dm644x.c
index 69bf785377cf..7650fadfaac8 100644
--- a/drivers/clk/davinci/pll-dm644x.c
+++ b/drivers/clk/davinci/pll-dm644x.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/clk/davinci.h>
 #include <linux/clkdev.h>
 #include <linux/init.h>
 #include <linux/types.h>
@@ -27,11 +28,11 @@ SYSCLK(2, pll1_sysclk2, pll1_pllen, 4, SYSCLK_FIXED_DIV);
 SYSCLK(3, pll1_sysclk3, pll1_pllen, 4, SYSCLK_FIXED_DIV);
 SYSCLK(5, pll1_sysclk5, pll1_pllen, 4, SYSCLK_FIXED_DIV);
 
-int dm644x_pll1_init(struct device *dev, void __iomem *base)
+int dm644x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
 	struct clk *clk;
 
-	davinci_pll_clk_register(dev, &dm644x_pll1_info, "ref_clk", base);
+	davinci_pll_clk_register(dev, &dm644x_pll1_info, "ref_clk", base, cfgchip);
 
 	clk = davinci_pll_sysclk_register(dev, &pll1_sysclk1, base);
 	clk_register_clkdev(clk, "pll1_sysclk1", "dm644x-psc");
@@ -66,9 +67,9 @@ static const struct davinci_pll_clk_info dm644x_pll2_info = {
 SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0);
 SYSCLK(2, pll2_sysclk2, pll2_pllen, 4, 0);
 
-int dm644x_pll2_init(struct device *dev, void __iomem *base)
+int dm644x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
-	davinci_pll_clk_register(dev, &dm644x_pll2_info, "oscin", base);
+	davinci_pll_clk_register(dev, &dm644x_pll2_info, "oscin", base, cfgchip);
 
 	davinci_pll_sysclk_register(dev, &pll2_sysclk1, base);
 
diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c
index 0ae827e3ce80..26982970df0e 100644
--- a/drivers/clk/davinci/pll-dm646x.c
+++ b/drivers/clk/davinci/pll-dm646x.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/clk/davinci.h>
 #include <linux/clkdev.h>
 #include <linux/init.h>
 #include <linux/types.h>
@@ -29,11 +30,11 @@ SYSCLK(6, pll1_sysclk6, pll1_pllen, 4, 0);
 SYSCLK(8, pll1_sysclk8, pll1_pllen, 4, 0);
 SYSCLK(9, pll1_sysclk9, pll1_pllen, 4, 0);
 
-int dm646x_pll1_init(struct device *dev, void __iomem *base)
+int dm646x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
 	struct clk *clk;
 
-	davinci_pll_clk_register(dev, &dm646x_pll1_info, "ref_clk", base);
+	davinci_pll_clk_register(dev, &dm646x_pll1_info, "ref_clk", base, cfgchip);
 
 	clk = davinci_pll_sysclk_register(dev, &pll1_sysclk1, base);
 	clk_register_clkdev(clk, "pll1_sysclk1", "dm646x-psc");
@@ -74,9 +75,9 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = {
 
 SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED);
 
-int dm646x_pll2_init(struct device *dev, void __iomem *base)
+int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
 {
-	davinci_pll_clk_register(dev, &dm646x_pll2_info, "oscin", base);
+	davinci_pll_clk_register(dev, &dm646x_pll2_info, "oscin", base, cfgchip);
 
 	davinci_pll_sysclk_register(dev, &pll2_sysclk1, base);
 
diff --git a/drivers/clk/davinci/pll.c b/drivers/clk/davinci/pll.c
index 23a24c944f1d..2eb981e61185 100644
--- a/drivers/clk/davinci/pll.c
+++ b/drivers/clk/davinci/pll.c
@@ -11,6 +11,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/clk.h>
+#include <linux/clk/davinci.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
@@ -223,6 +224,7 @@ static const struct clk_ops dm365_pll_ops = {
 
 /**
  * davinci_pll_div_register - common *DIV clock implementation
+ * @dev: The PLL platform device or NULL
  * @name: the clock name
  * @parent_name: the parent clock name
  * @reg: the *DIV register
@@ -240,17 +242,21 @@ static struct clk *davinci_pll_div_register(struct device *dev,
 	const struct clk_ops *divider_ops = &clk_divider_ops;
 	struct clk_gate *gate;
 	struct clk_divider *divider;
+	struct clk *clk;
+	int ret;
 
-	gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
 	if (!gate)
 		return ERR_PTR(-ENOMEM);
 
 	gate->reg = reg;
 	gate->bit_idx = DIV_ENABLE_SHIFT;
 
-	divider = devm_kzalloc(dev, sizeof(*divider), GFP_KERNEL);
-	if (!divider)
-		return ERR_PTR(-ENOMEM);
+	divider = kzalloc(sizeof(*divider), GFP_KERNEL);
+	if (!divider) {
+		ret = -ENOMEM;
+		goto err_free_gate;
+	}
 
 	divider->reg = reg;
 	divider->shift = DIV_RATIO_SHIFT;
@@ -261,9 +267,22 @@ static struct clk *davinci_pll_div_register(struct device *dev,
 		divider_ops = &clk_divider_ro_ops;
 	}
 
-	return clk_register_composite(dev, name, parent_names, num_parents,
-				      NULL, NULL, &divider->hw, divider_ops,
-				      &gate->hw, &clk_gate_ops, flags);
+	clk = clk_register_composite(dev, name, parent_names, num_parents,
+				     NULL, NULL, &divider->hw, divider_ops,
+				     &gate->hw, &clk_gate_ops, flags);
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto err_free_divider;
+	}
+
+	return clk;
+
+err_free_divider:
+	kfree(divider);
+err_free_gate:
+	kfree(gate);
+
+	return ERR_PTR(ret);
 }
 
 struct davinci_pllen_clk {
@@ -321,36 +340,17 @@ static int davinci_pllen_rate_change(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
-static struct davinci_pll_platform_data *davinci_pll_get_pdata(struct device *dev)
-{
-	struct davinci_pll_platform_data *pdata = dev_get_platdata(dev);
-
-	/*
-	 * Platform data is optional, so allocate a new struct if one was not
-	 * provided. For device tree, this will always be the case.
-	 */
-	if (!pdata)
-		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
-	if (!pdata)
-		return NULL;
-
-	/* for device tree, we need to fill in the struct */
-	if (dev->of_node)
-		pdata->cfgchip =
-			syscon_regmap_lookup_by_compatible("ti,da830-cfgchip");
-
-	return pdata;
-}
-
 static struct notifier_block davinci_pllen_notifier = {
 	.notifier_call = davinci_pllen_rate_change,
 };
 
 /**
  * davinci_pll_clk_register - Register a PLL clock
+ * @dev: The PLL platform device or NULL
  * @info: The device-specific clock info
  * @parent_name: The parent clock name
  * @base: The PLL's memory region
+ * @cfgchip: CFGCHIP syscon regmap for info->unlock_reg or NULL
  *
  * This creates a series of clocks that represent the PLL.
  *
@@ -366,9 +366,9 @@ static struct notifier_block davinci_pllen_notifier = {
 struct clk *davinci_pll_clk_register(struct device *dev,
 				     const struct davinci_pll_clk_info *info,
 				     const char *parent_name,
-				     void __iomem *base)
+				     void __iomem *base,
+				     struct regmap *cfgchip)
 {
-	struct davinci_pll_platform_data *pdata;
 	char prediv_name[MAX_NAME_SIZE];
 	char pllout_name[MAX_NAME_SIZE];
 	char postdiv_name[MAX_NAME_SIZE];
@@ -376,11 +376,12 @@ struct clk *davinci_pll_clk_register(struct device *dev,
 	struct clk_init_data init;
 	struct davinci_pll_clk *pllout;
 	struct davinci_pllen_clk *pllen;
-	struct clk *pllout_clk, *clk;
-
-	pdata = davinci_pll_get_pdata(dev);
-	if (!pdata)
-		return ERR_PTR(-ENOMEM);
+	struct clk *oscin_clk = NULL;
+	struct clk *prediv_clk = NULL;
+	struct clk *pllout_clk;
+	struct clk *postdiv_clk = NULL;
+	struct clk *pllen_clk;
+	int ret;
 
 	if (info->flags & PLL_HAS_CLKMODE) {
 		/*
@@ -392,10 +393,10 @@ struct clk *davinci_pll_clk_register(struct device *dev,
 		 * a number of different things. In this driver we use it to
 		 * mean the signal after the PLLCTL[CLKMODE] switch.
 		 */
-		clk = clk_register_fixed_factor(dev, OSCIN_CLK_NAME,
-						parent_name, 0, 1, 1);
-		if (IS_ERR(clk))
-			return clk;
+		oscin_clk = clk_register_fixed_factor(dev, OSCIN_CLK_NAME,
+						      parent_name, 0, 1, 1);
+		if (IS_ERR(oscin_clk))
+			return oscin_clk;
 
 		parent_name = OSCIN_CLK_NAME;
 	}
@@ -411,30 +412,34 @@ struct clk *davinci_pll_clk_register(struct device *dev,
 
 		/* Some? DM355 chips don't correctly report the PREDIV value */
 		if (info->flags & PLL_PREDIV_FIXED8)
-			clk = clk_register_fixed_factor(dev, prediv_name,
-						parent_name, flags, 1, 8);
+			prediv_clk = clk_register_fixed_factor(dev, prediv_name,
+							parent_name, flags, 1, 8);
 		else
-			clk = davinci_pll_div_register(dev, prediv_name,
+			prediv_clk = davinci_pll_div_register(dev, prediv_name,
 				parent_name, base + PREDIV, fixed, flags);
-		if (IS_ERR(clk))
-			return clk;
+		if (IS_ERR(prediv_clk)) {
+			ret = PTR_ERR(prediv_clk);
+			goto err_unregister_oscin;
+		}
 
 		parent_name = prediv_name;
 	}
 
 	/* Unlock writing to PLL registers */
 	if (info->unlock_reg) {
-		if (IS_ERR_OR_NULL(pdata->cfgchip))
+		if (IS_ERR_OR_NULL(cfgchip))
 			dev_warn(dev, "Failed to get CFGCHIP (%ld)\n",
-				 PTR_ERR(pdata->cfgchip));
+				 PTR_ERR(cfgchip));
 		else
-			regmap_write_bits(pdata->cfgchip, info->unlock_reg,
+			regmap_write_bits(cfgchip, info->unlock_reg,
 					  info->unlock_mask, 0);
 	}
 
-	pllout = devm_kzalloc(dev, sizeof(*pllout), GFP_KERNEL);
-	if (!pllout)
-		return ERR_PTR(-ENOMEM);
+	pllout = kzalloc(sizeof(*pllout), GFP_KERNEL);
+	if (!pllout) {
+		ret = -ENOMEM;
+		goto err_unregister_prediv;
+	}
 
 	snprintf(pllout_name, MAX_NAME_SIZE, "%s_pllout", info->name);
 
@@ -456,9 +461,11 @@ struct clk *davinci_pll_clk_register(struct device *dev,
 	pllout->pllm_min = info->pllm_min;
 	pllout->pllm_max = info->pllm_max;
 
-	pllout_clk = devm_clk_register(dev, &pllout->hw);
-	if (IS_ERR(pllout_clk))
-		return pllout_clk;
+	pllout_clk = clk_register(dev, &pllout->hw);
+	if (IS_ERR(pllout_clk)) {
+		ret = PTR_ERR(pllout_clk);
+		goto err_free_pllout;
+	}
 
 	clk_hw_set_rate_range(&pllout->hw, info->pllout_min_rate,
 			      info->pllout_max_rate);
@@ -474,17 +481,21 @@ struct clk *davinci_pll_clk_register(struct device *dev,
 		if (info->flags & PLL_POSTDIV_ALWAYS_ENABLED)
 			flags |= CLK_IS_CRITICAL;
 
-		clk = davinci_pll_div_register(dev, postdiv_name, parent_name,
-					       base + POSTDIV, fixed, flags);
-		if (IS_ERR(clk))
-			return clk;
+		postdiv_clk = davinci_pll_div_register(dev, postdiv_name,
+				parent_name, base + POSTDIV, fixed, flags);
+		if (IS_ERR(postdiv_clk)) {
+			ret = PTR_ERR(postdiv_clk);
+			goto err_unregister_pllout;
+		}
 
 		parent_name = postdiv_name;
 	}
 
-	pllen = devm_kzalloc(dev, sizeof(*pllout), GFP_KERNEL);
-	if (!pllen)
-		return ERR_PTR(-ENOMEM);
+	pllen = kzalloc(sizeof(*pllout), GFP_KERNEL);
+	if (!pllen) {
+		ret = -ENOMEM;
+		goto err_unregister_postdiv;
+	}
 
 	snprintf(pllen_name, MAX_NAME_SIZE, "%s_pllen", info->name);
 
@@ -497,17 +508,35 @@ struct clk *davinci_pll_clk_register(struct device *dev,
 	pllen->hw.init = &init;
 	pllen->base = base;
 
-	clk = devm_clk_register(dev, &pllen->hw);
-	if (IS_ERR(clk))
-		return clk;
+	pllen_clk = clk_register(dev, &pllen->hw);
+	if (IS_ERR(pllen_clk)) {
+		ret = PTR_ERR(pllen_clk);
+		goto err_free_pllen;
+	}
 
-	clk_notifier_register(clk, &davinci_pllen_notifier);
+	clk_notifier_register(pllen_clk, &davinci_pllen_notifier);
 
 	return pllout_clk;
+
+err_free_pllen:
+	kfree(pllen);
+err_unregister_postdiv:
+	clk_unregister(postdiv_clk);
+err_unregister_pllout:
+	clk_unregister(pllout_clk);
+err_free_pllout:
+	kfree(pllout);
+err_unregister_prediv:
+	clk_unregister(prediv_clk);
+err_unregister_oscin:
+	clk_unregister(oscin_clk);
+
+	return ERR_PTR(ret);
 }
 
 /**
  * davinci_pll_auxclk_register - Register bypass clock (AUXCLK)
+ * @dev: The PLL platform device or NULL
  * @name: The clock name
  * @base: The PLL memory region
  */
@@ -521,6 +550,7 @@ struct clk *davinci_pll_auxclk_register(struct device *dev,
 
 /**
  * davinci_pll_sysclkbp_clk_register - Register bypass divider clock (SYSCLKBP)
+ * @dev: The PLL platform device or NULL
  * @name: The clock name
  * @base: The PLL memory region
  */
@@ -535,6 +565,7 @@ struct clk *davinci_pll_sysclkbp_clk_register(struct device *dev,
 
 /**
  * davinci_pll_obsclk_register - Register oscillator divider clock (OBSCLK)
+ * @dev: The PLL platform device or NULL
  * @info: The clock info
  * @base: The PLL memory region
  */
@@ -546,9 +577,11 @@ davinci_pll_obsclk_register(struct device *dev,
 	struct clk_mux *mux;
 	struct clk_gate *gate;
 	struct clk_divider *divider;
+	struct clk *clk;
 	u32 oscdiv;
+	int ret;
 
-	mux = devm_kzalloc(dev, sizeof(*mux), GFP_KERNEL);
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
 	if (!mux)
 		return ERR_PTR(-ENOMEM);
 
@@ -556,16 +589,20 @@ davinci_pll_obsclk_register(struct device *dev,
 	mux->table = info->table;
 	mux->mask = info->ocsrc_mask;
 
-	gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
-	if (!gate)
-		return ERR_PTR(-ENOMEM);
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate) {
+		ret = -ENOMEM;
+		goto err_free_mux;
+	}
 
 	gate->reg = base + CKEN;
 	gate->bit_idx = CKEN_OBSCLK_SHIFT;
 
-	divider = devm_kzalloc(dev, sizeof(*divider), GFP_KERNEL);
-	if (!divider)
-		return ERR_PTR(-ENOMEM);
+	divider = kzalloc(sizeof(*divider), GFP_KERNEL);
+	if (!divider) {
+		ret = -ENOMEM;
+		goto err_free_gate;
+	}
 
 	divider->reg = base + OSCDIV;
 	divider->shift = DIV_RATIO_SHIFT;
@@ -576,11 +613,27 @@ davinci_pll_obsclk_register(struct device *dev,
 	oscdiv |= BIT(DIV_ENABLE_SHIFT);
 	writel(oscdiv, base + OSCDIV);
 
-	return clk_register_composite(dev, info->name, info->parent_names,
-				      info->num_parents,
-				      &mux->hw, &clk_mux_ops,
-				      &divider->hw, &clk_divider_ops,
-				      &gate->hw, &clk_gate_ops, 0);
+	clk = clk_register_composite(dev, info->name, info->parent_names,
+				     info->num_parents,
+				     &mux->hw, &clk_mux_ops,
+				     &divider->hw, &clk_divider_ops,
+				     &gate->hw, &clk_gate_ops, 0);
+
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto err_free_divider;
+	}
+
+	return clk;
+
+err_free_divider:
+	kfree(divider);
+err_free_gate:
+	kfree(gate);
+err_free_mux:
+	kfree(mux);
+
+	return ERR_PTR(ret);
 }
 
 /* The PLL SYSCLKn clocks have a mechanism for synchronizing rate changes. */
@@ -616,6 +669,7 @@ static struct notifier_block davinci_pll_sysclk_notifier = {
 
 /**
  * davinci_pll_sysclk_register - Register divider clocks (SYSCLKn)
+ * @dev: The PLL platform device or NULL
  * @info: The clock info
  * @base: The PLL memory region
  */
@@ -630,6 +684,7 @@ davinci_pll_sysclk_register(struct device *dev,
 	struct clk *clk;
 	u32 reg;
 	u32 flags = 0;
+	int ret;
 
 	/* PLLDIVn registers are not entirely consecutive */
 	if (info->id < 4)
@@ -637,16 +692,18 @@ davinci_pll_sysclk_register(struct device *dev,
 	else
 		reg = PLLDIV4 + 4 * (info->id - 4);
 
-	gate = devm_kzalloc(dev, sizeof(*gate), GFP_KERNEL);
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
 	if (!gate)
 		return ERR_PTR(-ENOMEM);
 
 	gate->reg = base + reg;
 	gate->bit_idx = DIV_ENABLE_SHIFT;
 
-	divider = devm_kzalloc(dev, sizeof(*divider), GFP_KERNEL);
-	if (!divider)
-		return ERR_PTR(-ENOMEM);
+	divider = kzalloc(sizeof(*divider), GFP_KERNEL);
+	if (!divider) {
+		ret = -ENOMEM;
+		goto err_free_gate;
+	}
 
 	divider->reg = base + reg;
 	divider->shift = DIV_RATIO_SHIFT;
@@ -668,22 +725,31 @@ davinci_pll_sysclk_register(struct device *dev,
 	clk = clk_register_composite(dev, info->name, &info->parent_name, 1,
 				     NULL, NULL, &divider->hw, divider_ops,
 				     &gate->hw, &clk_gate_ops, flags);
-	if (IS_ERR(clk))
-		return clk;
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		goto err_free_divider;
+	}
 
 	clk_notifier_register(clk, &davinci_pll_sysclk_notifier);
 
 	return clk;
+
+err_free_divider:
+	kfree(divider);
+err_free_gate:
+	kfree(gate);
+
+	return ERR_PTR(ret);
 }
 
-int of_davinci_pll_init(struct device *dev,
+int of_davinci_pll_init(struct device *dev, struct device_node *node,
 			const struct davinci_pll_clk_info *info,
 			const struct davinci_pll_obsclk_info *obsclk_info,
 			const struct davinci_pll_sysclk_info **div_info,
 			u8 max_sysclk_id,
-			void __iomem *base)
+			void __iomem *base,
+			struct regmap *cfgchip)
 {
-	struct device_node *node = dev->of_node;
 	struct device_node *child;
 	const char *parent_name;
 	struct clk *clk;
@@ -693,7 +759,7 @@ int of_davinci_pll_init(struct device *dev,
 	else
 		parent_name = OSCIN_CLK_NAME;
 
-	clk = davinci_pll_clk_register(dev, info, parent_name, base);
+	clk = davinci_pll_clk_register(dev, info, parent_name, base, cfgchip);
 	if (IS_ERR(clk)) {
 		dev_err(dev, "failed to register %s\n", info->name);
 		return PTR_ERR(clk);
@@ -711,13 +777,15 @@ int of_davinci_pll_init(struct device *dev,
 		int n_clks =  max_sysclk_id + 1;
 		int i;
 
-		clk_data = devm_kzalloc(dev, sizeof(*clk_data), GFP_KERNEL);
+		clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
 		if (!clk_data)
 			return -ENOMEM;
 
-		clks = devm_kmalloc_array(dev, n_clks, sizeof(*clks), GFP_KERNEL);
-		if (!clks)
+		clks = kmalloc_array(n_clks, sizeof(*clks), GFP_KERNEL);
+		if (!clks) {
+			kfree(clk_data);
 			return -ENOMEM;
+		}
 
 		clk_data->clks = clks;
 		clk_data->clk_num = n_clks;
@@ -770,6 +838,27 @@ int of_davinci_pll_init(struct device *dev,
 	return 0;
 }
 
+static struct davinci_pll_platform_data *davinci_pll_get_pdata(struct device *dev)
+{
+	struct davinci_pll_platform_data *pdata = dev_get_platdata(dev);
+
+	/*
+	 * Platform data is optional, so allocate a new struct if one was not
+	 * provided. For device tree, this will always be the case.
+	 */
+	if (!pdata)
+		pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	/* for device tree, we need to fill in the struct */
+	if (dev->of_node)
+		pdata->cfgchip =
+			syscon_regmap_lookup_by_compatible("ti,da830-cfgchip");
+
+	return pdata;
+}
+
 static const struct of_device_id davinci_pll_of_match[] = {
 	{ .compatible = "ti,da850-pll0", .data = of_da850_pll0_init },
 	{ .compatible = "ti,da850-pll1", .data = of_da850_pll1_init },
@@ -791,11 +880,13 @@ static const struct platform_device_id davinci_pll_id_table[] = {
 	{ }
 };
 
-typedef int (*davinci_pll_init)(struct device *dev, void __iomem *base);
+typedef int (*davinci_pll_init)(struct device *dev, void __iomem *base,
+				struct regmap *cfgchip);
 
 static int davinci_pll_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct davinci_pll_platform_data *pdata;
 	const struct of_device_id *of_id;
 	davinci_pll_init pll_init = NULL;
 	struct resource *res;
@@ -812,12 +903,18 @@ static int davinci_pll_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	pdata = davinci_pll_get_pdata(dev);
+	if (!pdata) {
+		dev_err(dev, "missing platform data\n");
+		return -EINVAL;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	return pll_init(dev, base);
+	return pll_init(dev, base, pdata->cfgchip);
 }
 
 static struct platform_driver davinci_pll_driver = {
diff --git a/drivers/clk/davinci/pll.h b/drivers/clk/davinci/pll.h
index b1b6fb23f972..562652fc0759 100644
--- a/drivers/clk/davinci/pll.h
+++ b/drivers/clk/davinci/pll.h
@@ -11,6 +11,7 @@
 #include <linux/bitops.h>
 #include <linux/clk-provider.h>
 #include <linux/of.h>
+#include <linux/regmap.h>
 #include <linux/types.h>
 
 #define PLL_HAS_CLKMODE			BIT(0) /* PLL has PLLCTL[CLKMODE] */
@@ -94,7 +95,8 @@ struct davinci_pll_obsclk_info {
 struct clk *davinci_pll_clk_register(struct device *dev,
 				     const struct davinci_pll_clk_info *info,
 				     const char *parent_name,
-				     void __iomem *base);
+				     void __iomem *base,
+				     struct regmap *cfgchip);
 struct clk *davinci_pll_auxclk_register(struct device *dev,
 					const char *name,
 					void __iomem *base);
@@ -110,32 +112,24 @@ davinci_pll_sysclk_register(struct device *dev,
 			    const struct davinci_pll_sysclk_info *info,
 			    void __iomem *base);
 
-int of_davinci_pll_init(struct device *dev,
+int of_davinci_pll_init(struct device *dev, struct device_node *node,
 			const struct davinci_pll_clk_info *info,
 			const struct davinci_pll_obsclk_info *obsclk_info,
 			const struct davinci_pll_sysclk_info **div_info,
 			u8 max_sysclk_id,
-			void __iomem *base);
+			void __iomem *base,
+			struct regmap *cfgchip);
 
 /* Platform-specific callbacks */
 
-int da830_pll_init(struct device *dev, void __iomem *base);
+int da850_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int of_da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int of_da850_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 
-int da850_pll0_init(struct device *dev, void __iomem *base);
-int da850_pll1_init(struct device *dev, void __iomem *base);
-int of_da850_pll0_init(struct device *dev, void __iomem *base);
-int of_da850_pll1_init(struct device *dev, void __iomem *base);
+int dm355_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 
-int dm355_pll1_init(struct device *dev, void __iomem *base);
-int dm355_pll2_init(struct device *dev, void __iomem *base);
+int dm644x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 
-int dm365_pll1_init(struct device *dev, void __iomem *base);
-int dm365_pll2_init(struct device *dev, void __iomem *base);
-
-int dm644x_pll1_init(struct device *dev, void __iomem *base);
-int dm644x_pll2_init(struct device *dev, void __iomem *base);
-
-int dm646x_pll1_init(struct device *dev, void __iomem *base);
-int dm646x_pll2_init(struct device *dev, void __iomem *base);
+int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 
 #endif /* __CLK_DAVINCI_PLL_H___ */
diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h
new file mode 100644
index 000000000000..ebdd9df1c0ef
--- /dev/null
+++ b/include/linux/clk/davinci.h
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Clock drivers for TI DaVinci PLL and PSC controllers
+ *
+ * Copyright (C) 2018 David Lechner <david@lechnology.com>
+ */
+
+#ifndef __LINUX_CLK_DAVINCI_PLL_H___
+#define __LINUX_CLK_DAVINCI_PLL_H___
+
+#include <linux/device.h>
+#include <linux/regmap.h>
+
+/* function for registering clocks in early boot */
+
+int da830_pll_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int dm355_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int dm365_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int dm365_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int dm644x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int dm646x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+
+#endif /* __LINUX_CLK_DAVINCI_PLL_H___ */
-- 
2.17.0

^ permalink raw reply related

* [PATCH 7/9] clk: davinci: da850-pll: change PLL0 to CLK_OF_DECLARE
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

PLL0 on davinci/da850-type device needs to be registered early in boot
because it is needed for clocksource/clockevent. Change the driver
to use CLK_OF_DECLARE for this special case.

Reviewed-by: Sekhar Nori <nsekhar@ti.com>
Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/clk/davinci/pll-da850.c | 21 +++++++++++++++++----
 drivers/clk/davinci/pll.c       |  4 +++-
 drivers/clk/davinci/pll.h       |  2 +-
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/davinci/pll-da850.c b/drivers/clk/davinci/pll-da850.c
index 59cc2e3733f9..0f7198191ea2 100644
--- a/drivers/clk/davinci/pll-da850.c
+++ b/drivers/clk/davinci/pll-da850.c
@@ -13,6 +13,8 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/mfd/da8xx-cfgchip.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_address.h>
 #include <linux/of.h>
 #include <linux/types.h>
 
@@ -136,11 +138,22 @@ static const struct davinci_pll_sysclk_info *da850_pll0_sysclk_info[] = {
 	NULL
 };
 
-int of_da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip)
+void of_da850_pll0_init(struct device_node *node)
 {
-	return of_davinci_pll_init(dev, dev->of_node, &da850_pll0_info,
-				   &da850_pll0_obsclk_info,
-				   da850_pll0_sysclk_info, 7, base, cfgchip);
+	void __iomem *base;
+	struct regmap *cfgchip;
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: ioremap failed\n", __func__);
+		return;
+	}
+
+	cfgchip = syscon_regmap_lookup_by_compatible("ti,da830-cfgchip");
+
+	of_davinci_pll_init(NULL, node, &da850_pll0_info,
+			    &da850_pll0_obsclk_info,
+			    da850_pll0_sysclk_info, 7, base, cfgchip);
 }
 
 static const struct davinci_pll_clk_info da850_pll1_info = {
diff --git a/drivers/clk/davinci/pll.c b/drivers/clk/davinci/pll.c
index 2eb981e61185..84a343060bc8 100644
--- a/drivers/clk/davinci/pll.c
+++ b/drivers/clk/davinci/pll.c
@@ -859,8 +859,10 @@ static struct davinci_pll_platform_data *davinci_pll_get_pdata(struct device *de
 	return pdata;
 }
 
+/* needed in early boot for clocksource/clockevent */
+CLK_OF_DECLARE(da850_pll0, "ti,da850-pll0", of_da850_pll0_init);
+
 static const struct of_device_id davinci_pll_of_match[] = {
-	{ .compatible = "ti,da850-pll0", .data = of_da850_pll0_init },
 	{ .compatible = "ti,da850-pll1", .data = of_da850_pll1_init },
 	{ }
 };
diff --git a/drivers/clk/davinci/pll.h b/drivers/clk/davinci/pll.h
index 562652fc0759..b2e5c4496645 100644
--- a/drivers/clk/davinci/pll.h
+++ b/drivers/clk/davinci/pll.h
@@ -123,7 +123,7 @@ int of_davinci_pll_init(struct device *dev, struct device_node *node,
 /* Platform-specific callbacks */
 
 int da850_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
-int of_da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+void of_da850_pll0_init(struct device_node *node);
 int of_da850_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 
 int dm355_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
-- 
2.17.0

^ permalink raw reply related

* [PATCH 8/9] clk: davinci: psc: allow for dev == NULL
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

On some davinci SoCs, we need to register the PSC clocks during early
boot because they are needed for clocksource/clockevent. These changes
allow for dev == NULL because in this case, we won't have a platform
device for the clocks.

Signed-off-by: David Lechner <david@lechnology.com>
Reviewed-by: Sekhar Nori <nsekhar@ti.com>
---
 drivers/clk/davinci/psc-dm355.c  |  3 +-
 drivers/clk/davinci/psc-dm365.c  |  3 +-
 drivers/clk/davinci/psc-dm644x.c |  3 +-
 drivers/clk/davinci/psc-dm646x.c |  3 +-
 drivers/clk/davinci/psc.c        | 58 ++++++++++++++++++++++++--------
 include/linux/clk/davinci.h      |  5 +++
 6 files changed, 57 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/davinci/psc-dm355.c b/drivers/clk/davinci/psc-dm355.c
index 128e7345b20c..ddd250107c4e 100644
--- a/drivers/clk/davinci/psc-dm355.c
+++ b/drivers/clk/davinci/psc-dm355.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/clk/davinci.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/init.h>
@@ -68,7 +69,7 @@ static const struct davinci_lpsc_clk_info dm355_psc_info[] = {
 	{ }
 };
 
-static int dm355_psc_init(struct device *dev, void __iomem *base)
+int dm355_psc_init(struct device *dev, void __iomem *base)
 {
 	return davinci_psc_register_clocks(dev, dm355_psc_info, 42, base);
 }
diff --git a/drivers/clk/davinci/psc-dm365.c b/drivers/clk/davinci/psc-dm365.c
index 289af3913fb0..8c73086cc676 100644
--- a/drivers/clk/davinci/psc-dm365.c
+++ b/drivers/clk/davinci/psc-dm365.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/clk/davinci.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/init.h>
@@ -86,7 +87,7 @@ static const struct davinci_lpsc_clk_info dm365_psc_info[] = {
 	{ }
 };
 
-static int dm365_psc_init(struct device *dev, void __iomem *base)
+int dm365_psc_init(struct device *dev, void __iomem *base)
 {
 	return davinci_psc_register_clocks(dev, dm365_psc_info, 52, base);
 }
diff --git a/drivers/clk/davinci/psc-dm644x.c b/drivers/clk/davinci/psc-dm644x.c
index c22367baa46f..fc0230e3a3d6 100644
--- a/drivers/clk/davinci/psc-dm644x.c
+++ b/drivers/clk/davinci/psc-dm644x.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/clk/davinci.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/init.h>
@@ -63,7 +64,7 @@ static const struct davinci_lpsc_clk_info dm644x_psc_info[] = {
 	{ }
 };
 
-static int dm644x_psc_init(struct device *dev, void __iomem *base)
+int dm644x_psc_init(struct device *dev, void __iomem *base)
 {
 	return davinci_psc_register_clocks(dev, dm644x_psc_info, 41, base);
 }
diff --git a/drivers/clk/davinci/psc-dm646x.c b/drivers/clk/davinci/psc-dm646x.c
index 468ef86ea40b..c3f82ed70a80 100644
--- a/drivers/clk/davinci/psc-dm646x.c
+++ b/drivers/clk/davinci/psc-dm646x.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/clk/davinci.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
 #include <linux/init.h>
@@ -58,7 +59,7 @@ static const struct davinci_lpsc_clk_info dm646x_psc_info[] = {
 	{ }
 };
 
-static int dm646x_psc_init(struct device *dev, void __iomem *base)
+int dm646x_psc_init(struct device *dev, void __iomem *base)
 {
 	return davinci_psc_register_clocks(dev, dm646x_psc_info, 46, base);
 }
diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
index ce170e600f09..6326ba1fe3cc 100644
--- a/drivers/clk/davinci/psc.c
+++ b/drivers/clk/davinci/psc.c
@@ -15,6 +15,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/clk.h>
+#include <linux/clk/davinci.h>
 #include <linux/clkdev.h>
 #include <linux/err.h>
 #include <linux/of_address.h>
@@ -63,7 +64,7 @@ struct davinci_psc_data {
 
 /**
  * struct davinci_lpsc_clk - LPSC clock structure
- * @dev: the device that provides this LPSC
+ * @dev: the device that provides this LPSC or NULL
  * @hw: clk_hw for the LPSC
  * @pm_domain: power domain for the LPSC
  * @genpd_clk: clock reference owned by @pm_domain
@@ -221,6 +222,7 @@ static void davinci_psc_genpd_detach_dev(struct generic_pm_domain *pm_domain,
 
 /**
  * davinci_lpsc_clk_register - register LPSC clock
+ * @dev: the clocks's device or NULL
  * @name: name of this clock
  * @parent_name: name of clock's parent
  * @regmap: PSC MMIO region
@@ -238,7 +240,7 @@ davinci_lpsc_clk_register(struct device *dev, const char *name,
 	int ret;
 	bool is_on;
 
-	lpsc = devm_kzalloc(dev, sizeof(*lpsc), GFP_KERNEL);
+	lpsc = kzalloc(sizeof(*lpsc), GFP_KERNEL);
 	if (!lpsc)
 		return ERR_PTR(-ENOMEM);
 
@@ -261,9 +263,15 @@ davinci_lpsc_clk_register(struct device *dev, const char *name,
 	lpsc->pd = pd;
 	lpsc->flags = flags;
 
-	ret = devm_clk_hw_register(dev, &lpsc->hw);
-	if (ret < 0)
+	ret = clk_hw_register(dev, &lpsc->hw);
+	if (ret < 0) {
+		kfree(lpsc);
 		return ERR_PTR(ret);
+	}
+
+	/* for now, genpd is only registered when using device-tree */
+	if (!dev || !dev->of_node)
+		return lpsc;
 
 	/* genpd attach needs a way to look up this clock */
 	ret = clk_hw_register_clkdev(&lpsc->hw, name, best_dev_name(dev));
@@ -378,13 +386,15 @@ __davinci_psc_register_clocks(struct device *dev,
 	struct regmap *regmap;
 	int i, ret;
 
-	psc = devm_kzalloc(dev, sizeof(*psc), GFP_KERNEL);
+	psc = kzalloc(sizeof(*psc), GFP_KERNEL);
 	if (!psc)
 		return ERR_PTR(-ENOMEM);
 
-	clks = devm_kmalloc_array(dev, num_clks, sizeof(*clks), GFP_KERNEL);
-	if (!clks)
-		return ERR_PTR(-ENOMEM);
+	clks = kmalloc_array(num_clks, sizeof(*clks), GFP_KERNEL);
+	if (!clks) {
+		ret = -ENOMEM;
+		goto err_free_psc;
+	}
 
 	psc->clk_data.clks = clks;
 	psc->clk_data.clk_num = num_clks;
@@ -396,16 +406,20 @@ __davinci_psc_register_clocks(struct device *dev,
 	for (i = 0; i < num_clks; i++)
 		clks[i] = ERR_PTR(-ENOENT);
 
-	pm_domains = devm_kcalloc(dev, num_clks, sizeof(*pm_domains), GFP_KERNEL);
-	if (!pm_domains)
-		return ERR_PTR(-ENOMEM);
+	pm_domains = kcalloc(num_clks, sizeof(*pm_domains), GFP_KERNEL);
+	if (!pm_domains) {
+		ret = -ENOMEM;
+		goto err_free_clks;
+	}
 
 	psc->pm_data.domains = pm_domains;
 	psc->pm_data.num_domains = num_clks;
 
-	regmap = devm_regmap_init_mmio(dev, base, &davinci_psc_regmap_config);
-	if (IS_ERR(regmap))
-		return ERR_CAST(regmap);
+	regmap = regmap_init_mmio(dev, base, &davinci_psc_regmap_config);
+	if (IS_ERR(regmap)) {
+		ret = PTR_ERR(regmap);
+		goto err_free_pm_domains;
+	}
 
 	for (; info->name; info++) {
 		struct davinci_lpsc_clk *lpsc;
@@ -423,6 +437,13 @@ __davinci_psc_register_clocks(struct device *dev,
 		pm_domains[info->md] = &lpsc->pm_domain;
 	}
 
+	/*
+	 * for now, a reset controller is only registered when there is a device
+	 * to associate it with.
+	 */
+	if (!dev)
+		return psc;
+
 	psc->rcdev.ops = &davinci_psc_reset_ops;
 	psc->rcdev.owner = THIS_MODULE;
 	psc->rcdev.dev = dev;
@@ -436,6 +457,15 @@ __davinci_psc_register_clocks(struct device *dev,
 		dev_warn(dev, "Failed to register reset controller (%d)\n", ret);
 
 	return psc;
+
+err_free_pm_domains:
+	kfree(pm_domains);
+err_free_clks:
+	kfree(clks);
+err_free_psc:
+	kfree(psc);
+
+	return ERR_PTR(ret);
 }
 
 int davinci_psc_register_clocks(struct device *dev,
diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h
index ebdd9df1c0ef..62764c5cc86e 100644
--- a/include/linux/clk/davinci.h
+++ b/include/linux/clk/davinci.h
@@ -21,4 +21,9 @@ int dm365_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgch
 int dm644x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 int dm646x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 
+int dm355_psc_init(struct device *dev, void __iomem *base);
+int dm365_psc_init(struct device *dev, void __iomem *base);
+int dm644x_psc_init(struct device *dev, void __iomem *base);
+int dm646x_psc_init(struct device *dev, void __iomem *base);
+
 #endif /* __LINUX_CLK_DAVINCI_PLL_H___ */
-- 
2.17.0

^ permalink raw reply related

* [PATCH 9/9] clk: davinci: Fix link errors when not all SoCs are enabled
From: David Lechner @ 2018-05-25 18:11 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180525181150.17873-1-david@lechnology.com>

This fixes linker errors due to undefined symbols when one or more of
the TI DaVinci SoCs is not enabled in the kernel config.

Signed-off-by: David Lechner <david@lechnology.com>
---
 drivers/clk/davinci/pll.c   | 16 ++++++++++++++++
 drivers/clk/davinci/pll.h   | 11 ++++++++---
 drivers/clk/davinci/psc.c   | 14 ++++++++++++++
 drivers/clk/davinci/psc.h   | 12 ++++++++++++
 include/linux/clk/davinci.h | 19 +++++++++++++++----
 5 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/davinci/pll.c b/drivers/clk/davinci/pll.c
index 84a343060bc8..65abd371692d 100644
--- a/drivers/clk/davinci/pll.c
+++ b/drivers/clk/davinci/pll.c
@@ -860,25 +860,41 @@ static struct davinci_pll_platform_data *davinci_pll_get_pdata(struct device *de
 }
 
 /* needed in early boot for clocksource/clockevent */
+#ifdef CONFIG_ARCH_DAVINCI_DA850
 CLK_OF_DECLARE(da850_pll0, "ti,da850-pll0", of_da850_pll0_init);
+#endif
 
 static const struct of_device_id davinci_pll_of_match[] = {
+#ifdef CONFIG_ARCH_DAVINCI_DA850
 	{ .compatible = "ti,da850-pll1", .data = of_da850_pll1_init },
+#endif
 	{ }
 };
 
 static const struct platform_device_id davinci_pll_id_table[] = {
+#ifdef CONFIG_ARCH_DAVINCI_DA830
 	{ .name = "da830-pll",   .driver_data = (kernel_ulong_t)da830_pll_init   },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DA850
 	{ .name = "da850-pll0",  .driver_data = (kernel_ulong_t)da850_pll0_init  },
 	{ .name = "da850-pll1",  .driver_data = (kernel_ulong_t)da850_pll1_init  },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM355
 	{ .name = "dm355-pll1",  .driver_data = (kernel_ulong_t)dm355_pll1_init  },
 	{ .name = "dm355-pll2",  .driver_data = (kernel_ulong_t)dm355_pll2_init  },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM365
 	{ .name = "dm365-pll1",  .driver_data = (kernel_ulong_t)dm365_pll1_init  },
 	{ .name = "dm365-pll2",  .driver_data = (kernel_ulong_t)dm365_pll2_init  },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM644x
 	{ .name = "dm644x-pll1", .driver_data = (kernel_ulong_t)dm644x_pll1_init },
 	{ .name = "dm644x-pll2", .driver_data = (kernel_ulong_t)dm644x_pll2_init },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM646x
 	{ .name = "dm646x-pll1", .driver_data = (kernel_ulong_t)dm646x_pll1_init },
 	{ .name = "dm646x-pll2", .driver_data = (kernel_ulong_t)dm646x_pll2_init },
+#endif
 	{ }
 };
 
diff --git a/drivers/clk/davinci/pll.h b/drivers/clk/davinci/pll.h
index b2e5c4496645..7cc354dd29e2 100644
--- a/drivers/clk/davinci/pll.h
+++ b/drivers/clk/davinci/pll.h
@@ -122,14 +122,19 @@ int of_davinci_pll_init(struct device *dev, struct device_node *node,
 
 /* Platform-specific callbacks */
 
+#ifdef CONFIG_ARCH_DAVINCI_DA850
 int da850_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 void of_da850_pll0_init(struct device_node *node);
 int of_da850_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
-
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM355
 int dm355_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
-
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM644x
 int dm644x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
-
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM646x
 int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+#endif
 
 #endif /* __CLK_DAVINCI_PLL_H___ */
diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
index 6326ba1fe3cc..fffbed5e263b 100644
--- a/drivers/clk/davinci/psc.c
+++ b/drivers/clk/davinci/psc.c
@@ -513,20 +513,34 @@ int of_davinci_psc_clk_init(struct device *dev,
 }
 
 static const struct of_device_id davinci_psc_of_match[] = {
+#ifdef CONFIG_ARCH_DAVINCI_DA850
 	{ .compatible = "ti,da850-psc0", .data = &of_da850_psc0_init_data },
 	{ .compatible = "ti,da850-psc1", .data = &of_da850_psc1_init_data },
+#endif
 	{ }
 };
 
 static const struct platform_device_id davinci_psc_id_table[] = {
+#ifdef CONFIG_ARCH_DAVINCI_DA830
 	{ .name = "da830-psc0", .driver_data = (kernel_ulong_t)&da830_psc0_init_data },
 	{ .name = "da830-psc1", .driver_data = (kernel_ulong_t)&da830_psc1_init_data },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DA850
 	{ .name = "da850-psc0", .driver_data = (kernel_ulong_t)&da850_psc0_init_data },
 	{ .name = "da850-psc1", .driver_data = (kernel_ulong_t)&da850_psc1_init_data },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM355
 	{ .name = "dm355-psc",  .driver_data = (kernel_ulong_t)&dm355_psc_init_data  },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM365
 	{ .name = "dm365-psc",  .driver_data = (kernel_ulong_t)&dm365_psc_init_data  },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM644x
 	{ .name = "dm644x-psc", .driver_data = (kernel_ulong_t)&dm644x_psc_init_data },
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM646x
 	{ .name = "dm646x-psc", .driver_data = (kernel_ulong_t)&dm646x_psc_init_data },
+#endif
 	{ }
 };
 
diff --git a/drivers/clk/davinci/psc.h b/drivers/clk/davinci/psc.h
index c2a7df6413fe..6a42529d31a9 100644
--- a/drivers/clk/davinci/psc.h
+++ b/drivers/clk/davinci/psc.h
@@ -94,15 +94,27 @@ struct davinci_psc_init_data {
 	int (*psc_init)(struct device *dev, void __iomem *base);
 };
 
+#ifdef CONFIG_ARCH_DAVINCI_DA830
 extern const struct davinci_psc_init_data da830_psc0_init_data;
 extern const struct davinci_psc_init_data da830_psc1_init_data;
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DA850
 extern const struct davinci_psc_init_data da850_psc0_init_data;
 extern const struct davinci_psc_init_data da850_psc1_init_data;
 extern const struct davinci_psc_init_data of_da850_psc0_init_data;
 extern const struct davinci_psc_init_data of_da850_psc1_init_data;
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM355
 extern const struct davinci_psc_init_data dm355_psc_init_data;
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM356
 extern const struct davinci_psc_init_data dm365_psc_init_data;
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM644x
 extern const struct davinci_psc_init_data dm644x_psc_init_data;
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM646x
 extern const struct davinci_psc_init_data dm646x_psc_init_data;
+#endif
 
 #endif /* __CLK_DAVINCI_PSC_H__ */
diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h
index 62764c5cc86e..8a7b5cd7eac0 100644
--- a/include/linux/clk/davinci.h
+++ b/include/linux/clk/davinci.h
@@ -13,17 +13,28 @@
 
 /* function for registering clocks in early boot */
 
+#ifdef CONFIG_ARCH_DAVINCI_DA830
 int da830_pll_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DA850
 int da850_pll0_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM355
 int dm355_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
+int dm355_psc_init(struct device *dev, void __iomem *base);
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM365
 int dm365_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 int dm365_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
-int dm644x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
-int dm646x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
-
-int dm355_psc_init(struct device *dev, void __iomem *base);
 int dm365_psc_init(struct device *dev, void __iomem *base);
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM644x
+int dm644x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 int dm644x_psc_init(struct device *dev, void __iomem *base);
+#endif
+#ifdef CONFIG_ARCH_DAVINCI_DM646x
+int dm646x_pll1_init(struct device *dev, void __iomem *base, struct regmap *cfgchip);
 int dm646x_psc_init(struct device *dev, void __iomem *base);
+#endif
 
 #endif /* __LINUX_CLK_DAVINCI_PLL_H___ */
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH v11 00/27] ARM: davinci: convert to common clock framework​
From: David Lechner @ 2018-05-25 18:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <850a0ec9-53aa-02a5-ecbd-e068b13b2764@ti.com>

On 05/22/2018 04:38 AM, Sekhar Nori wrote:
> Hi David,
> 
> On Friday 18 May 2018 10:18 PM, David Lechner wrote:
>> This series converts mach-davinci to use the common clock framework.
>>
>> The series works like this, the first 3 patches fix some issues with the clock
>> drivers that have already been accepted into the mainline kernel.
>>
>> Then, starting with "ARM: davinci: pass clock as parameter to
>> davinci_timer_init()", we get the mach code ready for the switch by adding the
>> code needed for the new clock drivers and adding #ifndef CONFIG_COMMON_CLK
>> around the legacy clocks so that we can switch easily between the old and the
>> new.
>>
>> "ARM: davinci: switch to common clock framework" actually flips the switch
>> to start using the new clock drivers. Then the next 8 patches remove all
>> of the old clock code.
>>
>> The final four patches add device tree clock support to the one SoC that
>> supports it.
>>
>> This series has been tested on TI OMAP-L138 LCDK (both device tree and legacy
>> board file).
> 
> If you do end up sending a v12, you can leave out the mach-davinci
> portions unless there are any changes you need to make. I will pick them
> up from this series once the driver dependencies are merged.
> 
> I do hope the drivers/clk/* changes can be merged from v4.18.
> 

I have resent all of the clk patches (including all of the ones I listed as
dependencies in addition to the three remaining in this series) under the
cover "clk: davinci: outstanding fixes?".

I also found that we need to add power-domains properties to the PWM nodes
in "ARM: dts: da850: Add clocks". I probably should just take your advice
and just globally added them even if they are not documented for some types
ofnodes.

^ permalink raw reply

* [PATCH] arm64: dts: qcom: msm8996: Use UFS_GDSC for UFS
From: Bjorn Andersson @ 2018-05-25 18:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAFp+6iH5bS5rmqZ1oy09COb2Rzuz6zbN7pFwxz0WTJNOYTTbkg@mail.gmail.com>

On Fri 25 May 05:51 PDT 2018, Vivek Gautam wrote:

> Hi Bjorn,
> 
> On Fri, May 25, 2018 at 4:01 AM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> > The UFS host controller occationally (20%) fails to enable
> > gcc_ufs_axi_clk because the UFS GDSC is not enabled. In most cases it's
> > enabled through the UFS phy driver, but to make sure it's enabled let's
> > enable it directly from the UFS host controller directly as well.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > index 380e14591686..03c7904bda14 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > @@ -674,6 +674,8 @@
> >                         vccq-max-microamp = <450000>;
> >                         vccq2-max-microamp = <450000>;
> >
> > +                       power-domains = <&gcc UFS_GDSC>;
> > +
> 
> We shouldn't need power-domain with the phy. UFS_GDSC should
> be attached to the controller, as the phy is powered up only after
> the controller is power-up, and during collapse too, we turn off
> the phy first.

Afaict you're right, it should only be needed by the resources for the
HCD.

> Can you try testing keeping UFS_GDSC only with ufs controller and
> remove it from the ufs-phy node? We are doing same on the 4.14 release
> branch too for db820.

I test booted this 10 times, with 100% success :)

> I apologize to have missed this in your patch for ufs-related dt nodes.
> Can we please fix this now?

I'll send a v2, that moves the power-domain to the HCD, rather than just
adding it there too. Thanks for the quick review!

Regards,
Bjorn

^ permalink raw reply

* [PATCH v2] arm64: dts: qcom: msm8996: Move UFS_GDSC to UFS HCD
From: Bjorn Andersson @ 2018-05-25 18:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180524223122.12601-1-bjorn.andersson@linaro.org>

The UFS_GDSC backs the resources needed by the UFS HCD, rather than the
PHY. This results in the UFS HCD occationally failing to enable some of
its clocks.

Move the UFS_GDSC reference to the UFS HCD node instead, to correct
this.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

Changes since v1:
- Dropped UFS_GDSC from phy node

 arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 380e14591686..8c7f9ca25b53 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -654,8 +654,6 @@
 			clocks = <&rpmcc RPM_SMD_LN_BB_CLK>,
 				 <&gcc GCC_UFS_CLKREF_CLK>;
 			status = "disabled";
-
-			power-domains = <&gcc UFS_GDSC>;
 		};
 
 		ufshc at 624000 {
@@ -674,6 +672,8 @@
 			vccq-max-microamp = <450000>;
 			vccq2-max-microamp = <450000>;
 
+			power-domains = <&gcc UFS_GDSC>;
+
 			clock-names =
 				"core_clk_src",
 				"core_clk",
-- 
2.17.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox