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: Lee Jones <lee.jones@linaro.org>
    > Sent: Tuesday, November 16, 2021 11:39 PM
    > To: Li, Meng <Meng.Li@windriver.com>
    > Cc: Arnd Bergmann <arnd@arndb.de>; thor.thayer@linux.intel.com; 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, 16 Nov 2021, Li, Meng wrote:
    >
    > >
    > >
    > > > -----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);
    > > }
    > >
    >
    > I dislike #ifery as a general rule. So with that in mind - if it's not required, I'd
    > prefer that it's removed.
    >

    Ok!
    There is no real difference if remove the #ifery.
    I will check the standard kernel and then sent v3 RR.

    Thanks,
    Limeng

    > --
    > Lee Jones [李琼斯]
    > Senior Technical Lead - Developer Services Linaro.org │ Open source
    > software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
    \
     
     \ /
      Last update: 2021-11-16 17:07    [W:2.871 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site