pinctrl: pinctrl-single vs custom driver
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:
Quoted 7 lines
Quoted 5 lines
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
Quoted 4 lines · Sherman Yin
Quoted 1 line
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.
Quoted 5 lines
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.)
Quoted 13 lines
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
Quoted 2 lines
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?
Quoted 2 lines
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
Quoted 8 lines
Quoted 2 lines
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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 4 lines
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