Re: [PATCH v3 5/7] platform: cznic: turris-omnia-mcu: Add support for MCU watchdog

["Marek Behún" <kabel@kernel.org>]

On Mon, 23 Oct 2023 07:47:44 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On 10/23/23 07:31, Marek Behún wrote:
> > Add support for the watchdog mechanism provided by the MCU.
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>  
> 
> Any reason for keeping this out of drivers/watchdog ?

Because this whole thing is compiled as one driver, turris-omnia-mcu.ko.
The whole driver is specific for one device, the Turris Omnia router.
Splitting it into separate modules would be unnecessary, the driver is
meant to provide all the Turris Omnia MCU functionality.

I've seen this done in various platform drivers.

Moreover, all of the functionality lives at one I2C address, so only
one I2C client is used.

Originally it was one .c file, but I split it into 4 files to make it
easier to review.

You can look at the other patches at
  https://patchwork.kernel.org/project/linux-soc/list/?series=795666

> > +	if (!(mcu->features & FEAT_WDT_PING))
> > +		return 0;
> > +  
> 
> Why check this here and not in the calling code ?

No particular reason, just wanted to keep the calling code cleaner.

> > @@ -36,6 +37,9 @@ struct omnia_mcu {
> >   	struct rtc_device *rtcdev;
> >   	u32 rtc_alarm;
> >   	bool front_button_poweron;
> > +
> > +	/* MCU watchdog */
> > +	struct watchdog_device wdt;  
> 
> This should be internal to the watchdog driver.

As explained above, all functionality is provided in one kernel module.
This makes sense here since the driver is meant to provide all
MCU functionality and it is specific for Turris Omnia.

Marek