* [PATCH v7 07/10] kernel/jump_label: abstract jump_entry member accessors
From: Ard Biesheuvel @ 2018-01-02 20:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102200549.22984-1-ard.biesheuvel@linaro.org>
In preparation of allowing architectures to use relative references
in jump_label entries [which can dramatically reduce the memory
footprint], introduce abstractions for references to the 'code' and
'key' members of struct jump_entry.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm/include/asm/jump_label.h | 27 ++++++++++++++
arch/arm64/include/asm/jump_label.h | 27 ++++++++++++++
arch/mips/include/asm/jump_label.h | 27 ++++++++++++++
arch/powerpc/include/asm/jump_label.h | 27 ++++++++++++++
arch/s390/include/asm/jump_label.h | 27 ++++++++++++++
arch/sparc/include/asm/jump_label.h | 27 ++++++++++++++
arch/tile/include/asm/jump_label.h | 27 ++++++++++++++
arch/x86/include/asm/jump_label.h | 27 ++++++++++++++
kernel/jump_label.c | 38 +++++++++-----------
9 files changed, 232 insertions(+), 22 deletions(-)
diff --git a/arch/arm/include/asm/jump_label.h b/arch/arm/include/asm/jump_label.h
index e12d7d096fc0..7b05b404063a 100644
--- a/arch/arm/include/asm/jump_label.h
+++ b/arch/arm/include/asm/jump_label.h
@@ -45,5 +45,32 @@ struct jump_entry {
jump_label_t key;
};
+static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
+{
+ return entry->code;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+ return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+ return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+ return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+ entry->code = 0;
+}
+
+#define jump_label_swap NULL
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
index 1b5e0e843c3a..9d6e46355c89 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -62,5 +62,32 @@ struct jump_entry {
jump_label_t key;
};
+static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
+{
+ return entry->code;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+ return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+ return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+ return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+ entry->code = 0;
+}
+
+#define jump_label_swap NULL
+
#endif /* __ASSEMBLY__ */
#endif /* __ASM_JUMP_LABEL_H */
diff --git a/arch/mips/include/asm/jump_label.h b/arch/mips/include/asm/jump_label.h
index e77672539e8e..70df9293dc49 100644
--- a/arch/mips/include/asm/jump_label.h
+++ b/arch/mips/include/asm/jump_label.h
@@ -66,5 +66,32 @@ struct jump_entry {
jump_label_t key;
};
+static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
+{
+ return entry->code;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+ return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+ return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+ return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+ entry->code = 0;
+}
+
+#define jump_label_swap NULL
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_MIPS_JUMP_LABEL_H */
diff --git a/arch/powerpc/include/asm/jump_label.h b/arch/powerpc/include/asm/jump_label.h
index 9a287e0ac8b1..412b2699c9f6 100644
--- a/arch/powerpc/include/asm/jump_label.h
+++ b/arch/powerpc/include/asm/jump_label.h
@@ -59,6 +59,33 @@ struct jump_entry {
jump_label_t key;
};
+static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
+{
+ return entry->code;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+ return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+ return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+ return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+ entry->code = 0;
+}
+
+#define jump_label_swap NULL
+
#else
#define ARCH_STATIC_BRANCH(LABEL, KEY) \
1098: nop; \
diff --git a/arch/s390/include/asm/jump_label.h b/arch/s390/include/asm/jump_label.h
index 40f651292aa7..1ecfd46835d9 100644
--- a/arch/s390/include/asm/jump_label.h
+++ b/arch/s390/include/asm/jump_label.h
@@ -50,5 +50,32 @@ struct jump_entry {
jump_label_t key;
};
+static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
+{
+ return entry->code;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+ return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+ return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+ return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+ entry->code = 0;
+}
+
+#define jump_label_swap NULL
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/sparc/include/asm/jump_label.h b/arch/sparc/include/asm/jump_label.h
index 94eb529dcb77..18e893687f7c 100644
--- a/arch/sparc/include/asm/jump_label.h
+++ b/arch/sparc/include/asm/jump_label.h
@@ -48,5 +48,32 @@ struct jump_entry {
jump_label_t key;
};
+static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
+{
+ return entry->code;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+ return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+ return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+ return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+ entry->code = 0;
+}
+
+#define jump_label_swap NULL
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/tile/include/asm/jump_label.h b/arch/tile/include/asm/jump_label.h
index cde7573f397b..86acaa6ff33d 100644
--- a/arch/tile/include/asm/jump_label.h
+++ b/arch/tile/include/asm/jump_label.h
@@ -55,4 +55,31 @@ struct jump_entry {
jump_label_t key;
};
+static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
+{
+ return entry->code;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+ return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+ return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+ return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+ entry->code = 0;
+}
+
+#define jump_label_swap NULL
+
#endif /* _ASM_TILE_JUMP_LABEL_H */
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 8c0de4282659..009ff2699d07 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -74,6 +74,33 @@ struct jump_entry {
jump_label_t key;
};
+static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
+{
+ return entry->code;
+}
+
+static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
+{
+ return (struct static_key *)((unsigned long)entry->key & ~1UL);
+}
+
+static inline bool jump_entry_is_branch(const struct jump_entry *entry)
+{
+ return (unsigned long)entry->key & 1UL;
+}
+
+static inline bool jump_entry_is_module_init(const struct jump_entry *entry)
+{
+ return entry->code == 0;
+}
+
+static inline void jump_entry_set_module_init(struct jump_entry *entry)
+{
+ entry->code = 0;
+}
+
+#define jump_label_swap NULL
+
#else /* __ASSEMBLY__ */
.macro STATIC_JUMP_IF_TRUE target, key, def
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 8594d24e4adc..4f44db58d981 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -37,10 +37,12 @@ static int jump_label_cmp(const void *a, const void *b)
const struct jump_entry *jea = a;
const struct jump_entry *jeb = b;
- if (jea->key < jeb->key)
+ if ((unsigned long)jump_entry_key(jea) <
+ (unsigned long)jump_entry_key(jeb))
return -1;
- if (jea->key > jeb->key)
+ if ((unsigned long)jump_entry_key(jea) >
+ (unsigned long)jump_entry_key(jeb))
return 1;
return 0;
@@ -53,7 +55,8 @@ jump_label_sort_entries(struct jump_entry *start, struct jump_entry *stop)
size = (((unsigned long)stop - (unsigned long)start)
/ sizeof(struct jump_entry));
- sort(start, size, sizeof(struct jump_entry), jump_label_cmp, NULL);
+ sort(start, size, sizeof(struct jump_entry), jump_label_cmp,
+ jump_label_swap);
}
static void jump_label_update(struct static_key *key);
@@ -254,8 +257,8 @@ EXPORT_SYMBOL_GPL(jump_label_rate_limit);
static int addr_conflict(struct jump_entry *entry, void *start, void *end)
{
- if (entry->code <= (unsigned long)end &&
- entry->code + JUMP_LABEL_NOP_SIZE > (unsigned long)start)
+ if (jump_entry_code(entry) <= (unsigned long)end &&
+ jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE > (unsigned long)start)
return 1;
return 0;
@@ -314,16 +317,6 @@ static inline void static_key_set_linked(struct static_key *key)
key->type |= JUMP_TYPE_LINKED;
}
-static inline struct static_key *jump_entry_key(struct jump_entry *entry)
-{
- return (struct static_key *)((unsigned long)entry->key & ~1UL);
-}
-
-static bool jump_entry_branch(struct jump_entry *entry)
-{
- return (unsigned long)entry->key & 1UL;
-}
-
/***
* A 'struct static_key' uses a union such that it either points directly
* to a table of 'struct jump_entry' or to a linked list of modules which in
@@ -348,7 +341,7 @@ static enum jump_label_type jump_label_type(struct jump_entry *entry)
{
struct static_key *key = jump_entry_key(entry);
bool enabled = static_key_enabled(key);
- bool branch = jump_entry_branch(entry);
+ bool branch = jump_entry_is_branch(entry);
/* See the comment in linux/jump_label.h */
return enabled ^ branch;
@@ -364,7 +357,8 @@ static void __jump_label_update(struct static_key *key,
* kernel_text_address() verifies we are not in core kernel
* init code, see jump_label_invalidate_module_init().
*/
- if (entry->code && kernel_text_address(entry->code))
+ if (!jump_entry_is_module_init(entry) &&
+ kernel_text_address(jump_entry_code(entry)))
arch_jump_label_transform(entry, jump_label_type(entry));
}
}
@@ -417,7 +411,7 @@ static enum jump_label_type jump_label_init_type(struct jump_entry *entry)
{
struct static_key *key = jump_entry_key(entry);
bool type = static_key_type(key);
- bool branch = jump_entry_branch(entry);
+ bool branch = jump_entry_is_branch(entry);
/* See the comment in linux/jump_label.h */
return type ^ branch;
@@ -541,7 +535,7 @@ static int jump_label_add_module(struct module *mod)
continue;
key = iterk;
- if (within_module(iter->key, mod)) {
+ if (within_module((unsigned long)key, mod)) {
static_key_set_entries(key, iter);
continue;
}
@@ -591,7 +585,7 @@ static void jump_label_del_module(struct module *mod)
key = jump_entry_key(iter);
- if (within_module(iter->key, mod))
+ if (within_module((unsigned long)key, mod))
continue;
/* No memory during module load */
@@ -634,8 +628,8 @@ static void jump_label_invalidate_module_init(struct module *mod)
struct jump_entry *iter;
for (iter = iter_start; iter < iter_stop; iter++) {
- if (within_module_init(iter->code, mod))
- iter->code = 0;
+ if (within_module_init(jump_entry_code(iter), mod))
+ jump_entry_set_module_init(iter);
}
}
--
2.11.0
^ permalink raw reply related
* [PATCH v7 08/10] arm64/kernel: jump_label: use relative references
From: Ard Biesheuvel @ 2018-01-02 20:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102200549.22984-1-ard.biesheuvel@linaro.org>
On a randomly chosen distro kernel build for arm64, vmlinux.o shows the
following sections, containing jump label entries, and the associated
RELA relocation records, respectively:
...
[38088] __jump_table PROGBITS 0000000000000000 00e19f30
000000000002ea10 0000000000000000 WA 0 0 8
[38089] .rela__jump_table RELA 0000000000000000 01fd8bb0
000000000008be30 0000000000000018 I 38178 38088 8
...
In other words, we have 190 KB worth of 'struct jump_entry' instances,
and 573 KB worth of RELA entries to relocate each entry's code, target
and key members. This means the RELA section occupies 10% of the .init
segment, and the two sections combined represent 5% of vmlinux's entire
memory footprint.
So let's switch from 64-bit absolute references to 32-bit relative
references: this reduces the size of the __jump_table by 50%, and gets
rid of the RELA section entirely.
Note that this requires some extra care in the sorting routine, given
that the offsets change when entries are moved around in the jump_entry
table.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm64/include/asm/jump_label.h | 27 ++++++++++++--------
arch/arm64/kernel/jump_label.c | 22 +++++++++++++---
2 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h
index 9d6e46355c89..8f82adeb7b0b 100644
--- a/arch/arm64/include/asm/jump_label.h
+++ b/arch/arm64/include/asm/jump_label.h
@@ -30,8 +30,8 @@ static __always_inline bool arch_static_branch(struct static_key *key, bool bran
{
asm goto("1: nop\n\t"
".pushsection __jump_table, \"aw\"\n\t"
- ".align 3\n\t"
- ".quad 1b, %l[l_yes], %c0\n\t"
+ ".align 2\n\t"
+ ".long 1b - ., %l[l_yes] - ., %c0 - .\n\t"
".popsection\n\t"
: : "i"(&((char *)key)[branch]) : : l_yes);
@@ -44,8 +44,8 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
{
asm goto("1: b %l[l_yes]\n\t"
".pushsection __jump_table, \"aw\"\n\t"
- ".align 3\n\t"
- ".quad 1b, %l[l_yes], %c0\n\t"
+ ".align 2\n\t"
+ ".long 1b - ., %l[l_yes] - ., %c0 - .\n\t"
".popsection\n\t"
: : "i"(&((char *)key)[branch]) : : l_yes);
@@ -57,19 +57,26 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
typedef u64 jump_label_t;
struct jump_entry {
- jump_label_t code;
- jump_label_t target;
- jump_label_t key;
+ s32 code;
+ s32 target;
+ s32 key;
};
static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
{
- return entry->code;
+ return (unsigned long)&entry->code + entry->code;
+}
+
+static inline jump_label_t jump_entry_target(const struct jump_entry *entry)
+{
+ return (unsigned long)&entry->target + entry->target;
}
static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
{
- return (struct static_key *)((unsigned long)entry->key & ~1UL);
+ unsigned long key = (unsigned long)&entry->key + entry->key;
+
+ return (struct static_key *)(key & ~1UL);
}
static inline bool jump_entry_is_branch(const struct jump_entry *entry)
@@ -87,7 +94,7 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
entry->code = 0;
}
-#define jump_label_swap NULL
+void jump_label_swap(void *a, void *b, int size);
#endif /* __ASSEMBLY__ */
#endif /* __ASM_JUMP_LABEL_H */
diff --git a/arch/arm64/kernel/jump_label.c b/arch/arm64/kernel/jump_label.c
index c2dd1ad3e648..2b8e459e91f7 100644
--- a/arch/arm64/kernel/jump_label.c
+++ b/arch/arm64/kernel/jump_label.c
@@ -25,12 +25,12 @@
void arch_jump_label_transform(struct jump_entry *entry,
enum jump_label_type type)
{
- void *addr = (void *)entry->code;
+ void *addr = (void *)jump_entry_code(entry);
u32 insn;
if (type == JUMP_LABEL_JMP) {
- insn = aarch64_insn_gen_branch_imm(entry->code,
- entry->target,
+ insn = aarch64_insn_gen_branch_imm(jump_entry_code(entry),
+ jump_entry_target(entry),
AARCH64_INSN_BRANCH_NOLINK);
} else {
insn = aarch64_insn_gen_nop();
@@ -50,4 +50,20 @@ void arch_jump_label_transform_static(struct jump_entry *entry,
*/
}
+void jump_label_swap(void *a, void *b, int size)
+{
+ long delta = (unsigned long)a - (unsigned long)b;
+ struct jump_entry *jea = a;
+ struct jump_entry *jeb = b;
+ struct jump_entry tmp = *jea;
+
+ jea->code = jeb->code - delta;
+ jea->target = jeb->target - delta;
+ jea->key = jeb->key - delta;
+
+ jeb->code = tmp.code + delta;
+ jeb->target = tmp.target + delta;
+ jeb->key = tmp.key + delta;
+}
+
#endif /* HAVE_JUMP_LABEL */
--
2.11.0
^ permalink raw reply related
* [PATCH v7 09/10] x86: jump_label: switch to jump_entry accessors
From: Ard Biesheuvel @ 2018-01-02 20:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102200549.22984-1-ard.biesheuvel@linaro.org>
In preparation of switching x86 to use place-relative references for
the code, target and key members of struct jump_entry, replace direct
references to the struct member with invocations of the new accessors.
This will allow us to make the switch by modifying the accessors only.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/x86/kernel/jump_label.c | 43 ++++++++++++--------
1 file changed, 26 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index e56c95be2808..d64296092ef5 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -52,22 +52,24 @@ static void __jump_label_transform(struct jump_entry *entry,
* Jump label is enabled for the first time.
* So we expect a default_nop...
*/
- if (unlikely(memcmp((void *)entry->code, default_nop, 5)
- != 0))
- bug_at((void *)entry->code, __LINE__);
+ if (unlikely(memcmp((void *)jump_entry_code(entry),
+ default_nop, 5) != 0))
+ bug_at((void *)jump_entry_code(entry),
+ __LINE__);
} else {
/*
* ...otherwise expect an ideal_nop. Otherwise
* something went horribly wrong.
*/
- if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
- != 0))
- bug_at((void *)entry->code, __LINE__);
+ if (unlikely(memcmp((void *)jump_entry_code(entry),
+ ideal_nop, 5) != 0))
+ bug_at((void *)jump_entry_code(entry),
+ __LINE__);
}
code.jump = 0xe9;
- code.offset = entry->target -
- (entry->code + JUMP_LABEL_NOP_SIZE);
+ code.offset = jump_entry_target(entry) -
+ (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
} else {
/*
* We are disabling this jump label. If it is not what
@@ -76,14 +78,18 @@ static void __jump_label_transform(struct jump_entry *entry,
* are converting the default nop to the ideal nop.
*/
if (init) {
- if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
- bug_at((void *)entry->code, __LINE__);
+ if (unlikely(memcmp((void *)jump_entry_code(entry),
+ default_nop, 5) != 0))
+ bug_at((void *)jump_entry_code(entry),
+ __LINE__);
} else {
code.jump = 0xe9;
- code.offset = entry->target -
- (entry->code + JUMP_LABEL_NOP_SIZE);
- if (unlikely(memcmp((void *)entry->code, &code, 5) != 0))
- bug_at((void *)entry->code, __LINE__);
+ code.offset = jump_entry_target(entry) -
+ (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
+ if (unlikely(memcmp((void *)jump_entry_code(entry),
+ &code, 5) != 0))
+ bug_at((void *)jump_entry_code(entry),
+ __LINE__);
}
memcpy(&code, ideal_nops[NOP_ATOMIC5], JUMP_LABEL_NOP_SIZE);
}
@@ -97,10 +103,13 @@ static void __jump_label_transform(struct jump_entry *entry,
*
*/
if (poker)
- (*poker)((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE);
+ (*poker)((void *)jump_entry_code(entry), &code,
+ JUMP_LABEL_NOP_SIZE);
else
- text_poke_bp((void *)entry->code, &code, JUMP_LABEL_NOP_SIZE,
- (void *)entry->code + JUMP_LABEL_NOP_SIZE);
+ text_poke_bp((void *)jump_entry_code(entry), &code,
+ JUMP_LABEL_NOP_SIZE,
+ (void *)jump_entry_code(entry) +
+ JUMP_LABEL_NOP_SIZE);
}
void arch_jump_label_transform(struct jump_entry *entry,
--
2.11.0
^ permalink raw reply related
* [PATCH v7 10/10] x86/kernel: jump_table: use relative references
From: Ard Biesheuvel @ 2018-01-02 20:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102200549.22984-1-ard.biesheuvel@linaro.org>
Similar to the arm64 case, 64-bit x86 can benefit from using 32-bit
relative references rather than 64-bit absolute ones when emitting
struct jump_entry instances. Not only does this reduce the memory
footprint of the entries themselves by 50%, it also removes the need
for carrying relocation metadata on relocatable builds (i.e., for KASLR)
which saves a fair chunk of .init space as well (although the savings
are not as dramatic as on arm64)
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/x86/include/asm/jump_label.h | 35 ++++++++++++--------
arch/x86/kernel/jump_label.c | 16 +++++++++
tools/objtool/special.c | 4 +--
3 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/arch/x86/include/asm/jump_label.h b/arch/x86/include/asm/jump_label.h
index 009ff2699d07..35fc2c5ec846 100644
--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -36,8 +36,8 @@ static __always_inline bool arch_static_branch(struct static_key *key, bool bran
asm_volatile_goto("1:"
".byte " __stringify(STATIC_KEY_INIT_NOP) "\n\t"
".pushsection __jump_table, \"aw\" \n\t"
- _ASM_ALIGN "\n\t"
- _ASM_PTR "1b, %l[l_yes], %c0 + %c1 \n\t"
+ ".balign 4\n\t"
+ ".long 1b - ., %l[l_yes] - ., %c0 + %c1 - .\n\t"
".popsection \n\t"
: : "i" (key), "i" (branch) : : l_yes);
@@ -52,8 +52,8 @@ static __always_inline bool arch_static_branch_jump(struct static_key *key, bool
".byte 0xe9\n\t .long %l[l_yes] - 2f\n\t"
"2:\n\t"
".pushsection __jump_table, \"aw\" \n\t"
- _ASM_ALIGN "\n\t"
- _ASM_PTR "1b, %l[l_yes], %c0 + %c1 \n\t"
+ ".balign 4\n\t"
+ ".long 1b - ., %l[l_yes] - ., %c0 + %c1 - .\n\t"
".popsection \n\t"
: : "i" (key), "i" (branch) : : l_yes);
@@ -69,19 +69,26 @@ typedef u32 jump_label_t;
#endif
struct jump_entry {
- jump_label_t code;
- jump_label_t target;
- jump_label_t key;
+ s32 code;
+ s32 target;
+ s32 key;
};
static inline jump_label_t jump_entry_code(const struct jump_entry *entry)
{
- return entry->code;
+ return (unsigned long)&entry->code + entry->code;
+}
+
+static inline jump_label_t jump_entry_target(const struct jump_entry *entry)
+{
+ return (unsigned long)&entry->target + entry->target;
}
static inline struct static_key *jump_entry_key(const struct jump_entry *entry)
{
- return (struct static_key *)((unsigned long)entry->key & ~1UL);
+ unsigned long key = (unsigned long)&entry->key + entry->key;
+
+ return (struct static_key *)(key & ~1UL);
}
static inline bool jump_entry_is_branch(const struct jump_entry *entry)
@@ -99,7 +106,7 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
entry->code = 0;
}
-#define jump_label_swap NULL
+void jump_label_swap(void *a, void *b, int size);
#else /* __ASSEMBLY__ */
@@ -114,8 +121,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
.byte STATIC_KEY_INIT_NOP
.endif
.pushsection __jump_table, "aw"
- _ASM_ALIGN
- _ASM_PTR .Lstatic_jump_\@, \target, \key
+ .balign 4
+ .long .Lstatic_jump_\@ - ., \target - ., \key - .
.popsection
.endm
@@ -130,8 +137,8 @@ static inline void jump_entry_set_module_init(struct jump_entry *entry)
.Lstatic_jump_after_\@:
.endif
.pushsection __jump_table, "aw"
- _ASM_ALIGN
- _ASM_PTR .Lstatic_jump_\@, \target, \key + 1
+ .balign 4
+ .long .Lstatic_jump_\@ - ., \target - ., \key + 1 - .
.popsection
.endm
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index d64296092ef5..cc5034b42335 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -149,4 +149,20 @@ __init_or_module void arch_jump_label_transform_static(struct jump_entry *entry,
__jump_label_transform(entry, type, text_poke_early, 1);
}
+void jump_label_swap(void *a, void *b, int size)
+{
+ long delta = (unsigned long)a - (unsigned long)b;
+ struct jump_entry *jea = a;
+ struct jump_entry *jeb = b;
+ struct jump_entry tmp = *jea;
+
+ jea->code = jeb->code - delta;
+ jea->target = jeb->target - delta;
+ jea->key = jeb->key - delta;
+
+ jeb->code = tmp.code + delta;
+ jeb->target = tmp.target + delta;
+ jeb->key = tmp.key + delta;
+}
+
#endif
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index 84f001d52322..98ae55b39037 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -30,9 +30,9 @@
#define EX_ORIG_OFFSET 0
#define EX_NEW_OFFSET 4
-#define JUMP_ENTRY_SIZE 24
+#define JUMP_ENTRY_SIZE 12
#define JUMP_ORIG_OFFSET 0
-#define JUMP_NEW_OFFSET 8
+#define JUMP_NEW_OFFSET 4
#define ALT_ENTRY_SIZE 13
#define ALT_ORIG_OFFSET 0
--
2.11.0
^ permalink raw reply related
* [RESEND PATCH v2 08/15] ASoC: qcom: q6asm: add support to audio stream apis
From: Bjorn Andersson @ 2018-01-02 20:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-9-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds support to open, write and media format commands
> in the q6asm module.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> sound/soc/qcom/qdsp6/q6asm.c | 530 ++++++++++++++++++++++++++++++++++++++++++-
> sound/soc/qcom/qdsp6/q6asm.h | 42 ++++
> 2 files changed, 571 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/qcom/qdsp6/q6asm.c b/sound/soc/qcom/qdsp6/q6asm.c
> index 4be92441f524..dabd6509ef99 100644
> --- a/sound/soc/qcom/qdsp6/q6asm.c
> +++ b/sound/soc/qcom/qdsp6/q6asm.c
> @@ -8,16 +8,34 @@
> #include <linux/soc/qcom/apr.h>
> #include <linux/device.h>
> #include <linux/platform_device.h>
> +#include <uapi/sound/asound.h>
> #include <linux/delay.h>
> #include <linux/slab.h>
> #include <linux/mm.h>
> #include "q6asm.h"
> #include "common.h"
>
> +#define ASM_STREAM_CMD_CLOSE 0x00010BCD
> +#define ASM_STREAM_CMD_FLUSH 0x00010BCE
> +#define ASM_SESSION_CMD_PAUSE 0x00010BD3
> +#define ASM_DATA_CMD_EOS 0x00010BDB
> +#define DEFAULT_POPP_TOPOLOGY 0x00010BE4
> +#define ASM_STREAM_CMD_FLUSH_READBUFS 0x00010C09
> #define ASM_CMD_SHARED_MEM_MAP_REGIONS 0x00010D92
> #define ASM_CMDRSP_SHARED_MEM_MAP_REGIONS 0x00010D93
> #define ASM_CMD_SHARED_MEM_UNMAP_REGIONS 0x00010D94
> -
> +#define ASM_DATA_CMD_MEDIA_FMT_UPDATE_V2 0x00010D98
> +#define ASM_DATA_EVENT_WRITE_DONE_V2 0x00010D99
> +#define ASM_SESSION_CMD_RUN_V2 0x00010DAA
> +#define ASM_MEDIA_FMT_MULTI_CHANNEL_PCM_V2 0x00010DA5
> +#define ASM_DATA_CMD_WRITE_V2 0x00010DAB
> +#define ASM_SESSION_CMD_SUSPEND 0x00010DEC
> +#define ASM_STREAM_CMD_OPEN_WRITE_V3 0x00010DB3
> +
> +#define ASM_LEGACY_STREAM_SESSION 0
> +#define ASM_END_POINT_DEVICE_MATRIX 0
> +#define DEFAULT_APP_TYPE 0
> +#define TUN_WRITE_IO_MODE 0x0008 /* tunnel read write mode */
> #define TUN_READ_IO_MODE 0x0004 /* tunnel read write mode */
> #define SYNC_IO_MODE 0x0001
> #define ASYNC_IO_MODE 0x0002
Probably prettier to reorder these and make them Q6ASM_IO_MODE_xyz
[..]
>
> +static int32_t q6asm_callback(struct apr_device *adev,
This callback is an extracted part of q6asm_srvc_callback(), can it be
given a more descriptive name?
> + struct apr_client_data *data, int session_id)
> +{
> + struct audio_client *ac;// = (struct audio_client *)priv;
> + uint32_t token;
> + uint32_t *payload;
> + uint32_t wakeup_flag = 1;
> + uint32_t client_event = 0;
> + struct q6asm *q6asm = dev_get_drvdata(&adev->dev);
> +
> + if (data == NULL)
> + return -EINVAL;
> +
> + ac = q6asm_get_audio_client(q6asm, session_id);
> + if (!q6asm_is_valid_audio_client(ac))
> + return -EINVAL;
> +
> + payload = data->payload;
> +
> + if (data->opcode == APR_BASIC_RSP_RESULT) {
Move this into the switch.
> + token = data->token;
> + switch (payload[0]) {
This is again that common response struct.
> + case ASM_SESSION_CMD_PAUSE:
> + client_event = ASM_CLIENT_EVENT_CMD_PAUSE_DONE;
> + break;
> + case ASM_SESSION_CMD_SUSPEND:
> + client_event = ASM_CLIENT_EVENT_CMD_SUSPEND_DONE;
> + break;
> + case ASM_DATA_CMD_EOS:
> + client_event = ASM_CLIENT_EVENT_CMD_EOS_DONE;
> + break;
> + break;
> + case ASM_STREAM_CMD_FLUSH:
> + client_event = ASM_CLIENT_EVENT_CMD_FLUSH_DONE;
> + break;
> + case ASM_SESSION_CMD_RUN_V2:
> + client_event = ASM_CLIENT_EVENT_CMD_RUN_DONE;
> + break;
> +
> + case ASM_STREAM_CMD_FLUSH_READBUFS:
> + if (token != ac->session) {
> + dev_err(ac->dev, "session invalid\n");
> + return -EINVAL;
> + }
> + case ASM_STREAM_CMD_CLOSE:
> + client_event = ASM_CLIENT_EVENT_CMD_CLOSE_DONE;
> + break;
> + case ASM_STREAM_CMD_OPEN_WRITE_V3:
> + case ASM_DATA_CMD_MEDIA_FMT_UPDATE_V2:
> + if (payload[1] != 0) {
> + dev_err(ac->dev,
> + "cmd = 0x%x returned error = 0x%x\n",
> + payload[0], payload[1]);
> + if (wakeup_flag) {
> + ac->cmd_state = payload[1];
> + wake_up(&ac->cmd_wait);
> + }
> + return 0;
> + }
> + break;
> + default:
> + dev_err(ac->dev, "command[0x%x] not expecting rsp\n",
> + payload[0]);
> + break;
> + }
> +
> + if (ac->cmd_state && wakeup_flag) {
> + ac->cmd_state = 0;
> + wake_up(&ac->cmd_wait);
> + }
> + if (ac->cb)
> + ac->cb(client_event, data->token,
> + data->payload, ac->priv);
> +
> + return 0;
> + }
> +
> + switch (data->opcode) {
> + case ASM_DATA_EVENT_WRITE_DONE_V2:{
> + struct audio_port_data *port =
> + &ac->port[SNDRV_PCM_STREAM_PLAYBACK];
> +
> + client_event = ASM_CLIENT_EVENT_DATA_WRITE_DONE;
> +
> + if (ac->io_mode & SYNC_IO_MODE) {
> + dma_addr_t phys = port->buf[data->token].phys;
> +
> + if (lower_32_bits(phys) != payload[0] ||
> + upper_32_bits(phys) != payload[1]) {
> + dev_err(ac->dev, "Expected addr %pa\n",
> + &port->buf[data->token].phys);
> + return -EINVAL;
> + }
> + token = data->token;
> + port->buf[token].used = 1;
> + }
> + break;
> + }
> + }
> + if (ac->cb)
> + ac->cb(client_event, data->token, data->payload, ac->priv);
> +
> + return 0;
> +}
> +
> static int q6asm_srvc_callback(struct apr_device *adev, struct apr_client_data *data)
> {
> struct q6asm *a, *q6asm = dev_get_drvdata(&adev->dev);
> @@ -415,12 +581,16 @@ static int q6asm_srvc_callback(struct apr_device *adev, struct apr_client_data *
> struct audio_port_data *port;
> uint32_t dir = 0;
> uint32_t sid = 0;
> + int dest_port;
> uint32_t *payload;
>
> if (!data) {
> dev_err(&adev->dev, "%s: Invalid CB\n", __func__);
> return 0;
> }
> + dest_port = (data->dest_port >> 8) & 0xFF;
> + if (dest_port)
> + return q6asm_callback(adev, data, dest_port);
You call dest_port "session_id" above, this seems to be a better name
for this variable.
>
> payload = data->payload;
> sid = (data->token >> 8) & 0x0F;
> @@ -540,6 +710,364 @@ struct audio_client *q6asm_audio_client_alloc(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(q6asm_audio_client_alloc);
>
> +static int __q6asm_open_write(struct audio_client *ac, uint32_t format,
> + uint16_t bits_per_sample, uint32_t stream_id,
> + bool is_gapless_mode)
> +{
> + struct asm_stream_cmd_open_write_v3 open;
> + int rc;
> +
> + q6asm_add_hdr(ac, &open.hdr, sizeof(open), true, stream_id);
> + ac->cmd_state = -1;
> +
> + open.hdr.opcode = ASM_STREAM_CMD_OPEN_WRITE_V3;
> + open.mode_flags = 0x00;
> + open.mode_flags |= ASM_LEGACY_STREAM_SESSION;
> + if (is_gapless_mode)
This is hard coded as false.
> + open.mode_flags |= 1 << ASM_SHIFT_GAPLESS_MODE_FLAG;
> +
> + /* source endpoint : matrix */
> + open.sink_endpointype = ASM_END_POINT_DEVICE_MATRIX;
> + open.bits_per_sample = bits_per_sample;
> + open.postprocopo_id = DEFAULT_POPP_TOPOLOGY;
> +
> + switch (format) {
> + case FORMAT_LINEAR_PCM:
> + open.dec_fmt_id = ASM_MEDIA_FMT_MULTI_CHANNEL_PCM_V2;
> + break;
> + default:
> + dev_err(ac->dev, "Invalid format 0x%x\n", format);
> + return -EINVAL;
> + }
> + rc = apr_send_pkt(ac->adev, (uint32_t *) &open);
> + if (rc < 0)
> + return rc;
> +
> + rc = wait_event_timeout(ac->cmd_wait, (ac->cmd_state >= 0), 5 * HZ);
> + if (!rc) {
> + dev_err(ac->dev, "timeout on open write\n");
> + return -ETIMEDOUT;
> + }
Almost every time you apr_send_pkt() you have this wait with timeout,
can this send/wait/return be wrapped in a helper function to reduce the
duplication?
Creating a q6asm_send_sync() and q6asm_send_async() pair with this logic
should help quite a bit.
> +
> + if (ac->cmd_state > 0)
> + return adsp_err_get_lnx_err_code(ac->cmd_state);
> +
> + ac->io_mode |= TUN_WRITE_IO_MODE;
> +
> + return 0;
> +}
> +
> +/**
> + * q6asm_open_write() - Open audio client for writing
> + *
> + * @ac: audio client pointer
> + * @format: audio sample format
> + * @bits_per_sample: bits per sample
> + *
> + * Return: Will be an negative value on error or zero on success
> + */
> +int q6asm_open_write(struct audio_client *ac, uint32_t format,
> + uint16_t bits_per_sample)
> +{
> + return __q6asm_open_write(ac, format, bits_per_sample,
I don't see a particular reason for not inlining this, is there one
coming later in the series?
> + ac->stream_id, false);
> +}
> +EXPORT_SYMBOL_GPL(q6asm_open_write);
> +
> +static int __q6asm_run(struct audio_client *ac, uint32_t flags,
> + uint32_t msw_ts, uint32_t lsw_ts, bool wait)
> +{
> + struct asm_session_cmd_run_v2 run;
> + int rc;
> +
> + q6asm_add_hdr(ac, &run.hdr, sizeof(run), true, ac->stream_id);
> + ac->cmd_state = -1;
> +
> + run.hdr.opcode = ASM_SESSION_CMD_RUN_V2;
> + run.flags = flags;
> + run.time_lsw = lsw_ts;
> + run.time_msw = msw_ts;
> +
> + rc = apr_send_pkt(ac->adev, (uint32_t *) &run);
> + if (rc < 0)
> + return rc;
> +
> + if (wait) {
Rather than having half of the function conditional I would recommend
inlining this function in the two callers.
In particular if you can come up with a helper function for the
send/wait/handle-error case.
> + rc = wait_event_timeout(ac->cmd_wait, (ac->cmd_state >= 0),
> + 5 * HZ);
> + if (!rc) {
> + dev_err(ac->dev, "timeout on run cmd\n");
> + return -ETIMEDOUT;
> + }
> + if (ac->cmd_state > 0)
> + return adsp_err_get_lnx_err_code(ac->cmd_state);
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * q6asm_run() - start the audio client
> + *
> + * @ac: audio client pointer
> + * @flags: flags associated with write
> + * @msw_ts: timestamp msw
> + * @lsw_ts: timestamp lsw
> + *
> + * Return: Will be an negative value on error or zero on success
> + */
> +int q6asm_run(struct audio_client *ac, uint32_t flags,
> + uint32_t msw_ts, uint32_t lsw_ts)
> +{
> + return __q6asm_run(ac, flags, msw_ts, lsw_ts, true);
> +}
> +EXPORT_SYMBOL_GPL(q6asm_run);
> +
> +/**
> + * q6asm_run_nowait() - start the audio client withou blocking
> + *
> + * @ac: audio client pointer
> + * @flags: flags associated with write
> + * @msw_ts: timestamp msw
> + * @lsw_ts: timestamp lsw
> + *
> + * Return: Will be an negative value on error or zero on success
> + */
> +int q6asm_run_nowait(struct audio_client *ac, uint32_t flags,
> + uint32_t msw_ts, uint32_t lsw_ts)
> +{
> + return __q6asm_run(ac, flags, msw_ts, lsw_ts, false);
> +}
> +EXPORT_SYMBOL_GPL(q6asm_run_nowait);
> +
> +/**
> + * q6asm_media_format_block_multi_ch_pcm() - setup pcm configuration
> + *
> + * @ac: audio client pointer
> + * @rate: audio sample rate
> + * @channels: number of audio channels.
> + * @use_default_chmap: flag to use default ch map.
> + * @channel_map: channel map pointer
> + * @bits_per_sample: bits per sample
> + *
> + * Return: Will be an negative value on error or zero on success
> + */
> +int q6asm_media_format_block_multi_ch_pcm(struct audio_client *ac,
> + uint32_t rate, uint32_t channels,
> + bool use_default_chmap,
> + char *channel_map,
This should be u8 channel_map[PCM_FORMAT_MAX_NUM_CHANNEL], possibly
char. Unless you, as I suggest below, want to be able to represent
use_default_chmap = false, by setting this to NULL.
> + uint16_t bits_per_sample)
> +{
> + struct asm_multi_channel_pcm_fmt_blk_v2 fmt;
> + u8 *channel_mapping;
> + int rc = 0;
Unnecessary initialization.
> +
> + q6asm_add_hdr(ac, &fmt.hdr, sizeof(fmt), true, ac->stream_id);
> + ac->cmd_state = -1;
> +
> + fmt.hdr.opcode = ASM_DATA_CMD_MEDIA_FMT_UPDATE_V2;
> + fmt.fmt_blk.fmt_blk_size = sizeof(fmt) - sizeof(fmt.hdr) -
> + sizeof(fmt.fmt_blk);
> + fmt.num_channels = channels;
> + fmt.bits_per_sample = bits_per_sample;
> + fmt.sample_rate = rate;
> + fmt.is_signed = 1;
> +
> + channel_mapping = fmt.channel_mapping;
> +
> + if (use_default_chmap) {
Passing NULL as channel_map would probably be a nicer way to say this,
instead of having a separate bool.
> + if (q6dsp_map_channels(channel_mapping, channels)) {
> + dev_err(ac->dev, " map channels failed %d\n", channels);
> + return -EINVAL;
> + }
> + } else {
> + memcpy(channel_mapping, channel_map,
> + PCM_FORMAT_MAX_NUM_CHANNEL);
> + }
> +
> + rc = apr_send_pkt(ac->adev, (uint32_t *) &fmt);
> + if (rc < 0)
> + goto fail_cmd;
> +
> + rc = wait_event_timeout(ac->cmd_wait, (ac->cmd_state >= 0), 5 * HZ);
> + if (!rc) {
> + dev_err(ac->dev, "timeout on format update\n");
> + return -ETIMEDOUT;
> + }
> + if (ac->cmd_state > 0)
> + return adsp_err_get_lnx_err_code(ac->cmd_state);
> +
> + return 0;
> +fail_cmd:
> + return rc;
> +}
> +EXPORT_SYMBOL_GPL(q6asm_media_format_block_multi_ch_pcm);
> +
> +/**
> + * q6asm_write_nolock() - non blocking write
> + *
> + * @ac: audio client pointer
> + * @len: lenght in bytes
> + * @msw_ts: timestamp msw
> + * @lsw_ts: timestamp lsw
> + * @flags: flags associated with write
> + *
> + * Return: Will be an negative value on error or zero on success
> + */
> +int q6asm_write_nolock(struct audio_client *ac, uint32_t len, uint32_t msw_ts,
> + uint32_t lsw_ts, uint32_t flags)
q6asm_write_async() is probably a better name, nolock indicates some
relationship to mutual exclusions...
> +{
> + struct asm_data_cmd_write_v2 write;
> + struct audio_port_data *port;
> + struct audio_buffer *ab;
> + int dsp_buf = 0;
> + int rc = 0;
> +
> + if (ac->io_mode & SYNC_IO_MODE) {
Bail early if this isn't true, to save you the indentation level.
> + port = &ac->port[SNDRV_PCM_STREAM_PLAYBACK];
> + q6asm_add_hdr(ac, &write.hdr, sizeof(write), false,
> + ac->stream_id);
> +
> + dsp_buf = port->dsp_buf;
> + ab = &port->buf[dsp_buf];
So we're just unconditionally telling the remote side about the next buf
in our ring buffer. Do we need to ensure that this is available/ready?
> +
> + write.hdr.token = port->dsp_buf;
> + write.hdr.opcode = ASM_DATA_CMD_WRITE_V2;
> + write.buf_addr_lsw = lower_32_bits(ab->phys);
> + write.buf_addr_msw = upper_32_bits(ab->phys);
> + write.buf_size = len;
> + write.seq_id = port->dsp_buf;
> + write.timestamp_lsw = lsw_ts;
> + write.timestamp_msw = msw_ts;
> + write.mem_map_handle =
> + ac->port[SNDRV_PCM_STREAM_PLAYBACK].mem_map_handle;
> +
> + if (flags == NO_TIMESTAMP)
> + write.flags = (flags & 0x800000FF);
Fill in the constant and this becomes
if flags == 0xff00:
write.flags = 0xff00 & 0x800000ff;
Or in other words:
if flags == 0xff00:
write.flags = 0;
> + else
> + write.flags = (0x80000000 | flags);
Drop the parenthesis and flip the |. It would be nice to have a define
or a comment indicating what BIT(31) is...
> +
> + port->dsp_buf++;
> +
> + if (port->dsp_buf >= port->max_buf_cnt)
> + port->dsp_buf = 0;
> +
> + rc = apr_send_pkt(ac->adev, (uint32_t *) &write);
> + if (rc < 0)
> + return rc;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(q6asm_write_nolock);
> +
> +static void q6asm_reset_buf_state(struct audio_client *ac)
> +{
> + int cnt = 0;
> + int loopcnt = 0;
> + int used;
> + struct audio_port_data *port = NULL;
> +
> + if (ac->io_mode & SYNC_IO_MODE) {
> + used = (ac->io_mode & TUN_WRITE_IO_MODE ? 1 : 0);
> + mutex_lock(&ac->cmd_lock);
> + for (loopcnt = 0; loopcnt <= SNDRV_PCM_STREAM_CAPTURE;
> + loopcnt++) {
> + port = &ac->port[loopcnt];
> + cnt = port->max_buf_cnt - 1;
> + port->dsp_buf = 0;
> + while (cnt >= 0) {
> + if (!port->buf)
> + continue;
> + port->buf[cnt].used = used;
> + cnt--;
> + }
> + }
> + mutex_unlock(&ac->cmd_lock);
> + }
> +}
> +
> +static int __q6asm_cmd(struct audio_client *ac, int cmd, bool wait)
> +{
> + int stream_id = ac->stream_id;
> + struct apr_hdr hdr;
> + int rc;
> +
> + q6asm_add_hdr(ac, &hdr, sizeof(hdr), true, stream_id);
> + ac->cmd_state = -1;
Resetting cmd_state relates to the send, don't mix it with building the
packet.
> + switch (cmd) {
> + case CMD_PAUSE:
> + hdr.opcode = ASM_SESSION_CMD_PAUSE;
> + break;
> + case CMD_SUSPEND:
> + hdr.opcode = ASM_SESSION_CMD_SUSPEND;
> + break;
> + case CMD_FLUSH:
> + hdr.opcode = ASM_STREAM_CMD_FLUSH;
> + break;
> + case CMD_OUT_FLUSH:
> + hdr.opcode = ASM_STREAM_CMD_FLUSH_READBUFS;
> + break;
> + case CMD_EOS:
> + hdr.opcode = ASM_DATA_CMD_EOS;
> + ac->cmd_state = 0;
> + break;
> + case CMD_CLOSE:
> + hdr.opcode = ASM_STREAM_CMD_CLOSE;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + rc = apr_send_pkt(ac->adev, (uint32_t *) &hdr);
> + if (rc < 0)
> + return rc;
> +
> + if (!wait)
> + return 0;
I've asked you to split the others into _sync() vs _async() operations.
One particular concern I have is that I don't see any mutual exclusion
protecting the cmd_state and a call with !wait will overwrite the
existing value, which might be unexpected.
> +
> + rc = wait_event_timeout(ac->cmd_wait, (ac->cmd_state >= 0), 5 * HZ);
> + if (!rc) {
> + dev_err(ac->dev, "timeout response for opcode[0x%x]\n",
> + hdr.opcode);
> + return -ETIMEDOUT;
> + }
> + if (ac->cmd_state > 0)
> + return adsp_err_get_lnx_err_code(ac->cmd_state);
> +
> + if (cmd == CMD_FLUSH)
> + q6asm_reset_buf_state(ac);
> +
> + return 0;
> +}
[..]
> diff --git a/sound/soc/qcom/qdsp6/q6asm.h b/sound/soc/qcom/qdsp6/q6asm.h
> index e1409c368600..b4896059da79 100644
> --- a/sound/soc/qcom/qdsp6/q6asm.h
> +++ b/sound/soc/qcom/qdsp6/q6asm.h
> @@ -2,7 +2,34 @@
> #ifndef __Q6_ASM_H__
> #define __Q6_ASM_H__
>
> +/* ASM client callback events */
> +#define CMD_PAUSE 0x0001
These defines has rather generic names...
[..]
> +
> +#define MSM_FRONTEND_DAI_MULTIMEDIA1 0
> +#define MSM_FRONTEND_DAI_MULTIMEDIA2 1
> +#define MSM_FRONTEND_DAI_MULTIMEDIA3 2
> +#define MSM_FRONTEND_DAI_MULTIMEDIA4 3
> +#define MSM_FRONTEND_DAI_MULTIMEDIA5 4
> +#define MSM_FRONTEND_DAI_MULTIMEDIA6 5
> +#define MSM_FRONTEND_DAI_MULTIMEDIA7 6
> +#define MSM_FRONTEND_DAI_MULTIMEDIA8 7
> +
> #define MAX_SESSIONS 16
> +#define NO_TIMESTAMP 0xFF00
> +#define FORMAT_LINEAR_PCM 0x0000
Ditto.
Regards,
Bjorn
^ permalink raw reply
* [GIT PULL] Amlogic 64-bit DT changes for v4.16, round 2
From: Kevin Hilman @ 2018-01-02 20:16 UTC (permalink / raw)
To: linux-arm-kernel
Arnd, Olof,
Here's a second round of 64-bit DT changes for v4.16. This branch is on
top of the previous round (already merged.)
This adds a few more basics (clock, pinctrl, PWM, reset) for the new AXG
family of Amlogic SoCs.
Kevin
The following changes since commit 3106507e1004dd398ef75d0caf048f97ba2dfd0b:
ARM64: dts: meson-gxm: fix q200 interrupt number (2017-12-08 10:47:28 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git tags/amlogic-dt64-2
for you to fetch changes up to 43b9f617b5f98f2f7abb508fef5e535e5fb66a41:
arm64: dts: meson-axg: add new reset DT node (2017-12-15 16:20:29 -0800)
----------------------------------------------------------------
Amlogic 64-bit DT updates for v4.16, round 2
- clock, pinctrl, PWM and reset nodes for new AXG SoC family
----------------------------------------------------------------
Jian Hu (1):
ARM64: dts: meson-axg: add PWM DT info for Meson-Axg SoC
Qiufang Dai (1):
arm64: dts: meson-axg: add clock DT info for Meson AXG SoC
Xingyu Chen (2):
documentation: Add compatibles for Amlogic Meson AXG pin controllers
ARM64: dts: meson-axg: add pinctrl DT info for Meson-AXG SoC
Yixun Lan (1):
arm64: dts: meson-axg: add new reset DT node
Documentation/devicetree/bindings/pinctrl/meson,pinctrl.txt | 2 +
arch/arm64/boot/dts/amlogic/meson-axg.dtsi | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 177 insertions(+)
^ permalink raw reply
* [PATCH 01/33] clk_ops: change round_rate() to return unsigned long
From: Bryan O'Donoghue @ 2018-01-02 20:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102190159.GH7997@codeaurora.org>
On 02/01/18 19:01, Stephen Boyd wrote:
> On 12/31, Bryan O'Donoghue wrote:
>> On 30/12/17 16:36, Mikko Perttunen wrote:
>>> FWIW, we had this problem some years ago with the Tegra CPU clock
>>> - then it was determined that a simpler solution was to have the
>>> determine_rate callback support unsigned long rates - so clock
>>> drivers that need to return rates higher than 2^31 can instead
>>> implement the determine_rate callback. That is what's currently
>>> implemented.
>>>
>>> Mikko
>>
>> Granted we could work around it but, having both zero and less than
>> zero indicate error means you can't support larger than LONG_MAX
>> which is I think worth fixing.
>>
>
> Ok. But can you implement the determine_rate op instead of the
> round_rate op for your clk?
Don't know .
> It's not a work-around, it's the
> preferred solution. That would allow rates larger than 2^31 for
> the clk without pushing through a change to all the drivers to
> express zero as "error" and non-zero as the rounded rate.
>
> I'm not entirely opposed to this approach, because we probably
> don't care to pass the particular error value from a clk provider
> to a clk consumer about what the error is.
Which was my thought. The return value of clk_ops->round_rate() appears
not to get pushed up the stack, which is what the last patch in this
series deals with.
[PATCH 33/33] clk: change handling of round_rate() such that only zero
is an error
> It's actually what we
> proposed as the solution for clk_round_rate() to return values
> larger than LONG_MAX to consumers. But doing that consumer API
> change or this provider side change is going to require us to
> evaluate all the consumers of these clks to make sure they don't
> check for some error value that's less than zero. This series
> does half the work,
Do you mean users of clk_rounda_rate() ? I have a set of patches for
that but wanted to separate that from clk_ops->round_rate() so as not to
send ~70 patches out to LKML at once - even if they are in two blocks.
If so, I can publish that set too for reference.
AFAICT on clk_ops->round_rate the last patch #33 ought to cover the
usage of the return value of clk_ops->round_rate().
Have I missed something ?
> by changing the provider side, while ignoring
> the consumer side and any potential fallout of the less than zero
> to zero return value change.
>
Can you look at #33 ? I'm not sure if you saw that one.
---
bod
^ permalink raw reply
* [PATCH v4 10/21] arm64: entry.S: move SError handling into a C function for future expansion
From: Adam Wallis @ 2018-01-02 21:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171019145807.23251-11-james.morse@arm.com>
James
On 10/19/2017 10:57 AM, James Morse wrote:
[..]
> kernel_ventry el1_fiq_invalid // FIQ EL1h
> - kernel_ventry el1_error_invalid // Error EL1h
> + kernel_ventry el1_error // Error EL1h
>
> kernel_ventry el0_sync // Synchronous 64-bit EL0
> kernel_ventry el0_irq // IRQ 64-bit EL0
> kernel_ventry el0_fiq_invalid // FIQ 64-bit EL0
> - kernel_ventry el0_error_invalid // Error 64-bit EL0
> + kernel_ventry el0_error // Error 64-bit EL0
>
> #ifdef CONFIG_COMPAT
> kernel_ventry el0_sync_compat // Synchronous 32-bit EL0
> kernel_ventry el0_irq_compat // IRQ 32-bit EL0
> kernel_ventry el0_fiq_invalid_compat // FIQ 32-bit EL0
> - kernel_ventry el0_error_invalid_compat // Error 32-bit EL0
> + kernel_ventry el0_error_compat // Error 32-bit EL0
> #else
> kernel_ventry el0_sync_invalid // Synchronous 32-bit EL0
> kernel_ventry el0_irq_invalid // IRQ 32-bit EL0
> @@ -455,10 +455,6 @@ ENDPROC(el0_error_invalid)
> el0_fiq_invalid_compat:
> inv_entry 0, BAD_FIQ, 32
> ENDPROC(el0_fiq_invalid_compat)
> -
> -el0_error_invalid_compat:
> - inv_entry 0, BAD_ERROR, 32
> -ENDPROC(el0_error_invalid_compat)
> #endif
Perhaps I missed something quite obvious, but is there any reason to not also
remove el1_error_invalid, since SError handling now jumps to el1_error?
> el1_sync_invalid:
> @@ -663,6 +659,10 @@ el0_svc_compat:
> el0_irq_compat:
> kernel_entry 0, 32
> b el0_irq_naked
> +
> +el0_error_compat:
> + kernel_entry 0, 32
> + b el0_error_naked
> #endif
>
> el0_da:
> @@ -780,6 +780,28 @@ el0_irq_naked:
> b ret_to_user
> ENDPROC(el0_irq)
>
> +el1_error:
> + kernel_entry 1
> + mrs x1, esr_el1
> + enable_dbg
> + mov x0, sp
> + bl do_serror
> + kernel_exit 1
> +ENDPROC(el1_error)
> +
> +el0_error:
> + kernel_entry 0
> +el0_error_naked:
> + mrs x1, esr_el1
> + enable_dbg
> + mov x0, sp
> + bl do_serror
> + enable_daif
> + ct_user_exit
> + b ret_to_user
> +ENDPROC(el0_error)
[..]
Thanks
Adam
--
Adam Wallis
Qualcomm Datacenter Technologies 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 v4 5/7] clk: Introduce davinci clocks
From: David Lechner @ 2018-01-02 21:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1514763588-31560-6-git-send-email-david@lechnology.com>
Forgot to cc linux-clk, so doing that now...
On 12/31/2017 05:39 PM, David Lechner wrote:
> This introduces new drivers for arch/arm/mach-davinci. The code is based
> on the clock drivers from there and adapted to use the common clock
> framework.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
> drivers/clk/Makefile | 1 +
> drivers/clk/davinci/Makefile | 3 +
> drivers/clk/davinci/da8xx-cfgchip-clk.c | 380 ++++++++++++++++++++++++++++++
> drivers/clk/davinci/pll.c | 333 ++++++++++++++++++++++++++
> drivers/clk/davinci/psc.c | 217 +++++++++++++++++
> include/linux/clk/davinci.h | 46 ++++
> include/linux/platform_data/davinci_clk.h | 25 ++
> 7 files changed, 1005 insertions(+)
> create mode 100644 drivers/clk/davinci/Makefile
> create mode 100644 drivers/clk/davinci/da8xx-cfgchip-clk.c
> create mode 100644 drivers/clk/davinci/pll.c
> create mode 100644 drivers/clk/davinci/psc.c
> create mode 100644 include/linux/clk/davinci.h
> create mode 100644 include/linux/platform_data/davinci_clk.h
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index f7f761b..c865fd0 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -60,6 +60,7 @@ obj-$(CONFIG_ARCH_ARTPEC) += axis/
> obj-$(CONFIG_ARC_PLAT_AXS10X) += axs10x/
> obj-y += bcm/
> obj-$(CONFIG_ARCH_BERLIN) += berlin/
> +obj-$(CONFIG_ARCH_DAVINCI) += davinci/
> obj-$(CONFIG_H8300) += h8300/
> obj-$(CONFIG_ARCH_HISI) += hisilicon/
> obj-y += imgtec/
> diff --git a/drivers/clk/davinci/Makefile b/drivers/clk/davinci/Makefile
> new file mode 100644
> index 0000000..5efbdcd
> --- /dev/null
> +++ b/drivers/clk/davinci/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_PHY_DA8XX_USB) += da8xx-cfgchip-clk.o
> +obj-y += pll.o
> +obj-y += psc.o
> \ No newline at end of file
> diff --git a/drivers/clk/davinci/da8xx-cfgchip-clk.c b/drivers/clk/davinci/da8xx-cfgchip-clk.c
> new file mode 100644
> index 0000000..780bb25
> --- /dev/null
> +++ b/drivers/clk/davinci/da8xx-cfgchip-clk.c
> @@ -0,0 +1,380 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * da8xx-cfgchip-clk - TI DaVinci DA8xx CFGCHIP clocks driver
> + *
> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
> + *
> + * This driver exposes the USB PHY clocks on DA8xx/AM18xx/OMAP-L13x SoCs.
> + * The clocks consist of two muxes and a PLL. The USB 2.0 PHY mux and PLL are
> + * combined into a single clock in Linux. The USB 1.0 PHY clock just consists
> + * of a mux. These clocks are controlled through CFGCHIP2, which is accessed
> + * as a syscon regmap since it is shared with other devices.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/mfd/da8xx-cfgchip.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/davinci_clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +/**
> + * da8xx_cfgchip_clk
> + * @usb0_hw: The USB 2.0 PHY clock (mux + PLL)
> + * @usb1_hw: The USB 1.1 PHY clock (mux)
> + * @usb0_clk: The USB 2.0 subsystem PSC clock
> + * @regmap: The CFGCHIP syscon regmap
> + */
> +struct da8xx_cfgchip_clk {
> + struct clk_hw usb0_hw;
> + struct clk_hw usb1_hw;
> + struct clk *usb0_clk;
> + struct regmap *regmap;
> +};
> +
> +/* The USB 2.0 PHY can use either USB_REFCLKIN or AUXCLK */
> +enum usb0_phy_clk_parent {
> + USB20_PHY_CLK_PARENT_USB_REFCLKIN,
> + USB20_PHY_CLK_PARENT_PLL0_AUX,
> +};
> +
> +/* The USB 1.1 PHY can use either the PLL output from the USB 2.0 PHY or
> + * USB_REFCLKIN
> + */
> +enum usb1_phy_clk_parent {
> + USB1_PHY_CLK_PARENT_USB_REFCLKIN,
> + USB1_PHY_CLK_PARENT_USB0_PHY_PLL,
> +};
> +
> +/* --- USB 2.0 PHY clock --- */
> +
> +static int usb0_phy_clk_prepare(struct clk_hw *hw)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb0_hw);
> +
> + /* The USB 2.0 PSC clock is only needed temporarily during the USB 2.0
> + * PHY clock enable, but since clk_prepare() can't be called in an
> + * atomic context (i.e. in clk_enable()), we have to prepare it here.
> + */
> + return clk_prepare(clk->usb0_clk);
> +}
> +
> +static void usb0_phy_clk_unprepare(struct clk_hw *hw)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb0_hw);
> +
> + clk_unprepare(clk->usb0_clk);
> +}
> +
> +static int usb0_phy_clk_enable(struct clk_hw *hw)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb0_hw);
> + unsigned int mask, val;
> + int ret;
> +
> + /* Locking the USB 2.O PLL requires that the USB 2.O PSC is enabled
> + * temporaily. It can be turned back off once the PLL is locked.
> + */
> + clk_enable(clk->usb0_clk);
> +
> + /* Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
> + * PHY may use the USB 2.0 PLL clock without USB 2.0 OTG being used.
> + */
> + mask = CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN | CFGCHIP2_PHY_PLLON;
> + val = CFGCHIP2_PHY_PLLON;
> +
> + regmap_write_bits(clk->regmap, CFGCHIP(2), mask, val);
> + ret = regmap_read_poll_timeout(clk->regmap, CFGCHIP(2), val,
> + val & CFGCHIP2_PHYCLKGD, 0, 500000);
> +
> + clk_disable(clk->usb0_clk);
> +
> + return ret;
> +}
> +
> +static void usb0_phy_clk_disable(struct clk_hw *hw)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb0_hw);
> + unsigned int val;
> +
> + val = CFGCHIP2_PHYPWRDN;
> + regmap_write_bits(clk->regmap, CFGCHIP(2), val, val);
> +}
> +
> +static unsigned long usb0_phy_clk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb0_hw);
> + unsigned int mask, val;
> +
> + /* The parent clock rate must be one of the following */
> + mask = CFGCHIP2_REFFREQ_MASK;
> + switch (parent_rate) {
> + case 12000000:
> + val = CFGCHIP2_REFFREQ_12MHZ;
> + break;
> + case 13000000:
> + val = CFGCHIP2_REFFREQ_13MHZ;
> + break;
> + case 19200000:
> + val = CFGCHIP2_REFFREQ_19_2MHZ;
> + break;
> + case 20000000:
> + val = CFGCHIP2_REFFREQ_20MHZ;
> + break;
> + case 24000000:
> + val = CFGCHIP2_REFFREQ_24MHZ;
> + break;
> + case 26000000:
> + val = CFGCHIP2_REFFREQ_26MHZ;
> + break;
> + case 38400000:
> + val = CFGCHIP2_REFFREQ_38_4MHZ;
> + break;
> + case 40000000:
> + val = CFGCHIP2_REFFREQ_40MHZ;
> + break;
> + case 48000000:
> + val = CFGCHIP2_REFFREQ_48MHZ;
> + break;
> + default:
> + return 0;
> + }
> +
> + regmap_write_bits(clk->regmap, CFGCHIP(2), mask, val);
> +
> + /* USB 2.0 PLL always supplies 48MHz */
> + return 48000000;
> +}
> +
> +static long usb0_phy_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + return 48000000;
> +}
> +
> +static int usb0_phy_clk_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb0_hw);
> + unsigned int mask, val;
> +
> + /* Set the mux depending on the parent clock. */
> + mask = CFGCHIP2_USB2PHYCLKMUX;
> + switch (index) {
> + case USB20_PHY_CLK_PARENT_USB_REFCLKIN:
> + val = 0;
> + break;
> + case USB20_PHY_CLK_PARENT_PLL0_AUX:
> + val = CFGCHIP2_USB2PHYCLKMUX;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + regmap_write_bits(clk->regmap, CFGCHIP(2), mask, val);
> +
> + return 0;
> +}
> +
> +static u8 usb0_phy_clk_get_parent(struct clk_hw *hw)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb0_hw);
> + unsigned int val;
> +
> + regmap_read(clk->regmap, CFGCHIP(2), &val);
> +
> + if (val & CFGCHIP2_USB2PHYCLKMUX)
> + return USB20_PHY_CLK_PARENT_PLL0_AUX;
> +
> + return USB20_PHY_CLK_PARENT_USB_REFCLKIN;
> +}
> +
> +static const struct clk_ops usb0_phy_clk_ops = {
> + .prepare = usb0_phy_clk_prepare,
> + .unprepare = usb0_phy_clk_unprepare,
> + .enable = usb0_phy_clk_enable,
> + .disable = usb0_phy_clk_disable,
> + .recalc_rate = usb0_phy_clk_recalc_rate,
> + .round_rate = usb0_phy_clk_round_rate,
> + .set_parent = usb0_phy_clk_set_parent,
> + .get_parent = usb0_phy_clk_get_parent,
> +};
> +
> +static const char * const usb0_phy_clk_parent_names[] = {
> + [USB20_PHY_CLK_PARENT_USB_REFCLKIN] = "usb_refclkin",
> + [USB20_PHY_CLK_PARENT_PLL0_AUX] = "pll0_aux_clk",
> +};
> +
> +static const struct clk_init_data usb0_phy_clk_init_data = {
> + .name = "usb0_phy_clk",
> + .ops = &usb0_phy_clk_ops,
> + .parent_names = usb0_phy_clk_parent_names,
> + .num_parents = ARRAY_SIZE(usb0_phy_clk_parent_names),
> +};
> +
> +/* --- USB 1.1 PHY clock --- */
> +
> +static int usb1_phy_clk_set_parent(struct clk_hw *hw, u8 index)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb1_hw);
> + unsigned int mask, val;
> +
> + /* Set the USB 1.1 PHY clock mux based on the parent clock. */
> + mask = CFGCHIP2_USB1PHYCLKMUX;
> + switch (index) {
> + case USB1_PHY_CLK_PARENT_USB_REFCLKIN:
> + val = CFGCHIP2_USB1PHYCLKMUX;
> + break;
> + case USB1_PHY_CLK_PARENT_USB0_PHY_PLL:
> + val = 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + regmap_write_bits(clk->regmap, CFGCHIP(2), mask, val);
> +
> + return 0;
> +}
> +
> +static u8 usb1_phy_clk_get_parent(struct clk_hw *hw)
> +{
> + struct da8xx_cfgchip_clk *clk =
> + container_of(hw, struct da8xx_cfgchip_clk, usb1_hw);
> + unsigned int val;
> +
> + regmap_read(clk->regmap, CFGCHIP(2), &val);
> +
> + if (val & CFGCHIP2_USB1PHYCLKMUX)
> + return USB1_PHY_CLK_PARENT_USB_REFCLKIN;
> +
> + return USB1_PHY_CLK_PARENT_USB0_PHY_PLL;
> +}
> +
> +static const struct clk_ops usb1_phy_clk_ops = {
> + .set_parent = usb1_phy_clk_set_parent,
> + .get_parent = usb1_phy_clk_get_parent,
> +};
> +
> +static const char * const usb1_phy_clk_parent_names[] = {
> + [USB1_PHY_CLK_PARENT_USB_REFCLKIN] = "usb_refclkin",
> + [USB1_PHY_CLK_PARENT_USB0_PHY_PLL] = "usb0_phy_clk",
> +};
> +
> +static struct clk_init_data usb1_phy_clk_init_data = {
> + .name = "usb1_phy_clk",
> + .ops = &usb1_phy_clk_ops,
> + .parent_names = usb1_phy_clk_parent_names,
> + .num_parents = ARRAY_SIZE(usb1_phy_clk_parent_names),
> +};
> +
> +/* --- platform driver --- */
> +
> +static int da8xx_cfgchip_clk_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct da8xx_cfgchip_clk_data *pdata = dev->platform_data;
> + struct da8xx_cfgchip_clk *phy_clk;
> + const char *parent_name;
> + struct clk *parent;
> + int ret;
> +
> + if (!pdata)
> + return -EINVAL;
> +
> + phy_clk = devm_kzalloc(dev, sizeof(*phy_clk), GFP_KERNEL);
> + if (!phy_clk)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, phy_clk);
> +
> + phy_clk->regmap = syscon_regmap_lookup_by_pdevname("syscon");
> + if (IS_ERR(phy_clk->regmap)) {
> + dev_err(dev, "Failed to get syscon\n");
> + return PTR_ERR(phy_clk->regmap);
> + }
> +
> + /* USB 2.0 subsystem PSC clock - needed to lock PLL */
> + phy_clk->usb0_clk = clk_get(dev, "usb20");
> + if (IS_ERR(phy_clk->usb0_clk)) {
> + dev_err(dev, "Failed to get usb20 clock\n");
> + return PTR_ERR(phy_clk->usb0_clk);
> + }
> +
> + phy_clk->usb0_hw.init = &usb0_phy_clk_init_data;
> + ret = devm_clk_hw_register(dev, &phy_clk->usb0_hw);
> + if (ret) {
> + dev_err(dev, "Failed to register usb0_phy_clk\n");
> + return ret;
> + }
> +
> + phy_clk->usb1_hw.init = &usb1_phy_clk_init_data;
> + ret = devm_clk_hw_register(dev, &phy_clk->usb1_hw);
> + if (ret) {
> + dev_err(dev, "Failed to register usb1_phy_clk\n");
> + return ret;
> + }
> +
> + parent_name = pdata->usb0_use_refclkin ? "usb_refclkin" : "pll0_aux";
> + parent = devm_clk_get(dev, parent_name);
> + if (IS_ERR(parent)) {
> + dev_err(dev, "Failed to get usb0 parent clock %s\n",
> + parent_name);
> + return PTR_ERR(parent);
> + }
> +
> + ret = clk_set_parent(phy_clk->usb0_hw.clk, parent);
> + if (ret) {
> + dev_err(dev, "Failed to set usb0 parent clock to %s\n",
> + parent_name);
> + return ret;
> + }
> +
> + clk_hw_register_clkdev(&phy_clk->usb0_hw, NULL, "da8xx-cfgchip-clk");
> +
> + parent_name = pdata->usb1_use_refclkin ? "usb_refclkin" : "usb0_phy_clk";
> + parent = devm_clk_get(dev, parent_name);
> + if (IS_ERR(parent)) {
> + dev_err(dev, "Failed to get usb1 parent clock %s\n",
> + parent_name);
> + return PTR_ERR(parent);
> + }
> +
> + ret = clk_set_parent(phy_clk->usb1_hw.clk, parent);
> + if (ret) {
> + dev_err(dev, "Failed to set usb1 parent clock to %s\n",
> + parent_name);
> + return ret;
> + }
> +
> + clk_hw_register_clkdev(&phy_clk->usb0_hw, "usb20_phy", "da8xx-usb-phy");
> + clk_hw_register_clkdev(&phy_clk->usb1_hw, "usb11_phy", "da8xx-usb-phy");
> +
> + return 0;
> +}
> +
> +static struct platform_driver da8xx_cfgchip_clk_driver = {
> + .probe = da8xx_cfgchip_clk_probe,
> + .driver = {
> + .name = "da8xx-cfgchip-clk",
> + },
> +};
> +module_platform_driver(da8xx_cfgchip_clk_driver);
> +
> +MODULE_ALIAS("platform:da8xx-cfgchip-clk");
> +MODULE_AUTHOR("David Lechner <david@lechnology.com>");
> +MODULE_DESCRIPTION("TI DA8xx CFGCHIP clock driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/clk/davinci/pll.c b/drivers/clk/davinci/pll.c
> new file mode 100644
> index 0000000..035cd91
> --- /dev/null
> +++ b/drivers/clk/davinci/pll.c
> @@ -0,0 +1,333 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PLL clock driver for Davinci devices
> + *
> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
> + *
> + * Based on drivers/clk/keystone/pll.c
> + * Copyright (C) 2013 Texas Instruments Inc.
> + * Murali Karicheri <m-karicheri2@ti.com>
> + * Santosh Shilimkar <santosh.shilimkar@ti.com>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +
> +#define REVID 0x000
> +#define PLLCTL 0x100
> +#define OCSEL 0x104
> +#define PLLSECCTL 0x108
> +#define PLLM 0x110
> +#define PREDIV 0x114
> +#define PLLDIV1 0x118
> +#define PLLDIV2 0x11c
> +#define PLLDIV3 0x120
> +#define OSCDIV 0x124
> +#define POSTDIV 0x128
> +#define BPDIV 0x12c
> +#define PLLCMD 0x138
> +#define PLLSTAT 0x13c
> +#define ALNCTL 0x140
> +#define DCHANGE 0x144
> +#define CKEN 0x148
> +#define CKSTAT 0x14c
> +#define SYSTAT 0x150
> +#define PLLDIV4 0x160
> +#define PLLDIV5 0x164
> +#define PLLDIV6 0x168
> +#define PLLDIV7 0x16c
> +#define PLLDIV8 0x170
> +#define PLLDIV9 0x174
> +
> +#define PLLM_MASK 0x1f
> +#define PREDIV_RATIO_MASK 0x1f
> +#define PLLDIV_RATIO_WIDTH 5
> +#define PLLDIV_ENABLE_SHIFT 15
> +#define OSCDIV_RATIO_WIDTH 5
> +#define POSTDIV_RATIO_MASK 0x1f
> +#define BPDIV_RATIO_SHIFT 0
> +#define BPDIV_RATIO_WIDTH 5
> +#define CKEN_OBSCLK_SHIFT 1
> +#define CKEN_AUXEN_SHIFT 0
> +
> +/**
> + * struct davinci_pll_clk - Main PLL clock
> + * @hw: clk_hw for the pll
> + * @base: Base memory address
> + * @parent_rate: Saved parent rate used by some child clocks
> + */
> +struct davinci_pll_clk {
> + struct clk_hw hw;
> + void __iomem *base;
> +};
> +
> +#define to_davinci_pll_clk(_hw) container_of((_hw), struct davinci_pll_clk, hw)
> +
> +static unsigned long davinci_pll_clk_recalc(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct davinci_pll_clk *pll = to_davinci_pll_clk(hw);
> + unsigned long rate = parent_rate;
> + u32 prediv, mult, postdiv;
> +
> + prediv = readl(pll->base + PREDIV) & PREDIV_RATIO_MASK;
> + mult = readl(pll->base + PLLM) & PLLM_MASK;
> + postdiv = readl(pll->base + POSTDIV) & POSTDIV_RATIO_MASK;
> +
> + rate /= prediv + 1;
> + rate *= mult + 1;
> + rate /= postdiv + 1;
> +
> + return rate;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +#include <linux/debugfs.h>
> +
> +#define DEBUG_REG(n) \
> +{ \
> + .name = #n, \
> + .offset = n, \
> +}
> +
> +static const struct debugfs_reg32 davinci_pll_regs[] = {
> + DEBUG_REG(REVID),
> + DEBUG_REG(PLLCTL),
> + DEBUG_REG(OCSEL),
> + DEBUG_REG(PLLSECCTL),
> + DEBUG_REG(PLLM),
> + DEBUG_REG(PREDIV),
> + DEBUG_REG(PLLDIV1),
> + DEBUG_REG(PLLDIV2),
> + DEBUG_REG(PLLDIV3),
> + DEBUG_REG(OSCDIV),
> + DEBUG_REG(POSTDIV),
> + DEBUG_REG(BPDIV),
> + DEBUG_REG(PLLCMD),
> + DEBUG_REG(PLLSTAT),
> + DEBUG_REG(ALNCTL),
> + DEBUG_REG(DCHANGE),
> + DEBUG_REG(CKEN),
> + DEBUG_REG(CKSTAT),
> + DEBUG_REG(SYSTAT),
> + DEBUG_REG(PLLDIV4),
> + DEBUG_REG(PLLDIV5),
> + DEBUG_REG(PLLDIV6),
> + DEBUG_REG(PLLDIV7),
> + DEBUG_REG(PLLDIV8),
> + DEBUG_REG(PLLDIV9),
> +};
> +
> +static int davinci_pll_debug_init(struct clk_hw *hw, struct dentry *dentry)
> +{
> + struct davinci_pll_clk *pll = to_davinci_pll_clk(hw);
> + struct debugfs_regset32 *regset;
> + struct dentry *d;
> +
> + regset = kzalloc(sizeof(regset), GFP_KERNEL);
> + if (!regset)
> + return -ENOMEM;
> +
> + regset->regs = davinci_pll_regs;
> + regset->nregs = ARRAY_SIZE(davinci_pll_regs);
> + regset->base = pll->base;
> +
> + d = debugfs_create_regset32("registers", 0400, dentry, regset);
> + if (IS_ERR(d)) {
> + kfree(regset);
> + return PTR_ERR(d);
> + }
> +
> + return 0;
> +}
> +#else
> +#define davinci_pll_debug_init NULL
> +#endif
> +
> +static const struct clk_ops davinci_pll_clk_ops = {
> + .recalc_rate = davinci_pll_clk_recalc,
> + .debug_init = davinci_pll_debug_init,
> +};
> +
> +/**
> + * davinci_pll_clk_register - Register a PLL clock
> + * @name: The clock name
> + * @parent_name: The parent clock name
> + * @base: The PLL's memory region
> + */
> +struct clk *davinci_pll_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base)
> +{
> + struct clk_init_data init;
> + struct davinci_pll_clk *pll;
> + struct clk *clk;
> +
> + pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> + if (!pll)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &davinci_pll_clk_ops;
> + init.parent_names = (parent_name ? &parent_name : NULL);
> + init.num_parents = (parent_name ? 1 : 0);
> +
> + pll->base = base;
> + pll->hw.init = &init;
> +
> + clk = clk_register(NULL, &pll->hw);
> + if (IS_ERR(clk))
> + kfree(pll);
> +
> + return clk;
> +}
> +
> +struct davinci_pll_aux_clk {
> + struct clk_hw hw;
> + struct davinci_pll_clk *pll;
> +};
> +
> +/**
> + * davinci_pll_aux_clk_register - Register bypass clock (AUXCLK)
> + * @name: The clock name
> + * @parent_name: The parent clock name (usually "ref_clk" since this bypasses
> + * the PLL)
> + * @base: The PLL memory region
> + */
> +struct clk *davinci_pll_aux_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base)
> +{
> + return clk_register_gate(NULL, name, parent_name, 0, base + CKEN,
> + CKEN_AUXEN_SHIFT, 0, NULL);
> +}
> +
> +/**
> + * davinci_pll_bpdiv_clk_register - Register bypass divider clock (SYSCLKBP)
> + * @name: The clock name
> + * @parent_name: The parent clock name (usually "ref_clk" since this bypasses
> + * the PLL)
> + * @base: The PLL memory region
> + */
> +struct clk *davinci_pll_bpdiv_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base)
> +{
> + return clk_register_divider(NULL, name, parent_name, 0, base + BPDIV,
> + BPDIV_RATIO_SHIFT, BPDIV_RATIO_WIDTH,
> + CLK_DIVIDER_READ_ONLY, NULL);
> +}
> +
> +/**
> + * davinci_pll_obs_clk_register - Register oscillator divider clock (OBSCLK)
> + * @name: The clock name
> + * @parent_names: The parent clock names
> + * @num_parents: The number of paren clocks
> + * @base: The PLL memory region
> + * @table: A table of values cooresponding to the parent clocks (see OCSEL
> + * register in SRM for values)
> + */
> +struct clk *davinci_pll_obs_clk_register(const char *name,
> + const char * const *parent_names,
> + u8 num_parents,
> + void __iomem *base,
> + u32 *table)
> +{
> + struct clk_mux *mux;
> + struct clk_gate *gate;
> + struct clk_divider *divider;
> + struct clk *clk;
> +
> + mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> + if (!mux)
> + return ERR_PTR(-ENOMEM);
> +
> + mux->reg = base + OCSEL;
> + mux->table = table;
> +
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate) {
> + kfree(mux);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + gate->reg = base + CKEN;
> + gate->bit_idx = CKEN_OBSCLK_SHIFT;
> +
> + divider = kzalloc(sizeof(*divider), GFP_KERNEL);
> + if (!divider) {
> + kfree(gate);
> + kfree(mux);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + divider->reg = base + OSCDIV;
> + divider->width = OSCDIV_RATIO_WIDTH;
> +
> + clk = clk_register_composite(NULL, name, parent_names, num_parents,
> + &mux->hw, &clk_mux_ops,
> + ÷r->hw, &clk_divider_ops,
> + &gate->hw, &clk_gate_ops, 0);
> + if (IS_ERR(clk)) {
> + kfree(divider);
> + kfree(gate);
> + kfree(mux);
> + }
> +
> + return clk;
> +}
> +
> +/**
> + * davinci_pll_div_clk_register - Register a PLLDIV (SYSCLK) clock
> + * @name: The clock name
> + * @parent_name: The parent clock name
> + * @base: The PLL memory region
> + * @id: The id of the divider (n in PLLDIVn)
> + */
> +struct clk *davinci_pll_div_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base,
> + u32 id)
> +{
> + const char * const *parent_names = (parent_name ? &parent_name : NULL);
> + int num_parents = (parent_name ? 1 : 0);
> + struct clk_gate *gate;
> + struct clk_divider *divider;
> + struct clk *clk;
> + u32 reg;
> +
> + /* PLLDIVn registers are not entirely consecutive */
> + if (id < 4)
> + reg = PLLDIV1 + 4 * (id - 1);
> + else
> + reg = PLLDIV4 + 4 * (id - 4);
> +
> + gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> + if (!gate)
> + return ERR_PTR(-ENOMEM);
> +
> + gate->reg = base + reg;
> + gate->bit_idx = PLLDIV_ENABLE_SHIFT;
> +
> + divider = kzalloc(sizeof(*divider), GFP_KERNEL);
> + if (!divider) {
> + kfree(gate);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + divider->reg = base + reg;
> + divider->width = PLLDIV_RATIO_WIDTH;
> + divider->flags = CLK_DIVIDER_READ_ONLY;
> +
> + clk = clk_register_composite(NULL, name, parent_names, num_parents,
> + NULL, NULL, ÷r->hw, &clk_divider_ops,
> + &gate->hw, &clk_gate_ops, 0);
> + if (IS_ERR(clk)) {
> + kfree(divider);
> + kfree(gate);
> + }
> +
> + return clk;
> +}
> diff --git a/drivers/clk/davinci/psc.c b/drivers/clk/davinci/psc.c
> new file mode 100644
> index 0000000..8ae85ee
> --- /dev/null
> +++ b/drivers/clk/davinci/psc.c
> @@ -0,0 +1,217 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Clock driver for DA8xx/AM17xx/AM18xx/OMAP-L13x PSC controllers
> + *
> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
> + *
> + * Based on: drivers/clk/keystone/gate.c
> + * Copyright (C) 2013 Texas Instruments.
> + * Murali Karicheri <m-karicheri2@ti.com>
> + * Santosh Shilimkar <santosh.shilimkar@ti.com>
> + *
> + * And: arch/arm/mach-davinci/psc.c
> + * Copyright (C) 2006 Texas Instruments.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/slab.h>
> +
> +/* PSC register offsets */
> +#define EPCPR 0x070
> +#define PTCMD 0x120
> +#define PTSTAT 0x128
> +#define PDSTAT 0x200
> +#define PDCTL 0x300
> +#define MDSTAT 0x800
> +#define MDCTL 0xa00
> +
> +/* PSC module states */
> +enum davinci_psc_state {
> + PSC_STATE_SWRSTDISABLE = 0,
> + PSC_STATE_SYNCRST = 1,
> + PSC_STATE_DISABLE = 2,
> + PSC_STATE_ENABLE = 3,
> +};
> +
> +#define MDSTAT_STATE_MASK 0x3f
> +#define MDSTAT_MCKOUT BIT(12)
> +#define PDSTAT_STATE_MASK 0x1f
> +#define MDCTL_FORCE BIT(31)
> +#define MDCTL_LRESET BIT(8)
> +#define PDCTL_EPCGOOD BIT(8)
> +#define PDCTL_NEXT BIT(0)
> +
> +/**
> + * struct davinci_psc_clk - PSC clock structure
> + * @hw: clk_hw for the psc
> + * @psc_data: PSC driver specific data
> + * @lpsc: Local PSC number (module id)
> + * @pd: Power domain
> + */
> +struct davinci_psc_clk {
> + struct clk_hw hw;
> + void __iomem *base;
> + u32 lpsc;
> + u32 pd;
> +};
> +
> +#define to_davinci_psc_clk(_hw) container_of(_hw, struct davinci_psc_clk, hw)
> +
> +static void psc_config(struct davinci_psc_clk *psc,
> + enum davinci_psc_state next_state)
> +{
> + u32 epcpr, ptcmd, pdstat, pdctl, mdstat, mdctl, ptstat;
> +
> + mdctl = readl(psc->base + MDCTL + 4 * psc->lpsc);
> + mdctl &= ~MDSTAT_STATE_MASK;
> + mdctl |= next_state;
> + /* TODO: old davinci clocks for da850 set MDCTL_FORCE bit for sata and
> + * dsp here. Is this really needed?
> + */
> + writel(mdctl, psc->base + MDCTL + 4 * psc->lpsc);
> +
> + pdstat = readl(psc->base + PDSTAT + 4 * psc->pd);
> + if ((pdstat & PDSTAT_STATE_MASK) == 0) {
> + pdctl = readl(psc->base + PDSTAT + 4 * psc->pd);
> + pdctl |= PDCTL_NEXT;
> + writel(pdctl, psc->base + PDSTAT + 4 * psc->pd);
> +
> + ptcmd = BIT(psc->pd);
> + writel(ptcmd, psc->base + PTCMD);
> +
> + do {
> + epcpr = __raw_readl(psc->base + EPCPR);
> + } while (!(epcpr & BIT(psc->pd)));
> +
> + pdctl = __raw_readl(psc->base + PDCTL + 4 * psc->pd);
> + pdctl |= PDCTL_EPCGOOD;
> + __raw_writel(pdctl, psc->base + PDCTL + 4 * psc->pd);
> + } else {
> + ptcmd = BIT(psc->pd);
> + writel(ptcmd, psc->base + PTCMD);
> + }
> +
> + do {
> + ptstat = readl(psc->base + PTSTAT);
> + } while (ptstat & BIT(psc->pd));
> +
> + do {
> + mdstat = readl(psc->base + MDSTAT + 4 * psc->lpsc);
> + } while (!((mdstat & MDSTAT_STATE_MASK) == next_state));
> +}
> +
> +static int davinci_psc_clk_enable(struct clk_hw *hw)
> +{
> + struct davinci_psc_clk *psc = to_davinci_psc_clk(hw);
> +
> + psc_config(psc, PSC_STATE_ENABLE);
> +
> + return 0;
> +}
> +
> +static void davinci_psc_clk_disable(struct clk_hw *hw)
> +{
> + struct davinci_psc_clk *psc = to_davinci_psc_clk(hw);
> +
> + psc_config(psc, PSC_STATE_DISABLE);
> +}
> +
> +static int davinci_psc_clk_is_enabled(struct clk_hw *hw)
> +{
> + struct davinci_psc_clk *psc = to_davinci_psc_clk(hw);
> + u32 mdstat;
> +
> + mdstat = readl(psc->base + MDSTAT + 4 * psc->lpsc);
> +
> + return (mdstat & MDSTAT_MCKOUT) ? 1 : 0;
> +}
> +
> +static const struct clk_ops davinci_psc_clk_ops = {
> + .enable = davinci_psc_clk_enable,
> + .disable = davinci_psc_clk_disable,
> + .is_enabled = davinci_psc_clk_is_enabled,
> +};
> +
> +/**
> + * davinci_psc_clk_register - register psc clock
> + * @dev: device that is registering this clock
> + * @name: name of this clock
> + * @parent_name: name of clock's parent
> + * @base: memory mapped register for the PSC
> + * @lpsc: local PSC number
> + * @pd: power domain
> + */
> +struct clk *davinci_psc_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base,
> + u32 lpsc, u32 pd)
> +{
> + struct clk_init_data init;
> + struct davinci_psc_clk *psc;
> + struct clk *clk;
> +
> + psc = kzalloc(sizeof(*psc), GFP_KERNEL);
> + if (!psc)
> + return ERR_PTR(-ENOMEM);
> +
> + init.name = name;
> + init.ops = &davinci_psc_clk_ops;
> + init.flags = 0;
> + init.parent_names = (parent_name ? &parent_name : NULL);
> + init.num_parents = (parent_name ? 1 : 0);
> +
> + psc->base = base;
> + psc->hw.init = &init;
> + psc->lpsc = lpsc;
> + psc->pd = pd;
> +
> + clk = clk_register(NULL, &psc->hw);
> + if (IS_ERR(clk))
> + kfree(psc);
> +
> + return clk;
> +}
> +
> +/* FIXME: This needs to be converted to a reset controller. But, the reset
> + * framework is currently device tree only.
> + */
> +
> +DEFINE_SPINLOCK(davinci_psc_reset_lock);
> +
> +static int davinci_psc_clk_reset(struct davinci_psc_clk *psc, bool reset)
> +{
> + unsigned long flags;
> + u32 mdctl;
> +
> + if (IS_ERR_OR_NULL(psc))
> + return -EINVAL;
> +
> + spin_lock_irqsave(&davinci_psc_reset_lock, flags);
> + mdctl = readl(psc->base + MDCTL + 4 * psc->lpsc);
> + if (reset)
> + mdctl &= ~MDCTL_LRESET;
> + else
> + mdctl |= MDCTL_LRESET;
> + writel(mdctl, psc->base + MDCTL + 4 * psc->lpsc);
> + spin_unlock_irqrestore(&davinci_psc_reset_lock, flags);
> +
> + return 0;
> +}
> +
> +int davinci_clk_reset_assert(struct clk *clk)
> +{
> + struct davinci_psc_clk *psc = to_davinci_psc_clk(__clk_get_hw(clk));
> +
> + return davinci_psc_clk_reset(psc, true);
> +}
> +EXPORT_SYMBOL(davinci_clk_reset_assert);
> +
> +int davinci_clk_reset_deassert(struct clk *clk)
> +{
> + struct davinci_psc_clk *psc = to_davinci_psc_clk(__clk_get_hw(clk));
> +
> + return davinci_psc_clk_reset(psc, false);
> +}
> +EXPORT_SYMBOL(davinci_clk_reset_deassert);
> diff --git a/include/linux/clk/davinci.h b/include/linux/clk/davinci.h
> new file mode 100644
> index 0000000..c5d2181
> --- /dev/null
> +++ b/include/linux/clk/davinci.h
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI Davinci clocks
> + *
> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
> + */
> +#ifndef __LINUX_CLK_DAVINCI_H__
> +#define __LINUX_CLK_DAVINCI_H__
> +
> +#include <linux/clk-provider.h>
> +#include <linux/types.h>
> +
> +struct clk *davinci_pll_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base);
> +struct clk *davinci_pll_aux_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base);
> +struct clk *davinci_pll_bpdiv_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base);
> +struct clk *davinci_pll_obs_clk_register(const char *name,
> + const char * const *parent_names,
> + u8 num_parents,
> + void __iomem *base,
> + u32 *table);
> +struct clk *davinci_pll_div_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base,
> + u32 id);
> +struct clk *davinci_psc_clk_register(const char *name,
> + const char *parent_name,
> + void __iomem *base,
> + u32 lpsc, u32 pd);
> +
> +/* convience macros for board declaration files */
> +#define EXT_CLK(n, r) clk_register_fixed_rate(NULL, (n), NULL, 0, (r))
> +#define FIX_CLK(n, p) clk_register_fixed_factor(NULL, (n), (p), 0, 1, 1)
> +#define PLL_CLK davinci_pll_clk_register
> +#define PLL_DIV_CLK davinci_pll_div_clk_register
> +#define PLL_AUX_CLK davinci_pll_aux_clk_register
> +#define PLL_BP_CLK davinci_pll_bpdiv_clk_register
> +#define PLL_OBS_CLK davinci_pll_obs_clk_register
> +#define PSC_CLK davinci_psc_clk_register
> +
> +#endif /* __LINUX_CLK_DAVINCI_H__ */
> diff --git a/include/linux/platform_data/davinci_clk.h b/include/linux/platform_data/davinci_clk.h
> new file mode 100644
> index 0000000..7576ace
> --- /dev/null
> +++ b/include/linux/platform_data/davinci_clk.h
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI DaVinci Clock support
> + *
> + * Copyright (C) 2017 David Lechner <david@lechnology.com>
> + */
> +
> +#ifndef __PLATFORM_DATA_DAVINCI_CLK_H
> +#define __PLATFORM_DATA_DAVINCI_CLK_H
> +
> +#include <linux/types.h>
> +
> +/**
> + * da8xx_cfgchip_clk_data - DA8xx CFGCHIP clock platform data
> + * @usb0_use_refclkin: when true, use USB_REFCLKIN, otherwise use AUXCLK for
> + * USB 2.0 PHY clock
> + * @usb1_use_refclkin: when true, use USB_REFCLKIN, otherwise use USB 2.0 PHY
> + * PLL for USB 1.1 PHY clock
> + */
> +struct da8xx_cfgchip_clk_data {
> + bool usb0_use_refclkin;
> + bool usb1_use_refclkin;
> +};
> +
> +#endif /* __PLATFORM_DATA_DAVINCI_CLK_H */
>
^ permalink raw reply
* [PATCH V3 3/3] arm64: Extend early page table code to allow for larger kernels
From: Ard Biesheuvel @ 2018-01-02 22:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102151254.4063-4-steve.capper@arm.com>
Hi Steve,
On 2 January 2018 at 15:12, Steve Capper <steve.capper@arm.com> wrote:
> Currently the early assembler page table code assumes that precisely
> 1xpgd, 1xpud, 1xpmd are sufficient to represent the early kernel text
> mappings.
>
> Unfortunately this is rarely the case when running with a 16KB granule,
> and we also run into limits with 4KB granule when building much larger
> kernels.
>
> This patch re-writes the early page table logic to compute indices of
> mappings for each level of page table, and if multiple indices are
> required, the next-level page table is scaled up accordingly.
>
> Also the required size of the swapper_pg_dir is computed at link time
> to cover the mapping [KIMAGE_ADDR + VOFFSET, _end]. When KASLR is
> enabled, an extra page is set aside for each level that may require extra
> entries at runtime.
>
> Signed-off-by: Steve Capper <steve.capper@arm.com>
>
> ---
> Changed in V3:
> Corrected KASLR computation
> Rebased against arm64/for-next/core, particularly Kristina's 52-bit
> PA series.
> ---
> arch/arm64/include/asm/kernel-pgtable.h | 47 ++++++++++-
> arch/arm64/include/asm/pgtable.h | 1 +
> arch/arm64/kernel/head.S | 145 +++++++++++++++++++++++---------
> arch/arm64/kernel/vmlinux.lds.S | 1 +
> arch/arm64/mm/mmu.c | 3 +-
> 5 files changed, 157 insertions(+), 40 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
> index 77a27af01371..82386e860dd2 100644
> --- a/arch/arm64/include/asm/kernel-pgtable.h
> +++ b/arch/arm64/include/asm/kernel-pgtable.h
> @@ -52,7 +52,52 @@
> #define IDMAP_PGTABLE_LEVELS (ARM64_HW_PGTABLE_LEVELS(PHYS_MASK_SHIFT))
> #endif
>
> -#define SWAPPER_DIR_SIZE (SWAPPER_PGTABLE_LEVELS * PAGE_SIZE)
> +
> +/*
> + * If KASLR is enabled, then an offset K is added to the kernel address
> + * space. The bottom 21 bits of this offset are zero to guarantee 2MB
> + * alignment for PA and VA.
> + *
> + * For each pagetable level of the swapper, we know that the shift will
> + * be larger than 21 (for the 4KB granule case we use section maps thus
> + * the smallest shift is actually 30) thus there is the possibility that
> + * KASLR can increase the number of pagetable entries by 1, so we make
> + * room for this extra entry.
> + *
> + * Note KASLR cannot increase the number of required entries for a level
> + * by more than one because it increments both the virtual start and end
> + * addresses equally (the extra entry comes from the case where the end
> + * address is just pushed over a boundary and the start address isn't).
> + */
> +
> +#ifdef CONFIG_RANDOMIZE_BASE
> +#define EARLY_KASLR (1)
> +#else
> +#define EARLY_KASLR (0)
> +#endif
> +
> +#define EARLY_ENTRIES(vstart, vend, shift) (((vend) >> (shift)) \
> + - ((vstart) >> (shift)) + 1 + EARLY_KASLR)
> +
> +#define EARLY_PGDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, PGDIR_SHIFT))
> +
> +#if SWAPPER_PGTABLE_LEVELS > 3
> +#define EARLY_PUDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, PUD_SHIFT))
> +#else
> +#define EARLY_PUDS(vstart, vend) (0)
> +#endif
> +
> +#if SWAPPER_PGTABLE_LEVELS > 2
> +#define EARLY_PMDS(vstart, vend) (EARLY_ENTRIES(vstart, vend, SWAPPER_TABLE_SHIFT))
> +#else
> +#define EARLY_PMDS(vstart, vend) (0)
> +#endif
> +
> +#define EARLY_PAGES(vstart, vend) ( 1 /* PGDIR page */ \
> + + EARLY_PGDS((vstart), (vend)) /* each PGDIR needs a next level page table */ \
> + + EARLY_PUDS((vstart), (vend)) /* each PUD needs a next level page table */ \
> + + EARLY_PMDS((vstart), (vend))) /* each PMD needs a next level page table */
> +#define SWAPPER_DIR_SIZE (PAGE_SIZE * EARLY_PAGES(KIMAGE_VADDR + TEXT_OFFSET, _end))
> #define IDMAP_DIR_SIZE (IDMAP_PGTABLE_LEVELS * PAGE_SIZE)
>
> #ifdef CONFIG_ARM64_SW_TTBR0_PAN
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index bfa237e892f1..54b0a8398055 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -706,6 +706,7 @@ static inline void pmdp_set_wrprotect(struct mm_struct *mm,
> #endif
>
> extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
> +extern pgd_t swapper_pg_end[];
> extern pgd_t idmap_pg_dir[PTRS_PER_PGD];
> extern pgd_t tramp_pg_dir[PTRS_PER_PGD];
>
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 66f01869e97c..539e2642ed41 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -191,44 +191,110 @@ ENDPROC(preserve_boot_args)
> .endm
>
> /*
> - * Macro to populate the PGD (and possibily PUD) for the corresponding
> - * block entry in the next level (tbl) for the given virtual address.
> + * Macro to populate page table entries, these entries can be pointers to the next level
> + * or last level entries pointing to physical memory.
> *
> - * Preserves: tbl, next, virt
> - * Corrupts: ptrs_per_pgd, tmp1, tmp2
> + * tbl: page table address
> + * rtbl: pointer to page table or physical memory
> + * index: start index to write
> + * eindex: end index to write - [index, eindex] written to
> + * flags: flags for pagetable entry to or in
> + * inc: increment to rtbl between each entry
> + * tmp1: temporary variable
> + *
> + * Preserves: tbl, eindex, flags, inc
> + * Corrupts: index, tmp1
> + * Returns: rtbl
> */
> - .macro create_pgd_entry, tbl, virt, ptrs_per_pgd, tmp1, tmp2
> - create_table_entry \tbl, \virt, PGDIR_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
> -#if SWAPPER_PGTABLE_LEVELS > 3
> - mov \ptrs_per_pgd, PTRS_PER_PUD
> - create_table_entry \tbl, \virt, PUD_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
> -#endif
> -#if SWAPPER_PGTABLE_LEVELS > 2
> - mov \ptrs_per_pgd, PTRS_PER_PTE
> - create_table_entry \tbl, \virt, SWAPPER_TABLE_SHIFT, \ptrs_per_pgd, \tmp1, \tmp2
> -#endif
> + .macro populate_entries, tbl, rtbl, index, eindex, flags, inc, tmp1
> +9999: phys_to_pte \rtbl, \tmp1
I know this is existing code, but you could take the opportunity to
replace this label with
.L\@ :
(and update the branch instruction accordingly) so that we don't have
to rely on the uniqueness of '9999'
> + orr \tmp1, \tmp1, \flags // tmp1 = table entry
> + str \tmp1, [\tbl, \index, lsl #3]
> + add \rtbl, \rtbl, \inc // rtbl = pa next level
> + add \index, \index, #1
> + cmp \index, \eindex
> + b.ls 9999b
> .endm
>
> /*
> - * Macro to populate block entries in the page table for the start..end
> - * virtual range (inclusive).
> + * Compute indices of table entries from virtual address range. If multiple entries
> + * were needed in the previous page table level then the next page table level is assumed
> + * to be composed of multiple pages. (This effectively scales the end index).
> + *
> + * vstart: virtual address of start of range
> + * vend: virtual address of end of range
> + * shift: shift used to transform virtual address into index
> + * ptrs: number of entries in page table
> + * istart: index in table corresponding to vstart
> + * iend: index in table corresponding to vend
> + * count: On entry: how many entries required in previous level, scales our end index
> + * On exit: returns how many entries required for next page table level
> *
If you make 'count' the number of /additional/ entries, you no longer
have to add/sub #1 each time.
> - * Preserves: tbl, flags
> - * Corrupts: phys, start, end, tmp, pstate
> + * Preserves: vstart, vend, shift, ptrs
> + * Returns: istart, iend, count
> */
> - .macro create_block_map, tbl, flags, phys, start, end, tmp
> - lsr \start, \start, #SWAPPER_BLOCK_SHIFT
> - and \start, \start, #PTRS_PER_PTE - 1 // table index
> - bic \phys, \phys, #SWAPPER_BLOCK_SIZE - 1
> - lsr \end, \end, #SWAPPER_BLOCK_SHIFT
> - and \end, \end, #PTRS_PER_PTE - 1 // table end index
> -9999: phys_to_pte \phys, \tmp
> - orr \tmp, \tmp, \flags // table entry
> - str \tmp, [\tbl, \start, lsl #3] // store the entry
> - add \start, \start, #1 // next entry
> - add \phys, \phys, #SWAPPER_BLOCK_SIZE // next block
> - cmp \start, \end
> - b.ls 9999b
> + .macro compute_indices, vstart, vend, shift, ptrs, istart, iend, count
> + lsr \iend, \vend, \shift
> + mov \istart, \ptrs
> + sub \istart, \istart, #1
> + and \iend, \iend, \istart // iend = (vend >> shift) & (ptrs - 1)
> + mov \istart, \ptrs
> + sub \count, \count, #1
> + mul \istart, \istart, \count
> + add \iend, \iend, \istart // iend += (count - 1) * ptrs
> + // our entries span multiple tables
> +
> + lsr \istart, \vstart, \shift
> + mov \count, \ptrs
> + sub \count, \count, #1
> + and \istart, \istart, \count
> +
> + sub \count, \iend, \istart
> + add \count, \count, #1
> + .endm
> +
You can simplify this macro by using an immediate for \ptrs. Please
see the diff below [whitespace mangling courtesy of Gmail]
> +/*
> + * Map memory for specified virtual address range. Each level of page table needed supports
> + * multiple entries. If a level requires n entries the next page table level is assumed to be
> + * formed from n pages.
> + *
> + * tbl: location of page table
> + * rtbl: address to be used for first level page table entry (typically tbl + PAGE_SIZE)
> + * vstart: start address to map
> + * vend: end address to map - we map [vstart, vend]
> + * flags: flags to use to map last level entries
> + * phys: physical address corresponding to vstart - physical memory is contiguous
> + * pgds: the number of pgd entries
> + *
> + * Temporaries: istart, iend, tmp, count, sv - these need to be different registers
> + * Preserves: vstart, vend, flags
> + * Corrupts: tbl, rtbl, istart, iend, tmp, count, sv
> + */
> + .macro map_memory, tbl, rtbl, vstart, vend, flags, phys, pgds, istart, iend, tmp, count, sv
> + add \rtbl, \tbl, #PAGE_SIZE
> + mov \sv, \rtbl
> + mov \count, #1
#0 if you make \count the number of additional entries.
In any case, for the series:
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> + compute_indices \vstart, \vend, #PGDIR_SHIFT, \pgds, \istart, \iend, \count
> + populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
> + mov \tbl, \sv
> + mov \sv, \rtbl
> +
> +#if SWAPPER_PGTABLE_LEVELS > 3
> + compute_indices \vstart, \vend, #PUD_SHIFT, #PTRS_PER_PUD, \istart, \iend, \count
> + populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
> + mov \tbl, \sv
> + mov \sv, \rtbl
> +#endif
> +
> +#if SWAPPER_PGTABLE_LEVELS > 2
> + compute_indices \vstart, \vend, #SWAPPER_TABLE_SHIFT, #PTRS_PER_PMD, \istart, \iend, \count
> + populate_entries \tbl, \rtbl, \istart, \iend, #PMD_TYPE_TABLE, #PAGE_SIZE, \tmp
> + mov \tbl, \sv
> +#endif
> +
> + compute_indices \vstart, \vend, #SWAPPER_BLOCK_SHIFT, #PTRS_PER_PTE, \istart, \iend, \count
> + bic \count, \phys, #SWAPPER_BLOCK_SIZE - 1
> + populate_entries \tbl, \count, \istart, \iend, \flags, #SWAPPER_BLOCK_SIZE, \tmp
> .endm
>
> /*
> @@ -246,14 +312,16 @@ __create_page_tables:
> * dirty cache lines being evicted.
> */
> adrp x0, idmap_pg_dir
> - ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
> + adrp x1, swapper_pg_end
> + sub x1, x1, x0
> bl __inval_dcache_area
>
> /*
> * Clear the idmap and swapper page tables.
> */
> adrp x0, idmap_pg_dir
> - ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
> + adrp x1, swapper_pg_end
> + sub x1, x1, x0
> 1: stp xzr, xzr, [x0], #16
> stp xzr, xzr, [x0], #16
> stp xzr, xzr, [x0], #16
> @@ -318,10 +386,10 @@ __create_page_tables:
> #endif
> 1:
> ldr_l x4, idmap_ptrs_per_pgd
> - create_pgd_entry x0, x3, x4, x5, x6
> mov x5, x3 // __pa(__idmap_text_start)
> adr_l x6, __idmap_text_end // __pa(__idmap_text_end)
> - create_block_map x0, x7, x3, x5, x6, x4
> +
> + map_memory x0, x1, x3, x6, x7, x3, x4, x10, x11, x12, x13, x14
>
> /*
> * Map the kernel image (starting with PHYS_OFFSET).
> @@ -330,12 +398,12 @@ __create_page_tables:
> mov_q x5, KIMAGE_VADDR + TEXT_OFFSET // compile time __va(_text)
> add x5, x5, x23 // add KASLR displacement
> mov x4, PTRS_PER_PGD
> - create_pgd_entry x0, x5, x4, x3, x6
> adrp x6, _end // runtime __pa(_end)
> adrp x3, _text // runtime __pa(_text)
> sub x6, x6, x3 // _end - _text
> add x6, x6, x5 // runtime __va(_end)
> - create_block_map x0, x7, x3, x5, x6, x4
> +
> + map_memory x0, x1, x5, x6, x7, x3, x4, x10, x11, x12, x13, x14
>
> /*
> * Since the page tables have been populated with non-cacheable
> @@ -343,7 +411,8 @@ __create_page_tables:
> * tables again to remove any speculatively loaded cache lines.
> */
> adrp x0, idmap_pg_dir
> - ldr x1, =(IDMAP_DIR_SIZE + SWAPPER_DIR_SIZE + RESERVED_TTBR0_SIZE)
> + adrp x1, swapper_pg_end
> + sub x1, x1, x0
> dmb sy
> bl __inval_dcache_area
>
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index 4c7112a47469..0221aca6493d 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -230,6 +230,7 @@ SECTIONS
> #endif
> swapper_pg_dir = .;
> . += SWAPPER_DIR_SIZE;
> + swapper_pg_end = .;
>
> __pecoff_data_size = ABSOLUTE(. - __initdata_begin);
> _end = .;
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 4071602031ed..fdac11979bae 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -644,7 +644,8 @@ void __init paging_init(void)
> * allocated with it.
> */
> memblock_free(__pa_symbol(swapper_pg_dir) + PAGE_SIZE,
> - SWAPPER_DIR_SIZE - PAGE_SIZE);
> + __pa_symbol(swapper_pg_end) - __pa_symbol(swapper_pg_dir)
> + - PAGE_SIZE);
> }
>
> /*
> --
> 2.11.0
>
------8<---------
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 539e2642ed41..0432dd8d083c 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -235,9 +235,7 @@ ENDPROC(preserve_boot_args)
*/
.macro compute_indices, vstart, vend, shift, ptrs, istart, iend, count
lsr \iend, \vend, \shift
- mov \istart, \ptrs
- sub \istart, \istart, #1
- and \iend, \iend, \istart // iend = (vend >> shift) & (ptrs - 1)
+ and \iend, \iend, \ptrs - 1 // iend = (vend >> shift) & (ptrs - 1)
mov \istart, \ptrs
sub \count, \count, #1
mul \istart, \istart, \count
@@ -245,9 +243,7 @@ ENDPROC(preserve_boot_args)
// our entries span multiple tables
lsr \istart, \vstart, \shift
- mov \count, \ptrs
- sub \count, \count, #1
- and \istart, \istart, \count
+ and \istart, \istart, \ptrs - 1
sub \count, \iend, \istart
add \count, \count, #1
@@ -376,6 +372,7 @@ __create_page_tables:
mov x4, EXTRA_PTRS
create_table_entry x0, x3, EXTRA_SHIFT, x4, x5, x6
+ .set .Lidmap_ptrs_per_pgd, PTRS_PER_PGD
#else
/*
* If VA_BITS == 48, we don't have to configure an additional
@@ -383,13 +380,13 @@ __create_page_tables:
*/
mov x4, #1 << (PHYS_MASK_SHIFT - PGDIR_SHIFT)
str_l x4, idmap_ptrs_per_pgd, x5
+ .set .Lidmap_ptrs_per_pgd, 1 << (PHYS_MASK_SHIFT - PGDIR_SHIFT)
#endif
1:
- ldr_l x4, idmap_ptrs_per_pgd
mov x5, x3 // __pa(__idmap_text_start)
adr_l x6, __idmap_text_end // __pa(__idmap_text_end)
- map_memory x0, x1, x3, x6, x7, x3, x4, x10, x11, x12, x13, x14
+ map_memory x0, x1, x3, x6, x7, x3, .Lidmap_ptrs_per_pgd, x10,
x11, x12, x13, x14
/*
* Map the kernel image (starting with PHYS_OFFSET).
@@ -397,13 +394,12 @@ __create_page_tables:
adrp x0, swapper_pg_dir
mov_q x5, KIMAGE_VADDR + TEXT_OFFSET // compile time __va(_text)
add x5, x5, x23 // add KASLR displacement
- mov x4, PTRS_PER_PGD
adrp x6, _end // runtime __pa(_end)
adrp x3, _text // runtime __pa(_text)
sub x6, x6, x3 // _end - _text
add x6, x6, x5 // runtime __va(_end)
- map_memory x0, x1, x5, x6, x7, x3, x4, x10, x11, x12, x13, x14
+ map_memory x0, x1, x5, x6, x7, x3, PTRS_PER_PGD, x10, x11, x12, x13, x14
/*
* Since the page tables have been populated with non-cacheable
^ permalink raw reply related
* [RESEND PATCH v2 09/15] ASoC: qcom: qdsp6: Add support to Q6CORE
From: Bjorn Andersson @ 2018-01-02 22:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-10-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds support to core apr service, which is used to query
> status of other static and dynamic services on the dsp.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> sound/soc/qcom/Kconfig | 5 +
> sound/soc/qcom/qdsp6/Makefile | 1 +
> sound/soc/qcom/qdsp6/q6core.c | 227 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 233 insertions(+)
> create mode 100644 sound/soc/qcom/qdsp6/q6core.c
>
> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
> index 7ebdb879a8a3..121b9c957024 100644
> --- a/sound/soc/qcom/Kconfig
> +++ b/sound/soc/qcom/Kconfig
> @@ -56,11 +56,16 @@ config SND_SOC_QDSP6_ASM
> tristate
> default n
>
> +config SND_SOC_QDSP6_CORE
> + tristate
> + default n
> +
> config SND_SOC_QDSP6
> tristate "SoC ALSA audio driver for QDSP6"
> select SND_SOC_QDSP6_AFE
> select SND_SOC_QDSP6_ADM
> select SND_SOC_QDSP6_ASM
> + select SND_SOC_QDSP6_CORE
> help
> To add support for MSM QDSP6 Soc Audio.
> This will enable sound soc platform specific
> diff --git a/sound/soc/qcom/qdsp6/Makefile b/sound/soc/qcom/qdsp6/Makefile
> index 49dd3ccab27b..ad7f10691e54 100644
> --- a/sound/soc/qcom/qdsp6/Makefile
> +++ b/sound/soc/qcom/qdsp6/Makefile
> @@ -1,3 +1,4 @@
> obj-$(CONFIG_SND_SOC_QDSP6_AFE) += q6afe.o
> obj-$(CONFIG_SND_SOC_QDSP6_ADM) += q6adm.o
> obj-$(CONFIG_SND_SOC_QDSP6_ASM) += q6asm.o
> +obj-$(CONFIG_SND_SOC_QDSP6_CORE) += q6core.o
> diff --git a/sound/soc/qcom/qdsp6/q6core.c b/sound/soc/qcom/qdsp6/q6core.c
> new file mode 100644
> index 000000000000..d4e2dbc62489
> --- /dev/null
> +++ b/sound/soc/qcom/qdsp6/q6core.c
> @@ -0,0 +1,227 @@
> +/* SPDX-License-Identifier: GPL-2.0
> +* Copyright (c) 2017, Linaro Limited
> +*/
> +#include <linux/slab.h>
> +#include <linux/wait.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/jiffies.h>
> +#include <linux/wait.h>
> +#include <linux/soc/qcom/apr.h>
> +#include <linux/platform_device.h>
> +#include <sound/asound.h>
> +#include "common.h"
> +
> +#define ADSP_STATE_READY_TIMEOUT_MS 3000
> +#define Q6_READY_TIMEOUT_MS 100
> +#define AVCS_CMD_ADSP_EVENT_GET_STATE 0x0001290C
> +#define AVCS_CMDRSP_ADSP_EVENT_GET_STATE 0x0001290D
> +#define AVCS_GET_VERSIONS 0x00012905
> +#define AVCS_GET_VERSIONS_RSP 0x00012906
> +
> +struct avcs_svc_info {
> + uint32_t service_id;
> + uint32_t version;
> +} __packed;
> +
> +struct q6core {
> + struct apr_device *adev;
> + wait_queue_head_t wait;
> + uint32_t avcs_state;
> + int resp_received;
bool
> + uint32_t num_services;
> + struct avcs_svc_info *svcs_info;
> +};
> +
> +static struct apr_device_id static_services[] = {
> + ADSP_AUDIO_APR_DEV("AFE", APR_SVC_AFE),
> + ADSP_AUDIO_APR_DEV("ASM", APR_SVC_ASM),
> + ADSP_AUDIO_APR_DEV("ADM", APR_SVC_ADM),
> + ADSP_AUDIO_APR_DEV("TEST", APR_SVC_TEST_CLIENT),
> + ADSP_AUDIO_APR_DEV("MVM", APR_SVC_ADSP_MVM),
> + ADSP_AUDIO_APR_DEV("CVS", APR_SVC_ADSP_CVS),
> + ADSP_AUDIO_APR_DEV("CVP", APR_SVC_ADSP_CVP),
> + ADSP_AUDIO_APR_DEV("USM", APR_SVC_USM),
> + ADSP_AUDIO_APR_DEV("VIDC", APR_SVC_VIDC),
> + ADSP_AUDIO_APR_DEV("LSM", APR_SVC_LSM),
> +};
> +
> +static int core_callback(struct apr_device *adev, struct apr_client_data *data)
> +{
> + struct q6core *core = dev_get_drvdata(&adev->dev);
> + uint32_t *payload;
> +
> + switch (data->opcode) {
> + case AVCS_GET_VERSIONS_RSP:
> + payload = data->payload;
> + core->num_services = payload[1];
Describe the payload in a struct (with flexible array member for the
svcs_info list).
> +
> + if (!core->svcs_info)
> + core->svcs_info = kcalloc(core->num_services,
> + sizeof(*core->svcs_info),
> + GFP_ATOMIC);
> + if (!core->svcs_info)
> + return -ENOMEM;
> +
If we receive this twice with different num_services for some reason the
memcpy might trash the heap.
But as this is the get_version response and we're only going to issue
that once you should remove the check for !core->svcs_info above.
And don't forget to free svcs_info once you have added your services.
> + /* svc info is after 8 bytes */
> + memcpy(core->svcs_info, payload + 2,
> + core->num_services * sizeof(*core->svcs_info));
> +
> + core->resp_received = 1;
> + wake_up(&core->wait);
> +
> + break;
> + case AVCS_CMDRSP_ADSP_EVENT_GET_STATE:
> + payload = data->payload;
> + core->avcs_state = payload[0];
> +
> + core->resp_received = 1;
> + wake_up(&core->wait);
> + break;
> + default:
> + dev_err(&adev->dev, "Message id from adsp core svc: 0x%x\n",
> + data->opcode);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +void q6core_add_service(struct device *dev, uint32_t svc_id, uint32_t version)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(static_services); i++) {
> + if (static_services[i].svc_id == svc_id) {
> + static_services[i].svc_version = version;
> + apr_add_device(dev->parent, &static_services[i]);
> + dev_info(dev,
Please don't spam the logs, dev_dbg() should be enough. And as
apr_add_device() returns you can find the devices registered in /sys
> + "Adding SVC: %s: id 0x%x API Ver 0x%x:0x%x\n",
> + static_services[i].name, svc_id,
> + APR_SVC_MAJOR_VERSION(version),
> + APR_SVC_MINOR_VERSION(version));
> + }
> + }
> +}
> +
> +static void q6core_add_static_services(struct q6core *core)
The name of this function is deceiving, it doesn't really add the static
services. It adds devices for the services that we've been informed
exists, by the other side - using the static list of services.
Per the comment on a previous patch I don't think the "name" in
apr_device_id provides any real value and in this case if forces you to
perform a lookup using this table.
If you drop the name, you can loop over the list of service ids returned
from the remote and just register them with a hard coded domain id
(based on apr instance?) and client_id. You don't need the lookup table.
> +{
> + int i;
> + struct apr_device *adev = core->adev;
> + struct avcs_svc_info *svcs_info = core->svcs_info;
> +
> + for (i = 0; i < core->num_services; i++)
> + q6core_add_service(&adev->dev, svcs_info[i].service_id,
> + svcs_info[i].version);
> +}
> +
> +static int q6core_get_svc_versions(struct q6core *core)
> +{
> + struct apr_device *adev = core->adev;
> + struct apr_hdr hdr = {0};
> + int rc;
> +
> + hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
> + APR_HDR_LEN(APR_HDR_SIZE), APR_PKT_VER);
> + hdr.pkt_size = APR_PKT_SIZE(APR_HDR_SIZE, 0);
> + hdr.opcode = AVCS_GET_VERSIONS;
> +
> + rc = apr_send_pkt(adev, (uint32_t *)&hdr);
> + if (rc < 0)
> + return rc;
> +
> + rc = wait_event_timeout(core->wait, (core->resp_received == 1),
> + msecs_to_jiffies(Q6_READY_TIMEOUT_MS));
The wait and resp_received could favourably be expressed as a completion
instead, as all we care about is that this happened once.
> + if (rc > 0 && core->resp_received) {
> + core->resp_received = 0;
> + return 0;
> + }
It wasn't obvious at first sight that this is the success case and the
return rc below was the error case...
> +
> + return rc;
And this will actually be 0 if core->resp_received has not become 1 at
the timeout.
> +}
> +
> +static bool q6core_is_adsp_ready(struct q6core *core)
> +{
> + struct apr_device *adev = core->adev;
> + struct apr_hdr hdr = {0};
> + int rc;
> +
> + hdr.hdr_field = APR_HDR_FIELD(APR_MSG_TYPE_SEQ_CMD,
> + APR_HDR_LEN(APR_HDR_SIZE), APR_PKT_VER);
> + hdr.pkt_size = APR_PKT_SIZE(APR_HDR_SIZE, 0);
> + hdr.opcode = AVCS_CMD_ADSP_EVENT_GET_STATE;
> +
> + rc = apr_send_pkt(adev, (uint32_t *)&hdr);
> + if (rc < 0)
> + return false;
> +
> + rc = wait_event_timeout(core->wait, (core->resp_received == 1),
> + msecs_to_jiffies(Q6_READY_TIMEOUT_MS));
I think this too would be nicer to describe with a completion.
Currently it's possible to ask is the dsp is ready, time out and ask
again, just to receive the first ack and continue. The service request
sleep might then wake up on this previous ack.
If you describe this by two different completions for the two waits you
avoid any such race conditions occurring.
> + if (rc > 0 && core->resp_received) {
> + core->resp_received = 0;
> + if (core->avcs_state == 0x1)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static int q6core_probe(struct apr_device *adev)
> +{
> + struct q6core *core;
> + unsigned long timeout = jiffies +
> + msecs_to_jiffies(ADSP_STATE_READY_TIMEOUT_MS);
> + int ret = 0;
> +
> + core = devm_kzalloc(&adev->dev, sizeof(*core), GFP_KERNEL);
> + if (!core)
> + return -ENOMEM;
> +
> + dev_set_drvdata(&adev->dev, core);
> +
> + core->adev = adev;
> + init_waitqueue_head(&core->wait);
> +
> + do {
> + if (!q6core_is_adsp_ready(core)) {
> + dev_info(&adev->dev, "ADSP Audio isn't ready\n");
> + } else {
> + dev_info(&adev->dev, "ADSP Audio is ready\n");
> +
> + ret = q6core_get_svc_versions(core);
> + if (!ret)
> + q6core_add_static_services(core);
> +
> + break;
> + }
> + } while (time_after(timeout, jiffies));
This would be much better rewritten as:
for (;;) {
if (q6core_is_adsp_ready(core))
break;
if (time_after(timeout, jiffies))
return -ETIMEDOUT;
}
ret = q6core_get_svc_versions(core);
if (ret)
return ret;
q6core_add_static_services(core);
> +
> + return ret;
> +}
> +
> +static int q6core_exit(struct apr_device *adev)
> +{
> + return 0;
> +}
> +
> +static const struct apr_device_id core_id[] = {
> + {"Q6CORE", APR_DOMAIN_ADSP, APR_SVC_ADSP_CORE, APR_CLIENT_AUDIO},
> + { },
> +};
> +
> +static struct apr_driver qcom_q6core_driver = {
> + .probe = q6core_probe,
> + .remove = q6core_exit,
> + .callback = core_callback,
> + .id_table = core_id,
> + .driver = {
> + .name = "qcom-q6core",
> + },
Indentation.
> +};
> +
> +module_apr_driver(qcom_q6core_driver);
> +
> +MODULE_AUTHOR("Srinivas Kandagatla <srinivas.kandagatla@linaro.org");
> +MODULE_DESCRIPTION("q6 core");
> +MODULE_LICENSE("GPL v2");
> --
> 2.15.0
>
^ permalink raw reply
* [PATCH] ARM: dts: n900: Add aliases for lcd and tvout displays
From: Ivaylo Dimitrov @ 2018-01-02 22:17 UTC (permalink / raw)
To: linux-arm-kernel
When both lcd and tv are enabled, the order in which they will be probed is
unknown, so it might happen (and it happens in reality) that tv is
configured as display0 and lcd as display1, which results in nothing
displayed on lcd, as display1 is disabled by default.
Fix that by providing correct aliases for lcd and tv
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
arch/arm/boot/dts/omap3-n900.dts | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index 669c51c..4b9c3a7 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -35,6 +35,8 @@
i2c1 = &i2c1;
i2c2 = &i2c2;
i2c3 = &i2c3;
+ display0 = &lcd;
+ display1 = &tv;
};
cpus {
@@ -965,7 +967,7 @@
ti,esd-recovery-timeout-ms = <8000>;
};
- acx565akm at 2 {
+ lcd: acx565akm at 2 {
compatible = "sony,acx565akm";
spi-max-frequency = <6000000>;
reg = <2>;
--
1.9.1
^ permalink raw reply related
* [PATCH v3 09/34] clk: bcm2835: change clk_get_rate() helper return type
From: Eric Anholt @ 2018-01-02 22:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1514835793-1104-10-git-send-email-pure.logic@nexus-software.ie>
Bryan O'Donoghue <pure.logic@nexus-software.ie> writes:
> bcm2835_pll_rate_from_divisor returns a long but the function calling it
> returns an unsigned long. There's no reason to have a type disparity here
> so tidy up the return type of bcm2835_pll_rate_from_divisor() from signed
> to unsigned long.
I'm still surprised that clocks are using longs instead of u64s, but
this seems like a fine change. For the 2 bcm2835 patches,
Reviewed-by: Eric Anholt <eric@anholt.net>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180102/622b6e3d/attachment.sig>
^ permalink raw reply
* [PATCH v2] PCI: imx6: Add PHY reference clock source support
From: Fabio Estevam @ 2018-01-02 22:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1514709094-4110-1-git-send-email-ilya@compulab.co.il>
Hi Ilya,
+ Rob and dt list
On Sun, Dec 31, 2017 at 6:31 AM, Ilya Ledvich <ilya@compulab.co.il> wrote:
> i.MX7D variant of the IP can use either Crystal Oscillator input
> or internal clock input as a Reference Clock input for PCIe PHY.
> Add support for an optional property 'pcie-phy-refclk-internal'.
> If present then an internal clock input is used as PCIe PHY
> reference clock source. By default an external oscillator input
> is still used.
>
> Verified on Compulab SBC-iMX7 Single Board Computer.
>
> Signed-off-by: Ilya Ledvich <ilya@compulab.co.il>
> ---
> Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 5 +++++
> drivers/pci/dwc/pci-imx6.c | 8 +++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 7b1e48b..581bc09 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -50,6 +50,11 @@ Additional required properties for imx7d-pcie:
> - "pciephy"
> - "apps"
>
> +Additional optional properties for imx7d-pcie:
> +- pcie-phy-refclk-internal: If present then an internal PLL input is used as
> + PCIe PHY reference clock source. By default an external oscillator input
> + is used.
Should this contain the vendor prefix, like fsl,pcie-phy-refclk-internal ?
^ permalink raw reply
* [RESEND PATCH v2 10/15] ASoC: qcom: qdsp6: Add support to q6routing driver
From: Bjorn Andersson @ 2018-01-02 23:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-11-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds support to q6 routing driver which configures route
> between ASM and AFE module using ADM apis.
>
> This driver uses dapm widgets to setup the matrix between AFE ports and
> ASM streams.
>
Why is this a separate driver from the q6adm?
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> sound/soc/qcom/Kconfig | 5 +
> sound/soc/qcom/qdsp6/Makefile | 1 +
> sound/soc/qcom/qdsp6/q6routing.c | 386 +++++++++++++++++++++++++++++++++++++++
> sound/soc/qcom/qdsp6/q6routing.h | 9 +
> 4 files changed, 401 insertions(+)
> create mode 100644 sound/soc/qcom/qdsp6/q6routing.c
> create mode 100644 sound/soc/qcom/qdsp6/q6routing.h
>
> diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig
> index 121b9c957024..dd8fb0cde614 100644
> --- a/sound/soc/qcom/Kconfig
> +++ b/sound/soc/qcom/Kconfig
> @@ -60,12 +60,17 @@ config SND_SOC_QDSP6_CORE
> tristate
> default n
>
> +config SND_SOC_QDSP6_ROUTING
> + tristate
> + default n
> +
> config SND_SOC_QDSP6
> tristate "SoC ALSA audio driver for QDSP6"
> select SND_SOC_QDSP6_AFE
> select SND_SOC_QDSP6_ADM
> select SND_SOC_QDSP6_ASM
> select SND_SOC_QDSP6_CORE
> + select SND_SOC_QDSP6_ROUTING
> help
> To add support for MSM QDSP6 Soc Audio.
> This will enable sound soc platform specific
> diff --git a/sound/soc/qcom/qdsp6/Makefile b/sound/soc/qcom/qdsp6/Makefile
> index ad7f10691e54..c1ad060a2341 100644
> --- a/sound/soc/qcom/qdsp6/Makefile
> +++ b/sound/soc/qcom/qdsp6/Makefile
> @@ -2,3 +2,4 @@ obj-$(CONFIG_SND_SOC_QDSP6_AFE) += q6afe.o
> obj-$(CONFIG_SND_SOC_QDSP6_ADM) += q6adm.o
> obj-$(CONFIG_SND_SOC_QDSP6_ASM) += q6asm.o
> obj-$(CONFIG_SND_SOC_QDSP6_CORE) += q6core.o
> +obj-$(CONFIG_SND_SOC_QDSP6_ROUTING) += q6routing.o
> diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
> new file mode 100644
> index 000000000000..f5f12d61a1ee
> --- /dev/null
> +++ b/sound/soc/qcom/qdsp6/q6routing.c
> @@ -0,0 +1,386 @@
> +/* SPDX-License-Identifier: GPL-2.0
> +* Copyright (c) 2011-2016, The Linux Foundation
> +* Copyright (c) 2017, Linaro Limited
> +*/
> +#include <linux/init.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/bitops.h>
> +#include <linux/mutex.h>
> +#include <linux/of_device.h>
> +#include <linux/slab.h>
> +#include <sound/core.h>
> +#include <sound/soc.h>
> +#include <sound/soc-dapm.h>
> +#include <sound/pcm.h>
> +#include <sound/control.h>
> +#include <sound/asound.h>
> +#include <sound/pcm_params.h>
> +#include "q6afe.h"
> +#include "q6asm.h"
> +#include "q6adm.h"
> +#include "q6routing.h"
> +
> +struct session_data {
> + int state;
> + int port_id;
> + int path_type;
> + int app_type;
> + int acdb_id;
> + int sample_rate;
> + int bits_per_sample;
> + int channels;
> + int format;
> + int perf_mode;
> + int numcopps;
> + int fedai_id;
> + unsigned long copp_map;
> +};
> +
> +struct msm_routing_data {
> + struct session_data sessions[MAX_SESSIONS];
> + struct device *dev;
> + struct mutex lock;
> +};
> +
> +static struct msm_routing_data *routing_data;
> +
> +/**
> + * q6routing_reg_phy_stream() - Register a new stream for route setup
> + *
> + * @fedai_id: Frontend dai id.
> + * @perf_mode: Performace mode.
"Performance"
> + * @stream_id: ASM stream id to map.
> + * @stream_type: Direction of stream
> + *
> + * Return: Will be an negative on error or a zero on success.
> + */
> +int q6routing_reg_phy_stream(int fedai_id, int perf_mode,
q6routing_stream_open() ?
> + int stream_id, int stream_type)
> +{
> + int j, topology, num_copps = 0;
> + struct route_payload payload;
> + int copp_idx;
> + struct session_data *session;
> +
> + if (!routing_data) {
> + pr_err("Routing driver not yet ready\n");
> + return -EINVAL;
> + }
> +
> + session = &routing_data->sessions[stream_id - 1];
> + mutex_lock(&routing_data->lock);
> + session->fedai_id = fedai_id;
> + payload.num_copps = 0; /* only RX needs to use payload */
> + topology = NULL_COPP_TOPOLOGY;
> + copp_idx = q6adm_open(routing_data->dev, session->port_id,
> + session->path_type, session->sample_rate,
> + session->channels, topology, perf_mode,
> + session->bits_per_sample, 0, 0);
> + if ((copp_idx < 0) || (copp_idx >= MAX_COPPS_PER_PORT)) {
Make q6adm_open() not return >= MAX_COPPS_PER_PORT.
And drop the extra parenthesis.
> + mutex_unlock(&routing_data->lock);
> + return -EINVAL;
> + }
> +
> + set_bit(copp_idx, &session->copp_map);
> + for (j = 0; j < MAX_COPPS_PER_PORT; j++) {
Use for_each_set_bit()
> + unsigned long copp = session->copp_map;
> +
> + if (test_bit(j, &copp)) {
> + payload.port_id[num_copps] = session->port_id;
> + payload.copp_idx[num_copps] = j;
> + num_copps++;
> + }
> + }
> +
> + if (num_copps) {
> + payload.num_copps = num_copps;
> + payload.session_id = stream_id;
> + q6adm_matrix_map(routing_data->dev, session->path_type,
> + payload, perf_mode);
> + }
> + mutex_unlock(&routing_data->lock);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(q6routing_reg_phy_stream);
> +
> +static struct session_data *routing_get_session(struct msm_routing_data *data,
> + int port_id, int port_type)
port_type is ignored
> +{
> + int i;
> +
> + for (i = 0; i < MAX_SESSIONS; i++)
> + if (port_id == data->sessions[i].port_id)
> + return &data->sessions[i];
> +
> + return NULL;
> +}
> +
> +static struct session_data *get_session_from_id(struct msm_routing_data *data,
> + int fedai_id)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_SESSIONS; i++) {
> + if (fedai_id == data->sessions[i].fedai_id)
> + return &data->sessions[i];
> + }
> +
> + return NULL;
> +}
> +/**
> + * q6routing_dereg_phy_stream() - Deregister a stream
> + *
> + * @fedai_id: Frontend dai id.
> + * @stream_type: Direction of stream
> + *
> + * Return: Will be an negative on error or a zero on success.
> + */
> +void q6routing_dereg_phy_stream(int fedai_id, int stream_type)
q6routing_stream_close()?
> +{
> + struct session_data *session;
> + int idx;
> +
> + session = get_session_from_id(routing_data, fedai_id);
> + if (!session)
> + return;
> +
> + for_each_set_bit(idx, &session->copp_map, MAX_COPPS_PER_PORT)
> + q6adm_close(routing_data->dev, session->port_id,
> + session->perf_mode, idx);
> +
> + session->fedai_id = -1;
> + session->copp_map = 0;
> +}
> +EXPORT_SYMBOL_GPL(q6routing_dereg_phy_stream);
> +
> +static int msm_routing_get_audio_mixer(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_dapm_context *dapm =
> + snd_soc_dapm_kcontrol_dapm(kcontrol);
> + struct soc_mixer_control *mc =
> + (struct soc_mixer_control *)kcontrol->private_value;
> + int session_id = mc->shift;
> + struct snd_soc_platform *platform = snd_soc_dapm_to_platform(dapm);
> + struct msm_routing_data *priv = snd_soc_platform_get_drvdata(platform);
> + struct session_data *session = &priv->sessions[session_id];
> +
> + if (session->port_id != -1)
> + ucontrol->value.integer.value[0] = 1;
> + else
> + ucontrol->value.integer.value[0] = 0;
> +
> + return 0;
> +}
> +
> +static int msm_routing_put_audio_mixer(struct snd_kcontrol *kcontrol,
> + struct snd_ctl_elem_value *ucontrol)
> +{
> + struct snd_soc_dapm_context *dapm =
> + snd_soc_dapm_kcontrol_dapm(kcontrol);
> + struct snd_soc_platform *platform = snd_soc_dapm_to_platform(dapm);
> + struct msm_routing_data *data = snd_soc_platform_get_drvdata(platform);
> + struct soc_mixer_control *mc =
> + (struct soc_mixer_control *)kcontrol->private_value;
> + struct snd_soc_dapm_update *update = NULL;
> + int be_id = mc->reg;
> + int session_id = mc->shift;
> + struct session_data *session = &data->sessions[session_id];
> +
> + if (ucontrol->value.integer.value[0]) {
> + session->port_id = be_id;
> + snd_soc_dapm_mixer_update_power(dapm, kcontrol, 1, update);
> + } else {
> + session->port_id = -1;
> + snd_soc_dapm_mixer_update_power(dapm, kcontrol, 0, update);
> + }
> +
> + return 1;
> +}
> +
> +static const struct snd_kcontrol_new hdmi_mixer_controls[] = {
> + SOC_SINGLE_EXT("MultiMedia1", AFE_PORT_HDMI_RX,
> + MSM_FRONTEND_DAI_MULTIMEDIA1, 1, 0,
> + msm_routing_get_audio_mixer,
> + msm_routing_put_audio_mixer),
> + SOC_SINGLE_EXT("MultiMedia2", AFE_PORT_HDMI_RX,
> + MSM_FRONTEND_DAI_MULTIMEDIA2, 1, 0,
> + msm_routing_get_audio_mixer,
> + msm_routing_put_audio_mixer),
> + SOC_SINGLE_EXT("MultiMedia3", AFE_PORT_HDMI_RX,
> + MSM_FRONTEND_DAI_MULTIMEDIA3, 1, 0,
> + msm_routing_get_audio_mixer,
> + msm_routing_put_audio_mixer),
> + SOC_SINGLE_EXT("MultiMedia4", AFE_PORT_HDMI_RX,
> + MSM_FRONTEND_DAI_MULTIMEDIA4, 1, 0,
> + msm_routing_get_audio_mixer,
> + msm_routing_put_audio_mixer),
> + SOC_SINGLE_EXT("MultiMedia5", AFE_PORT_HDMI_RX,
> + MSM_FRONTEND_DAI_MULTIMEDIA5, 1, 0,
> + msm_routing_get_audio_mixer,
> + msm_routing_put_audio_mixer),
> + SOC_SINGLE_EXT("MultiMedia6", AFE_PORT_HDMI_RX,
> + MSM_FRONTEND_DAI_MULTIMEDIA6, 1, 0,
> + msm_routing_get_audio_mixer,
> + msm_routing_put_audio_mixer),
> + SOC_SINGLE_EXT("MultiMedia7", AFE_PORT_HDMI_RX,
> + MSM_FRONTEND_DAI_MULTIMEDIA7, 1, 0,
> + msm_routing_get_audio_mixer,
> + msm_routing_put_audio_mixer),
> + SOC_SINGLE_EXT("MultiMedia8", AFE_PORT_HDMI_RX,
> + MSM_FRONTEND_DAI_MULTIMEDIA8, 1, 0,
> + msm_routing_get_audio_mixer,
> + msm_routing_put_audio_mixer),
> +};
> +
> +static const struct snd_soc_dapm_widget msm_qdsp6_widgets[] = {
> + /* Frontend AIF */
> + /* Widget name equals to Front-End DAI name<Need confirmation>,
Perhaps this should be confirmed and the comment updated?
> + * Stream name must contains substring of front-end dai name
> + */
> + SND_SOC_DAPM_AIF_IN("MM_DL1", "MultiMedia1 Playback", 0, 0, 0, 0),
> + SND_SOC_DAPM_AIF_IN("MM_DL2", "MultiMedia2 Playback", 0, 0, 0, 0),
> + SND_SOC_DAPM_AIF_IN("MM_DL3", "MultiMedia3 Playback", 0, 0, 0, 0),
> + SND_SOC_DAPM_AIF_IN("MM_DL4", "MultiMedia4 Playback", 0, 0, 0, 0),
> + SND_SOC_DAPM_AIF_IN("MM_DL5", "MultiMedia5 Playback", 0, 0, 0, 0),
> + SND_SOC_DAPM_AIF_IN("MM_DL6", "MultiMedia6 Playback", 0, 0, 0, 0),
> + SND_SOC_DAPM_AIF_IN("MM_DL7", "MultiMedia7 Playback", 0, 0, 0, 0),
> + SND_SOC_DAPM_AIF_IN("MM_DL8", "MultiMedia8 Playback", 0, 0, 0, 0),
> +
> + /* Mixer definitions */
> + SND_SOC_DAPM_MIXER("HDMI Mixer", SND_SOC_NOPM, 0, 0,
> + hdmi_mixer_controls,
> + ARRAY_SIZE(hdmi_mixer_controls)),
> +};
> +
> +static const struct snd_soc_dapm_route intercon[] = {
> + {"HDMI Mixer", "MultiMedia1", "MM_DL1"},
> + {"HDMI Mixer", "MultiMedia2", "MM_DL2"},
> + {"HDMI Mixer", "MultiMedia3", "MM_DL3"},
> + {"HDMI Mixer", "MultiMedia4", "MM_DL4"},
> + {"HDMI Mixer", "MultiMedia5", "MM_DL5"},
> + {"HDMI Mixer", "MultiMedia6", "MM_DL6"},
> + {"HDMI Mixer", "MultiMedia7", "MM_DL7"},
> + {"HDMI Mixer", "MultiMedia8", "MM_DL8"},
> + {"HDMI", NULL, "HDMI Mixer"},
> + {"HDMI-RX", NULL, "HDMI"},
> +};
> +
> +static int routing_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params)
> +{
> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
> + unsigned int be_id = rtd->cpu_dai->id;
> + struct snd_soc_platform *platform = rtd->platform;
> + struct msm_routing_data *data = snd_soc_platform_get_drvdata(platform);
> + struct session_data *session;
> + int port_id, port_type, path_type, bits_per_sample;
bits_per_sample is likely unused.
> +
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
> + path_type = ADM_PATH_PLAYBACK;
> + port_type = MSM_AFE_PORT_TYPE_RX;
> + }
> +
> + port_id = be_id;
Why alias this variable?
> +
> + session = routing_get_session(data, port_id, port_type);
> +
> + if (!session) {
> + pr_err("No session matrix setup yet..\n");
> + return -EINVAL;
> + }
> +
> + mutex_lock(&data->lock);
> +
> + session->path_type = path_type;
> + session->sample_rate = params_rate(params);
> + session->channels = params_channels(params);
> + session->format = params_format(params);
> +
> + if (session->format == SNDRV_PCM_FORMAT_S16_LE)
> + session->bits_per_sample = 16;
> + else if (session->format == SNDRV_PCM_FORMAT_S24_LE)
> + bits_per_sample = 24;
session-> ?
Perhaps switch on params_format(params) instead? And fail in the default
case.
> +
> + mutex_unlock(&data->lock);
> + return 0;
> +}
> +
> +static int routing_close(struct snd_pcm_substream *substream)
> +{
> + return 0;
> +}
> +
> +static int routing_prepare(struct snd_pcm_substream *substream)
> +{
> + return 0;
> +}
> +
> +static struct snd_pcm_ops q6pcm_routing_ops = {
> + .hw_params = routing_hw_params,
> + .close = routing_close,
> + .prepare = routing_prepare,
> +};
> +
> +/* Not used but frame seems to require it */
Remove comment?
> +static int msm_routing_probe(struct snd_soc_platform *platform)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_SESSIONS; i++)
> + routing_data->sessions[i].port_id = -1;
> +
> + snd_soc_platform_set_drvdata(platform, routing_data);
> +
> + return 0;
> +}
> +
> +static struct snd_soc_platform_driver msm_soc_routing_platform = {
> + .ops = &q6pcm_routing_ops,
> + .probe = msm_routing_probe,
> + .component_driver = {
> + .dapm_widgets = msm_qdsp6_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(msm_qdsp6_widgets),
> + .dapm_routes = intercon,
> + .num_dapm_routes = ARRAY_SIZE(intercon),
> + },
> +};
> +
> +static int q6pcm_routing_probe(struct platform_device *pdev)
> +{
> +
> + routing_data = devm_kzalloc(&pdev->dev,
> + sizeof(*routing_data), GFP_KERNEL);
> + if (!routing_data)
> + return -ENOMEM;
> +
> + routing_data->dev = &pdev->dev;
> +
> + mutex_init(&routing_data->lock);
> + dev_set_drvdata(&pdev->dev, routing_data);
> +
> + return devm_snd_soc_register_platform(&pdev->dev,
> + &msm_soc_routing_platform);
> +}
> +
> +static int q6pcm_routing_remove(struct platform_device *pdev)
> +{
As you return here routing_data will be freed. The early check in
q6routing_reg_phy_stream() seems to indicate that this driver can be
called even though the routing device isn't available.
So you probably want to clear that variable, at least.
> + return 0;
> +}
> +
> +static struct platform_driver q6pcm_routing_driver = {
> + .driver = {
> + .name = "q6routing",
> + .owner = THIS_MODULE,
Drop .owner
> + },
> + .probe = q6pcm_routing_probe,
> + .remove = q6pcm_routing_remove,
> +};
> +
> +module_platform_driver(q6pcm_routing_driver);
> +
> +MODULE_DESCRIPTION("Q6 Routing platform");
> +MODULE_LICENSE("GPL v2");
> diff --git a/sound/soc/qcom/qdsp6/q6routing.h b/sound/soc/qcom/qdsp6/q6routing.h
> new file mode 100644
> index 000000000000..7f0feb196acc
> --- /dev/null
> +++ b/sound/soc/qcom/qdsp6/q6routing.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _Q6_PCM_ROUTING_H
> +#define _Q6_PCM_ROUTING_H
> +
> +int q6routing_reg_phy_stream(int fedai_id, int perf_mode,
> + int stream_id, int stream_type);
> +void q6routing_dereg_phy_stream(int fedai_id, int stream_type);
> +
> +#endif /*_Q6_PCM_ROUTING_H */
> --
> 2.15.0
>
^ permalink raw reply
* [PATCH 01/33] clk_ops: change round_rate() to return unsigned long
From: Stephen Boyd @ 2018-01-02 23:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c2212d56-a8b5-cba5-46a7-c2c7f66e752b@nexus-software.ie>
On 01/02, Bryan O'Donoghue wrote:
> On 02/01/18 19:01, Stephen Boyd wrote:
> >On 12/31, Bryan O'Donoghue wrote:
> >>On 30/12/17 16:36, Mikko Perttunen wrote:
> >>>FWIW, we had this problem some years ago with the Tegra CPU clock
> >>>- then it was determined that a simpler solution was to have the
> >>>determine_rate callback support unsigned long rates - so clock
> >>>drivers that need to return rates higher than 2^31 can instead
> >>>implement the determine_rate callback. That is what's currently
> >>>implemented.
> >>>
> >>>Mikko
> >>
> >>Granted we could work around it but, having both zero and less than
> >>zero indicate error means you can't support larger than LONG_MAX
> >>which is I think worth fixing.
> >>
> >
> >Ok. But can you implement the determine_rate op instead of the
> >round_rate op for your clk?
>
> Don't know .
Please try.
>
> >It's not a work-around, it's the
> >preferred solution. That would allow rates larger than 2^31 for
> >the clk without pushing through a change to all the drivers to
> >express zero as "error" and non-zero as the rounded rate.
> >
> >I'm not entirely opposed to this approach, because we probably
> >don't care to pass the particular error value from a clk provider
> >to a clk consumer about what the error is.
>
> Which was my thought. The return value of clk_ops->round_rate()
> appears not to get pushed up the stack, which is what the last patch
> in this series deals with.
>
> [PATCH 33/33] clk: change handling of round_rate() such that only
> zero is an error
Hmm? clk_core_determine_round_nolock() returns 'rate' if rate < 0
from the round_rate op. clk_core_round_rate_nolock() returns that
value to clk_round_rate() which returns it to the consumer.
>
> >It's actually what we
> >proposed as the solution for clk_round_rate() to return values
> >larger than LONG_MAX to consumers. But doing that consumer API
> >change or this provider side change is going to require us to
> >evaluate all the consumers of these clks to make sure they don't
> >check for some error value that's less than zero. This series
> >does half the work,
>
> Do you mean users of clk_rounda_rate() ? I have a set of patches for
> that but wanted to separate that from clk_ops->round_rate() so as
> not to send ~70 patches out to LKML at once - even if they are in
> two blocks.
Ok. What have you done to the consumers of clk_round_rate()?
Made them treat 0 as an error instead of less than zero? The
documentation in clk.h needs to be updated. See this patch from
Paul Wamsley[1] for one proposed patch that went nowhere. Also
include Russell King please. It was also proposed to change the
function signature of clk_round_rate() to return unsigned long,
but that didn't go anywhere either.
>
> If so, I can publish that set too for reference.
>
> AFAICT on clk_ops->round_rate the last patch #33 ought to cover the
> usage of the return value of clk_ops->round_rate().
>
> Have I missed something ?
Hopefully not!
>
> >by changing the provider side, while ignoring
> >the consumer side and any potential fallout of the less than zero
> >to zero return value change.
> >
>
> Can you look at #33 ? I'm not sure if you saw that one.
>
Yeah I looked at it. From what I can tell it makes
clk_round_rate() return 0 now instead of whatever negative value
the clk_ops::round_rate function returns.
[1] https://lkml.kernel.org/r/alpine.DEB.2.02.1311251603310.23090 at tamien
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [RESEND PATCH v2 11/15] ASoC: qcom: qdsp6: Add support to q6afe dai driver
From: Bjorn Andersson @ 2018-01-02 23:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-12-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds support to q6afe backend dais driver.
>
Isn't the list of backend DAIs platform-dependent?
[..]
> +static const struct snd_soc_dapm_widget hdmi_dapm_widgets[] = {
> + SND_SOC_DAPM_AIF_OUT("HDMI", "HDMI Playback", 0, 0, 0, 0),
> + SND_SOC_DAPM_OUTPUT("HDMI-RX"),
> +};
> +
> +static const struct snd_soc_component_driver msm_dai_hdmi_q6_component = {
How will this look beyond HDMI? I'm having issues mapping this to
downstream.
> + .name = "msm-dai-q6-hdmi",
> + .dapm_widgets = hdmi_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(hdmi_dapm_widgets),
> + .controls = hdmi_config_controls,
> + .num_controls = ARRAY_SIZE(hdmi_config_controls),
> + .dapm_routes = hdmi_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(hdmi_dapm_routes),
> +};
> +
Regards,
Bjorn
^ permalink raw reply
* [PATCH 1/8] ARM: dts: keystone: Move keystone_irq to under device-state-control
From: Suman Anna @ 2018-01-02 23:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102170202.15045-2-afd@ti.com>
Hi Andrew,
On 01/02/2018 11:01 AM, Andrew F. Davis wrote:
> The keystone_irq node describes a device that is a component of the device
> state control module.
I would prefer 'address space' to be added to this statement as this
module is really not a single IP but really a collection of different
register sets providing different functionalities. Some of these
comments apply to the following patches as well.
As such, it should not be a member of soc0 bus
> but instead a sub-node of device-state-control.
>
> This move also fixes a warning about not having a reg property. Now
> that this is a sub-node of device-state-control, a syscon type node,
> we add this reg property but relative to the syscon base, this way
> when the dt-binding/driver are updated we can drop the non-standard
> ti,syscon-dev property completely and simply use get_resource() in
> the driver.
Please add an appropriate 'ranges' property in the parent node following
the parent-child node convention, it's upto individual drivers to use
the appropriate API for whether they want to deal with the offset or the
actual bus addresses. You should not tie this into forcing to use a
get_resource() without ranges to get the offset.
>
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Acked-by: Nishanth Menon <nm@ti.com>
> ---
> arch/arm/boot/dts/keystone.dtsi | 21 ++++++++++++---------
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/boot/dts/keystone.dtsi b/arch/arm/boot/dts/keystone.dtsi
> index 93ea5c69ea77..158e0a903f7e 100644
> --- a/arch/arm/boot/dts/keystone.dtsi
> +++ b/arch/arm/boot/dts/keystone.dtsi
> @@ -87,8 +87,19 @@
> };
>
> devctrl: device-state-control at 2620000 {
> - compatible = "ti,keystone-devctrl", "syscon";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "ti,keystone-devctrl", "syscon", "simple-mfd";
nit, can you please maintain the current order of compatible and reg,
and add the new properties after them.
regards
Suman
> reg = <0x02620000 0x1000>;
> +
> + kirq0: keystone_irq at 2a0 {
> + compatible = "ti,keystone-irq";
> + reg = <0x2a0 0x4>;
> + interrupts = <GIC_SPI 4 IRQ_TYPE_EDGE_RISING>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + ti,syscon-dev = <&devctrl 0x2a0>;
> + };
> };
>
> rstctrl: reset-controller {
> @@ -282,14 +293,6 @@
> 1 0 0x21000A00 0x00000100>;
> };
>
> - kirq0: keystone_irq at 26202a0 {
> - compatible = "ti,keystone-irq";
> - interrupts = <GIC_SPI 4 IRQ_TYPE_EDGE_RISING>;
> - interrupt-controller;
> - #interrupt-cells = <1>;
> - ti,syscon-dev = <&devctrl 0x2a0>;
> - };
> -
> pcie0: pcie at 21800000 {
> compatible = "ti,keystone-pcie", "snps,dw-pcie";
> clocks = <&clkpcie>;
>
^ permalink raw reply
* [PATCH v7 02/10] module: allow symbol exports to be disabled
From: Nicolas Pitre @ 2018-01-02 23:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180102200549.22984-3-ard.biesheuvel@linaro.org>
On Tue, 2 Jan 2018, Ard Biesheuvel wrote:
> To allow existing C code to be incorporated into the decompressor or
> the UEFI stub, introduce a CPP macro that turns all EXPORT_SYMBOL_xxx
> declarations into nops, and #define it in places where such exports
> are undesirable. Note that this gets rid of a rather dodgy redefine
> of linux/export.h's header guard.
[...]
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -83,6 +83,15 @@ extern struct module __this_module;
> */
> #define __EXPORT_SYMBOL(sym, sec) === __KSYM_##sym ===
>
> +#elif defined(__DISABLE_EXPORTS)
> +
> +/*
> + * Allow symbol exports to be disabled completely so that C code may
> + * be reused in other execution contexts such as the UEFI stub or the
> + * decompressor.
> + */
> +#define __EXPORT_SYMBOL(sym, sec)
> +
I think you should rather put this first thing in the #if sequence so to
override the defined(__KSYM_DEPS__) case too. No need to create build
dependencies for module symbols that you're going to stub out
afterwards anyway.
Nicolas
^ permalink raw reply
* [PATCH v7 02/10] module: allow symbol exports to be disabled
From: Ard Biesheuvel @ 2018-01-02 23:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <nycvar.YSQ.7.76.1801021841410.8567@knanqh.ubzr>
On 2 January 2018 at 23:47, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Tue, 2 Jan 2018, Ard Biesheuvel wrote:
>
>> To allow existing C code to be incorporated into the decompressor or
>> the UEFI stub, introduce a CPP macro that turns all EXPORT_SYMBOL_xxx
>> declarations into nops, and #define it in places where such exports
>> are undesirable. Note that this gets rid of a rather dodgy redefine
>> of linux/export.h's header guard.
> [...]
>
>> --- a/include/linux/export.h
>> +++ b/include/linux/export.h
>> @@ -83,6 +83,15 @@ extern struct module __this_module;
>> */
>> #define __EXPORT_SYMBOL(sym, sec) === __KSYM_##sym ===
>>
>> +#elif defined(__DISABLE_EXPORTS)
>> +
>> +/*
>> + * Allow symbol exports to be disabled completely so that C code may
>> + * be reused in other execution contexts such as the UEFI stub or the
>> + * decompressor.
>> + */
>> +#define __EXPORT_SYMBOL(sym, sec)
>> +
>
> I think you should rather put this first thing in the #if sequence so to
> override the defined(__KSYM_DEPS__) case too. No need to create build
> dependencies for module symbols that you're going to stub out
> afterwards anyway.
>
I wasn't sure, so thanks for clearing that up.
^ permalink raw reply
* [RESEND PATCH v2 12/15] ASoC: qcom: qdsp6: Add support to q6asm dai driver
From: Bjorn Andersson @ 2018-01-03 0:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-13-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
[..]
> +
> +enum stream_state {
> + IDLE = 0,
> + STOPPED,
> + RUNNING,
These are too generic.
> +};
> +
> +struct q6asm_dai_rtd {
> + struct snd_pcm_substream *substream;
> + dma_addr_t phys;
> + unsigned int pcm_size;
> + unsigned int pcm_count;
> + unsigned int pcm_irq_pos; /* IRQ position */
> + unsigned int periods;
> + uint16_t bits_per_sample;
> + uint16_t source; /* Encoding source bit mask */
> +
> + struct audio_client *audio_client;
> + uint16_t session_id;
> +
> + enum stream_state state;
> + bool set_channel_map;
> + char channel_map[8];
There's a define for this 8
> +};
> +
> +struct q6asm_dai_data {
> + u64 sid;
> +};
> +
> +static struct snd_pcm_hardware q6asm_dai_hardware_playback = {
> + .info = (SNDRV_PCM_INFO_MMAP |
> + SNDRV_PCM_INFO_BLOCK_TRANSFER |
> + SNDRV_PCM_INFO_MMAP_VALID |
> + SNDRV_PCM_INFO_INTERLEAVED |
> + SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME),
> + .formats = (SNDRV_PCM_FMTBIT_S16_LE |
> + SNDRV_PCM_FMTBIT_S24_LE),
> + .rates = SNDRV_PCM_RATE_8000_192000,
> + .rate_min = 8000,
> + .rate_max = 192000,
> + .channels_min = 1,
> + .channels_max = 8,
> + .buffer_bytes_max = (PLAYBACK_MAX_NUM_PERIODS *
> + PLAYBACK_MAX_PERIOD_SIZE),
> + .period_bytes_min = PLAYBACK_MIN_PERIOD_SIZE,
> + .period_bytes_max = PLAYBACK_MAX_PERIOD_SIZE,
> + .periods_min = PLAYBACK_MIN_NUM_PERIODS,
> + .periods_max = PLAYBACK_MAX_NUM_PERIODS,
If you just put the numbers here, instead of using the PLAYBACK_
defines, it's possible to grok the values of this struct without having
to jump to the defines for each one.
> + .fifo_size = 0,
> +};
> +
> +/* Conventional and unconventional sample rate supported */
> +static unsigned int supported_sample_rates[] = {
> + 8000, 11025, 12000, 16000, 22050, 24000, 32000, 44100, 48000,
> + 88200, 96000, 176400, 192000
> +};
> +
> +static struct snd_pcm_hw_constraint_list constraints_sample_rates = {
This is unreferenced.
> + .count = ARRAY_SIZE(supported_sample_rates),
> + .list = supported_sample_rates,
> + .mask = 0,
> +};
> +
> +static void event_handler(uint32_t opcode, uint32_t token,
> + uint32_t *payload, void *priv)
> +{
> + struct q6asm_dai_rtd *prtd = priv;
> + struct snd_pcm_substream *substream = prtd->substream;
> +
> + switch (opcode) {
> + case ASM_CLIENT_EVENT_CMD_RUN_DONE:
> + q6asm_write_nolock(prtd->audio_client,
> + prtd->pcm_count, 0, 0, NO_TIMESTAMP);
> + break;
> + case ASM_CLIENT_EVENT_CMD_EOS_DONE:
> + prtd->state = STOPPED;
> + break;
> + case ASM_CLIENT_EVENT_DATA_WRITE_DONE: {
> + prtd->pcm_irq_pos += prtd->pcm_count;
> + snd_pcm_period_elapsed(substream);
> + if (prtd->state == RUNNING)
> + q6asm_write_nolock(prtd->audio_client,
> + prtd->pcm_count, 0, 0, NO_TIMESTAMP);
> +
> + break;
> + }
> + default:
> + break;
> + }
> +}
> +
> +static int q6asm_dai_prepare(struct snd_pcm_substream *substream)
> +{
> + struct snd_pcm_runtime *runtime = substream->runtime;
> + struct snd_soc_pcm_runtime *soc_prtd = substream->private_data;
> + struct q6asm_dai_rtd *prtd = runtime->private_data;
> + struct q6asm_dai_data *pdata;
> + int ret;
> +
> + pdata = dev_get_drvdata(soc_prtd->platform->dev);
> + if (!pdata)
> + return -EINVAL;
> +
> + if (!prtd || !prtd->audio_client) {
> + pr_err("%s: private data null or audio client freed\n",
> + __func__);
> + return -EINVAL;
> + }
> +
> + prtd->pcm_count = snd_pcm_lib_period_bytes(substream);
> + prtd->pcm_irq_pos = 0;
> + /* rate and channels are sent to audio driver */
> + if (prtd->state) {
> + /* clear the previous setup if any */
> + q6asm_cmd(prtd->audio_client, CMD_CLOSE);
> + q6asm_unmap_memory_regions(substream->stream,
> + prtd->audio_client);
> + q6routing_dereg_phy_stream(soc_prtd->dai_link->id,
> + SNDRV_PCM_STREAM_PLAYBACK);
> + }
> +
> + ret = q6asm_map_memory_regions(substream->stream, prtd->audio_client,
> + prtd->phys,
> + (prtd->pcm_size / prtd->periods),
> + prtd->periods);
> +
> + if (ret < 0) {
> + pr_err("Audio Start: Buffer Allocation failed rc = %d\n",
> + ret);
> + return -ENOMEM;
> + }
> +
> + ret = q6asm_open_write(prtd->audio_client, FORMAT_LINEAR_PCM,
> + prtd->bits_per_sample);
> + if (ret < 0) {
> + pr_err("%s: q6asm_open_write failed\n", __func__);
> + q6asm_audio_client_free(prtd->audio_client);
> + prtd->audio_client = NULL;
Do you need to roll back the q6asm_map_memory_regions?
> + return -ENOMEM;
> + }
> +
> + prtd->session_id = q6asm_get_session_id(prtd->audio_client);
> + ret = q6routing_reg_phy_stream(soc_prtd->dai_link->id, LEGACY_PCM_MODE,
> + prtd->session_id, substream->stream);
> + if (ret) {
> + pr_err("%s: stream reg failed ret:%d\n", __func__, ret);
> + return ret;
> + }
> +
> + ret = q6asm_media_format_block_multi_ch_pcm(
> + prtd->audio_client, runtime->rate,
> + runtime->channels, !prtd->set_channel_map,
> + prtd->channel_map, prtd->bits_per_sample);
set_channel_map and channel_map aren't referenced elsewhere. If this
isn't used consider removing it for now.
> + if (ret < 0)
> + pr_info("%s: CMD Format block failed\n", __func__);
> +
> + prtd->state = RUNNING;
> +
> + return 0;
> +}
> +
[..]
> +static int q6asm_dai_pcm_new(struct snd_soc_pcm_runtime *rtd)
> +{
> + struct snd_pcm *pcm = rtd->pcm;
> + struct snd_pcm_substream *substream;
> + struct snd_card *card = rtd->card->snd_card;
> + struct device *dev = card->dev;
> + struct device_node *node = dev->of_node;
> + struct q6asm_dai_data *pdata = dev_get_drvdata(rtd->platform->dev);
> + struct of_phandle_args args;
> +
> + int size, ret = 0;
> +
> + ret = of_parse_phandle_with_fixed_args(node, "iommus", 1, 0, &args);
> + if (ret < 0)
> + pdata->sid = -1;
> + else
> + pdata->sid = args.args[0];
> +
Is this really how you're supposed to deal with the iommu?
> +
> +
> + substream = pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream;
> + size = q6asm_dai_hardware_playback.buffer_bytes_max;
> + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV, dev, size,
> + &substream->dma_buffer);
> + if (ret) {
> + dev_err(dev, "Cannot allocate buffer(s)\n");
> + return ret;
Just fall through.
> + }
> +
> + return ret;
> +}
> +
[..]
> +static struct snd_soc_dai_driver q6asm_fe_dais[] = {
> + {
> + .playback = {
> + .stream_name = "MultiMedia1 Playback",
> + .rates = (SNDRV_PCM_RATE_8000_192000|
> + SNDRV_PCM_RATE_KNOT),
> + .formats = (SNDRV_PCM_FMTBIT_S16_LE |
> + SNDRV_PCM_FMTBIT_S24_LE),
> + .channels_min = 1,
> + .channels_max = 8,
> + .rate_min = 8000,
> + .rate_max = 192000,
> + },
> + .name = "MM_DL1",
> + .probe = fe_dai_probe,
> + .id = MSM_FRONTEND_DAI_MULTIMEDIA1,
> + },
> + {
> + .playback = {
> + .stream_name = "MultiMedia2 Playback",
> + .rates = (SNDRV_PCM_RATE_8000_192000|
> + SNDRV_PCM_RATE_KNOT),
> + .formats = (SNDRV_PCM_FMTBIT_S16_LE |
> + SNDRV_PCM_FMTBIT_S24_LE),
> + .channels_min = 1,
> + .channels_max = 8,
> + .rate_min = 8000,
> + .rate_max = 192000,
I presume the listed frontend DAIs needs to match the firmware of the
DSP (and features of hardware)? Can we get away with a single list for
all versions of the adsp?
In msm-4.4 the max rate for these where changed to 384000, see:
9c46f74b2724 ("ASoC: msm: add 384KHz playback support")
> + },
> + .name = "MM_DL2",
> + .probe = fe_dai_probe,
> + .id = MSM_FRONTEND_DAI_MULTIMEDIA2,
> + },
> +};
> +
Regards,
Bjorn
^ permalink raw reply
* [RESEND PATCH v2 14/15] ASoC: qcom: apq8096: Add db820c machine driver
From: Bjorn Andersson @ 2018-01-03 0:16 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-15-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:34 PST 2017, srinivas.kandagatla at linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> uThis patch adds support to DB820c machine driver.
Drop 'u' and expand the message to claim that this is the machine driver
for 8996, used by the db820c.
[..]
> +static struct snd_soc_dai_link msm8996_dai_links[] = {
Are there any differences between the DAI links of apq8096 and msm8996?
> + /* FrontEnd DAI Links */
> + {
> + .name = "MultiMedia1 Playback",
> + .stream_name = "MultiMedia1",
> + .cpu_dai_name = "MM_DL1",
> + .platform_name = "q6asm_dai",
> + .dynamic = 1,
> + .dpcm_playback = 1,
> +
> + .codec_dai_name = "snd-soc-dummy-dai",
> + .codec_name = "snd-soc-dummy",
> + },
> + /* Backend DAI Links */
> + {
> + .name = "HDMI Playback",
> + .stream_name = "q6afe_dai",
> + .cpu_dai_name = "HDMI",
> + .platform_name = "q6routing",
> + .no_pcm = 1,
> + .dpcm_playback = 1,
> + .be_hw_params_fixup = msm8996_be_hw_params_fixup,
> + .codec_dai_name = "i2s-hifi",
> + .codec_name = "hdmi-audio-codec.0.auto",
> + },
> +};
> +
> +static int apq8096_sbc_parse_of(struct snd_soc_card *card)
msm8996_parse_of()
> +{
> + struct device *dev = card->dev;
> + int ret;
> +
> + ret = snd_soc_of_parse_card_name(card, "qcom,model");
> + if (ret)
> + dev_err(dev, "Error parsing card name: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static int msm_snd_apq8096_probe(struct platform_device *pdev)
msm_snd_msm8996_probe()?
> +{
> + int ret;
> + struct snd_soc_card *card;
> +
> + card = devm_kzalloc(&pdev->dev, sizeof(*card), GFP_KERNEL);
> + if (!card)
> + return -ENOMEM;
> +
> + card->dev = &pdev->dev;
> +
> + ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> +
> + card->dai_link = msm8996_dai_links;
> + card->num_links = ARRAY_SIZE(msm8996_dai_links);
> +
> + ret = apq8096_sbc_parse_of(card);
> + if (ret) {
> + dev_err(&pdev->dev, "Error parsing OF data\n");
No need to print in both parse_of() and here.
> + return ret;
> + }
> +
> + ret = devm_snd_soc_register_card(&pdev->dev, card);
> + if (ret)
> + dev_err(&pdev->dev, "sound card register failed (%d)!\n", ret);
> + else
> + dev_err(&pdev->dev, "sound card register Sucessfull\n");
This isn't an error, skip the print here.
> +
> + return ret;
> +}
> +
> +static const struct of_device_id msm_snd_apq8096_dt_match[] = {
> + {.compatible = "qcom,apq8096-sndcard"},
> + {}
> +};
> +
> +static struct platform_driver msm_snd_apq8096_driver = {
> + .probe = msm_snd_apq8096_probe,
> + .driver = {
> + .name = "msm-snd-apq8096",
> + .owner = THIS_MODULE,
Drop the .owner
Regards,
Bjorn
^ permalink raw reply
* [RESEND PATCH v2 15/15] arm64: dts: msm8996: db820c: Add sound card support
From: Bjorn Andersson @ 2018-01-03 0:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-16-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:34 PST 2017, srinivas.kandagatla at linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch adds hdmi sound card support to db820c via qdsp.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 5 +++++
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 33 ++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> index 9769053957af..b955769b100d 100644
> --- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> +++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi
> @@ -190,6 +190,11 @@
> };
> };
>
> + snd {
> + compatible = "qcom,apq8096-sndcard";
> + qcom,model = "DB820c";
> + iommus = <&lpass_q6_smmu 1>;
> + };
>
> gpio_keys {
> compatible = "gpio-keys";
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index a144cec7bb71..25c43fb8ab49 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -1262,6 +1262,7 @@
>
> phys = <&hdmi_phy>;
> phy-names = "hdmi_phy";
> + #sound-dai-cells = <0>;
>
> ports {
> #address-cells = <1>;
> @@ -1297,6 +1298,33 @@
> "ref_clk";
> };
> };
> +
> + lpass_q6_smmu: arm,smmu-lpass_q6 at 1600000 {
name this node "iommu"
> + compatible = "qcom,msm8996-smmu-v2";
> + reg = <0x1600000 0x20000>;
> + #iommu-cells = <1>;
> + power-domains = <&gcc HLOS1_VOTE_LPASS_CORE_GDSC>;
Indentation
> +
> + #global-interrupts = <1>;
> + interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 226 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 393 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 394 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 395 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 396 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 397 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 398 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 399 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 400 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 401 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 402 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 403 IRQ_TYPE_LEVEL_HIGH>;
> +
> + clocks = <&gcc GCC_HLOS1_VOTE_LPASS_CORE_SMMU_CLK>,
> + <&gcc GCC_HLOS1_VOTE_LPASS_ADSP_SMMU_CLK>;
> + clock-names = "iface", "bus";
> + status = "okay";
> + };
> };
>
> adsp-pil {
> @@ -1325,6 +1353,11 @@
> qcom,ipc = <&apcs 16 8>;
> qcom,smd-edge = <1>;
> qcom,remote-pid = <2>;
> +
> + apr {
"apr-audio-svc", as this is not the only apr channel on this edge.
> + compatible = "qcom,apr-msm8996";
> + qcom,smd-channels = "apr_audio_svc";
> + };
> };
Regards,
Bjorn
^ permalink raw reply
* [RESEND PATCH v2 13/15] dt-bindings: sound: qcom: Add devicetree bindings for apq8096
From: Bjorn Andersson @ 2018-01-03 0:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-14-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:34 PST 2017, srinivas.kandagatla at linaro.org wrote:
> +++ b/Documentation/devicetree/bindings/sound/qcom,apq8096.txt
Wouldn't it be possible to describe all(?) qdsp based machines in this
one document? I.e. should we name it a little bit more generic?
> @@ -0,0 +1,22 @@
> +* Qualcomm Technologies APQ8096 ASoC sound card driver
> +
> +This binding describes the APQ8096 sound card, which uses qdsp for audio.
> +
> +- compatible:
> + Usage: required
> + Value type: <stringlist>
> + Definition: must be "qcom,apq8096-sndcard"
> +
> +- qcom,audio-routing:
> + Usage: Optional
> + Value type: <stringlist>
> + Definition: A list of the connections between audio components.
Double space before A
> + Each entry is a pair of strings, the first being the
> + connection's sink, the second being the connection's
> + source. Valid names could be power supplies, MicBias
> + of codec and the jacks on the board:
> +Example:
> + sound {
> + compatible = "qcom,snd-apq8096";
Indentation
> + qcom,model = "DB820c";
> + };
Regards,
Bjorn
^ permalink raw reply
* [RESEND PATCH v2 01/15] dt-bindings: soc: qcom: Add bindings for APR bus
From: Bjorn Andersson @ 2018-01-03 0:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20171214173402.19074-2-srinivas.kandagatla@linaro.org>
On Thu 14 Dec 09:33 PST 2017, srinivas.kandagatla at linaro.org wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> This patch add dt bindings for Qualcomm APR bus driver
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
> .../devicetree/bindings/soc/qcom/qcom,apr.txt | 28 ++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> new file mode 100644
> index 000000000000..4e93213ae98d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
> @@ -0,0 +1,28 @@
> +Qualcomm APR (Asynchronous Packet Router) binding
> +
> +This binding describes the Qualcomm APR. APR is a IPC protocol for
> +communication between Application processor and QDSP. APR is mainly
> +used for audio/voice services on the QDSP.
> +
> +- compatible:
> + Usage: required
> + Value type: <stringlist>
> + Definition: must be "qcom,apr-<SOC-NAME>" example: "qcom,apr-msm8996"
This is not the only apr on msm8996 and some platform seems to have 3-4
aprs. I'm therefor hesitant to use the compatible to pick the static
list of services available on the particular firmware.
If this scheme is followed we at least would need to rename this
qcom,msm8996-apr-audio-svc
But I think it would be preferable to go with qcom,apr-v2 and then list
the static services as child nodes.
> +
> +
> +- qcom,smd-channel:
> + Usage: required
> + Value type: <string>
> + Definition: standard SMD property specifying the SMD channel used for
> + communication with the APR on QDSP.
> + Should be "apr_audio_svc".
This is not the only APR channel, but for apr itself this doesn't matter
and might as well be qcom,glink-channels; so perhaps we can omit this
from this document?
> + Described in soc/qcom/qcom,smd.txt
> +
> += EXAMPLE
> +The following example represents a QDSP based sound card on a MSM8996 device
> +which uses apr as communication between Apps and QDSP.
> +
> + apr {
> + compatible = "qcom,apr-msm8996";
> + qcom,smd-channels = "apr_audio_svc";
> + };
Regards,
Bjorn
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox