Re: [PATCH] booting-without-of: add Xilinx uart 16550.
Hello. Stephen Neuendorffer wrote: Instead of attempting to come up with a generic description of this, I recommend just naming it after the actual device instance; something like compatible=xlnx,opb-uart16550; Well, that means that we'll need a to add a code which glues the chip to 8250.c driver... well, of_serial.c could be that glue layer if we add to it the ability to recognize Xilinx UART... well, legacy_serial.c could be taught that trick too... Well, we could also add the new compatible, but still claim ns16550 compatibility... This actually makes more sense to me... I'd rather have the code set the reg-shift than have it explicitly set in the device tree anyway. The compatibility set should include (at the least): opb_uart16550_v1_00_c opb_uart16550_v1_00_d opb_uart16550_v1_00_e plb_uart16550_v1_00_c xps_uart16550_v1_00_a Sounds like too much? Couldn't this be handled via the model prop? I think this is somewhat independent of Sergei's arguments that generic ns16550 devices should allow having a reg-shift set Steve WBR, Sergei ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH] booting-without-of: add Xilinx uart 16550.
+ - reg-shift : registers offset shift (standard uart_port field). + Property is optional if regshift is zero. I was hoping to get an idea of what is required here, or when I might use it? It looks like the ARCH=ppc code instantiates this with a reg-shift of 2... Is this the expected value? When would it be not zero? or not two? Steve ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add Xilinx uart 16550.
Stephen Neuendorffer wrote: + - reg-shift : registers offset shift (standard uart_port field). + Property is optional if regshift is zero. I was hoping to get an idea of what is required here, or when I might use it? It looks like the ARCH=ppc code instantiates this with a reg-shift of 2... Is this the expected value? Yes, reg-shift = 2 should be set for Xilinx 16550 uart. Should I add this to patch? BTW regshift=2 is hardcoded for uartlite. When would it be not zero? or not two? Sorry, it seems I don't follow here. -- Regards, Pavel ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH] booting-without-of: add Xilinx uart 16550.
-Original Message- From: Pavel Kiryukhin [mailto:[EMAIL PROTECTED] Sent: Friday, February 15, 2008 9:41 AM To: Stephen Neuendorffer Cc: linuxppc-dev@ozlabs.org Subject: Re: [PATCH] booting-without-of: add Xilinx uart 16550. Stephen Neuendorffer wrote: + - reg-shift : registers offset shift (standard uart_port field). + Property is optional if regshift is zero. I was hoping to get an idea of what is required here, or when I might use it? It looks like the ARCH=ppc code instantiates this with a reg-shift of 2... Is this the expected value? Yes, reg-shift = 2 should be set for Xilinx 16550 uart. Should I add this to patch? Yes, please! BTW regshift=2 is hardcoded for uartlite. When would it be not zero? or not two? Sorry, it seems I don't follow here. I think all that needs to get added is 'For the Xilinx uart 16550 cores, reg-shift must be set to 2' Steve ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add Xilinx uart 16550.
On Sat Feb 16 00:40:01 EST 2008, Pavel Kiryukhin pkiryukhin wrote: Add uart 16550 properties description to Xilinx portion of booting-without-of.txt This patch description is a bit weak. How about adding what properties are being added. Also, as described below, it's going to become more than just adding a property. Signed-off-by: Pavel Kiryukhin pkiryukhin at ru.mvista.com --- Documentation/powerpc/booting-without-of.txt | 16 ++-- 1 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt index 7b4e8a7..dd77bbc 100644 --- a/Documentation/powerpc/booting-without-of.txt +++ b/Documentation/powerpc/booting-without-of.txt @@ -2575,10 +2575,22 @@ platforms are moved over to use the flattened-device-tree model. Xilinx uartlite devices are simple fixed speed serial ports. - Requred properties: + Required properties: - current-speed : Baud rate of uartlite - v) Xilinx hwicap + v) Xilinx Uart 16550 + + Xilinx uart 16550 device registers are compatible with all standard 16540 + and 16550 UARTs. + + Required properties: + - current-speed : Baud rate of uart. + - clock-frequency : Baud rate generator reference clock. May be driven + by OPB_Clk (100 MHz). Freqency of the reference clock generating the baud rate. (Sometimes this is connected to the OBP_Clk which is usually 100MHz). This is not the documentation for the macro, its the documentation for the device tree. + - reg-shift : registers offset shift (standard uart_port field). + Property is optional if regshift is zero. As Steven said, you must define what the property means (don't say that the value should be 2 on chip x, describe how to determine the correct value and how to use it). When this property is present, the address bits selecting the different 16550 registers are shifted left this many bits. Also, since this is not register compatible with previous the previous ns16550 binding, we need to use a new compatible string. ns16550-shifted or ns16550-offset would be ok with me. When we define this node, refer to 16550, and state that we expect nodes that have a zero shfit should include the ns16550 string. Looking in the bigger context, this binding is not Xilinx specific; but I don't see an obviousy better place and am not going to do more than mention that issue. milton ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add Xilinx uart 16550.
Grant Likely wrote: Add uart 16550 properties description to Xilinx portion of booting-without-of.txt Signed-off-by: Pavel Kiryukhin [EMAIL PROTECTED] --- Documentation/powerpc/booting-without-of.txt | 16 ++-- 1 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt index 7b4e8a7..dd77bbc 100644 --- a/Documentation/powerpc/booting-without-of.txt +++ b/Documentation/powerpc/booting-without-of.txt @@ -2575,10 +2575,22 @@ platforms are moved over to use the flattened-device-tree model. Xilinx uartlite devices are simple fixed speed serial ports. - Requred properties: + Required properties: - current-speed : Baud rate of uartlite - v) Xilinx hwicap + v) Xilinx Uart 16550 + + Xilinx uart 16550 device registers are compatible with all standard 16540 + and 16550 UARTs. Not strictly true; the xilinx uart is *almost* compatible with the ns16550. The same driver can be made to work, but it is not register level compatible so we cannot claim compatible=ns16550. How much incompatible it is with 16550? Does that only extend to register stride of 4 instead of 1 -- if so, it should be considered compatible since the same chip can be into address space mapped with stride of 1, 2, or 4, or whatever power of 2. If it has some actual register difference, like e. g. DLAB not existing and the divisor latch mapped to a separate register rather than 0..1, then indeed, new compatible property must be defined. We need a new compatible property for 16550 like devices with a reg shift and offset. No, we don't strictly need it if all incompatibilty is constrained to how the same 16550 registers mapped to address space which is a function of the address decoder, not the chip itself. Well, that's of course based in 8250.c's ability to handle different strides -- an imaginary driver could only handle 1:1 chip mapping. Instead of attempting to come up with a generic description of this, I recommend just naming it after the actual device instance; something like compatible=xlnx,opb-uart16550; Well, that means that we'll need a to add a code which glues the chip to 8250.c driver... well, of_serial.c could be that glue layer if we add to it the ability to recognize Xilinx UART... well, legacy_serial.c could be taught that trick too... Well, we could also add the new compatible, but still claim ns16550 compatibility... Cheers, g. WBR, Sergei ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add Xilinx uart 16550.
On Fri, Feb 15, 2008 at 11:56 AM, Sergei Shtylyov [EMAIL PROTECTED] wrote: Grant Likely wrote: + Xilinx uart 16550 device registers are compatible with all standard 16540 + and 16550 UARTs. Not strictly true; the xilinx uart is *almost* compatible with the ns16550. The same driver can be made to work, but it is not register level compatible so we cannot claim compatible=ns16550. How much incompatible it is with 16550? Does that only extend to register stride of 4 instead of 1 -- if so, it should be considered compatible since the same chip can be into address space mapped with stride of 1, 2, or 4, or whatever power of 2. If it has some actual register difference, like e. g. DLAB not existing and the divisor latch mapped to a separate register rather than 0..1, then indeed, new compatible property must be defined. The definition of compatible (from the OpenFirmware) docs is that the *device* is register level compatible. That includes register locations and offsets. The registers are not at the same location, therefore it is not compatible. However, the *driver* can be easily made compatible with the devices. We just teach the driver to bind against both ns16550 and whatever value is chosen for these reg-shifted devices. Not a big deal. We need a new compatible property for 16550 like devices with a reg shift and offset. No, we don't strictly need it if all incompatibilty is constrained to how the same 16550 registers mapped to address space which is a function of the address decoder, not the chip itself. Well, that's of course based in 8250.c's ability to handle different strides -- an imaginary driver could only handle 1:1 chip mapping. compatible also covers bus binding when it is a memory mapped device. Otherwise you need another node between the bus and the ns16550 type device that does translation from the wide stride (regshift=2) to the ns16550 register definitions (regshift=0). Instead of attempting to come up with a generic description of this, I recommend just naming it after the actual device instance; something like compatible=xlnx,opb-uart16550; Well, that means that we'll need a to add a code which glues the chip to 8250.c driver... well, of_serial.c could be that glue layer if we add to it the ability to recognize Xilinx UART... well, legacy_serial.c could be taught that trick too... Well, we could also add the new compatible, but still claim ns16550 compatibility... No, we cannot because it is not register level compatible (and once again, that definition includes the stride between registers) Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add Xilinx uart 16550.
On Fri, Feb 15, 2008 at 6:40 AM, Pavel Kiryukhin [EMAIL PROTECTED] wrote: Add uart 16550 properties description to Xilinx portion of booting-without-of.txt Signed-off-by: Pavel Kiryukhin [EMAIL PROTECTED] --- Documentation/powerpc/booting-without-of.txt | 16 ++-- 1 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt index 7b4e8a7..dd77bbc 100644 --- a/Documentation/powerpc/booting-without-of.txt +++ b/Documentation/powerpc/booting-without-of.txt @@ -2575,10 +2575,22 @@ platforms are moved over to use the flattened-device-tree model. Xilinx uartlite devices are simple fixed speed serial ports. - Requred properties: + Required properties: - current-speed : Baud rate of uartlite - v) Xilinx hwicap + v) Xilinx Uart 16550 + + Xilinx uart 16550 device registers are compatible with all standard 16540 + and 16550 UARTs. Not strictly true; the xilinx uart is *almost* compatible with the ns16550. The same driver can be made to work, but it is not register level compatible so we cannot claim compatible=ns16550. We need a new compatible property for 16550 like devices with a reg shift and offset. Instead of attempting to come up with a generic description of this, I recommend just naming it after the actual device instance; something like compatible=xlnx,opb-uart16550; Cheers, g. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add Xilinx uart 16550.
Grant Likely wrote: + Xilinx uart 16550 device registers are compatible with all standard 16540 + and 16550 UARTs. Not strictly true; the xilinx uart is *almost* compatible with the ns16550. The same driver can be made to work, but it is not register level compatible so we cannot claim compatible=ns16550. How much incompatible it is with 16550? Does that only extend to register stride of 4 instead of 1 -- if so, it should be considered compatible since the same chip can be into address space mapped with stride of 1, 2, or 4, or whatever power of 2. If it has some actual register difference, like e. g. DLAB not existing and the divisor latch mapped to a separate register rather than 0..1, then indeed, new compatible property must be defined. The definition of compatible (from the OpenFirmware) docs is that the *device* is register level compatible. That includes register locations and offsets. I'd disagree here: register offsets are the function of the chip select, not a chip itself -- you can possible wire chip selects to 16550 so that the registers would be spaced by 4 bytes. The registers are not at the same location, therefore it is not compatible. However, the *driver* can be easily made compatible with the devices. We just teach the driver to bind against both ns16550 and whatever value is chosen for these reg-shifted devices. Not a big deal. You're going to teach 8250.c? Good luck. :-) IMO we can only teach a glue layer which passes UARTs to 8250.c via platform devices. We need a new compatible property for 16550 like devices with a reg shift and offset. No, we don't strictly need it if all incompatibilty is constrained to how the same 16550 registers mapped to address space which is a function of the address decoder, not the chip itself. Well, that's of course based in 8250.c's ability to handle different strides -- an imaginary driver could only handle 1:1 chip mapping. compatible also covers bus binding when it is a memory mapped device. Otherwise you need another node between the bus and the ns16550 type device that does translation from the wide stride (regshift=2) to the ns16550 register definitions (regshift=0). The chip can be connected to the bus (via chip select circuitry) in different ways, therefore we need a glue node for that circuitry? Instead of attempting to come up with a generic description of this, I recommend just naming it after the actual device instance; something like compatible=xlnx,opb-uart16550; Well, that means that we'll need a to add a code which glues the chip to 8250.c driver... well, of_serial.c could be that glue layer if we add to it the ability to recognize Xilinx UART... well, legacy_serial.c could be taught that trick too... Well, we could also add the new compatible, but still claim ns16550 compatibility... No, we cannot because it is not register level compatible (and once again, that definition includes the stride between registers) Once again, I disagree. :-) Cheers, g. WBR, Sergei ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] booting-without-of: add Xilinx uart 16550.
On Fri, Feb 15, 2008 at 12:14 PM, Sergei Shtylyov [EMAIL PROTECTED] wrote: Grant Likely wrote: The registers are not at the same location, therefore it is not compatible. However, the *driver* can be easily made compatible with the devices. We just teach the driver to bind against both ns16550 and whatever value is chosen for these reg-shifted devices. Not a big deal. You're going to teach 8250.c? Good luck. :-) :-P As you already know, 8250.c already knows how to handle reg shifts. But that is not what this conversation is about. IMO we can only teach a glue layer which passes UARTs to 8250.c via platform devices. That's right. This discussion is only about the device tree binding. It is not about the core driver. compatible also covers bus binding when it is a memory mapped device. Otherwise you need another node between the bus and the ns16550 type device that does translation from the wide stride (regshift=2) to the ns16550 register definitions (regshift=0). The chip can be connected to the bus (via chip select circuitry) in different ways, therefore we need a glue node for that circuitry? I'm not arguing that we want a glue node. What I'm arguing is that compatible=ns16550 has a very specific meaning that has become a defacto standard over the years. To add a new interpretation of that meaning is problematic. Instead, if reg shift is needed then use a different value for compatible. It's not like we're talking about something that is hard to do. It is simply being explicit in the device tree layout. Historically ns16550 means no reg shift, so don't use it for devices that have a reg shift. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
RE: [PATCH] booting-without-of: add Xilinx uart 16550.
Instead of attempting to come up with a generic description of this, I recommend just naming it after the actual device instance; something like compatible=xlnx,opb-uart16550; Well, that means that we'll need a to add a code which glues the chip to 8250.c driver... well, of_serial.c could be that glue layer if we add to it the ability to recognize Xilinx UART... well, legacy_serial.c could be taught that trick too... Well, we could also add the new compatible, but still claim ns16550 compatibility... This actually makes more sense to me... I'd rather have the code set the reg-shift than have it explicitly set in the device tree anyway. The compatibility set should include (at the least): opb_uart16550_v1_00_c opb_uart16550_v1_00_d opb_uart16550_v1_00_e plb_uart16550_v1_00_c xps_uart16550_v1_00_a I think this is somewhat independent of Sergei's arguments that generic ns16550 devices should allow having a reg-shift set Steve ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
