All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Sasha Levin <levinsasha928@gmail.com>
Cc: penberg@kernel.org, asias.hejun@gmail.com, gorcunov@gmail.com,
	prasadjoshi124@gmail.com, kvm@vger.kernel.org
Subject: Re: [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry
Date: Fri, 6 May 2011 21:16:03 +0200	[thread overview]
Message-ID: <20110506191603.GD7151@elte.hu> (raw)
In-Reply-To: <1304700608.10534.11.camel@lappy>


* Sasha Levin <levinsasha928@gmail.com> wrote:

> On Fri, 2011-05-06 at 13:56 +0200, Ingo Molnar wrote:
> > * Sasha Levin <levinsasha928@gmail.com> wrote:
> > 
> > > +	bdev->pci_device.irq_pin	= pin;
> > > +	bdev->pci_device.irq_line	= line;
> > 
> > One small remaining naming inconsistency caught my eyes. The generic convention 
> > should be something like:
> > 
> >  - structure names should be along the 'struct xyz_device' scheme
> > 
> >  - structure field names should be 'xyz_dev'
> > 
> >  - variable names within xyz driver's .c file should be 'xdev',
> >    but 'xyz_dev' is OK too, especially if used in some other file)
> > 
> > In that sense, the above should be:
> > 
> > 	bdev->pci_dev.irq_pin	= pin;
> > 	bdev->pci_dev.irq_line	= line;
> > 
> > This could be fixed in a followup patch - and there's more of the same 
> > inconsistency in other driver files as well.
> > 
> > If such details are sorted out early on in a project's lifetime it will be 
> > applied in a very natural way as the code grows.
> 
> The struct name there actually refers to a PCI header of a device, and
> not an actual device.
> 
> I'll rename it to pci_hdr instead of making it pci_dev, since it looks
> more confusing than desirable.

Yeah, indeed that would be good, it certainly confused me! :-)

Thanks,

	Ingo

  reply	other threads:[~2011-05-06 19:16 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-06 11:24 [PATCH 1/3] kvm tools: Introduce IRQ registry Sasha Levin
2011-05-06 11:24 ` [PATCH 2/3] kvm tools: Dynamically add devices when creating mptable Sasha Levin
2011-05-06 11:24 ` [PATCH 3/3] kvm tools: Convert virtio devices to use IRQ registry Sasha Levin
2011-05-06 11:56   ` Ingo Molnar
2011-05-06 16:50     ` Sasha Levin
2011-05-06 19:16       ` Ingo Molnar [this message]
2011-05-07  9:13 ` [PATCH 1/3] kvm tools: Introduce " Pekka Enberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110506191603.GD7151@elte.hu \
    --to=mingo@elte.hu \
    --cc=asias.hejun@gmail.com \
    --cc=gorcunov@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=levinsasha928@gmail.com \
    --cc=penberg@kernel.org \
    --cc=prasadjoshi124@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.