Messages in this thread |  | | From | Joel Stanley <> | Date | Mon, 10 Apr 2017 12:37:57 +0930 | Subject | Re: [PATCH v2 2/2] drivers/serial: Add driver for Aspeed virtual UART |
| |
On Wed, Apr 5, 2017 at 8:24 PM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, Apr 5, 2017 at 7:03 AM, Joel Stanley <joel@jms.id.au> 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; >> + } >> + } >> +
|  |