Hello James,
This patch does not compile without errors.
On 06.04.2017 07:38, James Balean wrote:
Enables the pinctrl-single driver to support 16-bit registers. Only 32-bit registers were supported previously. Reduced width registers are required for some platforms, such as OMAP.
Signed-off-by: James Balean james@balean.com.au Cc: Felix Brack fb@ltec.ch Cc: Simon Glass sjg@chromium.org
Changes for v2:
- Added explanation of why this patch is needed.
- Changed fdt32_t to ulong type.
- Removed 8-bit support.
- Now with a single read and write function, instead of one for each register width.
drivers/pinctrl/pinctrl-single.c | 45 ++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index d2dcec0..defb66f 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -24,6 +24,30 @@ struct single_fdt_pin_cfg { fdt32_t val; /* configuration register value */ };
+static ulong single_read(ulong reg, int width) {
- switch (size) {
Use 'width' instead of 'size' here.
- case 16:
return readw(reg);- case 32:
return readl(reg);- default:
dev_warn(dev, "unsupported register width %i\n", width);
This function must return a value. What would you return here, i.e., in case of failure?
- }
+}
+static void single_write(ulong val, ulong reg, int width) {
- switch (width) {
- case 16:
writew(val, reg);break;- case 32:
writel(val, reg);break;- default:
dev_warn(dev, "unsupported register width %i\n", width;
Missing closing parentheses.
- }
+}
/**
- single_configure_pins() - Configure pins based on FDT data
@@ -47,28 +71,19 @@ static int single_configure_pins(struct udevice *dev, int n, reg; u32 val;
- for (n = 0; n < count; n++) {
- for (n = 0; n < count; n++, pins++) { reg = fdt32_to_cpu(pins->reg); if ((reg < 0) || (reg > pdata->offset)) { dev_dbg(dev, " invalid register offset 0x%08x\n", reg);
pins++; continue;switch (pdata->width) {case 32:val = readl(reg) & ~pdata->mask;val |= fdt32_to_cpu(pins->val) & pdata->mask;writel(val, reg);dev_dbg(dev, " reg/val 0x%08x/0x%08x\n",reg, val);break;default:dev_warn(dev, "unsupported register width %i\n",pdata->width);}pins++;
val = single_read(reg, pdata->width) & ~pdata->mask;
This is a no go as 'single_read' may fail (see above). You will have to check the return value. This is another reason for witch again I suggest you just keep the switch statement as it was.
val |= fdt32_to_cpu(pins->val) & pdata->mask;single_write(val, reg, pdata->width);dev_dbg(dev, " reg/val 0x%08x/0x%08x\n", reg, val);- return 0;
}
IMHO: You should make sure your patch is syntactically correct i.e. at least run a build cycle. Furthermore and specifically for this patch (as it directly deals with hardware registers) you should test it for '16 bit width' on your hardware.
regards Felix