From: Christoph Hellwig <hch@lst.de>
To: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
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>, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v1 2/2] nvme-pci: Convert to PCI_VDEVICE() macro for Apple devices
Date: Wed, 19 Feb 2020 16:06:25 +0100 [thread overview]
Message-ID: <20200219150625.GE17748@lst.de> (raw)
In-Reply-To: <20200212203418.GW10400@smile.fi.intel.com>
On Wed, Feb 12, 2020 at 10:34:18PM +0200, Andy Shevchenko wrote:
> > This actually makes the code longer
>
> I didn't get this. How? The code is for sure shorter.
Looks at the diffstat.
>
> > 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)?
If I want to grep for PCI_VENDOR_ID_APPLE I find the ids in the old code.
I won't find them in your new code.
> Are you going to remove PCI_VDEVICE() completely? Are you going to be
> consistent with the rest in this driver?
If you care strongly about using the same macro please send a patch to
remove it in nvme. I certainly don't want the sane version switched
over to it.
> > 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.
I'm just saying the current atch is not by luck. As said in the previous
mail I'm fine with moving the class match to the end as it generally
is a good idea. I just disagree with the "works by luck" statement for
the current entries.
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2020-02-19 15:06 UTC|newest]
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
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 [this message]
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=20200219150625.GE17748@lst.de \
--to=hch@lst.de \
--cc=andriy.shevchenko@linux.intel.com \
--cc=axboe@fb.com \
--cc=benh@kernel.crashing.org \
--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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).