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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240822101020.GG6858@google.com>
Date: Thu, 22 Aug 2024 11:10:20 +0100
From: Lee Jones <lee@...nel.org>
To: Heiko Stübner <heiko@...ech.de>
Cc: jdelvare@...e.com, linux@...ck-us.net, dmitry.torokhov@...il.com,
	pavel@....cz, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, ukleinek@...ian.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-rockchip@...ts.infradead.org, linux-hwmon@...r.kernel.org,
	linux-input@...r.kernel.org, linux-leds@...r.kernel.org
Subject: Re: [PATCH v4 2/7] mfd: add base driver for qnap-mcu devices

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