Re: [PATCH v2] power: reset: add linkstation-reset driver

[Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]