Hi All,
Thank you for your responses. I will submit a new version of the patch with your suggestions following this.
On 1 April 2017 at 15:22, Simon Glass sjg@chromium.org wrote:
Can you explain in your commit message why we want this?
Will do. I will be seeking to add TI OMAP device tree support to U-Boot shortly, and 16-bit register widths are needed for pinmux configuration on these platforms. Don't have an immediate need for 8-bit though, so will remove this.
I think ulong is better than fdt32_t, which is associated with devices tree.
Concur. Thanks for this suggestion.
Instead of lots of little functions, could you have:
pcs_read(ulong reg, int size) { switch (size) { case 8: return readb(reg); ...
I tried to maintain consistency with mainline Linux with the separate read functions, however it makes sense to combine the little functions.
On 2 April 2017 at 20:53, Felix Brack fb@ltec.ch wrote:
I also prefer this. The corresponding switch is already there in 'single_configure_pins(..)', i.e. no need for an additional function. Using the existing function also eliminates the 'pcs_' prefix which I would have preferred to be 'single_' due to naming consistency (nitpicking, I admit).
Thanks. Yes, I will rename this to 'single_' for consistency with the other functions (the pcs_ prefix was for consistency with Linux's pinctrl-single driver, however it doesn't make sense to copy this convention).
Keen for your thoughts on v2 of the patch (which I'll submit shortly) as to me it seems cleaner to have the register read and write switches in two separate functions, rather than in the 'single_configure_pins(..)'.
Thanks again, James