Re: [PATCH v11 2/8] platform: cznic: Add preliminary support for Turris Omnia MCU

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Wed, Jun 5, 2024 at 7:19 PM Marek Behún <kabel@xxxxxxxxxx> 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@xxxxxxxxxx>

-- 
With Best Regards,
Andy Shevchenko