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)?
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. And having a non-qnap device with this
driver in DT without defined cmd would not make any sense. It feels
kinda ugly.
Wouldn't it be more fitting to have these two generic drivers and then
retire old qnap driver while moving everything that uses it to the new one?
Thanks for the review.
EK.
