Re: [PATCH v6 3/4] Bluetooth: Allow driver specific cmd timeout handling
Hi Rajat, > Add a hook to allow the BT driver to do device or command specific > handling in case of timeouts. This is to be used by Intel driver to > reset the device after certain number of timeouts. > > Signed-off-by: Rajat Jain > --- > v6: Dropped the "sent command" parameter from cmd_timeout() > v5: Drop the quirk, and rename the hook function to cmd_timeout() > v4: same as v1 > v3: same as v1 > v2: same as v1 > > include/net/bluetooth/hci_core.h | 1 + > net/bluetooth/hci_core.c | 3 +++ > 2 files changed, 4 insertions(+) patch has been applied to bluetooth-next tree. Regards Marcel
Re: [PATCH v6 2/4] usb: assign ACPI companions for embedded USB devices
Hi Rajat, > USB devices permanently connected to USB ports may be described in ACPI > tables and share ACPI devices with ports they are connected to. See [1] > for details. > > This will allow us to describe sideband resources for devices, such as, > for example, hard reset line for BT USB controllers. > > [1] > https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects#acpi-namespace-hierarchy-and-adr-for-embedded-usb-devices > > Signed-off-by: Dmitry Torokhov > Signed-off-by: Rajat Jain (changed how we get the > usb_port) > Acked-by: Greg Kroah-Hartman > Tested-by: Sukumar Ghorai > --- > v6: same as v4 > v5: same as v4 > v4: Add Acked-by and Tested-by in signatures. > v3: same as v1 > v2: same as v1 > > drivers/usb/core/usb-acpi.c | 44 + > 1 file changed, 35 insertions(+), 9 deletions(-) patch has been applied to bluetooth-next tree. Regards Marcel
Re: [PATCH v6 1/4] usb: split code locating ACPI companion into port and device
Hi Rajat, > In preparation for handling embedded USB devices let's split > usb_acpi_find_companion() into usb_acpi_find_companion_for_device() and > usb_acpi_find_companion_for_port(). > > Signed-off-by: Dmitry Torokhov > Signed-off-by: Rajat Jain > Acked-by: Greg Kroah-Hartman > Tested-by: Sukumar Ghorai > --- > v6: same as v4 > v5: same as v4 > v4: Add Acked-by and Tested-by in signatures. > v3: same as v1 > v2: same as v1 > > drivers/usb/core/usb-acpi.c | 133 +++- > 1 file changed, 72 insertions(+), 61 deletions(-) patch has been applied to bluetooth-next tree. Regards Marcel
Re: [PATCH v6 4/4] Bluetooth: btusb: Use the cmd_timeout method to reset the Intel BT chip
Hi Rajat, > If the platform provides it, use the reset gpio to reset the Intel BT > chip, as part of cmd_timeout handling. This has been found helpful on > Intel bluetooth controllers where the firmware gets stuck and the only > way out is a hard reset pin provided by the platform. > > Signed-off-by: Rajat Jain > --- > v6: Move the cmd_timeout() hook assignment with other hooks, move the >reset_gpio check in the timeout function. > v5: Rename the hook to cmd_timeout, and wait for 5 timeouts before >resetting the device. > v4: Use data->flags instead of clearing the quirk in btusb_hw_reset() > v3: Better error handling for gpiod_get_optional() > v2: Handle the EPROBE_DEFER case. > > drivers/bluetooth/btusb.c | 54 +++ > 1 file changed, 54 insertions(+) patch has been applied to bluetooth-next tree. Regards Marcel
Re: [PATCH v5 4/4] Bluetooth: btusb: Use the cmd_timeout method to reset the Intel BT chip
Hi Rajat,
> If the platform provides it, use the reset gpio to reset the Intel BT
> chip, as part of cmd_timeout handling. This has been found helpful on
> Intel bluetooth controllers where the firmware gets stuck and the only
> way out is a hard reset pin provided by the platform.
>
> Signed-off-by: Rajat Jain
> ---
> v5: Rename the hook to cmd_timeout, and wait for 5 timeouts before
>resetting the device.
> v4: Use data->flags instead of clearing the quirk in btusb_hw_reset()
> v3: Better error handling for gpiod_get_optional()
> v2: Handle the EPROBE_DEFER case.
>
> drivers/bluetooth/btusb.c | 54 +++
> 1 file changed, 54 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 4761499db9ee..8949ea30a1bc 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -439,6 +440,7 @@ static const struct dmi_system_id
> btusb_needs_reset_resume_table[] = {
> #define BTUSB_BOOTING 9
> #define BTUSB_DIAG_RUNNING10
> #define BTUSB_OOB_WAKE_ENABLED11
> +#define BTUSB_HW_RESET_ACTIVE12
>
> struct btusb_data {
> struct hci_dev *hdev;
> @@ -476,6 +478,8 @@ struct btusb_data {
> struct usb_endpoint_descriptor *diag_tx_ep;
> struct usb_endpoint_descriptor *diag_rx_ep;
>
> + struct gpio_desc *reset_gpio;
> +
> __u8 cmdreq_type;
> __u8 cmdreq;
>
> @@ -489,8 +493,39 @@ struct btusb_data {
> int (*setup_on_usb)(struct hci_dev *hdev);
>
> int oob_wake_irq; /* irq for out-of-band wake-on-bt */
> + unsigned num_cmd_timeout;
Make this cmd_timeout_count or cmd_timeout_cnt.
> };
>
> +
> +static void btusb_intel_cmd_timeout(struct hci_dev *hdev,
> + struct hci_command_hdr *cmd)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> + struct gpio_desc *reset_gpio = data->reset_gpio;
> +
> + bt_dev_err(hdev, "btusb num_cmd_timeout = %u", ++data->num_cmd_timeout);
I would prefer if you don’t do the increase in the bt_dev_err(). And you can
also scrap the “btusb “ prefix here since that is all present via bt_dev_err if
really needed at some point. Actually I question the whole bt_dev_err here in
the first place since the core will put our the error. You are just repeating
it here.
> +
> + if (data->num_cmd_timeout < 5)
> + return;
So I would propose to do just this:
if (++data->cmd_timeout_count < 5)
return;
> +
> + /*
> + * Toggle the hard reset line if the platform provides one. The reset
> + * is going to yank the device off the USB and then replug. So doing
> + * once is enough. The cleanup is handled correctly on the way out
> + * (standard USB disconnect), and the new device is detected cleanly
> + * and bound to the driver again like it should be.
> + */
> + if (test_and_set_bit(BTUSB_HW_RESET_ACTIVE, &data->flags)) {
> + bt_dev_err(hdev, "last reset failed? Not resetting again");
> + return;
> + }
> +
> + bt_dev_err(hdev, "Initiating HW reset via gpio");
> + gpiod_set_value(reset_gpio, 1);
> + mdelay(100);
> + gpiod_set_value(reset_gpio, 0);
> +}
> +
> static inline void btusb_free_frags(struct btusb_data *data)
> {
> unsigned long flags;
> @@ -2915,6 +2950,7 @@ static int btusb_probe(struct usb_interface *intf,
> const struct usb_device_id *id)
> {
> struct usb_endpoint_descriptor *ep_desc;
> + struct gpio_desc *reset_gpio;
> struct btusb_data *data;
> struct hci_dev *hdev;
> unsigned ifnum_base;
> @@ -3028,6 +3064,15 @@ static int btusb_probe(struct usb_interface *intf,
>
> SET_HCIDEV_DEV(hdev, &intf->dev);
>
> + reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
> + GPIOD_OUT_LOW);
Any reason to not use the devm_gpiod_get_optional() version here?
> + if (IS_ERR(reset_gpio)) {
> + err = PTR_ERR(reset_gpio);
> + goto out_free_dev;
> + } else if (reset_gpio) {
> + data->reset_gpio = reset_gpio;
> + }
> +
> hdev->open = btusb_open;
> hdev->close = btusb_close;
> hdev->flush = btusb_flush;
> @@ -3085,6 +3130,8 @@ static int btusb_probe(struct usb_interface *intf,
> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> + if (data->reset_gpio)
> + hdev->cmd_timeout = btusb_intel_cmd_timeout;
> }
Always assign hdev->cmd_timeout = btusb_intel_cmd_timeout and put it after
btintel_set_bdaddr and before the quirks.
Move the if (data->reset_gpio) check into btusb_intel_cmd_timeout() functio
Re: [PATCH v5 3/4] Bluetooth: Allow driver specific cmd timeout handling
Hi Rajat,
> Add a hook to allow the BT driver to do device or command specific
> handling in case of timeouts. This is to be used by Intel driver to
> reset the device after certain number of timeouts.
>
> Signed-off-by: Rajat Jain
> ---
> v5: Drop the quirk, and rename the hook function to cmd_timeout()
> v4: same as v1
> v3: same as v1
> v2: same as v1
>
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 10 --
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h
> b/include/net/bluetooth/hci_core.h
> index e5ea633ea368..624d5f2b1f36 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -437,6 +437,7 @@ struct hci_dev {
> int (*post_init)(struct hci_dev *hdev);
> int (*set_diag)(struct hci_dev *hdev, bool enable);
> int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> + void (*cmd_timeout)(struct hci_dev *hdev, struct hci_command_hdr *cmd);
> };
>
> #define HCI_PHY_HANDLE(handle)(handle & 0xff)
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 7352fe85674b..c6917f57581a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2568,16 +2568,22 @@ static void hci_cmd_timeout(struct work_struct *work)
> {
> struct hci_dev *hdev = container_of(work, struct hci_dev,
> cmd_timer.work);
> + struct hci_command_hdr *sent = NULL;
>
> if (hdev->sent_cmd) {
> - struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
> - u16 opcode = __le16_to_cpu(sent->opcode);
> + u16 opcode;
> +
> + sent = (void *) hdev->sent_cmd->data;
> + opcode = __le16_to_cpu(sent->opcode);
>
> bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode);
> } else {
> bt_dev_err(hdev, "command tx timeout");
> }
>
> + if (hdev->cmd_timeout)
> + hdev->cmd_timeout(hdev, sent);
> +
drop the sent parameter. You are not using it and if at some point in the
future you might want to use it, then we add it and fix up all users.
And frankly, I would move the hdev->cmd_timeout call into the hdev->sent_cmd
block since I do not even know if it makes sense to deal with the fallback case
where hdev->sent_cmd is not available. Unless you have that kind of error case,
but that is only possible if you have external injection of HCI commands.
Regards
Marcel
Re: [PATCH v4 5/5] Bluetooth: btusb: Use the hw_reset method to allow resetting the BT chip
Hi Rajat,
> If the platform provides it, use the reset gpio to reset the BT
> chip (requested by the HCI core if needed). This has been found helpful
> on some of Intel bluetooth controllers where the firmware gets stuck and
> the only way out is a hard reset pin provided by the platform.
>
> Signed-off-by: Rajat Jain
> ---
> v4: Use data->flags instead of clearing the quirk in btusb_hw_reset()
> v3: Better error handling for gpiod_get_optional()
> v2: Handle the EPROBE_DEFER case.
>
> drivers/bluetooth/btusb.c | 45 +++
> 1 file changed, 45 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 59869643cc29..7cf1e4f749e9 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -439,6 +440,7 @@ static const struct dmi_system_id
> btusb_needs_reset_resume_table[] = {
> #define BTUSB_BOOTING 9
> #define BTUSB_DIAG_RUNNING10
> #define BTUSB_OOB_WAKE_ENABLED11
> +#define BTUSB_HW_RESET_DONE 12
I think you mean BTUSB_HW_RESET_ACTIVE or BTUSB_HW_RESET_IN_PROGRESS.
>
> struct btusb_data {
> struct hci_dev *hdev;
> @@ -476,6 +478,8 @@ struct btusb_data {
> struct usb_endpoint_descriptor *diag_tx_ep;
> struct usb_endpoint_descriptor *diag_rx_ep;
>
> + struct gpio_desc *reset_gpio;
> +
> __u8 cmdreq_type;
> __u8 cmdreq;
>
> @@ -491,6 +495,30 @@ struct btusb_data {
> int oob_wake_irq; /* irq for out-of-band wake-on-bt */
> };
>
> +
> +static void btusb_hw_reset(struct hci_dev *hdev)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> + struct gpio_desc *reset_gpio = data->reset_gpio;
> +
> + /*
> + * Toggle the hard reset line if the platform provides one. The reset
> + * is going to yank the device off the USB and then replug. So doing
> + * once is enough. The cleanup is handled correctly on the way out
> + * (standard USB disconnect), and the new device is detected cleanly
> + * and bound to the driver again like it should be.
> + */
> + if (test_and_set_bit(BTUSB_HW_RESET_DONE, &data->flags)) {
> + bt_dev_warn(hdev, "last reset failed? Not resetting again\n");
> + return;
> + }
> +
> + bt_dev_dbg(hdev, "Initiating HW reset via gpio\n”);
The bt_dev_ functions don’t need the \n at the end.
> + gpiod_set_value(reset_gpio, 1);
> + mdelay(100);
> + gpiod_set_value(reset_gpio, 0);
> +}
> +
> static inline void btusb_free_frags(struct btusb_data *data)
> {
> unsigned long flags;
> @@ -2915,6 +2943,7 @@ static int btusb_probe(struct usb_interface *intf,
> const struct usb_device_id *id)
> {
> struct usb_endpoint_descriptor *ep_desc;
> + struct gpio_desc *reset_gpio;
> struct btusb_data *data;
> struct hci_dev *hdev;
> unsigned ifnum_base;
> @@ -3028,6 +3057,16 @@ static int btusb_probe(struct usb_interface *intf,
>
> SET_HCIDEV_DEV(hdev, &intf->dev);
>
> + reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(reset_gpio)) {
> + err = PTR_ERR(reset_gpio);
> + goto out_free_dev;
> + } else if (reset_gpio) {
> + data->reset_gpio = reset_gpio;
> + hdev->hw_reset = btusb_hw_reset;
> + }
> +
> hdev->open = btusb_open;
> hdev->close = btusb_close;
> hdev->flush = btusb_flush;
> @@ -3083,6 +3122,7 @@ static int btusb_probe(struct usb_interface *intf,
> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> + set_bit(HCI_QUIRK_HW_RESET_ON_TIMEOUT, &hdev->quirks);
>
> if (id->driver_info & BTUSB_INTEL) {
> hdev->setup = btusb_setup_intel;
> @@ -3223,6 +3263,8 @@ static int btusb_probe(struct usb_interface *intf,
> return 0;
>
> out_free_dev:
> + if (data->reset_gpio)
> + gpiod_put(data->reset_gpio);
> hci_free_dev(hdev);
> return err;
> }
> @@ -3266,6 +3308,9 @@ static void btusb_disconnect(struct usb_interface *intf)
> if (data->oob_wake_irq)
> device_init_wakeup(&data->udev->dev, false);
>
> + if (data->reset_gpio)
> + gpiod_put(data->reset_gpio);
> +
> hci_free_dev(hdev);
> }
Regards
Marcel
Re: [PATCH v4 3/5] Bluetooth: Reset Bluetooth chip after multiple command timeouts
Hi Rajat,
> Add a quirk and a hook to allow the HCI core to reset the BT chip
> if needed (after a number of timed out commands). Use that new hook to
> initiate BT chip reset if the controller fails to respond to certain
> number of commands (currently 5) including the HCI reset commands.
> This is done based on a newly introduced quirk. This is done based
> on some initial work by Intel.
>
> Signed-off-by: Rajat Jain
> ---
> v4: same as v1
> v3: same as v1
> v2: same as v1
>
> include/net/bluetooth/hci.h | 8
> include/net/bluetooth/hci_core.h | 2 ++
> net/bluetooth/hci_core.c | 15 +--
> 3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index c36dc1e20556..af02fa5ffe54 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -192,6 +192,14 @@ enum {
>*
>*/
> HCI_QUIRK_NON_PERSISTENT_SETUP,
> +
> + /* When this quirk is set, hw_reset() would be run to reset the
> + * hardware, after a certain number of commands (currently 5)
> + * time out because the device fails to respond.
> + *
> + * This quirk should be set before hci_register_dev is called.
> + */
> + HCI_QUIRK_HW_RESET_ON_TIMEOUT,
> };
>
> /* HCI device flags */
> diff --git a/include/net/bluetooth/hci_core.h
> b/include/net/bluetooth/hci_core.h
> index e5ea633ea368..b86218304b80 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -313,6 +313,7 @@ struct hci_dev {
> unsigned intacl_cnt;
> unsigned intsco_cnt;
> unsigned intle_cnt;
> + unsigned inttimeout_cnt;
>
> unsigned intacl_mtu;
> unsigned intsco_mtu;
> @@ -437,6 +438,7 @@ struct hci_dev {
> int (*post_init)(struct hci_dev *hdev);
> int (*set_diag)(struct hci_dev *hdev, bool enable);
> int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> + void (*hw_reset)(struct hci_dev *hdev);
> };
>
> #define HCI_PHY_HANDLE(handle)(handle & 0xff)
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 7352fe85674b..ab3a6a8b7ba6 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2569,13 +2569,24 @@ static void hci_cmd_timeout(struct work_struct *work)
> struct hci_dev *hdev = container_of(work, struct hci_dev,
> cmd_timer.work);
>
> + hdev->timeout_cnt++;
> if (hdev->sent_cmd) {
> struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data;
> u16 opcode = __le16_to_cpu(sent->opcode);
>
> - bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode);
> + bt_dev_err(hdev, "command 0x%4.4x tx timeout (cnt = %u)",
> +opcode, hdev->timeout_cnt);
> } else {
> - bt_dev_err(hdev, "command tx timeout");
> + bt_dev_err(hdev, "command tx timeout (cnt = %u)",
> +hdev->timeout_cnt);
> + }
> +
> + if (test_bit(HCI_QUIRK_HW_RESET_ON_TIMEOUT, &hdev->quirks) &&
> + hdev->timeout_cnt >= 5) {
> + hdev->timeout_cnt = 0;
> + if (hdev->hw_reset)
> + hdev->hw_reset(hdev);
> + return;
> }
so I really do not see the need for the quirk here. Either hdev->hw_reset is
provided, then execute it, if it is not provided then don’t. The quirk is just
duplicate information.
I also don’t like hdev->hw_reset since that implies that the only way of
handling a command timeout is a hardware reset. I prefer you call this
hdev->cmd_timeout and also scrap the timeout_cnt. Let the driver decide what
number of timeouts it wants to react on. The number 5 is just an arbitrary
number you picked based on one hardware manufacturer.
Regards
Marcel
Re: [PATCH v4 2/5] usb: assign ACPI companions for embedded USB devices
Hi Rajat, > USB devices permanently connected to USB ports may be described in ACPI > tables and share ACPI devices with ports they are connected to. See [1] > for details. > > This will allow us to describe sideband resources for devices, such as, > for example, hard reset line for BT USB controllers. > > [1] > https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects#acpi-namespace-hierarchy-and-adr-for-embedded-usb-devices > > Signed-off-by: Dmitry Torokhov > Signed-off-by: Rajat Jain (changed how we get the > usb_port) > Acked-by: Greg Kroah-Hartman > Tested-by: Sukumar Ghorai > --- > v4: Add Acked-by and Tested-by in signatures. > v3: same as v1 > v2: same as v1 > > drivers/usb/core/usb-acpi.c | 44 + > 1 file changed, 35 insertions(+), 9 deletions(-) same question here. Which tree is suppose to take this? Regards Marcel
Re: [PATCH v4 1/5] usb: split code locating ACPI companion into port and device
Hi Rajat, > In preparation for handling embedded USB devices let's split > usb_acpi_find_companion() into usb_acpi_find_companion_for_device() and > usb_acpi_find_companion_for_port(). > > Signed-off-by: Dmitry Torokhov > Signed-off-by: Rajat Jain > Acked-by: Greg Kroah-Hartman > Tested-by: Sukumar Ghorai > --- > v4: Add Acked-by and Tested-by in signatures. > v3: same as v1 > v2: same as v1 > > drivers/usb/core/usb-acpi.c | 133 +++- > 1 file changed, 72 insertions(+), 61 deletions(-) what is the plan here? I take this via bluetooth-next tree? Regards Marcel
Re: [PATCH v4 4/5] Bluetooth: btusb: Collect the common Intel assignments together
Hi Rajat,
> The BTUSB_INTEL and BTUSB_INTEL_NEW have common functions & quirks are
> assigned to hdev structure. Lets collect them together instead of
> repeating them in different code branches.
>
> Signed-off-by: Rajat Jain
> ---
> v4: same as v1
> v3: same as v1
> v2: same as v1
>
> drivers/bluetooth/btusb.c | 27 ---
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 4761499db9ee..59869643cc29 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3075,28 +3075,25 @@ static int btusb_probe(struct usb_interface *intf,
> data->diag = usb_ifnum_to_if(data->udev, ifnum_base + 2);
> }
> #endif
> + if (id->driver_info & BTUSB_INTEL ||
> + id->driver_info & BTUSB_INTEL_NEW) {
>
> - if (id->driver_info & BTUSB_INTEL) {
> hdev->manufacturer = 2;
> - hdev->setup = btusb_setup_intel;
> - hdev->shutdown = btusb_shutdown_intel;
> - hdev->set_diag = btintel_set_diag_mfg;
> hdev->set_bdaddr = btintel_set_bdaddr;
> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> - }
>
> - if (id->driver_info & BTUSB_INTEL_NEW) {
> - hdev->manufacturer = 2;
> - hdev->send = btusb_send_frame_intel;
> - hdev->setup = btusb_setup_intel_new;
> - hdev->hw_error = btintel_hw_error;
> - hdev->set_diag = btintel_set_diag;
> - hdev->set_bdaddr = btintel_set_bdaddr;
> - set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> - set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> - set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> + if (id->driver_info & BTUSB_INTEL) {
> + hdev->setup = btusb_setup_intel;
> + hdev->shutdown = btusb_shutdown_intel;
> + hdev->set_diag = btintel_set_diag_mfg;
> + } else {
> + hdev->send = btusb_send_frame_intel;
> + hdev->setup = btusb_setup_intel_new;
> + hdev->hw_error = btintel_hw_error;
> + hdev->set_diag = btintel_set_diag;
> + }
> }
please scrap this patch since it is not making anything easier or simpler. You
think it does, but it really doesn’t.
Regards
Marcel
Re: [PATCH v3 5/5] Bluetooth: btusb: Use the hw_reset method to allow resetting the BT chip
Hi Rajat,
> If the platform provides it, use the reset gpio to reset the BT
> chip (requested by the HCI core if needed). This has been found helpful
> on some of Intel bluetooth controllers where the firmware gets stuck and
> the only way out is a hard reset pin provided by the platform.
>
> Signed-off-by: Rajat Jain
> ---
> v3: Better error handling for gpiod_get_optional()
> v2: Handle the EPROBE_DEFER case.
>
> drivers/bluetooth/btusb.c | 40 +++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index e8e148480c91..e7631f770fae 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -475,6 +476,8 @@ struct btusb_data {
> struct usb_endpoint_descriptor *diag_tx_ep;
> struct usb_endpoint_descriptor *diag_rx_ep;
>
> + struct gpio_desc *reset_gpio;
> +
> __u8 cmdreq_type;
> __u8 cmdreq;
>
> @@ -490,6 +493,26 @@ struct btusb_data {
> int oob_wake_irq; /* irq for out-of-band wake-on-bt */
> };
>
> +
> +static void btusb_hw_reset(struct hci_dev *hdev)
> +{
> + struct btusb_data *data = hci_get_drvdata(hdev);
> + struct gpio_desc *reset_gpio = data->reset_gpio;
> +
> + /*
> + * Toggle the hard reset line if the platform provides one. The reset
> + * is going to yank the device off the USB and then replug. So doing
> + * once is enough. The cleanup is handled correctly on the way out
> + * (standard USB disconnect), and the new device is detected cleanly
> + * and bound to the driver again like it should be.
> + */
> + bt_dev_dbg(hdev, "%s: Initiating HW reset via gpio", __func__);
No __func__ here. That bt_dev_dbg does all of that via dynamic debug already.
> + clear_bit(HCI_QUIRK_HW_RESET_ON_TIMEOUT, &hdev->quirks);
> + gpiod_set_value(reset_gpio, 1);
> + mdelay(100);
> + gpiod_set_value(reset_gpio, 0);
> +}
> +
> static inline void btusb_free_frags(struct btusb_data *data)
> {
> unsigned long flags;
> @@ -2917,6 +2940,7 @@ static int btusb_probe(struct usb_interface *intf,
> const struct usb_device_id *id)
> {
> struct usb_endpoint_descriptor *ep_desc;
> + struct gpio_desc *reset_gpio;
> struct btusb_data *data;
> struct hci_dev *hdev;
> unsigned ifnum_base;
> @@ -3030,6 +3054,16 @@ static int btusb_probe(struct usb_interface *intf,
>
> SET_HCIDEV_DEV(hdev, &intf->dev);
>
> + reset_gpio = gpiod_get_optional(&data->udev->dev, "reset",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(reset_gpio)) {
> + err = PTR_ERR(reset_gpio);
> + goto out_free_dev;
> + } else if (reset_gpio) {
> + data->reset_gpio = reset_gpio;
> + hdev->hw_reset = btusb_hw_reset;
> + }
> +
How do we ensure that this is the right “reset” line. And it also needs to be
bound to some hardware unless we can guarantee that this is always the same.
> hdev->open = btusb_open;
> hdev->close = btusb_close;
> hdev->flush = btusb_flush;
> @@ -3085,6 +3119,7 @@ static int btusb_probe(struct usb_interface *intf,
> set_bit(HCI_QUIRK_STRICT_DUPLICATE_FILTER, &hdev->quirks);
> set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks);
> set_bit(HCI_QUIRK_NON_PERSISTENT_DIAG, &hdev->quirks);
> + set_bit(HCI_QUIRK_HW_RESET_ON_TIMEOUT, &hdev->quirks);
You are not messing with the quirks here please. Clearing quirks is crazy. Use
the data->flags since this should be all btusb.c specific.
>
> if (id->driver_info & BTUSB_INTEL) {
> hdev->setup = btusb_setup_intel;
> @@ -3225,6 +3260,8 @@ static int btusb_probe(struct usb_interface *intf,
> return 0;
>
> out_free_dev:
> + if (data->reset_gpio)
> + gpiod_put(data->reset_gpio);
> hci_free_dev(hdev);
> return err;
> }
> @@ -3268,6 +3305,9 @@ static void btusb_disconnect(struct usb_interface *intf)
> if (data->oob_wake_irq)
> device_init_wakeup(&data->udev->dev, false);
>
> + if (data->reset_gpio)
> + gpiod_put(data->reset_gpio);
> +
> hci_free_dev(hdev);
> }
Regards
Marcel
Re: [PATCH] Bluetooth: btusb: use irqsave() in URB's complete callback
Hi Sebastian, > The USB completion callback does not disable interrupts while acquiring > the ->lock. We want to remove the local_irq_disable() invocation from > __usb_hcd_giveback_urb() and therefore it is required for the callback > handler to disable the interrupts while acquiring the lock. > The callback may be invoked either in IRQ or BH context depending on the > USB host controller. > Use the _irqsave variant of the locking primitives. > > Cc: Marcel Holtmann > Cc: Johan Hedberg > Cc: linux-blueto...@vger.kernel.org > Signed-off-by: Sebastian Andrzej Siewior > --- > drivers/bluetooth/btusb.c | 20 > 1 file changed, 12 insertions(+), 8 deletions(-) patch has been applied to bluetooth-next tree. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Bluetooth: btusb: use irqsave() in URB's complete callback
Hi Sebastian, > The USB completion callback does not disable interrupts while acquiring > the ->lock. We want to remove the local_irq_disable() invocation from > __usb_hcd_giveback_urb() and therefore it is required for the callback > handler to disable the interrupts while acquiring the lock. > The callback may be invoked either in IRQ or BH context depending on the > USB host controller. > Use the _irqsave variant of the locking primitives. > > Cc: Marcel Holtmann > Cc: Johan Hedberg > Cc: linux-blueto...@vger.kernel.org > Signed-off-by: Sebastian Andrzej Siewior > --- > drivers/bluetooth/btusb.c | 20 > 1 file changed, 12 insertions(+), 8 deletions(-) can I get an ACK from someone ensuring that this is the direction we are going with the USB host controllers? Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: quirks: Add reset-resume quirk for QCA6174 Rome Bluetooth
Hi Leif, > This is a rework of reverted commit fd865802c66bc451dc515ed89360f84376ce1a56 > The issue is that some QCA Rome bluetooth controllers stop functioning upon > resume from suspend. > These devices seem to be losing power during suspend. This patch will enable > reset_resume in usb core (instead of btusb) and will target the specific > device 0x0cf3:0xe300 > > Signed-off-by: Leif Liddy > --- > drivers/usb/core/quirks.c | 3 +++ > 1 file changed, 3 insertions(+) patch has been applied to bluetooth-next tree. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: quirks: Add reset-resume quirk for Dell DW1820 QCA Rome Bluetooth
Hi Kai-Heng,
> Commit ("fd865802c66bc451dc515ed89360f84376ce1a56 Bluetooth: btusb: fix
> QCA Rome suspend/resume") enables reset_resume in btusb_probe(). This
> makes the device resets during btusb_open(), firmware loading gets
> interrupted as a result.
>
> We still want to reset the device to solve the original issue, but we
> should do it before btusb_open().
>
> Hence, add reset-resume quirk in usb core intead of btusb.
>
> Cc: sta...@vger.kernel.org
> Cc: Leif Liddy
> Cc: Matthias Kaehlcke
> Cc: Brian Norris
> Cc: Daniel Drake
> Signed-off-by: Kai-Heng Feng
>
> ---
> drivers/usb/core/quirks.c | 3 +++
> 1 file changed, 3 insertions(+)
patch has been applied to bluetooth-next tree.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: quirks: Add reset-resume quirk for Dell DW1820 QCA Rome Bluetooth
Hi Greg,
> Commit ("fd865802c66bc451dc515ed89360f84376ce1a56 Bluetooth: btusb: fix
> QCA Rome suspend/resume") enables reset_resume in btusb_probe(). This
> makes the device resets during btusb_open(), firmware loading gets
> interrupted as a result.
>
> We still want to reset the device to solve the original issue, but we
> should do it before btusb_open().
>
> Hence, add reset-resume quirk in usb core intead of btusb.
>
> Cc: sta...@vger.kernel.org
> Cc: Leif Liddy
> Cc: Matthias Kaehlcke
> Cc: Brian Norris
> Cc: Daniel Drake
> Signed-off-by: Kai-Heng Feng
>
> ---
> drivers/usb/core/quirks.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
> index a10b346b9777..96951104c45b 100644
> --- a/drivers/usb/core/quirks.c
> +++ b/drivers/usb/core/quirks.c
> @@ -197,6 +197,9 @@ static const struct usb_device_id usb_quirk_list[] = {
> { USB_DEVICE(0x0b05, 0x17e0), .driver_info =
> USB_QUIRK_IGNORE_REMOTE_WAKEUP },
>
> + /* QCA Rome Bluetooth in Dell DW1820 wireless module */
> + { USB_DEVICE(0x0cf3, 0xe007), .driver_info = USB_QUIRK_RESET_RESUME },
> +
can I get an ACK from you to take this patch through bluetooth-next tree? Or
are you planning to take it?
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Revert "Bluetooth: btusb: fix QCA Rome suspend/resume"
Hi Kai-Heng,
> This reverts commit fd865802c66bc451dc515ed89360f84376ce1a56.
>
> This commit causes a regression on some QCA ROME chips. The USB device
> reset happens in btusb_open(), hence firmware loading gets interrupted.
>
> Furthermore, this commit stops working after commit
> ("a0085f2510e8976614ad8f766b209448b385492f Bluetooth: btusb: driver to
> enable the usb-wakeup feature"). Reset-resume quirk only gets enabled in
> btusb_suspend() when it's not a wakeup source.
>
> If we really want to reset the USB device, we need to do it before
> btusb_open(). Let's handle it in drivers/usb/core/quirks.c.
>
> Cc: sta...@vger.kernel.org
> Cc: Leif Liddy
> Cc: Matthias Kaehlcke
> Cc: Brian Norris
> Cc: Daniel Drake
> Signed-off-by: Kai-Heng Feng
>
> ---
>
> Daniel, Cc you because this also affects your original quirk patch for
> Realtek btusb.
>
> drivers/bluetooth/btusb.c | 6 --
> 1 file changed, 6 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] bluetooth: bcm203x: don't print error when allocating urb fails
Hi Wolfram, > kmalloc will print enough information in case of failure. > > Signed-off-by: Wolfram Sang > --- > drivers/bluetooth/bcm203x.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) patch has been applied to bluetooth-next tree. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0984/1285] Replace numeric parameter like 0444 with macro
Hi Felipe, >> I find that the developers often just specified the numeric value >> when calling a macro which is defined with a parameter for access permission. >> As we know, these numeric value for access permission have had the >> corresponding macro, >> and that using macro can improve the robustness and readability of the code, >> thus, I suggest replacing the numeric parameter with the macro. >> >> Signed-off-by: Chuansheng Liu >> Signed-off-by: Baole Ni >> --- >> drivers/usb/misc/usbtest.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c >> index 6b978f0..5e81dc3 100644 >> --- a/drivers/usb/misc/usbtest.c >> +++ b/drivers/usb/misc/usbtest.c >> @@ -15,7 +15,7 @@ >> /*-*/ >> >> static int override_alt = -1; >> -module_param_named(alt, override_alt, int, 0644); >> +module_param_named(alt, override_alt, int, S_IRUSR | S_IWUSR | S_IRGRP | >> S_IROTH); > > line too long. You need to run this series through scripts/checkpatch.pl please don't give them any ideas. Next thing you know and another 1285 patch bomb is coming our way. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0984/1285] Replace numeric parameter like 0444 with macro
Hi Felipe, >> I find that the developers often just specified the numeric value >> when calling a macro which is defined with a parameter for access permission. >> As we know, these numeric value for access permission have had the >> corresponding macro, >> and that using macro can improve the robustness and readability of the code, >> thus, I suggest replacing the numeric parameter with the macro. >> >> Signed-off-by: Chuansheng Liu >> Signed-off-by: Baole Ni >> --- >> drivers/usb/misc/usbtest.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c >> index 6b978f0..5e81dc3 100644 >> --- a/drivers/usb/misc/usbtest.c >> +++ b/drivers/usb/misc/usbtest.c >> @@ -15,7 +15,7 @@ >> /*-*/ >> >> static int override_alt = -1; >> -module_param_named(alt, override_alt, int, 0644); >> +module_param_named(alt, override_alt, int, S_IRUSR | S_IWUSR | S_IRGRP | >> S_IROTH); > > line too long. You need to run this series through scripts/checkpatch.pl please don't give them any ideas. Next thing you know and another 1285 patch bomb is coming our way. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Bluetooth: btusb: match generic class code in interface descriptor
Hi Daniel,
> btusb currently has a generic match on USB device descriptors:
>{ USB_DEVICE_INFO(0xe0, 0x01, 0x01) },
>
> However, http://www.usb.org/developers/defined_class states:
>
> Base Class E0h (Wireless Controller)
> This base class is defined for devices that are Wireless controllers.
> Values not shown in the table below are reserved. These class codes are
> to be used in Interface Descriptors, with the exception of the Bluetooth
> class code which can also be used in a Device Descriptor.
>
> Add a match on the interface descriptors accordingly.
>
> This fixes compatibility with the RTL8723AU device shown below.
> This device conforms to the USB Interface Association Descriptor
> specification, which requires the device to have class ef/02/01.
> The extra IAD descriptor then specifies that interfaces 0 and 1
> belong to the same function/driver, which is true. Provided that
> the Bluetooth device class spec accepts use of the IAD, I imagine that
> technically, all btusb devices should be configured like this.
>
> T: Bus=01 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#= 3 Spd=480 MxCh= 0
> D: Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 1
> P: Vendor=0bda ProdID=0724 Rev= 2.00
> S: Manufacturer=Realtek
> S: Product=802.11n WLAN Adapter
> S: SerialNumber=00e04c01
> C:* #Ifs= 3 Cfg#= 1 Atr=e0 MxPwr=500mA
> A: FirstIf#= 0 IfCount= 2 Cls=e0(wlcon) Sub=01 Prot=01
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=81(I) Atr=03(Int.) MxPS= 16 Ivl=1ms
> E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 0 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 0 Ivl=1ms
> I: If#= 1 Alt= 1 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 9 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 9 Ivl=1ms
> I: If#= 1 Alt= 2 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 17 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 17 Ivl=1ms
> I: If#= 1 Alt= 3 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 25 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 25 Ivl=1ms
> I: If#= 1 Alt= 4 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 33 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 33 Ivl=1ms
> I: If#= 1 Alt= 5 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> E: Ad=03(O) Atr=01(Isoc) MxPS= 49 Ivl=1ms
> E: Ad=83(I) Atr=01(Isoc) MxPS= 49 Ivl=1ms
> I:* If#= 2 Alt= 0 #EPs= 4 Cls=ff(vend.) Sub=ff Prot=ff Driver=rtl8723au
> E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E: Ad=06(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
> E: Ad=87(I) Atr=03(Int.) MxPS= 64 Ivl=500us
>
> Signed-off-by: Daniel Drake
> ---
> drivers/bluetooth/btusb.c | 1 +
> 1 file changed, 1 insertion(+)
>
> This replaces/obsoletes:
> [PATCH] Bluetooth: btusb: Recognize Realtek shared wifi/bluetooth devices
> [PATCH] Bluetooth: btusb: Add Realtek devices into module device table
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 93339a4..9874aac 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -64,6 +64,7 @@ static struct usb_driver btusb_driver;
> static const struct usb_device_id btusb_table[] = {
> /* Generic Bluetooth USB device */
> { USB_DEVICE_INFO(0xe0, 0x01, 0x01) },
> + { USB_INTERFACE_INFO(0xe0, 0x01, 0x01) },
I moved this into its own line with its own comment before applying your patch.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net:usb:cdc_ncm: fix that tag Huawei devices as wwan
Hi Bjorn, >> we introduced DEVTYPE in uevent a long time ago. That is what >> userspace should be using and not second guessing on interface names. > > Yes, sorry for confusing this by mentioning the device name. This is > really about DEVTYPE. > > usbnet minidrivers use FLAG_WWAN to set both the 'wwanX' device name and > DEVTYPE=wwan. The question here is whether or not to set that flag for > Huawei NCM devices. We can discuss that in the DEVTYPE context if you > prefer. > >> Why is userspace trying to hack around the kernel anyway. > > Because you can never expect DEVTYPE to be 100% correct. There isn't a > one-to-one relationship between USB classes and DEVTYPE. So we use a > default DEVTYPE and exception lists in class drivers like cdc_ether and > cdc_ncm. These exception lists will always be incomplete, like any such > whitelist/blacklist. > > I believe we have discussed this before, and my opinion on DEVTYPE is > still that it is a best effort thing which we would have been better off > without. But it's too late to do anything about that. Userspace has to > deal with it. The kernel provides a hint. The hint cannot be trusted. if this is a 2G/3G/LTE or whatever card providing broadband networking then this is DEVTYPE=wwan. It makes no difference if this is NCM or not. It is important to know what this network interface connects to. >> This never >> really goes well unless the kernel exposes clear information to >> identify devices. If there are some weird devices, then work this out >> in the kernel and have DEVTYPE identity them correctly. > > How? These devices share device IDs. We do not touch their management > interfaces from the kernel. We depend on being able to classify device > types based on USB descriptors. How can we identify which device is wwan > and which is not if the descriptors are identical down to the device ID? What is Huawei building besides broadband cards? Are they in the business of WiFi or Ethernet now? > It is tempting to say that Huawei knows best for their own devices, if > you all find the change acceptable. I most certainly don't know better > than they do. I would have loved to travel back in time and never submit > that patch... I mean if this is about distinguishing their phones from their data cards, then surely the other USB descriptors are different so that the driver can quirk this correctly. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net:usb:cdc_ncm: fix that tag Huawei devices as wwan
Hi Bjorn, >> Hmm. Oliver is marked as the maintainer of the USB CDC code, but >> others have touched it more recently. So I'm just wildly adding people >> to the cc to comment on this patch and maybe apply it. >> Oliver/David/Ben/Bjørn? > > Adding Aleksander and Dan, too. The 'wwanX' vs 'usbX' distinction is a > hint for userspace and nothing more. We try our best to make this hint > as precise as possible, but there has never been any guarantee that it > is 100% correct. So userspace will have to deal with hints being wrong. > This has been discussed before: > http://lists.freedesktop.org/archives/modemmanager-devel/2014-April/001068.html > > What makes the Huawei NCM devices special is that the same ID is reused > for both types of devices, making it impossible to achieve perfect > hinting by adding exceptions to the generic rule. We can choose between > wrong towards 'wwanX' or wrong towards 'usbX'. But the hint WILL be > wrong in some cases no matter what we do. > > The 12d1:1506 device ID is a perfect example. Here's a report of a > Huawei E5776 which should be used as a plain 'usbX' NCM device: > http://lists.freedesktop.org/archives/modemmanager-devel/2014-April/001040.html > And here's a report of a Huawei E3276 which needs wwan management and > therefore should be hinted as a 'wwanX' NCM device: > http://www.dd-wrt.com/phpBB2/viewtopic.php?p=764920 > > So which of the two wrongs should we choose? > > Huawei wanting this changed is a strong argument for the 'usbX' > direction, of course. But this will be a user visible change which is > likely to break currently working devices until userspace adapts. And > that will normally trump almost any other argument. I don't think we > can change this now. I sincerely apologise about having added the > generic rule in the first place, but I don't see how I can go back and > change that. > > I believe we have to follow the path of least surprise: Keeping what we > have. > > But we should definitely work with userspace to ensure that a wrongly > flagged 'wwanX' device is usable without any wwan management. > Preferably without the user really noticing anything different (except > possibly the device name). we introduced DEVTYPE in uevent a long time ago. That is what userspace should be using and not second guessing on interface names. Why is userspace trying to hack around the kernel anyway. This never really goes well unless the kernel exposes clear information to identify devices. If there are some weird devices, then work this out in the kernel and have DEVTYPE identity them correctly. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Bluetooth: Add reset_resume function
Hi Josh,
>>> Bluetooth devices off of some buses such as USB may lose power across
>>> suspend/resume. When this happens, drivers may need to have the setup
>>> function called again and behave differently than a cold power on.
>>> Add a reset_resume function for drivers to call. During the
>>> reset_resume case, the flag HCI_RESET_RESUME will be set to allow
>>> drivers to differentate.
>>>
>>> Signed-off-by: Laura Abbott
>>> ---
>>> This matches with what hci_reset_dev does and also ensures
>>> the setup function gets called again.
>>> ---
>>> include/net/bluetooth/hci.h | 1 +
>>> include/net/bluetooth/hci_core.h | 1 +
>>> net/bluetooth/hci_core.c | 16
>>> 3 files changed, 18 insertions(+)
>>>
>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>> index d95da83..6285410 100644
>>> --- a/include/net/bluetooth/hci.h
>>> +++ b/include/net/bluetooth/hci.h
>>> @@ -185,6 +185,7 @@ enum {
>>> HCI_RAW,
>>>
>>> HCI_RESET,
>>> + HCI_RESET_RESUME,
>>> };
>>
>> no more addition to this list of flags please. These are userspace exposed
>> flags and with that ABI that we are never ever touching again. If you need
>> flags on a per device basis, then use the second list.
>
> It would be helpful for other developers if you added a comment to
> that effect above the enum definition. Otherwise you're going to wind
> up repeating yourself over time.
nobody has done that so far ;)
> Also, if they're exposed to userspace, should this file be using the
> uapi mechanism? I'm confused how they're exposed today, given that
> they aren't installed via 'make headers_install'. Is this manually
> synced with some other .h file in a userspace package?
This code is from 2.4.6 and with that pretty much ancient and predates UAPI.
The BlueZ userspace library provides userspace versions of these defines etc.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Bluetooth: btusb: Add reset_resume function
Hi Laura,
> Some USB hubs may lose power across suspend/resume.
> Add a reset_resume callback to properly reset those bluetoot devices.
>
> Signed-off-by: Laura Abbott
> ---
> Now the setup function is called again with the HCI_RESET_RESUME
> flag set. The various functions could then use that RESET_RESUME
> flag to determine if loading the firmware is appropriate or not.
> ---
> drivers/bluetooth/btusb.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 3c10d4d..34884cf 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3382,6 +3382,21 @@ done:
>
> return err;
> }
> +
> +static int btusb_reset_resume(struct usb_interface *intf)
> +{
> + struct btusb_data *data = usb_get_intfdata(intf);
> + struct hci_dev *hdev = data->hdev;
> + int ret;
> +
> + BT_DBG("intf %p", intf);
> +
> + ret = btusb_resume(intf);
> + if (ret)
> + return ret;
> +
> + return hci_reset_resume_dev(hdev);
> +}
it seems convenient to call btusb_resume, but I would really prefer if we
didn’t. From what I know is that when reset_resume callback is called, then the
device has been reset. So that means any prior transfer we have remembered is
null and void. So even trying to replay any of it is just a lost cause.
Instead we should clear any pending transfers and clear everything and instead
pretend that we bring the transport back to its virgin state. It also means
that isochronous transfers should be all killed since we will have no SCO
connections after this. Remember that we are telling the Bluetooth core to
reset this device.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Bluetooth: Add reset_resume function
Hi Laura,
> Bluetooth devices off of some buses such as USB may lose power across
> suspend/resume. When this happens, drivers may need to have the setup
> function called again and behave differently than a cold power on.
> Add a reset_resume function for drivers to call. During the
> reset_resume case, the flag HCI_RESET_RESUME will be set to allow
> drivers to differentate.
>
> Signed-off-by: Laura Abbott
> ---
> This matches with what hci_reset_dev does and also ensures
> the setup function gets called again.
> ---
> include/net/bluetooth/hci.h | 1 +
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 16
> 3 files changed, 18 insertions(+)
>
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index d95da83..6285410 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -185,6 +185,7 @@ enum {
> HCI_RAW,
>
> HCI_RESET,
> + HCI_RESET_RESUME,
> };
no more addition to this list of flags please. These are userspace exposed
flags and with that ABI that we are never ever touching again. If you need
flags on a per device basis, then use the second list.
> /* HCI socket flags */
> diff --git a/include/net/bluetooth/hci_core.h
> b/include/net/bluetooth/hci_core.h
> index a056c2b..14f9c72 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -941,6 +941,7 @@ int hci_register_dev(struct hci_dev *hdev);
> void hci_unregister_dev(struct hci_dev *hdev);
> int hci_suspend_dev(struct hci_dev *hdev);
> int hci_resume_dev(struct hci_dev *hdev);
> +int hci_reset_resume_dev(struct hci_dev *hdev);
> int hci_reset_dev(struct hci_dev *hdev);
> int hci_dev_open(__u16 dev);
> int hci_dev_close(__u16 dev);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index c4802f3..090762b 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1558,6 +1558,7 @@ static int hci_dev_do_close(struct hci_dev *hdev)
> BT_DBG("%s %p", hdev->name, hdev);
>
> if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) &&
> + !test_bit(HCI_RESET_RESUME, &hdev->flags) &&
> test_bit(HCI_UP, &hdev->flags)) {
> /* Execute vendor specific shutdown routine */
> if (hdev->shutdown)
> @@ -2110,6 +2111,7 @@ static void hci_power_on(struct work_struct *work)
>*/
> mgmt_index_added(hdev);
> }
> + hci_dev_test_and_clear_flag(hdev, HCI_RESET_RESUME);
If you do not use the result of the test, why bother testing at all. Also you
realize that you are a now clearing the flag on hdev->dev_flags and not
hdev->flags.
It also means that this code is not tested when you actually have had a reset
resume and then get a clean power down. Not running the shutdown procedure
would be actually wrong in that case.
> }
>
> static void hci_power_off(struct work_struct *work)
> @@ -3298,6 +3300,20 @@ int hci_reset_dev(struct hci_dev *hdev)
> }
> EXPORT_SYMBOL(hci_reset_dev);
>
> +/*
> + * For USB reset_resume callbacks
> + */
> +int hci_reset_resume_dev(struct hci_dev *hdev)
> +{
> + set_bit(HCI_RESET_RESUME, &hdev->flags);
> + hci_dev_do_close(hdev);
> + hci_dev_set_flag(hdev, HCI_SETUP);
> +
> + queue_work(hdev->req_workqueue, &hdev->power_on);
> + return 0;
> +}
> +EXPORT_SYMBOL(hci_reset_resume_dev);
> +
When we are reacting to a hardware error, we do hci_dev_do_close followed
hci_dev_do_open. Why would you queue the power on work here. It sounds more
like that this should be actually similar to hci_error_reset that gets queued.
And this is where I said the really tricky part comes in. Is the device keeping
the firmware or not. We really need to know that one first. If it keeps its
firmware, then we do not need to run through hdev->setup again.
>From a driver point of view, the current guarantee is that hdev->setup is only
>executed once. And this means really only once. It does not need to protect
>itself against being run again. So it should only be run again if the device
>looses all its states.
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
Hi Laura,
> Then avoiding the failed firmware is no solution, indeed.
> If it's a new probe, it should be never executed during resume.
Can you expand this comment? What's wrong with probing during resume?
The USB stack does carry out probes during resume under certain
circumstances. A driver lacking a reset_resume callback is one of
those circumstances.
>>>
>>> in case the platform kills the power to the USB lines, we can never
>>> do anything about this. I do not want to hack around this in the
>>> driver.
>>>
>>> What are the cases where we should implement reset_resume and would
>>> it really help here. Since the btusb.ko driver implements
>>> suspend/resume support, would reset_resume ever be called?
>>
>> One of those cases is exactly what you have been talking about: when
>> the platform kills power to the USB lines during suspend. The driver's
>> reset_resume routine will be called during resume, as opposed to the
>> probe routine being called. Therefore the driver will be able to tell
>> that this is not a new device instance.
>>
>> The other cases are less likely to occur: a device is unable to resume
>> normally and requires a reset before it will start working again, or
>> something else goes wrong along those lines.
>>
>>> However I get the feeling someone needs to go back and see if the
>>> device is the same one and just gets probed again or if it is a new
>>> one from the USB host stack perspective.
>>
>> That can be done easily enough by enabling usbcore debugging before
>> carrying out the system suspend:
>>
>> echo 'module usbcore =p' >/debug/dynamic_debug/control
>>
>> The debugging information in the kernel log will tell just what
>> happened.
>>
>>
>
> Playing around in my test setup as a baseline
>
> [ 41.991035] usb usb1-port11: not reset yet, waiting 50ms
> [ 42.092902] usb 1-11: reset full-speed USB device number 4 using xhci_hcd
> [ 42.143575] usb usb1-port11: not reset yet, waiting 50ms
> [ 42.257822] btusb 1-11:1.0: no reset_resume for driver btusb?
> [ 42.257823] btusb 1-11:1.1: no reset_resume for driver btusb?
> [ 42.257825] btusb 1-11:1.0: forced unbind
> [ 42.258305] kworker/dying (826) used greatest stack depth: 10680 bytes left
> [ 42.331342] usb 1-9.2: reset full-speed USB device number 7 using xhci_hcd
> [ 42.416631] usb 1-9.2: ep0 maxpacket = 8
> [ 42.681288] usb 1-9.1: reset low-speed USB device number 5 using xhci_hcd
> [ 42.968138] usb 1-9.1: ep 0x81 - rounding interval to 64 microframes, ep
> desc says 80 microframes
> [ 42.968157] usb 1-9.1: ep 0x82 - rounding interval to 64 microframes, ep
> desc says 80 microframes
> [ 43.036290] usb 1-9.4: reset high-speed USB device number 8 using xhci_hcd
> [ 43.123126] hub 1-9.4:1.0: hub_reset_resume
> [ 43.123581] hub 1-9.4:1.0: enabling power on all ports
> [ 43.224853] PM: resume of devices complete after 2456.587 msecs
> [ 43.225038] btusb 1-11:1.0: usb_probe_interface
> [ 43.225040] btusb 1-11:1.0: usb_probe_interface - got id
> [ 43.225802] [ cut here ]
> [ 43.225807] WARNING: CPU: 7 PID: 2844 at
> drivers/base/firmware_class.c:1118 _request_firmware+0x5ee/0x890()
>
>
> so it is trying to call the reset resume. If I try a 'dummy reset resume'
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index a7bdac0..cda8137 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3401,6 +3401,7 @@ static struct usb_driver btusb_driver = {
> #ifdef CONFIG_PM
>.suspend= btusb_suspend,
>.resume = btusb_resume,
> + .reset_resume = btusb_resume,
> #endif
>.id_table = btusb_table,
>.supports_autosuspend = 1,
>
>
> I no longer see the warning which means that probe is no longer being called.
>
> Marcel, does implementing a proper reset_resume callback seem like the right
> approach or do you need more information?
I wonder what is the right work that needs to be done in the reset_resume
callback. I am curious if devices are forgetting their firmware or not. There
is a patch to make the Realtek devices forcefully reset_resume since they
forget their firmware. So there is at least one kind of devices where the
firmware does not survive normal suspend/resume behaviour.
If the devices forget the firmware, then this means that we actually need to
tell the Bluetooth core that this device has been reset and it has to run
through hdev->setup() again. If it does that, then we have the same problem
that the firmware will not be found since userspace is not yet ready. However
we could note the fact that we tried to lookup the firmware and just know that
it was not found. So that might help already.
Devices that always require the firmware, we can assume that the firmware will
have been cached since we successfully loaded it in the first place. Which will
most likely make the Realtek devices f
Re: Running ADB on a "stock" distribution (g_ffs)
Hi Bastien, Could you specify exactly the model? >>> >>> Onda v975w >>> If android is running fine on it you may check android kernel config for this device and check which udc is enabled. >>> >>> No kernel sources from this Chinese vendor. But it looks like the >>> USB_DWC3 config is the one to enable. >>> >> >> I see that this tablet is running windows by default so just flash >> it >> with stock image and check what device manager says about udc;) > > The Baytrail UDC driver is supposed to be a DWC3 PCI device, but it > doesn't show up in lspci at all. Given that I managed to make an NFC > chip show up that I didn't know was in the machine, I guess it's > possibly hidden in the firmware somewhere. not all devices enumerate via PCI. Some of them only enumerate via ACPI. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
Hi Alan, >> Then avoiding the failed firmware is no solution, indeed. >> If it's a new probe, it should be never executed during resume. > > Can you expand this comment? What's wrong with probing during resume? > > The USB stack does carry out probes during resume under certain > circumstances. A driver lacking a reset_resume callback is one of > those circumstances. in case the platform kills the power to the USB lines, we can never do anything about this. I do not want to hack around this in the driver. What are the cases where we should implement reset_resume and would it really help here. Since the btusb.ko driver implements suspend/resume support, would reset_resume ever be called? However I get the feeling someone needs to go back and see if the device is the same one and just gets probed again or if it is a new one from the USB host stack perspective. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
Hi Takashi, >> The data is cached in RAM. More specifically, the former loaded >> firmware files are reloaded and saved at suspend for each device >> object. See fw_pm_notify() in firmware_class.c. > > OK, this may be a stupid idea, but do we know the firmware > was successfully loaded in the first place? > Also btusb is in the habit of falling back to a generic > firmware in some places. It seems to me that caching > firmware is conceptually not enough, but we'd also need > to record the absence of firmware images. in a lot of cases the firmware is optional. The device will operate fine without the firmware. There are a few devices where the firmware is required, but for many it just contains patches. It would be nice if we could tell request_firmware() if it is optional or mandatory firmware. Or if it should just cache the status of a missing firmware as well. >>> >>> OK, below is a quick hack to record the failed f/w files, too. >>> Not sure whether this helps, though. Proper tests are appreciated. >>> >>> >> >> This doesn't quite work. We end up with the name on fw_names but >> the firmware isn't actually on the firmware cache list. >> >> If request_firmware fails to get the firmware from the filesystem, >> release firmware will be called which is going to free the >> firmware_buf which has been marked as failed anyway. The only >> way to make this work would be to always piggy back and increase >> the ref so it always stays around. But this also marks the firmware >> as a permanent failure. There would need to be a hook somewhere >> to force a cache drop, else there would be no way to add new >> firmware to a running system without a reboot. >> >> Perhaps we split the difference: keep a list of firmware images >> that failed to load in the past and if one is requested during >> a time when usermodehelper isn't available, silently return an >> error? This way, if correct firmware is loaded at a regular time >> the item can be removed from the list. > > Well, IMO, it's way too much expectation for the generic f/w loader. > The driver itself must know already which should be really loaded. > The fact is that it's the driver who calls the function that might not > work in the resume path. So the driver can deal with such exceptions > at best. I keep repeating myself here. From the driver point of view it goes via probe() callback of the USB driver. So the driver does not know. For the driver it looks like a brand new device. There are platforms that might decide to just kill the power to the USB bus where the Bluetooth controller sits on. It gets the power back on resume. However this means it is a brand new device at that point. So the driver should not have to remember everything. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
Hi Oliver, >> The data is cached in RAM. More specifically, the former loaded >> firmware files are reloaded and saved at suspend for each device >> object. See fw_pm_notify() in firmware_class.c. > > OK, this may be a stupid idea, but do we know the firmware > was successfully loaded in the first place? > Also btusb is in the habit of falling back to a generic > firmware in some places. It seems to me that caching > firmware is conceptually not enough, but we'd also need > to record the absence of firmware images. in a lot of cases the firmware is optional. The device will operate fine without the firmware. There are a few devices where the firmware is required, but for many it just contains patches. It would be nice if we could tell request_firmware() if it is optional or mandatory firmware. Or if it should just cache the status of a missing firmware as well. As long as the device in question gets disconnected and we run through the USB driver probe() callback again, the btusb.ko driver can not do anything smart in this case. It has to be done in request_firmware(). Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND][PATCH] Bluetooth: Make request workqueue freezable
Hi Alan, I am not convinced. Now we are hacking the Bluetooth core layer (which has nothing to do with the drivers suspend/resume or probe) to do something different so that we do not see this warning. I can not do anything about the platform in question choosing a unplug/replug for suspend/resume instead of having a proper USB suspend and resume handling. That is pretty much out of our control. > > Actually one can do something about this. I mean, one _can_ implement > proper USB suspend and resume handling in the Bluetooth driver. At > this point the details aren't clear to me, but perhaps if the driver in > question had a reset_resume callback then it might work better. the btusb.ko driver has suspend/resume support. Are you saying we also need reset_resume support? I would rather have the USB subsystem delay the probe() callback if we tell it to. > > This is possible. I am not sure it would be the right thing to do, > though. What happens if the probe routine gets called early on during > the boot-up procedure, before userspace is up and running? The same > thing should happen here. For modules this will be hard. Since you need userspace before being able to load the modules. If built-in code, then in theory this might be possible. Depending on the order of the init sections. Of just have request_firmware() actually sleep until userspace is ready. Seriously, why is request_firmware not just sleeping for us. > > It won't work. The request_firmware call is part of the probe > sequence, which in turn is part of the resume sequence. Userspace > doesn't start running again until the resume sequence is finished. If > request_firmware waited for userspace, it would hang. Then I really have no idea on how to solve this unless we silence the warning from request_firmware. From a driver perspective we go back trough probe(). So the driver has to treat this as a new device. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Regression fix revert: "Bluetooth: Add missing reset_resume dev_pm_ops"
Hi Gustavo, >> Many btusb devices have 2 modes, a hid mode and a bluetooth hci mode. These >> devices default to hid mode for BIOS use. This means that after having been >> reset they will revert to HID mode, and are no longer usable as a HCI. >> >> Therefor it is a very bad idea to just blindly make reset_resume point to >> the regular resume handler. Note that the btusb driver has no clue how to >> switch these devices from hid to hci mode, this is done in userspace through >> udev rules, so the proper way to deal with this is to not have a reset-resume >> handler and instead let the usb-system re-enumerate the device, and re-run >> the udev rules. >> >> I must also note, that the commit message for the commit causing this >> problem has a very weak motivation for the change: >> >> "Add missing reset_resume dev_pm_ops. Missing reset_resume results in the >> following message after power management device test. This change sets >> reset_resume to btusb_resume(). >> >> [ 2506.936134] btusb 1-1.5:1.0: no reset_resume for driver btusb? >> [ 2506.936137] btusb 1-1.5:1.1: no reset_resume for driver btusb?" >> >> Making a change solely to silence a warning while also changing important >> behavior (normal resume handling versus re-enumeration) requires a commit >> message with a proper explanation why it is safe to do so, which clearly >> lacks >> here, and unsurprisingly it turns out to not be safe to make this change. >> >> Reverting the commit in question fixes bt no longer working on my Dell >> E6430 after a suspend/resume, and I believe it likely also fixes the >> following bugs: >> https://bugzilla.redhat.com/show_bug.cgi?id=988481 >> https://bugzilla.redhat.com/show_bug.cgi?id=1010649 >> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1213239 >> >> This reverts commit 502f769662978a2fe99d0caed5e53e3006107381. >> >> Cc: Shuah Khan >> Cc: Gustavo Padovan >> Cc: sta...@vger.kernel.org >> Signed-off-by: Hans de Goede >> --- >> drivers/bluetooth/btusb.c | 1 - >> 1 file changed, 1 deletion(-) > > Patch has been applied to bluetooth.git. Thanks. why? Because we have one broken Dell Bluetooth dongle. Do we actually know how this affects other chips. The dell HID Proxy thing has always been special case and that is Dell's fault. Look at the extra code that we have in hid2hci tool and its udev rules for Dell hardware. Is anybody actually willing to investigate this one properly. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] memory mapping for usbfs (v0.4)
Hi Markus, >> Do you have a userspace test program that we can use to verify that this >> does work, and that others can use to run on some different platforms to >> verify that this is actually faster? >> > > You will need one of our devices for testing I guess. Some scanners > (which use USBFS) or other low speed devices won't really utilize > usbfs too much. I think I could provide some grabber device for > testing if you want to. So no test userspace program you can knock up for us? I really hate adding new core functionality to the kernel that I have no way of testing at all, that's a recipe for it quickly breaking... >>> >>> Well do you have any device which has a userspace driver? Without a >>> device you can barely test it. >>> There's a settopbox company which added the backported patch to Linux >>> 3.2 they use USB cardreaders and even tested the devices with and >>> without mmap support. >>> I doubt that the SG support has any good testing on low end systems, >>> I'm worried that it will introduce those latency issues again which we >>> saw with 15k buffers. >> >> There are lots of userspace drivers based on libusb or libusbx. If you >> post an enhancement for libusbx to make use of your buffer mappings, > > those are free to adapt their libraries to use that feature (see first > post how to use the ioctls). > You need to find a reasonable application for those patches, I do not > know anyone else using such a heavy bulk or isochronous application on > low end systems not even on high end systems (otherwise you'd very > likely be able to find more such kernel allocation messages on the > internet and not only from us, obviously they all link to our forum or > devices). > VMWare, Qemu or Xen might be a good target for testing (when passing > through windows requests and using a camera in windows). I think the same can be done with any standard Bluetooth controller and using SCO audio data for a headset connection. These also use ISOC transfers and as of today, do not work inside KVM. So if anybody wants to see if these changes actually make a difference, then a Qemu/KVM using them might be a good test case. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Plug and play for a tty line disciple networking device
Hi Greg, How can you achieve plug and play for a ft2232 based USB serial device implementing 802.15.4 networking? The device has a 802.15.4 SOC with a UART attached to a ft2232. With firmware loaded the only thing it can do is talk the 802.15.4 tty line discipline, it is not a general purpose serial port. Right now the device works by plugging it in and it appears as a generic USB serial device like ttyUSB0. You then run a user space app which sets the line discipline, holds the port open and attaches it to the 6lowpan implementation in the networking code. But doing that is inconvenient and users needs to be trained to do it. Much simpler if we could just plug the device in and it worked. We can add a EEPROM to the ft2232 to give it a unique USB ID. Is it possible to make a kernel driver that see this ID, sets the line discipline and wires the serial port directly into the networking code? >>> >>> Yes, you can do that. >> >> Is there an existing driver in the kernel that does this? >> So far all of the ones I've checked still need a user space app. > > Look at the bluetooth drivers, they have their own line dicipline I > think. yes we do. And we also have a userspace tool (hciattach) to setup the line discipline. However the automatic setup can be easily automated with a simple udev rule. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new device for qmi_wwan and option Vodafone/ZTE K5006Z
Hi Thomas, > Now I can confirm the device works via wwan-interface with ipv6 too. > > In detail: ipv6 works in 3G-mode. (umts/hspa) > > My problem was: I am more and more surrounded by LTE, and I was not able to > force the device to register to the right network. > > LTE and IPv6 seems to be a new topic, not for the linux-driver, but for the > mobileproviders. which German operator is giving you IPv6 over LTE? Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: new device for qmi_wwan and option Vodafone/ZTE K5006Z
Hi Thomas, > The network connection is working via wwan1 , at least with IPv4. > The LTE speed here with 8-16Mbit/s is limited by my contract, not the device. > > At the moment I have problems with IPv6, but I think this is not a problem of > the device, more a problem of my apn, which may be only accessible via 3G. if you are on Vodafone Germany LTE, then yes, it only gives you IPv4 for some weird reason. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: add USB_VENDOR_AND_INTERFACE_INFO() macro
Hi Gustavo,
> A lot of Broadcom Bluetooth devices provides vendor specific interface
> class and we are getting flooded by patches adding new device support.
> This change will help us enable support for any other Broadcom with vendor
> specific device that arrives in the future.
>
> Only the product id changes for those devices, so this macro would be
> perfect for us:
>
> { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01) }
>
> Signed-off-by: Gustavo Padovan
> ---
> include/linux/usb.h | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index dea39dc..dad156b 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -829,6 +829,27 @@ static inline int usb_make_path(struct usb_device *dev,
> char *buf, size_t size)
> .bInterfaceSubClass = (sc), \
> .bInterfaceProtocol = (pr)
>
> +/**
> + * USB_VENDOR_AND_INTERFACE_INFO - describe a specific usb vendor with a
> class of usb interfaces
> + * @vend: the 16 bit USB Vendor ID
> + * @cl: bInterfaceClass value
> + * @sc: bInterfaceSubClass value
> + * @pr: bInterfaceProtocol value
> + *
> + * This macro is used to create a struct usb_device_id that matches a
> + * specific vendor with a specific class of interfaces.
> + *
> + * This is especially useful when explicitly matching devices that have
> + * vendor specific bDeviceClass values, but standards-compliant interfaces.
> + */
> +#define USB_VENDOR_AND_INTERFACE_INFO(vend, cl, sc, pr) \
> + .match_flags = USB_DEVICE_ID_MATCH_INT_INFO \
> + | USB_DEVICE_ID_MATCH_DEVICE, \
this should be USB_DEVICE_ID_MATCH_VENDOR.
> + .idVendor = (vend), \
> + .bInterfaceClass = (cl), \
> + .bInterfaceSubClass = (sc), \
> + .bInterfaceProtocol = (pr)
> +
> /* --- */
>
> /* Stuff for dynamic usb ids */
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: right place for CDMA / GSM modem IDs for usbserial autoloading?
Hi Greg, > > > > I have no problem exporting a simple sysfs attribute showing if the > > > > device is either CDMA or GSDM. I would think with that, HAL would not > > > > need to keep any kind of tables at all, and then the device info only > > > > has to stay in one place. > > > > > > This would be ideal. IIRC the only reason for keeping the tables was > > > that there was no such attribute. Sounds about right Dan? > > > > Yes, that was correct. > > > > It's not like the kernel is immutable people. If you need to work > around something you find strange in the kernel, or the kernel is not > exporting something that you need for userspace to work easier, please > ask the kernel developers! Otherwise we don't know there's an issue at > all. It's not like it is hard to find and get ahold of us... > > > However, we should keep the current HAL > > specification addition for a few reasons: > > > > 1) devices that are only supported by usb-serial; which includes things > > that are not CDMA/GSM modems > > Like what? NO DEVICES THAT ARE MODEMS SHOULD BE USING THE GENERIC > USB-SERIAL DRIVER!!! Do I need to say this again somewhere else? They > should ALL have a "real" usb-serial child driver to allow for proper > flow control, auto-loading of drivers, and the ability to actually send > data at a speed that isn't comparible to RFC 1149. > > > 2) devices that are PCMCIA serial cards (which are matched only on > > class, not the manf IDs necessarily) > > These devices are no longer being manufactured from what I can tell. So > that list should be fixed. > > > 3) previous kernel versions that wouldn't have the magic sysfs attribute > > Sure, but don't go adding new devices to the table, as that would be > wrong. > > > It would be worth exploring how to do this; but the problem is that > > since there are devices that support both GSM and CDMA, we'd need to > > figure out how to deal with that vs. sysfs-one-value-per-file. We > > shouldn't really call them "GSM" and "CDMA" but use the standards names > > as Marcel correctly pointed out on the HAL list. > > Why not have two files, one that shows up if GSM is supported, one if > CDMA is. Modems that support both would export both. It's up to > userspace to pick which to use, the kernel sure doesn't care. please see Dan's and my comments. We are not interested in the radio technology. We care about the "command set". If we wanna do it inside the kernel, then I would simply export a bitmap and then let HAL map this properly into HAL properties/lists. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: right place for CDMA / GSM modem IDs for usbserial autoloading?
Hi Greg, > > > > > I have no problem exporting a simple sysfs attribute showing if the > > > > > device is either CDMA or GSDM. I would think with that, HAL would not > > > > > need to keep any kind of tables at all, and then the device info only > > > > > has to stay in one place. > > > > > > > > This would be ideal. IIRC the only reason for keeping the tables was > > > > that there was no such attribute. Sounds about right Dan? > > > > > > Yes, that was correct. However, we should keep the current HAL > > > specification addition for a few reasons: > > > > > > 1) devices that are only supported by usb-serial; which includes things > > > that are not CDMA/GSM modems > > > > > > 2) devices that are PCMCIA serial cards (which are matched only on > > > class, not the manf IDs necessarily) > > > > > > 3) previous kernel versions that wouldn't have the magic sysfs attribute > > > > > > It would be worth exploring how to do this; but the problem is that > > > since there are devices that support both GSM and CDMA, we'd need to > > > figure out how to deal with that vs. sysfs-one-value-per-file. We > > > shouldn't really call them "GSM" and "CDMA" but use the standards names > > > as Marcel correctly pointed out on the HAL list. > > > > > > Thoughts greg? It would save us a huge .fdi file because then we could > > > simply match on the linux driver name, and do some other magic in HAL > > > itself to pull out the supported standards. > > > > what we could do is exporting a bitmap from the kernel. However this > > should be done within a subsystem and not in a per driver approach. Only > > fixing this for USB serial devices is the wrong approach in my eyes. > > It would be trivial to add this as a tty attribute, which would be the > correct place, right? for now, yes. However if we wanna handle binary/vendor protocols that encapsulate AT commands etc., then it might be not a good idea. Exporting everything as TTY devices seems to bad. However that is what we currently doing for everything in this area. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: right place for CDMA / GSM modem IDs for usbserial autoloading?
Hi Dan, > > > I have no problem exporting a simple sysfs attribute showing if the > > > device is either CDMA or GSDM. I would think with that, HAL would not > > > need to keep any kind of tables at all, and then the device info only > > > has to stay in one place. > > > > This would be ideal. IIRC the only reason for keeping the tables was > > that there was no such attribute. Sounds about right Dan? > > Yes, that was correct. However, we should keep the current HAL > specification addition for a few reasons: > > 1) devices that are only supported by usb-serial; which includes things > that are not CDMA/GSM modems > > 2) devices that are PCMCIA serial cards (which are matched only on > class, not the manf IDs necessarily) > > 3) previous kernel versions that wouldn't have the magic sysfs attribute > > It would be worth exploring how to do this; but the problem is that > since there are devices that support both GSM and CDMA, we'd need to > figure out how to deal with that vs. sysfs-one-value-per-file. We > shouldn't really call them "GSM" and "CDMA" but use the standards names > as Marcel correctly pointed out on the HAL list. > > Thoughts greg? It would save us a huge .fdi file because then we could > simply match on the linux driver name, and do some other magic in HAL > itself to pull out the supported standards. what we could do is exporting a bitmap from the kernel. However this should be done within a subsystem and not in a per driver approach. Only fixing this for USB serial devices is the wrong approach in my eyes. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Daniel, > > > > It makes no difference if you > > > > distribute the GPL library with it or not. > > > > > > If you do not distribute the GPL library, the library is simply being > > > used in the intended, ordinary way. You do not need to agree to, nor can > > > you violate, the GPL simply by using a work in its ordinary intended way. > > > > > > If the application contains insufficient copyrightable expression from > > > the library to be considered a derivative work (and purely functional > > > things do not count), then it cannot be a derivative work. The library is > > > not being copied or distributed. So how can its copyright be infringed? > > > > go ahead and create an application that uses a GPL only library. Then > > ask a lawyer if it is okay to distribute your application in binary only > > form without making the source code available (according to the GPL). > > > > http://www.gnu.org/licenses/old-licenses/gpl-2.0-faq.html#IfLibraryIsGPL > > > > http://www.gnu.org/licenses/old-licenses/gpl-2.0-faq.html#LinkingWithGPL > > In the US, at least, the belief that "Linking", in *ANY* form, with a GPL > library creates a derivative work, is fallacious. that is how FSF states it and it seems that most legal departments of big companies (US and EU based) are not taking any risk on this. So it seems that someone actually has to prove in court that these assumptions for the GPL case are wrong. > Were I to create an > application that uses, say, GTK for the interface the protected expression is > my "unique and creative" use of the GTK API for creating the specific > interface and any other code I have written using the API. I hold sole > license to the copyright on that code and am able to license said code under > the specific license of my choice. Not even getting into this one since GTK+ is a LGPL based library. Get your examples straight. > Why? Because the pre-processor is what is including any GPL'd code in my > application and expanding any macros. That is a purely mechanical process and > hence the output is not able to be separately copyrighted - if it could be, > then the copyright would be held by the *COMPILER*, and I am *NOT* bound by > the license on that code. The same applies if GPL'd code is included in my > application during the linking process. QED: The "Linking" argument used by > most people is wholly fallacious in at least one major country - and if I'm > not mistaken, the output from an automated process is similarly not > considered as carrying a separate copyright in all nations that are > signatories of or follow the Bern Convention. The GPL is a license. Nobody is talking about the copyright of your code here. You always have the copyright on your code. The point is that you have to license your code under GPL (when using a GPL library) and you are distributing your code. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: mark USB drivers as being GPL only
Hi David, > > Lets phrase this in better words as Valdis pointed out: You can't > > distribute an application (binary or source form) under anything else > > than GPL if it uses a GPL library. > > This simply cannot be correct. The only way it could be true is if the work > was a derivative work of a GPL'd work. There is no other way it could become > subject to the GPL. > > So this argument reduces to -- any work that uses a library is a derivative > work of that library. But this is clearly wrong. For work X to be a > derivative work of work Y, it must contain substantial protected expression > from work Y, but an application need not have any expression from the > libraries it uses. > > > It makes no difference if you > > distribute the GPL library with it or not. > > If you do not distribute the GPL library, the library is simply being used > in the intended, ordinary way. You do not need to agree to, nor can you > violate, the GPL simply by using a work in its ordinary intended way. > > If the application contains insufficient copyrightable expression from the > library to be considered a derivative work (and purely functional things do > not count), then it cannot be a derivative work. The library is not being > copied or distributed. So how can its copyright be infringed? go ahead and create an application that uses a GPL only library. Then ask a lawyer if it is okay to distribute your application in binary only form without making the source code available (according to the GPL). http://www.gnu.org/licenses/old-licenses/gpl-2.0-faq.html#IfLibraryIsGPL http://www.gnu.org/licenses/old-licenses/gpl-2.0-faq.html#LinkingWithGPL Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi David, > > Anyway you are still under the impression that a Linux kernel module can > > be original work in the end. We keep telling you that could be a wrong > > assumption which is based on the view of many of the kernel developers > > and of most of the lawyers that looked at this specific topic. > > > Yes, I am of that view. I accept that I could be wrong, but that also > means that I could be right. We agree, so far. The important point is > that I could be right. What will be done when somebody brings forth such > an work? Will the restriction in EXPORT_SYMBOL_GPL be removed, or will > the driver be unfairly restricted from using those other modules? You > did agree I could be right, so positing such a driver, what happens? (I > predict nothing; the driver is unfairly restricted.) whatever you feel you get away with, but hey I am not a lawyer and my reading is that any kernel module is derivative work and thus has to be placed under GPL. Feel free to disagree with me. If you think you can convince me, than you are under the wrong impression. Since even if (and this is a big if) I am wrong, my action won't lead to a copyright violation. Yours however would if you are wrong. So pick your battle. > USB drivers must NOT be restricted to GPL-licence only; that would > damage Linux. Not writing and publishing GPL drivers damages Linux. Nothing else. > > And while you are talking to a lawyer. Ask him/her if it is okay to > > create a binary only application that uses a GPL library. Tell him/her > > that it is original work. > > Where does this come from? It's right out of left field. Since I've > never suggested such a thing, could you please do me the courtesy of > retracting the suggestion that I have? Lets phrase this in better words as Valdis pointed out: You can't distribute an application (binary or source form) under anything else than GPL if it uses a GPL library. It makes no difference if you distribute the GPL library with it or not. But hey (again), feel free to disagree with me here. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Valdis, > > And while you are talking to a lawyer. Ask him/her if it is okay to > > create a binary only application that uses a GPL library. Tell him/her > > It's perfectly legal to create such an application. > > It only gets interesting if you *distribute* it... > > (And yes, this is where you *have* to be pedantic about the wording used)... true, true and true. However I was under the impression we passed that discussion point, that you can do whatever inside your own walls :) Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi David, > > I think you're missing my point: as long as the license stays the way > > it is now, you can never distribute proprietary code unless you've > > consulted a lawyer and even then you run the risk of being sued for > > infringement if the copyright holder thinks what you have is derived > > work. > > > > > Yes I can, if the proprietary code is not linked with GPL code (and the > proprietary code is original). Loadable modules are not linked. This > is a > very clear-cut case. > > > >>> that is not clear-cut case. You link at run-time. Otherwise the module > >>> would do nothing. > >>> > >> That's why it's allowed. The module isn't linked when it's distributed, > >> and the author doesn't do or cause the linking; the user does. And the > >> user never distributes in the linked state. Distribution is key to GPL. > >> > > > > so how do you build this module that is not linked without using the > > Linux kernel. > You could hand code in assembler, using Microsoft's assembler under > Windows. You could compile from C, using GCC on FreeBSD. But that's > immaterial. A module which is an original, non-derivative work, is, > well, original and non-derivative. Do you say that it must be > otherwise? Why would that be? since when does the language make any difference. Anyway you are still under the impression that a Linux kernel module can be original work in the end. We keep telling you that could be a wrong assumption which is based on the view of many of the kernel developers and of most of the lawyers that looked at this specific topic. Let me repeat this. Ask a legal counsel before you go ahead with your assumption. You might get away with it or you might not. What Greg, Alan and I try to tell you is that using EXPORT_SYMBOL or EXPORT_SYMBOL_GPL will create derivative work, but if you don't wanna believe us, that is your prerogative. So go ahead, but don't complain if you actually get sued for copyright infringement at some point and tell the court you didn't know. And while you are talking to a lawyer. Ask him/her if it is okay to create a binary only application that uses a GPL library. Tell him/her that it is original work. Good luck :) Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi David, > > I disagree here. They either play by the roles or they really do pay > > Microsoft or go with BSD. I really couldn't care less. > Then you should keep away from the kernel. The last thing that Linux > needs is someone who doesn't care if Linux succeeds or fails. "I don't > care" will lead to failure. I actually care about Linux certainly more than you do. I care about the copyright of my kernel contributions and the contributions of others. Not caring about these things is what is the worst. It means you disrespect all the work that a lot of people and companies have put into Linux. And don't quote we with "I don't care". Alan already complained about your quoting style. Don't put words in my mouth that I never said. The Linux kernel is licensed under GPL. If you don't like, go and play with someone else. You can quote me on that one. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Diego, > > I think it is perfectly within their rights to do so. I think it's > > kind of silly to try to hide it, if someone wants to boost the maximum > > transmit power, they're going to hack the firmware anyway. But if it > > makes Intel happy, well... :-) > And break the HW :-) Actually, they could be happier, since then you > have to buy another... > > [...] > > preliminary go ahead from the bosses to provide documentation under an > > NDA to Linux developers that would like to write GPL drivers for it. I > [...] > Urgh... I don't think NDAs and Open Source mix well... Are you sure your > bosses KNOW Linux and the OS in general? :-) talk to Greg KH about this. Have NDAs and open source work together is really simple. Have the NDA say that you can use all provided documentation to write an open source driver and publish it under GPL. A lot of companies are okay with this these days. The Linux Foundation should have NDA template texts for this. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi David, > >>> I think you're missing my point: as long as the license stays the way > >>> it is now, you can never distribute proprietary code unless you've > >>> consulted a lawyer and even then you run the risk of being sued for > >>> infringement if the copyright holder thinks what you have is derived > >>> work. > >>> > >> Yes I can, if the proprietary code is not linked with GPL code (and the > >> proprietary code is original). Loadable modules are not linked. This is > >> a > >> very clear-cut case. > >> > > > > that is not clear-cut case. You link at run-time. Otherwise the module > > would do nothing. > > That's why it's allowed. The module isn't linked when it's distributed, > and the author doesn't do or cause the linking; the user does. And the > user never distributes in the linked state. Distribution is key to GPL. so how do you build this module that is not linked without using the Linux kernel. Hence derivative work. Hence dynamic linking at runtime of binary only code is violating the GPL. Same goes for dynamic linking at runtime against GPL libraries. Nobody thinks that is possible and ships binary applications that link against GPL libaries. So why do you think you can distribute a binary only kernel module. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Christer, > > while the HAL case of Atheros might be still true despite the fact > > that an OpenHAL has been around for a long time now. The Intel > > argument is out of the picture since quite some time. The regulatory > > daemon was an interim solution and has been replaced by a proper > > firmware solution. So please get your examples up-to-date. > > So how does that invalidate my point? Intel did jump through a lot of > hoops to avoid giving away the code that controls their radio. When > the regulatory daemon stuff got too much complaints, they finally > redid their firmware to avoid the daemon. But they still have not > exposed the details on how to control their radio. find an Intel engineer that worked on it. There is a bigger story behind it and I am not telling it. And btw. it is perfectly fine that Intel is not giving full access to their radios. Why should they? > > You might wanna now point to hiding something in firmware, but the > > hardware, firmware, driver separation (with being hardware and > > firmware closed source) is an accepted solution. It is a clean > > separation. Interface wise and license wise. > > Yes, that is a nice solution. Provided that you have any firmware at > all. But price is everything, chips become dumber and dumber and more > control functions are pushed to the host. If you want to sell a device > in Korea, price is everything; if you can shave off 30 cents by putting > the firmware in ROM, or by using 1.5 mbits of flash instead of 2 mbits, > that means an increase in market share or profit margins. I heard this all before and I don't buy it anymore. At some point the companies in Asia will understand that the whole picture looks different and that not always cheap, cheap, cheap is best for their margins. And btw. the fully supported Linux hardware is in a lot of cases not more expensive than the other ones. > > Remember that nobody inside the community ever asked for any kind of > > IP or trade secrets. We only want specifications so we can write the > > drivers under an appropriate open source license. If the > > specification describes an API exposed via firmware then that is > > perfectly fine. > > I definitely agree. I think it's stupid of companies to hide away > their documentation out of fear of, well, something. I find it > extremely frustrating when I bought a device touted as "the first open > Linux mobile", just to find out that it used a binary-only kernel module > for the M-Systems DiskOnChip. A quite nice phone, but due to that one > module, it was completely impossible to use anything but the ancient > 2.4 kernel it came with. You got one of the Greenphones ;) > I also think that my customers, that decided to keep their kernel > modules binary only, made a big mistake and have told them so. But I > still think it's better for the Linux community to be a bit soft on > such companies for a while. It's better to let them get away with it > for a while, and slowly try to convince them about the error of their > ways, rather than see them go with Windows CE or a BSD. I disagree here. They either play by the roles or they really do pay Microsoft or go with BSD. I really couldn't care less. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Graeme, > > if the overall intention is to write a Linux kernel module/driver then > > it counts as derivative work for me. No matter how tricky you are and > > much you try to circumvent or try to hide this fact. However that is my > > personal opinion. You don't have to agree with me here. Ask your legal > > counsel. > > I think you're way of base here. Copyright doesn't cover intentions, > it covers expression in a tangible form. So the intention is irrelevant, > what is expressed in the file is what's relevant. If the file doesn't > contain GPL code, then in itself it isn't subject to the GPL. > > [ If I were the author of such a file, then I would be highly upset >at a bunch of people claiming rights over my independent work, >just because it could possibly be linked with their code. ] nobody said that btw. You can write whatever code you want and put it under whatever license you want. But in the case you wanna write a kernel driver/module you have to use exported kernel symbols from this specific kernel that is released und GPL. This goes for me under derivative work. We are on the topic of kernel modules and not some other code. I am not a lawyer. So feel free to disagree with we :) > > You have to make it GPL if you link it against a GPL shim. And in case > > the shim uses EXPORT_SYMBOL_GPL, the shim has to be GPL. Again, you > > don't have to agree with me, but I would advise legal counsel now. > > It is entirely possible that there is no way of licensing a shim > in such a way that it can be distributed with GPL code. I don't > know, I haven't investigated this issue. But irrespective > of this, the licensing of the shim doesn't affect the > licensing of the non-GPL code in itself. People with lots > of lawyers like NVidia seem to think that it is possible > to create a license compatible shim. Maybe they are wrong. This has been stated so many times. If NVidia's legal counsel think they get away with it. Fine. My personal opinion is that they are violating the copyright of the kernel developers. > > To put this in clear and understandable words. The end user has to break > > the GPL license and thus violate the copyright of the kernel developers. > > Not true. The GPL covers copying, not use. The end user is free to > use the GPL code in almost any way they like, including linking > it to non-GPL code. They just can't copy (ie. distribute) the result. This is true, but the real-world problem here is that they have to keep the created binary to themself. Any kind of distribution would make them responsible to distribute the full source code (including build tools). I don't wanna even get into this area since it opens a total different can of worms :( Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Alan, > > To put this in clear and understandable words. The end user has to break > > the GPL license and thus violate the copyright of the kernel developers. > > Not quite - the GPL deals with distribution. You can put whatever you > like in your own kernel if you don't ever distribute it. Shipping code > for people to put back into kernels with that as a clear intent gets you > back into more murky waters. I know about that, but my comment was more about the fact that the end user receives a 3rd party source+binary distribution and then has to assemble it on their side. Btw. not to mention the fact that the GPL also talks about the tools to actually build the source code :) Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Graeme, > > how to do you wanna write a new original Linux driver (modular or > > built-in) without creating derivative work. This is not possible and due > > to the fact that it is derivative work the driver becomes automatically > > licensed under GPL. This is not a gray area. > > By not using any of the Linux kernel includes or libraries ? > If a file was (for instance) pure standard C with no reference > to the Linux Kernel, you can't argue that somehow this > file (in itself) is a derivative work of the Linux kernel. > Make it loadable with a (carefully licensed) shim to let it interact > with the rest of the system, and nothing has changed > about that particular files copyright or licensable status > (but see the discussion below about collective works). if the overall intention is to write a Linux kernel module/driver then it counts as derivative work for me. No matter how tricky you are and much you try to circumvent or try to hide this fact. However that is my personal opinion. You don't have to agree with me here. Ask your legal counsel. > > The gray area that exists if you have code that was written for some > > other operating system and licensed under some proprietary license in > > the first place. And now that code is used in conjunction with a glue > > layer to make it loadable as kernel module. > > Why is this gray ? It shouldn't matter what the origin of > such a file is, if the file doesn't include anything that is > GPL, it (in itself) isn't derivative and doesn't get covered > by the GPL. You have to make it GPL if you link it against a GPL shim. And in case the shim uses EXPORT_SYMBOL_GPL, the shim has to be GPL. Again, you don't have to agree with me, but I would advise legal counsel now. > >>It is precisely the fact that it is a loadable module, and does not form > >>part of the kernel, that removes the requirement to distribute it under > >>GPL. > > The mechanism of linking is irrelevant. It's a matter of > whether a collective work (that may include the non-GPL > file talked about previously) includes GPL components. > If it does, then the collective work is a derivative work > of the GPL components, and the whole work has to be covered > by the GPL. So while the non-GPL file could be distributed > separately with a non-GPL license, as soon as you package > it together with GPL components in such a way that it > is not "mere aggregation", (and the fact that it interacts > and indirectly depends on GPL components to operate > shows clearly that it's not mere aggregation), then > it becomes subject to the GPL. > > So if you don't want this non-GPL component to > be subject to the GPL, you must distribute it independently, > and get the end user to load it into their system. You > can't package or distribute it with the GPL components > it interacts with. To put this in clear and understandable words. The end user has to break the GPL license and thus violate the copyright of the kernel developers. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Chris, > > If the developers say that this symbol can only be used in GPL code (and > > with EXPORT_SYMBOL_GPL it is quite clear) then you have to obey to that > > license or don't use this symbol at all. > > > > If you use that symbol inside non-GPL (meaning you link at runtime) then > > you are in violation of the GPL license. We can't make it much clearer. > > Not necessarily so. The developers feel that any code using that symbol > is necessarily a derivative work, but at the end of the day it would be > up to the legal system to decide whether it really is or not. > > If the courts decided that the symbol could be used and the driver > wouldn't be a derivative work, it would be perfectly legal to use a > GPL'd shim to "re-export" the symbol, essentially stripping off the > GPL-only protection for that symbol. I agree with you that a court can decide that the usage of a EXPORT_SYMBOL_GPL symbol is not derivative work, but I see the likelihood of this happening as almost non existent. And even if so then you still have to deal with the fact that the license of this symbol is clearly GPL. No questions asked about that, because it says so and due technical means you can't load a non-GPL kernel module that uses EXPORT_SYMBOL_GPL symbol without tainting the kernel. The same fact is valid in userspace where you can't link (not even runtime) a GPL library into a non-GPL program. However if anyone wants to fight the license, be my guest and do so :) > In our group all kernel modules that we write are GPL'd, as it lets us > sleep at night, simplifies our lives, and makes the lawyers much > happier. Other people may be willing to take more risks. All big companies are going this way. And licenses beside, there are valid technical points in making your driver open source and get it merged upstream. Just a hint for all these binary-only people :) Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi David, > > if a new drivers is originally written for Linux, then you are breaking > > the GPL. > > Completely wrong. However if the driver is distributed as built-in, then it > would need to be licensed under GPL. This means that a driver can be > written and distributed as a module under any licence, proprietary or > otherwise, presumably with the restriction that it may NOT be built-in. how to do you wanna write a new original Linux driver (modular or built-in) without creating derivative work. This is not possible and due to the fact that it is derivative work the driver becomes automatically licensed under GPL. This is not a gray area. The gray area that exists if you have code that was written for some other operating system and licensed under some proprietary license in the first place. And now that code is used in conjunction with a glue layer to make it loadable as kernel module. > > You driver was meant to be > > running as Linux kernel module and thus it is derivative work. > > It is precisely the fact that it is a loadable module, and does not form > part of the kernel, that removes the requirement to distribute it under GPL. That is such a nonsense. Stop distributing FUD and start talking to a lawyer. You are clearly under some weird impression what the GPL means and what it implies. > > What are you arguing here. It makes no difference if it is technical or > > not. The EXPORT_SYMBOL_GPL gives you a clear hint that when using this > > symbol, you have to obey to the GPL. > > And that "hint" is a lie. If the developers say that this symbol can only be used in GPL code (and with EXPORT_SYMBOL_GPL it is quite clear) then you have to obey to that license or don't use this symbol at all. If you use that symbol inside non-GPL (meaning you link at runtime) then you are in violation of the GPL license. We can't make it much clearer. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi David, > > I think you're missing my point: as long as the license stays the way > > it is now, you can never distribute proprietary code unless you've > > consulted a lawyer and even then you run the risk of being sued for > > infringement if the copyright holder thinks what you have is derived > > work. > > Yes I can, if the proprietary code is not linked with GPL code (and the > proprietary code is original). Loadable modules are not linked. This is a > very clear-cut case. that is not clear-cut case. You link at run-time. Otherwise the module would do nothing. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi Christer, > > In the cited example it's illegal to go outside certain parameters > > SOMEWHERE (if it was illegal everywhere, the the hardware shouldn't > > allow it and the sw could do nothing... not considering hw mods). > > Another example is WiFi: USA, Europe and Japan allows a different number > > of channels. But every AP I have had simply asks which country the user > > is in, then allows only a limited choice for the channel to use. > > If I'm in Europe but tell my AP that I'm in Japan, it lets me choose > > channel 13 (Europe allows up to ch 11). If the "cops" find it out, they > > prosecute ME, not my AP! It's not a TECHNICAL limit, it's a LEGAL one. > > So there's no point in keeping a driver closed *just* because else > > someone could hack it to make it work outside the allowed parameters: > > even a closed driver is hackable (just reverse engineer it...). > > > > Think about this scenario: a closed source driver contains: > > if(power<100) > > setpower(power); > > else > > setpower(100); > > to limit tx power and make it legal. Then the user finds this 100 and > > replaces it with 255 (inside the binary-only module), actually allowing > > for more than twice (if power is in mW) the legally permitted power. > > Is it legal? > > > I don't think so (and I doubt you can find any lawyer that does). But > > it's not THE DRIVER that's doing something illegal. It's THE USER. > > It would be equally illegal if the driver was open source. Simpler to > > do, but equally illegal. > > It isn't that easy. The "Tamper-Proof Torx" screws on a vacuum cleaner > or a toaster won't stop anybody from opening up the thing, I mean every > little hardware store stocks those Torx bits. But by using a slightly > odd screw, the company can say "look, we'we done all we can to stop > them, but the user bypassed our security device, and it's not our > fault". Apparently Intel and Atheros are trying to protect themselves > in a similar way, they Open Source everything except for the regulatory > daemon (Intel) or HAL object file (Atheros). Why? Because they belive > that if they give away the sources to those parts they do the software > equivalent of putting a normal Phillips screw in a home appliance. > (Personally I think what they are doing is ridiculous, but apparently > those companies' lawyers dont' agree). while the HAL case of Atheros might be still true despite the fact that an OpenHAL has been around for a long time now. The Intel argument is out of the picture since quite some time. The regulatory daemon was an interim solution and has been replaced by a proper firmware solution. So please get your examples up-to-date. You might wanna now point to hiding something in firmware, but the hardware, firmware, driver separation (with being hardware and firmware closed source) is an accepted solution. It is a clean separation. Interface wise and license wise. Remember that nobody inside the community ever asked for any kind of IP or trade secrets. We only want specifications so we can write the drivers under an appropriate open source license. If the specification describes an API exposed via firmware then that is perfectly fine. Regards Marcel - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: mark USB drivers as being GPL only
Hi David,
> > As there is some controversy over the definition of derived work
> > (think Linus' comments on porting a driver or a filesystem from
> > another operating system here), we use the EXPORT_SYMBOL_GPL
> > annotations as a big warning sign that what you're doing is likely to
> > be considered as a derived work.
> Let's consider a totally original USB driver. There are an infinite
> number of them, some still to be written.
if a new drivers is originally written for Linux, then you are breaking
the GPL. There is no other way to name this. Using EXPORT_SYMBOL or
EXPORT_SYMBOL_GPL make no difference here. You driver was meant to be
running as Linux kernel module and thus it is derivative work. While
there is a gray area, but this case has always been pretty clear.
> > If the USB developers want to
> > annotate their code with EXPORT_SYMBOL_GPL, why the hell do you want
> > to argue about it?
> Have I the wrong end of the stick? Isn't that mark restricting an
> interface to GPL _callers_?Isn't it a technical switch that means,
> "Don't use my software if yours isn't (also) GPL"? As such it's mere
> political rhetoric, devoid of any binding power.
What are you arguing here. It makes no difference if it is technical or
not. The EXPORT_SYMBOL_GPL gives you a clear hint that when using this
symbol, you have to obey to the GPL. Even the EXPORT_SYMBOL is protected
by the same GPL license. And thus both has the same binding power to be
used from GPL modules only.
At this point I would strongly advise to talk to lawyer since you are
obvious missing the point here.
> > If you want to
> > develop for Linux, you're most certainly better off always
> > distributing your code under the GPLv2
>
> I agree; but let's not disadvantage applications where regulatory
> requirements prohibit GPL code, nor applications where the proprietor
> simply chooses to keep the work proprietary. A proprietary module is
> simply a piece of software. Many people couldn't use Linux if they
> couldn't run proprietary software on it.
First of all we are talking about kernel modules here. Not the
userspace. So stop this FUD.
> > But what I don't understand
> > is why people insist using the Linux kernel for something it clearly
> > can never really properly support (proprietary code)?
> >
>
> That's defeatist. Of course the Linux kernel can properly support
> ("run") proprietary code. It would be a miserable excuse for an
> operating system if it couldn't.
In userspace, yes, the kernel would "run" proprietary code fully legally
without any problem. As a kernel module, the only safe answer is no. And
in case of EXPORT_SYMBOL_GPL, it is pretty clear. You would obviously
violate the license.
Regards
Marcel
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] Cleanup FISH/SOUP mess in pl2303 driver (part 1).
Hi Dave,
> There's a lot of code motion in the first four patches
> (with no explanation) that seems to be greatly larger than
> the net effect of applying all four patches.
> I did so, just to see the end result, which was a lot more 'reviewable',
> ending up with this..
>
>
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index cf8add9..15097a4 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -310,6 +310,28 @@ static unsigned int pl2303_buf_get(struct pl2303_buf
> *pb, char *buf,
> return count;
> }
>
> +static int pl2303_vendor_read(__u16 value, __u16 index,
> + struct usb_serial *serial, unsigned char *buf)
> +{
> + int res = usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev, 0),
> + VENDOR_READ_REQUEST, VENDOR_READ_REQUEST_TYPE,
> + value, index, buf, 1, 100);
> + dbg("0x%x:0x%x:0x%x:0x%x %d - %x", VENDOR_READ_REQUEST_TYPE,
> + VENDOR_READ_REQUEST, value, index, res, buf[0]);
> + return res;
> +}
> +
> +static int pl2303_vendor_write(__u16 value, __u16 index,
> + struct usb_serial *serial)
> +{
> + int res = usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev, 0),
> + VENDOR_WRITE_REQUEST, VENDOR_WRITE_REQUEST_TYPE,
> + value, index, NULL, 0, 100);
> + dbg("0x%x:0x%x:0x%x:0x%x %d", VENDOR_WRITE_REQUEST_TYPE,
> + VENDOR_WRITE_REQUEST, value, index, res);
> + return res;
> +}
> +
> static int pl2303_startup(struct usb_serial *serial)
> {
> struct pl2303_private *priv;
> @@ -671,7 +693,7 @@ static int pl2303_open(struct usb_serial_port *port,
> struct file *filp)
> struct ktermios tmp_termios;
> struct usb_serial *serial = port->serial;
> struct pl2303_private *priv = usb_get_serial_port_data(port);
> - unsigned char *buf;
> + unsigned char buf;
> int result;
>
> dbg("%s - port %d", __FUNCTION__, port->number);
> @@ -681,43 +703,27 @@ static int pl2303_open(struct usb_serial_port *port,
> struct file *filp)
> usb_clear_halt(serial->dev, port->read_urb->pipe);
> }
>
> - buf = kmalloc(10, GFP_KERNEL);
> - if (buf==NULL)
> - return -ENOMEM;
> -
> -#define FISH(a,b,c,d)
> \
> - result=usb_control_msg(serial->dev, usb_rcvctrlpipe(serial->dev,0),
> \
> -b, a, c, d, buf, 1, 100);
> \
> - dbg("0x%x:0x%x:0x%x:0x%x %d - %x",a,b,c,d,result,buf[0]);
> -
> -#define SOUP(a,b,c,d)
> \
> - result=usb_control_msg(serial->dev, usb_sndctrlpipe(serial->dev,0),
> \
> -b, a, c, d, NULL, 0, 100);
> \
> - dbg("0x%x:0x%x:0x%x:0x%x %d",a,b,c,d,result);
> -
> - FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8484, 0);
> - SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 0x0404, 0);
> - FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8484, 0);
> - FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8383, 0);
> - FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8484, 0);
> - SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 0x0404, 1);
> - FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8484, 0);
> - FISH (VENDOR_READ_REQUEST_TYPE, VENDOR_READ_REQUEST, 0x8383, 0);
> - SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 0, 1);
> - SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 1, 0);
> + pl2303_vendor_read(0x8484, 0, serial, &buf);
> + pl2303_vendor_write(0x0404, 0, serial);
> + pl2303_vendor_read(0x8484, 0, serial, &buf);
> + pl2303_vendor_read(0x8383, 0, serial, &buf);
> + pl2303_vendor_read(0x8484, 0, serial, &buf);
> + pl2303_vendor_write(0x0404, 1, serial);
> + pl2303_vendor_read(0x8484, 0, serial, &buf);
> + pl2303_vendor_read(0x8383, 0, serial, &buf);
> + pl2303_vendor_write(0, 1, serial);
> + pl2303_vendor_write(1, 0, serial);
>
> if (priv->type == HX) {
> /* HX chip */
> - SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 2, 0x44);
> + pl2303_vendor_write(2, 0x44, serial);
> /* reset upstream data pipes */
> - SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 8, 0);
> - SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 9, 0);
> + pl2303_vendor_write(8, 0, serial);
> + pl2303_vendor_write(9, 0, serial);
> } else {
> - SOUP (VENDOR_WRITE_REQUEST_TYPE, VENDOR_WRITE_REQUEST, 2, 0x24);
> + pl2303_vendor_write(2, 0x24, serial);
> }
>
> - kfree(buf);
> -
> /* Setup termios */
> if (port->tty) {
> pl2303_set_termios(por
