| Message ID | 1448477209-22340-2-git-send-email-curt@cumulusnetworks.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, Nov 25, 2015 at 10:46:49AM -0800, Curt Brune wrote: > + /* Check if the ports have an ordering, defined by 'cell-index' */ > + if (((indexp = (u32*)of_get_property(np, "cell-index", &len)) != NULL) && > + (len == sizeof(u32))) > + index = *indexp; > + > /* Add port, irq will be dealt with later. We passed a translated > * IO port value. It will be fixed up later along with the irq > */ > if (tsi && !strcmp(tsi->type, "tsi-bridge")) > - return add_legacy_port(np, -1, UPIO_TSI, addr, addr, > + return add_legacy_port(np, index, UPIO_TSI, addr, addr, > NO_IRQ, legacy_port_flags, 0); > else > - return add_legacy_port(np, -1, UPIO_MEM, addr, addr, > + return add_legacy_port(np, index, UPIO_MEM, addr, addr, > NO_IRQ, legacy_port_flags, 0); What does this do if the value in cell-index is a duplicate. or the port with that number is already created some other way? Are the numbers in cell-index global anyway, or relative to some parent device (I couldn't find the documentation for this). Segher
On Wed, 2015-11-25 at 10:46 -0800, Curt Brune wrote: > Currently for PowerPC systems using device tree, the legacy serial > ports are allocated in the order by which they are found in the device > tree structure. > > This can cause problems in cases where the device tree nodes are > sysnthesized by an external program (kexec for example), which may not > preserve the original order of the device tree nodes. This in turn > can result in the serial console changing from ttyS0 to ttyS1 > unexpectedly. > > This patch uses the 'cell-index' property (if present) to map UART > device tree nodes to legacy serial port number. The cell-index > property is widely used in all the Freescale .dts files for PowerPC. > > Signed-off-by: Curt Brune <curt@cumulusnetworks.com> > --- > arch/powerpc/kernel/legacy_serial.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kernel/legacy_serial.c > b/arch/powerpc/kernel/legacy_serial.c > index 7b750c4..b717133 100644 > --- a/arch/powerpc/kernel/legacy_serial.c > +++ b/arch/powerpc/kernel/legacy_serial.c > @@ -164,6 +164,8 @@ static int __init add_legacy_soc_port(struct device_node > *np, > u64 addr; > const __be32 *addrp; > struct device_node *tsi = of_get_parent(np); > + int index = -1, len; > + u32 *indexp; > > /* We only support ports that have a clock frequency properly > * encoded in the device-tree. > @@ -188,14 +190,19 @@ static int __init add_legacy_soc_port(struct > device_node *np, > if (addr == OF_BAD_ADDR) > return -1; > > + /* Check if the ports have an ordering, defined by 'cell-index' */ > + if (((indexp = (u32*)of_get_property(np, "cell-index", &len)) != > NULL) && > + (len == sizeof(u32))) > + index = *indexp; const u32 * Or better, use of_read_number() or similar. -Scott
On Sun Dec 20 09:02, Segher Boessenkool wrote: > On Wed, Nov 25, 2015 at 10:46:49AM -0800, Curt Brune wrote: > > + /* Check if the ports have an ordering, defined by 'cell-index' */ > > + if (((indexp = (u32*)of_get_property(np, "cell-index", &len)) != NULL) && > > + (len == sizeof(u32))) > > + index = *indexp; > > + > > /* Add port, irq will be dealt with later. We passed a translated > > * IO port value. It will be fixed up later along with the irq > > */ > > if (tsi && !strcmp(tsi->type, "tsi-bridge")) > > - return add_legacy_port(np, -1, UPIO_TSI, addr, addr, > > + return add_legacy_port(np, index, UPIO_TSI, addr, addr, > > NO_IRQ, legacy_port_flags, 0); > > else > > - return add_legacy_port(np, -1, UPIO_MEM, addr, addr, > > + return add_legacy_port(np, index, UPIO_MEM, addr, addr, > > NO_IRQ, legacy_port_flags, 0); > > What does this do if the value in cell-index is a duplicate. or the port > with that number is already created some other way? Are the numbers in > cell-index global anyway, or relative to some parent device (I couldn't > find the documentation for this). > > > Segher add_legacy_port() has support for handling duplicate index entries. Reading that code it looks like the recent entry wins and the older entry is moved to the next available index. The numbers in cell-index are not global nor explicitly relative to a parent device. I also could not find documentation on this. In common usage, however, the cell-index property is used in two different ways: 1. For devices that have "channels" or "regions" (DMA for example), the parent device node has child nodes enumerated by cell-index. For example see: arch/powerpc/boot/dts/fsl/pq3-dma-0.dtsi 2. For multiple instances of a device, for example UARTS, each device node is enumerated using cell-index. See: arch/powerpc/boot/dts/fsl/qoriq-duart-0.dtsi Cheers, Curt
On Mon Dec 21 21:42, Scott Wood wrote: > On Wed, 2015-11-25 at 10:46 -0800, Curt Brune wrote: > > Currently for PowerPC systems using device tree, the legacy serial > > ports are allocated in the order by which they are found in the device > > tree structure. > > > > This can cause problems in cases where the device tree nodes are > > sysnthesized by an external program (kexec for example), which may not > > preserve the original order of the device tree nodes. This in turn > > can result in the serial console changing from ttyS0 to ttyS1 > > unexpectedly. > > > > This patch uses the 'cell-index' property (if present) to map UART > > device tree nodes to legacy serial port number. The cell-index > > property is widely used in all the Freescale .dts files for PowerPC. > > > > Signed-off-by: Curt Brune <curt@cumulusnetworks.com> > > --- > > arch/powerpc/kernel/legacy_serial.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/arch/powerpc/kernel/legacy_serial.c > > b/arch/powerpc/kernel/legacy_serial.c > > index 7b750c4..b717133 100644 > > --- a/arch/powerpc/kernel/legacy_serial.c > > +++ b/arch/powerpc/kernel/legacy_serial.c > > @@ -164,6 +164,8 @@ static int __init add_legacy_soc_port(struct device_node > > *np, > > u64 addr; > > const __be32 *addrp; > > struct device_node *tsi = of_get_parent(np); > > + int index = -1, len; > > + u32 *indexp; > > > > /* We only support ports that have a clock frequency properly > > * encoded in the device-tree. > > @@ -188,14 +190,19 @@ static int __init add_legacy_soc_port(struct > > device_node *np, > > if (addr == OF_BAD_ADDR) > > return -1; > > > > + /* Check if the ports have an ordering, defined by 'cell-index' */ > > + if (((indexp = (u32*)of_get_property(np, "cell-index", &len)) != > > NULL) && > > + (len == sizeof(u32))) > > + index = *indexp; > > const u32 * > > Or better, use of_read_number() or similar. > > -Scott > Ack. Thanks for the pointer. -Curt
On Wed, Dec 30, 2015 at 08:36:01AM -0800, Curt Brune wrote: > > What does this do if the value in cell-index is a duplicate. or the port > > with that number is already created some other way? Are the numbers in > > cell-index global anyway, or relative to some parent device (I couldn't > > find the documentation for this). > > add_legacy_port() has support for handling duplicate index entries. > Reading that code it looks like the recent entry wins and the older > entry is moved to the next available index. > > The numbers in cell-index are not global nor explicitly relative to a > parent device. I also could not find documentation on this. That sounds like it should work then. It does change behaviour for existing configs. Thanks, Segher
On Wed, Nov 25, 2015 at 12:46 PM, Curt Brune <curt@cumulusnetworks.com> wrote: > Currently for PowerPC systems using device tree, the legacy serial > ports are allocated in the order by which they are found in the device > tree structure. > > This can cause problems in cases where the device tree nodes are > sysnthesized by an external program (kexec for example), which may not > preserve the original order of the device tree nodes. This in turn > can result in the serial console changing from ttyS0 to ttyS1 > unexpectedly. > > This patch uses the 'cell-index' property (if present) to map UART > device tree nodes to legacy serial port number. The cell-index > property is widely used in all the Freescale .dts files for PowerPC. If cell-index is already present in the dtbs, then this is okay. Otherwise use aliases. cell-index is pretty much deprecated. Rob
diff --git a/arch/powerpc/kernel/legacy_serial.c b/arch/powerpc/kernel/legacy_serial.c index 7b750c4..b717133 100644 --- a/arch/powerpc/kernel/legacy_serial.c +++ b/arch/powerpc/kernel/legacy_serial.c @@ -164,6 +164,8 @@ static int __init add_legacy_soc_port(struct device_node *np, u64 addr; const __be32 *addrp; struct device_node *tsi = of_get_parent(np); + int index = -1, len; + u32 *indexp; /* We only support ports that have a clock frequency properly * encoded in the device-tree. @@ -188,14 +190,19 @@ static int __init add_legacy_soc_port(struct device_node *np, if (addr == OF_BAD_ADDR) return -1; + /* Check if the ports have an ordering, defined by 'cell-index' */ + if (((indexp = (u32*)of_get_property(np, "cell-index", &len)) != NULL) && + (len == sizeof(u32))) + index = *indexp; + /* Add port, irq will be dealt with later. We passed a translated * IO port value. It will be fixed up later along with the irq */ if (tsi && !strcmp(tsi->type, "tsi-bridge")) - return add_legacy_port(np, -1, UPIO_TSI, addr, addr, + return add_legacy_port(np, index, UPIO_TSI, addr, addr, NO_IRQ, legacy_port_flags, 0); else - return add_legacy_port(np, -1, UPIO_MEM, addr, addr, + return add_legacy_port(np, index, UPIO_MEM, addr, addr, NO_IRQ, legacy_port_flags, 0); }
Currently for PowerPC systems using device tree, the legacy serial ports are allocated in the order by which they are found in the device tree structure. This can cause problems in cases where the device tree nodes are sysnthesized by an external program (kexec for example), which may not preserve the original order of the device tree nodes. This in turn can result in the serial console changing from ttyS0 to ttyS1 unexpectedly. This patch uses the 'cell-index' property (if present) to map UART device tree nodes to legacy serial port number. The cell-index property is widely used in all the Freescale .dts files for PowerPC. Signed-off-by: Curt Brune <curt@cumulusnetworks.com> --- arch/powerpc/kernel/legacy_serial.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)