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

[Marek Behún <kabel@xxxxxxxxxx>]

On Wed, May 08, 2024 at 01:58:30PM +0300, Andy Shevchenko wrote:
> On Wed, May 08, 2024 at 12:31:11PM +0200, Marek Behún wrote:
> > Add the basic skeleton for a new platform driver for the microcontroller
> > found on the Turris Omnia board.
> 
> Some cosmetics in case you need a new version.
> Possibly can be done as follow up(s).
> 
> ...
> 
> > +Date:		July 2024
> > +KernelVersion:	6.10
> 
> TBH, I'm not sure you manage to squeeze this rather big driver to v6.10.
> 
> ...
> 
> > +static const struct attribute_group *omnia_mcu_groups[] = {
> > +	&omnia_mcu_base_group,
> > +	NULL
> > +};
> 
> __ATTRIBUTE_GROUPS()

I think we already discussed this one. __ATTRIBUTE_GROUPS defines only
one group, and I am adding to the array in subsequent patches.

> 
> ...
> 
> Perhaps also
> 
> 	struct i2c_client *client = mcu->client;
> 
> > +	struct device *dev = &mcu->client->dev;
> 
> 	struct device *dev = &client->dev;
> 
> > +	bool suggest_fw_upgrade = false;
> > +	u16 status;
> > +	int err;
> > +
> > +	/* status word holds MCU type, which we need below */
> > +	err = omnia_cmd_read_u16(mcu->client, OMNIA_CMD_GET_STATUS_WORD,
> > +				 &status);
> 
> 	err = omnia_cmd_read_u16(client, OMNIA_CMD_GET_STATUS_WORD, &status);
> 
> and so on...
> 
> 
> > +	if (err)
> > +		return err;
> 
> ...
> 
> > +	/*
> > +	 * check whether MCU firmware supports the OMNIA_CMD_GET_FEATURES
> 
> Check
> 
> > +	 * command
> 
> command.
> 
> > +	 */
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
>