On Wed, Apr 5, 2017 at 8:24 PM, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Wed, Apr 5, 2017 at 7:03 AM, Joel Stanley <joel@xxxxxxxxx> wrote: > >> + port.port.irq = irq_of_parse_and_map(np, 0); > > Isn't better to get this via platform_get_irq() ? I can't see the benefit. > >> + port.port.irqflags = IRQF_SHARED; >> + port.port.iotype = UPIO_MEM; > >> + if (of_property_read_u32(np, "reg-io-width", &prop) == 0) { > > I would still go with usual pattern. > >> + switch (prop) { >> + case 1: >> + port.port.iotype = UPIO_MEM; >> + break; >> + case 4: > >> + port.port.iotype = of_device_is_big_endian(np) ? >> + UPIO_MEM32BE : UPIO_MEM32; > > Hmm... And this one is not in align with IO accessors used in this > driver. (readx()/writex() are little endian IO accessors). We only perform readb/writeb, however you raise a good point that we're assuming LE in the register layout. I will remove checking of this optional property. I will send v3 with the other cleanups you mentioned. Cheers, Joel > >> + break; >> + default: >> + dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n", >> + prop); >> + rc = -EINVAL; >> + goto err_clk_disable; >> + } >> + } >> + -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html