From: Anup Patel <anup@brainfault.org> To: Atish Patra <atish.patra@wdc.com> Cc: linux-riscv@lists.infradead.org, "Alan Kao" <alankao@andestech.com>, "Albert Ou" <aou@eecs.berkeley.edu>, "Andreas Schwab" <schwab@suse.de>, "Daniel Lezcano" <daniel.lezcano@linaro.org>, "Dmitriy Cherkasov" <dmitriy@oss-tech.org>, "Jason Cooper" <jason@lakedaemon.net>, "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>, "Marc Zyngier" <marc.zyngier@arm.com>, "Michael Clark" <michaeljclark@mac.com>, "Palmer Dabbelt" <palmer@sifive.com>, "Patrick Stählin" <me@packi.ch>, "Thomas Gleixner" <tglx@linutronix.de>, "Zong Li" <zongbox@gmail.com> Subject: Re: [PATCH v2 6/8] RISC-V: Add required checks during clock source init Date: Tue, 8 Jan 2019 17:26:45 +0530 [thread overview] Message-ID: <CAAhSdy0VsOjiSRhBY7fdXVOccWV+4j_UJwKdNX4cNWHBJKyGeA@mail.gmail.com> (raw) In-Reply-To: <1546940318-9752-7-git-send-email-atish.patra@wdc.com> Few nit changes.. Prefer, "clocksource/drivers/riscv:" prefix instead of "RISC-V:" for this patch. On Tue, Jan 8, 2019 at 3:08 PM Atish Patra <atish.patra@wdc.com> wrote: > > Currently, clocksource registration happens for an invalid cpu > for non-smp kernels. This lead to kernel panic as cpu hotplug > registration will fail for those cpus. Moreover, > riscv_hartid_to_cpuid can return errors now. > > Do not proceed if hartid or cpuid is invalid. Take this opprtunity > to print appropriate error strings for different failure cases. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > drivers/clocksource/timer-riscv.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index 43189220..d9b914e9 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -95,14 +95,31 @@ static int __init riscv_timer_init_dt(struct device_node *n) > struct clocksource *cs; > > hartid = riscv_of_processor_hartid(n); > + if (hartid < 0) { > + pr_warn("Not valid hartid for node [%pOF] error = [%d]\n", > + n, hartid); > + return hartid; > + } Add empty line here. > cpuid = riscv_hartid_to_cpuid(hartid); > Remove empty line here > + if (cpuid < 0) { > + pr_warn("Invalid cpuid for hartid [%d]\n", hartid); > + return cpuid; > + } > + > if (cpuid != smp_processor_id()) > return 0; > > + pr_err("%s: Registering clocksource cpuid [%d] hartid [%d]\n", > + __func__, cpuid, hartid); > cs = per_cpu_ptr(&riscv_clocksource, cpuid); > - clocksource_register_hz(cs, riscv_timebase); > + error = clocksource_register_hz(cs, riscv_timebase); > Remove empty line here. > + if (error) { > + pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", > + error, cpuid); > + return error; > + } Add empty line here. > sched_clock_register(riscv_sched_clock, > BITS_PER_LONG, riscv_timebase); > > @@ -110,8 +127,8 @@ static int __init riscv_timer_init_dt(struct device_node *n) > "clockevents/riscv/timer:starting", > riscv_timer_starting_cpu, riscv_timer_dying_cpu); > if (error) > - pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", > - error, cpuid); > + pr_err("cpu hp setup state failed for RISCV timer [%d]\n", > + error); > return error; > } > > -- > 2.7.4 > Apart from above, looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup
WARNING: multiple messages have this Message-ID (diff)
From: Anup Patel <anup@brainfault.org> To: Atish Patra <atish.patra@wdc.com> Cc: "Patrick Stählin" <me@packi.ch>, "Albert Ou" <aou@eecs.berkeley.edu>, "Jason Cooper" <jason@lakedaemon.net>, "Alan Kao" <alankao@andestech.com>, "Dmitriy Cherkasov" <dmitriy@oss-tech.org>, "Andreas Schwab" <schwab@suse.de>, "Daniel Lezcano" <daniel.lezcano@linaro.org>, "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>, "Michael Clark" <michaeljclark@mac.com>, "Marc Zyngier" <marc.zyngier@arm.com>, "Palmer Dabbelt" <palmer@sifive.com>, linux-riscv@lists.infradead.org, "Thomas Gleixner" <tglx@linutronix.de>, "Zong Li" <zongbox@gmail.com> Subject: Re: [PATCH v2 6/8] RISC-V: Add required checks during clock source init Date: Tue, 8 Jan 2019 17:26:45 +0530 [thread overview] Message-ID: <CAAhSdy0VsOjiSRhBY7fdXVOccWV+4j_UJwKdNX4cNWHBJKyGeA@mail.gmail.com> (raw) In-Reply-To: <1546940318-9752-7-git-send-email-atish.patra@wdc.com> Few nit changes.. Prefer, "clocksource/drivers/riscv:" prefix instead of "RISC-V:" for this patch. On Tue, Jan 8, 2019 at 3:08 PM Atish Patra <atish.patra@wdc.com> wrote: > > Currently, clocksource registration happens for an invalid cpu > for non-smp kernels. This lead to kernel panic as cpu hotplug > registration will fail for those cpus. Moreover, > riscv_hartid_to_cpuid can return errors now. > > Do not proceed if hartid or cpuid is invalid. Take this opprtunity > to print appropriate error strings for different failure cases. > > Signed-off-by: Atish Patra <atish.patra@wdc.com> > --- > drivers/clocksource/timer-riscv.c | 23 ++++++++++++++++++++--- > 1 file changed, 20 insertions(+), 3 deletions(-) > > diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c > index 43189220..d9b914e9 100644 > --- a/drivers/clocksource/timer-riscv.c > +++ b/drivers/clocksource/timer-riscv.c > @@ -95,14 +95,31 @@ static int __init riscv_timer_init_dt(struct device_node *n) > struct clocksource *cs; > > hartid = riscv_of_processor_hartid(n); > + if (hartid < 0) { > + pr_warn("Not valid hartid for node [%pOF] error = [%d]\n", > + n, hartid); > + return hartid; > + } Add empty line here. > cpuid = riscv_hartid_to_cpuid(hartid); > Remove empty line here > + if (cpuid < 0) { > + pr_warn("Invalid cpuid for hartid [%d]\n", hartid); > + return cpuid; > + } > + > if (cpuid != smp_processor_id()) > return 0; > > + pr_err("%s: Registering clocksource cpuid [%d] hartid [%d]\n", > + __func__, cpuid, hartid); > cs = per_cpu_ptr(&riscv_clocksource, cpuid); > - clocksource_register_hz(cs, riscv_timebase); > + error = clocksource_register_hz(cs, riscv_timebase); > Remove empty line here. > + if (error) { > + pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", > + error, cpuid); > + return error; > + } Add empty line here. > sched_clock_register(riscv_sched_clock, > BITS_PER_LONG, riscv_timebase); > > @@ -110,8 +127,8 @@ static int __init riscv_timer_init_dt(struct device_node *n) > "clockevents/riscv/timer:starting", > riscv_timer_starting_cpu, riscv_timer_dying_cpu); > if (error) > - pr_err("RISCV timer register failed [%d] for cpu = [%d]\n", > - error, cpuid); > + pr_err("cpu hp setup state failed for RISCV timer [%d]\n", > + error); > return error; > } > > -- > 2.7.4 > Apart from above, looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2019-01-08 11:57 UTC|newest] Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-01-08 9:38 [PATCH v2 0/8] Various SMP related fixes Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-08 9:38 ` [PATCH v2 1/8] RISC-V: Do not wait indefinitely in __cpu_up Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-15 13:51 ` Christoph Hellwig 2019-01-15 13:51 ` Christoph Hellwig 2019-01-18 2:35 ` Atish Patra 2019-01-18 2:35 ` Atish Patra 2019-01-18 7:20 ` Christoph Hellwig 2019-01-18 7:20 ` Christoph Hellwig 2019-01-08 9:38 ` [PATCH v2 2/8] RISC-V: Move cpuid to hartid mapping to SMP Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-15 13:52 ` Christoph Hellwig 2019-01-15 13:52 ` Christoph Hellwig 2019-01-18 2:08 ` Atish Patra 2019-01-18 2:08 ` Atish Patra 2019-01-08 9:38 ` [PATCH v2 3/8] RISC-V: Remove NR_CPUs check during hartid search from DT Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-08 11:48 ` Anup Patel 2019-01-08 11:48 ` Anup Patel 2019-01-15 13:52 ` Christoph Hellwig 2019-01-15 13:52 ` Christoph Hellwig 2019-01-08 9:38 ` [PATCH v2 4/8] RISC-V: Allow hartid-to-cpuid function to fail Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-08 11:49 ` Anup Patel 2019-01-08 11:49 ` Anup Patel 2019-01-15 13:53 ` Christoph Hellwig 2019-01-15 13:53 ` Christoph Hellwig 2019-01-08 9:38 ` [PATCH v2 5/8] RISC-V: Compare cpuid with NR_CPUS before mapping Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-08 11:49 ` Anup Patel 2019-01-08 11:49 ` Anup Patel 2019-01-15 13:53 ` Christoph Hellwig 2019-01-15 13:53 ` Christoph Hellwig 2019-01-08 9:38 ` [PATCH v2 6/8] RISC-V: Add required checks during clock source init Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-08 11:56 ` Anup Patel [this message] 2019-01-08 11:56 ` Anup Patel 2019-01-18 2:10 ` Atish Patra 2019-01-18 2:10 ` Atish Patra 2019-01-15 13:54 ` Christoph Hellwig 2019-01-15 13:54 ` Christoph Hellwig 2019-01-08 9:38 ` [PATCH v2 7/8] RISC-V: Check and continue in case of an invalid cpuid Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-08 11:59 ` Anup Patel 2019-01-08 11:59 ` Anup Patel 2019-01-18 2:10 ` Atish Patra 2019-01-18 2:10 ` Atish Patra 2019-01-15 13:55 ` Christoph Hellwig 2019-01-15 13:55 ` Christoph Hellwig 2019-01-08 9:38 ` [PATCH v2 8/8] RISC-V: Assign hwcap only according to current cpu Atish Patra 2019-01-08 9:38 ` Atish Patra 2019-01-08 10:33 ` Atish Patra 2019-01-08 10:33 ` Atish Patra 2019-01-08 12:01 ` Anup Patel 2019-01-08 12:01 ` Anup Patel 2019-01-15 13:56 ` Christoph Hellwig 2019-01-15 13:56 ` Christoph Hellwig 2019-01-18 2:13 ` Atish Patra 2019-01-18 2:13 ` Atish Patra
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAAhSdy0VsOjiSRhBY7fdXVOccWV+4j_UJwKdNX4cNWHBJKyGeA@mail.gmail.com \ --to=anup@brainfault.org \ --cc=alankao@andestech.com \ --cc=aou@eecs.berkeley.edu \ --cc=atish.patra@wdc.com \ --cc=daniel.lezcano@linaro.org \ --cc=dmitriy@oss-tech.org \ --cc=jason@lakedaemon.net \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=marc.zyngier@arm.com \ --cc=me@packi.ch \ --cc=michaeljclark@mac.com \ --cc=palmer@sifive.com \ --cc=schwab@suse.de \ --cc=tglx@linutronix.de \ --cc=zongbox@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.