* RE: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. [not found] <689CB232690D8D4E97DA6C76DA098E6C05FC4762@XCO-EXCHVS1.xlnx.xilinx.com> @ 2008-04-02 21:39 ` Stephen Neuendorffer 2008-04-03 11:59 ` Sergei Shtylyov 0 siblings, 1 reply; 7+ messages in thread From: Stephen Neuendorffer @ 2008-04-02 21:39 UTC (permalink / raw) To: John Linn, Grant Likely, sshtylyov; +Cc: linuxppc-dev I don't think big-endian has the same context as reg-shift/reg-offset. The OpenPOC is fundamentally a 32 bit device, but ns16550 is not... If we were talking about a 32 bit device, then I'd probably agree with you, but in this case, the reg-shift (and to some extent) reg-offset have been used before and probably make more sense, I think. Steve > -----Original Message----- > From: Sergei Shtylyov [mailto:sshtylyov@ru•mvista.com] > Sent: Wednesday, April 02, 2008 1:20 PM > To: John Linn > Cc: linuxppc-dev@ozlabs•org; grant.likely@secretlab•ca > Subject: Re: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. >=20 > Hello. >=20 > John Linn wrote: >=20 > > 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. >=20 > > This code updates the of_serial driver to handle the reg-offset > > and reg-shift properties to enable this core to be used. >=20 > > Signed-off-by: John Linn <john.linn@xilinx•com> >=20 > > 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 >=20 > I'm proposing you to use the already existing "big-endian" property ISO > "reg-offset" (used in the nodes describing OpenPIC, for example). >=20 > WBR, Sergei ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. 2008-04-02 21:39 ` [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550 Stephen Neuendorffer @ 2008-04-03 11:59 ` Sergei Shtylyov 0 siblings, 0 replies; 7+ messages in thread From: Sergei Shtylyov @ 2008-04-03 11:59 UTC (permalink / raw) To: Stephen Neuendorffer; +Cc: John Linn, linuxppc-dev Hello. Stephen Neuendorffer wrote: > I don't think big-endian has the same context as reg-shift/reg-offset. The "big-endian" is about how the byte addresses are laid out, so the context is the same -- in this case, it would determine where each UART register is located within the address stride specified by "reg-shift". It'll alwaay be at offset 0 or (2 << reg-shift) - 1 (unless some vendor goes and implements something with "middle-endian" layout of course :-) > The OpenPOC is fundamentally a 32 bit device, but ns16550 is not... If So what? > we were talking about a 32 bit device, then I'd probably agree with you, There are 16550 clones that *are* 32-bit. > but in this case, the reg-shift I'm not arguing about "reg-shift" already -- look like it's been spec'ed. :-) > (and to some extent) reg-offset have > been used before and probably make more sense, I think. The "reg-offset" has been used before? Where? > Steve WBR, Sergei ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <12071551351007-git-send-email-john.linn@xilinx.com>]
[parent not found: <12071551354058-git-send-email-john.linn@xilinx.com>]
* [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. [not found] ` <12071551354058-git-send-email-john.linn@xilinx.com> @ 2008-04-02 16:52 ` John Linn 2008-04-02 18:00 ` Grant Likely 2008-04-02 19:19 ` Sergei Shtylyov 0 siblings, 2 replies; 7+ messages in thread From: John Linn @ 2008-04-02 16:52 UTC (permalink / raw) To: linuxppc-dev, grant.likely; +Cc: John Linn 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 <john.linn@xilinx•com> --- 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; 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; + port->irq = irq_of_parse_and_map(np, 0); port->iotype = UPIO_MEM; port->type = type; -- 1.5.2.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. 2008-04-02 16:52 ` John Linn @ 2008-04-02 18:00 ` Grant Likely 2008-04-02 18:20 ` John Linn 2008-04-02 19:19 ` Sergei Shtylyov 1 sibling, 1 reply; 7+ messages in thread From: Grant Likely @ 2008-04-02 18:00 UTC (permalink / raw) To: John Linn; +Cc: linuxppc-dev On Wed, Apr 2, 2008 at 10:52 AM, John Linn <john.linn@xilinx•com> 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 <john.linn@xilinx•com> 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. 2008-04-02 18:00 ` Grant Likely @ 2008-04-02 18:20 ` John Linn 2008-04-02 19:27 ` Grant Likely 0 siblings, 1 reply; 7+ messages in thread From: John Linn @ 2008-04-02 18:20 UTC (permalink / raw) To: Grant Likely; +Cc: linuxppc-dev Sounds good, those are easy changes and make sense. Since I'm a newbie, I don't know any better sometimes when I copy other code that may not be as safe. =20 The same thing, of_get_property(np, "current-speed", NULL);, is done right above my code I added. =20 Should the other code in the driver using the same method be fixed, or just my patch? Thanks for your patience, John -----Original Message----- From: glikely@secretlab•ca [mailto:glikely@secretlab•ca] On Behalf Of Grant Likely Sent: Wednesday, April 02, 2008 12:00 PM To: John Linn Cc: linuxppc-dev@ozlabs•org Subject: Re: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. On Wed, Apr 2, 2008 at 10:52 AM, John Linn <john.linn@xilinx•com> 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 <john.linn@xilinx•com> 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 =3D 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 =3D resource.start; > + > + reg_offset =3D of_get_property(np, "reg-offset", NULL); > + reg_shift =3D of_get_property(np, "reg-shift", NULL); > + > + if (!reg_offset) > + port->mapbase =3D resource.start; > + else > + port->mapbase =3D resource.start + *reg_offset; > + > + if (reg_shift) > + port->regshift =3D *reg_shift; > + This is a little unsafe since it doesn't check the property size, I'd do the following instead: port->mapbase =3D resource.start /* Check for shifted address mapping */ prop =3D of_get_property(np, "reg-offset", &prop_size); if (prop && (prop_size =3D=3D sizeof(u32)) port->mapbase +=3D *prop; /* Check for registers offset within the devices address range */ prop =3D of_get_property(np, "reg-shift", &prop_size); if (prop && (prop_size =3D=3D sizeof(u32))) port->regshift =3D *prop; Cheers, g. --=20 Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. 2008-04-02 18:20 ` John Linn @ 2008-04-02 19:27 ` Grant Likely 0 siblings, 0 replies; 7+ messages in thread From: Grant Likely @ 2008-04-02 19:27 UTC (permalink / raw) To: John Linn; +Cc: linuxppc-dev On Wed, Apr 2, 2008 at 12:20 PM, John Linn <John.Linn@xilinx•com> wrote: > Sounds good, those are easy changes and make sense. > > Since I'm a newbie, I don't know any better sometimes when I copy other > code that may not be as safe. > > The same thing, of_get_property(np, "current-speed", NULL);, is done > right above my code I added. > > Should the other code in the driver using the same method be fixed, or > just my patch? It would be good to fix the other code, but not in this patch. Write another patch to fix that. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550. 2008-04-02 16:52 ` John Linn 2008-04-02 18:00 ` Grant Likely @ 2008-04-02 19:19 ` Sergei Shtylyov 1 sibling, 0 replies; 7+ messages in thread From: Sergei Shtylyov @ 2008-04-02 19:19 UTC (permalink / raw) To: John Linn; +Cc: linuxppc-dev Hello. 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 <john.linn@xilinx•com> > 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 I'm proposing you to use the already existing "big-endian" property ISO "reg-offset" (used in the nodes describing OpenPIC, for example). WBR, Sergei ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-04-03 12:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <689CB232690D8D4E97DA6C76DA098E6C05FC4762@XCO-EXCHVS1.xlnx.xilinx.com>
2008-04-02 21:39 ` [PATCH 2/3][POWERPC][V2] Xilinx: of_serial support for Xilinx uart 16550 Stephen Neuendorffer
2008-04-03 11:59 ` Sergei Shtylyov
[not found] <12071551351007-git-send-email-john.linn@xilinx.com>
[not found] ` <12071551354058-git-send-email-john.linn@xilinx.com>
2008-04-02 16:52 ` John Linn
2008-04-02 18:00 ` Grant Likely
2008-04-02 18:20 ` John Linn
2008-04-02 19:27 ` Grant Likely
2008-04-02 19:19 ` Sergei Shtylyov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox