Hi James, On 26 March 2017 at 23:55, James Balean <[email protected]> wrote: > Enables the pinctrl-single driver to support 8 and 16-bit registers. > Only 32-bit registers were supported previously.
Can you explain in your commit message why we want this? > > Signed-off-by: James Balean <[email protected]> > Cc: Felix Brack <[email protected]> > Cc: Simon Glass <[email protected]> > --- > drivers/pinctrl/pinctrl-single.c | 58 > ++++++++++++++++++++++++++++++++++------ > 1 file changed, 50 insertions(+), 8 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-single.c > b/drivers/pinctrl/pinctrl-single.c > index d2dcec0..63fec8d 100644 > --- a/drivers/pinctrl/pinctrl-single.c > +++ b/drivers/pinctrl/pinctrl-single.c > @@ -24,6 +24,36 @@ struct single_fdt_pin_cfg { > fdt32_t val; /* configuration register value */ > }; > > +static fdt32_t pcs_readb(fdt32_t reg) I think ulong is better than fdt32_t, which is associated with devices tree. > +{ > + return readb(reg); > +} > + > +static fdt32_t pcs_readw(fdt32_t reg) > +{ > + return readw(reg); > +} > + > +static fdt32_t pcs_readl(fdt32_t reg) > +{ > + return readl(reg); > +} > + > +static void pcs_writeb(fdt32_t val, fdt32_t reg) > +{ > + writeb(val, reg); > +} > + > +static void pcs_writew(fdt32_t val, fdt32_t reg) > +{ > + writew(val, reg); > +} > + > +static void pcs_writel(fdt32_t val, fdt32_t reg) > +{ > + writel(val, reg); > +} Instead of lots of little functions, could you have: pcs_read(ulong reg, int size) { switch (size) { case 8: return readb(reg); ... ? > + > /** > * single_configure_pins() - Configure pins based on FDT data > * > @@ -46,28 +76,40 @@ static int single_configure_pins(struct udevice *dev, > int count = size / sizeof(struct single_fdt_pin_cfg); > int n, reg; > u32 val; > + fdt32_t (*pcs_read)(fdt32_t reg); > + void (*pcs_write)(fdt32_t val, fdt32_t reg); > > - 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; > } > reg += pdata->base; > switch (pdata->width) { > + case 8: > + pcs_read = pcs_readb; > + pcs_write = pcs_writeb; > + break; > + case 16: > + pcs_read = pcs_readw; > + pcs_write = pcs_writew; > + break; > 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); > + pcs_read = pcs_readl; > + pcs_write = pcs_writel; > break; > default: > dev_warn(dev, "unsupported register width %i\n", > pdata->width); > + continue; > } > - pins++; > + > + val = pcs_read(reg) & ~pdata->mask; > + val |= fdt32_to_cpu(pins->val) & pdata->mask; > + pcs_write(val, reg); > + dev_dbg(dev, " reg/val 0x%08x/0x%08x\n", > + reg, val); > } > return 0; > } > -- > 2.7.4 > Regards, Simon _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
