I finally got around to answering a very old email in my inbox that was
a response to our original patch set. These are meant to cause little
to no functional changes to the port, but I've given them very little
testing so I wouldn't be surprised if I've managed to screw something
up here.
I'll be sure to give these some testing before putting them on for-next,
but I figured it'd be better to send them out now rather than later.
I'm not sure how I managed to miss this the first time, but this is much
better.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/include/asm/smp.h | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index 36016845461d..27fd085188df 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -14,11 +14,6 @@
#ifndef _ASM_RISCV_SMP_H
#define _ASM_RISCV_SMP_H
-/* This both needs asm-offsets.h and is used when generating it. */
-#ifndef GENERATING_ASM_OFFSETS
-#include <asm/asm-offsets.h>
-#endif
-
#include <linux/cpumask.h>
#include <linux/irqreturn.h>
@@ -33,13 +28,12 @@ void arch_send_call_function_ipi_mask(struct cpumask *mask);
/* Hook for the generic smp_call_function_single() routine. */
void arch_send_call_function_single_ipi(int cpu);
-/*
- * This is particularly ugly: it appears we can't actually get the definition
- * of task_struct here, but we need access to the CPU this task is running on.
- * Instead of using C we're using asm-offsets.h to get the current processor
- * ID.
- */
-#define raw_smp_processor_id() (*((int*)((char*)get_current() + TASK_TI_CPU)))
+/* Obtains the hart ID of the currently executing task. This relies on
+ * THREAD_INFO_IN_TASK, but we define that unconditionally. */
+#ifdef CONFIG_THREAD_INFO_IN_TASK
+#define get_ti() ((struct thread_info *)get_current())
+#define raw_smp_processor_id() (get_ti()->cpu)
+#endif
#endif /* CONFIG_SMP */
--
2.16.4
It's a bit confusing exactly what this function does: it actually
returns the hartid of an OF processor node, failing with -1 on invalid
nodes. I've changed the name to _hartid() in order to make that a bit
more clear, as well as adding a comment.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/include/asm/processor.h | 4 +++-
arch/riscv/kernel/cpu.c | 2 +-
arch/riscv/kernel/smpboot.c | 2 +-
drivers/clocksource/riscv_timer.c | 2 +-
drivers/irqchip/irq-sifive-plic.c | 2 +-
5 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 3fe4af8147d2..9d32670d84a4 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -88,7 +88,9 @@ static inline void wait_for_interrupt(void)
}
struct device_node;
-extern int riscv_of_processor_hart(struct device_node *node);
+/* Returns the hart ID of the given device tree node, or -1 if the device tree
+ * node isn't a RISC-V hart. */
+extern int riscv_of_processor_hartid(struct device_node *node);
extern void riscv_fill_hwcap(void);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index ca6c81e54e37..19e98c1710dd 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -16,7 +16,7 @@
#include <linux/of.h>
/* Return -1 if not a valid hart */
-int riscv_of_processor_hart(struct device_node *node)
+int riscv_of_processor_hartid(struct device_node *node)
{
const char *isa, *status;
u32 hart;
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 56abab6a9812..5f29f8562cf6 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -53,7 +53,7 @@ void __init setup_smp(void)
int hart, im_okay_therefore_i_am = 0;
while ((dn = of_find_node_by_type(dn, "cpu"))) {
- hart = riscv_of_processor_hart(dn);
+ hart = riscv_of_processor_hartid(dn);
if (hart >= 0) {
set_cpu_possible(hart, true);
set_cpu_present(hart, true);
diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
index 4e8b347e43e2..ad7453fc3129 100644
--- a/drivers/clocksource/riscv_timer.c
+++ b/drivers/clocksource/riscv_timer.c
@@ -84,7 +84,7 @@ void riscv_timer_interrupt(void)
static int __init riscv_timer_init_dt(struct device_node *n)
{
- int cpu_id = riscv_of_processor_hart(n), error;
+ int cpu_id = riscv_of_processor_hartid(n), error;
struct clocksource *cs;
if (cpu_id != smp_processor_id())
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
index 532e9d68c704..c55eaa31cde2 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -176,7 +176,7 @@ static int plic_find_hart_id(struct device_node *node)
{
for (; node; node = node->parent) {
if (of_device_is_compatible(node, "riscv"))
- return riscv_of_processor_hart(node);
+ return riscv_of_processor_hartid(node);
}
return -1;
--
2.16.4
The old name was a bit odd.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/kernel/smpboot.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5f29f8562cf6..e1f6a5ad0416 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -50,7 +50,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
void __init setup_smp(void)
{
struct device_node *dn = NULL;
- int hart, im_okay_therefore_i_am = 0;
+ int hart, found_boot_cpu = 0;
while ((dn = of_find_node_by_type(dn, "cpu"))) {
hart = riscv_of_processor_hartid(dn);
@@ -58,13 +58,13 @@ void __init setup_smp(void)
set_cpu_possible(hart, true);
set_cpu_present(hart, true);
if (hart == smp_processor_id()) {
- BUG_ON(im_okay_therefore_i_am);
- im_okay_therefore_i_am = 1;
+ BUG_ON(found_boot_cpu);
+ found_boot_cpu = 1;
}
}
}
- BUG_ON(!im_okay_therefore_i_am);
+ BUG_ON(!found_boot_cpu);
}
int __cpu_up(unsigned int cpu, struct task_struct *tidle)
--
2.16.4
f1f1007644ff ("mm: add new mmgrab() helper") added a helper that we
missed out on.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/kernel/smpboot.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index e1f6a5ad0416..5437a04babcd 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -30,6 +30,7 @@
#include <linux/irq.h>
#include <linux/of.h>
#include <linux/sched/task_stack.h>
+#include <linux/sched/mm.h>
#include <asm/irq.h>
#include <asm/mmu_context.h>
#include <asm/tlbflush.h>
@@ -79,8 +80,8 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
* the spinning harts that they can continue the boot process.
*/
smp_mb();
- __cpu_up_stack_pointer[cpu] = task_stack_page(tidle) + THREAD_SIZE;
- __cpu_up_task_pointer[cpu] = tidle;
+ WRITE_ONCE(__cpu_up_stack_pointer[cpu], task_stack_page(tidle) + THREAD_SIZE);
+ WRITE_ONCE(__cpu_up_task_pointer[cpu], tidle);
while (!cpu_online(cpu))
cpu_relax();
@@ -100,7 +101,7 @@ asmlinkage void __init smp_callin(void)
struct mm_struct *mm = &init_mm;
/* All kernel threads share the same mm context. */
- atomic_inc(&mm->mm_count);
+ mmgrab(mm);
current->active_mm = mm;
trap_init();
--
2.16.4
This isn't readily apparent from reading the code.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/kernel/smpboot.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5437a04babcd..953bc540207d 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -107,6 +107,8 @@ asmlinkage void __init smp_callin(void)
trap_init();
notify_cpu_starting(smp_processor_id());
set_cpu_online(smp_processor_id(), 1);
+ /* Remote TLB flushes are ignored while the CPU is offline, so emit a local
+ * TLB flush right now just in case. */
local_flush_tlb_all();
local_irq_enable();
preempt_disable();
--
2.16.4
I'm not sure, but I think this was a bug: if the scheduler fired right
here then I believe it would blow up.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/kernel/smpboot.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 953bc540207d..45515cc70181 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -110,7 +110,9 @@ asmlinkage void __init smp_callin(void)
/* Remote TLB flushes are ignored while the CPU is offline, so emit a local
* TLB flush right now just in case. */
local_flush_tlb_all();
- local_irq_enable();
+ /* Disable preemption before enabling interrupts, so we don't try to
+ * schedule a CPU that hasn't actually started yet. */
preempt_disable();
+ local_irq_enable();
cpu_startup_entry(CPUHP_AP_ONLINE_IDLE);
}
--
2.16.4
We shouldn't be directly passing device tree values to userspace, both
because there could be mistakes in device trees and because the kernel
doesn't support arbitrary ISAs.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/kernel/cpu.c | 62 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 55 insertions(+), 7 deletions(-)
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 19e98c1710dd..a18b4e3962a1 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -58,6 +58,57 @@ int riscv_of_processor_hartid(struct device_node *node)
#ifdef CONFIG_PROC_FS
+static void print_isa(struct seq_file *f, const char *orig_isa)
+{
+ static const char *ext = "mafdc";
+ const char *isa = orig_isa;
+ const char *e;
+
+ /* Linux doesn't support rv32e or rv128i, and we only support booting
+ * kernels on harts with the same ISA that the kernel is compiled for. */
+#if defined(CONFIG_32BIT)
+ if (strncmp(isa, "rv32i", 5) != 0)
+ return;
+#elif defined(CONFIG_64BIT)
+ if (strncmp(isa, "rv64i", 5) != 0)
+ return;
+#endif
+
+ /* Print the base ISA, as we already know it's legal. */
+ seq_printf(f, "isa\t: ");
+ seq_write(f, isa, 5);
+ isa += 5;
+
+ /* Check the rest of the ISA string for valid extensions, printing those we
+ * find. RISC-V ISA strings define an order, so we only print the
+ * extension bits when they're in order. */
+ for (e = ext; *e != '\0'; ++e) {
+ if (isa[0] == e[0]) {
+ seq_write(f, isa, 1);
+ isa++;
+ }
+ }
+
+ /* If we were given an unsupported ISA in the device tree then print a bit
+ * of info describing what went wrong. */
+ if (isa[0] != '\0')
+ pr_info("unsupported ISA \"%s\" in device tree", orig_isa);
+}
+
+static void print_mmu(struct seq_file *f, const char *mmu_type)
+{
+#if defined(CONFIG_32BIT)
+ if (strcmp(mmu_type, "riscv,sv32") != 0)
+ return;
+#elif defined(CONFIG_64BIT)
+ if ((strcmp(mmu_type, "riscv,sv39") != 0)
+ && (strcmp(mmu_type, "riscv,sv48") != 0))
+ return;
+#endif
+
+ seq_printf(f, "mmu\t: %s\n", mmu_type+6);
+}
+
static void *c_start(struct seq_file *m, loff_t *pos)
{
*pos = cpumask_next(*pos - 1, cpu_online_mask);
@@ -83,13 +134,10 @@ static int c_show(struct seq_file *m, void *v)
const char *compat, *isa, *mmu;
seq_printf(m, "hart\t: %lu\n", hart_id);
- if (!of_property_read_string(node, "riscv,isa", &isa)
- && isa[0] == 'r'
- && isa[1] == 'v')
- seq_printf(m, "isa\t: %s\n", isa);
- if (!of_property_read_string(node, "mmu-type", &mmu)
- && !strncmp(mmu, "riscv,", 6))
- seq_printf(m, "mmu\t: %s\n", mmu+6);
+ if (!of_property_read_string(node, "riscv,isa", &isa))
+ print_isa(m, isa);
+ if (!of_property_read_string(node, "mmu-type", &mmu))
+ print_mmu(m, mmu);
if (!of_property_read_string(node, "compatible", &compat)
&& strcmp(compat, "riscv"))
seq_printf(m, "uarch\t: %s\n", compat);
--
2.16.4
These are just hard coded in the RISC-V port, which doesn't make any
sense. We should probably be setting these from device tree entries
when they exist, but for now I think it's saner to just leave them all
as their default values.
Signed-off-by: Palmer Dabbelt <[email protected]>
---
arch/riscv/kernel/cacheinfo.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
index 0bc86e5f8f3f..cb35ffd8ec6b 100644
--- a/arch/riscv/kernel/cacheinfo.c
+++ b/arch/riscv/kernel/cacheinfo.c
@@ -22,13 +22,6 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
{
this_leaf->level = level;
this_leaf->type = type;
- /* not a sector cache */
- this_leaf->physical_line_partition = 1;
- /* TODO: Add to DTS */
- this_leaf->attributes =
- CACHE_WRITE_BACK
- | CACHE_READ_ALLOCATE
- | CACHE_WRITE_ALLOCATE;
}
static int __init_cache_level(unsigned int cpu)
--
2.16.4
On 8/27/18 11:58 AM, Palmer Dabbelt wrote:
> It's a bit confusing exactly what this function does: it actually
> returns the hartid of an OF processor node, failing with -1 on invalid
> nodes. I've changed the name to _hartid() in order to make that a bit
> more clear, as well as adding a comment.
>
> Signed-off-by: Palmer Dabbelt <[email protected]>
> ---
> arch/riscv/include/asm/processor.h | 4 +++-
> arch/riscv/kernel/cpu.c | 2 +-
> arch/riscv/kernel/smpboot.c | 2 +-
> drivers/clocksource/riscv_timer.c | 2 +-
> drivers/irqchip/irq-sifive-plic.c | 2 +-
> 5 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 3fe4af8147d2..9d32670d84a4 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -88,7 +88,9 @@ static inline void wait_for_interrupt(void)
> }
>
> struct device_node;
> -extern int riscv_of_processor_hart(struct device_node *node);
> +/* Returns the hart ID of the given device tree node, or -1 if the device tree
> + * node isn't a RISC-V hart. */
I think multi-line commenting format should be:
/*
*
*/
Otherwise, Reviewed-by: Atish Patra <[email protected]>
> +extern int riscv_of_processor_hartid(struct device_node *node);
>
> extern void riscv_fill_hwcap(void);
>
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index ca6c81e54e37..19e98c1710dd 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -16,7 +16,7 @@
> #include <linux/of.h>
>
> /* Return -1 if not a valid hart */
> -int riscv_of_processor_hart(struct device_node *node)
> +int riscv_of_processor_hartid(struct device_node *node)
> {
> const char *isa, *status;
> u32 hart;
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 56abab6a9812..5f29f8562cf6 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -53,7 +53,7 @@ void __init setup_smp(void)
> int hart, im_okay_therefore_i_am = 0;
>
> while ((dn = of_find_node_by_type(dn, "cpu"))) {
> - hart = riscv_of_processor_hart(dn);
> + hart = riscv_of_processor_hartid(dn);
> if (hart >= 0) {
> set_cpu_possible(hart, true);
> set_cpu_present(hart, true);
> diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
> index 4e8b347e43e2..ad7453fc3129 100644
> --- a/drivers/clocksource/riscv_timer.c
> +++ b/drivers/clocksource/riscv_timer.c
> @@ -84,7 +84,7 @@ void riscv_timer_interrupt(void)
>
> static int __init riscv_timer_init_dt(struct device_node *n)
> {
> - int cpu_id = riscv_of_processor_hart(n), error;
> + int cpu_id = riscv_of_processor_hartid(n), error;
> struct clocksource *cs;
>
> if (cpu_id != smp_processor_id())
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index 532e9d68c704..c55eaa31cde2 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -176,7 +176,7 @@ static int plic_find_hart_id(struct device_node *node)
> {
> for (; node; node = node->parent) {
> if (of_device_is_compatible(node, "riscv"))
> - return riscv_of_processor_hart(node);
> + return riscv_of_processor_hartid(node);
> }
>
> return -1;
>
On Mon, Aug 27, 2018 at 11:42:36AM -0700, Palmer Dabbelt wrote:
> +/* Obtains the hart ID of the currently executing task. This relies on
> + * THREAD_INFO_IN_TASK, but we define that unconditionally. */
Kernel comment style would be:
/*
* Obtains the hart ID of the currently executing task. This relies on
* THREAD_INFO_IN_TASK, but we define that unconditionally.
*/
> +#ifdef CONFIG_THREAD_INFO_IN_TASK
THREAD_INFO_IN_TASK is always defined for riscv, so no need for this
ifdef.
> +#define get_ti() ((struct thread_info *)get_current())
> +#define raw_smp_processor_id() (get_ti()->cpu)
Please don't cast structs around. This should be something like
#define raw_smp_processor_id() \
(container_of(get_current(), struct task_struct, thread_info)->cpu)
On Mon, Aug 27, 2018 at 11:42:37AM -0700, Palmer Dabbelt wrote:
> These are just hard coded in the RISC-V port, which doesn't make any
> sense. We should probably be setting these from device tree entries
> when they exist, but for now I think it's saner to just leave them all
> as their default values.
>
> Signed-off-by: Palmer Dabbelt <[email protected]>
Looks fine:
Reviewed-by: Christoph Hellwig <[email protected]>
> +/* Returns the hart ID of the given device tree node, or -1 if the device tree
> + * node isn't a RISC-V hart. */
In addition to the comment formatting: we usually keep the description
near the implementation, not the header. It could also become a
kerneldoc comment for added clarity.
> +extern int riscv_of_processor_hartid(struct device_node *node);
No need to declare function prototypes in headers with the extern
attribute, btw.
> struct device_node *dn = NULL;
> - int hart, im_okay_therefore_i_am = 0;
> + int hart, found_boot_cpu = 0;
If you rename this anyway please switch to use a bool.
> +#include <linux/sched/mm.h>
> #include <asm/irq.h>
> #include <asm/mmu_context.h>
> #include <asm/tlbflush.h>
> @@ -79,8 +80,8 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle)
> * the spinning harts that they can continue the boot process.
> */
> smp_mb();
> - __cpu_up_stack_pointer[cpu] = task_stack_page(tidle) + THREAD_SIZE;
> - __cpu_up_task_pointer[cpu] = tidle;
> + WRITE_ONCE(__cpu_up_stack_pointer[cpu], task_stack_page(tidle) + THREAD_SIZE);
> + WRITE_ONCE(__cpu_up_task_pointer[cpu], tidle);
This looks unrelated.
> @@ -100,7 +101,7 @@ asmlinkage void __init smp_callin(void)
> struct mm_struct *mm = &init_mm;
>
> /* All kernel threads share the same mm context. */
> - atomic_inc(&mm->mm_count);
> + mmgrab(mm);
> current->active_mm = mm;
This part looks fine to me:
Reviewed-by: Christoph Hellwig <[email protected]>
> notify_cpu_starting(smp_processor_id());
> set_cpu_online(smp_processor_id(), 1);
> + /* Remote TLB flushes are ignored while the CPU is offline, so emit a local
> + * TLB flush right now just in case. */
Please use the normal kernel comment style and break the line after 80
characters.
On 8/30/18 7:41 AM, Christoph Hellwig wrote:
>> struct device_node *dn = NULL;
>> - int hart, im_okay_therefore_i_am = 0;
>> + int hart, found_boot_cpu = 0;
>
> If you rename this anyway please switch to use a bool.
>
I can address the comment on this patch and fold it in my series to
avoid unnecessary conflict.
@Palmer: You can drop this patch.
Regards,
Atish
Hi,
On 08/27/2018 01:42 PM, Palmer Dabbelt wrote:
> These are just hard coded in the RISC-V port, which doesn't make any
> sense. We should probably be setting these from device tree entries
> when they exist, but for now I think it's saner to just leave them all
> as their default values.
Default value here means unset and not visible in /sys.
Which looks fine to me.
Reviewed-by: Jeremy Linton <[email protected]>
Thanks,
>
> Signed-off-by: Palmer Dabbelt <[email protected]>
> ---
> arch/riscv/kernel/cacheinfo.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c
> index 0bc86e5f8f3f..cb35ffd8ec6b 100644
> --- a/arch/riscv/kernel/cacheinfo.c
> +++ b/arch/riscv/kernel/cacheinfo.c
> @@ -22,13 +22,6 @@ static void ci_leaf_init(struct cacheinfo *this_leaf,
> {
> this_leaf->level = level;
> this_leaf->type = type;
> - /* not a sector cache */
> - this_leaf->physical_line_partition = 1;
> - /* TODO: Add to DTS */
> - this_leaf->attributes =
> - CACHE_WRITE_BACK
> - | CACHE_READ_ALLOCATE
> - | CACHE_WRITE_ALLOCATE;
> }
>
> static int __init_cache_level(unsigned int cpu)
>
On Thu, Aug 30, 2018 at 09:11:11AM -0700, Atish Patra wrote:
> On 8/30/18 7:41 AM, Christoph Hellwig wrote:
> > > struct device_node *dn = NULL;
> > > - int hart, im_okay_therefore_i_am = 0;
> > > + int hart, found_boot_cpu = 0;
> >
> > If you rename this anyway please switch to use a bool.
> >
>
> I can address the comment on this patch and fold it in my series to avoid
> unnecessary conflict.
>
> @Palmer: You can drop this patch.
If Palmer is ok with it how about you include a respin of his whole
series? There are some overlaps I think and having a single person
in charge might be helpful.
Btw, I've found your series in my linux-kernel folder, but it did
not make it to linux-riscv or my personal inbox despite the CC.
On 8/30/18 10:54 PM, Christoph Hellwig wrote:
> On Thu, Aug 30, 2018 at 09:11:11AM -0700, Atish Patra wrote:
>> On 8/30/18 7:41 AM, Christoph Hellwig wrote:
>>>> struct device_node *dn = NULL;
>>>> - int hart, im_okay_therefore_i_am = 0;
>>>> + int hart, found_boot_cpu = 0;
>>>
>>> If you rename this anyway please switch to use a bool.
>>>
>>
>> I can address the comment on this patch and fold it in my series to avoid
>> unnecessary conflict.
>>
>> @Palmer: You can drop this patch.
>
> If Palmer is ok with it how about you include a respin of his whole
> series? There are some overlaps I think and having a single person
> in charge might be helpful.
>
Sure. I am fine with it. Palmer ?
> Btw, I've found your series in my linux-kernel folder, but it did
> not make it to linux-riscv or my personal inbox despite the CC.
>
Thanks for the review.
I checked my scripts. I couldn't figure out what went wrong.
Not sure if there was some issue with infradead.org at that time.
Anyways, sorry again for the mess. I will keep a watch on this next time.
Regards,
Atish
On Fri, 31 Aug 2018 14:18:02 PDT (-0700), [email protected] wrote:
> On 8/30/18 10:54 PM, Christoph Hellwig wrote:
>> On Thu, Aug 30, 2018 at 09:11:11AM -0700, Atish Patra wrote:
>>> On 8/30/18 7:41 AM, Christoph Hellwig wrote:
>>>>> struct device_node *dn = NULL;
>>>>> - int hart, im_okay_therefore_i_am = 0;
>>>>> + int hart, found_boot_cpu = 0;
>>>>
>>>> If you rename this anyway please switch to use a bool.
>>>>
>>>
>>> I can address the comment on this patch and fold it in my series to avoid
>>> unnecessary conflict.
>>>
>>> @Palmer: You can drop this patch.
>>
>> If Palmer is ok with it how about you include a respin of his whole
>> series? There are some overlaps I think and having a single person
>> in charge might be helpful.
>>
>
> Sure. I am fine with it. Palmer ?
Ya, I have no problem with it.
>
>> Btw, I've found your series in my linux-kernel folder, but it did
>> not make it to linux-riscv or my personal inbox despite the CC.
>>
>
> Thanks for the review.
> I checked my scripts. I couldn't figure out what went wrong.
> Not sure if there was some issue with infradead.org at that time.
>
> Anyways, sorry again for the mess. I will keep a watch on this next time.
>
> Regards,
> Atish
On Thu, 30 Aug 2018 09:11:11 PDT (-0700), [email protected] wrote:
> On 8/30/18 7:41 AM, Christoph Hellwig wrote:
>>> struct device_node *dn = NULL;
>>> - int hart, im_okay_therefore_i_am = 0;
>>> + int hart, found_boot_cpu = 0;
>>
>> If you rename this anyway please switch to use a bool.
>>
>
> I can address the comment on this patch and fold it in my series to
> avoid unnecessary conflict.
>
> @Palmer: You can drop this patch.
Thanks!