Re: [PATCH v2 2/2] hw/usb/hcd-xhci-pci: Add TI TUSB73X0 XHCI controller model

[Philippe Mathieu-Daudé]

On 19/12/24 01:48, Bernhard Beschow wrote:
> Am 12. Dezember 2024 08:52:07 UTC schrieb Nicholas Piggin <npiggin@gmail.com>:
> > The TI TUSB73X0 controller has some interesting differences from NEC,
> > notably a separate BAR for MSIX, and PM capabilities. The spec is freely
> > available without sign-up.
> >
> > This controller is accepted by IBM Power proprietary firmware and
> > software (when the subsystem IDs are set to Power servers, which is not
> > done here). IBM code is picky about device support, so the NEC device
> > can not be used.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > include/hw/pci/pci_ids.h        |  1 +
> > include/hw/usb/xhci.h           |  1 +
> > hw/usb/hcd-xhci-ti.c            | 92 +++++++++++++++++++++++++++++++++
> > tests/qtest/usb-hcd-xhci-test.c | 21 +++++---
> > hw/usb/Kconfig                  |  5 ++
> > hw/usb/meson.build              |  1 +
> > 6 files changed, 115 insertions(+), 6 deletions(-)
> > create mode 100644 hw/usb/hcd-xhci-ti.c


> > diff --git a/hw/usb/hcd-xhci-ti.c b/hw/usb/hcd-xhci-ti.c
> > new file mode 100644
> > index 00000000000..6d4b44f6aaf
> > --- /dev/null
> > +++ b/hw/usb/hcd-xhci-ti.c
> > @@ -0,0 +1,92 @@
> > +/*
> > + * USB xHCI controller emulation
> > + * Datasheet https://www.ti.com/product/TUSB7340
> > + *
> > + * Copyright (c) 2011 Securiforest
> > + * Date: 2011-05-11 ;  Author: Hector Martin <hector@marcansoft.com>
> > + * Based on usb-xhci-nec.c, emulates TI TUSB73X0
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/usb.h"
> > +#include "qemu/module.h"
> > +#include "hw/pci/pci.h"
> > +#include "hw/qdev-properties.h"
> > +
> > +#include "hcd-xhci-pci.h"
> > +
> > +OBJECT_DECLARE_SIMPLE_TYPE(XHCITiState, TI_XHCI)
> > +
> > +struct XHCITiState {
> > +    /*< private >*/
> > +    XHCIPciState parent_obj;
> > +    /*< public >*/
>
> These markers are obsolete. Instead, a blank line after parent_obj should be 
> inserted.
>
> > +    uint32_t intrs;
> > +    uint32_t slots;
> > +};
> > +
> > +static Property ti_xhci_properties[] = {
>
> s/static Property/static const Property/ as of recent tree-wide changes.
>
> Best regards,
> Bernhard
>
> > +    DEFINE_PROP_ON_OFF_AUTO("msi", XHCIPciState, msi, ON_OFF_AUTO_AUTO),
> > +    DEFINE_PROP_ON_OFF_AUTO("msix", XHCIPciState, msix, ON_OFF_AUTO_AUTO),
> > +    DEFINE_PROP_UINT32("intrs", XHCITiState, intrs, 8),
> > +    DEFINE_PROP_UINT32("slots", XHCITiState, slots, XHCI_MAXSLOTS),
> > +    DEFINE_PROP_END_OF_LIST(),

Also remove this "DEFINE_PROP_END_OF_LIST()" line.

> > +};