Messages in this thread | | | From | Evgeny Kolesnikov <> | Subject | Re: [PATCH 0/5] Add support for WD MyCloud EX2 Ultra (+ versatile UART-based restart/poweroff drivers) | Date | Tue, 23 Jul 2019 19:48:49 +0200 |
| |
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.
| |