From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH V2 21/33] xen/arm: Use device tree API in pl011 UART driver Date: Wed, 08 May 2013 17:23:39 +0100 Message-ID: <518A7C0B.7010500@linaro.org> References: <651d1e6d364d53a3f09c77f086a0a36c15e068c3.1367979526.git.julien.grall@linaro.org> <1368026240.17285.23.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1368026240.17285.23.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Stefano Stabellini , "patches@linaro.org" , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 05/08/2013 04:17 PM, Ian Campbell wrote: > On Wed, 2013-05-08 at 03:33 +0100, Julien Grall wrote: >> @@ -227,32 +239,56 @@ static struct uart_driver __read_mostly pl011_driver = { >> .tx_ready = pl011_tx_ready, >> .putc = pl011_putc, >> .getc = pl011_getc, >> - .irq = pl011_irq >> + .irq = pl011_irq, >> + .dt_irq_get = pl011_dt_irq, >> }; >> >> -/* TODO: Parse UART config from device-tree or command-line */ >> - >> -void __init pl011_init(int index, unsigned long register_base_address) >> +/* TODO: Parse UART config from the command line */ >> +static int __init pl011_uart_init(struct dt_device_node *dev, >> + const void *data) >> { >> + const struct serial_arm_defaults *defaults = data; >> struct pl011 *uart; >> + int res; >> >> - if ( (index < 0) || (index > 1) ) >> - return; >> + if ( (defaults->index < 0) || (defaults->index > 1) ) >> + return -EINVAL; >> >> - uart = &pl011_com[index]; >> + uart = &pl011_com[defaults->index]; >> >> uart->clock_hz = 0x16e3600; >> uart->baud = 38400; >> uart->data_bits = 8; >> uart->parity = PARITY_NONE; >> uart->stop_bits = 1; >> - uart->irq = 37; /* TODO Need to find this from devicetree */ >> - uart->regs = (uint32_t *) register_base_address; >> + uart->regs = (uint32_t *) defaults->register_base_address; > > Should this not come from struct dt_device_node? > > Perhaps the driver needs to be instantiating its own FIXMAP mapping > instead of doing it in common code? In fact can we not get rid of the > fixmap for the runtime (not early) console and use the ioremap stuff > which Stefano just enabled with his vmap patches, or the early_ioremap > stuff if necessary. Right. I will use ioremap in the next patch series. > That does then call into question the whole serial_arm_defaults thing. > Perhaps index should just be the order in which they are registered (and > at the moment it is effectively hardcoded to 0 anyway) I'm thinking about using serial_arm_defaults for baud rate,... For the moment the different values are hardcoded. {arm,dt}_init_uart will parse the dtuart parameters on the command line and then fill serial_arm_defaults. -- Julien