[Patch] Add support for UART FIFO status registers

Bug #970252 reported by Jan Vesely
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Linaro QEMU
Fix Released
Low
Unassigned

Bug Description

AM/DM37x Multimedia Device Silicon Revision 1.x Version O Technical Reference Manual page 2951
describes UART RO registers at offsets 0x64 and 0x68, that report FIFO levels.

Using these registers one can monitor the FIFO levels and handle arbitrary number of recieved characters in interrupt handler (wihtout knowing the value of the FIFO trigger level).

The attached patch adds fifo level report for the serial device and uses this functionality in omap_uart device.

Revision history for this message
Peter Maydell (pmaydell) wrote :

Thanks. Do you have a test case for this feature?

Revision history for this message
Jan Vesely (jan.vesely) wrote :

I work on porting HelenOS(www.helenos.org) to beagleboardxm and use this simple loop in the interrupt handler:

        while ((uart->regs->rx_fifo_lvl)) {
                const uint8_t val = uart->regs->rhr;
                if (uart->indev && val) {
                        indev_push_character(uart->indev, val);
                }
        }

it works on beagleboardxm hw and after applying the patch it works in the same way in linaro-qemu (today's git).

Other than OS driver I don't know what such a test case should look like.

Note that this is a very simple way to add this functionality to allow such interrupt handling. Those registers are not available on omap35x and I found no way to limit them to beagleboardxm emulation target.

Revision history for this message
Peter Maydell (pmaydell) wrote :

Well, if you can provide an sd card image and qemu command line for HelenOS that would do.

The right way to limit this to omap36xx/37xx is to define a new qdev property on the omap_uart device whose value would be the UART revision ID (ie the 8 bit value to be returned by MVR_REG). [I'm assuming the 36xx/37xx OMAPs with these registers have a different MVR_REG...] Then in omap3.c you can conditionally set the revision property to the right value depending on s->mpu_model, and in omap_uart.c you can make the registers only appear if the revision is set to the right value.
(The 'revision' property on omap2-intc is an example of how to do it, although we don't actually drive any change in behaviour off it in that case.)

Revision history for this message
Jan Vesely (jan.vesely) wrote :

I have added revision destinction to my patch.
datasheets mention only versions 1.0 2.0 3.0.
3.0 is mentioned in omap35xx, omap36xx and omap37xx trm, so I used value (5.2) that is reported by the bbxm hw I have.

It might take some time until I have a usable image that could be used as a test case. Should I post it here when I have one? (I expect it to be cca 10MB)

Revision history for this message
Peter Maydell (pmaydell) wrote :

TI's TRMs always seem to provide only "example" revision numbers to demonstrate the format of the register -- they don't seem to bear any resemblance to the actual values in the hardware. I checked on a beagleboard which has an omap35xx, and it reports an MVR of 0x46.

So I guess we should:
 * leave the default revision property at 0x30 (which is probably still bogus for omap1/omap2 but doesn't change existing qemu behaviour)
 * have the omap34xx/35xx set the revision to 0x46
 * have omap36xx/37xx set revision to 0x52
 * enable new registers for revision 0x52
[your patch does most of that already, so I think it's just setting the revision to 0x46 for 34xx/35xx that's still todo.]

The thing I definitely need from you is a Signed-off-by: line for this patch (
http://lxr.linux.no/#linux+v3.3.1/Documentation/SubmittingPatches#L297 -- basically it's just certifying that you wrote it and have the right to submit it under the license that applies to QEMU).

Optional extras:
 * a git commit message
 * you'll find that your patch fails to pass ./scripts/checkpatch.pl -- all if statements need braces, even single line ones.
 * you've added a stray blank line to hw/pc.h

10MB is a bit big for an attachment I guess -- if you have a handy place to put it you could just provide a link to it. If you don't have a suitable server then go ahead and attach it to the bug.

Revision history for this message
Jan Vesely (jan.vesely) wrote :

Thank you for your review. I ahve split the patch in two. The first one adds support for variable revision value. The second one fifo level registers if the revision is >= 0x52. This time I used git to generate them so signed-off lines and commit messages are included. Both patches pass checkpatch.pl.

HelenOS uImage file is 5MB. I was able to find a place to make it accessible for download, but that might be timelimited.
http://www.ms.mff.cuni.cz/~vesej5bm/uImage.bin

I uploaded a current state snapshot (only kernel debug console on uart3 is usable) if it's enough as a test case.
Without the patches qemu spams "omap_uart_read: Bad register access 0x000064" messages after first keypress,
applying the patches makes the console work as expected (there are few commands, like help).

Revision history for this message
Jan Vesely (jan.vesely) wrote :
Revision history for this message
Jan Vesely (jan.vesely) wrote :
Revision history for this message
Peter Maydell (pmaydell) wrote :

Thanks for the patches. They look good to me so I have added them to qemu-linaro.

Your test image doesn't seem to be an SD card image. It looked like a u-boot u-image so I tried sticking it into an sd card image in place of the linux u-image, but that didn't boot:

cam-vm-266:maverick:qemu$ ./arm-softmmu/qemu-system-arm -M beaglexm -drive if=sd,cache=writeback,file=/home/petmay01/linaro/helenos/helenos.img -clock unix -serial stdio

Texas Instruments X-Loader 1.5.1 (Jul 26 2011 - 00:39:12)
Beagle xM
Reading boot sector
Loading u-boot.bin from mmc

U-Boot 2011.06 (Aug 19 2011 - 17:43:34)

OMAP36XX/37XX-GP ES2.0, CPU-OPP2, L3-165MHz, Max CPU Clock 1 Ghz
OMAP3 Beagle board + LPDDR/NAND
I2C: ready
DRAM: 512 MiB
NAND: 256 MiB
MMC: OMAP SD/MMC: 0
*** Warning - bad CRC, using default environment

ERROR : Unsupport USB mode
Check that mini-B USB cable is attached to the device
In: serial
Out: serial
Err: serial
Beagle xM Rev A
No EEPROM on expansion board
Die ID #51454d5551454d555400000051454d55
Hit any key to stop autoboot: 0
SD/MMC found on device 0
reading boot.scr

508 bytes read
Running bootscript from mmc ...
## Executing script at 82000000
reading uImage

5162040 bytes read
reading uInitrd

1854857 bytes read
reading board.dtb

316 bytes read
## Booting kernel from Legacy Image at 80000000 ...
   Image Name: HelenOS-0.4.3
   Image Type: ARM NetBSD Kernel Image (uncompressed)
   Data Size: 5161976 Bytes = 4.9 MiB
   Load Address: 80000000
   Entry Point: 80000000
   Verifying Checksum ... OK
   XIP Kernel Image ... OK
OK
## Transferring control to NetBSD stage-2 loader (at address 80000000) ...

...and then nothing more. Can you provide a qemu command line and whatever image file is needed to use with that command line, please?

Changed in qemu-linaro:
status: New → Fix Committed
milestone: none → 2012.04
importance: Undecided → Low
Revision history for this message
Jan Vesely (jan.vesely) wrote :

Hi
thank you for pushing those changes.
The image I prepared is indeed an u-boot image. I had problems creating my own sdcard images so I just download beagle-nano.img and use that one.
Other than adding uImage.bin a I have a uEnv.txt file with these lines:

bootfile=uImage.bin
address_uimage=0x82000000
uenvcmd=fatload mmc 0:1 ${address_uimage} ${bootfile}; bootm ${address_uimage}

the problem appers to be that u-boot uploaded the image to address 0x80000000
and that is the starting address. thus no copying was done(XIP flag), but execution at address 0x80000000
failed baceuse the 64B uImage header is still there.

I will provide an SD card image as soon as I have one that works in qemu.

Peter Maydell (pmaydell)
Changed in qemu-linaro:
status: Fix Committed → Fix Released
Revision history for this message
Jan Vesely (jan.vesely) wrote :

here is the sd card image I promised:
http://www.ms.mff.cuni.cz/~vesej5bm/HelenOS.img

it's a bit bigger than necessary, that's because of the fat32

the command line I use to run it:
qemu-system-arm -s -M beaglexm -serial stdio -sd HelenOS.img

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.