[Qemu-devel] [Bug 1753314] Re: UART in sabrelite machine simulation doesn't work with VxWorks 7

[Peter Maydell]

Hi. Thanks for this patch. I've had a quick look at it against the imx
datasheet, and here are my comments:

* Firstly, we can't do anything with this patch without a Signed-off-by:
line from you. In QEMU's process this is how people submitting code
state that you're legally OK to contribute the code under QEMU's license
and for it to go into QEMU.

 * Secondly, it would be very helpful if you could send patches as a
simple patch format, rather than as a zipfile, and to the QEMU mailing
list. https://wiki.qemu.org/Contribute/SubmitAPatch has our guidelines
on this.

 * Simply adding a new VMSTATE_UINT32() field will break migration. It's
better to put the new field into its own vmstate subsection so that this
doesn't happen; see docs/devel/migration.rst. If we don't care about
cross-version migration we could just bump the
version_id/minimum_version_id fields.

 * If you run scripts/checkpatch.pl over your patch it should warn you
about minor coding style issues. For instance we prefer all if()
statements to use {}, even if there's only one line in them.

 * Your change to imx_update() does this:
+    if (s->ucr4 & UCR4_TXEN)
+        flags |= USR1_TRDY;

but that isn't what the spec says UCR4_TXEN does; it says we raise an
interrupt only if UCR4_TXEN and USR2_TXDC are both high.

 * the imx_update() function is already rather confused in how it
handles the 'flags' variable, and this change extends that confusion.
The function is trying to treat 'flags' as a single set of interrupt
flag bits, but the device doesn't actually have a single unified set of
interrupt flags like that, they're spread over UTS and USR1 and USR2.
The code as it is looks odd -- should USR1.TXMPTYEN == 0 really suppress
USR1_TRDY interrupts ? I suspect this is a bug, and we should also clean
things up to make 'flags' be a bool.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1753314

Title:
  UART in sabrelite machine simulation doesn't work with VxWorks 7

Status in QEMU:
  New

Bug description:
  The imx_serial.c driver currently implements only partial support for
  the i.MX6 UART hardware. (I understand it's a work in progress and
  that's fine.) dIn particular, it does not implement support for the
  Transmit Complete Interrupt Enable bit in the UCR4 register. The
  VxWorks 7 i.MX6 serial driver depends on the behavior of this bit in
  actual hardware in order to send characters through the UART
  correctly. The result is that with the current machine model, VxWorks
  will boot and run in QEMU but it's unable to print any characters to
  the console serial port.

  I have produced a small patch for the imx_serial.c module to make it
  nominally functional with VxWorks 7. It works well enough to allow the
  boot banner to appear and for the user to interact with the target
  shell.

  I'm not submitting this as a patch to the development list as I'm not
  fully certain it complies with the hardware spec and doesn't break any
  other functionality. I would prefer if the maintainer (or someone)
  reviewed it for any issues/refinements first.

  I'm attaching the patch to this bug report. A copy can also be
  obtained from:

  http://people.freebsd.org/~wpaul/qemu/imx_serial.zip

  This patch was generated against QEMU 2.11.0 but also works with QEMU
  2.11.1.

  My host environment is FreeBSD/amd64 11.1-RELEASE with QEMU
  2.11.0/2.11.11 built from source.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1753314/+subscriptions