* [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 9:30 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 9:30 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
moves to the get_serial_info() function. Some magic numbers moves to
#define directives.
Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
---
drivers/usb/serial/ark3116.c | 54 ++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 23d46ef87..f45f69e18 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -36,13 +36,19 @@
#define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
#define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
#define DRIVER_NAME "ark3116"
+#define ARK3116_BAUDRATE 460800
+
+#define RS232_VENDOR 0x6547
+#define RS232_PRODUCT 0x0232
+#define IRDA_VENDOR 0x18ec
+#define IRDA_PRODUCT 0x3118
/* usb timeout of 1 second */
#define ARK_TIMEOUT 1000
static const struct usb_device_id id_table[] = {
- { USB_DEVICE(0x6547, 0x0232) },
- { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
+ { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
+ { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
{ },
};
MODULE_DEVICE_TABLE(usb, id_table);
@@ -50,8 +56,8 @@ MODULE_DEVICE_TABLE(usb, id_table);
static int is_irda(struct usb_serial *serial)
{
struct usb_device *dev = serial->dev;
- if (le16_to_cpu(dev->descriptor.idVendor) == 0x18ec &&
- le16_to_cpu(dev->descriptor.idProduct) == 0x3118)
+ if (le16_to_cpu(dev->descriptor.idVendor) == IRDA_VENDOR &&
+ le16_to_cpu(dev->descriptor.idProduct) == IRDA_PRODUCT)
return 1;
return 0;
}
@@ -397,31 +403,35 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
return result;
}
+static int ark3116_get_serial_info(struct usb_serial_port *port,
+ unsigned long arg)
+{
+ struct serial_struct ser;
+
+ memset(&ser, 0, sizeof(ser));
+
+ ser.type = PORT_16654;
+ ser.line = port->minor;
+ ser.port = port->port_number;
+ ser.custom_divisor = 0;
+ ser.baud_base = ARK3116_BAUDRATE;
+
+ if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int ark3116_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
{
struct usb_serial_port *port = tty->driver_data;
- struct serial_struct serstruct;
- void __user *user_arg = (void __user *)arg;
switch (cmd) {
case TIOCGSERIAL:
- /* XXX: Some of these values are probably wrong. */
- memset(&serstruct, 0, sizeof(serstruct));
- serstruct.type = PORT_16654;
- serstruct.line = port->minor;
- serstruct.port = port->port_number;
- serstruct.custom_divisor = 0;
- serstruct.baud_base = 460800;
-
- if (copy_to_user(user_arg, &serstruct, sizeof(serstruct)))
- return -EFAULT;
-
- return 0;
- case TIOCSSERIAL:
- if (copy_from_user(&serstruct, user_arg, sizeof(serstruct)))
- return -EFAULT;
- return 0;
+ return ark3116_get_serial_info(port, arg);
+ default:
+ break;
}
return -ENOIOCTLCMD;
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 9:30 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 9:30 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
moves to the get_serial_info() function. Some magic numbers moves to
#define directives.
Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
---
drivers/usb/serial/ark3116.c | 54 ++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 23d46ef87..f45f69e18 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -36,13 +36,19 @@
#define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
#define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
#define DRIVER_NAME "ark3116"
+#define ARK3116_BAUDRATE 460800
+
+#define RS232_VENDOR 0x6547
+#define RS232_PRODUCT 0x0232
+#define IRDA_VENDOR 0x18ec
+#define IRDA_PRODUCT 0x3118
/* usb timeout of 1 second */
#define ARK_TIMEOUT 1000
static const struct usb_device_id id_table[] = {
- { USB_DEVICE(0x6547, 0x0232) },
- { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
+ { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
+ { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
{ },
};
MODULE_DEVICE_TABLE(usb, id_table);
@@ -50,8 +56,8 @@ MODULE_DEVICE_TABLE(usb, id_table);
static int is_irda(struct usb_serial *serial)
{
struct usb_device *dev = serial->dev;
- if (le16_to_cpu(dev->descriptor.idVendor) == 0x18ec &&
- le16_to_cpu(dev->descriptor.idProduct) == 0x3118)
+ if (le16_to_cpu(dev->descriptor.idVendor) == IRDA_VENDOR &&
+ le16_to_cpu(dev->descriptor.idProduct) == IRDA_PRODUCT)
return 1;
return 0;
}
@@ -397,31 +403,35 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
return result;
}
+static int ark3116_get_serial_info(struct usb_serial_port *port,
+ unsigned long arg)
+{
+ struct serial_struct ser;
+
+ memset(&ser, 0, sizeof(ser));
+
+ ser.type = PORT_16654;
+ ser.line = port->minor;
+ ser.port = port->port_number;
+ ser.custom_divisor = 0;
+ ser.baud_base = ARK3116_BAUDRATE;
+
+ if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int ark3116_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
{
struct usb_serial_port *port = tty->driver_data;
- struct serial_struct serstruct;
- void __user *user_arg = (void __user *)arg;
switch (cmd) {
case TIOCGSERIAL:
- /* XXX: Some of these values are probably wrong. */
- memset(&serstruct, 0, sizeof(serstruct));
- serstruct.type = PORT_16654;
- serstruct.line = port->minor;
- serstruct.port = port->port_number;
- serstruct.custom_divisor = 0;
- serstruct.baud_base = 460800;
-
- if (copy_to_user(user_arg, &serstruct, sizeof(serstruct)))
- return -EFAULT;
-
- return 0;
- case TIOCSSERIAL:
- if (copy_from_user(&serstruct, user_arg, sizeof(serstruct)))
- return -EFAULT;
- return 0;
+ return ark3116_get_serial_info(port, arg);
+ default:
+ break;
}
return -ENOIOCTLCMD;
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 10:17 ` Oliver Neukum
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Neukum @ 2017-12-13 10:17 UTC (permalink / raw)
To: Mikhail Zaytsev, Johan Hovold; +Cc: Greg Kroah-Hartman, linux-kernel, linux-usb
Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> +#define RS232_VENDOR 0x6547
> +#define RS232_PRODUCT 0x0232
> +#define IRDA_VENDOR 0x18ec
> +#define IRDA_PRODUCT 0x3118
>
> /* usb timeout of 1 second */
> #define ARK_TIMEOUT 1000
>
> static const struct usb_device_id id_table[] = {
> - { USB_DEVICE(0x6547, 0x0232) },
> - { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> + { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> + { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
Hi,
what is the purpose of this change? It just makes it harder to grep.
The constants are arbitrary and they are clearly device IDs.
Regards
Oliver
^ permalink raw reply [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 10:17 ` Oliver Neukum
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Neukum @ 2017-12-13 10:17 UTC (permalink / raw)
To: Mikhail Zaytsev, Johan Hovold; +Cc: Greg Kroah-Hartman, linux-kernel, linux-usb
Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> +#define RS232_VENDOR 0x6547
> +#define RS232_PRODUCT 0x0232
> +#define IRDA_VENDOR 0x18ec
> +#define IRDA_PRODUCT 0x3118
>
> /* usb timeout of 1 second */
> #define ARK_TIMEOUT 1000
>
> static const struct usb_device_id id_table[] = {
> - { USB_DEVICE(0x6547, 0x0232) },
> - { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> + { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> + { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
Hi,
what is the purpose of this change? It just makes it harder to grep.
The constants are arbitrary and they are clearly device IDs.
Regards
Oliver
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 11:24 ` Johan Hovold
0 siblings, 0 replies; 27+ messages in thread
From: Johan Hovold @ 2017-12-13 11:24 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel
On Wed, Dec 13, 2017 at 12:30:04PM +0300, Mikhail Zaytsev wrote:
> The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
> moves to the get_serial_info() function. Some magic numbers moves to
> #define directives.
You need to split logical changes up in separate patches, and there's at
least three things being done here.
Thanks,
Johan
^ permalink raw reply [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 11:24 ` Johan Hovold
0 siblings, 0 replies; 27+ messages in thread
From: Johan Hovold @ 2017-12-13 11:24 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel
On Wed, Dec 13, 2017 at 12:30:04PM +0300, Mikhail Zaytsev wrote:
> The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
> moves to the get_serial_info() function. Some magic numbers moves to
> #define directives.
You need to split logical changes up in separate patches, and there's at
least three things being done here.
Thanks,
Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 11:31 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 11:31 UTC (permalink / raw)
To: Oliver Neukum; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-kernel, linux-usb
On Wed, 13 Dec 2017 11:17:28 +0100 Oliver Neukum <oneukum@suse.com> wrote:
> Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> > +#define RS232_VENDOR 0x6547
> > +#define RS232_PRODUCT 0x0232
> > +#define IRDA_VENDOR 0x18ec
> > +#define IRDA_PRODUCT 0x3118
> >
> > /* usb timeout of 1 second */
> > #define ARK_TIMEOUT 1000
> >
> > static const struct usb_device_id id_table[] = {
> > - { USB_DEVICE(0x6547, 0x0232) },
> > - { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> > + { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> > + { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
>
> Hi,
>
> what is the purpose of this change? It just makes it harder to grep.
> The constants are arbitrary and they are clearly device IDs.
The constants are using in several places.
I think the names easier to read.
--
Mikhail
^ permalink raw reply [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 11:31 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 11:31 UTC (permalink / raw)
To: Oliver Neukum; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-kernel, linux-usb
On Wed, 13 Dec 2017 11:17:28 +0100 Oliver Neukum <oneukum@suse.com> wrote:
> Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> > +#define RS232_VENDOR 0x6547
> > +#define RS232_PRODUCT 0x0232
> > +#define IRDA_VENDOR 0x18ec
> > +#define IRDA_PRODUCT 0x3118
> >
> > /* usb timeout of 1 second */
> > #define ARK_TIMEOUT 1000
> >
> > static const struct usb_device_id id_table[] = {
> > - { USB_DEVICE(0x6547, 0x0232) },
> > - { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> > + { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> > + { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
>
> Hi,
>
> what is the purpose of this change? It just makes it harder to grep.
> The constants are arbitrary and they are clearly device IDs.
The constants are using in several places.
I think the names easier to read.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 11:40 ` Oliver Neukum
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Neukum @ 2017-12-13 11:40 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-kernel, linux-usb
Am Mittwoch, den 13.12.2017, 14:31 +0300 schrieb Mikhail Zaytsev:
> On Wed, 13 Dec 2017 11:17:28 +0100 Oliver Neukum <oneukum@suse.com> wrote:
>
> >
> > Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> > >
> > > +#define RS232_VENDOR 0x6547
> > > +#define RS232_PRODUCT 0x0232
> > > +#define IRDA_VENDOR 0x18ec
> > > +#define IRDA_PRODUCT 0x3118
> > >
> > > /* usb timeout of 1 second */
> > > #define ARK_TIMEOUT 1000
> > >
> > > static const struct usb_device_id id_table[] = {
> > > - { USB_DEVICE(0x6547, 0x0232) },
> > > - { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> > > + { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> > > + { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
> >
> > Hi,
> >
> > what is the purpose of this change? It just makes it harder to grep.
> > The constants are arbitrary and they are clearly device IDs.
>
> The constants are using in several places.
> I think the names easier to read.
They give you nothing. If you are looking at a vendor ID nothing but the
bare number makes sense. You are just making peoples' life harder when
they have to look up that definition. A symbolic name is fine if it gives
meaning. Even if the information you give is that the value is magic
and therefore not understood. But a vendor ID is an arbitrary yet
meaningful number. There is no point in hiding it.
Regards
Oliver
^ permalink raw reply [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 11:40 ` Oliver Neukum
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Neukum @ 2017-12-13 11:40 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-kernel, linux-usb
Am Mittwoch, den 13.12.2017, 14:31 +0300 schrieb Mikhail Zaytsev:
> On Wed, 13 Dec 2017 11:17:28 +0100 Oliver Neukum <oneukum@suse.com> wrote:
>
> >
> > Am Mittwoch, den 13.12.2017, 12:30 +0300 schrieb Mikhail Zaytsev:
> > >
> > > +#define RS232_VENDOR 0x6547
> > > +#define RS232_PRODUCT 0x0232
> > > +#define IRDA_VENDOR 0x18ec
> > > +#define IRDA_PRODUCT 0x3118
> > >
> > > /* usb timeout of 1 second */
> > > #define ARK_TIMEOUT 1000
> > >
> > > static const struct usb_device_id id_table[] = {
> > > - { USB_DEVICE(0x6547, 0x0232) },
> > > - { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
> > > + { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
> > > + { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
> >
> > Hi,
> >
> > what is the purpose of this change? It just makes it harder to grep.
> > The constants are arbitrary and they are clearly device IDs.
>
> The constants are using in several places.
> I think the names easier to read.
They give you nothing. If you are looking at a vendor ID nothing but the
bare number makes sense. You are just making peoples' life harder when
they have to look up that definition. A symbolic name is fine if it gives
meaning. Even if the information you give is that the value is magic
and therefore not understood. But a vendor ID is an arbitrary yet
meaningful number. There is no point in hiding it.
Regards
Oliver
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 12:30 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 12:30 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-kernel, linux-usb
On Wed, 13 Dec 2017 12:40:48 +0100 Oliver Neukum <oneukum@suse.com> wrote:
> They give you nothing. If you are looking at a vendor ID nothing but the
> bare number makes sense. You are just making peoples' life harder when
> they have to look up that definition. A symbolic name is fine if it gives
> meaning. Even if the information you give is that the value is magic
> and therefore not understood. But a vendor ID is an arbitrary yet
> meaningful number. There is no point in hiding it.
Thanks. I hear you, Oliver. What about:
- serstruct.baud_base = 460800;
Is it a magic number? I think yes.
--
Mikhail
^ permalink raw reply [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 12:30 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 12:30 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-kernel, linux-usb
On Wed, 13 Dec 2017 12:40:48 +0100 Oliver Neukum <oneukum@suse.com> wrote:
> They give you nothing. If you are looking at a vendor ID nothing but the
> bare number makes sense. You are just making peoples' life harder when
> they have to look up that definition. A symbolic name is fine if it gives
> meaning. Even if the information you give is that the value is magic
> and therefore not understood. But a vendor ID is an arbitrary yet
> meaningful number. There is no point in hiding it.
Thanks. I hear you, Oliver. What about:
- serstruct.baud_base = 460800;
Is it a magic number? I think yes.
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 0/2] USB: serial: ark3116.c: ioctl changes
@ 2017-12-13 13:44 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 13:44 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
moves to the get_serial_info() function. Some magic numbers moves to
#define directives.
Mikhail Zaytsev (2):
USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case.
USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function.
drivers/usb/serial/ark3116.c | 41 +++++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 18 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 13:44 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 13:44 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
moves to the get_serial_info() function. Some magic numbers moves to
#define directives.
Mikhail Zaytsev (2):
USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case.
USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function.
drivers/usb/serial/ark3116.c | 41 +++++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 18 deletions(-)
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 1/2] USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case.
@ 2017-12-13 13:44 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 13:44 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
The patch removes unused TIOCSSERIAL ioctl case and adds the default block
to the switch.
Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
---
drivers/usb/serial/ark3116.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 23d46ef87..2e957c76f 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -418,10 +418,8 @@ static int ark3116_ioctl(struct tty_struct *tty,
return -EFAULT;
return 0;
- case TIOCSSERIAL:
- if (copy_from_user(&serstruct, user_arg, sizeof(serstruct)))
- return -EFAULT;
- return 0;
+ default:
+ break;
}
return -ENOIOCTLCMD;
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [1/2] USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case.
@ 2017-12-13 13:44 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 13:44 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
The patch removes unused TIOCSSERIAL ioctl case and adds the default block
to the switch.
Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
---
drivers/usb/serial/ark3116.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 23d46ef87..2e957c76f 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -418,10 +418,8 @@ static int ark3116_ioctl(struct tty_struct *tty,
return -EFAULT;
return 0;
- case TIOCSSERIAL:
- if (copy_from_user(&serstruct, user_arg, sizeof(serstruct)))
- return -EFAULT;
- return 0;
+ default:
+ break;
}
return -ENOIOCTLCMD;
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH 2/2] USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function.
@ 2017-12-13 13:45 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 13:45 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
The patch moves TIOCGSERIAL ioctl case to get_serial_info function.
Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
---
drivers/usb/serial/ark3116.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 2e957c76f..2ce8fe10e 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -36,6 +36,7 @@
#define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
#define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
#define DRIVER_NAME "ark3116"
+#define ARK3116_BAUDRATE 460800
/* usb timeout of 1 second */
#define ARK_TIMEOUT 1000
@@ -397,27 +398,33 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
return result;
}
+static int ark3116_get_serial_info(struct usb_serial_port *port,
+ unsigned long arg)
+{
+ struct serial_struct ser;
+
+ memset(&ser, 0, sizeof(ser));
+
+ ser.type = PORT_16654;
+ ser.line = port->minor;
+ ser.port = port->port_number;
+ ser.custom_divisor = 0;
+ ser.baud_base = ARK3116_BAUDRATE;
+
+ if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int ark3116_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
{
struct usb_serial_port *port = tty->driver_data;
- struct serial_struct serstruct;
- void __user *user_arg = (void __user *)arg;
switch (cmd) {
case TIOCGSERIAL:
- /* XXX: Some of these values are probably wrong. */
- memset(&serstruct, 0, sizeof(serstruct));
- serstruct.type = PORT_16654;
- serstruct.line = port->minor;
- serstruct.port = port->port_number;
- serstruct.custom_divisor = 0;
- serstruct.baud_base = 460800;
-
- if (copy_to_user(user_arg, &serstruct, sizeof(serstruct)))
- return -EFAULT;
-
- return 0;
+ return ark3116_get_serial_info(port, arg);
default:
break;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [2/2] USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function.
@ 2017-12-13 13:45 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 13:45 UTC (permalink / raw)
To: Johan Hovold; +Cc: Greg Kroah-Hartman, linux-usb, linux-kernel
The patch moves TIOCGSERIAL ioctl case to get_serial_info function.
Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
---
drivers/usb/serial/ark3116.c | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 2e957c76f..2ce8fe10e 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -36,6 +36,7 @@
#define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
#define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
#define DRIVER_NAME "ark3116"
+#define ARK3116_BAUDRATE 460800
/* usb timeout of 1 second */
#define ARK_TIMEOUT 1000
@@ -397,27 +398,33 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
return result;
}
+static int ark3116_get_serial_info(struct usb_serial_port *port,
+ unsigned long arg)
+{
+ struct serial_struct ser;
+
+ memset(&ser, 0, sizeof(ser));
+
+ ser.type = PORT_16654;
+ ser.line = port->minor;
+ ser.port = port->port_number;
+ ser.custom_divisor = 0;
+ ser.baud_base = ARK3116_BAUDRATE;
+
+ if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int ark3116_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
{
struct usb_serial_port *port = tty->driver_data;
- struct serial_struct serstruct;
- void __user *user_arg = (void __user *)arg;
switch (cmd) {
case TIOCGSERIAL:
- /* XXX: Some of these values are probably wrong. */
- memset(&serstruct, 0, sizeof(serstruct));
- serstruct.type = PORT_16654;
- serstruct.line = port->minor;
- serstruct.port = port->port_number;
- serstruct.custom_divisor = 0;
- serstruct.baud_base = 460800;
-
- if (copy_to_user(user_arg, &serstruct, sizeof(serstruct)))
- return -EFAULT;
-
- return 0;
+ return ark3116_get_serial_info(port, arg);
default:
break;
}
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 14:39 ` Oliver Neukum
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Neukum @ 2017-12-13 14:39 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: linux-kernel, linux-usb
Am Mittwoch, den 13.12.2017, 15:30 +0300 schrieb Mikhail Zaytsev:
> On Wed, 13 Dec 2017 12:40:48 +0100 Oliver Neukum <oneukum@suse.com> wrote:
>
> >
> > They give you nothing. If you are looking at a vendor ID nothing but the
> > bare number makes sense. You are just making peoples' life harder when
> > they have to look up that definition. A symbolic name is fine if it gives
> > meaning. Even if the information you give is that the value is magic
> > and therefore not understood. But a vendor ID is an arbitrary yet
> > meaningful number. There is no point in hiding it.
>
> Thanks. I hear you, Oliver. What about:
>
> - serstruct.baud_base = 460800;
>
> Is it a magic number? I think yes.
>
Hi,
yes sure. That is a candidate for a symbolic name. Though if you use
it once, I see no benefit, but it does not hurt either. The member
is named and that is the important thing.
A line like
if (rate > 38400) return -EINVAL;
is not so good
if (rate > MAX_BAUD) return -EINVAL;
better
But:
device->maxbaudrate = 38400
is better than
device->maxbaudrate = MAX_BAUD
You see the point?
Regards
Oliver
^ permalink raw reply [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 14:39 ` Oliver Neukum
0 siblings, 0 replies; 27+ messages in thread
From: Oliver Neukum @ 2017-12-13 14:39 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: linux-kernel, linux-usb
Am Mittwoch, den 13.12.2017, 15:30 +0300 schrieb Mikhail Zaytsev:
> On Wed, 13 Dec 2017 12:40:48 +0100 Oliver Neukum <oneukum@suse.com> wrote:
>
> >
> > They give you nothing. If you are looking at a vendor ID nothing but the
> > bare number makes sense. You are just making peoples' life harder when
> > they have to look up that definition. A symbolic name is fine if it gives
> > meaning. Even if the information you give is that the value is magic
> > and therefore not understood. But a vendor ID is an arbitrary yet
> > meaningful number. There is no point in hiding it.
>
> Thanks. I hear you, Oliver. What about:
>
> - serstruct.baud_base = 460800;
>
> Is it a magic number? I think yes.
>
Hi,
yes sure. That is a candidate for a symbolic name. Though if you use
it once, I see no benefit, but it does not hurt either. The member
is named and that is the important thing.
A line like
if (rate > 38400) return -EINVAL;
is not so good
if (rate > MAX_BAUD) return -EINVAL;
better
But:
device->maxbaudrate = 38400
is better than
device->maxbaudrate = MAX_BAUD
You see the point?
Regards
Oliver
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 16:43 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 16:43 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-kernel, linux-usb
On Wed, 13 Dec 2017 15:39:21 +0100 Oliver Neukum <oneukum@suse.com> wrote:
> But:
>
> device->maxbaudrate = 38400
>
> is better than
>
> device->maxbaudrate = MAX_BAUD
>
> You see the point?
Yes, I see. This is better, because it's more important to know =<how many>, but not =<what>.
thank you Oliver.
---
Mikhail
^ permalink raw reply [flat|nested] 27+ messages in thread
* USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 16:43 ` Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 16:43 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-kernel, linux-usb
On Wed, 13 Dec 2017 15:39:21 +0100 Oliver Neukum <oneukum@suse.com> wrote:
> But:
>
> device->maxbaudrate = 38400
>
> is better than
>
> device->maxbaudrate = MAX_BAUD
>
> You see the point?
Yes, I see. This is better, because it's more important to know =<how many>, but not =<what>.
thank you Oliver.
---
Mikhail
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/2] USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case.
@ 2018-01-02 14:50 ` Johan Hovold
0 siblings, 0 replies; 27+ messages in thread
From: Johan Hovold @ 2018-01-02 14:50 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel
On Wed, Dec 13, 2017 at 04:44:55PM +0300, Mikhail Zaytsev wrote:
> The patch removes unused TIOCSSERIAL ioctl case and adds the default block
> to the switch.
>
> Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
> ---
> drivers/usb/serial/ark3116.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
> index 23d46ef87..2e957c76f 100644
> --- a/drivers/usb/serial/ark3116.c
> +++ b/drivers/usb/serial/ark3116.c
> @@ -418,10 +418,8 @@ static int ark3116_ioctl(struct tty_struct *tty,
> return -EFAULT;
>
> return 0;
> - case TIOCSSERIAL:
> - if (copy_from_user(&serstruct, user_arg, sizeof(serstruct)))
> - return -EFAULT;
> - return 0;
> + default:
> + break;
> }
>
> return -ENOIOCTLCMD;
This will make the ioctl return -ENOTTY to user space (e.g. setserial),
which I guess should be fine as TIOCSSERIAL really isn't supported for
these devices currently.
But you should at least mention this changed behaviour in the commit
message.
Please also drop the ".c" part from the subject prefix while at it.
Thanks,
Johan
^ permalink raw reply [flat|nested] 27+ messages in thread
* [1/2] USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case.
@ 2018-01-02 14:50 ` Johan Hovold
0 siblings, 0 replies; 27+ messages in thread
From: Johan Hovold @ 2018-01-02 14:50 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel
On Wed, Dec 13, 2017 at 04:44:55PM +0300, Mikhail Zaytsev wrote:
> The patch removes unused TIOCSSERIAL ioctl case and adds the default block
> to the switch.
>
> Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
> ---
> drivers/usb/serial/ark3116.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
> index 23d46ef87..2e957c76f 100644
> --- a/drivers/usb/serial/ark3116.c
> +++ b/drivers/usb/serial/ark3116.c
> @@ -418,10 +418,8 @@ static int ark3116_ioctl(struct tty_struct *tty,
> return -EFAULT;
>
> return 0;
> - case TIOCSSERIAL:
> - if (copy_from_user(&serstruct, user_arg, sizeof(serstruct)))
> - return -EFAULT;
> - return 0;
> + default:
> + break;
> }
>
> return -ENOIOCTLCMD;
This will make the ioctl return -ENOTTY to user space (e.g. setserial),
which I guess should be fine as TIOCSSERIAL really isn't supported for
these devices currently.
But you should at least mention this changed behaviour in the commit
message.
Please also drop the ".c" part from the subject prefix while at it.
Thanks,
Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/2] USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function.
@ 2018-01-02 15:00 ` Johan Hovold
0 siblings, 0 replies; 27+ messages in thread
From: Johan Hovold @ 2018-01-02 15:00 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel
On Wed, Dec 13, 2017 at 04:45:06PM +0300, Mikhail Zaytsev wrote:
> The patch moves TIOCGSERIAL ioctl case to get_serial_info function.
>
> Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
> ---
> drivers/usb/serial/ark3116.c | 35 +++++++++++++++++++++--------------
> 1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
> index 2e957c76f..2ce8fe10e 100644
> --- a/drivers/usb/serial/ark3116.c
> +++ b/drivers/usb/serial/ark3116.c
> @@ -36,6 +36,7 @@
> #define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
> #define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
> #define DRIVER_NAME "ark3116"
> +#define ARK3116_BAUDRATE 460800
I thought you and Oliver agreed this wasn't needed (wanted). Just
hard-code this number in the new helper as before.
> /* usb timeout of 1 second */
> #define ARK_TIMEOUT 1000
> @@ -397,27 +398,33 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
> return result;
> }
>
> +static int ark3116_get_serial_info(struct usb_serial_port *port,
> + unsigned long arg)
Keep the (void __user *) cast in ark3116_ioctl below and pass a struct
serial_struct __user pointer here.
> +{
> + struct serial_struct ser;
> +
> + memset(&ser, 0, sizeof(ser));
> +
> + ser.type = PORT_16654;
> + ser.line = port->minor;
> + ser.port = port->port_number;
> + ser.custom_divisor = 0;
Might as well drop this redundant assignment as well.
> + ser.baud_base = ARK3116_BAUDRATE;
> +
> + if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> static int ark3116_ioctl(struct tty_struct *tty,
> unsigned int cmd, unsigned long arg)
> {
> struct usb_serial_port *port = tty->driver_data;
> - struct serial_struct serstruct;
> - void __user *user_arg = (void __user *)arg;
>
> switch (cmd) {
> case TIOCGSERIAL:
> - /* XXX: Some of these values are probably wrong. */
> - memset(&serstruct, 0, sizeof(serstruct));
> - serstruct.type = PORT_16654;
> - serstruct.line = port->minor;
> - serstruct.port = port->port_number;
> - serstruct.custom_divisor = 0;
> - serstruct.baud_base = 460800;
> -
> - if (copy_to_user(user_arg, &serstruct, sizeof(serstruct)))
> - return -EFAULT;
> -
> - return 0;
> + return ark3116_get_serial_info(port, arg);
> default:
> break;
> }
Thanks,
Johan
^ permalink raw reply [flat|nested] 27+ messages in thread
* [2/2] USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function.
@ 2018-01-02 15:00 ` Johan Hovold
0 siblings, 0 replies; 27+ messages in thread
From: Johan Hovold @ 2018-01-02 15:00 UTC (permalink / raw)
To: Mikhail Zaytsev; +Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, linux-kernel
On Wed, Dec 13, 2017 at 04:45:06PM +0300, Mikhail Zaytsev wrote:
> The patch moves TIOCGSERIAL ioctl case to get_serial_info function.
>
> Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
> ---
> drivers/usb/serial/ark3116.c | 35 +++++++++++++++++++++--------------
> 1 file changed, 21 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
> index 2e957c76f..2ce8fe10e 100644
> --- a/drivers/usb/serial/ark3116.c
> +++ b/drivers/usb/serial/ark3116.c
> @@ -36,6 +36,7 @@
> #define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
> #define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
> #define DRIVER_NAME "ark3116"
> +#define ARK3116_BAUDRATE 460800
I thought you and Oliver agreed this wasn't needed (wanted). Just
hard-code this number in the new helper as before.
> /* usb timeout of 1 second */
> #define ARK_TIMEOUT 1000
> @@ -397,27 +398,33 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
> return result;
> }
>
> +static int ark3116_get_serial_info(struct usb_serial_port *port,
> + unsigned long arg)
Keep the (void __user *) cast in ark3116_ioctl below and pass a struct
serial_struct __user pointer here.
> +{
> + struct serial_struct ser;
> +
> + memset(&ser, 0, sizeof(ser));
> +
> + ser.type = PORT_16654;
> + ser.line = port->minor;
> + ser.port = port->port_number;
> + ser.custom_divisor = 0;
Might as well drop this redundant assignment as well.
> + ser.baud_base = ARK3116_BAUDRATE;
> +
> + if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> static int ark3116_ioctl(struct tty_struct *tty,
> unsigned int cmd, unsigned long arg)
> {
> struct usb_serial_port *port = tty->driver_data;
> - struct serial_struct serstruct;
> - void __user *user_arg = (void __user *)arg;
>
> switch (cmd) {
> case TIOCGSERIAL:
> - /* XXX: Some of these values are probably wrong. */
> - memset(&serstruct, 0, sizeof(serstruct));
> - serstruct.type = PORT_16654;
> - serstruct.line = port->minor;
> - serstruct.port = port->port_number;
> - serstruct.custom_divisor = 0;
> - serstruct.baud_base = 460800;
> -
> - if (copy_to_user(user_arg, &serstruct, sizeof(serstruct)))
> - return -EFAULT;
> -
> - return 0;
> + return ark3116_get_serial_info(port, arg);
> default:
> break;
> }
Thanks,
Johan
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl
@ 2017-12-13 8:29 Mikhail Zaytsev
0 siblings, 0 replies; 27+ messages in thread
From: Mikhail Zaytsev @ 2017-12-13 8:29 UTC (permalink / raw)
To: Mikhail Zaytsev
The patch removes unused TIOCSSERIAL case from ioctl. TIOCGSERIAL case
moves to the get_serial_info() function. Any magic numbers moves to
#define directives.
Signed-off-by: Mikhail Zaytsev <flashed@mail.ru>
---
drivers/usb/serial/ark3116.c | 54 ++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/serial/ark3116.c b/drivers/usb/serial/ark3116.c
index 23d46ef87..f45f69e18 100644
--- a/drivers/usb/serial/ark3116.c
+++ b/drivers/usb/serial/ark3116.c
@@ -36,13 +36,19 @@
#define DRIVER_DESC "USB ARK3116 serial/IrDA driver"
#define DRIVER_DEV_DESC "ARK3116 RS232/IrDA"
#define DRIVER_NAME "ark3116"
+#define ARK3116_BAUDRATE 460800
+
+#define RS232_VENDOR 0x6547
+#define RS232_PRODUCT 0x0232
+#define IRDA_VENDOR 0x18ec
+#define IRDA_PRODUCT 0x3118
/* usb timeout of 1 second */
#define ARK_TIMEOUT 1000
static const struct usb_device_id id_table[] = {
- { USB_DEVICE(0x6547, 0x0232) },
- { USB_DEVICE(0x18ec, 0x3118) }, /* USB to IrDA adapter */
+ { USB_DEVICE(RS232_VENDOR, RS232_PRODUCT) },
+ { USB_DEVICE(IRDA_VENDOR, IRDA_PRODUCT) }, /* USB to IrDA adapter */
{ },
};
MODULE_DEVICE_TABLE(usb, id_table);
@@ -50,8 +56,8 @@ MODULE_DEVICE_TABLE(usb, id_table);
static int is_irda(struct usb_serial *serial)
{
struct usb_device *dev = serial->dev;
- if (le16_to_cpu(dev->descriptor.idVendor) == 0x18ec &&
- le16_to_cpu(dev->descriptor.idProduct) == 0x3118)
+ if (le16_to_cpu(dev->descriptor.idVendor) == IRDA_VENDOR &&
+ le16_to_cpu(dev->descriptor.idProduct) == IRDA_PRODUCT)
return 1;
return 0;
}
@@ -397,31 +403,35 @@ static int ark3116_open(struct tty_struct *tty, struct usb_serial_port *port)
return result;
}
+static int ark3116_get_serial_info(struct usb_serial_port *port,
+ unsigned long arg)
+{
+ struct serial_struct ser;
+
+ memset(&ser, 0, sizeof(ser));
+
+ ser.type = PORT_16654;
+ ser.line = port->minor;
+ ser.port = port->port_number;
+ ser.custom_divisor = 0;
+ ser.baud_base = ARK3116_BAUDRATE;
+
+ if (copy_to_user((void __user *)arg, &ser, sizeof(ser)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int ark3116_ioctl(struct tty_struct *tty,
unsigned int cmd, unsigned long arg)
{
struct usb_serial_port *port = tty->driver_data;
- struct serial_struct serstruct;
- void __user *user_arg = (void __user *)arg;
switch (cmd) {
case TIOCGSERIAL:
- /* XXX: Some of these values are probably wrong. */
- memset(&serstruct, 0, sizeof(serstruct));
- serstruct.type = PORT_16654;
- serstruct.line = port->minor;
- serstruct.port = port->port_number;
- serstruct.custom_divisor = 0;
- serstruct.baud_base = 460800;
-
- if (copy_to_user(user_arg, &serstruct, sizeof(serstruct)))
- return -EFAULT;
-
- return 0;
- case TIOCSSERIAL:
- if (copy_from_user(&serstruct, user_arg, sizeof(serstruct)))
- return -EFAULT;
- return 0;
+ return ark3116_get_serial_info(port, arg);
+ default:
+ break;
}
return -ENOIOCTLCMD;
--
2.11.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-01-02 15:00 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-13 9:30 [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl Mikhail Zaytsev
2017-12-13 9:30 ` Mikhail Zaytsev
2017-12-13 10:17 ` [PATCH] " Oliver Neukum
2017-12-13 10:17 ` Oliver Neukum
2017-12-13 11:31 ` [PATCH] " Mikhail Zaytsev
2017-12-13 11:31 ` Mikhail Zaytsev
2017-12-13 11:40 ` [PATCH] " Oliver Neukum
2017-12-13 11:40 ` Oliver Neukum
2017-12-13 12:30 ` [PATCH] " Mikhail Zaytsev
2017-12-13 12:30 ` Mikhail Zaytsev
2017-12-13 14:39 ` [PATCH] " Oliver Neukum
2017-12-13 14:39 ` Oliver Neukum
2017-12-13 16:43 ` [PATCH] " Mikhail Zaytsev
2017-12-13 16:43 ` Mikhail Zaytsev
2017-12-13 11:24 ` [PATCH] " Johan Hovold
2017-12-13 11:24 ` Johan Hovold
2017-12-13 13:44 ` [PATCH 0/2] USB: serial: ark3116.c: ioctl changes Mikhail Zaytsev
2017-12-13 13:44 ` USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl Mikhail Zaytsev
2017-12-13 13:44 ` [PATCH 1/2] USB: serial: ark3116.c: Remove unused TIOCSSERIAL ioctl case Mikhail Zaytsev
2017-12-13 13:44 ` [1/2] " Mikhail Zaytsev
2018-01-02 14:50 ` [PATCH 1/2] " Johan Hovold
2018-01-02 14:50 ` [1/2] " Johan Hovold
2017-12-13 13:45 ` [PATCH 2/2] USB: serial: ark3116.c: Move TIOCGSERIAL ioctl case to function Mikhail Zaytsev
2017-12-13 13:45 ` [2/2] " Mikhail Zaytsev
2018-01-02 15:00 ` [PATCH 2/2] " Johan Hovold
2018-01-02 15:00 ` [2/2] " Johan Hovold
-- strict thread matches above, loose matches on Subject: below --
2017-12-13 8:29 [PATCH] USB: serial: ark3116.c: Remove unused TIOCSSERIAL case from ioctl Mikhail Zaytsev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.