patch "USB: serial: cp210x: add support for GPIOs on CP2108" added to usb-next
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
A message that is not in the archive (sent off-list or not yet received): PH0PR11MB4888B9D24A5F90E8AA02B28B81019@PH0PR11MB4888.namprd11.prod.outlook.com
Johan Hovold
maintainerHi Tung,
On Wed, Jun 30, 2021 at 08:58:29AM +0000, Tung Pham wrote:
First, thanks for testing the patch.
Please make sure to CC the usb list when reporting problems. I've added it to CC now.
Quoted 6 lines
When I test the apply patch with Linux kernel.
https://github.com/torvalds/linux/blob/master/drivers/usb/serial/cp210x.cwhen I setting the CP2108 with GPIO 0 and GPIO 1 are alternative functions (not GPIO) using simplicity studio.
And then test with the driver, it shows:
Quoted 8 lines
root@ubuntu:/sys/class/gpio# ls export gpiochip496 unexport root@ubuntu:/sys/class/gpio# echo 496 > /sys/class/gpio/export bash: echo: write error: No such device root@ubuntu:/sys/class/gpio# echo 497 > /sys/class/gpio/export bash: echo: write error: No such device This is what we expected because the GPIO 0 and GPIO 1 are alternative functions (not GPIO).
Are you sure you're using the latest mainline kernel here, that is, 5.13 here?
With recent kernel's you would not see -ENODEV ("No such device") but rather -EINVAL ("Invalid argument").
What does "uname -a" say?
And which version of the patch are you applying? Are you sure it's the latest?
Quoted 10 lines
But when I test the branch of Mr Gregkh.
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/tree/drivers/usb/serial/cp210x.c?h=usb-testing
when I setting the CP2108 with GPIO 0 and GPIO 1 are alternative functions (not GPIO) using simplicity studio.
And then test with the driver, it shows:root@ubuntu:/sys/class/gpio# ls export gpiochip496 unexport root@ubuntu:/sys/class/gpio# echo 496 > /sys/class/gpio/export root@ubuntu:/sys/class/gpio# echo 497 > /sys/class/gpio/export That mean it still use GPIO 0 and GPIO 1 as GPIO functions.
It does seem that way, but to be sure, do the
/sys/class/gpio/gpio496 /sys/class/gpio/gpio497
directories show up as well?
Quoted 2 lines
I don't know why there is that different. We can discuss more if you know the information.
I just did a quick test using the code in the usb-next (testing) branch and it seems to work as expected when hard-coding alternate functions for some of the pins in the driver.
I did not actually reprogram the device so perhaps something is broken with regards to reading the port configuration. But then you should see the problem if you apply the patch to an earlier kernel as well.
Can you apply the below patch on top of Greg's usb-next (or usb-testing) branch and send me the logs from when connecting the device?
Johan
From 4ac38b282cfa1e6a17ff9d11b886cbebce3b9654 Mon Sep 17 00:00:00 2001
From: Johan Hovold <[email protected]>
Date: Wed, 30 Jun 2021 11:59:28 +0200
Subject: [PATCH] dbg: USB: serial: cp210x: dump CP2108 alternate function
config
--- drivers/usb/serial/cp210x.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/usb/serial/cp210x.c +3 -0
@@ -1797,6 +1797,9 @@ static int cp2108_gpio_init(struct usb_serial *serial)if (result <0 )return result; +dev_info (&serial->interface->dev,"%s - %*ph\n" , __func__, +4 , config.enhancedfxn_ifc); + priv->gc.ngpio =16 ; priv->gpio_pushpull =le16_to_cpu (config.reset_state.gpio_mode_pb1); gpio_latch =le16_to_cpu (config.reset_state.gpio_latch_pb1);
Signature · 1 line
2.31.1
Dear Johan.
Quoted 1 line
Please make sure to CC the usb list when reporting problems. I've added it to CC now.
Yes, I will remember to do this next time.
Quoted 4 lines
Are you sure you're using the latest mainline kernel here, that is,
5.13 here?
With recent kernel's you would not see-ENODEV("No such device") but rather-EINVAL("Invalid argument"). What does "uname -a" say?
I use the kernel 5.8.0-59-generic, I think either result ("No such device") or ("Invalid argument") may be accepted in this case.
Quoted 1 line
And which version of the patch are you applying? Are you sure it's the latest?
I apply patch V13 from last mail that you send.
Quoted 7 lines
Quoted 6 lines
But when I test the branch of Mr Gregkh.
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/tree/dr
ivers/usb/serial/cp210x.c?h=usb-testingroot@ubuntu:/sys/class/gpio# echo 496 > /sys/class/gpio/export root@ubuntu:/sys/class/gpio# echo 497 > /sys/class/gpio/export That mean it still use GPIO 0 and GPIO 1 as GPIO functions.
Quoted 6 lines
It does seem that way, but to be sure, do the
/sys/class/gpio/gpio496 /sys/class/gpio/gpio497directories show up as well?
Today I retest it and it seem everything is ok. May be previous test I have some loading driver problem. The result is as below:
echo 496 > /sys/class/gpio/export
-bash: echo: write error: Invalid argument
echo 497 > /sys/class/gpio/export
-bash: echo: write error: Invalid argument
tung@ubuntu:~$ /sys/class/gpio/gpio496 bash: /sys/class/gpio/gpio496: No such file or directory
Quoted 5 lines
I just did a quick test using the code in the usb-next (testing) branch and it seems to work as expected when hard-coding alternate functions for some of >the pins in the driver.
I did not actually reprogram the device so perhaps something is broken with regards to reading the port configuration. But then you should see the >problem if you apply the patch to an earlier kernel as well.
Can you apply the below patch on top of Greg's usb-next (or usb-testing) branch and send me the logs from when connecting the device?
The log written is:
[ 3599.781531] cp210x 2-2.1:1.0: cp2108_gpio_init - 03 00 00 00
Anyway both drivers are now working ok.
Thank you.
Johan Hovold
maintainerQuoted 7 lines · Tung Pham
Quoted 4 lines
Are you sure you're using the latest mainline kernel here, that is, 5.13 here? With recent kernel's you would not see
-ENODEV("No such device") but rather-EINVAL("Invalid argument"). What does "uname -a" say?I use the kernel 5.8.0-59-generic, I think either result ("No such device") or ("Invalid argument") may be accepted in this case.
Yes, you should be able to apply this patch to older kernels without many additional changes, but you should still mention which kernel you're using since backporting may not always work as expected.
Quoted 4 lines
Quoted 2 lines
And which version of the patch are you applying? Are you sure it's the latest?
I apply patch V13 from last mail that you send.
Good.
Quoted 17 lines
Quoted 7 lines
Quoted 6 lines
But when I test the branch of Mr Gregkh.
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git/tree/dr
ivers/usb/serial/cp210x.c?h=usb-testingroot@ubuntu:/sys/class/gpio# echo 496 > /sys/class/gpio/export root@ubuntu:/sys/class/gpio# echo 497 > /sys/class/gpio/export That mean it still use GPIO 0 and GPIO 1 as GPIO functions.Quoted 6 lines
It does seem that way, but to be sure, do the
/sys/class/gpio/gpio496 /sys/class/gpio/gpio497directories show up as well?
Today I retest it and it seem everything is ok. May be previous test I have some loading driver problem. The result is as below:
Quoted 6 lines
echo 496 >
/sys/class/gpio/export
-bash: echo: write error: Invalid argument
echo 497 >/sys/class/gpio/export
-bash: echo: write error: Invalid argumenttung@ubuntu:~$ /sys/class/gpio/gpio496 bash: /sys/class/gpio/gpio496: No such file or directory
Very odd.
Were you using the very same device in both tests? And did you try with the older kernel before the new one? Perhaps the new settings had not yet taken effect otherwise?
Quoted 9 lines
Quoted 9 lines
I just did a quick test using the code in the usb-next (testing) branch and it seems to work as expected when hard-coding alternate functions for some of >the pins in the driver.
I did not actually reprogram the device so perhaps something is broken with regards to reading the port configuration. But then you should see the >problem if you apply the patch to an earlier kernel as well.
Quoted 6 lines
Quoted 3 lines
Can you apply the below patch on top of Greg's usb-next (or usb-testing) branch and send me the logs from when connecting the device?
The log written is:
[ 3599.781531] cp210x 2-2.1:1.0: cp2108_gpio_init - 03 00 00 00
Thanks for confirming. So everything appears to work as expected, and you should have seen the expected -EINVAL unless the device actually did return all-zero here for some reason.
Johan
Dear Johan.
Quoted 1 line
Yes, you should be able to apply this patch to older kernels without many additional changes, but you should still mention which kernel you're using >since backporting may not always work as expected.
Yes. Thank you for remaining me.
Quoted 3 lines
Good.
Very odd.
Were you using the very same device in both tests? And did you try with the older kernel before the new one? Perhaps the new settings had not yet >taken effect otherwise?
Both test I use same device and same kernel version 5.8.0-59-generic.
May be in the wrong test, the device that have been configuring and the driver may not load new setting from device, I think the wrong may come from device. Just some plug and unplug problem.
Quoted 4 lines
Quoted 2 lines
The log written is:
[ 3599.781531] cp210x 2-2.1:1.0: cp2108_gpio_init - 03 00 00 00Thanks for confirming. So everything appears to work as expected, and you should have seen the expected
-EINVALunless the device actually did return >all-zero here for some reason.
Yes, now it return -EINVAL ("Invalid argument"). As expected.
Thank you.