This commit fixes a regression introduced by the commit 82f09a637dd3
("HID: ft260: improve error handling of ft260_hid_feature_report_get()")
when upon USB disconnect, the ft260 i2c device is still available within
the /dev folder.
Signed-off-by: Michael Zaidman <[email protected]>
Acked-by: Aaron Jones (FTDI-UK) <[email protected]>
---
drivers/hid/hid-ft260.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index f43a8406cb9a..fa73a35aaf98 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -742,8 +742,8 @@ static int ft260_is_interface_enabled(struct hid_device *hdev)
int ret;
ret = ft260_get_system_config(hdev, &cfg);
- if (ret)
- return ret;
+ if (ret < 0)
+ return 0;
ft260_dbg("interface: 0x%02x\n", interface);
ft260_dbg("chip mode: 0x%02x\n", cfg.chip_mode);
@@ -754,23 +754,16 @@ static int ft260_is_interface_enabled(struct hid_device *hdev)
switch (cfg.chip_mode) {
case FT260_MODE_ALL:
case FT260_MODE_BOTH:
- if (interface == 1) {
+ if (interface == 1)
hid_info(hdev, "uart interface is not supported\n");
- return 0;
- }
- ret = 1;
+ else
+ ret = 1;
break;
case FT260_MODE_UART:
- if (interface == 0) {
- hid_info(hdev, "uart is unsupported on interface 0\n");
- ret = 0;
- }
+ hid_info(hdev, "uart interface is not supported\n");
break;
case FT260_MODE_I2C:
- if (interface == 1) {
- hid_info(hdev, "i2c is unsupported on interface 1\n");
- ret = 0;
- }
+ ret = 1;
break;
}
return ret;
@@ -1004,11 +997,9 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
static void ft260_remove(struct hid_device *hdev)
{
- int ret;
struct ft260_device *dev = hid_get_drvdata(hdev);
- ret = ft260_is_interface_enabled(hdev);
- if (ret <= 0)
+ if (!dev)
return;
sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
--
2.25.1
On Tue, 20 Jul 2021, Michael Zaidman wrote:
> This commit fixes a regression introduced by the commit 82f09a637dd3
> ("HID: ft260: improve error handling of ft260_hid_feature_report_get()")
> when upon USB disconnect, the ft260 i2c device is still available within
> the /dev folder.
Thanks for the fix. Could you please make the changelog a little bit more
verbose though? Namely describe the issue in the code the patch is fixing
(and how). Thanks,
--
Jiri Kosina
SUSE Labs
This commit fixes a functional regression introduced by the commit 82f09a637dd3
("HID: ft260: improve error handling of ft260_hid_feature_report_get()")
when upon USB disconnect, the FTDI FT260 i2c device is still available within
the /dev folder.
In my company's product, where the host USB to FT260 USB connection is
hard-wired in the PCB, the issue is not reproducible. To reproduce it, I used
the VirtualBox Ubuntu 20.04 VM and the UMFT260EV1A development module for the
FTDI FT260 chip:
Plug the UMFT260EV1A module into a USB port and attach it to VM.
The VM shows 2 i2c devices under the /dev:
michael@michael-VirtualBox:~$ ls /dev/i2c-*
/dev/i2c-0 /dev/i2c-1
The i2c-0 is not related to the FTDI FT260:
michael@michael-VirtualBox:~$ cat /sys/bus/i2c/devices/i2c-0/name
SMBus PIIX4 adapter at 4100
The i2c-1 is created by hid-ft260.ko:
michael@michael-VirtualBox:~$ cat /sys/bus/i2c/devices/i2c-1/name
FT260 usb-i2c bridge on hidraw1
Now, detach the FTDI FT260 USB device from VM. We expect the /dev/i2c-1
to disappear, but it's still here:
michael@michael-VirtualBox:~$ ls /dev/i2c-*
/dev/i2c-0 /dev/i2c-1
And the kernel log shows:
[ +0.001202] usb 2-2: USB disconnect, device number 3
[ +0.000109] ft260 0003:0403:6030.0002: failed to retrieve system status
[ +0.000316] ft260 0003:0403:6030.0003: failed to retrieve system status
It happens because the commit 82f09a637dd3 changed the ft260_get_system_config()
return logic. This caused the ft260_is_interface_enabled() to exit with error
upon the FT260 device USB disconnect, which in turn, aborted the ft260_remove()
before deleting the FT260 i2c device and cleaning its sysfs stuff.
This commit restores the FT260 USB removal functionality and improves the
ft260_is_interface_enabled() code to handle correctly all chip modes defined
by the device interface configuration pins DCNF0 and DCNF1.
Signed-off-by: Michael Zaidman <[email protected]>
Acked-by: Aaron Jones (FTDI-UK) <[email protected]>
---
drivers/hid/hid-ft260.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index f43a8406cb9a..e73776ae6976 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -742,7 +742,7 @@ static int ft260_is_interface_enabled(struct hid_device *hdev)
int ret;
ret = ft260_get_system_config(hdev, &cfg);
- if (ret)
+ if (ret < 0)
return ret;
ft260_dbg("interface: 0x%02x\n", interface);
@@ -754,23 +754,16 @@ static int ft260_is_interface_enabled(struct hid_device *hdev)
switch (cfg.chip_mode) {
case FT260_MODE_ALL:
case FT260_MODE_BOTH:
- if (interface == 1) {
+ if (interface == 1)
hid_info(hdev, "uart interface is not supported\n");
- return 0;
- }
- ret = 1;
+ else
+ ret = 1;
break;
case FT260_MODE_UART:
- if (interface == 0) {
- hid_info(hdev, "uart is unsupported on interface 0\n");
- ret = 0;
- }
+ hid_info(hdev, "uart interface is not supported\n");
break;
case FT260_MODE_I2C:
- if (interface == 1) {
- hid_info(hdev, "i2c is unsupported on interface 1\n");
- ret = 0;
- }
+ ret = 1;
break;
}
return ret;
@@ -1004,11 +997,9 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
static void ft260_remove(struct hid_device *hdev)
{
- int ret;
struct ft260_device *dev = hid_get_drvdata(hdev);
- ret = ft260_is_interface_enabled(hdev);
- if (ret <= 0)
+ if (!dev)
return;
sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
--
2.25.1
On Thu, 29 Jul 2021, Michael Zaidman wrote:
> This commit fixes a functional regression introduced by the commit 82f09a637dd3
> ("HID: ft260: improve error handling of ft260_hid_feature_report_get()")
> when upon USB disconnect, the FTDI FT260 i2c device is still available within
> the /dev folder.
>
> In my company's product, where the host USB to FT260 USB connection is
> hard-wired in the PCB, the issue is not reproducible. To reproduce it, I used
> the VirtualBox Ubuntu 20.04 VM and the UMFT260EV1A development module for the
> FTDI FT260 chip:
>
> Plug the UMFT260EV1A module into a USB port and attach it to VM.
>
> The VM shows 2 i2c devices under the /dev:
> michael@michael-VirtualBox:~$ ls /dev/i2c-*
> /dev/i2c-0 /dev/i2c-1
>
> The i2c-0 is not related to the FTDI FT260:
> michael@michael-VirtualBox:~$ cat /sys/bus/i2c/devices/i2c-0/name
> SMBus PIIX4 adapter at 4100
>
> The i2c-1 is created by hid-ft260.ko:
> michael@michael-VirtualBox:~$ cat /sys/bus/i2c/devices/i2c-1/name
> FT260 usb-i2c bridge on hidraw1
>
> Now, detach the FTDI FT260 USB device from VM. We expect the /dev/i2c-1
> to disappear, but it's still here:
> michael@michael-VirtualBox:~$ ls /dev/i2c-*
> /dev/i2c-0 /dev/i2c-1
>
> And the kernel log shows:
> [ +0.001202] usb 2-2: USB disconnect, device number 3
> [ +0.000109] ft260 0003:0403:6030.0002: failed to retrieve system status
> [ +0.000316] ft260 0003:0403:6030.0003: failed to retrieve system status
>
> It happens because the commit 82f09a637dd3 changed the ft260_get_system_config()
> return logic. This caused the ft260_is_interface_enabled() to exit with error
> upon the FT260 device USB disconnect, which in turn, aborted the ft260_remove()
> before deleting the FT260 i2c device and cleaning its sysfs stuff.
>
> This commit restores the FT260 USB removal functionality and improves the
> ft260_is_interface_enabled() code to handle correctly all chip modes defined
> by the device interface configuration pins DCNF0 and DCNF1.
>
> Signed-off-by: Michael Zaidman <[email protected]>
> Acked-by: Aaron Jones (FTDI-UK) <[email protected]>
Thanks for the respin. Queued in for-5.14/upstream-fixes.
--
Jiri Kosina
SUSE Labs