* Re: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver [not found] ` <SA1PR02MB860660B6F33011E5A97F7930903A9@SA1PR02MB8606.namprd02.prod.outlook.com> @ 2022-02-22 16:37 ` mka 2022-02-23 7:40 ` 回复: " Tao Wang (Consultant) (QUIC) 0 siblings, 1 reply; 10+ messages in thread From: mka @ 2022-02-22 16:37 UTC (permalink / raw) To: Tao Wang (Consultant) (QUIC) Cc: balbi, devicetree, dianders, frowand.list, gregkh, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd, Linyu Yuan (QUIC) On Mon, Feb 21, 2022 at 06:20:00AM +0000, Tao Wang (Consultant) (QUIC) wrote: > Hi, > > > > Regarding on board hub driver, > > [1]https://lore.kernel.org/linux-usb/20220119124327.v20.3.I7c9a1f1d6ced > 41dd8310e8a03da666a32364e790@changeid/#R > > > I have one comment below, > > > +static const struct usb_device_id onboard_hub_id_table[] = { > > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 > */ > > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 > */ > > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2 > */ > > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 > */ > > + {} > > +}; > > +MODULE_DEVICE_TABLE(usb, onboard_hub_id_table); > > > Can we support read VID/PID from device tree which provide platfrom > device info? As far as I understand the kernel exclusively uses the VID/PID reported by the USB device, the compatible string in the device tree is purely informational (though this driver uses it for the platform device). ^ permalink raw reply [flat|nested] 10+ messages in thread
* 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-02-22 16:37 ` 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver mka @ 2022-02-23 7:40 ` Tao Wang (Consultant) (QUIC) 2022-02-23 7:56 ` gregkh 0 siblings, 1 reply; 10+ messages in thread From: Tao Wang (Consultant) (QUIC) @ 2022-02-23 7:40 UTC (permalink / raw) To: mka, Tao Wang (Consultant) (QUIC) Cc: balbi, devicetree, dianders, frowand.list, gregkh, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd, Linyu Yuan (QUIC) Ok, thanks your reply. Here is my question, we must modify the driver "onboard_usb_hub.c" if we want to use it. But it's hard to complete because it's an opensource code. My suggestion is can we use a common compatible string for onboard_hub_driver which is a platform_driver, and read compatible string from device tree for onboard_hub_usbdev_driver which is a usb_device_driver. If so, we only need to modify our device tree if we want to use the driver. Best regards, Wangtao 13709202879 -----邮件原件----- 发件人: mka@chromium.org <mka@chromium.org> 发送时间: 2022年2月23日 0:37 收件人: Tao Wang (Consultant) (QUIC) <quic_wat@quicinc.com> 抄送: balbi@kernel.org; devicetree@vger.kernel.org; dianders@chromium.org; frowand.list@gmail.com; gregkh@linuxfoundation.org; hadess@hadess.net; krzk@kernel.org; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; mathias.nyman@intel.com; michal.simek@xilinx.com; peter.chen@kernel.org; ravisadineni@chromium.org; robh+dt@kernel.org; rogerq@kernel.org; stern@rowland.harvard.edu; swboyd@chromium.org; Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> 主题: Re: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver On Mon, Feb 21, 2022 at 06:20:00AM +0000, Tao Wang (Consultant) (QUIC) wrote: > Hi, > > > > Regarding on board hub driver, > > [1]https://lore.kernel.org/linux-usb/20220119124327.v20.3.I7c9a1f1d6ced > 41dd8310e8a03da666a32364e790@changeid/#R > > > I have one comment below, > > > +static const struct usb_device_id onboard_hub_id_table[] = { > > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 > */ > > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 > */ > > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2 > */ > > + { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 > */ > > + {} > > +}; > > +MODULE_DEVICE_TABLE(usb, onboard_hub_id_table); > > > Can we support read VID/PID from device tree which provide platfrom > device info? As far as I understand the kernel exclusively uses the VID/PID reported by the USB device, the compatible string in the device tree is purely informational (though this driver uses it for the platform device). ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-02-23 7:40 ` 回复: " Tao Wang (Consultant) (QUIC) @ 2022-02-23 7:56 ` gregkh 2022-02-28 3:08 ` Linyu Yuan (QUIC) 0 siblings, 1 reply; 10+ messages in thread From: gregkh @ 2022-02-23 7:56 UTC (permalink / raw) To: Tao Wang (Consultant) (QUIC) Cc: mka, balbi, devicetree, dianders, frowand.list, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd, Linyu Yuan (QUIC) On Wed, Feb 23, 2022 at 07:40:31AM +0000, Tao Wang (Consultant) (QUIC) wrote: > Ok, thanks your reply. > > Here is my question, we must modify the driver "onboard_usb_hub.c" if we want to use it. But it's hard to complete because it's an opensource code. I do not understand. We do not deal with code that is not in the kernel source tree, as we have no idea what is out there. Please just submit your changes to be merged into the tree and all will be fine. thanks, greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-02-23 7:56 ` gregkh @ 2022-02-28 3:08 ` Linyu Yuan (QUIC) 2022-02-28 18:28 ` mka 0 siblings, 1 reply; 10+ messages in thread From: Linyu Yuan (QUIC) @ 2022-02-28 3:08 UTC (permalink / raw) To: gregkh, Tao Wang (Consultant) (QUIC) Cc: mka, balbi, devicetree, dianders, frowand.list, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd, Linyu Yuan (QUIC) > From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org> > Sent: Wednesday, February 23, 2022 3:56 PM > To: Tao Wang (Consultant) (QUIC) <quic_wat@quicinc.com> > Cc: mka@chromium.org; balbi@kernel.org; devicetree@vger.kernel.org; > dianders@chromium.org; frowand.list@gmail.com; hadess@hadess.net; > krzk@kernel.org; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > mathias.nyman@intel.com; michal.simek@xilinx.com; > peter.chen@kernel.org; ravisadineni@chromium.org; robh+dt@kernel.org; > rogerq@kernel.org; stern@rowland.harvard.edu; swboyd@chromium.org; > Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add > onboard_usb_hub driver > > On Wed, Feb 23, 2022 at 07:40:31AM +0000, Tao Wang (Consultant) (QUIC) > wrote: > > Ok, thanks your reply. > > > > Here is my question, we must modify the driver "onboard_usb_hub.c" if > we want to use it. But it's hard to complete because it's an opensource code. > > I do not understand. We do not deal with code that is not in the kernel > source tree, as we have no idea what is out there. Please just submit > your changes to be merged into the tree and all will be fine. Hi Greg and mka, Let's make it clear that we are talking about once this driver is approved into usb tree, If we use different USB HUB which have VID/PID not defined in this driver, We need to update this driver. But if we defined VID/PID in device tree(for a specific board, manufacture should know VID/PID from HUB it used), dynamic parsed by the driver, then we don't need to change this driver (increase VID/PID table). > > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-02-28 3:08 ` Linyu Yuan (QUIC) @ 2022-02-28 18:28 ` mka 2022-03-01 2:30 ` Linyu Yuan (QUIC) 0 siblings, 1 reply; 10+ messages in thread From: mka @ 2022-02-28 18:28 UTC (permalink / raw) To: Linyu Yuan (QUIC) Cc: gregkh, Tao Wang (Consultant) (QUIC), balbi, devicetree, dianders, frowand.list, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd On Mon, Feb 28, 2022 at 03:08:34AM +0000, Linyu Yuan (QUIC) wrote: > > From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org> > > Sent: Wednesday, February 23, 2022 3:56 PM > > To: Tao Wang (Consultant) (QUIC) <quic_wat@quicinc.com> > > Cc: mka@chromium.org; balbi@kernel.org; devicetree@vger.kernel.org; > > dianders@chromium.org; frowand.list@gmail.com; hadess@hadess.net; > > krzk@kernel.org; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > > mathias.nyman@intel.com; michal.simek@xilinx.com; > > peter.chen@kernel.org; ravisadineni@chromium.org; robh+dt@kernel.org; > > rogerq@kernel.org; stern@rowland.harvard.edu; swboyd@chromium.org; > > Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add > > onboard_usb_hub driver > > > > On Wed, Feb 23, 2022 at 07:40:31AM +0000, Tao Wang (Consultant) (QUIC) > > wrote: > > > Ok, thanks your reply. > > > > > > Here is my question, we must modify the driver "onboard_usb_hub.c" if > > we want to use it. But it's hard to complete because it's an opensource code. > > > > I do not understand. We do not deal with code that is not in the kernel > > source tree, as we have no idea what is out there. Please just submit > > your changes to be merged into the tree and all will be fine. > > Hi Greg and mka, > > Let's make it clear that we are talking about once this driver is approved into usb tree, > If we use different USB HUB which have VID/PID not defined in this driver, > We need to update this driver. > > But if we defined VID/PID in device tree(for a specific board, manufacture should know VID/PID from HUB it used), > dynamic parsed by the driver, then we don't need to change this driver (increase VID/PID table). As per my earlier reply, the kernel/USB core uses the VID:PID reported by the USB device, the compatible string in the device tree is purely informational. That's not something that could be changed by this driver. And even if the VID:PID from the device tree was used: how is the kernel supposed to know that the onboard_hub driver should be probed for a given VID:PID from the device tree, without listing the VID:PID (or compatible string) in the driver (which is what you seem to seek to avoid)? ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-02-28 18:28 ` mka @ 2022-03-01 2:30 ` Linyu Yuan (QUIC) 2022-03-01 16:33 ` mka 0 siblings, 1 reply; 10+ messages in thread From: Linyu Yuan (QUIC) @ 2022-03-01 2:30 UTC (permalink / raw) To: mka, Linyu Yuan (QUIC) Cc: gregkh, Tao Wang (Consultant) (QUIC), balbi, devicetree, dianders, frowand.list, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd > From: mka@chromium.org <mka@chromium.org> > Sent: Tuesday, March 1, 2022 2:29 AM > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > Cc: gregkh@linuxfoundation.org; Tao Wang (Consultant) (QUIC) > <quic_wat@quicinc.com>; balbi@kernel.org; devicetree@vger.kernel.org; > dianders@chromium.org; frowand.list@gmail.com; hadess@hadess.net; > krzk@kernel.org; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > mathias.nyman@intel.com; michal.simek@xilinx.com; > peter.chen@kernel.org; ravisadineni@chromium.org; robh+dt@kernel.org; > rogerq@kernel.org; stern@rowland.harvard.edu; swboyd@chromium.org > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add > onboard_usb_hub driver > > > > > Hi Greg and mka, > > > > Let's make it clear that we are talking about once this driver is approved > into usb tree, > > If we use different USB HUB which have VID/PID not defined in this driver, > > We need to update this driver. > > > > But if we defined VID/PID in device tree(for a specific board, manufacture > should know VID/PID from HUB it used), > > dynamic parsed by the driver, then we don't need to change this driver > (increase VID/PID table). > > As per my earlier reply, the kernel/USB core uses the VID:PID reported > by the USB device, the compatible string in the device tree is purely > informational. That's not something that could be changed by this > driver. I can't fully understand this comment, could you please share step if we want to add a new HUB support, what should we do ? nothing ? If do nothing, can we remove id_table from onboard_hub_usbdev_driver ? > > And even if the VID:PID from the device tree was used: how is the > kernel supposed to know that the onboard_hub driver should be > probed for a given VID:PID from the device tree, without listing > the VID:PID (or compatible string) in the driver (which is what > you seem to seek to avoid)? In my opinion, if it need update VID/PID table in this driver to support a new HUB, we can parse VID/PID from device tree and create dynamic VID/PID entry to id_table of onboard_hub_usbdev_driver. Hope you can understand what I said. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-03-01 2:30 ` Linyu Yuan (QUIC) @ 2022-03-01 16:33 ` mka 2022-03-02 5:14 ` Linyu Yuan (QUIC) 0 siblings, 1 reply; 10+ messages in thread From: mka @ 2022-03-01 16:33 UTC (permalink / raw) To: Linyu Yuan (QUIC) Cc: gregkh, Tao Wang (Consultant) (QUIC), balbi, devicetree, dianders, frowand.list, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd On Tue, Mar 01, 2022 at 02:30:00AM +0000, Linyu Yuan (QUIC) wrote: > > From: mka@chromium.org <mka@chromium.org> > > Sent: Tuesday, March 1, 2022 2:29 AM > > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > > Cc: gregkh@linuxfoundation.org; Tao Wang (Consultant) (QUIC) > > <quic_wat@quicinc.com>; balbi@kernel.org; devicetree@vger.kernel.org; > > dianders@chromium.org; frowand.list@gmail.com; hadess@hadess.net; > > krzk@kernel.org; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > > mathias.nyman@intel.com; michal.simek@xilinx.com; > > peter.chen@kernel.org; ravisadineni@chromium.org; robh+dt@kernel.org; > > rogerq@kernel.org; stern@rowland.harvard.edu; swboyd@chromium.org > > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add > > onboard_usb_hub driver > > > > > > > > Hi Greg and mka, > > > > > > Let's make it clear that we are talking about once this driver is approved > > into usb tree, > > > If we use different USB HUB which have VID/PID not defined in this driver, > > > We need to update this driver. > > > > > > But if we defined VID/PID in device tree(for a specific board, manufacture > > should know VID/PID from HUB it used), > > > dynamic parsed by the driver, then we don't need to change this driver > > (increase VID/PID table). > > > > As per my earlier reply, the kernel/USB core uses the VID:PID reported > > by the USB device, the compatible string in the device tree is purely > > informational. That's not something that could be changed by this > > driver. > I can't fully understand this comment, could you please share step if we want to add a new HUB support, what should we do ? nothing ? Add the VID:PID and compatible strings to onboard_usb_hub.c, analogous to those for the RTS5411 and RTS5414. More work will be needed if the hub needs a special power up or power down sequence (multiple regulators, GPIOs, ...) > If do nothing, can we remove id_table from onboard_hub_usbdev_driver ? > > > > And even if the VID:PID from the device tree was used: how is the > > kernel supposed to know that the onboard_hub driver should be > > probed for a given VID:PID from the device tree, without listing > > the VID:PID (or compatible string) in the driver (which is what > > you seem to seek to avoid)? > In my opinion, if it need update VID/PID table in this driver to support a new HUB, > we can parse VID/PID from device tree and create dynamic VID/PID entry to id_table of onboard_hub_usbdev_driver. > > Hope you can understand what I said. Not really. I doubt that what you are suggesting would work. The easiest thing to convince people would probably be to send a patch (based on this one) with a working implementation of your idea. ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-03-01 16:33 ` mka @ 2022-03-02 5:14 ` Linyu Yuan (QUIC) 2022-03-04 16:47 ` mka 0 siblings, 1 reply; 10+ messages in thread From: Linyu Yuan (QUIC) @ 2022-03-02 5:14 UTC (permalink / raw) To: mka, Linyu Yuan (QUIC) Cc: gregkh, Tao Wang (Consultant) (QUIC), balbi, devicetree, dianders, frowand.list, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd > From: mka@chromium.org <mka@chromium.org> > Sent: Wednesday, March 2, 2022 12:33 AM > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > Cc: gregkh@linuxfoundation.org; Tao Wang (Consultant) (QUIC) > <quic_wat@quicinc.com>; balbi@kernel.org; devicetree@vger.kernel.org; > dianders@chromium.org; frowand.list@gmail.com; hadess@hadess.net; > krzk@kernel.org; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > mathias.nyman@intel.com; michal.simek@xilinx.com; > peter.chen@kernel.org; ravisadineni@chromium.org; robh+dt@kernel.org; > rogerq@kernel.org; stern@rowland.harvard.edu; swboyd@chromium.org > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add > onboard_usb_hub driver > > > In my opinion, if it need update VID/PID table in this driver to support a > new HUB, > > we can parse VID/PID from device tree and create dynamic VID/PID entry > to id_table of onboard_hub_usbdev_driver. > > > > Hope you can understand what I said. > > Not really. > > I doubt that what you are suggesting would work. The easiest thing > to convince people would probably be to send a patch (based on this > one) with a working implementation of your idea. I show my idea, but not test, diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c index e280409..1811317 100644 --- a/drivers/usb/misc/onboard_usb_hub.c +++ b/drivers/usb/misc/onboard_usb_hub.c @@ -10,6 +10,7 @@ #include <linux/init.h> #include <linux/kernel.h> #include <linux/list.h> +#include <linux/slab.h> #include <linux/module.h> #include <linux/mutex.h> #include <linux/of.h> @@ -173,6 +174,58 @@ static void onboard_hub_remove_usbdev(struct onboard_hub *hub, const struct usb_ mutex_unlock(&hub->lock); } +#define MAX_HUB_NUM 4 +#define MAX_TABLE_SIZE (MAX_HUB_NUM * 2) +static struct usb_device_id onboard_hub_id_table[MAX_TABLE_SIZE + 1]; +MODULE_DEVICE_TABLE(usb, onboard_hub_id_table); + +static void onboard_hub_add_idtable(__u16 vid, __u16 pid) +{ + int i; + + for (i = 0; i < MAX_TABLE_SIZE; i++) { + if (!onboard_hub_id_table[i].idVendor) + break; + + if (onboard_hub_id_table[i].idVendor == vid && + onboard_hub_id_table[i].idProduct == pid) + return; + } + if (i == MAX_TABLE_SIZE) + return; + + onboard_hub_id_table[i].idVendor = vid; + onboard_hub_id_table[i].idProduct = pid; + onboard_hub_id_table[i].match_flags = USB_DEVICE_ID_MATCH_DEVICE; +} + +static int onboard_hub_parse_idtable(struct device_node *np) +{ + int size = of_property_count_elems_of_size(np, "vidpid", sizeof(int)); + int ret, i; + u16 *ids; + + if (!size || size % 2) + return -EINVAL; + + ids = kzalloc(sizeof(u16) * size, GFP_KERNEL); + if (!ids) + return -ENOMEM; + + ret = of_property_read_u16_array(np, "vidpid", ids, size); + if (ret) { + kfree(ids); + return -EINVAL; + } + + for (i = 0; i < size; i+=2) + onboard_hub_add_idtable(ids[i], ids[i+1]); + + kfree(ids); + + return 0; +} + static ssize_t always_powered_in_suspend_show(struct device *dev, struct device_attribute *attr, char *buf) { @@ -210,6 +263,10 @@ static int onboard_hub_probe(struct platform_device *pdev) struct onboard_hub *hub; int err; + err = onboard_hub_parse_idtable(dev->of_node); + if (err) + return err; + hub = devm_kzalloc(dev, sizeof(*hub), GFP_KERNEL); if (!hub) return -ENOMEM; @@ -378,15 +435,6 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev) onboard_hub_remove_usbdev(hub, udev); } -static const struct usb_device_id onboard_hub_id_table[] = { - { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */ - { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */ - { USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2 */ - { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 */ - {} -}; -MODULE_DEVICE_TABLE(usb, onboard_hub_id_table); - static struct usb_device_driver onboard_hub_usbdev_driver = { .name = "onboard-usb-hub", .probe = onboard_hub_usbdev_probe, ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-03-02 5:14 ` Linyu Yuan (QUIC) @ 2022-03-04 16:47 ` mka 2022-03-07 1:24 ` Linyu Yuan (QUIC) 0 siblings, 1 reply; 10+ messages in thread From: mka @ 2022-03-04 16:47 UTC (permalink / raw) To: Linyu Yuan (QUIC) Cc: gregkh, Tao Wang (Consultant) (QUIC), balbi, devicetree, dianders, frowand.list, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd On Wed, Mar 02, 2022 at 05:14:30AM +0000, Linyu Yuan (QUIC) wrote: > > From: mka@chromium.org <mka@chromium.org> > > Sent: Wednesday, March 2, 2022 12:33 AM > > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > > Cc: gregkh@linuxfoundation.org; Tao Wang (Consultant) (QUIC) > > <quic_wat@quicinc.com>; balbi@kernel.org; devicetree@vger.kernel.org; > > dianders@chromium.org; frowand.list@gmail.com; hadess@hadess.net; > > krzk@kernel.org; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > > mathias.nyman@intel.com; michal.simek@xilinx.com; > > peter.chen@kernel.org; ravisadineni@chromium.org; robh+dt@kernel.org; > > rogerq@kernel.org; stern@rowland.harvard.edu; swboyd@chromium.org > > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add > > onboard_usb_hub driver > > > > > In my opinion, if it need update VID/PID table in this driver to support a > > new HUB, > > > we can parse VID/PID from device tree and create dynamic VID/PID entry > > to id_table of onboard_hub_usbdev_driver. > > > > > > Hope you can understand what I said. > > > > Not really. > > > > I doubt that what you are suggesting would work. The easiest thing > > to convince people would probably be to send a patch (based on this > > one) with a working implementation of your idea. > > I show my idea, but not test, > > diff --git a/drivers/usb/misc/onboard_usb_hub.c b/drivers/usb/misc/onboard_usb_hub.c > index e280409..1811317 100644 > --- a/drivers/usb/misc/onboard_usb_hub.c > +++ b/drivers/usb/misc/onboard_usb_hub.c > @@ -10,6 +10,7 @@ > #include <linux/init.h> > #include <linux/kernel.h> > #include <linux/list.h> > +#include <linux/slab.h> > #include <linux/module.h> > #include <linux/mutex.h> > #include <linux/of.h> > @@ -173,6 +174,58 @@ static void onboard_hub_remove_usbdev(struct onboard_hub *hub, const struct usb_ > mutex_unlock(&hub->lock); > } > > +#define MAX_HUB_NUM 4 > +#define MAX_TABLE_SIZE (MAX_HUB_NUM * 2) > +static struct usb_device_id onboard_hub_id_table[MAX_TABLE_SIZE + 1]; > +MODULE_DEVICE_TABLE(usb, onboard_hub_id_table); > + > +static void onboard_hub_add_idtable(__u16 vid, __u16 pid) > +{ > + int i; > + > + for (i = 0; i < MAX_TABLE_SIZE; i++) { > + if (!onboard_hub_id_table[i].idVendor) > + break; > + > + if (onboard_hub_id_table[i].idVendor == vid && > + onboard_hub_id_table[i].idProduct == pid) > + return; > + } > + if (i == MAX_TABLE_SIZE) > + return; > + > + onboard_hub_id_table[i].idVendor = vid; > + onboard_hub_id_table[i].idProduct = pid; > + onboard_hub_id_table[i].match_flags = USB_DEVICE_ID_MATCH_DEVICE; > +} > + > +static int onboard_hub_parse_idtable(struct device_node *np) > +{ > + int size = of_property_count_elems_of_size(np, "vidpid", sizeof(int)); > + int ret, i; > + u16 *ids; > + > + if (!size || size % 2) > + return -EINVAL; > + > + ids = kzalloc(sizeof(u16) * size, GFP_KERNEL); > + if (!ids) > + return -ENOMEM; > + > + ret = of_property_read_u16_array(np, "vidpid", ids, size); > + if (ret) { > + kfree(ids); > + return -EINVAL; > + } > + > + for (i = 0; i < size; i+=2) > + onboard_hub_add_idtable(ids[i], ids[i+1]); > + > + kfree(ids); > + > + return 0; > +} > + > static ssize_t always_powered_in_suspend_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > @@ -210,6 +263,10 @@ static int onboard_hub_probe(struct platform_device *pdev) > struct onboard_hub *hub; > int err; > > + err = onboard_hub_parse_idtable(dev->of_node); > + if (err) > + return err; > + > hub = devm_kzalloc(dev, sizeof(*hub), GFP_KERNEL); > if (!hub) > return -ENOMEM; > @@ -378,15 +435,6 @@ static void onboard_hub_usbdev_disconnect(struct usb_device *udev) > onboard_hub_remove_usbdev(hub, udev); > } > > -static const struct usb_device_id onboard_hub_id_table[] = { > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 */ > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 */ > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2 */ > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 */ > - {} > -}; > -MODULE_DEVICE_TABLE(usb, onboard_hub_id_table); > - > static struct usb_device_driver onboard_hub_usbdev_driver = { > .name = "onboard-usb-hub", > .probe = onboard_hub_usbdev_probe, I see multiple issues with this approach: 1. The new device tree property 'vidpid'. It is (or should be) redundant with the compatible string, I very much doubt you could convince DT maintainers to add it. 2. You don't want to modify the driver to enabled support for new USB hubs. That means you would have to use a compatible string that is already in the driver, but which doesn't match the VID:PID of the hub. While this might work it's a hack. 3. If the USB hub is probed before the platform device it won't use this driver because the VID:PID isn't in the device id table. 4. Possible race conditions when changing the device id table on the fly ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver 2022-03-04 16:47 ` mka @ 2022-03-07 1:24 ` Linyu Yuan (QUIC) 0 siblings, 0 replies; 10+ messages in thread From: Linyu Yuan (QUIC) @ 2022-03-07 1:24 UTC (permalink / raw) To: mka, Linyu Yuan (QUIC) Cc: gregkh, Tao Wang (Consultant) (QUIC), balbi, devicetree, dianders, frowand.list, hadess, krzk, linux-kernel, linux-usb, mathias.nyman, michal.simek, peter.chen, ravisadineni, robh+dt, rogerq, stern, swboyd > From: mka@chromium.org <mka@chromium.org> > Sent: Saturday, March 5, 2022 12:48 AM > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > Cc: gregkh@linuxfoundation.org; Tao Wang (Consultant) (QUIC) > <quic_wat@quicinc.com>; balbi@kernel.org; devicetree@vger.kernel.org; > dianders@chromium.org; frowand.list@gmail.com; hadess@hadess.net; > krzk@kernel.org; linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; > mathias.nyman@intel.com; michal.simek@xilinx.com; > peter.chen@kernel.org; ravisadineni@chromium.org; robh+dt@kernel.org; > rogerq@kernel.org; stern@rowland.harvard.edu; swboyd@chromium.org > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add > onboard_usb_hub driver > > On Wed, Mar 02, 2022 at 05:14:30AM +0000, Linyu Yuan (QUIC) wrote: > > > From: mka@chromium.org <mka@chromium.org> > > > Sent: Wednesday, March 2, 2022 12:33 AM > > > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > > > Cc: gregkh@linuxfoundation.org; Tao Wang (Consultant) (QUIC) > > > <quic_wat@quicinc.com>; balbi@kernel.org; > devicetree@vger.kernel.org; > > > dianders@chromium.org; frowand.list@gmail.com; hadess@hadess.net; > > > krzk@kernel.org; linux-kernel@vger.kernel.org; linux- > usb@vger.kernel.org; > > > mathias.nyman@intel.com; michal.simek@xilinx.com; > > > peter.chen@kernel.org; ravisadineni@chromium.org; > robh+dt@kernel.org; > > > rogerq@kernel.org; stern@rowland.harvard.edu; > swboyd@chromium.org > > > Subject: Re: 回复: 回复: Re: [PATCH v20 3/5] usb: misc: Add > > > onboard_usb_hub driver > > > > > > > In my opinion, if it need update VID/PID table in this driver to support a > > > new HUB, > > > > we can parse VID/PID from device tree and create dynamic VID/PID > entry > > > to id_table of onboard_hub_usbdev_driver. > > > > > > > > Hope you can understand what I said. > > > > > > Not really. > > > > > > I doubt that what you are suggesting would work. The easiest thing > > > to convince people would probably be to send a patch (based on this > > > one) with a working implementation of your idea. > > > > I show my idea, but not test, > > > > diff --git a/drivers/usb/misc/onboard_usb_hub.c > b/drivers/usb/misc/onboard_usb_hub.c > > index e280409..1811317 100644 > > --- a/drivers/usb/misc/onboard_usb_hub.c > > +++ b/drivers/usb/misc/onboard_usb_hub.c > > @@ -10,6 +10,7 @@ > > #include <linux/init.h> > > #include <linux/kernel.h> > > #include <linux/list.h> > > +#include <linux/slab.h> > > #include <linux/module.h> > > #include <linux/mutex.h> > > #include <linux/of.h> > > @@ -173,6 +174,58 @@ static void onboard_hub_remove_usbdev(struct > onboard_hub *hub, const struct usb_ > > mutex_unlock(&hub->lock); > > } > > > > +#define MAX_HUB_NUM 4 > > +#define MAX_TABLE_SIZE (MAX_HUB_NUM * 2) > > +static struct usb_device_id onboard_hub_id_table[MAX_TABLE_SIZE + 1]; > > +MODULE_DEVICE_TABLE(usb, onboard_hub_id_table); > > + > > +static void onboard_hub_add_idtable(__u16 vid, __u16 pid) > > +{ > > + int i; > > + > > + for (i = 0; i < MAX_TABLE_SIZE; i++) { > > + if (!onboard_hub_id_table[i].idVendor) > > + break; > > + > > + if (onboard_hub_id_table[i].idVendor == vid && > > + onboard_hub_id_table[i].idProduct == pid) > > + return; > > + } > > + if (i == MAX_TABLE_SIZE) > > + return; > > + > > + onboard_hub_id_table[i].idVendor = vid; > > + onboard_hub_id_table[i].idProduct = pid; > > + onboard_hub_id_table[i].match_flags = > USB_DEVICE_ID_MATCH_DEVICE; > > +} > > + > > +static int onboard_hub_parse_idtable(struct device_node *np) > > +{ > > + int size = of_property_count_elems_of_size(np, "vidpid", sizeof(int)); > > + int ret, i; > > + u16 *ids; > > + > > + if (!size || size % 2) > > + return -EINVAL; > > + > > + ids = kzalloc(sizeof(u16) * size, GFP_KERNEL); > > + if (!ids) > > + return -ENOMEM; > > + > > + ret = of_property_read_u16_array(np, "vidpid", ids, size); > > + if (ret) { > > + kfree(ids); > > + return -EINVAL; > > + } > > + > > + for (i = 0; i < size; i+=2) > > + onboard_hub_add_idtable(ids[i], ids[i+1]); > > + > > + kfree(ids); > > + > > + return 0; > > +} > > + > > static ssize_t always_powered_in_suspend_show(struct device *dev, > struct device_attribute *attr, > > char *buf) > > { > > @@ -210,6 +263,10 @@ static int onboard_hub_probe(struct > platform_device *pdev) > > struct onboard_hub *hub; > > int err; > > > > + err = onboard_hub_parse_idtable(dev->of_node); > > + if (err) > > + return err; > > + > > hub = devm_kzalloc(dev, sizeof(*hub), GFP_KERNEL); > > if (!hub) > > return -ENOMEM; > > @@ -378,15 +435,6 @@ static void > onboard_hub_usbdev_disconnect(struct usb_device *udev) > > onboard_hub_remove_usbdev(hub, udev); > > } > > > > -static const struct usb_device_id onboard_hub_id_table[] = { > > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x0411) }, /* RTS5411 USB 3.1 > */ > > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x5411) }, /* RTS5411 USB 2.1 > */ > > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x0414) }, /* RTS5414 USB 3.2 > */ > > - { USB_DEVICE(VENDOR_ID_REALTEK, 0x5414) }, /* RTS5414 USB 2.1 > */ > > - {} > > -}; > > -MODULE_DEVICE_TABLE(usb, onboard_hub_id_table); > > - > > static struct usb_device_driver onboard_hub_usbdev_driver = { > > .name = "onboard-usb-hub", > > .probe = onboard_hub_usbdev_probe, > > I see multiple issues with this approach: > > 1. The new device tree property 'vidpid'. It is (or should be) redundant > with the compatible string, I very much doubt you could convince DT > maintainers to add it. > 2. You don't want to modify the driver to enabled support for new USB hubs. > That means you would have to use a compatible string that is already in > the driver, but which doesn't match the VID:PID of the hub. While this > might work it's a hack. > 3. If the USB hub is probed before the platform device it won't use this > driver because the VID:PID isn't in the device id table. This is another topic which I don't know if discuss or not, What is power state requirement of hub for this driver ? Do it expect hub power off when system enter linux kernel stage ? If so, all this driver may not work as hub may enumerated before this driver load. > 4. Possible race conditions when changing the device id table on the fly If hub is default power off, there is no race. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-03-07 1:24 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <SA1PR02MB86067ACF0C96F18B7306D208903A9@SA1PR02MB8606.namprd02.prod.outlook.com>
[not found] ` <SA1PR02MB860660B6F33011E5A97F7930903A9@SA1PR02MB8606.namprd02.prod.outlook.com>
2022-02-22 16:37 ` 回复: Re: [PATCH v20 3/5] usb: misc: Add onboard_usb_hub driver mka
2022-02-23 7:40 ` 回复: " Tao Wang (Consultant) (QUIC)
2022-02-23 7:56 ` gregkh
2022-02-28 3:08 ` Linyu Yuan (QUIC)
2022-02-28 18:28 ` mka
2022-03-01 2:30 ` Linyu Yuan (QUIC)
2022-03-01 16:33 ` mka
2022-03-02 5:14 ` Linyu Yuan (QUIC)
2022-03-04 16:47 ` mka
2022-03-07 1:24 ` Linyu Yuan (QUIC)
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).