Hi Andrew,
On Wed, Dec 21, 2016 at 05:41:36PM +0100, Andrew Lunn wrote:
> > These models can just be added to qnap-poweroff, which handles
> > exactly this special case as far as I can see.
>
> Nope, qnap is much, much simpler. The configuration of the serial port
> is simpler, and it only needs to send a single byte. Here we have all
> sorts of checksums to calculate, stuff coming back from the
> microcontroller, etc. The complexity is much higher.
>
> V1 of this patchset did extend the qnap driver. But in fact, very
> little of the original code was left afterwards, and lots of new code
> was added. So i requested a new driver be written, rather than extend
> my qnap driver.
>
> I would not like to see the nice and simple qnap driver get all this
> code added to it, making it much harder to maintain, for very little
> gain.
I'm talking about the special case, where it also sends only a
single byte:
> + /* send the power-off command to PIC */
> + if(cfg->cmd[0][0] == 1 && cfg->cmd[1][0] == 0) {
> + /* if it's simply one-byte command, send it directly */
> + writel(cfg->cmd[0][1], UART1_REG(TX));
> + }
The configuration is different, but
a) probably it can just use the config from the qnap driver, since
it just sends a single byte.
b) making the 4 config registers configurable in the qnap driver does
not add much complexity.
So this case should be removed from the linkstation-reset driver. If
any board needs it, it should use the qnap-poweroff driver instead.
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
