This series fix up some minor issues found through inspection.
Note that the fourth patch changes which cpu (hart) devicetree nodes
are enabled by following the Linux convention of considering nodes
without a status property as enabled.
These patches are against the riscv-next (and fixes) branch with Andreas
node-reference fix applied:
https://lore.kernel.org/lkml/[email protected]/
and have been tested using QEMU.
Johan
Johan Hovold (5):
riscv: add missing newlines to printk messages
riscv: use pr_info and friends
riscv: fix riscv_of_processor_hartid() comment
riscv: treat cpu devicetree nodes without status as enabled
riscv: use for_each_of_cpu_node iterator
arch/riscv/kernel/cpu.c | 28 ++++++++++++----------------
arch/riscv/kernel/cpufeature.c | 13 +++++++------
arch/riscv/kernel/ftrace.c | 2 +-
arch/riscv/kernel/setup.c | 6 +++---
arch/riscv/kernel/smpboot.c | 4 ++--
5 files changed, 25 insertions(+), 28 deletions(-)
--
2.20.1
The riscv_of_processor_hartid() helper returns -ENODEV when the
specified node isn't an enabled and valid RISC-V hart node.
Also drop the unnecessary parenthesis around errno defines.
Signed-off-by: Johan Hovold <[email protected]>
---
arch/riscv/kernel/cpu.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 11ba67f010e7..974d374fd36b 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -17,8 +17,8 @@
#include <asm/smp.h>
/*
- * Returns the hart ID of the given device tree node, or -1 if the device tree
- * node isn't a RISC-V hart.
+ * Returns the hart ID of the given device tree node, or -ENODEV if the node
+ * isn't an enabled and valid RISC-V hart node.
*/
int riscv_of_processor_hartid(struct device_node *node)
{
@@ -27,34 +27,34 @@ int riscv_of_processor_hartid(struct device_node *node)
if (!of_device_is_compatible(node, "riscv")) {
pr_warn("Found incompatible CPU\n");
- return -(ENODEV);
+ return -ENODEV;
}
if (of_property_read_u32(node, "reg", &hart)) {
pr_warn("Found CPU without hart ID\n");
- return -(ENODEV);
+ return -ENODEV;
}
if (hart >= NR_CPUS) {
pr_info("Found hart ID %d, which is above NR_CPUs. Disabling this hart\n", hart);
- return -(ENODEV);
+ return -ENODEV;
}
if (of_property_read_string(node, "status", &status)) {
pr_warn("CPU with hartid=%d has no \"status\" property\n", hart);
- return -(ENODEV);
+ return -ENODEV;
}
if (strcmp(status, "okay")) {
pr_info("CPU with hartid=%d has a non-okay status of \"%s\"\n", hart, status);
- return -(ENODEV);
+ return -ENODEV;
}
if (of_property_read_string(node, "riscv,isa", &isa)) {
pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n", hart);
- return -(ENODEV);
+ return -ENODEV;
}
if (isa[0] != 'r' || isa[1] != 'v') {
pr_warn("CPU with hartid=%d has an invalid ISA of \"%s\"\n", hart, isa);
- return -(ENODEV);
+ return -ENODEV;
}
return hart;
--
2.20.1
Use the pr_info and pr_err macros instead of printk with explicit log
levels.
Signed-off-by: Johan Hovold <[email protected]>
---
arch/riscv/kernel/setup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 6e079e94b638..5c15bb0560ee 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -76,11 +76,11 @@ static void __init setup_initrd(void)
unsigned long size;
if (initrd_start >= initrd_end) {
- printk(KERN_INFO "initrd not found or empty");
+ pr_info("initrd not found or empty");
goto disable;
}
if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) {
- printk(KERN_ERR "initrd extends beyond end of memory");
+ pr_err("initrd extends beyond end of memory");
goto disable;
}
@@ -88,7 +88,7 @@ static void __init setup_initrd(void)
memblock_reserve(__pa(initrd_start), size);
initrd_below_start_ok = 1;
- printk(KERN_INFO "Initial ramdisk at: 0x%p (%lu bytes)\n",
+ pr_info("Initial ramdisk at: 0x%p (%lu bytes)\n",
(void *)(initrd_start), size);
return;
disable:
--
2.20.1
Follow the Linux convention and treat devicetree nodes without a status
property as enabled rather than disabled, while also allowing "ok" as a
shorthand for "okay".
Signed-off-by: Johan Hovold <[email protected]>
---
arch/riscv/kernel/cpu.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 974d374fd36b..d1d9bfd5a89f 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -22,7 +22,7 @@
*/
int riscv_of_processor_hartid(struct device_node *node)
{
- const char *isa, *status;
+ const char *isa;
u32 hart;
if (!of_device_is_compatible(node, "riscv")) {
@@ -39,12 +39,8 @@ int riscv_of_processor_hartid(struct device_node *node)
return -ENODEV;
}
- if (of_property_read_string(node, "status", &status)) {
- pr_warn("CPU with hartid=%d has no \"status\" property\n", hart);
- return -ENODEV;
- }
- if (strcmp(status, "okay")) {
- pr_info("CPU with hartid=%d has a non-okay status of \"%s\"\n", hart, status);
+ if (!of_device_is_available(node)) {
+ pr_info("CPU with hartid=%d is not available\n", hart);
return -ENODEV;
}
--
2.20.1
Use the new for_each_of_cpu_node() helper to iterate over cpu nodes
instead of open coding. Note that this will allow matching also on the
node name instead of the (for FDT) deprecated device_type property.
Signed-off-by: Johan Hovold <[email protected]>
---
arch/riscv/kernel/cpufeature.c | 5 +++--
arch/riscv/kernel/smpboot.c | 4 ++--
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 4891fd62b95e..e7a4701f0256 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -28,7 +28,7 @@ bool has_fpu __read_mostly;
void riscv_fill_hwcap(void)
{
- struct device_node *node = NULL;
+ struct device_node *node;
const char *isa;
size_t i;
static unsigned long isa2hwcap[256] = {0};
@@ -46,9 +46,10 @@ void riscv_fill_hwcap(void)
* We don't support running Linux on hertergenous ISA systems. For
* now, we just check the ISA of the first "okay" processor.
*/
- while ((node = of_find_node_by_type(node, "cpu")))
+ for_each_of_cpu_node(node) {
if (riscv_of_processor_hartid(node) >= 0)
break;
+ }
if (!node) {
pr_warn("Unable to find \"cpu\" devicetree entry\n");
return;
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 18cda0e8cf94..6e2813257e03 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -50,12 +50,12 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
void __init setup_smp(void)
{
- struct device_node *dn = NULL;
+ struct device_node *dn;
int hart;
bool found_boot_cpu = false;
int cpuid = 1;
- while ((dn = of_find_node_by_type(dn, "cpu"))) {
+ for_each_of_cpu_node(dn) {
hart = riscv_of_processor_hartid(dn);
if (hart < 0)
continue;
--
2.20.1
Add missing newline characters to printk messages.
Also replace two pr_warning with the shorter pr_warn, and fix up the
tense of one error message while at it.
Signed-off-by: Johan Hovold <[email protected]>
---
arch/riscv/kernel/cpu.c | 2 +-
arch/riscv/kernel/cpufeature.c | 8 ++++----
arch/riscv/kernel/ftrace.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index f8fa2c63aa89..11ba67f010e7 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -106,7 +106,7 @@ static void print_isa(struct seq_file *f, const char *orig_isa)
* a bit of info describing what went wrong.
*/
if (isa[0] != '\0')
- pr_info("unsupported ISA \"%s\" in device tree", orig_isa);
+ pr_info("unsupported ISA \"%s\" in device tree\n", orig_isa);
}
static void print_mmu(struct seq_file *f, const char *mmu_type)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index a6e369edbbd7..4891fd62b95e 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -50,12 +50,12 @@ void riscv_fill_hwcap(void)
if (riscv_of_processor_hartid(node) >= 0)
break;
if (!node) {
- pr_warning("Unable to find \"cpu\" devicetree entry");
+ pr_warn("Unable to find \"cpu\" devicetree entry\n");
return;
}
if (of_property_read_string(node, "riscv,isa", &isa)) {
- pr_warning("Unable to find \"riscv,isa\" devicetree entry");
+ pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
of_node_put(node);
return;
}
@@ -67,11 +67,11 @@ void riscv_fill_hwcap(void)
/* We don't support systems with F but without D, so mask those out
* here. */
if ((elf_hwcap & COMPAT_HWCAP_ISA_F) && !(elf_hwcap & COMPAT_HWCAP_ISA_D)) {
- pr_info("This kernel does not support systems with F but not D");
+ pr_info("This kernel does not support systems with F but not D\n");
elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
}
- pr_info("elf_hwcap is 0x%lx", elf_hwcap);
+ pr_info("elf_hwcap is 0x%lx\n", elf_hwcap);
#ifdef CONFIG_FPU
if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index a840b7d074f7..b94d8db5ddcc 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -32,7 +32,7 @@ static int ftrace_check_current_call(unsigned long hook_pos,
* return must be -EINVAL on failed comparison
*/
if (memcmp(expected, replaced, sizeof(replaced))) {
- pr_err("%p: expected (%08x %08x) but get (%08x %08x)",
+ pr_err("%p: expected (%08x %08x) but got (%08x %08x)\n",
(void *)hook_pos, expected[0], expected[1], replaced[0],
replaced[1]);
return -EINVAL;
--
2.20.1
On Fri, 18 Jan 2019, Johan Hovold wrote:
> Use the pr_info and pr_err macros instead of printk with explicit log
> levels.
>
> Signed-off-by: Johan Hovold <[email protected]>
Reviewed-by: Paul Walmsley <[email protected]>
- Paul
On Fri, 18 Jan 2019, Johan Hovold wrote:
> The riscv_of_processor_hartid() helper returns -ENODEV when the
> specified node isn't an enabled and valid RISC-V hart node.
>
> Also drop the unnecessary parenthesis around errno defines.
>
> Signed-off-by: Johan Hovold <[email protected]>
Reviewed-by: Paul Walmsley <[email protected]>
- Paul
On Fri, 18 Jan 2019, Johan Hovold wrote:
> Follow the Linux convention and treat devicetree nodes without a status
> property as enabled rather than disabled, while also allowing "ok" as a
> shorthand for "okay".
>
> Signed-off-by: Johan Hovold <[email protected]>
Reviewed-by: Paul Walmsley <[email protected]>
... although I probably would have phrased the commit message slightly
differently - something like "Use of_device_is_available() in
riscv_of_processor_hartid(), in place of an open-coded test." That frames
the change as one that removes custom code in place of an existing
standard function for the same purpose.
- Paul
On Sat, Jan 19, 2019 at 01:43:58AM +0000, Paul Walmsley wrote:
> On Fri, 18 Jan 2019, Johan Hovold wrote:
>
> > Follow the Linux convention and treat devicetree nodes without a status
> > property as enabled rather than disabled, while also allowing "ok" as a
> > shorthand for "okay".
> >
> > Signed-off-by: Johan Hovold <[email protected]>
>
> Reviewed-by: Paul Walmsley <[email protected]>
>
> ... although I probably would have phrased the commit message slightly
> differently - something like "Use of_device_is_available() in
> riscv_of_processor_hartid(), in place of an open-coded test." That frames
> the change as one that removes custom code in place of an existing
> standard function for the same purpose.
You're right, and that was my motivation too. In the end I decided to
highlight the side effect in case someone was perhaps unknowingly
relying on the current behaviour.
Thanks for the review!
Johan
On Fri, Jan 18, 2019 at 03:03:03PM +0100, Johan Hovold wrote:
> This series fix up some minor issues found through inspection.
>
> Note that the fourth patch changes which cpu (hart) devicetree nodes
> are enabled by following the Linux convention of considering nodes
> without a status property as enabled.
>
> These patches are against the riscv-next (and fixes) branch with Andreas
> node-reference fix applied:
>
> https://lore.kernel.org/lkml/[email protected]/
>
> and have been tested using QEMU.
> Johan Hovold (5):
> riscv: add missing newlines to printk messages
> riscv: use pr_info and friends
> riscv: fix riscv_of_processor_hartid() comment
> riscv: treat cpu devicetree nodes without status as enabled
> riscv: use for_each_of_cpu_node iterator
Are these still in your queue, Palmer?
Note that the node-reference dependency mentioned above is now in
Linus's tree.
Johan
On Mon, 11 Feb 2019 01:34:06 PST (-0800), [email protected] wrote:
> On Fri, Jan 18, 2019 at 03:03:03PM +0100, Johan Hovold wrote:
>> This series fix up some minor issues found through inspection.
>>
>> Note that the fourth patch changes which cpu (hart) devicetree nodes
>> are enabled by following the Linux convention of considering nodes
>> without a status property as enabled.
>>
>> These patches are against the riscv-next (and fixes) branch with Andreas
>> node-reference fix applied:
>>
>> https://lore.kernel.org/lkml/[email protected]/
>>
>> and have been tested using QEMU.
>
>> Johan Hovold (5):
>> riscv: add missing newlines to printk messages
>> riscv: use pr_info and friends
>> riscv: fix riscv_of_processor_hartid() comment
>> riscv: treat cpu devicetree nodes without status as enabled
>> riscv: use for_each_of_cpu_node iterator
>
> Are these still in your queue, Palmer?
>
> Note that the node-reference dependency mentioned above is now in
> Linus's tree.
They're still in my queue. With any luck I'll have time tonight to go through
by Linux patch review backlog.
On Fri, Jan 18, 2019 at 03:03:04PM +0100, Johan Hovold wrote:
> Add missing newline characters to printk messages.
>
> Also replace two pr_warning with the shorter pr_warn, and fix up the
> tense of one error message while at it.
>
> Signed-off-by: Johan Hovold <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
On Fri, Jan 18, 2019 at 03:03:05PM +0100, Johan Hovold wrote:
> Use the pr_info and pr_err macros instead of printk with explicit log
> levels.
>
> Signed-off-by: Johan Hovold <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
On Fri, Jan 18, 2019 at 03:03:07PM +0100, Johan Hovold wrote:
> Follow the Linux convention and treat devicetree nodes without a status
> property as enabled rather than disabled, while also allowing "ok" as a
> shorthand for "okay".
>
> Signed-off-by: Johan Hovold <[email protected]>
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
On Fri, Jan 18, 2019 at 03:03:06PM +0100, Johan Hovold wrote:
> The riscv_of_processor_hartid() helper returns -ENODEV when the
> specified node isn't an enabled and valid RISC-V hart node.
>
> Also drop the unnecessary parenthesis around errno defines.
>
> Signed-off-by: Johan Hovold <[email protected]>
Looks good,
Reviewed-by: Christoph Hellwig <[email protected]>
On Fri, Jan 18, 2019 at 03:03:08PM +0100, Johan Hovold wrote:
> Use the new for_each_of_cpu_node() helper to iterate over cpu nodes
> instead of open coding. Note that this will allow matching also on the
> node name instead of the (for FDT) deprecated device_type property.
>
> Signed-off-by: Johan Hovold <[email protected]>
I think this is going to conflict with the ELF caps changes from
Atish. Maybe the riscv_fill_hwcap hunk should be included in his
patch?
Otherwise looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
On Mon, Feb 11, 2019 at 11:13:39PM -0800, Christoph Hellwig wrote:
> On Fri, Jan 18, 2019 at 03:03:08PM +0100, Johan Hovold wrote:
> > Use the new for_each_of_cpu_node() helper to iterate over cpu nodes
> > instead of open coding. Note that this will allow matching also on the
> > node name instead of the (for FDT) deprecated device_type property.
> >
> > Signed-off-by: Johan Hovold <[email protected]>
>
> I think this is going to conflict with the ELF caps changes from
> Atish. Maybe the riscv_fill_hwcap hunk should be included in his
> patch?
Since that patch had some issues (e.g. the node reference underflow) it
may be better to rebase it on top of this series. The changes are
otherwise distinct after all.
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
Thanks for reviewing!
Johan
On 2/12/19 12:26 AM, Johan Hovold wrote:
> On Mon, Feb 11, 2019 at 11:13:39PM -0800, Christoph Hellwig wrote:
>> On Fri, Jan 18, 2019 at 03:03:08PM +0100, Johan Hovold wrote:
>>> Use the new for_each_of_cpu_node() helper to iterate over cpu nodes
>>> instead of open coding. Note that this will allow matching also on the
>>> node name instead of the (for FDT) deprecated device_type property.
>>>
>>> Signed-off-by: Johan Hovold <[email protected]>
>>
>> I think this is going to conflict with the ELF caps changes from
>> Atish. Maybe the riscv_fill_hwcap hunk should be included in his
>> patch?
>
> Since that patch had some issues (e.g. the node reference underflow) it
> may be better to rebase it on top of this series. The changes are
> otherwise distinct after all.
>
I have fixed the node reference underflow issue and made some more
changes based on comments.
Some other patches(1,3,4,5) in this series may conflict with my series
as well. I can rebase my series on top of yours and send it as one
series if that's okay with you and Palmer.
Regards,
Atish
>> Otherwise looks good:
>>
>> Reviewed-by: Christoph Hellwig <[email protected]>
>
> Thanks for reviewing!
>
> Johan
>
On Tue, Feb 12, 2019 at 12:47:09AM -0800, Atish Patra wrote:
> On 2/12/19 12:26 AM, Johan Hovold wrote:
> > On Mon, Feb 11, 2019 at 11:13:39PM -0800, Christoph Hellwig wrote:
> >> On Fri, Jan 18, 2019 at 03:03:08PM +0100, Johan Hovold wrote:
> >>> Use the new for_each_of_cpu_node() helper to iterate over cpu nodes
> >>> instead of open coding. Note that this will allow matching also on the
> >>> node name instead of the (for FDT) deprecated device_type property.
> >>>
> >>> Signed-off-by: Johan Hovold <[email protected]>
> >>
> >> I think this is going to conflict with the ELF caps changes from
> >> Atish. Maybe the riscv_fill_hwcap hunk should be included in his
> >> patch?
> >
> > Since that patch had some issues (e.g. the node reference underflow) it
> > may be better to rebase it on top of this series. The changes are
> > otherwise distinct after all.
> >
>
> I have fixed the node reference underflow issue and made some more
> changes based on comments.
>
> Some other patches(1,3,4,5) in this series may conflict with my series
> as well. I can rebase my series on top of yours and send it as one
> series if that's okay with you and Palmer.
No need for you to resend these patches as they are reviewed any ready
to be merged. Just rebase your series and mention the dependency in the
cover letter as I did with Andreas's patch which this series depended
on.
Thanks,
Johan
On 2/12/19 12:53 AM, Johan Hovold wrote:
> On Tue, Feb 12, 2019 at 12:47:09AM -0800, Atish Patra wrote:
>> On 2/12/19 12:26 AM, Johan Hovold wrote:
>>> On Mon, Feb 11, 2019 at 11:13:39PM -0800, Christoph Hellwig wrote:
>>>> On Fri, Jan 18, 2019 at 03:03:08PM +0100, Johan Hovold wrote:
>>>>> Use the new for_each_of_cpu_node() helper to iterate over cpu nodes
>>>>> instead of open coding. Note that this will allow matching also on the
>>>>> node name instead of the (for FDT) deprecated device_type property.
>>>>>
>>>>> Signed-off-by: Johan Hovold <[email protected]>
>>>>
>>>> I think this is going to conflict with the ELF caps changes from
>>>> Atish. Maybe the riscv_fill_hwcap hunk should be included in his
>>>> patch?
>>>
>>> Since that patch had some issues (e.g. the node reference underflow) it
>>> may be better to rebase it on top of this series. The changes are
>>> otherwise distinct after all.
>>>
>>
>> I have fixed the node reference underflow issue and made some more
>> changes based on comments.
>>
>> Some other patches(1,3,4,5) in this series may conflict with my series
>> as well. I can rebase my series on top of yours and send it as one
>> series if that's okay with you and Palmer.
>
> No need for you to resend these patches as they are reviewed any ready
> to be merged. Just rebase your series and mention the dependency in the
> cover letter as I did with Andreas's patch which this series depended
> on.
>
Sure. I will do that.
Regards,
Atish
> Thanks,
> Johan
>