From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from an-out-0708.google.com (an-out-0708.google.com [209.85.132.243]) by ozlabs.org (Postfix) with ESMTP id 9737BDE262 for ; Thu, 3 Apr 2008 05:00:26 +1100 (EST) Received: by an-out-0708.google.com with SMTP id c37so677262anc.78 for ; Wed, 02 Apr 2008 11:00:24 -0700 (PDT) Message-ID: Date: Wed, 2 Apr 2008 12:00:24 -0600 From: "Grant Likely" Sender: glikely@secretlab.ca To: "John Linn" Subject: Re: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. In-Reply-To: <20080402165222.2468414080A9@mail104-dub.bigfish.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 References: <12071551351007-git-send-email-john.linn@xilinx.com> <12071551354058-git-send-email-john.linn@xilinx.com> <20080402165222.2468414080A9@mail104-dub.bigfish.com> Cc: linuxppc-dev@ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Wed, Apr 2, 2008 at 10:52 AM, John Linn wrote: > The Xilinx 16550 uart core is not a standard 16550 because it uses > word-based addressing rather than byte-based addressing. With > additional properties it is compatible with the open firmware > 'ns16550' compatible binding. > > This code updates the of_serial driver to handle the reg-offset > and reg-shift properties to enable this core to be used. > > Signed-off-by: John Linn Comments below... > --- > Documentation/powerpc/booting-without-of.txt | 11 +++++++++++ > drivers/serial/of_serial.c | 15 +++++++++++++-- > 2 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt > index 87f4d84..af112d9 100644 > --- a/Documentation/powerpc/booting-without-of.txt > +++ b/Documentation/powerpc/booting-without-of.txt > @@ -2539,6 +2539,17 @@ platforms are moved over to use the flattened-device-tree model. > differ between different families. May be > 'virtex2p', 'virtex4', or 'virtex5'. > > + iv) Xilinx Uart 16550 > + > + Xilinx UART 16550 devices are very similar to the NS16550 such that they > + use the ns16550 binding with properties to specify register spacing and > + an offset from the base address. > + > + Requred properties: > + - clock-frequency : Frequency of the clock input > + - reg-offset : A value of 3 is required > + - reg-shift : A value of 2 is required > + > More devices will be defined as this spec matures. > > VII - Specifying interrupt information for devices > diff --git a/drivers/serial/of_serial.c b/drivers/serial/of_serial.c > index 2efb892..af9ed48 100644 > --- a/drivers/serial/of_serial.c > +++ b/drivers/serial/of_serial.c > @@ -30,7 +30,7 @@ static int __devinit of_platform_serial_setup(struct of_device *ofdev, > { > struct resource resource; > struct device_node *np = ofdev->node; > - const unsigned int *clk, *spd; > + const unsigned int *clk, *spd, *reg_offset, *reg_shift; These should really be u32's I believe; on 64 bit architectures this will misbehave (not an immediate practical problem, but it's best to be explicit about these things). > int ret; > > memset(port, 0, sizeof *port); > @@ -48,7 +48,18 @@ static int __devinit of_platform_serial_setup(struct of_device *ofdev, > } > > spin_lock_init(&port->lock); > - port->mapbase = resource.start; > + > + reg_offset = of_get_property(np, "reg-offset", NULL); > + reg_shift = of_get_property(np, "reg-shift", NULL); > + > + if (!reg_offset) > + port->mapbase = resource.start; > + else > + port->mapbase = resource.start + *reg_offset; > + > + if (reg_shift) > + port->regshift = *reg_shift; > + This is a little unsafe since it doesn't check the property size, I'd do the following instead: port->mapbase = resource.start /* Check for shifted address mapping */ prop = of_get_property(np, "reg-offset", &prop_size); if (prop && (prop_size == sizeof(u32)) port->mapbase += *prop; /* Check for registers offset within the devices address range */ prop = of_get_property(np, "reg-shift", &prop_size); if (prop && (prop_size == sizeof(u32))) port->regshift = *prop; Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd.