From: Johan Hovold <johan@kernel.org>
To: Mychaela Falconia <mychaela.falconia@gmail.com>
Cc: Johan Hovold <johan@kernel.org>, linux-usb@vger.kernel.org
Subject: Re: [RFC] ftdi_sio driver: use altsetting or cur_altsetting?
Date: Fri, 2 Oct 2020 09:01:48 +0200 [thread overview]
Message-ID: <20201002070148.GF5141@localhost> (raw)
In-Reply-To: <CA+uuBqbCtc3EB0zPUE1WJ9s_+=Oyc5aHzYqUug7D4GpcsgoJcA@mail.gmail.com>
On Thu, Oct 01, 2020 at 11:20:08AM -0800, Mychaela Falconia wrote:
> Hello esteemed linux-usb folks,
>
> While I am trying to get a little quirk for my custom hardware device
> added to the ftdi_sio driver, I noticed an inconsistency between use
> of interface->altsetting vs. interface->cur_altsetting in the existing
> driver code. Specifically:
>
> * ftdi_determine_type() function uses this construct to get the number
> of the interface it is operating on:
>
> inter = serial->interface->altsetting->desc.bInterfaceNumber;
>
> * ftdi_set_max_packet_size() uses interface->cur_altsetting to get to
> desc.bNumEndpoints and endpoint[i].desc
>
> * The JTAG cleanup patch which Johan just applied uses
> intf->cur_altsetting->desc.bInterfaceNumber
Yeah, I noticed that too.
> So which is the right one to use, intf->altsetting or intf->cur_altsetting?
> The comments in include/linux/usb.h say that the altsetting member
> points to an array of struct usb_host_interface stored in no particular
> order, so using that pointer in ftdi_determine_type() seems wrong -
> but then I am a total novice in this area, hence I am hoping that
> someone more knowledgeable could confirm.
As long as you only access bNumInterfaces, which by definition is
identical for all altsettings, it's not wrong per se. But in general,
drivers should use cur_altsetting to not have to worry about such
subtleties and as a safe guard in case further fields are ever needed.
> The most recent version of my DUART28C quirk adding patch uses
> serial->interface->altsetting, copied from ftdi_determine_type() -
> should I change my proposed patch to use cur_altsetting instead?
Yes, please use that for new code.
> Should I also make a patch to ftdi_determine_type() changing it to use
> cur_altsetting as well?
Sure; it's not needed for correctness, but let's do it for consistency.
Johan
next prev parent reply other threads:[~2020-10-02 7:01 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-01 19:20 [RFC] ftdi_sio driver: use altsetting or cur_altsetting? Mychaela Falconia
2020-10-02 7:01 ` Johan Hovold [this message]
2020-10-02 8:37 ` Mychaela Falconia
2020-10-02 8:40 ` Johan Hovold
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=20201002070148.GF5141@localhost \
--to=johan@kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mychaela.falconia@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 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).