pinctrl: pinctrl-single vs custom driver

pinctrl: pinctrl-single vs custom driver

[Sherman Yin]

Hi Linus,

After upstreaming the bcm281xx (capri) pinctrl driver, I'm working on 
the driver for another SoC.  I'd like your input before I get too far 
with the driver.

Recall we had a discussion about whether to use pinctrl-single for the 
bcm281xx driver, and you mentioned a custom driver is better:

 >> Now I have written a driver that's pretty much ready-to-go, but on the
 >> other hand, pinctrl-single also does what my chip needs, so my driver
 >> is sort of duplicated other than the fact that it defines pins and
 >> functions.  I'm just trying to determine which driver is the more
 >> preferred way to go for upstreaming.
 >
 > Your driver defining pins and functions is better.

Like the bcm281xx, in this SoC, each pin is also controlled by 1 
register, so pinctrl-single should work.  The difference between this 
SoC and bcm281xx is that the pin registers in this SoC are only 
accessible via a messaging mechanism (send a command then wait for a 
reply), not a simple register read-write.  We have defined one message 
for reading the register and one for writing to it.

In a driver like pinctrl-bcm281xx, pinmux and pin configs are separately 
applied and on a per-pin basis.  A simple update of 1 pin will thus 
result in numerous messages:

  1. Send cmd to read register, block for reply
  2. Rcv reply with value
  3. Modify pinmux portion
  4. Send cmd to write register
  5. Send cmd to read register, block for reply
  6. Rcv reply with value
  7. Modify pin configs portion
  8. Send cmd to write register

With pinctrl-single, updating pins will be much simpler with just 1 
message to directly write the whole register.

Is this efficiency enough to warrant using pinctrl-single over a 
"normal" pinctrl driver in this case?

Appreciate your time.

Regards,
Sherman

pinctrl: pinctrl-single vs custom driver

[Linus Walleij]

On Tue, Feb 25, 2014 at 10:24 AM, Sherman Yin <syin@broadcom.com> wrote:

>> Your driver defining pins and functions is better.
>
> Like the bcm281xx, in this SoC, each pin is also controlled by 1 register,
> so pinctrl-single should work.

One other deciding factor is what kind of information you have available
on these pin registers and what format they are in. If your system is
well documented and not delivered in some register map description
format you should typically open code the hardware information in the
driver.

>  The difference between this SoC and bcm281xx
> is that the pin registers in this SoC are only accessible via a messaging
> mechanism (send a command then wait for a reply), not a simple register
> read-write.  We have defined one message for reading the register and one
> for writing to it.

This is no different from e.g. drivers/pinctrl/pinctrl-abx500.c which marshals
register writes to an MFD device. (Which in turn is on I2C.)

> In a driver like pinctrl-bcm281xx, pinmux and pin configs are separately
> applied and on a per-pin basis.  A simple update of 1 pin will thus result
> in numerous messages:
>
>  1. Send cmd to read register, block for reply
>  2. Rcv reply with value
>  3. Modify pinmux portion
>  4. Send cmd to write register
>  5. Send cmd to read register, block for reply
>  6. Rcv reply with value
>  7. Modify pin configs portion
>  8. Send cmd to write register
>
> With pinctrl-single, updating pins will be much simpler with just 1 message
> to directly write the whole register.

OK I see the point, however... which solution is easiest for a developer
to understand?

> Is this efficiency enough to warrant using pinctrl-single over a "normal"
> pinctrl driver in this case?

So I would say that this code execution is likely not on the time-critical
path?

Also take the debuggability and understandability of the code as a
very important factor. Which driver will be easiest for a developer to
cope with, and which will be easiest to use when debugging the
system?

Yours,
Linus Walleij

pinctrl: pinctrl-single vs custom driver

[Sherman Yin]

>> Like the bcm281xx, in this SoC, each pin is also controlled by 1 register,
>> so pinctrl-single should work.
>
> One other deciding factor is what kind of information you have available
> on these pin registers and what format they are in. If your system is
> well documented and not delivered in some register map description
> format you should typically open code the hardware information in the
> driver.

The info available is similar to the bcm281xx pinctrl driver - basically 
just what is available in the binding documentation.  Although that is 
still not 100% clear, it is still much better than just numerical 
register values.


>> Is this efficiency enough to warrant using pinctrl-single over a "normal"
>> pinctrl driver in this case?
>
> So I would say that this code execution is likely not on the time-critical
> path?

Pins are typically changed on suspend/resume to save power, so I don't 
think it is time-critical.  Also, I could cache the pin values (provided 
we can assure that all software change pins via my driver) to save a few 
messages should timing/messaging congestion become a concern.  I won't 
implement the cache unless we really run into timing problems without it.

> Also take the debuggability and understandability of the code as a
> very important factor. Which driver will be easiest for a developer to
> cope with, and which will be easiest to use when debugging the
> system?

Okay.  The new driver will be similar to the bcm281xx one and will not 
be based on pinctrl-single, so that code is a bit easier to understand 
and the device tree nodes will be more readable.

Thanks for your input, Linus!

Regards,
Sherman