Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Leif Liddy <leif.liddy@gmail.com>,
	linux-nvme@lists.infradead.org, Jens Axboe <axboe@fb.com>,
	Keith Busch <kbusch@kernel.org>
Subject: Re: [PATCH v1 2/2] nvme-pci: Convert to PCI_VDEVICE() macro for Apple devices
Date: Wed, 12 Feb 2020 22:34:18 +0200
Message-ID: <20200212203418.GW10400@smile.fi.intel.com> (raw)
In-Reply-To: <20200212173901.GB5708@lst.de>

On Wed, Feb 12, 2020 at 06:39:01PM +0100, Christoph Hellwig wrote:
> On Wed, Feb 12, 2020 at 12:32:19PM +0200, Andy Shevchenko wrote:
> > Like for Intel devices use PCI_VDEVICE() macro to describe
> > Apple devices.
> 
> This actually makes the code longer

I didn't get this. How? The code is for sure shorter.

> and adds the antipattern of macros
> that use string pasting on their argument,

Anti-pattern to what? Can you elaborate a bit more? Sounds like I missed some
very basic thing.

> 	and this breaking grep-ability > badly, so NAK.

This like a mantra people are telling, but looks like simple they didn't try.
What grep issue do you see in this case (I would agree if we talk about device
ID, though)?

Are you going to remove PCI_VDEVICE() completely? Are you going to be
consistent with the rest in this driver?

This patch (first part of it) is to bring some consistency to this table. I
don't see how it may thing worse than they are already.

> > Besides that it's a luck that we got devices enumerated when
> > they are listed after class matching entry. So, move them above
> > and leave class matching entry the last in the ID table.
> 
> It isn't by luck.  If the Apple devices were using the proper NVMe
> class ID we'd never hit the apple specific entries, but the actually
> use a different class code (the nvme one in big endian IIRC).  That
> being said I'm all for having the class match last.

Nobody prevents Apple to fix this and re-use old ID. So, under luck we may
consider Apple's negligence to the standards.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

  reply index

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-12 10:32 [PATCH v1 1/2] nvme-pci: Use single IRQ vector for old Apple models Andy Shevchenko
2020-02-12 10:32 ` [PATCH v1 2/2] nvme-pci: Convert to PCI_VDEVICE() macro for Apple devices Andy Shevchenko
2020-02-12 17:39   ` Christoph Hellwig
2020-02-12 20:34     ` Andy Shevchenko [this message]
2020-02-12 20:40       ` Keith Busch
2020-02-13  8:52         ` Andy Shevchenko
2020-02-13 15:51           ` Keith Busch
2020-02-19 15:06       ` Christoph Hellwig
2020-02-24  9:34         ` Andy Shevchenko
2020-02-12 17:37 ` [PATCH v1 1/2] nvme-pci: Use single IRQ vector for old Apple models Christoph Hellwig
2020-02-12 20:26   ` Andy Shevchenko
2020-02-19 15:58 ` Keith Busch

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=20200212203418.GW10400@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=axboe@fb.com \
    --cc=benh@kernel.crashing.org \
    --cc=hch@lst.de \
    --cc=kbusch@kernel.org \
    --cc=leif.liddy@gmail.com \
    --cc=linux-nvme@lists.infradead.org \
    --cc=sagi@grimberg.me \
    /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

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git