From: "Marek Behún" <kabel@kernel.org>
To: Matti Vaittinen <mazziesaccount@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
Gregory CLEMENT <gregory.clement@bootlin.com>,
soc@kernel.org, arm@kernel.org,
Linus Walleij <linus.walleij@linaro.org>,
Bartosz Golaszewski <brgl@bgdev.pl>,
Andy Shevchenko <andy@kernel.org>,
linux-gpio@vger.kernel.org
Subject: Re: [PATCH v5 03/11] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs
Date: Mon, 25 Mar 2024 10:53:49 +0100 [thread overview]
Message-ID: <20240325105349.6f6c21c7@dellmb> (raw)
In-Reply-To: <0352287d-cbd0-4ed7-8551-a23191487279@gmail.com>
On Mon, 25 Mar 2024 11:10:04 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> Hi Marek,
>
> I can't say I did a proper review but browsing through the code without
> proper understanding of the platform raised one small question :)
>
> On 3/23/24 18:43, Marek Behún wrote:
> > Add support for GPIOs connected to the MCU on the Turris Omnia board.
> >
> > This includes:
> > - front button pin
> > - enable pins for USB regulators
> > - MiniPCIe / mSATA card presence pins in MiniPCIe port 0
> > - LED output pins from WAN ethernet PHY, LAN switch and MiniPCIe ports
> > - on board revisions 32+ also various peripheral resets and another
> > voltage regulator enable pin
> >
> > Signed-off-by: Marek Behún <kabel@kernel.org>
>
> ...
>
> > +/**
> > + * omnia_mask_interleave - Interleaves the bytes from @rising and @falling
> > + * @dst: the destination u8 array of interleaved bytes
> > + * @rising: rising mask
> > + * @falling: falling mask
> > + *
> > + * Interleaves the little-endian bytes from @rising and @falling words.
> This means the 'rising' and 'falling' should always be little-endian?
> Should the parameter types reflect this? Or should we see some
> cpu_to_le() calls? (Or, is this code just guaranteed to be always
> running on a le-machine?).
>
> > + * If @rising = (r0, r1, r2, r3) and @falling = (f0, f1, f2, f3), the result is
> > + * @dst = (r0, f0, r1, f1, r2, f2, r3, f3).
> > + *
> > + * The MCU receives interrupt mask and reports pending interrupt bitmap int this
> > + * interleaved format. The rationale behind it is that the low-indexed bits are
> > + * more important - in many cases, the user will be interested only in
> > + * interrupts with indexes 0 to 7, and so the system can stop reading after
> > + * first 2 bytes (r0, f0), to save time on the slow I2C bus.
> > + *
> > + * Feel free to remove this function and its inverse, omnia_mask_deinterleave,
> > + * and use an appropriate bitmap_* function once such a function exists.
> > + */
> > +static void omnia_mask_interleave(u8 *dst, u32 rising, u32 falling)
> > +{
> > + for (int i = 0; i < sizeof(u32); ++i) {
> > + dst[2 * i] = rising >> (8 * i);
> > + dst[2 * i + 1] = falling >> (8 * i);
> > + }
> > +}
> > +
> > +/**
> > + * omnia_mask_deinterleave - Deinterleaves the bytes into @rising and @falling
> > + * @src: the source u8 array containing the interleaved bytes
> > + * @rising: pointer where to store the rising mask gathered from @src
> > + * @falling: pointer where to store the falling mask gathered from @src
> > + *
> > + * This is the inverse function to omnia_mask_interleave.
> > + */
> > +static void omnia_mask_deinterleave(const u8 *src, u32 *rising, u32 *falling)
> > +{
> > + *rising = *falling = 0;
> > +
> > + for (int i = 0; i < sizeof(u32); ++i) {
> > + *rising |= src[2 * i] << (8 * i);
> > + *falling |= src[2 * i + 1] << (8 * i);
> > + }
>
> Also here I could expect seeing le_to_cpu() unless I am (again :])
> missing something.
I don't understand. The rising and falling variables have native
endiannes, as they should have.
And the src and dst are u8 arrays, which contain two LE32
unsigned integers, but these integers are interleaved. I am converting
then to two native unsigned integers byte by byte, so I am basically
doing what a theoretical le32_interleaved_le32_to_cpu() would have done
if it did exist. Putting another le*_to_cpu() would only break things.
Marek
next prev parent reply other threads:[~2024-03-25 9:53 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-23 16:43 [PATCH v5 00/11] Turris Omnia MCU driver Marek Behún
2024-03-23 16:43 ` [PATCH v5 01/11] dt-bindings: arm: add cznic,turris-omnia-mcu binding Marek Behún
2024-03-26 8:45 ` Krzysztof Kozlowski
2024-03-26 9:02 ` Marek Behún
2024-03-23 16:43 ` [PATCH v5 02/11] platform: cznic: Add preliminary support for Turris Omnia MCU Marek Behún
2024-03-24 11:01 ` Andy Shevchenko
2024-03-24 15:04 ` Marek Behún
2024-03-24 15:30 ` Andy Shevchenko
2024-03-25 10:39 ` Marek Behún
2024-04-02 16:41 ` Marek Behún
2024-03-23 16:43 ` [PATCH v5 03/11] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs Marek Behún
2024-03-25 9:10 ` Matti Vaittinen
2024-03-25 9:53 ` Marek Behún [this message]
2024-03-25 10:25 ` Matti Vaittinen
2024-04-02 9:59 ` Dan Carpenter
2024-03-23 16:43 ` [PATCH v5 04/11] platform: cznic: turris-omnia-mcu: Add support for poweroff and wakeup Marek Behún
2024-03-23 16:43 ` [PATCH v5 05/11] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog Marek Behún
2024-03-23 16:43 ` [PATCH v5 06/11] devm-helpers: Add resource managed version of irq_create_mapping() Marek Behún
2024-03-25 9:40 ` Matti Vaittinen
2024-03-25 9:57 ` Marek Behún
2024-03-26 9:00 ` Dan Carpenter
2024-03-27 9:34 ` Marek Behún
2024-03-27 11:39 ` Dan Carpenter
2024-03-23 16:43 ` [PATCH v5 07/11] platform: cznic: turris-omnia-mcu: Add support for MCU provided TRNG Marek Behún
2024-03-23 16:43 ` [PATCH v5 08/11] devm-helpers: Add resource managed version of debugfs directory create function Marek Behún
2024-03-23 17:21 ` Guenter Roeck
2024-03-23 16:43 ` [PATCH v5 09/11] platform: cznic: turris-omnia-mcu: Add support for digital message signing via debugfs Marek Behún
2024-03-23 16:43 ` [PATCH v5 10/11] ARM: dts: turris-omnia: Add MCU system-controller node Marek Behún
2024-03-23 16:43 ` [PATCH v5 11/11] ARM: dts: turris-omnia: Add GPIO key node for front button Marek Behún
[not found] ` <20240323164359.21642-9-kabel__6885.49310886941$1711212291$gmane$org@kernel.org>
2024-03-23 21:10 ` [PATCH v5 08/11] devm-helpers: Add resource managed version of debugfs directory create function Christophe JAILLET
2024-03-23 21:25 ` Marek Behún
2024-03-24 9:21 ` Christophe JAILLET
2024-03-24 15:08 ` Marek Behún
2024-03-25 11:05 ` Dan Carpenter
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=20240325105349.6f6c21c7@dellmb \
--to=kabel@kernel.org \
--cc=andy@kernel.org \
--cc=arm@kernel.org \
--cc=arnd@arndb.de \
--cc=brgl@bgdev.pl \
--cc=gregory.clement@bootlin.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=mazziesaccount@gmail.com \
--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).