From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: "Marek Behún" <kabel@kernel.org>
Cc: "Andy Shevchenko" <andy@kernel.org>,
"Gregory CLEMENT" <gregory.clement@bootlin.com>,
"Arnd Bergmann" <arnd@arndb.de>,
soc@kernel.org, arm@kernel.org,
"Hans de Goede" <hdegoede@redhat.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
"Linus Walleij" <linus.walleij@linaro.org>,
"Bartosz Golaszewski" <brgl@bgdev.pl>,
linux-gpio@vger.kernel.org
Subject: Re: [PATCH v9 3/9] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs
Date: Fri, 10 May 2024 16:34:31 +0300 [thread overview]
Message-ID: <CAHp75VdEDhShmZDriikP0799BrV0sCZiwENhpLBoo2zGNMkLsA@mail.gmail.com> (raw)
In-Reply-To: <20240510100946.270c00e3@dellmb>
On Fri, May 10, 2024 at 11:09 AM Marek Behún <kabel@kernel.org> wrote:
> On Wed, 8 May 2024 14:16:26 +0300
> Andy Shevchenko <andy@kernel.org> wrote:
> > On Wed, May 08, 2024 at 12:31:12PM +0200, Marek Behún wrote:
...
> > > + if (type & IRQ_TYPE_EDGE_RISING)
> > > + mcu->rising |= bit;
> > > + else
> > > + mcu->rising &= ~bit;
> > > +
> > > + if (type & IRQ_TYPE_EDGE_FALLING)
> > > + mcu->falling |= bit;
> > > + else
> > > + mcu->falling &= ~bit;
> >
> > If those variables was defined as unsigned long, these can be just
> >
> > __assign_bit()
> > __assign_bit()
> >
> > And other non-atomic bitops elsewhere, like __clear_bit().
>
> Changing this propagated to many other variables and even required
> some refactoring of the omnia_gpio structure, since the bit, ctl_bit
> and int_bit members are stored as a masks, but __assign_bit() /
> __set_bit() / __clear_bit() requires bit numbers.
>
> For example
> if (gpio->int_bit && (mcu->is_cached & gpio->int_bit))
> return !!(mcu->cached & gpio->int_bit);
> needed to change to
> if (gpio->has_int && (mcu->is_cached & BIT(gpio->int_bit)))
> return !!(mcu->cached & BIT(gpio->int_bit));
You rather meant
return test_bit(...);
> and so on.
...and so on :-)
> Moreover, I agree that the if-else statement which you commented on,
> when changed to __assign_bit(), looks much nicer, but some changes that
> sprouted from this are in my opinion less readable.
Maybe. It can be done later on.
> I have prepared the fixup patch, but I am not confident enough that
> everything is done correctly. I would prefer leaving this one for
> later, if it is okay with you.
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2024-05-10 13:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 10:31 [PATCH v9 0/9] Turris Omnia MCU driver Marek Behún
2024-05-08 10:31 ` [PATCH v9 2/9] platform: cznic: Add preliminary support for Turris Omnia MCU Marek Behún
2024-05-08 10:58 ` Andy Shevchenko
2024-05-08 11:07 ` Marek Behún
2024-05-08 10:31 ` [PATCH v9 3/9] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs Marek Behún
2024-05-08 11:16 ` Andy Shevchenko
2024-05-10 8:09 ` Marek Behún
2024-05-10 13:34 ` Andy Shevchenko [this message]
2024-05-08 11:17 ` [PATCH v9 0/9] Turris Omnia MCU driver Andy Shevchenko
2024-05-08 17:50 ` Marek Behún
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=CAHp75VdEDhShmZDriikP0799BrV0sCZiwENhpLBoo2zGNMkLsA@mail.gmail.com \
--to=andy.shevchenko@gmail.com \
--cc=andy@kernel.org \
--cc=arm@kernel.org \
--cc=arnd@arndb.de \
--cc=brgl@bgdev.pl \
--cc=gregory.clement@bootlin.com \
--cc=hdegoede@redhat.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=kabel@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=soc@kernel.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).