Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1306518 > unrolled thread

Re: [PATCHv7 0/6] Driver for new "VMD" device

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCHv7 0/6] Driver for new "VMD" device Bjorn Helgaas <helgaas@kernel.org> - 2016-01-11 18:10 +0100
    Re: [PATCHv7 0/6] Driver for new "VMD" device Keith Busch <keith.busch@intel.com> - 2016-01-11 21:50 +0100
      Re: [PATCHv7 0/6] Driver for new "VMD" device Bjorn Helgaas <helgaas@kernel.org> - 2016-01-11 22:20 +0100

#1306518 — Re: [PATCHv7 0/6] Driver for new "VMD" device

Hi Keith,

On Mon, Dec 21, 2015 at 10:44:54AM -0700, Keith Busch wrote:
> Now on version 7, mostly cosmetic.
> 
> v6->v7:
> 
>  Removed PCI resource window patch rejecting child bus that doesn't fit in
>  parent's window. There are additional caveats the maintainer brought up
>  that need more consideration. Since we are not dependent on that patch,
>  it is removed from the series.
> 
>  Dropped the pciutils patch since it is an external projet not part
>  of this series anyway. Will send tooling updates to the appropriate
>  mailing list once this settles.
> 
>  Modified commit log messages to align with maintainer's requested format.
> 
>  Using u32's for domains instead of ints.
> 
>  Fixed code comment on posted/non-posted clarification. 
> 
>  Added a dmesg info message to show which domain a VMD end point bound to.
> 
>  Fixed the vmd domain's root bus parent device so power management has
>  the correct dependency tree.
> 
> Keith Busch (5):
>   PCI/MSI: Export msi functions for module use
>   x86/IRQ: Export IRQ domain function for module use
>   x86/PCI: Allow PCI domain specific dma ops
>   x86/PCI: Initial commit for new VMD device driver
>   PCI/AER: Use 32 bit int type domains
> 
> Liu Jiang (1):
>   msi: Relax msi_domain_alloc() to support parentless MSI irqdomains
> 
>  arch/x86/Kconfig                  |  13 +
>  arch/x86/include/asm/device.h     |  10 +
>  arch/x86/include/asm/hw_irq.h     |   5 +
>  arch/x86/pci/Makefile             |   2 +
>  arch/x86/pci/common.c             |  38 +++
>  arch/x86/pci/vmd.c                | 698 ++++++++++++++++++++++++++++++++++++++
>  drivers/pci/msi.c                 |   2 +
>  drivers/pci/pcie/aer/aer_inject.c |  16 +-
>  kernel/irq/irqdomain.c            |   1 +
>  kernel/irq/msi.c                  |   8 +-
>  10 files changed, 782 insertions(+), 11 deletions(-)
>  create mode 100644 arch/x86/pci/vmd.c

Checkpatch complained about a few things that should be easy to fix up:

  x86-pci-allow-pci-domain
  -:26: WARNING: struct dma_map_ops should normally be const
  x86-pci-initial-commit-for-new
  -:125: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
  -:168: WARNING: please, no space before tabs
  -:187: WARNING: please, no space before tabs
  -:212: WARNING: struct dma_map_ops should normally be const
  -:269: WARNING: line over 80 characters
  -:384: WARNING: Missing a blank line after declarations
  -:391: WARNING: Missing a blank line after declarations

This is from:

+       struct device *vdev = dev_to_vmd_dev(dev);
+       return vdev->archdata.dma_ops->alloc(vdev, size, addr, flag, attrs);

There are a lot of these, and I'm not going to be obsessive about all
these two-line functions.  But maybe a macro or helper function could
turn them all into one-line functions and side-step it, e.g.,:

  static struct dma_ops *vmd_dma_ops(struct device *dev)
  {
    return &dev_to_vmd_dev(dev)->archdata.dma_ops;
  }

  static void *vmd_alloc(...)
  {
    return vmd_dma_ops(dev)->alloc(...);
  }

  -:398: WARNING: Missing a blank line after declarations
  -:406: WARNING: Missing a blank line after declarations
  -:415: WARNING: Missing a blank line after declarations
  -:425: WARNING: Missing a blank line after declarations
  -:434: WARNING: Missing a blank line after declarations
  -:442: WARNING: Missing a blank line after declarations
  -:450: WARNING: Missing a blank line after declarations
  -:457: WARNING: Missing a blank line after declarations
  -:464: WARNING: Missing a blank line after declarations
  -:471: WARNING: Missing a blank line after declarations
  -:478: WARNING: Missing a blank line after declarations
  -:484: WARNING: Missing a blank line after declarations
  -:490: WARNING: Missing a blank line after declarations
  -:497: WARNING: Missing a blank line after declarations
  -:509: ERROR: Macros with multiple statements should be enclosed in a do - while loop
  -:509: WARNING: macros should not use a trailing semicolon
  -:510: ERROR: trailing statements should be on next line

We should definitely fix this one.

  -:514: WARNING: struct dma_map_ops should normally be const
  -:515: WARNING: struct dma_map_ops should normally be const
  -:654: WARNING: line over 80 characters
  -:656: WARNING: line over 80 characters
  -:662: WARNING: line over 80 characters
  -:664: WARNING: line over 80 characters
  -:692: WARNING: line over 80 characters

These could be easily fixed with a "struct resource *res", which would
make the code more readable as well.

  -:747: ERROR: space required before the open parenthesis '('

[toc] | [next] | [standalone]

#1306686

Hi Bjorn,

Thanks for the catches. Most were simple fixes, and I used your
cleanup suggestions for others. Will resend shortly.

On Mon, Jan 11, 2016 at 11:06:17AM -0600, Bjorn Helgaas wrote:
> Checkpatch complained about a few things that should be easy to fix up:
> 
>   x86-pci-allow-pci-domain
>   -:26: WARNING: struct dma_map_ops should normally be const
>   x86-pci-initial-commit-for-new
>   -:125: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

Question on the maintenance: does this new driver file need a maintainer,
or does it fall under arch/x86/pci/ umbrella? I'm happy to supply updates
and/or fixes once this is merged, but I just want to understand who to
direct patches/pull requests to if the need arises.

[toc] | [prev] | [next] | [standalone]

#1306709

On Mon, Jan 11, 2016 at 08:49:11PM +0000, Keith Busch wrote:
> >   x86-pci-initial-commit-for-new
> >   -:125: WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> 
> Question on the maintenance: does this new driver file need a maintainer,
> or does it fall under arch/x86/pci/ umbrella? I'm happy to supply updates
> and/or fixes once this is merged, but I just want to understand who to
> direct patches/pull requests to if the need arises.

It might be worth adding a MAINTAINERS update, similar to the
"PCI DRIVER" entries we have for host bridge drivers.  In those
cases, I take care of merging updates and sending them to Linus,
but I do ask for an ack from the listed maintainer for non-trivial
changes.

Bjorn

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web