From: Brian Norris <briannorris@chromium.org>
To: Rajat Jain <rajatja@google.com>
Cc: Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Marcel Holtmann <marcel@holtmann.org>,
Gustavo Padovan <gustavo@padovan.org>,
Johan Hedberg <johan.hedberg@gmail.com>,
Amitkumar Karwar <akarwar@marvell.com>,
Wei-Ning Huang <wnhuang@chromium.org>,
Xinming Hu <huxm@marvell.com>,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
Rajat Jain <rajatxjain@gmail.com>
Subject: Re: [PATCH v2 2/3] Bluetooth: btusb: Add out-of-band wakeup support
Date: Mon, 19 Dec 2016 19:41:23 -0800 [thread overview]
Message-ID: <20161220034122.GA40060@google.com> (raw)
In-Reply-To: <CACK8Z6FteRYmrNq5Oq9LpHOUOYg49XrL0WsxD8G45pTw_cwaSg@mail.gmail.com>
On Mon, Dec 19, 2016 at 05:46:19PM -0800, Rajat Jain wrote:
> On Mon, Dec 19, 2016 at 3:10 PM, Brian Norris <briannorris@chromium.org> wrote:
> > On Fri, Dec 16, 2016 at 11:30:03AM -0800, Rajat Jain wrote:
> >> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >> index ce22cef..beca4e9 100644
> >> --- a/drivers/bluetooth/btusb.c
> >> +++ b/drivers/bluetooth/btusb.c
> >> +static const struct of_device_id btusb_match_table[] = {
> >> + { .compatible = "usb1286,204e" },
> >> + { }
> >> +};
> >> +MODULE_DEVICE_TABLE(of, btusb_match_table);
> >
> > You define a match table here, but you also define essentially same
> > table for Marvell-specific additions in patch 3. It looks like maybe
> > it's legal to have more than one OF table in a module? But it seems like
> > it would get confusing, besides being somewhat strange to maintain. It
> > might also produce duplicate 'modinfo' output.
> >
> > If you really want to independently opt into device-tree-specified
> > interrupts vs. Marvell-specific interrrupt configuration, then you
> > should probably just merge the latter into the former table, and
> > implement a Marvell/GPIO flag to stick in the .data field of this table.
>
> The data we want to stick (The pin number on the chip) is board
> specific rather than being chip specific, and hence .data may not be
> useful here.
I just meant that if you conceptually wanted Marvell devices to "opt in"
to this, then you could turn the .data field into some kind of flag or
struct for describing capabilities (e.g., MARVELL_GPIO_WAKE or similar),
effectively merging the two tables instead of having two
mostly-overlapping tables.
But you could do the same by putting such flag(s) in the
{btusb,blacklist}_table[], or add such flag(s) later whenever there's a
Marvell device that doesn't support this feature.
> > Or it might be fine to drop one or both "match" checks. Particularly for
> > the Marvell-specific stuff, it's probably fair just to check if it has
> > an ->of_node and if 'id->driver_info & BTUSB_MARVELL'. Any other Marvell
> > device-specific quirks could probably be keyed off of the
> > (weirdly-named?) blacklist_table[], which already matches PID/VID.
>
> Yup, I think I can remove the compatible string checks.
>
> I'll be sending a V3.
Great!
Brian
next prev parent reply other threads:[~2016-12-20 3:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-16 19:30 [PATCH v2 1/3] Bluetooth: btusb: Use an error label for error paths Rajat Jain
2016-12-16 19:30 ` [PATCH v2 2/3] Bluetooth: btusb: Add out-of-band wakeup support Rajat Jain
2016-12-19 23:10 ` Brian Norris
2016-12-20 1:46 ` Rajat Jain
2016-12-20 3:41 ` Brian Norris [this message]
2016-12-16 19:30 ` [PATCH v2 3/3] Bluetooth: btusb: Configure Marvell to use one of the pins for oob wakeup Rajat Jain
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=20161220034122.GA40060@google.com \
--to=briannorris@chromium.org \
--cc=akarwar@marvell.com \
--cc=devicetree@vger.kernel.org \
--cc=gustavo@padovan.org \
--cc=huxm@marvell.com \
--cc=johan.hedberg@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcel@holtmann.org \
--cc=mark.rutland@arm.com \
--cc=netdev@vger.kernel.org \
--cc=rajatja@google.com \
--cc=rajatxjain@gmail.com \
--cc=robh+dt@kernel.org \
--cc=wnhuang@chromium.org \
/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).