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

Lee Jones
2024-08-22T10:10:26Z

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 [李琼斯]