Hi Marek,
> -----Original Message-----
> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of
> Subject: [PATCH v8 2/2] drivers/char: suspend handling in XHCI console
> driver
>
> 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
> <marma...@invisiblethingslab.com>
> Acked-by: Jan Beulich <jbeul...@suse.com>
As per Matrix discussion, this is the only one left in this series and
according to Andrew, this patch was already acked and entirely
contained within the new driver itself, fixing a real bug, so if no
objections from other maintainers:
Release-acked-by: Henry Wang <henry.w...@arm.com>
Kind regards,
Henry
> ---
> Changes in v8:
> - move 'bool suspended' to other bools
> 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(-)
>
> diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c
> index 43ed64a004e2..86f6df6bef67 100644
> --- a/xen/drivers/char/xhci-dbc.c
> +++ b/xen/drivers/char/xhci-dbc.c
> @@ -251,14 +251,17 @@ 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;
>
> bool enable; /* whether dbgp=xhci was set at all */
> bool open;
> + bool suspended;
> enum xhci_share share;
> unsigned int xhc_num; /* look for n-th xhc */
> + /* state saved across suspend */
> + 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. */
> --
> git-series 0.9.1
