Re: [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices

[Lee Jones <lee@kernel.org>]

On Sun, 18 Aug 2024, Heiko Stübner wrote:

> Hi Lee,
> 
> thanks a lot for your review,
> 
> Am Freitag, 16. August 2024, 19:13:36 CEST schrieb Lee Jones:
> > On Sat, 10 Aug 2024, Heiko Stuebner wrote:
> 
> 
> > > +/*
> > > + * MFD core driver for the MCU in Qnap NAS devices that is connected
> > 
> > No such thing as an "MFD".  Please describe the device.
> > 
> > Is it QNAP or Qnap?  Please be consistent.
> 
> Looks like QNAP spells itself in all upper case on their website, so I did
> go with that

Super, thanks.

> > > +
> > > +/*
> > > + * The MCU controls power to the peripherals but not the CPU.
> > > + *
> > > + * So using the pmic to power off the system keeps the MCU and hard-drives
> > > + * running. This also then prevents the system from turning back on until
> > > + * the MCU is turned off by unplugging the power-cable.
> > > + * Turning off the MCU alone on the other hand turns off the hard-drives,
> > > + * LEDs, etc while the main SoC stays running - including its network ports.
> > > + */
> > > +static int qnap_mcu_power_off(struct sys_off_data *data)
> > > +{
> > > +	struct qnap_mcu *mcu = data->cb_data;
> > > +	int ret;
> > > +	u8 cmd[] = {
> > > +		[0] = 0x40, /* @ */
> > > +		[1] = 0x43, /* C */
> > > +		[2] = 0x30  /* 0 */
> > > +	};
> > 
> > u8 cmd [] = { '@', 'C', '0' };  ?
> 
> see above.
> 
> I guess this is a style choice, we could of course go with
> 	u8 cmd[] = { 0x40, 0x43, 0x30 }
> if you prefer that.

Yes please.

-- 
Lee Jones [李琼斯]