lkml.org 
[lkml]   [2021]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] mfd: altera-sysmgr: enable raw spinlock feature for preempt-rt kernel
Date


> -----Original Message-----
> From: Arnd Bergmann <arnd@arndb.de>
> Sent: Tuesday, November 16, 2021 8:02 PM
> To: Li, Meng <Meng.Li@windriver.com>
> Cc: thor.thayer@linux.intel.com; Lee Jones <lee.jones@linaro.org>; Arnd
> Bergmann <arnd@arndb.de>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>
> Subject: Re: [PATCH] mfd: altera-sysmgr: enable raw spinlock feature for
> preempt-rt kernel
>
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Tue, Nov 16, 2021 at 11:54 AM Meng Li <Meng.Li@windriver.com> wrote:
> > diff --git a/drivers/mfd/altera-sysmgr.c b/drivers/mfd/altera-sysmgr.c
> > index 5d3715a28b28..27271cec5d53 100644
> > --- a/drivers/mfd/altera-sysmgr.c
> > +++ b/drivers/mfd/altera-sysmgr.c
> > @@ -83,6 +83,9 @@ static struct regmap_config altr_sysmgr_regmap_cfg =
> {
> > .fast_io = true,
> > .use_single_read = true,
> > .use_single_write = true,
> > +#ifdef CONFIG_PREEMPT_RT
> > + .use_raw_spinlock = true,
> > +#endif
>
> I think you should remove the #ifdef here: if PREEMPT_RT is disabled, the
> flag has no effect because spinlock behaves the same way as raw_spinlock. If
> anything else starts requiring the use of raw spinlocks, then we probably
> want the flag to be set here as well.
>

Thanks for your suggestion, and I also agree with the spinlock action when PREEMPT_RT is disabled.
But please allow me to explain why I keep the "ifdef"
1. although I send this patch to mainline upstream, I only want to fix this issue in RT kernel.
Moreover, the commit 67021f25d952("regmap: teach regmap to use raw spinlocks if requested in the config ") is also for RT kernel even if it doesn't use "ifdef CONFIG_PREEMPT_RT"
My ideal is that if this patch is merged into mainline, Linux-rt maintainer will not spend extra effort to focus on this patch. After all, this fixing is more related with driver.
In addition, I found out there are other patches with "ifdef CONFIG_PREEMPT_RT" merged by mainline, so I also send this patch to mainline, not Linux-rt.

2. I check regmap.c code that is related with use_raw_spinlock. If PREEMPT_RT is disabled and use_raw_spinlock is set as true, the else case will not be entered any longer.
In other words, in mainline standard kernel, if use_raw_spinlock is set as true, raw spinlock will be used forever, and the code in else case will become useless.
I feel it is a little unreasonable. So, I keep the "ifdef"
if ((bus && bus->fast_io) ||
config->fast_io) {
if (config->use_raw_spinlock) {
raw_spin_lock_init(&map->raw_spinlock);
map->lock = regmap_lock_raw_spinlock;
map->unlock = regmap_unlock_raw_spinlock;
lockdep_set_class_and_name(&map->raw_spinlock,
lock_key, lock_name);
} else {
spin_lock_init(&map->spinlock);
map->lock = regmap_lock_spinlock;
map->unlock = regmap_unlock_spinlock;
lockdep_set_class_and_name(&map->spinlock,
lock_key, lock_name);
}
} else {
mutex_init(&map->mutex);
map->lock = regmap_lock_mutex;
map->unlock = regmap_unlock_mutex;
map->can_sleep = true;
lockdep_set_class_and_name(&map->mutex,
lock_key, lock_name);
}

Thanks,
Limeng

> Arnd
\
 
 \ /
  Last update: 2021-11-16 15:45    [W:0.074 / U:0.964 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site