Messages in this thread | | | From | "Li, Meng" <> | Subject | RE: [PATCH] mfd: altera-sysmgr: enable raw spinlock feature for preempt-rt kernel | Date | Tue, 16 Nov 2021 14:43:25 +0000 |
| |
> -----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
| |