From: "Bjørn Mork" <bjorn@mork.no>
To: Hayes Wang <hayeswang@realtek.com>
Cc: "netdev\@vger.kernel.org" <netdev@vger.kernel.org>,
nic_swsd <nic_swsd@realtek.com>,
"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-usb\@vger.kernel.org" <linux-usb@vger.kernel.org>,
Oliver Neukum <oliver@neukum.org>
Subject: Re: [PATCH net-next 0/3] r8152: configuration setting
Date: Thu, 08 Sep 2016 09:54:51 +0200 [thread overview]
Message-ID: <87vay6hlmc.fsf@miraculix.mork.no> (raw)
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2010259B5@RTITMBSV03.realtek.com.tw> (Hayes Wang's message of "Thu, 8 Sep 2016 02:44:21 +0000")
Hayes Wang <hayeswang@realtek.com> writes:
> Bjørn Mork [mailto:bjorn@mork.no]
>> Sent: Wednesday, September 07, 2016 9:51 PM
> [...]
>> So this adds a lot of code to work around the issues you introduced by
>> unnecessarily blacklisting the CDC ECM configuration earlier, and still
>> makes the r8152 driver handle the device even in ECM mode.
>
> I suggest to use vendor mode only, but some people ask me to
> submit such patches. If these patches are rejected, I have
> enough reasons to tell them it is unacceptable rather than
> I don't do it.
>
>> Just remove the completely unnecessary blacklist, and let the cdc_ether
>> driver handle the device if the user selects the ECM configuration.
>> That't how the USB system works. There is no need for any code in r8152
>> to do that.
>
> The pure cdc_ether driver couldn't change the speed of the
> ethernet, because it doesn't know how to access the PHY of
> the device. Therefore, I add relative code in r8152 driver.
Yes, I see that. But is that strictly necessary? Couldn't you just say:
"CDC ECM is supported by cdc_ether and therefore limited to the features
implemented by cdc_ether. If you want feature X, then please use our
vendor specific mode with the r8152 driver?"
As for the dynamic switching of multi-configuration USB devices: This
is not special for your device. It is very common for e.g. LTE modems
to provide 2 or more configurations, allowing the user to select between
for example
1: ECM class + ACM class functions
2: MBIM class function
3: vendor specific functions
Each USB configuation comes with a set of descriptors identifying the
functions, and USB interface drivers attach to the functions they
support. The user can dynamically switch the device from e.g. cfg #1 to
cfg #3 by writing "3" to /sys/bus/usb/devices/<port>/bConfigurationValue
This will cause the ECM and ACM USB interfaces to disappear, and the
associated class drivers will unbind, and new vendor specific USB
interfaces appear instead, causing a matching vendor specific driver to
load and bind.
Naturally, end users will not switch configurations all the time. They
will select the configuration providing the set of functions they want.
If this is different from the default configuration selected by the
Linux USB core, then that's a simple udev rule to update the
bConfigurationValue sysfs attribute on device disceovery.
This is how the USB core works by *default*, as long as you do not
deliberately break it by blacklisting any functions or forcing a
specific configuration.
You are overdesigning to solve a non-existing problem.
Bjørn
next prev parent reply other threads:[~2016-09-08 7:55 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-07 8:12 [PATCH net-next 0/3] r8152: configuration setting Hayes Wang
2016-09-07 8:12 ` Hayes Wang
2016-09-07 8:12 ` [PATCH net-next 1/3] r8152: check hw version first Hayes Wang
2016-09-07 8:12 ` [PATCH net-next 2/3] r8152: support ECM mode Hayes Wang
2016-09-18 18:37 ` kbuild test robot
2016-09-18 18:37 ` kbuild test robot
2016-09-19 0:43 ` kbuild test robot
2016-09-07 8:12 ` [PATCH net-next 3/3] r8152: add CONFIG_RTL8152_CONFIG_VALUE Hayes Wang
2016-09-07 13:51 ` [PATCH net-next 0/3] r8152: configuration setting Bjørn Mork
2016-09-08 2:44 ` Hayes Wang
2016-09-08 7:54 ` Bjørn Mork [this message]
2016-09-08 13:02 ` Hayes Wang
2016-09-08 13:08 ` Oliver Neukum
2016-09-08 13:16 ` Bjørn Mork
2016-09-08 0:37 ` David Miller
2016-09-08 0:37 ` David Miller
2016-09-08 3:00 ` Hayes Wang
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=87vay6hlmc.fsf@miraculix.mork.no \
--to=bjorn@mork.no \
--cc=hayeswang@realtek.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=nic_swsd@realtek.com \
--cc=oliver@neukum.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 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.