diff mbox series

[v7,10/11] drivers/char: suspend handling in XHCI console driver

Message ID 927d01aa54cf6f5291e506179e3d15dc32ebad40.1663383053.git-series.marmarek@invisiblethingslab.com (mailing list archive)
State Superseded
Headers show
Series Add Xue - console over USB 3 Debug Capability | expand

Commit Message

Marek Marczykowski-Górecki Sept. 17, 2022, 2:51 a.m. UTC
Similar to the EHCI driver - save/restore relevant BAR and command
register, re-configure DbC on resume and stop/start timer.
On resume trigger sending anything that was queued in the meantime.
Save full BAR value, instead of just the address part, to ease restoring
on resume.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
New in v7

Without this patch, the console is broken after S3, and in some cases
the suspend doesn't succeed at all (when xhci console is enabled).

Very similar (if not the same) functions might be used for coordinated
reset handling. I tried to include it in this patch too, but it's a bit
more involved, mostly due to share=yes case (PHYSDEVOP_dbgp_op can be
called by the hardware domain only).
---
 xen/drivers/char/xhci-dbc.c | 55 +++++++++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 6 deletions(-)

Comments

Jan Beulich Sept. 20, 2022, 3:07 p.m. UTC | #1
On 17.09.2022 04:51, Marek Marczykowski-Górecki wrote:
> @@ -259,6 +259,9 @@ struct dbc {
>      bool open;
>      enum xhci_share share;
>      unsigned int xhc_num; /* look for n-th xhc */
> +    /* state saved across suspend */
> +    bool suspended;
> +    uint16_t pci_cr;
>  };

While perhaps of limited relevance right here, may I nevertheless
suggest to move the new boolean next to the other one in context.
That's both to avoid setting a bad precedent (of not using available
padding slots for data like this) and because imo the comment also
isn't really applicable to this field (but just the other one you
add). Preferably with that adjustment
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan
diff mbox series

Patch

diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c
index 5f92234a9594..81a4fd5b12c3 100644
--- a/xen/drivers/char/xhci-dbc.c
+++ b/xen/drivers/char/xhci-dbc.c
@@ -251,7 +251,7 @@  struct dbc {
     struct xhci_string_descriptor *dbc_str;
 
     pci_sbdf_t sbdf;
-    uint64_t xhc_mmio_phys;
+    uint64_t bar_val;
     uint64_t xhc_dbc_offset;
     void __iomem *xhc_mmio;
 
@@ -259,6 +259,9 @@  struct dbc {
     bool open;
     enum xhci_share share;
     unsigned int xhc_num; /* look for n-th xhc */
+    /* state saved across suspend */
+    bool suspended;
+    uint16_t pci_cr;
 };
 
 static void *dbc_sys_map_xhc(uint64_t phys, size_t size)
@@ -358,8 +361,9 @@  static bool __init dbc_init_xhc(struct dbc *dbc)
 
     pci_conf_write16(dbc->sbdf, PCI_COMMAND, cmd);
 
-    dbc->xhc_mmio_phys = (bar0 & PCI_BASE_ADDRESS_MEM_MASK) | (bar1 << 32);
-    dbc->xhc_mmio = dbc_sys_map_xhc(dbc->xhc_mmio_phys, xhc_mmio_size);
+    dbc->bar_val = bar0 | (bar1 << 32);
+    dbc->xhc_mmio = dbc_sys_map_xhc(dbc->bar_val & PCI_BASE_ADDRESS_MEM_MASK,
+                                    xhc_mmio_size);
 
     if ( dbc->xhc_mmio == NULL )
         return false;
@@ -979,6 +983,9 @@  static bool dbc_ensure_running(struct dbc *dbc)
     uint32_t ctrl;
     uint16_t cmd;
 
+    if ( dbc->suspended )
+        return false;
+
     if ( dbc->share != XHCI_SHARE_NONE )
     {
         /*
@@ -1213,9 +1220,11 @@  static void __init cf_check dbc_uart_init_postirq(struct serial_port *port)
      * page, so keep it simple.
      */
     if ( rangeset_add_range(mmio_ro_ranges,
-                PFN_DOWN(uart->dbc.xhc_mmio_phys + uart->dbc.xhc_dbc_offset),
-                PFN_UP(uart->dbc.xhc_mmio_phys + uart->dbc.xhc_dbc_offset +
-                       sizeof(*uart->dbc.dbc_reg)) - 1) )
+                PFN_DOWN((uart->dbc.bar_val & PCI_BASE_ADDRESS_MEM_MASK) +
+                         uart->dbc.xhc_dbc_offset),
+                PFN_UP((uart->dbc.bar_val & PCI_BASE_ADDRESS_MEM_MASK) +
+                       uart->dbc.xhc_dbc_offset +
+                sizeof(*uart->dbc.dbc_reg)) - 1) )
         printk(XENLOG_INFO
                "Error while adding MMIO range of device to mmio_ro_ranges\n");
 #endif
@@ -1255,6 +1264,38 @@  static void cf_check dbc_uart_flush(struct serial_port *port)
         set_timer(&uart->timer, goal);
 }
 
+static void cf_check dbc_uart_suspend(struct serial_port *port)
+{
+    struct dbc_uart *uart = port->uart;
+    struct dbc *dbc = &uart->dbc;
+
+    dbc_pop_events(dbc);
+    stop_timer(&uart->timer);
+    dbc->pci_cr = pci_conf_read16(dbc->sbdf, PCI_COMMAND);
+    dbc->suspended = true;
+}
+
+static void cf_check dbc_uart_resume(struct serial_port *port)
+{
+    struct dbc_uart *uart = port->uart;
+    struct dbc *dbc = &uart->dbc;
+
+    pci_conf_write32(dbc->sbdf, PCI_BASE_ADDRESS_0, dbc->bar_val & 0xFFFFFFFF);
+    pci_conf_write32(dbc->sbdf, PCI_BASE_ADDRESS_1, dbc->bar_val >> 32);
+    pci_conf_write16(dbc->sbdf, PCI_COMMAND, dbc->pci_cr);
+
+    if ( !dbc_init_dbc(dbc) )
+    {
+        dbc_error("resume failed\n");
+        return;
+    }
+
+    dbc_enable_dbc(dbc);
+    dbc->suspended = false;
+    dbc_flush(dbc, &dbc->dbc_oring, &dbc->dbc_owork);
+    set_timer(&uart->timer, NOW() + MICROSECS(DBC_POLL_INTERVAL));
+}
+
 static struct uart_driver dbc_uart_driver = {
     .init_preirq = dbc_uart_init_preirq,
     .init_postirq = dbc_uart_init_postirq,
@@ -1262,6 +1303,8 @@  static struct uart_driver dbc_uart_driver = {
     .putc = dbc_uart_putc,
     .getc = dbc_uart_getc,
     .flush = dbc_uart_flush,
+    .suspend = dbc_uart_suspend,
+    .resume = dbc_uart_resume,
 };
 
 /* Those are accessed via DMA. */