Re: [PATCH] powerpc/opal_elog: Handle multiple writes to ack attribute
On 2020-10-14 12:18:13 Wed, Aneesh Kumar K.V wrote:
> Even though we use self removing sysfs helper, we still need
> to make sure we do the final kobject delete conditionally.
> sysfs_remove_file_self() will handle parallel calls to remove
> the sysfs attribute file and returns true only in the caller
> that removed the attribute file. The other parallel callers
> are returned false. Do the final kobject delete checking
> the return value of sysfs_remove_file_self().
>
> Cc: Mahesh Salgaonkar
> Cc: Oliver O'Halloran
> Signed-off-by: Aneesh Kumar K.V
> ---
> arch/powerpc/platforms/powernv/opal-elog.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-elog.c
> b/arch/powerpc/platforms/powernv/opal-elog.c
> index 5e33b1fc67c2..37b380eef41a 100644
> --- a/arch/powerpc/platforms/powernv/opal-elog.c
> +++ b/arch/powerpc/platforms/powernv/opal-elog.c
> @@ -72,9 +72,14 @@ static ssize_t elog_ack_store(struct elog_obj *elog_obj,
> const char *buf,
> size_t count)
> {
> - opal_send_ack_elog(elog_obj->id);
> - sysfs_remove_file_self(&elog_obj->kobj, &attr->attr);
> - kobject_put(&elog_obj->kobj);
> + /*
> + * Try to self remove this attribute. If we are successful,
> + * delete the kobject itself.
> + */
> + if (sysfs_remove_file_self(&elog_obj->kobj, &attr->attr)) {
> + opal_send_ack_elog(elog_obj->id);
> + kobject_put(&elog_obj->kobj);
> + }
> return count;
> }
Looks good.
Reviewed-by: Mahesh Salgaonkar
Thanks,
-Mahesh.
[PATCH v4 1/2] powerpc/64: Set up a kernel stack for secondaries before cpu_restore()
Currently in generic_secondary_smp_init(), cur_cpu_spec->cpu_restore()
is called before a stack has been set up in r1. This was previously fine
as the cpu_restore() functions were implemented in assembly and did not
use a stack. However commit 5a61ef74f269 ("powerpc/64s: Support new
device tree binding for discovering CPU features") used
__restore_cpu_cpufeatures() as the cpu_restore() function for a
device-tree features based cputable entry. This is a C function and
hence uses a stack in r1.
generic_secondary_smp_init() is entered on the secondary cpus via the
primary cpu using the OPAL call opal_start_cpu(). In OPAL, each hardware
thread has its own stack. The OPAL call is ran in the primary's hardware
thread. During the call, a job is scheduled on a secondary cpu that will
start executing at the address of generic_secondary_smp_init(). Hence
the value that will be left in r1 when the secondary cpu enters the
kernel is part of that secondary cpu's individual OPAL stack. This means
that __restore_cpu_cpufeatures() will write to that OPAL stack. This is
not horribly bad as each hardware thread has its own stack and the call
that enters the kernel from OPAL never returns, but it is still wrong
and should be corrected.
Create the temp kernel stack before calling cpu_restore().
As noted by mpe, for a kexec boot, the secondary CPUs are released from
the spin loop at address 0x60 by smp_release_cpus() and then jump to
generic_secondary_smp_init(). The call to smp_release_cpus() is in
setup_arch(), and it comes before the call to emergency_stack_init().
emergency_stack_init() allocates an emergency stack in the PACA for each
CPU. This address in the PACA is what is used to set up the temp kernel
stack in generic_secondary_smp_init(). Move releasing the secondary CPUs
to after the PACAs have been allocated an emergency stack, otherwise the
PACA stack pointer will contain garbage and hence the temp kernel stack
created from it will be broken.
Fixes: 5a61ef74f269 ("powerpc/64s: Support new device tree binding for
discovering CPU features")
Signed-off-by: Jordan Niethe
---
v2: Add more detail to the commit message
v3: Release secondary CPUs after the emergency stack is created
v4: No need to guard smp_release_cpus() with #ifdef CONFIG_SMP
---
arch/powerpc/kernel/head_64.S | 8
arch/powerpc/kernel/setup-common.c | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 0e05a9a47a4b..4b7f4c6c2600 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -420,6 +420,10 @@ generic_secondary_common_init:
/* From now on, r24 is expected to be logical cpuid */
mr r24,r5
+ /* Create a temp kernel stack for use before relocation is on. */
+ ld r1,PACAEMERGSP(r13)
+ subir1,r1,STACK_FRAME_OVERHEAD
+
/* See if we need to call a cpu state restore handler */
LOAD_REG_ADDR(r23, cur_cpu_spec)
ld r23,0(r23)
@@ -448,10 +452,6 @@ generic_secondary_common_init:
sync/* order paca.run and cur_cpu_spec */
isync /* In case code patching happened */
- /* Create a temp kernel stack for use before relocation is on. */
- ld r1,PACAEMERGSP(r13)
- subir1,r1,STACK_FRAME_OVERHEAD
-
b __secondary_start
#endif /* SMP */
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index 808ec9fab605..da8c71f321ad 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -919,8 +919,6 @@ void __init setup_arch(char **cmdline_p)
/* On BookE, setup per-core TLB data structures. */
setup_tlb_core_data();
-
- smp_release_cpus();
#endif
/* Print various info about the machine that has been gathered so far.
*/
@@ -944,6 +942,8 @@ void __init setup_arch(char **cmdline_p)
exc_lvl_early_init();
emergency_stack_init();
+ smp_release_cpus();
+
initmem_init();
early_memtest(min_low_pfn << PAGE_SHIFT, max_low_pfn << PAGE_SHIFT);
--
2.17.1
[PATCH v4 2/2] powerpc/64s: Convert some cpu_setup() and cpu_restore() functions to C
The only thing keeping the cpu_setup() and cpu_restore() functions used
in the cputable entries for Power7, Power8, Power9 and Power10 in
assembly was cpu_restore() being called before there was a stack in
generic_secondary_smp_init(). Commit ("powerpc/64: Set up a kernel stack
for secondaries before cpu_restore()") means that it is now possible to
use C.
Rewrite the functions in C so they are a little bit easier to read. This
is not changing their functionality.
Signed-off-by: Jordan Niethe
---
arch/powerpc/include/asm/cpu_setup_power.h | 12 +
arch/powerpc/kernel/cpu_setup_power.S | 252 ---
arch/powerpc/kernel/cpu_setup_power.c | 269 +
arch/powerpc/kernel/cputable.c | 12 +-
4 files changed, 285 insertions(+), 260 deletions(-)
create mode 100644 arch/powerpc/include/asm/cpu_setup_power.h
delete mode 100644 arch/powerpc/kernel/cpu_setup_power.S
create mode 100644 arch/powerpc/kernel/cpu_setup_power.c
diff --git a/arch/powerpc/include/asm/cpu_setup_power.h
b/arch/powerpc/include/asm/cpu_setup_power.h
new file mode 100644
index ..24be9131f803
--- /dev/null
+++ b/arch/powerpc/include/asm/cpu_setup_power.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2020 IBM Corporation
+ */
+void __setup_cpu_power7(unsigned long offset, struct cpu_spec *spec);
+void __restore_cpu_power7(void);
+void __setup_cpu_power8(unsigned long offset, struct cpu_spec *spec);
+void __restore_cpu_power8(void);
+void __setup_cpu_power9(unsigned long offset, struct cpu_spec *spec);
+void __restore_cpu_power9(void);
+void __setup_cpu_power10(unsigned long offset, struct cpu_spec *spec);
+void __restore_cpu_power10(void);
diff --git a/arch/powerpc/kernel/cpu_setup_power.S
b/arch/powerpc/kernel/cpu_setup_power.S
deleted file mode 100644
index 704e8b9501ee..
--- a/arch/powerpc/kernel/cpu_setup_power.S
+++ /dev/null
@@ -1,252 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * This file contains low level CPU setup functions.
- *Copyright (C) 2003 Benjamin Herrenschmidt (b...@kernel.crashing.org)
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-/* Entry: r3 = crap, r4 = ptr to cputable entry
- *
- * Note that we can be called twice for pseudo-PVRs
- */
-_GLOBAL(__setup_cpu_power7)
- mflrr11
- bl __init_hvmode_206
- mtlrr11
- beqlr
- li r0,0
- mtspr SPRN_LPID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- li r4,(LPCR_LPES1 >> LPCR_LPES_SH)
- bl __init_LPCR_ISA206
- mtlrr11
- blr
-
-_GLOBAL(__restore_cpu_power7)
- mflrr11
- mfmsr r3
- rldicl. r0,r3,4,63
- beqlr
- li r0,0
- mtspr SPRN_LPID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- li r4,(LPCR_LPES1 >> LPCR_LPES_SH)
- bl __init_LPCR_ISA206
- mtlrr11
- blr
-
-_GLOBAL(__setup_cpu_power8)
- mflrr11
- bl __init_FSCR
- bl __init_PMU
- bl __init_PMU_ISA207
- bl __init_hvmode_206
- mtlrr11
- beqlr
- li r0,0
- mtspr SPRN_LPID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- ori r3, r3, LPCR_PECEDH
- li r4,0 /* LPES = 0 */
- bl __init_LPCR_ISA206
- bl __init_HFSCR
- bl __init_PMU_HV
- bl __init_PMU_HV_ISA207
- mtlrr11
- blr
-
-_GLOBAL(__restore_cpu_power8)
- mflrr11
- bl __init_FSCR
- bl __init_PMU
- bl __init_PMU_ISA207
- mfmsr r3
- rldicl. r0,r3,4,63
- mtlrr11
- beqlr
- li r0,0
- mtspr SPRN_LPID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- ori r3, r3, LPCR_PECEDH
- li r4,0 /* LPES = 0 */
- bl __init_LPCR_ISA206
- bl __init_HFSCR
- bl __init_PMU_HV
- bl __init_PMU_HV_ISA207
- mtlrr11
- blr
-
-_GLOBAL(__setup_cpu_power10)
- mflrr11
- bl __init_FSCR_power10
- bl __init_PMU
- bl __init_PMU_ISA31
- b 1f
-
-_GLOBAL(__setup_cpu_power9)
- mflrr11
- bl __init_FSCR_power9
- bl __init_PMU
-1: bl __init_hvmode_206
- mtlrr11
- beqlr
- li r0,0
- mtspr SPRN_PSSCR,r0
- mtspr SPRN_LPID,r0
- mtspr SPRN_PID,r0
- LOAD_REG_IMMEDIATE(r0, PCR_MASK)
- mtspr SPRN_PCR,r0
- mfspr r3,SPRN_LPCR
- LOAD_REG_IMMEDIATE(r4, LPCR_PECEDH | LPCR_PECE_HVEE | LPCR_HVICE |
LPCR_HEIC)
- or r3, r3, r4
- LOAD_REG_IMMEDIATE(r4, L
Re: [PATCH v5 1/5] powerpc/sstep: Emulate prefixed instructions only when CPU_FTR_ARCH_31 is set
Hi Daniel, On 10/12/20 7:14 PM, Daniel Axtens wrote: Hi, To review this, I looked through the supported instructions to see if there were any that I thought might have been missed. I didn't find any other v3.1 ones, although I don't have a v3.1 ISA to hand so I was basically looking for instructions I didn't recognise. I did, however, find a number of instructions that are new in ISA 3.0 that aren't guarded: - addpcis - lxvl/stxvl - lxvll/stxvll - lxvwsx - stxvx - lxsipzx - lxvh8x - lxsihzx - lxvb16x/stxvb16x - stxsibx - stxsihx - lxvb16x - lxsd/stxsd - lxssp/stxssp - lxv/stxv Also, I don't know how bothered we are about P7, but if I'm reading the ISA correctly, lqarx/stqcx. are not supported before ISA 2.07. Likewise a number of the vector instructions like lxsiwzx and lxsiwax (and the companion stores). I realise it's not really the point of this particular patch, so I don't think this should block acceptance. What I would like to know - and maybe this is something where we need mpe to weigh in - is whether we need consistent guards for 2.07 and 3.0. We have some 3.0 guards already but clearly not everywhere. Yes, those needs to be handled properly. Otherwise they can be harmful when emulated on a non-supporting platform. Will work on it when I get some time. Thanks for reporting it. With all that said - the patch does what it says it does, and looks good to me: Reviewed-by: Daniel Axtens Thanks! Ravi
[PATCH] soc: fsl: dpio: Change 'cpumask_t mask' to global variable
From: Hao Si
The local variable 'cpumask_t mask' is in the stack memory, and its address
is assigned to 'desc->affinity' in 'irq_set_affinity_hint()'.
But the memory area where this variable is located is at risk of being
modified.
During LTP testing, the following error was generated:
Unable to handle kernel paging request at virtual address 12e9b790
Mem abort info:
ESR = 0x9607
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x0007
CM = 0, WnR = 0
swapper pgtable: 4k pages, 48-bit VAs, pgdp = 75ac5e07
[12e9b790] pgd=0027dbffe003, pud=0027dbffd003,
pmd=0027b6d61003, pte=
Internal error: Oops: 9607 [#1] PREEMPT SMP
Modules linked in: xt_conntrack
Process read_all (pid: 20171, stack limit = 0x44ea4095)
CPU: 14 PID: 20171 Comm: read_all Tainted: GB W
Hardware name: NXP Layerscape LX2160ARDB (DT)
pstate: 8085 (Nzcv daIf -PAN -UAO)
pc : irq_affinity_hint_proc_show+0x54/0xb0
lr : irq_affinity_hint_proc_show+0x4c/0xb0
sp : 1138bc10
x29: 1138bc10 x28: d131d1e0
x27: 007000c0 x26: 8025b9480dc0
x25: 8025b9480da8 x24: 03ff
x23: 8027334f8300 x22: 80272e97d000
x21: 80272e97d0b0 x20: 8025b9480d80
x19: 09a49000 x18:
x17: x16:
x15: x14:
x13: x12: 0040
x11: x10: 802735b79b88
x9 : x8 :
x7 : 09a49848 x6 : 0003
x5 : x4 : 08157d6c
x3 : 1138bc10 x2 : 12e9b790
x1 : x0 :
Call trace:
irq_affinity_hint_proc_show+0x54/0xb0
seq_read+0x1b0/0x440
proc_reg_read+0x80/0xd8
__vfs_read+0x60/0x178
vfs_read+0x94/0x150
ksys_read+0x74/0xf0
__arm64_sys_read+0x24/0x30
el0_svc_common.constprop.0+0xd8/0x1a0
el0_svc_handler+0x34/0x88
el0_svc+0x10/0x14
Code: f9001bbf 943e0732 f94066c2 b462 (f9400041)
---[ end trace b495bdcb0b3b732b ]---
Kernel panic - not syncing: Fatal exception
SMP: stopping secondary CPUs
SMP: failed to stop secondary CPUs 0,2-4,6,8,11,13-15
Kernel Offset: disabled
CPU features: 0x0,21006008
Memory Limit: none
---[ end Kernel panic - not syncing: Fatal exception ]---
Fix it by changing 'cpumask_t mask' to global variable.
Signed-off-by: Hao Si
Signed-off-by: Lin Chen
Signed-off-by: Yi Wang
---
drivers/soc/fsl/dpio/dpio-driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/dpio/dpio-driver.c
b/drivers/soc/fsl/dpio/dpio-driver.c
index 7b642c3..b31ec53 100644
--- a/drivers/soc/fsl/dpio/dpio-driver.c
+++ b/drivers/soc/fsl/dpio/dpio-driver.c
@@ -31,6 +31,7 @@ struct dpio_priv {
struct dpaa2_io *io;
};
+static cpumask_t mask;
static cpumask_var_t cpus_unused_mask;
static const struct soc_device_attribute ls1088a_soc[] = {
@@ -95,7 +96,6 @@ static int register_dpio_irq_handlers(struct fsl_mc_device
*dpio_dev, int cpu)
{
int error;
struct fsl_mc_device_irq *irq;
- cpumask_t mask;
irq = dpio_dev->irqs[0];
error = devm_request_irq(&dpio_dev->dev,
--
2.15.2
[PATCH] powerpc/perf: Add support for programming of Thresholding in P10
Thresholding, a performance monitoring unit feature, can be
used to identify marked instructions which take more than
expected cycles between start event and end event.
Threshold compare (thresh_cmp) bits are programmed in MMCRA
register. In Power9, thresh_cmp bits were part of the
event code. But in case of P10, thresh_cmp are not part of
event code due to inclusion of MMCR3 bits.
Patch here adds an option to use attr.config1 variable
to be used to pass thresh_cmp value to be programmed in
MMCRA register. A new ppmu flag called PPMU_HAS_ATTR_CONFIG1
has been added and this flag is used to notify the use of
attr.config1 variable. Secondly, thresh_cmp bits are not
part of the group constraints.
Patch has extended the parameter list of 'compute_mmcr',
to include power_pmu's 'flags' element.
command#: cat /sys/devices/cpu/format/thresh_cmp
config1:0-17
ex. usage:
command#: perf record -I --weight -d -e
cpu/event=0x67340101EC,thresh_cmp=500/ ./ebizzy -S 2 -t 1 -s 4096
1826636 records/s
real 2.00 s
user 2.00 s
sys 0.00 s
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.038 MB perf.data (61 samples) ]
Signed-off-by: Kajol Jain
---
arch/powerpc/include/asm/perf_event_server.h | 4 +-
arch/powerpc/perf/core-book3s.c | 2 +-
arch/powerpc/perf/isa207-common.c| 54 ++--
arch/powerpc/perf/isa207-common.h| 6 ++-
arch/powerpc/perf/mpc7450-pmu.c | 3 +-
arch/powerpc/perf/power10-pmu.c | 4 +-
arch/powerpc/perf/power5+-pmu.c | 3 +-
arch/powerpc/perf/power5-pmu.c | 3 +-
arch/powerpc/perf/power6-pmu.c | 3 +-
arch/powerpc/perf/power7-pmu.c | 3 +-
arch/powerpc/perf/ppc970-pmu.c | 3 +-
11 files changed, 73 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/perf_event_server.h
b/arch/powerpc/include/asm/perf_event_server.h
index f6acabb6c9be..566e41d4ff6a 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -34,9 +34,10 @@ struct power_pmu {
int max_alternatives;
unsigned long add_fields;
unsigned long test_adder;
+ /* TODO: parameter list is growing and should consider folding all this
to a struct */
int (*compute_mmcr)(u64 events[], int n_ev,
unsigned int hwc[], struct mmcr_regs *mmcr,
- struct perf_event *pevents[]);
+ struct perf_event *pevents[], u32 flags);
int (*get_constraint)(u64 event_id, unsigned long *mskp,
unsigned long *valp);
int (*get_alternatives)(u64 event_id, unsigned int flags,
@@ -82,6 +83,7 @@ struct power_pmu {
#define PPMU_ARCH_207S 0x0080 /* PMC is architecture v2.07S */
#define PPMU_NO_SIAR 0x0100 /* Do not use SIAR */
#define PPMU_ARCH_31 0x0200 /* Has MMCR3, SIER2 and SIER3 */
+#define PPMU_HAS_ATTR_CONFIG1 0x0400 /* Using config1 attribute */
/*
* Values for flags to get_alternatives()
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 08643cba1494..e4c817f64566 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1356,7 +1356,7 @@ static void power_pmu_enable(struct pmu *pmu)
memset(&cpuhw->mmcr, 0, sizeof(cpuhw->mmcr));
if (ppmu->compute_mmcr(cpuhw->events, cpuhw->n_events, hwc_index,
- &cpuhw->mmcr, cpuhw->event)) {
+ &cpuhw->mmcr, cpuhw->event, ppmu->flags)) {
/* shouldn't ever get here */
printk(KERN_ERR "oops compute_mmcr failed\n");
goto out;
diff --git a/arch/powerpc/perf/isa207-common.c
b/arch/powerpc/perf/isa207-common.c
index 964437adec18..cfad88b7ca85 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -110,10 +110,48 @@ static void mmcra_sdar_mode(u64 event, unsigned long
*mmcra)
static u64 thresh_cmp_val(u64 value)
{
- if (cpu_has_feature(CPU_FTR_ARCH_300))
- return value << p9_MMCRA_THR_CMP_SHIFT;
+ int exp = 0;
+ u64 result = value;
+
+ if (!value)
+ return value;
+
+ /*
+* Incase of P10, thresh_cmp value is not part of raw event code
+* and provided via attr.config1 parameter. To program threshold in
MMCRA,
+* take a 18 bit number N and shift right 2 places and increment
+* the exponent E by 1 until the upper 10 bits of N are zero.
+* Write E to the threshold exponent and write the lower 8 bits of N
+* to the threshold mantissa.
+* The max threshold that can be written is 261120.
+*/
+ if (cpu_has_feature(CPU_FTR_ARCH_31)) {
+ if (value >
Re: [PATCH] soc: fsl: dpio: Change 'cpumask_t mask' to global variable
Hi,
Thanks for finding this. Comment inline.
On 10/14/2020 10:27 AM, Yi Wang wrote:
> From: Hao Si
>
> The local variable 'cpumask_t mask' is in the stack memory, and its address
> is assigned to 'desc->affinity' in 'irq_set_affinity_hint()'.
> But the memory area where this variable is located is at risk of being
> modified.
>
> During LTP testing, the following error was generated:
>
> Unable to handle kernel paging request at virtual address 12e9b790
> Mem abort info:
> ESR = 0x9607
> Exception class = DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x0007
> CM = 0, WnR = 0
> swapper pgtable: 4k pages, 48-bit VAs, pgdp = 75ac5e07
> [12e9b790] pgd=0027dbffe003, pud=0027dbffd003,
> pmd=0027b6d61003, pte=
> Internal error: Oops: 9607 [#1] PREEMPT SMP
> Modules linked in: xt_conntrack
> Process read_all (pid: 20171, stack limit = 0x44ea4095)
> CPU: 14 PID: 20171 Comm: read_all Tainted: GB W
> Hardware name: NXP Layerscape LX2160ARDB (DT)
> pstate: 8085 (Nzcv daIf -PAN -UAO)
> pc : irq_affinity_hint_proc_show+0x54/0xb0
> lr : irq_affinity_hint_proc_show+0x4c/0xb0
> sp : 1138bc10
> x29: 1138bc10 x28: d131d1e0
> x27: 007000c0 x26: 8025b9480dc0
> x25: 8025b9480da8 x24: 03ff
> x23: 8027334f8300 x22: 80272e97d000
> x21: 80272e97d0b0 x20: 8025b9480d80
> x19: 09a49000 x18:
> x17: x16:
> x15: x14:
> x13: x12: 0040
> x11: x10: 802735b79b88
> x9 : x8 :
> x7 : 09a49848 x6 : 0003
> x5 : x4 : 08157d6c
> x3 : 1138bc10 x2 : 12e9b790
> x1 : x0 :
> Call trace:
> irq_affinity_hint_proc_show+0x54/0xb0
> seq_read+0x1b0/0x440
> proc_reg_read+0x80/0xd8
> __vfs_read+0x60/0x178
> vfs_read+0x94/0x150
> ksys_read+0x74/0xf0
> __arm64_sys_read+0x24/0x30
> el0_svc_common.constprop.0+0xd8/0x1a0
> el0_svc_handler+0x34/0x88
> el0_svc+0x10/0x14
> Code: f9001bbf 943e0732 f94066c2 b462 (f9400041)
> ---[ end trace b495bdcb0b3b732b ]---
> Kernel panic - not syncing: Fatal exception
> SMP: stopping secondary CPUs
> SMP: failed to stop secondary CPUs 0,2-4,6,8,11,13-15
> Kernel Offset: disabled
> CPU features: 0x0,21006008
> Memory Limit: none
> ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> Fix it by changing 'cpumask_t mask' to global variable.
>
> Signed-off-by: Hao Si
> Signed-off-by: Lin Chen
> Signed-off-by: Yi Wang
> ---
> drivers/soc/fsl/dpio/dpio-driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/fsl/dpio/dpio-driver.c
> b/drivers/soc/fsl/dpio/dpio-driver.c
> index 7b642c3..b31ec53 100644
> --- a/drivers/soc/fsl/dpio/dpio-driver.c
> +++ b/drivers/soc/fsl/dpio/dpio-driver.c
> @@ -31,6 +31,7 @@ struct dpio_priv {
> struct dpaa2_io *io;
> };
>
> +static cpumask_t mask;
There can be multiple dpio devices with their associated driver
instances so it's not ok to make the variable global. Please place it in
the driver's private data and while at it, please rename it to cpu_mask.
---
Thanks & Best Regards, Laurentiu
[PATCH v2] powerpc/perf: Fix Threshold Event Counter Multiplier width for P10
Threshold Event Counter Multiplier (TECM) is part of Monitor Mode
Control Register A (MMCRA). This field along with Threshold Event
Counter Exponent (TECE) is used to get threshould counter value.
ISA v3.1 has 7bit mantissa field for TECM, but in Power10, the
width of TECM field is increase to 8bits. Patch fixes the current
code to modify the MMCRA[TECM] extraction macro to handle this changes.
Fixes: 170a315f41c64 ('powerpc/perf: Support to export MMCRA[TEC*] field to
userspace')
Signed-off-by: Madhavan Srinivasan
---
Changelog v1:
- Fixed the commit message
- Fixed the condition check
arch/powerpc/include/asm/reg.h| 1 +
arch/powerpc/perf/isa207-common.c | 3 +++
arch/powerpc/perf/isa207-common.h | 4
3 files changed, 8 insertions(+)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 88fb88491fe9..a1bb0ebb3a46 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1355,6 +1355,7 @@
#define PVR_POWER9 0x004E
#define PVR_BE 0x0070
#define PVR_PA6T 0x0090
+#define PVR_POWER100x0080
/* "Logical" PVR values defined in PAPR, representing architecture levels */
#define PVR_ARCH_204 0x0f01
diff --git a/arch/powerpc/perf/isa207-common.c
b/arch/powerpc/perf/isa207-common.c
index 964437adec18..480bbe525904 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -247,6 +247,9 @@ void isa207_get_mem_weight(u64 *weight)
u64 sier = mfspr(SPRN_SIER);
u64 val = (sier & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
+ if (pvr_version_is(PVR_POWER10))
+ mantissa = P10_MMCRA_THR_CTR_MANT(mmcra);
+
if (val == 0 || val == 7)
*weight = 0;
else
diff --git a/arch/powerpc/perf/isa207-common.h
b/arch/powerpc/perf/isa207-common.h
index 044de65e96b9..71380e854f48 100644
--- a/arch/powerpc/perf/isa207-common.h
+++ b/arch/powerpc/perf/isa207-common.h
@@ -219,6 +219,10 @@
#define MMCRA_THR_CTR_EXP(v) (((v) >> MMCRA_THR_CTR_EXP_SHIFT) &\
MMCRA_THR_CTR_EXP_MASK)
+#define P10_MMCRA_THR_CTR_MANT_MASK0xFFul
+#define P10_MMCRA_THR_CTR_MANT(v) (((v) >> MMCRA_THR_CTR_MANT_SHIFT) &\
+ P10_MMCRA_THR_CTR_MANT_MASK)
+
/* MMCRA Threshold Compare bit constant for power9 */
#define p9_MMCRA_THR_CMP_SHIFT 45
--
2.26.2
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Wed, 14 Oct 2020 at 12:23, Serge Semin
wrote:
>
> In accordance with the DWC USB3 bindings the corresponding node name is
> suppose to comply with Generic USB HCD DT schema, which requires the USB
> nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> the dtbs_check procedure failure. Let's fix the nodes naming to be
> compatible with the DWC USB3 DT schema to make dtbs_check happy.
>
> Note we don't change the DWC USB3-compatible nodes names of
> arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> in-source comment says that the nodes name need to be preserved as
> "^dwusb@.*" for some backward compatibility.
>
> Signed-off-by: Serge Semin
>
> ---
>
> Please, test the patch out to make sure it doesn't brake the dependent DTS
> files. I did only a manual grepping of the possible nodes dependencies.
1. It is you who should compare the decompiled DTS, not us. For example:
$ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
$ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
dts-new/${i#dts-old/}.fdt ; done
2. Split it per arm architectures (and proper subject prefix - not
"arch") and subarchitectures so maintainers can pick it up.
3. The subject title could be more accurate - there is no fix here
because there was no errors in the first place. Requirement of DWC
node names comes recently, so it is more alignment with dtschema.
Otherwise automatic-pickup-stable-bot might want to pick up... and it
should not go to stable.
Best regards,
Krzysztof
> arch/arm/boot/dts/armada-375.dtsi | 2 +-
> arch/arm/boot/dts/exynos5250.dtsi | 2 +-
> arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
> arch/arm/boot/dts/keystone-k2e.dtsi| 4 ++--
> arch/arm/boot/dts/keystone.dtsi| 2 +-
> arch/arm/boot/dts/ls1021a.dtsi | 2 +-
> arch/arm/boot/dts/omap5-l4.dtsi| 2 +-
> arch/arm/boot/dts/stih407-family.dtsi | 2 +-
> arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 ++--
> arch/arm64/boot/dts/exynos/exynos7.dtsi| 2 +-
> arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 ++--
> arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
> arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
> arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++--
> arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 ++--
> arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
> arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++--
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
> arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +-
> 25 files changed, 38 insertions(+), 38 deletions(-)
>
Re: [PATCH] powerpc/features: Remove CPU_FTR_NODSISRALIGN
"Aneesh Kumar K.V" writes:
> On 10/13/20 3:45 PM, Michael Ellerman wrote:
>> Christophe Leroy writes:
>>> Le 13/10/2020 à 09:23, Aneesh Kumar K.V a écrit :
Christophe Leroy writes:
> CPU_FTR_NODSISRALIGN has not been used since
> commit 31bfdb036f12 ("powerpc: Use instruction emulation
> infrastructure to handle alignment faults")
>
> Remove it.
>
> Signed-off-by: Christophe Leroy
> ---
>arch/powerpc/include/asm/cputable.h | 22 ++
>arch/powerpc/kernel/dt_cpu_ftrs.c | 8
>arch/powerpc/kernel/prom.c | 2 +-
>3 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
> b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 1098863e17ee..c598961d9f15 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -273,13 +273,6 @@ static int __init feat_enable_idle_nap(struct
> dt_cpu_feature *f)
> return 1;
>}
>
> -static int __init feat_enable_align_dsisr(struct dt_cpu_feature *f)
> -{
> - cur_cpu_spec->cpu_features &= ~CPU_FTR_NODSISRALIGN;
> -
> - return 1;
> -}
> -
>static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
>{
> u64 lpcr;
> @@ -641,7 +634,6 @@ static struct dt_cpu_feature_match __initdata
> {"tm-suspend-hypervisor-assist", feat_enable,
> CPU_FTR_P9_TM_HV_ASSIST},
> {"tm-suspend-xer-so-bug", feat_enable,
> CPU_FTR_P9_TM_XER_SO_BUG},
> {"idle-nap", feat_enable_idle_nap, 0},
> - {"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
>>
>> Rather than removing it entirely, I'd rather we left a comment, so that
>> it's obvious that we are ignoring that feature on purpose, not because
>> we forget about it.
>>
>> eg:
>>
>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
>> b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> index f204ad79b6b5..45cb7e59bd13 100644
>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> @@ -640,7 +640,7 @@ static struct dt_cpu_feature_match __initdata
>> {"tm-suspend-hypervisor-assist", feat_enable, CPU_FTR_P9_TM_HV_ASSIST},
>> {"tm-suspend-xer-so-bug", feat_enable, CPU_FTR_P9_TM_XER_SO_BUG},
>> {"idle-nap", feat_enable_idle_nap, 0},
>> -{"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
>> +// "alignment-interrupt-dsisr" ignored
>> {"idle-stop", feat_enable_idle_stop, 0},
>> {"machine-check-power8", feat_enable_mce_power8, 0},
>> {"performance-monitor-power8", feat_enable_pmu_power8, 0},
>>
>
>
> why not do it as
> static int __init feat_enable_align_dsisr(struct dt_cpu_feature *f)
> {
> /* This feature should not be enabled */
> #ifdef DEBUG
> WARN(1);
> #endif
>
> return 1;
> }
No one will ever turn that #define on.
No one will ever turn the feature on either.
Even if they did, it's not a bug because the kernel doesn't use the
DSISR for alignment interrupts any more.
All I want is to be able to compare the list of features defined in
skiboot vs the ones in Linux and see that none are missing in Linux.
cheers
Re: [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time
On Wed, 2020-10-14 at 17:35 +0800, Chester Lin wrote: > Hi Ard & Mimi, > > On Tue, Oct 13, 2020 at 06:59:21PM +0200, Ard Biesheuvel wrote: > > On Tue, 13 Oct 2020 at 18:46, Mimi Zohar wrote: > > > > > > [Cc'ing linuxppc-dev@lists.ozlabs.org] > > > > > > On Tue, 2020-10-13 at 10:18 +0200, Ard Biesheuvel wrote: > > > > Chester reports that it is necessary to introduce a new way to pass > > > > the EFI secure boot status between the EFI stub and the core kernel > > > > on ARM systems. The usual way of obtaining this information is by > > > > checking the SecureBoot and SetupMode EFI variables, but this can > > > > only be done after the EFI variable workqueue is created, which > > > > occurs in a subsys_initcall(), whereas arch_ima_get_secureboot() > > > > is called much earlier by the IMA framework. > > > > > > > > However, the IMA framework itself is started as a late_initcall, > > > > and the only reason the call to arch_ima_get_secureboot() occurs > > > > so early is because it happens in the context of a __setup() > > > > callback that parses the ima_appraise= command line parameter. > > > > > > > > So let's refactor this code a little bit, by using a core_param() > > > > callback to capture the command line argument, and deferring any > > > > reasoning based on its contents to the IMA init routine. > > > > > > > > Cc: Chester Lin > > > > Cc: Mimi Zohar > > > > Cc: Dmitry Kasatkin > > > > Cc: James Morris > > > > Cc: "Serge E. Hallyn" > > > > Link: > > > > https://lore.kernel.org/linux-arm-kernel/20200904072905.25332-2-c...@suse.com/ > > > > Signed-off-by: Ard Biesheuvel > > > > --- > > > > v2: rebase onto series 'integrity: improve user feedback for invalid > > > > bootparams' > > > > > > Thanks, Ard. Based on my initial, limited testing on Power, it looks > > > good, but I'm hesistant to include it in the integrity 5.10 pull > > > request without it having been in linux-next and some additional > > > testing. It's now queued in the next-integrity-testing branch awaiting > > > some tags. > > > > > Tested-by: Chester Lin > > I have tested this patch on x86 VM. > > * System configuration: > - Platform: QEMU/KVM > - Firmware: EDK2/OVMF + secure boot enabled. > - OS: SLE15-SP2 + SUSE's kernel-vanilla (=linux v5.9) + the follow commits > from linux-next and upstream: > * [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time > https://www.spinics.net/lists/linux-efi/msg20645.html > * e4d7e2df3a09 "ima: limit secure boot feedback scope for appraise" > * 7fe2bb7e7e5c "integrity: invalid kernel parameters feedback" > * 4afb28ab03d5 "ima: add check for enforced appraise option" > > * Logs with UEFI secure boot enabled: > > [0.00] Linux version 5.9.0-858-g865c50e1d279-1.g8764d18-vanilla > (geeko@b > uildhost) (gcc (SUSE Linux) 10.2.1 20200825 [revision > c0746a1beb1ba073c7981eb09f > 55b3d993b32e5c], GNU ld (GNU Binutils; openSUSE Tumbleweed) > 2.34.0.20200325-1) # > 1 SMP Wed Oct 14 04:00:11 UTC 2020 (8764d18) > [0.00] Command line: > BOOT_IMAGE=/boot/vmlinuz-5.9.0-858-g865c50e1d279-1. > g8764d18-vanilla root=UUID=5304c03e-4d8a-4d67-873a-32a32e57cdeb > console=ttyS0,11 > 5200 resume=/dev/disk/by-path/pci-:04:00.0-part4 mitigations=auto > ignore_log > level crashkernel=192M,high crashkernel=72M,low ima_appraise=off > [0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point > regi > sters' > [0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers' > [0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers' > > > [1.720309] ima: Secure boot enabled: ignoring ima_appraise=off option > [1.720314] ima: No TPM chip found, activating TPM-bypass! > [1.722129] ima: Allocated hash algorithm: sha256 Thank you for testing the options aren't being set in secure boot mode. My main concern, however, is that IMA doesn't go into TPM-bypass mode. Does this system have a TPM? Mimi
Re: [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time
Hi Ard & Mimi, On Tue, Oct 13, 2020 at 06:59:21PM +0200, Ard Biesheuvel wrote: > On Tue, 13 Oct 2020 at 18:46, Mimi Zohar wrote: > > > > [Cc'ing linuxppc-dev@lists.ozlabs.org] > > > > On Tue, 2020-10-13 at 10:18 +0200, Ard Biesheuvel wrote: > > > Chester reports that it is necessary to introduce a new way to pass > > > the EFI secure boot status between the EFI stub and the core kernel > > > on ARM systems. The usual way of obtaining this information is by > > > checking the SecureBoot and SetupMode EFI variables, but this can > > > only be done after the EFI variable workqueue is created, which > > > occurs in a subsys_initcall(), whereas arch_ima_get_secureboot() > > > is called much earlier by the IMA framework. > > > > > > However, the IMA framework itself is started as a late_initcall, > > > and the only reason the call to arch_ima_get_secureboot() occurs > > > so early is because it happens in the context of a __setup() > > > callback that parses the ima_appraise= command line parameter. > > > > > > So let's refactor this code a little bit, by using a core_param() > > > callback to capture the command line argument, and deferring any > > > reasoning based on its contents to the IMA init routine. > > > > > > Cc: Chester Lin > > > Cc: Mimi Zohar > > > Cc: Dmitry Kasatkin > > > Cc: James Morris > > > Cc: "Serge E. Hallyn" > > > Link: > > > https://lore.kernel.org/linux-arm-kernel/20200904072905.25332-2-c...@suse.com/ > > > Signed-off-by: Ard Biesheuvel > > > --- > > > v2: rebase onto series 'integrity: improve user feedback for invalid > > > bootparams' > > > > Thanks, Ard. Based on my initial, limited testing on Power, it looks > > good, but I'm hesistant to include it in the integrity 5.10 pull > > request without it having been in linux-next and some additional > > testing. It's now queued in the next-integrity-testing branch awaiting > > some tags. > > Tested-by: Chester Lin I have tested this patch on x86 VM. * System configuration: - Platform: QEMU/KVM - Firmware: EDK2/OVMF + secure boot enabled. - OS: SLE15-SP2 + SUSE's kernel-vanilla (=linux v5.9) + the follow commits from linux-next and upstream: * [PATCH v2] ima: defer arch_ima_get_secureboot() call to IMA init time https://www.spinics.net/lists/linux-efi/msg20645.html * e4d7e2df3a09 "ima: limit secure boot feedback scope for appraise" * 7fe2bb7e7e5c "integrity: invalid kernel parameters feedback" * 4afb28ab03d5 "ima: add check for enforced appraise option" * Logs with UEFI secure boot enabled: [0.00] Linux version 5.9.0-858-g865c50e1d279-1.g8764d18-vanilla (geeko@b uildhost) (gcc (SUSE Linux) 10.2.1 20200825 [revision c0746a1beb1ba073c7981eb09f 55b3d993b32e5c], GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.34.0.20200325-1) # 1 SMP Wed Oct 14 04:00:11 UTC 2020 (8764d18) [0.00] Command line: BOOT_IMAGE=/boot/vmlinuz-5.9.0-858-g865c50e1d279-1. g8764d18-vanilla root=UUID=5304c03e-4d8a-4d67-873a-32a32e57cdeb console=ttyS0,11 5200 resume=/dev/disk/by-path/pci-:04:00.0-part4 mitigations=auto ignore_log level crashkernel=192M,high crashkernel=72M,low ima_appraise=off [0.00] x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point regi sters' [0.00] x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers' [0.00] x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers' [1.720309] ima: Secure boot enabled: ignoring ima_appraise=off option [1.720314] ima: No TPM chip found, activating TPM-bypass! [1.722129] ima: Allocated hash algorithm: sha256 > > Thanks. No rush as far as I am concerned, although I suppose Chester > may want to rebase his arm64 IMA enablement series on this. Yes, I've finished coding but still verifying it. As you have suggested, My v2 patch will separate the get_sb_mode() from arch/x86 so that other architectures can reuse it. Thanks, Chester > > Suggestion: can we take the get_sb_mode() code from ima_arch.c in > arch/x86, and generalize it for all EFI architectures? That way, we > can enable 32-bit ARM and RISC-V seamlessly once someone gets around > to enabling IMA on those platforms. In fact, get_sb_mode() itself > should probably be factored out into a generic helper for use outside > of IMA as well (Xen/x86 has code that does roughly the same already) >
Re: [PATCH 04/20] dt-bindings: usb: usb-hcd: Add "tpl-support" property
On Wed, 14 Oct 2020 13:13:46 +0300, Serge Semin wrote: > The host controller device might be designed to work for the particular > products or applications. In that case its DT node is supposed to be > equipped with the tpl-support property. > > Signed-off-by: Serge Semin > > --- > > Changelog v2: > - Grammar fix: "s/it'/its" > - Discard '|' from the property description, since we don't need to preserve > the text formatting. > --- > Documentation/devicetree/bindings/usb/usb-hcd.yaml | 6 ++ > 1 file changed, 6 insertions(+) > My bot found errors running 'make dt_binding_check' on your patch: Traceback (most recent call last): File "/usr/local/bin/dt-extract-example", line 45, in binding = yaml.load(open(args.yamlfile, encoding='utf-8').read()) File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 343, in load return constructor.get_single_data() File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data node = self.composer.get_single_node() File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node File "_ruamel_yaml.pyx", line 891, in _ruamel_yaml.CParser._compose_mapping_node File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this context in "", line 27, column 14 make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/usb/usb-hcd.example.dts] Error 1 make[1]: *** Deleting file 'Documentation/devicetree/bindings/usb/usb-hcd.example.dts' make[1]: *** Waiting for unfinished jobs ./Documentation/devicetree/bindings/usb/usb-hcd.yaml:27:14: [error] syntax error: mapping values are not allowed here (syntax) make[1]: *** [Documentation/devicetree/bindings/Makefile:59: Documentation/devicetree/bindings/processed-schema-examples.json] Error 123 make: *** [Makefile:1366: dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1382001 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit.
Re: [PATCH 09/20] dt-bindings: usb: Convert DWC USB3 bindings to DT schema
On Wed, 14 Oct 2020 13:13:51 +0300, Serge Semin wrote: > DWC USB3 DT node is supposed to be compliant with the Generic xHCI > Controller schema, but with additional vendor-specific properties, the > controller-specific reference clocks and PHYs. So let's convert the > currently available legacy text-based DWC USB3 bindings to the DT schema > and make sure the DWC USB3 nodes are also validated against the > usb-xhci.yaml schema. > > Note we have to discard the nodename restriction of being prefixed with > "dwc3@" string, since in accordance with the usb-hcd.yaml schema USB nodes > are supposed to be named as "^usb(@.*)". > > Signed-off-by: Serge Semin > > --- > > Changelog v2: > - Discard '|' from the descriptions, since we don't need to preserve > the text formatting in any of them. > - Drop quotes from around the string constants. > - Fix the "clock-names" prop description to be referring the enumerated > clock-names instead of the ones from the Databook. > --- > .../devicetree/bindings/usb/dwc3.txt | 125 > .../devicetree/bindings/usb/snps,dwc3.yaml| 295 ++ > 2 files changed, 295 insertions(+), 125 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt > create mode 100644 Documentation/devicetree/bindings/usb/snps,dwc3.yaml > My bot found errors running 'make dt_binding_check' on your patch: ./Documentation/devicetree/bindings/usb/snps,dwc3.yaml:44:4: [warning] wrong indentation: expected 4 but found 3 (indentation) /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dt.yaml: dwc3@a60: $nodename:0: 'dwc3@a60' does not match '^usb(@.*)?' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.example.dt.yaml: usb@ff50: snps,quirk-frame-length-adjustment: True is not of type 'array' From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.yaml See https://patchwork.ozlabs.org/patch/1382003 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit.
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
Hi Serge,
Serge Semin writes:
> In accordance with the DWC USB3 bindings the corresponding node name is
> suppose to comply with Generic USB HCD DT schema, which requires the USB
DWC3 is not a simple HDC, though.
> nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> the dtbs_check procedure failure. Let's fix the nodes naming to be
> compatible with the DWC USB3 DT schema to make dtbs_check happy.
>
> Note we don't change the DWC USB3-compatible nodes names of
> arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> in-source comment says that the nodes name need to be preserved as
> "^dwusb@.*" for some backward compatibility.
interesting, compatibility with what? Some debugfs files, perhaps? :-)
In any case, I don't have any problems with this, so I'll let other
folks comment.
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH] powerpc/pci: Fix PHB removal/rescan on PowerNV
On Thu, 8 Oct 2020 06:37:02 +0200 Cédric Le Goater wrote: > On 10/8/20 4:23 AM, Oliver O'Halloran wrote: > > On Fri, Sep 25, 2020 at 7:23 PM Cédric Le Goater wrote: > >> > >> To fix an issue with PHB hotplug on pSeries machine (HPT/XIVE), commit > >> 3a3181e16fbd introduced a PPC specific pcibios_remove_bus() routine to > >> clear all interrupt mappings when the bus is removed. This routine > >> frees an array allocated in pcibios_scan_phb(). > >> > >> This broke PHB hotplug on PowerNV because, when a PHB is removed and > >> re-scanned through sysfs, the PCI layer un-assigns and re-assigns > >> resources to the PHB but does not destroy and recreate the PCI > >> controller structure. Since pcibios_remove_bus() does not clear the > >> 'irq_map' array pointer, a second removal of the PHB will try to free > >> the array a second time and corrupt memory. > > > > "PHB hotplug" and "hot-plugging devices under a PHB" are different > > things. What you're saying here doesn't make a whole lot of sense to > > me unless you're conflating the two. The distinction is important > > since on pseries we can use DLPAR to add and remove actual PHBs (i.e. > > the pci_controller) at runtime, but there's no corresponding mechanism > > on PowerNV. > > And it's even different on QEMU. > If the real HW doesn't have the notion of adding/removing a PHB at runtime, then QEMU should stick to that, ie. setting dc->hotpluggable to false for PNV PHB device types. > >> Free the 'irq_map' array in pcibios_free_controller() to fix > >> corruption and clear interrupt mapping after it has been > >> disposed. This to avoid filling up the array with successive > >> remove/rescan of a bus. > > > > Even with this patch I think we're still broken. With this patch > > applied the init path is something like: > > > > per-phb init: > > allocate phb->irq_map array > > per-bus init: > > nothing > > per-device init: > > pcibios_bus_add_device() > >pci_read_irq_line() > > pci_irq_map_register(pci_dev, virq) > >*record the device's interrupt in phb->irq_map* > > > > And the teardown path: > > > > per-device teardown: > > nothing > > per-bus teardown: > > pcibios_remove_bus() > > pci_irq_map_dispose() > > *walk phb->irq_map and dispose of each mapped interrupt* > > per-phb teardown: > > free(phb->irq_map) > > > > There's a lot of asymmetry here, which is a problem in itself, but the > > real issue is that when removing *any* pci_bus under a PHB we dispose > > of the LSI\ for *every* device under that PHB. Not good. > > > > Ideally we should be fixing this by having the per-device teardown > > handle disposing the mapping. Unfortunately, there's no pcibios hook > > that's called when removing a pci_dev. However, we can register a bus > > notifier which will be called when the pci_dev is removed from its bus > > and we already do that for the per-device EEH teardown and to handle > > IOMMU TCE invalidation when the device is removed. > > I lack the knowledge here and I think some else should take over, > as I am not doing a good job. > > Michael, can you drop the initial patch again :/ It is better not > to merge anything. > > Thanks, > > C. > >
Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
On 10/14/20 3:14 AM, Serge Semin wrote: > In accordance with the Generic EHCI/OHCI bindings the corresponding node > name is suppose to comply with the Generic USB HCD DT schema, which > requires the USB nodes to have the name acceptable by the regexp: > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > incompatible names. > > Signed-off-by: Serge Semin > > --- > > Please, test the patch out to make sure it doesn't brake the dependent DTS > files. I did only a manual grepping of the possible nodes dependencies. Not sure how you envisioned these change to be picked up, but you may need to split these changes between ARM/ARM64, MIPS and PowerPC at least. And within ARM/ARM64 you will most likely have to split according to the various SoC maintainers. -- Florian
[PATCH -next] Revert "powerpc/pci: unmap legacy INTx interrupts when a PHB is removed"
This reverts commit 3a3181e16fbde752007759f8759d25e0ff1fc425 which
causes memory corruptions on POWER9 NV.
Signed-off-by: Qian Cai
---
arch/powerpc/include/asm/pci-bridge.h | 6 --
arch/powerpc/kernel/pci-common.c | 114 --
2 files changed, 120 deletions(-)
diff --git a/arch/powerpc/include/asm/pci-bridge.h
b/arch/powerpc/include/asm/pci-bridge.h
index d21e070352dc..d2a2a14e56f9 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -48,9 +48,6 @@ struct pci_controller_ops {
/*
* Structure of a PCI controller (host bridge)
- *
- * @irq_count: number of interrupt mappings
- * @irq_map: interrupt mappings
*/
struct pci_controller {
struct pci_bus *bus;
@@ -130,9 +127,6 @@ struct pci_controller {
void *private_data;
struct npu *npu;
-
- unsigned int irq_count;
- unsigned int *irq_map;
};
/* These are used for config access before all the PCI probing
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index deb831f0ae13..be108616a721 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -353,115 +353,6 @@ struct pci_controller *pci_find_controller_for_domain(int
domain_nr)
return NULL;
}
-/*
- * Assumption is made on the interrupt parent. All interrupt-map
- * entries are considered to have the same parent.
- */
-static int pcibios_irq_map_count(struct pci_controller *phb)
-{
- const __be32 *imap;
- int imaplen;
- struct device_node *parent;
- u32 intsize, addrsize, parintsize, paraddrsize;
-
- if (of_property_read_u32(phb->dn, "#interrupt-cells", &intsize))
- return 0;
- if (of_property_read_u32(phb->dn, "#address-cells", &addrsize))
- return 0;
-
- imap = of_get_property(phb->dn, "interrupt-map", &imaplen);
- if (!imap) {
- pr_debug("%pOF : no interrupt-map\n", phb->dn);
- return 0;
- }
- imaplen /= sizeof(u32);
- pr_debug("%pOF : imaplen=%d\n", phb->dn, imaplen);
-
- if (imaplen < (addrsize + intsize + 1))
- return 0;
-
- imap += intsize + addrsize;
- parent = of_find_node_by_phandle(be32_to_cpup(imap));
- if (!parent) {
- pr_debug("%pOF : no imap parent found !\n", phb->dn);
- return 0;
- }
-
- if (of_property_read_u32(parent, "#interrupt-cells", &parintsize)) {
- pr_debug("%pOF : parent lacks #interrupt-cells!\n", phb->dn);
- return 0;
- }
-
- if (of_property_read_u32(parent, "#address-cells", ¶ddrsize))
- paraddrsize = 0;
-
- return imaplen / (addrsize + intsize + 1 + paraddrsize + parintsize);
-}
-
-static void pcibios_irq_map_init(struct pci_controller *phb)
-{
- phb->irq_count = pcibios_irq_map_count(phb);
- if (phb->irq_count < PCI_NUM_INTX)
- phb->irq_count = PCI_NUM_INTX;
-
- pr_debug("%pOF : interrupt map #%d\n", phb->dn, phb->irq_count);
-
- phb->irq_map = kcalloc(phb->irq_count, sizeof(unsigned int),
- GFP_KERNEL);
-}
-
-static void pci_irq_map_register(struct pci_dev *pdev, unsigned int virq)
-{
- struct pci_controller *phb = pci_bus_to_host(pdev->bus);
- int i;
-
- if (!phb->irq_map)
- return;
-
- for (i = 0; i < phb->irq_count; i++) {
- /*
-* Look for an empty or an equivalent slot, as INTx
-* interrupts can be shared between adapters.
-*/
- if (phb->irq_map[i] == virq || !phb->irq_map[i]) {
- phb->irq_map[i] = virq;
- break;
- }
- }
-
- if (i == phb->irq_count)
- pr_err("PCI:%s all platform interrupts mapped\n",
- pci_name(pdev));
-}
-
-/*
- * Clearing the mapped interrupts will also clear the underlying
- * mappings of the ESB pages of the interrupts when under XIVE. It is
- * a requirement of PowerVM to clear all memory mappings before
- * removing a PHB.
- */
-static void pci_irq_map_dispose(struct pci_bus *bus)
-{
- struct pci_controller *phb = pci_bus_to_host(bus);
- int i;
-
- if (!phb->irq_map)
- return;
-
- pr_debug("PCI: Clearing interrupt mappings for PHB %04x:%02x...\n",
-pci_domain_nr(bus), bus->number);
- for (i = 0; i < phb->irq_count; i++)
- irq_dispose_mapping(phb->irq_map[i]);
-
- kfree(phb->irq_map);
-}
-
-void pcibios_remove_bus(struct pci_bus *bus)
-{
- pci_irq_map_dispose(bus);
-}
-EXPORT_SYMBOL_GPL(pcibios_remove_bus);
-
/*
* Reads the interrupt pin to determine if interrupt is use by card.
* If the interrupt is used, then gets the interrupt line from the
@@ -510,8 +401,6 @@ static int pci_read_irq_line(struct pci_dev *pci_dev)
pci_dev
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Wed, Oct 14, 2020 at 9:37 AM Serge Semin
wrote:
>
> On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> >
> > Hi Serge,
> >
> > Serge Semin writes:
> > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> >
>
> > DWC3 is not a simple HDC, though.
>
> Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> which are tuned by the DWC USB3 driver in the kernel. But after that the
> controller is registered as xhci-hcd device so it's serviced by the xHCI
> driver,
> which then registers the HCD device so the corresponding DT node is supposed
> to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml
> and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted
> compatibility.
>
> >
> > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > >
> > > Note we don't change the DWC USB3-compatible nodes names of
> > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > in-source comment says that the nodes name need to be preserved as
> > > "^dwusb@.*" for some backward compatibility.
> >
>
> > interesting, compatibility with what? Some debugfs files, perhaps? :-)
>
> Don't really know.) In my experience the worst type of such compatibility is
> connected with some bootloader magic, which may add/remove/modify properties
> to nodes with pre-defined names.
I seriously doubt anyone is using the APM machines with DT (even ACPI
is somewhat doubtful). I say change them. Or remove the dts files and
see what happens. Either way it can always be reverted.
Rob
Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
On 10/14/20 11:11 AM, Serge Semin wrote: > On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote: >> On 10/14/20 3:14 AM, Serge Semin wrote: >>> In accordance with the Generic EHCI/OHCI bindings the corresponding node >>> name is suppose to comply with the Generic USB HCD DT schema, which >>> requires the USB nodes to have the name acceptable by the regexp: >>> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with >>> incompatible names. >>> >>> Signed-off-by: Serge Semin >>> >>> --- >>> >>> Please, test the patch out to make sure it doesn't brake the dependent DTS >>> files. I did only a manual grepping of the possible nodes dependencies. >> > >> Not sure how you envisioned these change to be picked up, but you may >> need to split these changes between ARM/ARM64, MIPS and PowerPC at >> least. And within ARM/ARM64 you will most likely have to split according >> to the various SoC maintainers. > > Hmm, I don't really know how it's going to be done in this case, but there > must > be a way to get the cross-platform patches picked up in general. For > instance, see the patches like: > 714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread() > 140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS > They touched the files from different files, but still have been merged in. That situation is different, when a new facility is added and someone has gone through the work of adding support for all architectures (or nearly all of them), you want them to be merged in a way that limits merge conflicts with other architecture changes. Here you are fixing warnings, and each file you touch can clearly be independently change from other files in the series without causing merge conflicts. You are however creating the potential for merge conflicts with other changes that the various SoC maintainers have queued up. > Maybe I should have copied these three patches to the > "linux-a...@vger.kernel.org" > list or some other mailing list... Maybe Rob can queue them through his device tree repository, with the ack of the various SoC maintainers... -- Florian
Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
Le mer. 14 oct. 2020 à 13:14, Serge Semin
a écrit :
In accordance with the Generic EHCI/OHCI bindings the corresponding
node
name is suppose to comply with the Generic USB HCD DT schema, which
requires the USB nodes to have the name acceptable by the regexp:
"^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined
with
incompatible names.
Signed-off-by: Serge Semin
---
Please, test the patch out to make sure it doesn't brake the
dependent DTS
files. I did only a manual grepping of the possible nodes
dependencies.
---
arch/arc/boot/dts/axs10x_mb.dtsi | 4 ++--
arch/arc/boot/dts/hsdk.dts | 4 ++--
arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +-
arch/arm/boot/dts/bcm5301x.dtsi| 4 ++--
arch/arm/boot/dts/bcm53573.dtsi| 4 ++--
arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 ++--
arch/arm/boot/dts/lpc18xx.dtsi | 4 ++--
arch/arm/boot/dts/stm32mp151.dtsi | 4 ++--
arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 4 ++--
arch/arm64/boot/dts/hisilicon/hip06.dtsi | 4 ++--
arch/arm64/boot/dts/hisilicon/hip07.dtsi | 4 ++--
arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +-
arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +-
For jz4740.dtsi and jz4770.dtsi:
Acked-by: Paul Cercueil
Cheers,
-Paul
arch/mips/boot/dts/mti/sead3.dts | 2 +-
arch/mips/boot/dts/ralink/mt7628a.dtsi | 2 +-
arch/powerpc/boot/dts/akebono.dts | 6 +++---
16 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi
b/arch/arc/boot/dts/axs10x_mb.dtsi
index 99d3e7175bf7..b64435385304 100644
--- a/arch/arc/boot/dts/axs10x_mb.dtsi
+++ b/arch/arc/boot/dts/axs10x_mb.dtsi
@@ -87,13 +87,13 @@ gmac: ethernet@18000 {
mac-address = [00 00 00 00 00 00]; /* Filled in by
U-Boot */
};
- ehci@4 {
+ usb@4 {
compatible = "generic-ehci";
reg = < 0x4 0x100 >;
interrupts = < 8 >;
};
- ohci@6 {
+ usb@6 {
compatible = "generic-ohci";
reg = < 0x6 0x100 >;
interrupts = < 8 >;
diff --git a/arch/arc/boot/dts/hsdk.dts b/arch/arc/boot/dts/hsdk.dts
index dcaa44e408ac..fdd4f7f635d3 100644
--- a/arch/arc/boot/dts/hsdk.dts
+++ b/arch/arc/boot/dts/hsdk.dts
@@ -234,7 +234,7 @@ phy0: ethernet-phy@0 { /* Micrel KSZ9031 */
};
};
- ohci@6 {
+ usb@6 {
compatible = "snps,hsdk-v1.0-ohci", "generic-ohci";
reg = <0x6 0x100>;
interrupts = <15>;
@@ -242,7 +242,7 @@ ohci@6 {
dma-coherent;
};
- ehci@4 {
+ usb@4 {
compatible = "snps,hsdk-v1.0-ehci", "generic-ehci";
reg = <0x4 0x100>;
interrupts = <15>;
diff --git a/arch/arc/boot/dts/vdk_axs10x_mb.dtsi
b/arch/arc/boot/dts/vdk_axs10x_mb.dtsi
index cbb179770293..90a412026e64 100644
--- a/arch/arc/boot/dts/vdk_axs10x_mb.dtsi
+++ b/arch/arc/boot/dts/vdk_axs10x_mb.dtsi
@@ -46,7 +46,7 @@ ethernet@18000 {
clock-names = "stmmaceth";
};
- ehci@4 {
+ usb@4 {
compatible = "generic-ehci";
reg = < 0x4 0x100 >;
interrupts = < 8 >;
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi
b/arch/arm/boot/dts/bcm5301x.dtsi
index 0016720ce530..bf5656d79a55 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -261,7 +261,7 @@ usb2: usb2@21000 {
interrupt-parent = <&gic>;
- ehci: ehci@21000 {
+ ehci: usb@21000 {
#usb-cells = <0>;
compatible = "generic-ehci";
@@ -283,7 +283,7 @@ ehci_port2: port@2 {
};
};
- ohci: ohci@22000 {
+ ohci: usb@22000 {
#usb-cells = <0>;
compatible = "generic-ohci";
diff --git a/arch/arm/boot/dts/bcm53573.dtsi
b/arch/arm/boot/dts/bcm53573.dtsi
index 4af8e3293cff..51546fccc616 100644
--- a/arch/arm/boot/dts/bcm53573.dtsi
+++ b/arch/arm/boot/dts/bcm53573.dtsi
@@ -135,7 +135,7 @@ usb2: usb2@4000 {
#address-cells = <1>;
#size-cells = <1>;
- ehci: ehci@4000 {
+ ehci: usb@4000 {
compatible = "generic-ehci";
Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
Hi Sergey, > arch/arc/boot/dts/axs10x_mb.dtsi | 4 ++-- > arch/arc/boot/dts/hsdk.dts | 4 ++-- > arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +- For ARC boards Acked-by: Alexey Brodkin
RE: [RFC v1 0/2] Plumbing to support multiple secure memory backends.
On Wed, Oct 14, 2020 at 07:31:17AM +0100, Christoph Hellwig wrote: > Please don't add an abstraction without a second implementation. > Once we have the implementation we can consider the tradeoffs. E.g. > if expensive indirect function calls are really needed vs simple > branches. Ok. Not planning on upstreaming these patches till we have atleast another backend. -- Ram Pai
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Wed, 14 Oct 2020 at 19:16, Serge Semin
wrote:
>
> On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> > On Wed, 14 Oct 2020 at 12:23, Serge Semin
> > wrote:
> > >
> > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > >
> > > Note we don't change the DWC USB3-compatible nodes names of
> > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > in-source comment says that the nodes name need to be preserved as
> > > "^dwusb@.*" for some backward compatibility.
> > >
> > > Signed-off-by: Serge Semin
> > >
> > > ---
> > >
> > > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > > files. I did only a manual grepping of the possible nodes dependencies.
> >
>
> > 1. It is you who should compare the decompiled DTS, not us. For example:
> > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
> >
> > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> > dts-new/${i#dts-old/}.fdt ; done
>
> So basically you suggest first to compile the old and new dts files, then to
> de-compile them, then diff old and new fdt's, and visually compare the
> results.
> Personally it isn't that much better than what I did, since each old and new
> dtbs will for sure differ due to the node names change suggested in this
> patch.
> So it will lead to the visual debugging too, which isn't that effective. But
> your approach is still more demonstrative to make sure that I didn't loose any
> nodes redefinition, since in the occasion the old and new de-compiled nodes
> will
> differ not only by the node names but with an additional old named node.
My suggestion is to compare the entire, effective DTS after all
inclusions. Maybe you did it already, I don't know. The point is that
when you change node names in DTSI but you miss one in DTS, you end up
with two nodes. This is much easier to spot with dtxdiff or with
fdtdump (which behaves better for node moves).
Indeed it is still a visual comparison - if you have any ideas how to
automate it (e.g. ignore phandle changes), please share. It would
solve my testings as well. But asking others to test because you do
not want to check it is not the best way to handle such changes.
>
> So to speak thanks for suggesting it. I'll try it to validate the proposed
> changes.
>
> Two questions:
> 1) Any advise of a good inliner/command to compile all dtbs at once? Of
> course I
> can get all the updated dtsi'es, then find out all the dts'es which include
> them, then directly use dtc to compile the found dts'es... On the other hand I
> can just compile all dts'es, then compare old and new ones. The diff of the
> non-modified dtb'es will be just empty...
make dtbs
touch your dts or git stash pop
make dtbs
compare
diff for all unchanged will be simply empty, so easy to spot
> 2) What crosc64 is?
Ah, just an alias for cross compiling + ccache + kbuild out. I just
copied you my helpers, so you need to tweak them.
>
> >
> > 2. Split it per arm architectures (and proper subject prefix - not
> > "arch") and subarchitectures so maintainers can pick it up.
>
> Why? The changes are simple and can be formatted as a single patch. I've seen
> tons of patches submitted like that, accepted and then merged. What you
> suggest
> is just much more work, which I don't see quite required.
DTS changes go separate between arm64 and arm. There is nothing
unusual here - all changes are submitted like this.
Second topic is to split by subarchitectures which is necessary if you
want it to be picked up by maintainers. It also makes it easier to
review. Sure, without split ber subarchitectures this could be picked
up by SoC folks but you did not even CC them. So if you do not want to
split it per subarchitectures for maintainers and you do not CC SoC,
then how do you believe this should be picked up? Out of the regular
patch submission way? That's not how the changes are handled.
>
> >
> > 3. The subject title could be more accurate - there is no fix here
> > because there was no errors in the first place. Requirement of DWC
> > node names comes recently, so it is more alignment with dtschema.
> > Otherwise automatic-pickup-stable-bot might want to pick up... and it
> > should not go to stable.
>
> Actually it is a fix, because the USB DT nodes should have been nam
Re: [PATCH 11/20] dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string
On Wed, Oct 14, 2020 at 01:13:53PM +0300, Serge Semin wrote: > The DWC USB3 driver and some DTS files like Exynos 5250, Keystone k2e, etc > expects the DWC USB3 DT node to have the compatible string with the > "synopsys" vendor prefix. Let's add the corresponding compatible string to > the controller DT schema, but mark it as deprecated seeing the Synopsys, > Inc. is presented with just "snps" vendor prefix. Instead of adding deprecated schema just correct the DTSes to use snps. The "synopsys" is not even in vendor prefixes. Best regards, Krzysztof
Re: [PATCH v4 00/13] mm/debug_vm_pgtable fixes
On Wed, 14 Oct 2020 08:45:16 +0530 "Aneesh Kumar K.V"
wrote:
> > Against mm-debug_vm_pgtable-avoid-none-pte-in-pte_clear_test.patch:
> >
> > https://lkml.kernel.org/r/87zh5wx51b@linux.ibm.com
>
>
> yes this one we should get fixed. I was hoping someone familiar with
> Riscv pte updates rules would pitch in. IIUC we need to update
> RANDON_ORVALUE similar to how we updated it for s390 and ppc64.
>
>
> Alternatively we can do this
>
> modified mm/debug_vm_pgtable.c
> @@ -548,7 +548,7 @@ static void __init pte_clear_tests(struct mm_struct
> *mm, pte_t *ptep,
> pte_t pte = pfn_pte(pfn, prot);
>
> pr_debug("Validating PTE clear\n");
> - pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
> +// pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
> set_pte_at(mm, vaddr, ptep, pte);
> barrier();
> pte_clear(mm, vaddr, ptep);
>
> till we get that feedback from RiscV team?
Would it be better to do
#ifdef CONFIG_S390
pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
#endif
?
[PATCH 15/20] dt-bindings: usb: meson-g12a-usb: Validate DWC2/DWC3 sub-nodes
Amlogic G12A USB DT sub-nodes are supposed to be compatible with the generic DWC USB2 and USB3 devices. Since now we've got DT schemas for both of the later IP cores let's make sure that the Amlogic G12A USB DT nodes are fully evaluated including the DWC sub-nodes. Signed-off-by: Serge Semin Reviewed-by: Neil Armstrong --- Changelog v2: - Use "oneOf: [dwc2.yaml#, snps,dwc3.yaml#]" instead of the bulky "if: properties: compatibe: ..." statement. --- .../devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml index a4b44a16aaef..7b2dc905c8ce 100644 --- a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml +++ b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml @@ -78,7 +78,9 @@ properties: patternProperties: "^usb@[0-9a-f]+$": -type: object +oneOf: + - $ref: dwc2.yaml# + - $ref: snps,dwc3.yaml# additionalProperties: false -- 2.27.0
[PATCH 02/20] dt-bindings: usb: usb-hcd: Add "otg-rev" property restriction
There are only four OTG revisions are currently supported by the kernel: 0x0100, 0x0120, 0x0130, 0x0200. Any another value is considered as invalid. Signed-off-by: Serge Semin --- Documentation/devicetree/bindings/usb/usb-hcd.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml b/Documentation/devicetree/bindings/usb/usb-hcd.yaml index ee7ea205c71d..e01d8a54971e 100644 --- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml +++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml @@ -60,6 +60,7 @@ properties: features (HNP/SRP/ADP) is enabled. If ADP is required, otg-rev should be 0x0200 or above. $ref: /schemas/types.yaml#/definitions/uint32 +enum: [0x0100, 0x0120, 0x0130, 0x0200] companion: description: Phandle of a companion device -- 2.27.0
[PATCH 04/20] dt-bindings: usb: usb-hcd: Add "tpl-support" property
The host controller device might be designed to work for the particular
products or applications. In that case its DT node is supposed to be
equipped with the tpl-support property.
Signed-off-by: Serge Semin
---
Changelog v2:
- Grammar fix: "s/it'/its"
- Discard '|' from the property description, since we don't need to preserve
the text formatting.
---
Documentation/devicetree/bindings/usb/usb-hcd.yaml | 6 ++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml
b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
index a1a6cde7327d..1f9b40fdea70 100644
--- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
@@ -101,6 +101,12 @@ properties:
enum: [host, peripheral]
default: peripheral
+ tpl-support:
+description:
+ Indicates if the Targeted Peripheral List is supported for given
+ targeted hosts (non-PC hosts).
+type: boolean
+
examples:
- |
usb {
--
2.27.0
[PATCH 06/20] dt-bindings: usb: Convert xHCI bindings to DT schema
Currently the DT bindings of Generic xHCI Controllers are described by means
of the legacy text file. Since such format is deprecated in favor of the
DT schema, let's convert the Generic xHCI Controllers bindings file to the
corresponding yaml files. There will be two of them: a DT schema for the
xHCI controllers on a generic platform and a DT schema validating a generic
xHCI controllers properties. The later will be used to validate the xHCI
controllers, which aside from some vendor-specific features support the
basic xHCI functionality.
An xHCI-compatible DT node shall support the standard USB HCD properties
and custom ones like: usb2-lpm-disable, usb3-lpm-capable,
quirk-broken-port-ped and imod-interval-ns. In addition if a generic xHCI
controller is being validated against the DT schema it is also supposed to
be equipped with mandatory compatible string, single registers range,
single interrupts source, and is supposed to optionally contain up to two
reference clocks for the controller core and CSRs.
Signed-off-by: Serge Semin
---
Changelog v2:
- Add explicit "additionalProperties: true" to the usb-xhci.yaml schema,
since additionalProperties/unevaluatedProperties are going to be mandary
for each binding.
---
.../devicetree/bindings/usb/generic-xhci.yaml | 63 +++
.../devicetree/bindings/usb/usb-xhci.txt | 41
.../devicetree/bindings/usb/usb-xhci.yaml | 42 +
3 files changed, 105 insertions(+), 41 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/generic-xhci.yaml
delete mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.txt
create mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.yaml
diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml
b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
new file mode 100644
index ..1ea1d49a8175
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/generic-xhci.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: USB xHCI Controller Device Tree Bindings
+
+maintainers:
+ - Mathias Nyman
+
+allOf:
+ - $ref: "usb-xhci.yaml#"
+
+properties:
+ compatible:
+oneOf:
+ - description: Generic xHCI device
+const: generic-xhci
+ - description: Armada 37xx/375/38x/8k SoCs
+items:
+ - enum:
+ - marvell,armada3700-xhci
+ - marvell,armada-375-xhci
+ - marvell,armada-380-xhci
+ - marvell,armada-8k-xhci
+ - const: generic-xhci
+ - description: Broadcom STB SoCs with xHCI
+const: brcm,bcm7445-xhci
+ - description: Generic xHCI device
+const: xhci-platform
+deprecated: true
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+minItems: 1
+maxItems: 2
+
+ clock-names:
+minItems: 1
+items:
+ - const: core
+ - const: reg
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+examples:
+ - |
+usb@f0931000 {
+ compatible = "generic-xhci";
+ reg = <0xf0931000 0x8c8>;
+ interrupts = <0x0 0x4e 0x0>;
+};
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
deleted file mode 100644
index 0c5cff84a969..
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ /dev/null
@@ -1,41 +0,0 @@
-USB xHCI controllers
-
-Required properties:
- - compatible: should be one or more of
-
-- "generic-xhci" for generic XHCI device
-- "marvell,armada3700-xhci" for Armada 37xx SoCs
-- "marvell,armada-375-xhci" for Armada 375 SoCs
-- "marvell,armada-380-xhci" for Armada 38x SoCs
-- "brcm,bcm7445-xhci" for Broadcom STB SoCs with XHCI
-- "xhci-platform" (deprecated)
-
-When compatible with the generic version, nodes must list the
-SoC-specific version corresponding to the platform first
-followed by the generic version.
-
- - reg: should contain address and length of the standard XHCI
-register set for the device.
- - interrupts: one XHCI interrupt should be described here.
-
-Optional properties:
- - clocks: reference to the clocks
- - clock-names: mandatory if there is a second clock, in this case
-the name must be "core" for the first clock and "reg" for the
-second one
- - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
- - usb3-lpm-capable: determines if platform is USB3 LPM capable
- - quirk-broken-port-ped: set if the controller has broken port disable
mechanism
- - imod-interval-ns: default interrupt moderation interval is 5000ns
- - phys : see usb-hcd.yaml in the current directory
-
-additionally the properties from usb-hcd.yaml (in the current directory) are
-supported.
-
-
-Example:
- usb@f0931000 {
- compatible
[PATCH 16/20] dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node
TI Keystone DWC3 compatible DT node is supposed to have a DWC USB3 compatible sub-node to describe a fully functioning USB interface. Since DWC USB3 has now got a DT schema describing its DT node, let's make sure the TI Keystone DWC3 sub-node passes validation against it. Signed-off-by: Serge Semin --- Changelog v2: - Grammar fix: "s/it'/its" --- Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml index c1b19fc5d0a2..ca7fbe3ed22e 100644 --- a/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/ti,keystone-dwc3.yaml @@ -64,9 +64,7 @@ properties: patternProperties: "usb@[a-f0-9]+$": -type: object -description: This is the node representing the DWC3 controller instance - Documentation/devicetree/bindings/usb/dwc3.txt +$ref: snps,dwc3.yaml# required: - compatible -- 2.27.0
[PATCH 13/20] dt-bindings: usb: dwc3: Add Frame Length Adj restrictions
In accordance with the IP core databook the snps,quirk-frame-length-adjustment property can be set within [0, 0x3F]. Let's make sure the DT schema applies a correct restriction on the property. Signed-off-by: Serge Semin --- Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml index 307affb669aa..eafd4be03058 100644 --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml @@ -226,6 +226,8 @@ properties: length adjustment when the fladj_30mhz_sdbnd signal is invalid or incorrect. $ref: /schemas/types.yaml#/definitions/uint32 +minimum: 0 +maximum: 0x3f snps,rx-thr-num-pkt-prd: description: -- 2.27.0
[PATCH 09/20] dt-bindings: usb: Convert DWC USB3 bindings to DT schema
DWC USB3 DT node is supposed to be compliant with the Generic xHCI Controller schema, but with additional vendor-specific properties, the controller-specific reference clocks and PHYs. So let's convert the currently available legacy text-based DWC USB3 bindings to the DT schema and make sure the DWC USB3 nodes are also validated against the usb-xhci.yaml schema. Note we have to discard the nodename restriction of being prefixed with "dwc3@" string, since in accordance with the usb-hcd.yaml schema USB nodes are supposed to be named as "^usb(@.*)". Signed-off-by: Serge Semin --- Changelog v2: - Discard '|' from the descriptions, since we don't need to preserve the text formatting in any of them. - Drop quotes from around the string constants. - Fix the "clock-names" prop description to be referring the enumerated clock-names instead of the ones from the Databook. --- .../devicetree/bindings/usb/dwc3.txt | 125 .../devicetree/bindings/usb/snps,dwc3.yaml| 295 ++ 2 files changed, 295 insertions(+), 125 deletions(-) delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt create mode 100644 Documentation/devicetree/bindings/usb/snps,dwc3.yaml diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt b/Documentation/devicetree/bindings/usb/dwc3.txt deleted file mode 100644 index d03edf9d3935.. --- a/Documentation/devicetree/bindings/usb/dwc3.txt +++ /dev/null @@ -1,125 +0,0 @@ -synopsys DWC3 CORE - -DWC3- USB3 CONTROLLER. Complies to the generic USB binding properties - as described in 'usb/generic.txt' - -Required properties: - - compatible: must be "snps,dwc3" - - reg : Address and length of the register set for the device - - interrupts: Interrupts used by the dwc3 controller. - - clock-names: list of clock names. Ideally should be "ref", -"bus_early", "suspend" but may be less or more. - - clocks: list of phandle and clock specifier pairs corresponding to - entries in the clock-names property. - -Exception for clocks: - clocks are optional if the parent node (i.e. glue-layer) is compatible to - one of the following: -"cavium,octeon-7130-usb-uctl" -"qcom,dwc3" -"samsung,exynos5250-dwusb3" -"samsung,exynos5433-dwusb3" -"samsung,exynos7-dwusb3" -"sprd,sc9860-dwc3" -"st,stih407-dwc3" -"ti,am437x-dwc3" -"ti,dwc3" -"ti,keystone-dwc3" -"rockchip,rk3399-dwc3" -"xlnx,zynqmp-dwc3" - -Optional properties: - - usb-phy : array of phandle for the PHY device. The first element - in the array is expected to be a handle to the USB2/HS PHY and - the second element is expected to be a handle to the USB3/SS PHY - - phys: from the *Generic PHY* bindings - - phy-names: from the *Generic PHY* bindings; supported names are "usb2-phy" - or "usb3-phy". - - resets: set of phandle and reset specifier pairs - - snps,usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM - - snps,usb3_lpm_capable: determines if platform is USB3 LPM capable - - snps,dis-start-transfer-quirk: when set, disable isoc START TRANSFER command - failure SW work-around for DWC_usb31 version 1.70a-ea06 - and prior. - - snps,disable_scramble_quirk: true when SW should disable data scrambling. - Only really useful for FPGA builds. - - snps,has-lpm-erratum: true when DWC3 was configured with LPM Erratum enabled - - snps,lpm-nyet-threshold: LPM NYET threshold - - snps,u2exit_lfps_quirk: set if we want to enable u2exit lfps quirk - - snps,u2ss_inp3_quirk: set if we enable P3 OK for U2/SS Inactive quirk - - snps,req_p1p2p3_quirk: when set, the core will always request for - P1/P2/P3 transition sequence. - - snps,del_p1p2p3_quirk: when set core will delay P1/P2/P3 until a certain - amount of 8B10B errors occur. - - snps,del_phy_power_chg_quirk: when set core will delay PHY power change - from P0 to P1/P2/P3. - - snps,lfps_filter_quirk: when set core will filter LFPS reception. - - snps,rx_detect_poll_quirk: when set core will disable a 400us delay to start - Polling LFPS after RX.Detect. - - snps,tx_de_emphasis_quirk: when set core will set Tx de-emphasis value. - - snps,tx_de_emphasis: the value driven to the PHY is controlled by the - LTSSM during USB3 Compliance mode. - - snps,dis_u3_susphy_quirk: when set core will disable USB3 suspend phy. - - snps,dis_u2_susphy_quirk: when set core will disable USB2 suspend phy. - - snps,dis_enblslpm_quirk: when set clears the enblslpm in GUSB2PHYCFG, - disabling the suspend signal to the PHY. - - snps,dis-u1-entry-quirk: set if link entering into U1 needs to be disabled. - - snps,dis-u2-entry-quirk: set if link entering into U2 needs to be disabled. - - snps,dis_rxdet_inp3_quirk: when set core will disable receiver detection - in PHY P3 power state. - - sn
[PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
In accordance with the Generic EHCI/OHCI bindings the corresponding node
name is suppose to comply with the Generic USB HCD DT schema, which
requires the USB nodes to have the name acceptable by the regexp:
"^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with
incompatible names.
Signed-off-by: Serge Semin
---
Please, test the patch out to make sure it doesn't brake the dependent DTS
files. I did only a manual grepping of the possible nodes dependencies.
---
arch/arc/boot/dts/axs10x_mb.dtsi | 4 ++--
arch/arc/boot/dts/hsdk.dts | 4 ++--
arch/arc/boot/dts/vdk_axs10x_mb.dtsi | 2 +-
arch/arm/boot/dts/bcm5301x.dtsi| 4 ++--
arch/arm/boot/dts/bcm53573.dtsi| 4 ++--
arch/arm/boot/dts/hisi-x5hd2.dtsi | 4 ++--
arch/arm/boot/dts/lpc18xx.dtsi | 4 ++--
arch/arm/boot/dts/stm32mp151.dtsi | 4 ++--
arch/arm64/boot/dts/hisilicon/hi3798cv200.dtsi | 4 ++--
arch/arm64/boot/dts/hisilicon/hip06.dtsi | 4 ++--
arch/arm64/boot/dts/hisilicon/hip07.dtsi | 4 ++--
arch/mips/boot/dts/ingenic/jz4740.dtsi | 2 +-
arch/mips/boot/dts/ingenic/jz4770.dtsi | 2 +-
arch/mips/boot/dts/mti/sead3.dts | 2 +-
arch/mips/boot/dts/ralink/mt7628a.dtsi | 2 +-
arch/powerpc/boot/dts/akebono.dts | 6 +++---
16 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi
index 99d3e7175bf7..b64435385304 100644
--- a/arch/arc/boot/dts/axs10x_mb.dtsi
+++ b/arch/arc/boot/dts/axs10x_mb.dtsi
@@ -87,13 +87,13 @@ gmac: ethernet@18000 {
mac-address = [00 00 00 00 00 00]; /* Filled in by
U-Boot */
};
- ehci@4 {
+ usb@4 {
compatible = "generic-ehci";
reg = < 0x4 0x100 >;
interrupts = < 8 >;
};
- ohci@6 {
+ usb@6 {
compatible = "generic-ohci";
reg = < 0x6 0x100 >;
interrupts = < 8 >;
diff --git a/arch/arc/boot/dts/hsdk.dts b/arch/arc/boot/dts/hsdk.dts
index dcaa44e408ac..fdd4f7f635d3 100644
--- a/arch/arc/boot/dts/hsdk.dts
+++ b/arch/arc/boot/dts/hsdk.dts
@@ -234,7 +234,7 @@ phy0: ethernet-phy@0 { /* Micrel KSZ9031 */
};
};
- ohci@6 {
+ usb@6 {
compatible = "snps,hsdk-v1.0-ohci", "generic-ohci";
reg = <0x6 0x100>;
interrupts = <15>;
@@ -242,7 +242,7 @@ ohci@6 {
dma-coherent;
};
- ehci@4 {
+ usb@4 {
compatible = "snps,hsdk-v1.0-ehci", "generic-ehci";
reg = <0x4 0x100>;
interrupts = <15>;
diff --git a/arch/arc/boot/dts/vdk_axs10x_mb.dtsi
b/arch/arc/boot/dts/vdk_axs10x_mb.dtsi
index cbb179770293..90a412026e64 100644
--- a/arch/arc/boot/dts/vdk_axs10x_mb.dtsi
+++ b/arch/arc/boot/dts/vdk_axs10x_mb.dtsi
@@ -46,7 +46,7 @@ ethernet@18000 {
clock-names = "stmmaceth";
};
- ehci@4 {
+ usb@4 {
compatible = "generic-ehci";
reg = < 0x4 0x100 >;
interrupts = < 8 >;
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index 0016720ce530..bf5656d79a55 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -261,7 +261,7 @@ usb2: usb2@21000 {
interrupt-parent = <&gic>;
- ehci: ehci@21000 {
+ ehci: usb@21000 {
#usb-cells = <0>;
compatible = "generic-ehci";
@@ -283,7 +283,7 @@ ehci_port2: port@2 {
};
};
- ohci: ohci@22000 {
+ ohci: usb@22000 {
#usb-cells = <0>;
compatible = "generic-ohci";
diff --git a/arch/arm/boot/dts/bcm53573.dtsi b/arch/arm/boot/dts/bcm53573.dtsi
index 4af8e3293cff..51546fccc616 100644
--- a/arch/arm/boot/dts/bcm53573.dtsi
+++ b/arch/arm/boot/dts/bcm53573.dtsi
@@ -135,7 +135,7 @@ usb2: usb2@4000 {
#address-cells = <1>;
#size-cells = <1>;
- ehci: ehci@4000 {
+ ehci: usb@4000 {
compatible = "generic-ehci";
reg = <0x4000 0x1000>;
interrupt-parent = <&gic>;
@@ -155,7 +155,7 @@ ehci_port2: port@2 {
[PATCH 00/20] dt-bindings: usb: Add generic USB HCD, xHCI, DWC USB3 DT schema
We've performed some work on the Generic USB HCD, xHCI and DWC USB3 DT bindings in the framework of the Baikal-T1 SoC support integration into the kernel. This patchset is a result of that work. First of all we moved the generic USB properties from the legacy text bindings into the USB HCD DT schema. So now the generic USB HCD-compatible DT nodes are validated taking into account the optional properties like: maximum-speed, dr_mode, otg-rev, usb-role-switch, etc. We've fixed these properties a bit so they would correspond to what functionality kernel currently supports. Secondly we converted generic USB xHCI text bindings file into the DT schema. It had to be split up into two bindings: DT schema with generic xHCI properties and a generic xHCI device DT schema. The later will be used to validate the pure xHCI-based nodes, while the former can be utilized by some vendor-specific versions of xHCI. Thirdly, what was primarily intended to be done for Baikal-T1 SoC USB we converted the legacy text-based DWC USB3 bindings to DT schema and altered the result a bit so it would be more coherent with what actually controller and its driver support. Since we've now got the DWC USB3 DT schema, we made it used to validate the sub-nodes of the Qualcom, TI and Amlogic DWC3 DT nodes. Finally we've also fixed all the OHCI/EHCI, xHCI and DW USB3 compatible DT nodes so they would comply with the nodes naming scema declared in the USB HCD DT bindings file. Link: https://lore.kernel.org/linux-usb/20201010224121.12672-1-sergey.se...@baikalelectronics.ru/ Changelog v2: - Thanks to Sergei Shtylyov for suggesting the commit logs grammar fixes: [PATCH 04/18] dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types [PATCH 05/18] dt-bindings: usb: usb-hcd: Add "tpl-support" property [PATCH 11/18] dt-bindings: usb: dwc3: Add interrupt-names property support [PATCH 13/18] dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions [PATCH 17/18] dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node - Set FL-adj of the amlogiv,meson-g12a-usb controller with value 0x20 instead of completely removing the property. - Drop the patch: [PATCH 02/18] dt-bindings: usb: usb-hcd: Add "wireless" maximum-speed property value since "wireless" speed type is depracated due to lack of the device supporting it. - Drop quotes from around the compat string constant. - Discard '|' from the property descriptions, since we don't need to preserve the text formatting. - Convert abbreviated form of the "maximum-speed" enum constraint into the multi-lined version of the list. - Fix the DW USB3 "clock-names" prop description to be refererring to the enumerated clock-names instead of the ones from the Databook. - Add explicit "additionalProperties: true" to the usb-xhci.yaml schema, since additionalProperties/unevaluatedProperties are going to be mandary for each binding. - Use "oneOf: [dwc2.yaml#, snps,dwc3.yaml#]" instead of the bulky "if: properties: compatibe: ..." statement. - Discard the "^dwc3@[0-9a-f]+$" nodes from being acceptable as sub-nodes of the Qualcomm DWC3 DT nodes. - Add new patches: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name [PATCH 19/20] arch: dts: Fix xHCI DT nodes name [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name Signed-off-by: Serge Semin Cc: Alexey Malahov Cc: Pavel Parkhomenko Cc: Andy Gross Cc: Bjorn Andersson Cc: Manu Gautam Cc: Roger Quadros Cc: Lad Prabhakar Cc: Yoshihiro Shimoda Cc: Neil Armstrong Cc: Kevin Hilman Cc: linux-arm-ker...@lists.infradead.org Cc: linux-snps-...@lists.infradead.org Cc: linux-m...@vger.kernel.org Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: devicet...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Serge Semin (20): dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema dt-bindings: usb: usb-hcd: Add "otg-rev" property restriction dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types dt-bindings: usb: usb-hcd: Add "tpl-support" property dt-bindings: usb: usb-hcd: Add generic "usb-phy" property dt-bindings: usb: Convert xHCI bindings to DT schema dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device dt-bindings: usb: renesas-xhci: Refer to the usb-xhci.yaml file dt-bindings: usb: Convert DWC USB3 bindings to DT schema dt-bindings: usb: dwc3: Add interrupt-names property support dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions dt-bindings: usb: dwc3: Add Frame Length Adj restrictions dt-bindings: usb: meson-g12a-usb: Fix FL-adj property value dt-bindings: usb: meson-g12a-usb: Validate DWC2/DWC3 sub-nodes dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node arch: dts: Fix EHCI/OHCI DT nodes name arch: dts: Fix xHCI DT nodes name arch: dts: Fix DWC USB3 DT nodes name .../usb/amlogic,meson-g12a-usb-ctrl.yaml
[PATCH 19/20] arch: dts: Fix xHCI DT nodes name
In accordance with the Generic xHCI bindings the corresponding node name
is suppose to comply with the Generic USB HCD DT schema, which requires
the USB nodes to have the name acceptable by the regexp: "^usb(@.*)?" .
Let's fix the DTS files, which have the xHCI-nodes defined with
incompatible names.
Signed-off-by: Serge Semin
---
Please, test the patch out to make sure it doesn't brake the dependent DTS
files. I did only a manual grepping of the possible nodes dependencies.
---
arch/arm/boot/dts/bcm5301x.dtsi | 2 +-
arch/arm64/boot/dts/marvell/armada-cp11x.dtsi | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boot/dts/bcm5301x.dtsi b/arch/arm/boot/dts/bcm5301x.dtsi
index bf5656d79a55..cf89a44673c5 100644
--- a/arch/arm/boot/dts/bcm5301x.dtsi
+++ b/arch/arm/boot/dts/bcm5301x.dtsi
@@ -314,7 +314,7 @@ usb3: usb3@23000 {
interrupt-parent = <&gic>;
- xhci: xhci@23000 {
+ xhci: usb@23000 {
#usb-cells = <0>;
compatible = "generic-xhci";
diff --git a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
index 9dcf16beabf5..1e37ae181acf 100644
--- a/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
+++ b/arch/arm64/boot/dts/marvell/armada-cp11x.dtsi
@@ -275,7 +275,7 @@ CP11X_LABEL(thermal): thermal-sensor@70 {
};
};
- CP11X_LABEL(usb3_0): usb3@50 {
+ CP11X_LABEL(usb3_0): usb@50 {
compatible = "marvell,armada-8k-xhci",
"generic-xhci";
reg = <0x50 0x4000>;
@@ -287,7 +287,7 @@ CP11X_LABEL(usb3_0): usb3@50 {
status = "disabled";
};
- CP11X_LABEL(usb3_1): usb3@51 {
+ CP11X_LABEL(usb3_1): usb@51 {
compatible = "marvell,armada-8k-xhci",
"generic-xhci";
reg = <0x51 0x4000>;
--
2.27.0
[PATCH 05/20] dt-bindings: usb: usb-hcd: Add generic "usb-phy" property
Even though the Generic PHY framework is the more preferable way of setting the USB PHY up, there are still many dts-files and DT bindings which rely on having the legacy "usb-phy" specified to attach particular USB PHYs to USB cores. Let's have the "usb-phy" property described in the generic USB HCD binding file so it would be validated against the nodes in which it's specified. Mark the property as deprecated to discourage the developers from using it. Signed-off-by: Serge Semin --- Changelog v2: - Discard '|' from the property description, since we don't need to preserve the text formatting. --- Documentation/devicetree/bindings/usb/usb-hcd.yaml | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml b/Documentation/devicetree/bindings/usb/usb-hcd.yaml index 1f9b40fdea70..264a660dc6ea 100644 --- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml +++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml @@ -22,6 +22,13 @@ properties: description: Name specifier for the USB PHY + usb-phy: +$ref: /schemas/types.yaml#/definitions/phandle-array +description: + List of all the USB PHYs on this HCD to be accepted by the legacy USB + Physical Layer subsystem. +deprecated: true + maximum-speed: description: Tells USB controllers we want to work up to a certain speed. In case this -- 2.27.0
[PATCH 12/20] dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions
In accordance with the driver comments the PIPE3 de-emphasis can be tuned to be either -6dB, -2.5dB or disabled. Let's add the de-emphasis property restriction so the DT schema would make sure the controller DT node is equipped with correct value. Signed-off-by: Serge Semin --- Changelog v2: - Grammar fix: "s/tunned/tuned" - Grammar fix: remove redundant "or" conjunction. --- Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml index 2f746ac64e71..307affb669aa 100644 --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml @@ -145,6 +145,10 @@ properties: The value driven to the PHY is controlled by the LTSSM during USB3 Compliance mode. $ref: /schemas/types.yaml#/definitions/uint8 +enum: + - 0 # -6dB de-emphasis + - 1 # -3.5dB de-emphasis + - 2 # No de-emphasis snps,dis_u3_susphy_quirk: description: When set core will disable USB3 suspend phy -- 2.27.0
[PATCH 17/20] dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node
Qualcomm msm8996/sc7180/sdm845 DWC3 compatible DT nodes are supposed to
have a DWC USB3 compatible sub-node to describe a fully functioning USB
interface. Let's use the available DWC USB3 DT schema to validate the
Qualcomm DWC3 sub-nodes.
Note since the generic DWC USB3 DT node is supposed to be named as generic
USB HCD ("^usb(@.*)?") one we have to accordingly fix the sub-nodes name
regexp and fix the DT node example.
Signed-off-by: Serge Semin
---
Changelog v2:
- Discard the "^dwc3@[0-9a-f]+$" nodes from being acceptable as sub-nodes.
---
Documentation/devicetree/bindings/usb/qcom,dwc3.yaml | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index dac10848dd7f..8f8d781e73a0 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -103,11 +103,8 @@ properties:
# Required child node:
patternProperties:
- "^dwc3@[0-9a-f]+$":
-type: object
-description:
- A child node must exist to represent the core DWC3 IP block
- The content of the node is defined in dwc3.txt.
+ "^usb@[0-9a-f]+$":
+$ref: snps,dwc3.yaml#
required:
- compatible
@@ -160,7 +157,7 @@ examples:
resets = <&gcc GCC_USB30_PRIM_BCR>;
-dwc3@a60 {
+usb@a60 {
compatible = "snps,dwc3";
reg = <0 0x0a60 0 0xcd00>;
interrupts = ;
--
2.27.0
[PATCH 07/20] dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device
For some reason the "brcm,xhci-brcm-v2" compatible string has been missing in the original bindings file. Add it to the Generic xHCI Controllers DT schema since the controller driver expects it to be supported. Signed-off-by: Serge Semin --- Documentation/devicetree/bindings/usb/generic-xhci.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml index 1ea1d49a8175..23d73df96ea3 100644 --- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml +++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml @@ -26,7 +26,9 @@ properties: - marvell,armada-8k-xhci - const: generic-xhci - description: Broadcom STB SoCs with xHCI -const: brcm,bcm7445-xhci +enum: + - brcm,xhci-brcm-v2 + - brcm,bcm7445-xhci - description: Generic xHCI device const: xhci-platform deprecated: true -- 2.27.0
[PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
In accordance with the DWC USB3 bindings the corresponding node name is
suppose to comply with Generic USB HCD DT schema, which requires the USB
nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
the dtbs_check procedure failure. Let's fix the nodes naming to be
compatible with the DWC USB3 DT schema to make dtbs_check happy.
Note we don't change the DWC USB3-compatible nodes names of
arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
in-source comment says that the nodes name need to be preserved as
"^dwusb@.*" for some backward compatibility.
Signed-off-by: Serge Semin
---
Please, test the patch out to make sure it doesn't brake the dependent DTS
files. I did only a manual grepping of the possible nodes dependencies.
---
arch/arm/boot/dts/armada-375.dtsi | 2 +-
arch/arm/boot/dts/exynos5250.dtsi | 2 +-
arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
arch/arm/boot/dts/keystone-k2e.dtsi| 4 ++--
arch/arm/boot/dts/keystone.dtsi| 2 +-
arch/arm/boot/dts/ls1021a.dtsi | 2 +-
arch/arm/boot/dts/omap5-l4.dtsi| 2 +-
arch/arm/boot/dts/stih407-family.dtsi | 2 +-
arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 ++--
arch/arm64/boot/dts/exynos/exynos7.dtsi| 2 +-
arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 4 ++--
arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 +++---
arch/arm64/boot/dts/freescale/fsl-ls1088a.dtsi | 4 ++--
arch/arm64/boot/dts/freescale/fsl-ls208xa.dtsi | 4 ++--
arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 2 +-
arch/arm64/boot/dts/qcom/apq8096-db820c.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/ipq8074.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/msm8996.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/msm8998.dtsi | 2 +-
arch/arm64/boot/dts/qcom/qcs404-evb.dtsi | 2 +-
arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 4 ++--
arch/arm64/boot/dts/qcom/sm8150.dtsi | 2 +-
25 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/arch/arm/boot/dts/armada-375.dtsi
b/arch/arm/boot/dts/armada-375.dtsi
index 9805e507c695..7f2f24a29e6c 100644
--- a/arch/arm/boot/dts/armada-375.dtsi
+++ b/arch/arm/boot/dts/armada-375.dtsi
@@ -426,7 +426,7 @@ usb1: usb@54000 {
status = "disabled";
};
- usb2: usb3@58000 {
+ usb2: usb@58000 {
compatible = "marvell,armada-375-xhci";
reg = <0x58000 0x2>,<0x5b880 0x80>;
interrupts = ;
diff --git a/arch/arm/boot/dts/exynos5250.dtsi
b/arch/arm/boot/dts/exynos5250.dtsi
index e3dbe4166836..ebcf8b40ea81 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -629,7 +629,7 @@ usb_dwc3 {
#size-cells = <1>;
ranges;
- usbdrd_dwc3: dwc3@1200 {
+ usbdrd_dwc3: usb@1200 {
compatible = "synopsys,dwc3";
reg = <0x1200 0x1>;
interrupts = ;
diff --git a/arch/arm/boot/dts/exynos54xx.dtsi
b/arch/arm/boot/dts/exynos54xx.dtsi
index 8aa5117e58ce..339243d19a80 100644
--- a/arch/arm/boot/dts/exynos54xx.dtsi
+++ b/arch/arm/boot/dts/exynos54xx.dtsi
@@ -148,7 +148,7 @@ usbdrd3_0: usb3-0 {
#size-cells = <1>;
ranges;
- usbdrd_dwc3_0: dwc3@1200 {
+ usbdrd_dwc3_0: usb@1200 {
compatible = "snps,dwc3";
reg = <0x1200 0x1>;
interrupts = ;
@@ -170,7 +170,7 @@ usbdrd3_1: usb3-1 {
#size-cells = <1>;
ranges;
- usbdrd_dwc3_1: dwc3@1240 {
+ usbdrd_dwc3_1: usb@1240 {
compatible = "snps,dwc3";
reg = <0x1240 0x1>;
phys = <&usbdrd_phy1 0>, <&usbdrd_phy1 1>;
diff --git a/arch/arm/boot/dts/keystone-k2e.dtsi
b/arch/arm/boot/dts/keystone-k2e.dtsi
index 2d94faf31fab..d625ad10cfad 100644
--- a/arch/arm/boot/dts/keystone-k2e.dtsi
+++ b/arch/arm/boot/dts/keystone-k2e.dtsi
@@ -52,7 +52,7 @@ &soc0 {
usb: usb@268 {
interrupts = ;
- dwc3@269 {
+ usb@269 {
[PATCH 08/20] dt-bindings: usb: renesas-xhci: Refer to the usb-xhci.yaml file
With minor peculiarities (like uploading some vendor-specific firmware) these are just Generic xHCI controllers fully compatible with its properties. Make sure the Renesas USB xHCI DT nodes are also validated against the Generic xHCI DT schema. Signed-off-by: Serge Semin --- Documentation/devicetree/bindings/usb/renesas,usb-xhci.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/renesas,usb-xhci.yaml b/Documentation/devicetree/bindings/usb/renesas,usb-xhci.yaml index add9f7b66da0..4491567152a1 100644 --- a/Documentation/devicetree/bindings/usb/renesas,usb-xhci.yaml +++ b/Documentation/devicetree/bindings/usb/renesas,usb-xhci.yaml @@ -11,7 +11,7 @@ maintainers: - Yoshihiro Shimoda allOf: - - $ref: "usb-hcd.yaml" + - $ref: "usb-xhci.yaml" properties: compatible: @@ -68,7 +68,7 @@ required: - power-domains - resets -additionalProperties: false +unevaluatedProperties: false examples: - | -- 2.27.0
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
>
> Hi Serge,
>
> Serge Semin writes:
> > In accordance with the DWC USB3 bindings the corresponding node name is
> > suppose to comply with Generic USB HCD DT schema, which requires the USB
>
> DWC3 is not a simple HDC, though.
Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
which are tuned by the DWC USB3 driver in the kernel. But after that the
controller is registered as xhci-hcd device so it's serviced by the xHCI driver,
which then registers the HCD device so the corresponding DT node is supposed
to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml
and usb/snps,dwc3,yaml. I've created the later one so to validate the denoted
compatibility.
>
> > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> >
> > Note we don't change the DWC USB3-compatible nodes names of
> > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > in-source comment says that the nodes name need to be preserved as
> > "^dwusb@.*" for some backward compatibility.
>
> interesting, compatibility with what? Some debugfs files, perhaps? :-)
Don't really know.) In my experience the worst type of such compatibility is
connected with some bootloader magic, which may add/remove/modify properties
to nodes with pre-defined names.
-Sergey
>
> In any case, I don't have any problems with this, so I'll let other
> folks comment.
>
> --
> balbi
[PATCH 14/20] dt-bindings: usb: meson-g12a-usb: Fix FL-adj property value
An empty snps,quirk-frame-length-adjustment won't cause any change performed by the driver. Moreover the DT schema validation will fail, since it expects the property being assigned with some value. So set fix the example by setting a valid FL-adj value in accordance with Neil Armstrong comment. Link: https://lore.kernel.org/linux-usb/20201010224121.12672-16-sergey.se...@baikalelectronics.ru/ Signed-off-by: Serge Semin Acked-by: Neil Armstrong --- Note the same problem is in the DT source file arch/arm64/boot/dts/amlogic/meson-g12-common.dtsi . --- .../devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml| 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml index 5b04a7dfa018..a4b44a16aaef 100644 --- a/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml +++ b/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.yaml @@ -209,6 +209,6 @@ examples: interrupts = <30>; dr_mode = "host"; snps,dis_u2_susphy_quirk; - snps,quirk-frame-length-adjustment; + snps,quirk-frame-length-adjustment = <0x20>; }; }; -- 2.27.0
[PATCH 03/20] dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types
Aside from the UTMI+ there are also ULPI, Serial and HSIC PHY types that can be specified in the phy_type HCD property. Add them to the enumeration of the acceptable values. Signed-off-by: Serge Semin --- Changelog v2: - Grammar fix: "s/PHY types can be/PHY types that can be" - Drop quotes from around the string constants. --- Documentation/devicetree/bindings/usb/usb-hcd.yaml | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml b/Documentation/devicetree/bindings/usb/usb-hcd.yaml index e01d8a54971e..a1a6cde7327d 100644 --- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml +++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml @@ -46,11 +46,13 @@ properties: phy_type: description: Tells USB controllers that we want to configure the core to support a - UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is selected. In case - this isn't passed via DT, USB controllers should default to HW - capability. + UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is selected, UTMI+ low + pin interface if ULPI is specified, Serial core/PHY interconnect if + serial is specified and High-Speed Inter-Chip feature if HSIC is + selected. In case this isn't passed via DT, USB controllers should + default to HW capability. $ref: /schemas/types.yaml#/definitions/string -enum: [utmi, utmi_wide] +enum: [utmi, utmi_wide, ulpi, serial, hsic] otg-rev: description: -- 2.27.0
[PATCH 10/20] dt-bindings: usb: dwc3: Add interrupt-names property support
The controller driver supports two types of DWC USB3 devices: with a common interrupt lane and with individual interrupts for each mode. Add support for both these cases to the DWC USB3 DT schema. Signed-off-by: Serge Semin --- Changelog v2: - Grammar fix: "s/both of these cases support/support for both these cases" - Drop quotes from around the string constants. --- Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 11 +++ 1 file changed, 11 insertions(+) diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml index 994813f2e25f..12102a84faf6 100644 --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml @@ -23,9 +23,20 @@ properties: const: snps,dwc3 interrupts: +description: | + It's either a single common DWC3 interrupt (dwc_usb3) or individual + interrupts for the host, gadget and DRD modes. minItems: 1 maxItems: 3 + interrupt-names: +minItems: 1 +maxItems: 3 +oneOf: + - const: dwc_usb3 + - items: + enum: [host, peripheral, otg] + clocks: description: In general the core supports three types of clocks. bus_early is a -- 2.27.0
[PATCH 11/20] dt-bindings: usb: dwc3: Add synopsys, dwc3 compatible string
The DWC USB3 driver and some DTS files like Exynos 5250, Keystone k2e, etc expects the DWC USB3 DT node to have the compatible string with the "synopsys" vendor prefix. Let's add the corresponding compatible string to the controller DT schema, but mark it as deprecated seeing the Synopsys, Inc. is presented with just "snps" vendor prefix. Signed-off-by: Serge Semin --- Changelog v2: - Drop quotes from around the compat string constant. --- Documentation/devicetree/bindings/usb/snps,dwc3.yaml | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml index 12102a84faf6..2f746ac64e71 100644 --- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml +++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml @@ -20,7 +20,10 @@ allOf: properties: compatible: contains: - const: snps,dwc3 + oneOf: +- const: snps,dwc3 +- const: synopsys,dwc3 + deprecated: true interrupts: description: | -- 2.27.0
[PATCH 01/20] dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema
The generic USB HCD properties have been described in the legacy bindings
text file: Documentation/devicetree/bindings/usb/generic.txt . Let's
convert it' content into the USB HCD DT schema properties so all USB DT
nodes would be validated to have them properly utilized.
Signed-off-by: Serge Semin
---
Changelog v2:
- Discard '|' in all the new properties, since we don't need to preserve
the text formatting.
- Convert abbreviated form of the "maximum-speed" enum restriction into
the multi-lined version of the list.
- Drop quotes from around the string constants.
---
.../devicetree/bindings/usb/generic.txt | 57
.../devicetree/bindings/usb/usb-hcd.yaml | 88 +++
2 files changed, 88 insertions(+), 57 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/usb/generic.txt
diff --git a/Documentation/devicetree/bindings/usb/generic.txt
b/Documentation/devicetree/bindings/usb/generic.txt
deleted file mode 100644
index ba472e7aefc9..
--- a/Documentation/devicetree/bindings/usb/generic.txt
+++ /dev/null
@@ -1,57 +0,0 @@
-Generic USB Properties
-
-Optional properties:
- - maximum-speed: tells USB controllers we want to work up to a certain
- speed. Valid arguments are "super-speed-plus",
- "super-speed", "high-speed", "full-speed" and
- "low-speed". In case this isn't passed via DT, USB
- controllers should default to their maximum HW
- capability.
- - dr_mode: tells Dual-Role USB controllers that we want to work on a
- particular mode. Valid arguments are "host",
- "peripheral" and "otg". In case this attribute isn't
- passed via DT, USB DRD controllers should default to
- OTG.
- - phy_type: tells USB controllers that we want to configure the core to
support
- a UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is
- selected. Valid arguments are "utmi" and "utmi_wide".
- In case this isn't passed via DT, USB controllers should
- default to HW capability.
- - otg-rev: tells usb driver the release number of the OTG and EH supplement
- with which the device and its descriptors are compliant,
- in binary-coded decimal (i.e. 2.0 is 0200H). This
- property is used if any real OTG features(HNP/SRP/ADP)
- is enabled, if ADP is required, otg-rev should be
- 0x0200 or above.
- - companion: phandle of a companion
- - hnp-disable: tells OTG controllers we want to disable OTG HNP, normally HNP
- is the basic function of real OTG except you want it
- to be a srp-capable only B device.
- - srp-disable: tells OTG controllers we want to disable OTG SRP, SRP is
- optional for OTG device.
- - adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
- optional for OTG device.
- - usb-role-switch: boolean, indicates that the device is capable of assigning
- the USB data role (USB host or USB device) for a given
- USB connector, such as Type-C, Type-B(micro).
- see connector/usb-connector.yaml.
- - role-switch-default-mode: indicating if usb-role-switch is enabled, the
- device default operation mode of controller while usb
- role is USB_ROLE_NONE. Valid arguments are "host" and
- "peripheral". Defaults to "peripheral" if not
- specified.
-
-
-This is an attribute to a USB controller such as:
-
-dwc3@4a03 {
- compatible = "synopsys,dwc3";
- reg = <0x4a03 0xcfff>;
- interrupts = <0 92 4>
- usb-phy = <&usb2_phy>, <&usb3,phy>;
- maximum-speed = "super-speed";
- dr_mode = "otg";
- phy_type = "utmi_wide";
- otg-rev = <0x0200>;
- adp-disable;
-};
diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml
b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
index 7263b7f2b510..ee7ea205c71d 100644
--- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
@@ -22,9 +22,97 @@ properties:
description:
Name specifier for the USB PHY
+ maximum-speed:
+ description:
+ Tells USB controllers we want to work up to a certain speed. In case this
+ isn't passed via DT, USB controllers should default to their maximum HW
+ capability.
+ $ref: /schemas/types.yaml#/definitions/string
+ enum:
+ - low-speed
+ - full-speed
+ - high-speed
+ - super-speed
+ - super-speed-plus
+
+ dr_mode:
+description:
+ Tells Dual-Role USB controllers that we want to work on a particula
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> On Wed, 14 Oct 2020 at 12:23, Serge Semin
> wrote:
> >
> > In accordance with the DWC USB3 bindings the corresponding node name is
> > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a lot
> > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will cause
> > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> >
> > Note we don't change the DWC USB3-compatible nodes names of
> > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > in-source comment says that the nodes name need to be preserved as
> > "^dwusb@.*" for some backward compatibility.
> >
> > Signed-off-by: Serge Semin
> >
> > ---
> >
> > Please, test the patch out to make sure it doesn't brake the dependent DTS
> > files. I did only a manual grepping of the possible nodes dependencies.
>
> 1. It is you who should compare the decompiled DTS, not us. For example:
> $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
>
> $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> dts-new/${i#dts-old/}.fdt ; done
So basically you suggest first to compile the old and new dts files, then to
de-compile them, then diff old and new fdt's, and visually compare the results.
Personally it isn't that much better than what I did, since each old and new
dtbs will for sure differ due to the node names change suggested in this patch.
So it will lead to the visual debugging too, which isn't that effective. But
your approach is still more demonstrative to make sure that I didn't loose any
nodes redefinition, since in the occasion the old and new de-compiled nodes will
differ not only by the node names but with an additional old named node.
So to speak thanks for suggesting it. I'll try it to validate the proposed
changes.
Two questions:
1) Any advise of a good inliner/command to compile all dtbs at once? Of course I
can get all the updated dtsi'es, then find out all the dts'es which include
them, then directly use dtc to compile the found dts'es... On the other hand I
can just compile all dts'es, then compare old and new ones. The diff of the
non-modified dtb'es will be just empty...
2) What crosc64 is?
>
> 2. Split it per arm architectures (and proper subject prefix - not
> "arch") and subarchitectures so maintainers can pick it up.
Why? The changes are simple and can be formatted as a single patch. I've seen
tons of patches submitted like that, accepted and then merged. What you suggest
is just much more work, which I don't see quite required.
>
> 3. The subject title could be more accurate - there is no fix here
> because there was no errors in the first place. Requirement of DWC
> node names comes recently, so it is more alignment with dtschema.
> Otherwise automatic-pickup-stable-bot might want to pick up... and it
> should not go to stable.
Actually it is a fix, because the USB DT nodes should have been named with "usb"
prefix in the first place. Legacy DWC USB3 bindings didn't define the nodes
naming, but implied to be "usb"-prefixed by the USB HCD schema. The Qualcomm
DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which was
wrong in the first place.
Regarding automatic-pickup-stable-bot if it exists I don't think it scans all
the
emails, but most likely the sta...@vger.kernel.org list only or the emails
having the "Fixes:" tag. If I am wrong please give me a link to the bot sources
or refer to a doc where I can read about the way it works, to take it into
account in future commits. Just to note I submitted patches which did some
fixes,
had the word "fix" in the subject but weren't selected to be backported to the
stable kernel.
Anyway I don't really care that much about the subject text using the word "fix"
or some else. So if you suggest some better alternative, I'd be glad to consider
it.
-Sergey
>
> Best regards,
> Krzysztof
>
> > arch/arm/boot/dts/armada-375.dtsi | 2 +-
> > arch/arm/boot/dts/exynos5250.dtsi | 2 +-
> > arch/arm/boot/dts/exynos54xx.dtsi | 4 ++--
> > arch/arm/boot/dts/keystone-k2e.dtsi| 4 ++--
> > arch/arm/boot/dts/keystone.dtsi| 2 +-
> > arch/arm/boot/dts/ls1021a.dtsi | 2 +-
> > arch/arm/boot/dts/omap5-l4.dtsi| 2 +-
> > arch/arm/boot/dts/stih407-family.dtsi | 2 +-
> > arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 2 +-
> > arch/arm64/boot/dts/exynos/exynos5433.dtsi | 4 ++--
> > arch/arm64/boot/dts/exynos/exynos7.dtsi| 2
Re: [PATCH 04/20] dt-bindings: usb: usb-hcd: Add "tpl-support" property
On Wed, Oct 14, 2020 at 08:27:56AM -0500, Rob Herring wrote: > On Wed, 14 Oct 2020 13:13:46 +0300, Serge Semin wrote: > > The host controller device might be designed to work for the particular > > products or applications. In that case its DT node is supposed to be > > equipped with the tpl-support property. > > > > Signed-off-by: Serge Semin > > > > --- > > > > Changelog v2: > > - Grammar fix: "s/it'/its" > > - Discard '|' from the property description, since we don't need to preserve > > the text formatting. > > --- > > Documentation/devicetree/bindings/usb/usb-hcd.yaml | 6 ++ > > 1 file changed, 6 insertions(+) > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > Traceback (most recent call last): > File "/usr/local/bin/dt-extract-example", line 45, in > binding = yaml.load(open(args.yamlfile, encoding='utf-8').read()) > File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line > 343, in load > return constructor.get_single_data() > File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", > line 111, in get_single_data > node = self.composer.get_single_node() > File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node > File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document > File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node > File "_ruamel_yaml.pyx", line 891, in > _ruamel_yaml.CParser._compose_mapping_node > File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event > ruamel.yaml.scanner.ScannerError: mapping values are not allowed in this > context > in "", line 27, column 14 > make[1]: *** [Documentation/devicetree/bindings/Makefile:20: > Documentation/devicetree/bindings/usb/usb-hcd.example.dts] Error 1 > make[1]: *** Deleting file > 'Documentation/devicetree/bindings/usb/usb-hcd.example.dts' > make[1]: *** Waiting for unfinished jobs > ./Documentation/devicetree/bindings/usb/usb-hcd.yaml:27:14: [error] syntax > error: mapping values are not allowed here (syntax) > make[1]: *** [Documentation/devicetree/bindings/Makefile:59: > Documentation/devicetree/bindings/processed-schema-examples.json] Error 123 > make: *** [Makefile:1366: dt_binding_check] Error 2 > > > See https://patchwork.ozlabs.org/patch/1382001 > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure dt-schema is up to date: > > pip3 install git+https://github.com/devicetree-org/dt-schema.git@master > --upgrade > > Please check and re-submit. Hm, that's weird. Of course I did the dt_binding_check before submission, but even after the dt-schema repo update I failed to see the error: $ make -j8 ARCH=mips CROSS_COMPILE=mipsel-baikal-linux- dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/usb/usb-hcd.yaml CHKDT Documentation/devicetree/bindings/usb/usb-hcd.yaml SCHEMA Documentation/devicetree/bindings/processed-schema-examples.yaml DTC Documentation/devicetree/bindings/usb/usb-hcd.example.dt.yaml CHECK Documentation/devicetree/bindings/usb/usb-hcd.example.dt.yaml Rob, any idea why has the bot got mad at me? -Sergey >
Re: [PATCH 09/20] dt-bindings: usb: Convert DWC USB3 bindings to DT schema
On Wed, Oct 14, 2020 at 08:32:19AM -0500, Rob Herring wrote: > On Wed, 14 Oct 2020 13:13:51 +0300, Serge Semin wrote: > > DWC USB3 DT node is supposed to be compliant with the Generic xHCI > > Controller schema, but with additional vendor-specific properties, the > > controller-specific reference clocks and PHYs. So let's convert the > > currently available legacy text-based DWC USB3 bindings to the DT schema > > and make sure the DWC USB3 nodes are also validated against the > > usb-xhci.yaml schema. > > > > Note we have to discard the nodename restriction of being prefixed with > > "dwc3@" string, since in accordance with the usb-hcd.yaml schema USB nodes > > are supposed to be named as "^usb(@.*)". > > > > Signed-off-by: Serge Semin > > > > --- > > > > Changelog v2: > > - Discard '|' from the descriptions, since we don't need to preserve > > the text formatting in any of them. > > - Drop quotes from around the string constants. > > - Fix the "clock-names" prop description to be referring the enumerated > > clock-names instead of the ones from the Databook. > > --- > > .../devicetree/bindings/usb/dwc3.txt | 125 > > .../devicetree/bindings/usb/snps,dwc3.yaml| 295 ++ > > 2 files changed, 295 insertions(+), 125 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/usb/dwc3.txt > > create mode 100644 Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > ./Documentation/devicetree/bindings/usb/snps,dwc3.yaml:44:4: [warning] wrong > indentation: expected 4 but found 3 (indentation) > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/qcom,dwc3.example.dt.yaml: > dwc3@a60: $nodename:0: 'dwc3@a60' does not match '^usb(@.*)?' > From schema: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/amlogic,meson-g12a-usb-ctrl.example.dt.yaml: > usb@ff50: snps,quirk-frame-length-adjustment: True is not of type 'array' > From schema: > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/usb/snps,dwc3.yaml > > > See https://patchwork.ozlabs.org/patch/1382003 > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure dt-schema is up to date: > > pip3 install git+https://github.com/devicetree-org/dt-schema.git@master > --upgrade > > Please check and re-submit. > Both of these errors are fixed in the following patches of the series: [PATCH 17/20] dt-bindings: usb: qcom,dwc3: Validate DWC3 sub-node [PATCH 15/20] dt-bindings: usb: meson-g12a-usb: Fix FL-adj property value This patch preserves the original legacy bindings and doesn't touch the depended bogus DT schemas. -Sergey
Re: [PATCH 00/20] dt-bindings: usb: Add generic USB HCD, xHCI, DWC USB3 DT schema
Ah, forgot to mark the series as v2. Sorry about that. The next one will be v3 then... -Sergey On Wed, Oct 14, 2020 at 01:13:42PM +0300, Serge Semin wrote: > We've performed some work on the Generic USB HCD, xHCI and DWC USB3 DT > bindings in the framework of the Baikal-T1 SoC support integration into > the kernel. This patchset is a result of that work. > > First of all we moved the generic USB properties from the legacy text > bindings into the USB HCD DT schema. So now the generic USB HCD-compatible > DT nodes are validated taking into account the optional properties like: > maximum-speed, dr_mode, otg-rev, usb-role-switch, etc. We've fixed these > properties a bit so they would correspond to what functionality kernel > currently supports. > > Secondly we converted generic USB xHCI text bindings file into the DT > schema. It had to be split up into two bindings: DT schema with generic > xHCI properties and a generic xHCI device DT schema. The later will be > used to validate the pure xHCI-based nodes, while the former can be > utilized by some vendor-specific versions of xHCI. > > Thirdly, what was primarily intended to be done for Baikal-T1 SoC USB we > converted the legacy text-based DWC USB3 bindings to DT schema and altered > the result a bit so it would be more coherent with what actually > controller and its driver support. Since we've now got the DWC USB3 DT > schema, we made it used to validate the sub-nodes of the Qualcom, TI and > Amlogic DWC3 DT nodes. > > Finally we've also fixed all the OHCI/EHCI, xHCI and DW USB3 compatible DT > nodes so they would comply with the nodes naming scema declared in the USB > HCD DT bindings file. > > Link: > https://lore.kernel.org/linux-usb/20201010224121.12672-1-sergey.se...@baikalelectronics.ru/ > Changelog v2: > - Thanks to Sergei Shtylyov for suggesting the commit logs grammar fixes: > [PATCH 04/18] dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types > [PATCH 05/18] dt-bindings: usb: usb-hcd: Add "tpl-support" property > [PATCH 11/18] dt-bindings: usb: dwc3: Add interrupt-names property support > [PATCH 13/18] dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions > [PATCH 17/18] dt-bindings: usb: keystone-dwc3: Validate DWC3 sub-node > - Set FL-adj of the amlogiv,meson-g12a-usb controller with value 0x20 instead > of completely removing the property. > - Drop the patch: > [PATCH 02/18] dt-bindings: usb: usb-hcd: Add "wireless" maximum-speed > property value > since "wireless" speed type is depracated due to lack of the device > supporting it. > - Drop quotes from around the compat string constant. > - Discard '|' from the property descriptions, since we don't need to preserve > the text formatting. > - Convert abbreviated form of the "maximum-speed" enum constraint into > the multi-lined version of the list. > - Fix the DW USB3 "clock-names" prop description to be refererring to the > enumerated clock-names instead of the ones from the Databook. > - Add explicit "additionalProperties: true" to the usb-xhci.yaml schema, > since additionalProperties/unevaluatedProperties are going to be mandary > for each binding. > - Use "oneOf: [dwc2.yaml#, snps,dwc3.yaml#]" instead of the bulky "if: > properties: compatibe: ..." statement. > - Discard the "^dwc3@[0-9a-f]+$" nodes from being acceptable as sub-nodes > of the Qualcomm DWC3 DT nodes. > - Add new patches: > [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name > [PATCH 19/20] arch: dts: Fix xHCI DT nodes name > [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name > > Signed-off-by: Serge Semin > Cc: Alexey Malahov > Cc: Pavel Parkhomenko > Cc: Andy Gross > Cc: Bjorn Andersson > Cc: Manu Gautam > Cc: Roger Quadros > Cc: Lad Prabhakar > Cc: Yoshihiro Shimoda > Cc: Neil Armstrong > Cc: Kevin Hilman > Cc: linux-arm-ker...@lists.infradead.org > Cc: linux-snps-...@lists.infradead.org > Cc: linux-m...@vger.kernel.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-...@vger.kernel.org > Cc: devicet...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org > > Serge Semin (20): > dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema > dt-bindings: usb: usb-hcd: Add "otg-rev" property restriction > dt-bindings: usb: usb-hcd: Add "ulpi/serial/hsic" PHY types > dt-bindings: usb: usb-hcd: Add "tpl-support" property > dt-bindings: usb: usb-hcd: Add generic "usb-phy" property > dt-bindings: usb: Convert xHCI bindings to DT schema > dt-bindings: usb: xhci: Add Broadcom STB v2 compatible device > dt-bindings: usb: renesas-xhci: Refer to the usb-xhci.yaml file > dt-bindings: usb: Convert DWC USB3 bindings to DT schema > dt-bindings: usb: dwc3: Add interrupt-names property support > dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string > dt-bindings: usb: dwc3: Add Tx De-emphasis restrictions > dt-bindings: usb: dwc3: Add Frame Length Adj restrictions > dt-bindings: usb: meson-g12a-usb: Fix FL-adj pr
Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote: > On 10/14/20 3:14 AM, Serge Semin wrote: > > In accordance with the Generic EHCI/OHCI bindings the corresponding node > > name is suppose to comply with the Generic USB HCD DT schema, which > > requires the USB nodes to have the name acceptable by the regexp: > > "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > > incompatible names. > > > > Signed-off-by: Serge Semin > > > > --- > > > > Please, test the patch out to make sure it doesn't brake the dependent DTS > > files. I did only a manual grepping of the possible nodes dependencies. > > Not sure how you envisioned these change to be picked up, but you may > need to split these changes between ARM/ARM64, MIPS and PowerPC at > least. And within ARM/ARM64 you will most likely have to split according > to the various SoC maintainers. Hmm, I don't really know how it's going to be done in this case, but there must be a way to get the cross-platform patches picked up in general. For instance, see the patches like: 714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread() 140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS They touched the files from different files, but still have been merged in. Maybe I should have copied these three patches to the "linux-a...@vger.kernel.org" list or some other mailing list... -Sergey > -- > Florian
Re: [PATCH 18/20] arch: dts: Fix EHCI/OHCI DT nodes name
On Wed, Oct 14, 2020 at 11:41:17AM -0700, Florian Fainelli wrote: > On 10/14/20 11:11 AM, Serge Semin wrote: > > On Wed, Oct 14, 2020 at 11:00:45AM -0700, Florian Fainelli wrote: > >> On 10/14/20 3:14 AM, Serge Semin wrote: > >>> In accordance with the Generic EHCI/OHCI bindings the corresponding node > >>> name is suppose to comply with the Generic USB HCD DT schema, which > >>> requires the USB nodes to have the name acceptable by the regexp: > >>> "^usb(@.*)?" . Let's fix the DTS files, which have the nodes defined with > >>> incompatible names. > >>> > >>> Signed-off-by: Serge Semin > >>> > >>> --- > >>> > >>> Please, test the patch out to make sure it doesn't brake the dependent DTS > >>> files. I did only a manual grepping of the possible nodes dependencies. > >> > > > >> Not sure how you envisioned these change to be picked up, but you may > >> need to split these changes between ARM/ARM64, MIPS and PowerPC at > >> least. And within ARM/ARM64 you will most likely have to split according > >> to the various SoC maintainers. > > > > Hmm, I don't really know how it's going to be done in this case, but there > > must > > be a way to get the cross-platform patches picked up in general. For > > instance, see the patches like: > > 714acdbd1c94 arch: rename copy_thread_tls() back to copy_thread() > > 140c8180eb7c arch: remove HAVE_COPY_THREAD_TLS > > They touched the files from different files, but still have been merged in. > > That situation is different, when a new facility is added and someone > has gone through the work of adding support for all architectures (or > nearly all of them), you want them to be merged in a way that limits > merge conflicts with other architecture changes. > > Here you are fixing warnings, and each file you touch can clearly be > independently change from other files in the series without causing > merge conflicts. You are however creating the potential for merge > conflicts with other changes that the various SoC maintainers have > queued up. > > > Maybe I should have copied these three patches to the > > "linux-a...@vger.kernel.org" > > list or some other mailing list... > > Maybe Rob can queue them through his device tree repository, with the > ack of the various SoC maintainers... That's what I hoped for in the first place. But AFAICS now Rob does the splitting of his patches himself, while the repo is used either for the Documentation/devicetree/ patches or for the Rob'es own work. So it seems to me I'll have to split the series up and resubmit... ( Hope I am wrong. So, @Rob, will you be able to merge this and the next two patches in via your repo or you'd rather suggest for me to split it up and resubmit? -Sergey > -- > Florian
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Wed, Oct 14, 2020 at 01:35:16PM -0500, Rob Herring wrote:
> On Wed, Oct 14, 2020 at 9:37 AM Serge Semin
> wrote:
> >
> > On Wed, Oct 14, 2020 at 05:09:37PM +0300, Felipe Balbi wrote:
> > >
> > > Hi Serge,
> > >
> > > Serge Semin writes:
> > > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > >
> >
> > > DWC3 is not a simple HDC, though.
> >
> > Yeah, strictly speaking it is equipped with a lot of vendor-specific stuff,
> > which are tuned by the DWC USB3 driver in the kernel. But after that the
> > controller is registered as xhci-hcd device so it's serviced by the xHCI
> > driver,
> > which then registers the HCD device so the corresponding DT node is supposed
> > to be compatible with the next bindings: usb/usb-hcd.yaml, usb/usb-xhci.yaml
> > and usb/snps,dwc3,yaml. I've created the later one so to validate the
> > denoted
> > compatibility.
> >
> > >
> > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a
> > > > lot
> > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will
> > > > cause
> > > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > > >
> > > > Note we don't change the DWC USB3-compatible nodes names of
> > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > > in-source comment says that the nodes name need to be preserved as
> > > > "^dwusb@.*" for some backward compatibility.
> > >
> >
> > > interesting, compatibility with what? Some debugfs files, perhaps? :-)
> >
> > Don't really know.) In my experience the worst type of such compatibility is
> > connected with some bootloader magic, which may add/remove/modify properties
> > to nodes with pre-defined names.
>
> I seriously doubt anyone is using the APM machines with DT (even ACPI
> is somewhat doubtful). I say change them. Or remove the dts files and
> see what happens. Either way it can always be reverted.
Ok. I'll change them in v3.
-Sergey
>
> Rob
Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
On 10/13/20 3:16 AM, Catalin Marinas wrote: > On Mon, Oct 12, 2020 at 01:14:50PM -0600, Khalid Aziz wrote: >> On 10/12/20 11:22 AM, Catalin Marinas wrote: >>> On Mon, Oct 12, 2020 at 11:03:33AM -0600, Khalid Aziz wrote: On 10/10/20 5:09 AM, Catalin Marinas wrote: > On Wed, Oct 07, 2020 at 02:14:09PM -0600, Khalid Aziz wrote: >> On 10/7/20 1:39 AM, Jann Horn wrote: >>> arch_validate_prot() is a hook that can validate whether a given set of >>> protection flags is valid in an mprotect() operation. It is given the >>> set >>> of protection flags and the address being modified. >>> >>> However, the address being modified can currently not actually be used >>> in >>> a meaningful way because: >>> >>> 1. Only the address is given, but not the length, and the operation can >>>span multiple VMAs. Therefore, the callee can't actually tell which >>>virtual address range, or which VMAs, are being targeted. >>> 2. The mmap_lock is not held, meaning that if the callee were to check >>>the VMA at @addr, that VMA would be unrelated to the one the >>>operation is performed on. >>> >>> Currently, custom arch_validate_prot() handlers are defined by >>> arm64, powerpc and sparc. >>> arm64 and powerpc don't care about the address range, they just check >>> the >>> flags against CPU support masks. >>> sparc's arch_validate_prot() attempts to look at the VMA, but doesn't >>> take >>> the mmap_lock. >>> >>> Change the function signature to also take a length, and move the >>> arch_validate_prot() call in mm/mprotect.c down into the locked region. > [...] >> As Chris pointed out, the call to arch_validate_prot() from do_mmap2() >> is made without holding mmap_lock. Lock is not acquired until >> vm_mmap_pgoff(). This variance is uncomfortable but I am more >> uncomfortable forcing all implementations of validate_prot to require >> mmap_lock be held when non-sparc implementations do not have such need >> yet. Since do_mmap2() is in powerpc specific code, for now this patch >> solves a current problem. > > I still think sparc should avoid walking the vmas in > arch_validate_prot(). The core code already has the vmas, though not > when calling arch_validate_prot(). That's one of the reasons I added > arch_validate_flags() with the MTE patches. For sparc, this could be > (untested, just copied the arch_validate_prot() code): I am little uncomfortable with the idea of validating protection bits inside the VMA walk loop in do_mprotect_pkey(). When ADI is being enabled across multiple VMAs and arch_validate_flags() fails on a VMA later, do_mprotect_pkey() will bail out with error leaving ADI enabled on earlier VMAs. This will apply to protection bits other than ADI as well of course. This becomes a partial failure of mprotect() call. I think it should be all or nothing with mprotect() - when one calls mprotect() from userspace, either the entire address range passed in gets its protection bits updated or none of it does. That requires validating protection bits upfront or undoing what earlier iterations of VMA walk loop might have done. >>> >>> I thought the same initially but mprotect() already does this with the >>> VM_MAY* flag checking. If you ask it for an mprotect() that crosses >>> multiple vmas and one of them fails, it doesn't roll back the changes to >>> the prior ones. I considered that a similar approach is fine for MTE >>> (it's most likely a user error). >> >> You are right about the current behavior with VM_MAY* flags, but that is >> not the right behavior. Adding more cases to this just perpetuates >> incorrect behavior. It is not easy to roll back changes after VMAs have >> potentially been split/merged which is probably why the current code >> simply throws in the towel and returns with partially modified address >> space. It is lot easier to do all the checks upfront and then proceed or >> not proceed with modifying VMAs. One approach might be to call >> arch_validate_flags() in a loop before modifying VMAs and walk all VMAs >> with a read lock held. Current code also bails out with ENOMEM if it >> finds a hole in the address range and leaves any modifications already >> made in place. This is another case where a hole could have been >> detected earlier. > > This should be ideal indeed though with the risk of breaking the current > ABI (FWIW, FreeBSD seems to do a first pass to check for violations: > https://github.com/freebsd/freebsd/blob/master/sys/vm/vm_map.c#L2630). I am not sure I understand where the ABI breakage would be. Are we aware of apps that intentionally modify address space partially using the current code? What FreeBSD does seems like a reasonable thing to do. Any way first thing to do is to update sparc to use arch_validate_flags() and update spa
Re: [PATCH 11/20] dt-bindings: usb: dwc3: Add synopsys,dwc3 compatible string
On Wed, Oct 14, 2020 at 10:18:18PM +0200, Krzysztof Kozlowski wrote: > On Wed, Oct 14, 2020 at 01:13:53PM +0300, Serge Semin wrote: > > The DWC USB3 driver and some DTS files like Exynos 5250, Keystone k2e, etc > > expects the DWC USB3 DT node to have the compatible string with the > > "synopsys" vendor prefix. Let's add the corresponding compatible string to > > the controller DT schema, but mark it as deprecated seeing the Synopsys, > > Inc. is presented with just "snps" vendor prefix. > > Instead of adding deprecated schema just correct the DTSes to use snps. > The "synopsys" is not even in vendor prefixes. Yeah, it's not, but the driver and some dts'es use it this way. I am not sure that the solution suggested by you is much better than mine. So let's hear the Rob'es opinion out in this matter. @Rob, what do you think? -Sergey > > Best regards, > Krzysztof
Re: [PATCH -next] Revert "powerpc/pci: unmap legacy INTx interrupts when a PHB is removed"
On Thu, Oct 15, 2020 at 5:28 AM Qian Cai wrote: > > This reverts commit 3a3181e16fbde752007759f8759d25e0ff1fc425 which > causes memory corruptions on POWER9 NV. I was going to post this along with a fix for Cedric's original bug, but I can do that separately so: Acked-by: Oliver O'Halloran
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Wed, Oct 14, 2020 at 10:04:32PM +0200, Krzysztof Kozlowski wrote:
> On Wed, 14 Oct 2020 at 19:16, Serge Semin
> wrote:
> >
> > On Wed, Oct 14, 2020 at 12:33:25PM +0200, Krzysztof Kozlowski wrote:
> > > On Wed, 14 Oct 2020 at 12:23, Serge Semin
> > > wrote:
> > > >
> > > > In accordance with the DWC USB3 bindings the corresponding node name is
> > > > suppose to comply with Generic USB HCD DT schema, which requires the USB
> > > > nodes to have the name acceptable by the regexp: "^usb(@.*)?" . But a
> > > > lot
> > > > of the DWC USB3-compatible nodes defined in the ARM/ARM64 DTS files have
> > > > name as "^dwc3@.*" or "^usb[1-3]@.*" or even "^dwusb@.*", which will
> > > > cause
> > > > the dtbs_check procedure failure. Let's fix the nodes naming to be
> > > > compatible with the DWC USB3 DT schema to make dtbs_check happy.
> > > >
> > > > Note we don't change the DWC USB3-compatible nodes names of
> > > > arch/arm64/boot/dts/apm/{apm-storm.dtsi,apm-shadowcat.dtsi} since the
> > > > in-source comment says that the nodes name need to be preserved as
> > > > "^dwusb@.*" for some backward compatibility.
> > > >
> > > > Signed-off-by: Serge Semin
> > > >
> > > > ---
> > > >
> > > > Please, test the patch out to make sure it doesn't brake the dependent
> > > > DTS
> > > > files. I did only a manual grepping of the possible nodes dependencies.
> > >
> >
> > > 1. It is you who should compare the decompiled DTS, not us. For example:
> > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > > scripts/dtc/dtx_diff ${i} dts-new/${i#dts-old/} ; done
> > >
> > > $ for i in dts-old/*/*dtb dts-old/*/*/*dtb; do echo $i; crosc64
> > > fdtdump ${i} > ${i}.fdt ; crosc64 fdtdump dts-new/${i#dts-old/} >
> > > dts-new/${i#dts-old/}.fdt ; diff -ubB ${i}.fdt
> > > dts-new/${i#dts-old/}.fdt ; done
> >
> > So basically you suggest first to compile the old and new dts files, then to
> > de-compile them, then diff old and new fdt's, and visually compare the
> > results.
> > Personally it isn't that much better than what I did, since each old and new
> > dtbs will for sure differ due to the node names change suggested in this
> > patch.
> > So it will lead to the visual debugging too, which isn't that effective. But
> > your approach is still more demonstrative to make sure that I didn't loose
> > any
> > nodes redefinition, since in the occasion the old and new de-compiled nodes
> > will
> > differ not only by the node names but with an additional old named node.
>
> My suggestion is to compare the entire, effective DTS after all
> inclusions. Maybe you did it already, I don't know.
Only by grepping the dts'es, which include the dtsi'es modified in this patch.
So your suggestion of compiling and de-compiling has been indeed relevant.
> The point is that
> when you change node names in DTSI but you miss one in DTS, you end up
> with two nodes.
Yep, that's exactly what I meant when I said that your approach was more
demonstrative, etc.
> This is much easier to spot with dtxdiff or with
> fdtdump (which behaves better for node moves).
>
> Indeed it is still a visual comparison - if you have any ideas how to
> automate it (e.g. ignore phandle changes), please share. It would
> solve my testings as well.
Alas I don't. That's why to save my time of coming up with an automated solution
I did a very thorough modification making sure that each affected node isn't
updated in the corresponding dts'es and asked to test the patches out.
Anyway the approach suggested by you will indeed give us a better understanding
of my patches correctness. So I'll use it before sending v3. Thanks.
> But asking others to test because you do
> not want to check it is not the best way to handle such changes.
>
> >
> > So to speak thanks for suggesting it. I'll try it to validate the proposed
> > changes.
> >
> > Two questions:
> > 1) Any advise of a good inliner/command to compile all dtbs at once? Of
> > course I
> > can get all the updated dtsi'es, then find out all the dts'es which include
> > them, then directly use dtc to compile the found dts'es... On the other
> > hand I
> > can just compile all dts'es, then compare old and new ones. The diff of the
> > non-modified dtb'es will be just empty...
>
> make dtbs
It's not that easy.) "make dtbs" will build dtbs only for enabled boards, which
first need to be enabled in the kernel config. So I'll need to have a config
with all the affected dts. The later is the same as if I just found all the
affected dts and built them one-by-one by directly calling dtc.
> touch your dts or git stash pop
> make dtbs
> compare
> diff for all unchanged will be simply empty, so easy to spot
>
> > 2) What crosc64 is?
>
> Ah, just an alias for cross compiling + ccache + kbuild out. I just
> copied you my helpers, so you need to tweak them.
>
> >
> > >
> > > 2. Split it per arm architectures (and proper subject prefix - not
> > > "arch") and subarchitectures so maintainers can pick it
Re: [PATCH -next] Revert "powerpc/pci: unmap legacy INTx interrupts when a PHB is removed"
Hi Oliver, On Thu, 15 Oct 2020 10:00:49 +1100 "Oliver O'Halloran" wrote: > > On Thu, Oct 15, 2020 at 5:28 AM Qian Cai wrote: > > > > This reverts commit 3a3181e16fbde752007759f8759d25e0ff1fc425 which > > causes memory corruptions on POWER9 NV. > > I was going to post this along with a fix for Cedric's original bug, > but I can do that separately so: > > Acked-by: Oliver O'Halloran I added that to linux-next today. -- Cheers, Stephen Rothwell pgpp2HQOwZOCj.pgp Description: OpenPGP digital signature
Re: [PATCH v4 00/13] mm/debug_vm_pgtable fixes
On 10/15/2020 02:06 AM, Andrew Morton wrote:
> On Wed, 14 Oct 2020 08:45:16 +0530 "Aneesh Kumar K.V"
> wrote:
>
>>> Against mm-debug_vm_pgtable-avoid-none-pte-in-pte_clear_test.patch:
>>>
>>> https://lkml.kernel.org/r/87zh5wx51b@linux.ibm.com
>>
>>
>> yes this one we should get fixed. I was hoping someone familiar with
>> Riscv pte updates rules would pitch in. IIUC we need to update
>> RANDON_ORVALUE similar to how we updated it for s390 and ppc64.
>>
>>
>> Alternatively we can do this
>>
>> modified mm/debug_vm_pgtable.c
>> @@ -548,7 +548,7 @@ static void __init pte_clear_tests(struct mm_struct
>> *mm, pte_t *ptep,
>> pte_t pte = pfn_pte(pfn, prot);
>>
>> pr_debug("Validating PTE clear\n");
>> -pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>> +// pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>> set_pte_at(mm, vaddr, ptep, pte);
>> barrier();
>> pte_clear(mm, vaddr, ptep);
>>
>> till we get that feedback from RiscV team?
>
> Would it be better to do
>
> #ifdef CONFIG_S390
> pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
> #endif
I would suggest just dropping RANDOM_ORVALUE from pte_clear_tests()
possibly with a comment saying it needs to be fixed on RISCV before
being added back later.
pte = __pte(pte_val(pte));
OR
Disable RANDOM_ORVALUE only for RISCV.
#ifndef CONFIG_RISCV
pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
#endif
Re: [PATCH 20/20] arch: dts: Fix DWC USB3 DT nodes name
On Thu, Oct 15, 2020 at 02:51:05AM +0300, Serge Semin wrote: > > > > > So to speak thanks for suggesting it. I'll try it to validate the proposed > > > changes. > > > > > > Two questions: > > > 1) Any advise of a good inliner/command to compile all dtbs at once? Of > > > course I > > > can get all the updated dtsi'es, then find out all the dts'es which > > > include > > > them, then directly use dtc to compile the found dts'es... On the other > > > hand I > > > can just compile all dts'es, then compare old and new ones. The diff of > > > the > > > non-modified dtb'es will be just empty... > > > > > make dtbs > > It's not that easy.) "make dtbs" will build dtbs only for enabled boards, > which > first need to be enabled in the kernel config. So I'll need to have a config > with all the affected dts. The later is the same as if I just found all the > affected dts and built them one-by-one by directly calling dtc. True. Sometimes allyesconfig for given arch might be helpful but not always (e.g. for ARM it does not select all of ARMv4 and ARMv5 boards). Most likely your approach is actually faster/more reliable. > > > touch your dts or git stash pop > > make dtbs > > compare > > diff for all unchanged will be simply empty, so easy to spot > > > > > 2) What crosc64 is? > > > > Ah, just an alias for cross compiling + ccache + kbuild out. I just > > copied you my helpers, so you need to tweak them. > > > > > > > > > > > > > 2. Split it per arm architectures (and proper subject prefix - not > > > > "arch") and subarchitectures so maintainers can pick it up. > > > > > > Why? The changes are simple and can be formatted as a single patch. I've > > > seen > > > tons of patches submitted like that, accepted and then merged. What you > > > suggest > > > is just much more work, which I don't see quite required. > > > > > DTS changes go separate between arm64 and arm. There is nothing > > unusual here - all changes are submitted like this. > > Second topic is to split by subarchitectures which is necessary if you > > want it to be picked up by maintainers. It also makes it easier to > > review. > > The current patches are easy enough for review. The last three patches of the > series is a collection of the one-type changes concerning the same type of > nodes. So reviewing them won't cause any difficulty. But I assume that's not > the main point in this discussion. > > > Sure, without split ber subarchitectures this could be picked > > up by SoC folks but you did not even CC them. So if you do not want to > > split it per subarchitectures for maintainers and you do not CC SoC, > > then how do you believe this should be picked up? Out of the regular > > patch submission way? That's not how the changes are handled. > > AFAIU there are another ways of merging comprehensive patches. If they get to > collect > all the Acked-by tags, they could be merged in, for instance, through Greg' > or Rob' > (for dts) repos, if of course they get to agree with doing that. Am I wrong? > > My hope was to ask Rob or Greg to get the patches merged in when they get > to collect all the ackes, since I thought it was an option in such cases. So > if > they refuse to do so I'll have no choice but to split the series up into a > smaller patches as you say. This is neither Rob's nor Greg's patch to pick up, but ARM SoC (which was not CCed here). And most likely they won't pick it up because judging by contents it is obvious it should go via ARM SoC. Sure, if there are dependencies between some patches they can go with acks through unrelated trees, but this not the usual way. This is an exception in the process to solve particular dependency problem. It has drawbacks - increases the chances of annoying conflicts. The case here does not fall into this criteria - there is no dependency of this patch on the others Therefore there is no reason to use the unusual/exceptional way of handling patches. There is no reason why this shouldn't go via either specific ARM subarchitecture maintainers or via ARM SoC. > > > > 3. The subject title could be more accurate - there is no fix here > > > > because there was no errors in the first place. Requirement of DWC > > > > node names comes recently, so it is more alignment with dtschema. > > > > Otherwise automatic-pickup-stable-bot might want to pick up... and it > > > > should not go to stable. > > > > > > Actually it is a fix, because the USB DT nodes should have been named > > > with "usb" > > > prefix in the first place. Legacy DWC USB3 bindings didn't define the > > > nodes > > > naming, but implied to be "usb"-prefixed by the USB HCD schema. The > > > Qualcomm > > > DWC3 schema should have defined the sub-nodes as "dwc3@"-prefixed, which > > > was > > > wrong in the first place. > > > > > Not following the naming convention of DT spec which was loosely > > enforced is not an error which should be "fixed". Simply wrong title. > > This is an alignment with dtschema or corre
