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] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190723220628.GA13517@lunn.ch>
Date:   Wed, 24 Jul 2019 00:06:28 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Evgeny Kolesnikov <evgenyz@...il.com>
Cc:     Mark Rutland <mark.rutland@....com>,
        Jason Cooper <jason@...edaemon.net>, linux-pm@...r.kernel.org,
        Gregory Clement <gregory.clement@...tlin.com>,
        Sebastian Reichel <sre@...nel.org>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        Rob Herring <robh+dt@...nel.org>,
        linux-arm-kernel@...ts.infradead.org,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Subject: Re: [PATCH 0/5] Add support for WD MyCloud EX2 Ultra (+ versatile
 UART-based restart/poweroff drivers)

On Tue, Jul 23, 2019 at 07:48:49PM +0200, Evgeny Kolesnikov wrote:
> On 23/07/2019 03:56, Andrew Lunn wrote:
> >On Mon, Jul 22, 2019 at 09:53:00PM +0200, Evgeny Kolesnikov wrote:
> >>
> >>The difference between uart-poweroff and qnap-poweroff is small, but important:
> >>uart-poweroff is able to send to an MCU a command of arbitrary length, and the command
> >>itself is defined in a DTS file for a specific device/board, thus making this driver
> >>applicable to wider range of devices.
> >
> >There is a lot of replicated code here, and in the original
> >qnap-poweroff.c driver. Please consolidate it by extending the current
> >driver. It should be easy to add a new compatible string, and turn
> >power_off_cfg.cmd into an array.
> 
> Hi, Andrew.
> 
> I've considered extending qnap driver, but I have some doubts about this
> approach.
> 
> First of all there is only a poweroff counterpart. As there is no
> qnap-restart driver, what should I do with uart-restart? Is it OK to have
> xxx-restart-poweroff driver (never saw anything like that)?

Hi Evgeny

There are a few options. You can refactor all the code into a library
and small drivers which wrap around the library. Or you can make the
driver handle both, using the compatible string to determine which it
should do.
 
> While I can add cmd as a parameter to qnap driver (having it converted
> into an array) it should be optional as original qnap relies on two
> hardcoded values for its devices.

That is not what i meant. You can make the current code more generic
by changing the single byte in power_off_cfg to an array. DT should
describe the hardware, not bytes you poke into registers. So it is
perfectly valid to have the bytes hard coded in the driver.

	  Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ