[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/5] fix some coding style problems
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4 1/5] fix some coding style problems |
Date: |
Fri, 08 Apr 2016 08:29:33 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Cao jin <address@hidden> writes:
> This patch comes along with patch "Add param Error ** for msi_init".
What do you want to say with this sentence? I think it could be dropped
without loss.
> Add more newlines to make the code block well separated; add more
> comments for msi_init; and fix a indentation.
>
> Signed-off-by: Cao jin <address@hidden>
> CC: Dmitry Fleytman <address@hidden>
> CC: Jason Wang <address@hidden>
> CC: Michael S. Tsirkin <address@hidden>
>
> ---
> hw/net/vmxnet3.c | 2 +-
> hw/pci-bridge/ioh3420.c | 7 ++++++-
> hw/pci-bridge/pci_bridge_dev.c | 4 ++++
> hw/pci-bridge/xio3130_downstream.c | 6 +++++-
> hw/pci-bridge/xio3130_upstream.c | 3 +++
> hw/pci/msi.c | 13 +++++++++++++
> 6 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 093a71e..7a38e47 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -348,7 +348,7 @@ typedef struct {
> /* Interrupt management */
>
> /*
> - *This function returns sign whether interrupt line is in asserted state
> + * This function returns sign whether interrupt line is in asserted state
> * This depends on the type of interrupt used. For INTX interrupt line will
> * be asserted until explicit deassertion, for MSI(X) interrupt line will
> * be deasserted automatically due to notification semantics of the MSI(X)
> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index 0937fa3..b4a7806 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -106,12 +106,14 @@ static int ioh3420_initfn(PCIDevice *d)
> if (rc < 0) {
> goto err_bridge;
> }
> +
> rc = msi_init(d, IOH_EP_MSI_OFFSET, IOH_EP_MSI_NR_VECTOR,
> IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
> IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT);
> if (rc < 0) {
> goto err_bridge;
> }
> +
> rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT,
> p->port);
> if (rc < 0) {
> goto err_msi;
> @@ -120,18 +122,21 @@ static int ioh3420_initfn(PCIDevice *d)
> pcie_cap_arifwd_init(d);
> pcie_cap_deverr_init(d);
> pcie_cap_slot_init(d, s->slot);
> + pcie_cap_root_init(d);
> +
> pcie_chassis_create(s->chassis);
> rc = pcie_chassis_add_slot(s);
> if (rc < 0) {
> goto err_pcie_cap;
> }
> - pcie_cap_root_init(d);
> +
Code motion, not covered by commit message. Should this be in a later
patch?
> rc = pcie_aer_init(d, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
> if (rc < 0) {
> goto err;
> }
> pcie_aer_root_init(d);
> ioh3420_aer_vector_update(d);
> +
> return 0;
>
> err:
> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
> index 862a236..32f4daa 100644
> --- a/hw/pci-bridge/pci_bridge_dev.c
> +++ b/hw/pci-bridge/pci_bridge_dev.c
> @@ -67,10 +67,12 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
> /* MSI is not applicable without SHPC */
> bridge_dev->flags &= ~(1 << PCI_BRIDGE_DEV_F_MSI_REQ);
> }
> +
> err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0);
> if (err) {
> goto slotid_error;
> }
> +
> if ((bridge_dev->flags & (1 << PCI_BRIDGE_DEV_F_MSI_REQ)) &&
> msi_nonbroken) {
> err = msi_init(dev, 0, 1, true, true);
> @@ -78,6 +80,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
> goto msi_error;
> }
> }
> +
> if (shpc_present(dev)) {
> /* TODO: spec recommends using 64 bit prefetcheable BAR.
> * Check whether that works well. */
> @@ -85,6 +88,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
> PCI_BASE_ADDRESS_MEM_TYPE_64, &bridge_dev->bar);
> }
> return 0;
> +
> msi_error:
> slotid_cap_cleanup(dev);
> slotid_error:
> diff --git a/hw/pci-bridge/xio3130_downstream.c
> b/hw/pci-bridge/xio3130_downstream.c
> index cf1ee63..e6d653d 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -70,11 +70,13 @@ static int xio3130_downstream_initfn(PCIDevice *d)
> if (rc < 0) {
> goto err_bridge;
> }
> +
> rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
> XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
> if (rc < 0) {
> goto err_bridge;
> }
> +
> rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_DOWNSTREAM,
> p->port);
> if (rc < 0) {
> @@ -83,12 +85,14 @@ static int xio3130_downstream_initfn(PCIDevice *d)
> pcie_cap_flr_init(d);
> pcie_cap_deverr_init(d);
> pcie_cap_slot_init(d, s->slot);
> + pcie_cap_arifwd_init(d);
> +
> pcie_chassis_create(s->chassis);
> rc = pcie_chassis_add_slot(s);
> if (rc < 0) {
> goto err_pcie_cap;
> }
> - pcie_cap_arifwd_init(d);
> +
> rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
> if (rc < 0) {
> goto err;
Likewise.
> diff --git a/hw/pci-bridge/xio3130_upstream.c
> b/hw/pci-bridge/xio3130_upstream.c
> index 164ef58..d976844 100644
> --- a/hw/pci-bridge/xio3130_upstream.c
> +++ b/hw/pci-bridge/xio3130_upstream.c
> @@ -66,11 +66,13 @@ static int xio3130_upstream_initfn(PCIDevice *d)
> if (rc < 0) {
> goto err_bridge;
> }
> +
> rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
> XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
> if (rc < 0) {
> goto err_bridge;
> }
> +
> rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_UPSTREAM,
> p->port);
> if (rc < 0) {
> @@ -78,6 +80,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
> }
> pcie_cap_flr_init(d);
> pcie_cap_deverr_init(d);
> +
> rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
> if (rc < 0) {
> goto err;
> diff --git a/hw/pci/msi.c b/hw/pci/msi.c
> index e0e64c2..e2a701b 100644
> --- a/hw/pci/msi.c
> +++ b/hw/pci/msi.c
> @@ -165,6 +165,19 @@ bool msi_enabled(const PCIDevice *dev)
> PCI_MSI_FLAGS_ENABLE);
> }
>
> +/*
> + * Make PCI device @dev MSI-capable.
> + * Non-zero @offset puts capability MSI at that offset in PCI config
> + * space.
> + * @nr_vectors is the number of MSI vectors (1, 2, 4, 8, 16 or 32).
> + * If @msi64bit, make the device capable of sending a 64-bit message
> + * address.
> + * If @msi_per_vector_mask, make the device support per-vector masking.
> + * @errp is for returning errors.
> + * Return the offset of capability MSI in config space on success,
> + * set @errp and return -errno on error.
> + * -ENOTSUP means lacking msi support for a msi-capable platform.
Missing: -ENOSPC and -EINVAL. Intentional?
> + */
> int msi_init(struct PCIDevice *dev, uint8_t offset,
> unsigned int nr_vectors, bool msi64bit, bool
> msi_per_vector_mask)
> {
- [Qemu-devel] [PATCH v4 0/5] Add param Error ** for msi_init(), Cao jin, 2016/04/05
- [Qemu-devel] [PATCH v4 2/5] change pvscsi_init_msi() type to void, Cao jin, 2016/04/05
- [Qemu-devel] [PATCH v4 4/5] mptsas: change .realize function name, Cao jin, 2016/04/05
- [Qemu-devel] [PATCH v4 1/5] fix some coding style problems, Cao jin, 2016/04/05
- Re: [Qemu-devel] [PATCH v4 1/5] fix some coding style problems,
Markus Armbruster <=
- [Qemu-devel] [PATCH v4 3/5] megasas: bugfix, Cao jin, 2016/04/05
- [Qemu-devel] [PATCH v4 5/5] Add param Error ** for msi_init(), Cao jin, 2016/04/05