From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: "Marek Behún" <kabel@kernel.org>
Cc: "Gregory CLEMENT" <gregory.clement@bootlin.com>,
"Arnd Bergmann" <arnd@arndb.de>,
soc@kernel.org, arm@kernel.org,
"Andy Shevchenko" <andy@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,
"Alessandro Zummo" <a.zummo@towertech.it>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>,
linux-rtc@vger.kernel.org,
"Wim Van Sebroeck" <wim@linux-watchdog.org>,
"Guenter Roeck" <linux@roeck-us.net>,
linux-watchdog@vger.kernel.org
Subject: Re: [PATCH v11 2/8] platform: cznic: Add preliminary support for Turris Omnia MCU
Date: Wed, 5 Jun 2024 21:04:27 +0300 [thread overview]
Message-ID: <CAHp75VccGwei9XE9vp04or7dbjOSvN7Hb0kvD06_H7=sFTKy_g@mail.gmail.com> (raw)
In-Reply-To: <20240605161851.13911-3-kabel@kernel.org>
On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@kernel.org> wrote:
>
> Add the basic skeleton for a new platform driver for the microcontroller
> found on the Turris Omnia board.
...
I paid attention to this block because of ETH_ALEN, see below.
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/device.h>
> +#include <linux/hex.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/turris-omnia-mcu-interface.h>
This is part of the niche of the driver, I would move it
> +#include <linux/types.h>
t is followed by s :-)
> +#include <linux/string.h>
> +#include <linux/sysfs.h>
...here as a separate group.
> +#include "turris-omnia-mcu.h"
...
> + /* we can't use ether_addr_copy() because reply is not u16-aligned */
> + memcpy(mcu->board_first_mac, &reply[9], ETH_ALEN);
The inclusion block misses the header for ETH_ALEN, but I realise that
instead it's better to use sizeof() as it makes this rely to the real
size of the buffer and header is not needed either.
...
Other than above LGTM, FWIW,
Reviewed-by: Andy Shevchenko <andy@kernel.org>
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2024-06-05 18:05 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-05 16:18 [PATCH v11 0/8] Turris Omnia MCU driver Marek Behún
2024-06-05 16:18 ` [PATCH v11 2/8] platform: cznic: Add preliminary support for Turris Omnia MCU Marek Behún
2024-06-05 18:04 ` Andy Shevchenko [this message]
2024-06-05 16:18 ` [PATCH v11 3/8] platform: cznic: turris-omnia-mcu: Add support for MCU connected GPIOs Marek Behún
2024-06-05 18:29 ` Andy Shevchenko
2024-06-05 19:03 ` Andy Shevchenko
2024-06-05 19:05 ` [PATCH v11 0/8] Turris Omnia MCU driver Andy Shevchenko
2024-06-06 7:25 ` 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='CAHp75VccGwei9XE9vp04or7dbjOSvN7Hb0kvD06_H7=sFTKy_g@mail.gmail.com' \
--to=andy.shevchenko@gmail.com \
--cc=a.zummo@towertech.it \
--cc=alexandre.belloni@bootlin.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=linux-rtc@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=soc@kernel.org \
--cc=wim@linux-watchdog.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 NNTP newsgroup(s).