The F81232 will report data and LSR with bulk like following format:
bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]...
LSR will auto clear frame/parity/break error flag when reading by H/W,
but overrrun will only cleared when reading LSR. So this patch add a
worker to read LSR when OE.
Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
drivers/usb/serial/f81232.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 96036f87b1de..46836041c50e 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -41,12 +41,14 @@ MODULE_DEVICE_TABLE(usb, id_table);
#define FIFO_CONTROL_REGISTER (0x02 + SERIAL_BASE_ADDRESS)
#define LINE_CONTROL_REGISTER (0x03 + SERIAL_BASE_ADDRESS)
#define MODEM_CONTROL_REGISTER (0x04 + SERIAL_BASE_ADDRESS)
+#define LINE_STATUS_REGISTER (0x05 + SERIAL_BASE_ADDRESS)
#define MODEM_STATUS_REGISTER (0x06 + SERIAL_BASE_ADDRESS)
struct f81232_private {
struct mutex lock;
u8 modem_control;
u8 modem_status;
+ struct work_struct lsr_work;
struct work_struct interrupt_work;
struct usb_serial_port *port;
};
@@ -282,6 +284,7 @@ static void f81232_read_int_callback(struct urb *urb)
static void f81232_process_read_urb(struct urb *urb)
{
struct usb_serial_port *port = urb->context;
+ struct f81232_private *priv = usb_get_serial_port_data(port);
unsigned char *data = urb->transfer_buffer;
char tty_flag;
unsigned int i;
@@ -315,6 +318,7 @@ static void f81232_process_read_urb(struct urb *urb)
if (lsr & UART_LSR_OE) {
port->icount.overrun++;
+ schedule_work(&priv->lsr_work);
tty_insert_flip_char(&port->port, 0,
TTY_OVERRUN);
}
@@ -623,6 +627,21 @@ static void f81232_interrupt_work(struct work_struct *work)
f81232_read_msr(priv->port);
}
+static void f81232_lsr_worker(struct work_struct *work)
+{
+ struct f81232_private *priv;
+ struct usb_serial_port *port;
+ int status;
+ u8 tmp;
+
+ priv = container_of(work, struct f81232_private, lsr_work);
+ port = priv->port;
+
+ status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
+ if (status)
+ dev_warn(&port->dev, "read LSR failed: %d\n", status);
+}
+
static int f81232_port_probe(struct usb_serial_port *port)
{
struct f81232_private *priv;
@@ -633,6 +652,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
mutex_init(&priv->lock);
INIT_WORK(&priv->interrupt_work, f81232_interrupt_work);
+ INIT_WORK(&priv->lsr_work, f81232_lsr_worker);
usb_set_serial_port_data(port, priv);
--
2.7.4
Implement Fintek F81232 break on/off with LCR register.
It's the same with 16550A LCR register layout.
Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
drivers/usb/serial/f81232.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index dadf024ae494..a054f69446fd 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -377,13 +377,18 @@ static void f81232_process_read_urb(struct urb *urb)
static void f81232_break_ctl(struct tty_struct *tty, int break_state)
{
- /* FIXME - Stubbed out for now */
+ struct usb_serial_port *port = tty->driver_data;
+ struct f81232_private *priv = usb_get_serial_port_data(port);
+ int status;
- /*
- * break_state = -1 to turn on break, and 0 to turn off break
- * see drivers/char/tty_io.c to see it used.
- * last_set_data_urb_value NEVER has the break bit set in it.
- */
+ mutex_lock(&priv->lock);
+
+ status = f81232_set_mask_register(port, LINE_CONTROL_REGISTER,
+ UART_LCR_SBC, !!break_state);
+ if (status)
+ dev_err(&port->dev, "set break failed: %d\n", status);
+
+ mutex_unlock(&priv->lock);
}
static int f81232_find_clk(speed_t baudrate)
--
2.7.4
Fix Fintek F81232 bulk_in/out size to 64/16 according to the spec.
http://html.alldatasheet.com/html-pdf/406315/FINTEK/F81232/1762/8/F81232.html
Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
drivers/usb/serial/f81232.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index a054f69446fd..f3ee537d643c 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
},
.id_table = id_table,
.num_ports = 1,
- .bulk_in_size = 256,
- .bulk_out_size = 256,
+ .bulk_out_size = 16,
.open = f81232_open,
.close = f81232_close,
.dtr_rts = f81232_dtr_rts,
--
2.7.4
The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
can be up to 1.5Mbits with 24MHz.
F81232 Clock registers (106h)
Bit1-0: Clock source selector
00: 1.846MHz.
01: 18.46MHz.
10: 24MHz.
11: 14.77MHz.
Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
drivers/usb/serial/f81232.c | 105 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 94 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index 46836041c50e..bdd7f337cd5f 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -28,7 +28,8 @@ static const struct usb_device_id id_table[] = {
MODULE_DEVICE_TABLE(usb, id_table);
/* Maximum baudrate for F81232 */
-#define F81232_MAX_BAUDRATE 115200
+#define F81232_MAX_BAUDRATE 1500000
+#define F81232_DEF_BAUDRATE 9600
/* USB Control EP parameter */
#define F81232_REGISTER_REQUEST 0xa0
@@ -44,18 +45,42 @@ MODULE_DEVICE_TABLE(usb, id_table);
#define LINE_STATUS_REGISTER (0x05 + SERIAL_BASE_ADDRESS)
#define MODEM_STATUS_REGISTER (0x06 + SERIAL_BASE_ADDRESS)
+/*
+ * F81232 Clock registers (106h)
+ *
+ * Bit1-0: Clock source selector
+ * 00: 1.846MHz.
+ * 01: 18.46MHz.
+ * 10: 24MHz.
+ * 11: 14.77MHz.
+ */
+#define F81232_CLK_REGISTER 0x106
+#define F81232_CLK_1_846_MHZ 0
+#define F81232_CLK_18_46_MHZ BIT(0)
+#define F81232_CLK_24_MHZ BIT(1)
+#define F81232_CLK_14_77_MHZ (BIT(1) | BIT(0))
+#define F81232_CLK_MASK GENMASK(1, 0)
+
struct f81232_private {
struct mutex lock;
u8 modem_control;
u8 modem_status;
+ speed_t baud_base;
struct work_struct lsr_work;
struct work_struct interrupt_work;
struct usb_serial_port *port;
};
-static int calc_baud_divisor(speed_t baudrate)
+static u32 const baudrate_table[] = { 115200, 921600, 1152000, 1500000 };
+static u8 const clock_table[] = { F81232_CLK_1_846_MHZ, F81232_CLK_14_77_MHZ,
+ F81232_CLK_18_46_MHZ, F81232_CLK_24_MHZ };
+
+static int calc_baud_divisor(speed_t baudrate, speed_t clockrate)
{
- return DIV_ROUND_CLOSEST(F81232_MAX_BAUDRATE, baudrate);
+ if (!baudrate)
+ return 0;
+
+ return DIV_ROUND_CLOSEST(clockrate, baudrate);
}
static int f81232_get_register(struct usb_serial_port *port, u16 reg, u8 *val)
@@ -129,6 +154,21 @@ static int f81232_set_register(struct usb_serial_port *port, u16 reg, u8 val)
return status;
}
+static int f81232_set_mask_register(struct usb_serial_port *port, u16 reg,
+ u8 mask, u8 val)
+{
+ int status;
+ u8 tmp;
+
+ status = f81232_get_register(port, reg, &tmp);
+ if (status)
+ return status;
+
+ tmp = (tmp & ~mask) | (val & mask);
+
+ return f81232_set_register(port, reg, tmp);
+}
+
static void f81232_read_msr(struct usb_serial_port *port)
{
int status;
@@ -346,13 +386,53 @@ static void f81232_break_ctl(struct tty_struct *tty, int break_state)
*/
}
-static void f81232_set_baudrate(struct usb_serial_port *port, speed_t baudrate)
+static int f81232_find_clk(speed_t baudrate)
+{
+ int idx;
+
+ for (idx = 0; idx < ARRAY_SIZE(baudrate_table); ++idx) {
+ if (baudrate <= baudrate_table[idx] &&
+ baudrate_table[idx] % baudrate == 0)
+ return idx;
+ }
+
+ return -EINVAL;
+}
+
+static void f81232_set_baudrate(struct tty_struct *tty,
+ struct usb_serial_port *port, speed_t baudrate,
+ speed_t old_baudrate)
{
+ struct f81232_private *priv = usb_get_serial_port_data(port);
u8 lcr;
int divisor;
int status = 0;
+ int i;
+ int idx;
+ speed_t baud_list[] = {baudrate, old_baudrate, F81232_DEF_BAUDRATE};
+
+ for (i = 0; i < ARRAY_SIZE(baud_list); ++i) {
+ idx = f81232_find_clk(baud_list[i]);
+ if (idx >= 0) {
+ baudrate = baud_list[i];
+ tty_encode_baud_rate(tty, baudrate, baudrate);
+ break;
+ }
+ }
- divisor = calc_baud_divisor(baudrate);
+ if (idx < 0)
+ return;
+
+ priv->baud_base = baudrate_table[idx];
+ divisor = calc_baud_divisor(baudrate, priv->baud_base);
+
+ status = f81232_set_mask_register(port, F81232_CLK_REGISTER,
+ F81232_CLK_MASK, clock_table[idx]);
+ if (status) {
+ dev_err(&port->dev, "%s failed to set CLK_REG: %d\n",
+ __func__, status);
+ return;
+ }
status = f81232_get_register(port, LINE_CONTROL_REGISTER,
&lcr); /* get LCR */
@@ -442,6 +522,7 @@ static void f81232_set_termios(struct tty_struct *tty,
u8 new_lcr = 0;
int status = 0;
speed_t baudrate;
+ speed_t old_baud;
/* Don't change anything if nothing has changed */
if (old_termios && !tty_termios_hw_change(&tty->termios, old_termios))
@@ -454,11 +535,12 @@ static void f81232_set_termios(struct tty_struct *tty,
baudrate = tty_get_baud_rate(tty);
if (baudrate > 0) {
- if (baudrate > F81232_MAX_BAUDRATE) {
- baudrate = F81232_MAX_BAUDRATE;
- tty_encode_baud_rate(tty, baudrate, baudrate);
- }
- f81232_set_baudrate(port, baudrate);
+ if (old_termios)
+ old_baud = tty_termios_baud_rate(old_termios);
+ else
+ old_baud = F81232_DEF_BAUDRATE;
+
+ f81232_set_baudrate(tty, port, baudrate, old_baud);
}
if (C_PARENB(tty)) {
@@ -590,6 +672,7 @@ static int f81232_carrier_raised(struct usb_serial_port *port)
static int f81232_get_serial_info(struct usb_serial_port *port,
unsigned long arg)
{
+ struct f81232_private *priv = usb_get_serial_port_data(port);
struct serial_struct ser;
memset(&ser, 0, sizeof(ser));
@@ -597,7 +680,7 @@ static int f81232_get_serial_info(struct usb_serial_port *port,
ser.type = PORT_16550A;
ser.line = port->minor;
ser.port = port->port_number;
- ser.baud_base = F81232_MAX_BAUDRATE;
+ ser.baud_base = priv->baud_base;
if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
return -EFAULT;
--
2.7.4
The F81232 can do remote wakeup via RX/RI pin with pulse.
This patch will use device_set_wakeup_enable to enable this
feature.
Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
---
drivers/usb/serial/f81232.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
index bdd7f337cd5f..dadf024ae494 100644
--- a/drivers/usb/serial/f81232.c
+++ b/drivers/usb/serial/f81232.c
@@ -742,6 +742,7 @@ static int f81232_port_probe(struct usb_serial_port *port)
port->port.drain_delay = 256;
priv->port = port;
+ device_set_wakeup_enable(&port->serial->dev->dev, true);
return 0;
}
@@ -752,6 +753,7 @@ static int f81232_port_remove(struct usb_serial_port *port)
priv = usb_get_serial_port_data(port);
kfree(priv);
+ device_set_wakeup_enable(&port->serial->dev->dev, false);
return 0;
}
--
2.7.4
Am Montag, den 22.01.2018, 15:58 +0800 schrieb Ji-Ze Hong (Peter Hong)
:
> The F81232 will report data and LSR with bulk like following format:
> bulk-in data: [LSR(1Byte)+DATA(1Byte)][LSR(1Byte)+DATA(1Byte)]...
>
> LSR will auto clear frame/parity/break error flag when reading by H/W,
> but overrrun will only cleared when reading LSR. So this patch add a
> worker to read LSR when OE.
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
[..]
> +static void f81232_lsr_worker(struct work_struct *work)
> +{
> + struct f81232_private *priv;
> + struct usb_serial_port *port;
> + int status;
> + u8 tmp;
> +
> + priv = container_of(work, struct f81232_private, lsr_work);
> + port = priv->port;
> +
> + status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
> + if (status)
> + dev_warn(&port->dev, "read LSR failed: %d\n", status);
> +}
Hi,
I am afraid this is incomplete. You are scheduling a work that does IO.
Hence you must cancel that work when the driver is unbound from the
interface. You must also not do IO like this while the system is suspending.
Regards
Oliver
On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
<[email protected]> wrote:
> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
> can be up to 1.5Mbits with 24MHz.
>
> F81232 Clock registers (106h)
>
> Bit1-0: Clock source selector
> 00: 1.846MHz.
> 01: 18.46MHz.
> 10: 24MHz.
> 11: 14.77MHz.
Hmm... Why not to provide a proper clk driver (based on table variant
of clk-divider) and use it here?
--
With Best Regards,
Andy Shevchenko
Hi Oliver,
Oliver Neukum 於 2018/1/22 下午 06:06 寫道:
>> +static void f81232_lsr_worker(struct work_struct *work)
>> +{
>> + struct f81232_private *priv;
>> + struct usb_serial_port *port;
>> + int status;
>> + u8 tmp;
>> +
>> + priv = container_of(work, struct f81232_private, lsr_work);
>> + port = priv->port;
>> +
>> + status = f81232_get_register(port, LINE_STATUS_REGISTER, &tmp);
>> + if (status)
>> + dev_warn(&port->dev, "read LSR failed: %d\n", status);
>> +}
>
> Hi,
>
> I am afraid this is incomplete. You are scheduling a work that does IO.
> Hence you must cancel that work when the driver is unbound from the
> interface. You must also not do IO like this while the system is suspending.
We'll add cancel worker operation on close() and suspend() to prevent
from I/O operations in wrong time with next patch version.
Thanks
--
With Best Regards,
Peter Hong
Hi Andy,
Andy Shevchenko 於 2018/1/22 下午 10:55 寫道:
> On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
> <[email protected]> wrote:
>> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
>> can be up to 1.5Mbits with 24MHz.
>>
>> F81232 Clock registers (106h)
>>
>> Bit1-0: Clock source selector
>> 00: 1.846MHz.
>> 01: 18.46MHz.
>> 10: 24MHz.
>> 11: 14.77MHz.
>
> Hmm... Why not to provide a proper clk driver (based on table variant
> of clk-divider) and use it here?
>
>
It seems too complex to use clock framework in this driver.
What do you think about this, Johan ?
Thanks
--
With Best Regards,
Peter Hong
On Tue, Jan 23, 2018 at 10:08:24AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Andy,
>
> Andy Shevchenko 於 2018/1/22 下午 10:55 寫道:
> > On Mon, Jan 22, 2018 at 9:58 AM, Ji-Ze Hong (Peter Hong)
> > <[email protected]> wrote:
> >> The F81232 had 4 clocksource 1.846/18.46/14.77/24MHz and baud rates
> >> can be up to 1.5Mbits with 24MHz.
> >>
> >> F81232 Clock registers (106h)
> >>
> >> Bit1-0: Clock source selector
> >> 00: 1.846MHz.
> >> 01: 18.46MHz.
> >> 10: 24MHz.
> >> 11: 14.77MHz.
> >
> > Hmm... Why not to provide a proper clk driver (based on table variant
> > of clk-divider) and use it here?
>
> It seems too complex to use clock framework in this driver.
> What do you think about this, Johan ?
Yeah, you don't need to implement a clk driver for this. If anyone
thinks that would simplify things, I'd be happy to consider it as a
follow-on patch.
Thanks,
Johan
On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> The F81232 can do remote wakeup via RX/RI pin with pulse.
> This patch will use device_set_wakeup_enable to enable this
> feature.
This is a policy decision that should be made by user space by setting
the power/wakeup attribute, and not something that something that
drivers should enable directly themselves.
Perhaps you really wanted to use device_set_wakeup_capable()? But then
you also need to honour the current setting in suspend() as well.
How have you tested this feature?
Johan
On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Fix Fintek F81232 bulk_in/out size to 64/16 according to the spec.
> http://html.alldatasheet.com/html-pdf/406315/FINTEK/F81232/1762/8/F81232.html
>
> Signed-off-by: Ji-Ze Hong (Peter Hong) <[email protected]>
> ---
> drivers/usb/serial/f81232.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> index a054f69446fd..f3ee537d643c 100644
> --- a/drivers/usb/serial/f81232.c
> +++ b/drivers/usb/serial/f81232.c
> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
> },
> .id_table = id_table,
> .num_ports = 1,
> - .bulk_in_size = 256,
> - .bulk_out_size = 256,
> + .bulk_out_size = 16,
These fields control the URB buffer sizes and defaults to the corresponding
endpoint max-packet size, which would be 16 for all endpoints according
to the datasheet above.
So it seems you should really be setting bulk_in_size to 64 here (and
possibly leave bulk_out_size unset) as that would appear to match your
device buffer sizes.
Johan
Hi Johan,
Johan Hovold 於 2018/1/30 上午 11:57 寫道:
> On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> The F81232 can do remote wakeup via RX/RI pin with pulse.
>> This patch will use device_set_wakeup_enable to enable this
>> feature.
>
> This is a policy decision that should be made by user space by setting
> the power/wakeup attribute, and not something that something that
> drivers should enable directly themselves.
>
> Perhaps you really wanted to use device_set_wakeup_capable()? But then
> you also need to honour the current setting in suspend() as well.
>
> How have you tested this feature?
>
Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
other peripherals and we had tested it by following procedure with
device_set_wakeup_enable() enabled:
1. Using pm-suspend to S3
2. Trigger a pulse to RI/RX to wake up system.
In our test, we can do remote wakeup only with
device_set_wakeup_enable() enabled.
Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
like following link??
https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476
Thanks
--
With Best Regards,
Peter Hong
Hi Johan,
Johan Hovold 於 2018/1/30 下午 12:11 寫道:
> On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
>> index a054f69446fd..f3ee537d643c 100644
>> --- a/drivers/usb/serial/f81232.c
>> +++ b/drivers/usb/serial/f81232.c
>> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
>> },
>> .id_table = id_table,
>> .num_ports = 1,
>> - .bulk_in_size = 256,
>> - .bulk_out_size = 256,
>> + .bulk_out_size = 16,
>
> So it seems you should really be setting bulk_in_size to 64 here (and
> possibly leave bulk_out_size unset) as that would appear to match your
> device buffer sizes.
Yes, we want to set the bulk_in_size as 64. The public datasheet has
some error with bulk in/out, the correct size is 64.
We had test the bulk_out_size set the same with internal TX FIFO will
make the best performance in tests, but it's ok to set 64. In my opinion
, I'll prefer to set 16.
The following information is the F81232 dump by lsusb:
Bus 002 Device 007: ID 1934:0706 Feature Integration Technology Inc.
(Fintek)
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.10
bDeviceClass 0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 16
idVendor 0x1934 Feature Integration Technology Inc. (Fintek)
idProduct 0x0706
bcdDevice 0.01
iManufacturer 1 FINTEK
iProduct 2 USB TO UART BRIDGE
iSerial 3 88635600168801
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 39
bNumInterfaces 1
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 100mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 0 (Defined at Interface level)
bInterfaceSubClass 0
bInterfaceProtocol 0
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 10
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01 EP 1 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Device Status: 0x0000
(Bus Powered)
Thanks
--
With Best Regards,
Peter Hong
On Thu, Feb 01, 2018 at 11:13:01AM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
>
> Johan Hovold 於 2018/1/30 上午 11:57 寫道:
> > On Mon, Jan 22, 2018 at 03:58:45PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> The F81232 can do remote wakeup via RX/RI pin with pulse.
> >> This patch will use device_set_wakeup_enable to enable this
> >> feature.
> >
> > This is a policy decision that should be made by user space by setting
> > the power/wakeup attribute, and not something that something that
> > drivers should enable directly themselves.
> >
> > Perhaps you really wanted to use device_set_wakeup_capable()? But then
> > you also need to honour the current setting in suspend() as well.
> >
> > How have you tested this feature?
> >
>
> Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
> other peripherals and we had tested it by following procedure with
> device_set_wakeup_enable() enabled:
> 1. Using pm-suspend to S3
> 2. Trigger a pulse to RI/RX to wake up system.
>
> In our test, we can do remote wakeup only with
> device_set_wakeup_enable() enabled.
Yeah, but you need to enable it though sysfs. Not every device should be
able to wake the system up. That's a decision left for user space.
> Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
> like following link??
> https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476
No, your driver should not call device_set_wakeup_enable() itself. Just
set the wakeup capable flag in probe. And if you can disable the wake up
feature, this needs to be done at suspend depending on what user space
has requested.
Johan
On Thu, Feb 01, 2018 at 01:50:55PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Hi Johan,
>
> Johan Hovold 於 2018/1/30 下午 12:11 寫道:
> > On Mon, Jan 22, 2018 at 03:58:47PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> >> diff --git a/drivers/usb/serial/f81232.c b/drivers/usb/serial/f81232.c
> >> index a054f69446fd..f3ee537d643c 100644
> >> --- a/drivers/usb/serial/f81232.c
> >> +++ b/drivers/usb/serial/f81232.c
> >> @@ -769,8 +769,7 @@ static struct usb_serial_driver f81232_device = {
> >> },
> >> .id_table = id_table,
> >> .num_ports = 1,
> >> - .bulk_in_size = 256,
> >> - .bulk_out_size = 256,
> >> + .bulk_out_size = 16,
> >
> > So it seems you should really be setting bulk_in_size to 64 here (and
> > possibly leave bulk_out_size unset) as that would appear to match your
> > device buffer sizes.
>
> Yes, we want to set the bulk_in_size as 64. The public datasheet has
> some error with bulk in/out, the correct size is 64.
>
> We had test the bulk_out_size set the same with internal TX FIFO will
> make the best performance in tests, but it's ok to set 64. In my opinion
> , I'll prefer to set 16.
Having larger URB buffers than the endpoint size is typically more
efficient, but sometimes there are hardware issues that needs to be
worked around.
Johan
Hi Johan,
Johan Hovold 於 2018/2/4 上午 09:46 寫道:
> On Thu, Feb 01, 2018 at 11:13:01AM +0800, Ji-Ze Hong (Peter Hong) wrote:
>> Our USB-To-Serial support RI/ RX remote wakeup by Modem, Fax or
>> other peripherals and we had tested it by following procedure with
>> device_set_wakeup_enable() enabled:
>> 1. Using pm-suspend to S3
>> 2. Trigger a pulse to RI/RX to wake up system.
>>
>> In our test, we can do remote wakeup only with
>> device_set_wakeup_enable() enabled.
>
> Yeah, but you need to enable it though sysfs. Not every device should be
> able to wake the system up. That's a decision left for user space.
>
>> Should we add device_set_wakeup_capable() & device_set_wakeup_enable()
>> like following link??
>> https://elixir.free-electrons.com/linux/latest/source/drivers/media/rc/mceusb.c#L1476
>
> No, your driver should not call device_set_wakeup_enable() itself. Just
> set the wakeup capable flag in probe. And if you can disable the wake up
> feature, this needs to be done at suspend depending on what user space
> has requested.
We'll change to device_set_wakeup_capable() and send the v2 patch when
test OK.
Thanks
--
With Best Regards,
Peter Hong