Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
For the JSM driver, its up to you whether you feel its needed or not.
However, I would like to mention that the DIGI drivers that currently
reside in the kernel sources *do* reserve that ioctl space,
and is acknowledged by "Documentation/ioctl-number.txt":
d' F0-FF linux/digi1.h
I understand that the list is not a reservation list,
but a current list of potential ioctl conflicts...
It's not a reservation issue, it's the fact that we don't want to allow
new ioctls, and if we do, they had better work properly (your
implementation does not.)
Hi Greg,
We understood that you don't want more new ioctls in the driver.
For DPA tool(it is very good for customers) works, I think first I want
to make this part code working, then you can decide if you pickup or not.
Old ioctls work with DPA tool. From your comments, looks old
implementation does not work.
Need I use "register_ioctl32_convension" and
"unregister_ioctl32_convension"?
Thank you so much!
wendy
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_mgmt.c
linux-2.6.11.new/drivers/serial/jsm/jsm_mgmt.c
--- linux-2.6.11.org/drivers/serial/jsm/jsm_mgmt.c 1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_mgmt.c 2005-03-04
11:28:39.420979504 -0600
@@ -0,0 +1,336 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau <[EMAIL PROTECTED]>
+ * Wendy Xiong <[EMAIL PROTECTED]>
+ *
+ ***/
+
+/
+ *
+ * This file implements the mgmt functionality for the
+ * Neo and ClassicBoard based product lines.
+ *
+
+ */
+#include
+#include/* For jiffies, task states */
+#include/* For tasklet and interrupt structs/defines */
+#include
+#include/* For copy_from_user/copy_to_user */
+
+#include "jsm_driver.h"
+
+/* Our "in use" variables, to enforce 1 open only */
+static int jsm_mgmt_in_use[8];
+spinlock_t jsm_global_lock = SPIN_LOCK_UNLOCKED;
+
+/*
+ * jsm_mgmt_open()
+ *
+ * Open the mgmt/downld/dpa device
+ */
+int jsm_mgmt_open(struct inode *inode, struct file *file)
+{
+ unsigned long lock_flags;
+ unsigned int minor = JSM_MINOR(inode);
+
+ DPRINTK(MGMT, INFO, "start.\n");
+
+ spin_lock_irqsave(_global_lock, lock_flags);
+
+ /* mgmt device */
+ if (minor < 8) {
+ /* Only allow 1 open at a time on mgmt device */
+ if (jsm_mgmt_in_use[minor]) {
+ spin_unlock_irqrestore(_global_lock, lock_flags);
+ return -EBUSY;
+ }
+ jsm_mgmt_in_use[minor]++;
+ }
+ else {
+ spin_unlock_irqrestore(_global_lock, lock_flags);
+ return -ENXIO;
+ }
+
+ spin_unlock_irqrestore(_global_lock, lock_flags);
+
+ DPRINTK(MGMT, INFO, "finish.\n");
+
+ return 0;
+}
+
+/*
+ * jsm_mgmt_close()
+ *
+ * Open the mgmt/dpa device
+ */
+int jsm_mgmt_close(struct inode *inode, struct file *file)
+{
+ unsigned long lock_flags;
+ unsigned int minor = JSM_MINOR(inode);
+
+ DPRINTK(MGMT, INFO, "start.\n");
+
+ spin_lock_irqsave(_global_lock, lock_flags);
+
+ /* mgmt device */
+ if (minor < 8) {
+ if (jsm_mgmt_in_use[minor])
+ jsm_mgmt_in_use[minor] = 0;
+ }
+ spin_unlock_irqrestore(_global_lock, lock_flags);
+
+ DPRINTK(MGMT, INFO, "finish.\n");
+
+ return 0;
+}
+
+/*
+ * jsm_mgmt_ioctl()
+ *
+ * ioctl the mgmt/dpa device
+ */
+
+int jsm_mgmt_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
unsigned long arg)
+{
+ unsigned long lock_flags;
+ void __user *uarg = (void __user *) arg;
+
+ DPRINTK(MGMT, INFO, "start.\n");
+
+ switch (cmd) {
+
+ case DIGI_GETDD:
+ {
+ /*
+* This returns the total number
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote: On Wed, Mar 09, 2005 at 01:35:41PM -0600, Kilau, Scott wrote: As it stands today, your requirement appears to be that she needs to yank all diags ioctls and sysfs files before the driver can make it into the kernel sources. Not all sysfs files, sysfs files are fine, as long as they are implemented properly, and are there for things that "make sense". But yes, it should would be easier to accept the driver if the ioctls were not there :) thanks, greg k-h Hi All, I think Digi's DPA magagement tool has very good user interfaces. I am going to change and fix the problem. Then Greg can decide if he want to pick it up or not. I will attatch the DPA graphic interface for you next time. Thanks, wendy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Wed, Mar 09, 2005 at 01:35:41PM -0600, Kilau, Scott wrote: > As it stands today, your requirement appears to be that she needs > to yank all diags ioctls and sysfs files before the driver can make > it into the kernel sources. Not all sysfs files, sysfs files are fine, as long as they are implemented properly, and are there for things that "make sense". But yes, it should would be easier to accept the driver if the ioctls were not there :) thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [ patch 6/7] drivers/serial/jsm: new serial device driver
> > DPA support is a requirement for all Digi drivers, so it would > > not be possible for me to remove them from my "dgnc" version > > of the driver. > "requirement" from whom and to who? The Linux kernel community? >From our customers who are moving from other OS's to Linux, and expect DPA support to be under Linux as well. > It's not a reservation issue, it's the fact that we don't want to allow > new ioctls, and if we do, they had better work properly (your > implementation does not.) > > thanks, > > greg k-h Which is fine and I accept the blame for. This is something Wendy can change and fix. I am explaining why they exist today and my argument of why we need them to stay. As it stands today, your requirement appears to be that she needs to yank all diags ioctls and sysfs files before the driver can make it into the kernel sources. This is also fine, but Wendy and IBM will need to decide whether all our diags utilties are needed for the JSM driver or not. Scott - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Wed, Mar 09, 2005 at 11:42:31AM -0600, Kilau, Scott wrote: > Hi Wendy, Greg, all, > > If IBM intends on our DPA management program to work for the JSM > products, the ioctls are needed. Wendy, what is IBM's stance on this? > DPA support is a requirement for all Digi drivers, so it would > not be possible for me to remove them from my "dgnc" version > of the driver. "requirement" from whom and to who? The Linux kernel community? > For the JSM driver, its up to you whether you feel its needed or not. > > However, I would like to mention that the DIGI drivers that currently > reside in the kernel sources *do* reserve that ioctl space, > and is acknowledged by "Documentation/ioctl-number.txt": > > d' F0-FF linux/digi1.h > > I understand that the list is not a reservation list, > but a current list of potential ioctl conflicts... It's not a reservation issue, it's the fact that we don't want to allow new ioctls, and if we do, they had better work properly (your implementation does not.) thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [ patch 6/7] drivers/serial/jsm: new serial device driver
Hi Wendy, Greg, all,
If IBM intends on our DPA management program to work for the JSM
products,
the ioctls are needed.
DPA support is a requirement for all Digi drivers, so it would
not be possible for me to remove them from my "dgnc" version
of the driver.
For the JSM driver, its up to you whether you feel its needed or not.
However, I would like to mention that the DIGI drivers that currently
reside in the kernel sources *do* reserve that ioctl space,
and is acknowledged by "Documentation/ioctl-number.txt":
> d' F0-FF linux/digi1.h
I understand that the list is not a reservation list,
but a current list of potential ioctl conflicts...
But the JSM driver sure doesn't add any new ioctl ranges,
or cause any new conflicts that didn't already exist.
Scott
-Original Message-
From: Wen Xiong [mailto:[EMAIL PROTECTED]
Sent: Wednesday, March 09, 2005 11:32 AM
To: Greg KH
Cc: Kilau, Scott; linux-kernel@vger.kernel.org
Subject: Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
>On Wed, Mar 09, 2005 at 10:50:04AM -0500, Wen Xiong wrote:
>
>
>>+/* Ioctls needed for dpa operation */
>>+#define DIGI_GETDD ('d'<<8) | 248 /* get driver info
*/
>>+#define DIGI_GETBD ('d'<<8) | 249 /* get board info
*/
>>+#define DIGI_GET_NI_INFO ('d'<<8) | 250 /*
nonintelligent state snfo */
>>
>>
>
>Hm, new ioctls still?...
>
>And the structures you are attempting to access through these ioctls
are
>incorrect, so if you are still insisting you need them, at least make
>the code work properly :(
>
>thanks,
>
>greg k-h
>
>
>
Hi Scott,
The above three ioctls are for dpa/managment. If I removed these ioctls,
I have to remove jsm_mgmt.c(patch5.jasmine).
Do you mind I remove jsm_mgmt.c code now? From my side, I don't think I
need them now.
Maybe we can have a solution in the kernel as Russell and Greg said
later.
Thanks,
wendy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Wed, Mar 09, 2005 at 10:50:04AM -0500, Wen Xiong wrote:
+/* Ioctls needed for dpa operation */
+#define DIGI_GETDD ('d'<<8) | 248 /* get driver info */
+#define DIGI_GETBD ('d'<<8) | 249 /* get board info */
+#define DIGI_GET_NI_INFO ('d'<<8) | 250 /* nonintelligent state snfo */
Hm, new ioctls still?...
And the structures you are attempting to access through these ioctls are
incorrect, so if you are still insisting you need them, at least make
the code work properly :(
thanks,
greg k-h
Hi Scott,
The above three ioctls are for dpa/managment. If I removed these ioctls,
I have to remove jsm_mgmt.c(patch5.jasmine).
Do you mind I remove jsm_mgmt.c code now? From my side, I don't think I
need them now.
Maybe we can have a solution in the kernel as Russell and Greg said later.
Thanks,
wendy
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Wed, Mar 09, 2005 at 10:50:04AM -0500, Wen Xiong wrote:
> +/* Ioctls needed for dpa operation */
> +#define DIGI_GETDD ('d'<<8) | 248 /* get driver info */
> +#define DIGI_GETBD ('d'<<8) | 249 /* get board info */
> +#define DIGI_GET_NI_INFO ('d'<<8) | 250 /* nonintelligent state
> snfo */
Hm, new ioctls still?...
And the structures you are attempting to access through these ioctls are
incorrect, so if you are still insisting you need them, at least make
the code work properly :(
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Tue, Mar 08, 2005 at 01:42:31PM -0500, Wen Xiong wrote:
The following email I got from Scott Kilau in digi:
Scott Kilau wrote:
The DPA program is very old, and is shared among other drivers
and OS's,
so changing the code to read the sysfs instead of doing ioctls
is not possible.
Hm, so we are supposed to support, for forever, custom ioctls just
because another OS, that we care nothing about, supports it? Hm, I just
can't think this is a acceptable thing, sorry. Especially due to the
nasty 64/32 bit issues...
However, any *new* tools I write, will use sysfs, which is why
we need to have both the ioctl calls and sysfs files.
Please, no new ioctls, end of story.
The digi.h file has extra structures and ioctls that may not be
used in the driver, as that header
is shared among other drivers and OS's.
Please remove them as they are not needed in this OS, right? As you
already had to change the naming, structure definitions, and format, you
aren't sharing the file.
Removed the all things are not needed(digi.h) in this OS.
Attached the new patch6.jasmine for your reviewing.
Thanks,
wendy
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_driver.h
linux-2.6.11.new/drivers/serial/jsm/jsm_driver.h
--- linux-2.6.11.org/drivers/serial/jsm/jsm_driver.h1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_driver.h2005-03-09
09:43:11.995943064 -0600
@@ -0,0 +1,518 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau <[EMAIL PROTECTED]>
+ * Wendy Xiong <[EMAIL PROTECTED]>
+ *
+ ***/
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include
+#include
+#include/* To pick up the varions Linux types */
+#include
+#include
+#include
+#include
+
+/*
+ * Debugging levels can be set using debug insmod variable
+ * They can also be compiled out completely.
+ */
+enum {
+ DBG_INIT= 0x01,
+ DBG_BASIC = 0x02,
+ DBG_CORE= 0x04,
+ DBG_OPEN= 0x08,
+ DBG_CLOSE = 0x10,
+ DBG_READ= 0x20,
+ DBG_WRITE = 0x40,
+ DBG_IOCTL = 0x80,
+ DBG_PROC= 0x100,
+ DBG_PARAM = 0x200,
+ DBG_PSCAN = 0x400,
+ DBG_EVENT = 0x800,
+ DBG_DRAIN = 0x1000,
+ DBG_MSIGS = 0x2000,
+ DBG_MGMT= 0x4000,
+ DBG_INTR= 0x8000,
+ DBG_CARR= 0x1,
+};
+
+#define jsm_printk(nlevel, klevel, pdev, fmt, args...) \
+ if ((DBG_##nlevel & jsm_debug)) \
+ dev_printk(KERN_##klevel, pdev->dev, fmt, ## args)
+
+#define MAXPORTS 8
+#define MAX_STOPS_SENT 5
+
+/* Board type definitions */
+
+#define T_NEO
+#define T_CLASSIC 0001
+#define T_PCIBUS0400
+
+/* Board State Definitions */
+
+#define BD_RUNNING 0x0
+#define BD_REASON 0x7f
+#define BD_NOTFOUND 0x1
+#define BD_NOIOPORT 0x2
+#define BD_NOMEM0x3
+#define BD_NOBIOS 0x4
+#define BD_NOFEP0x5
+#define BD_FAILED 0x6
+#define BD_ALLOCATED0x7
+#define BD_TRIBOOT 0x8
+#define BD_BADKME 0x80
+
+/* 4 extra for alignment play space */
+#define WRITEBUFLEN((4096) + 4)
+#define MYFLIPLEN N_TTY_BUF_SIZE
+
+#define JSM_VERSION"jsm: 1.1-1-INKERNEL"
+#define JSM_PARTNUM"40002438_A-INKERNEL"
+
+/*
+ * All the possible states the driver can be while being loaded.
+ */
+enum {
+ DRIVER_INITIALIZED = 0,
+ DRIVER_READY
+};
+
+/*
+ * All the possible states the board can be while booting up.
+ */
+enum {
+ BOARD_FAILED = 0,
+ BOARD_FOUND,
+ BOARD_READY
+};
+
+struct board_id {
+ u8 *name;
+ u32 maxports;
+};
+
+struct jsm_board;
+struct jsm_channel;
+
+/
+ * Per board operations structure *
+
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Tue, Mar 08, 2005 at 01:42:31PM -0500, Wen Xiong wrote:
The following email I got from Scott Kilau in digi:
Scott Kilau wrote:
The DPA program is very old, and is shared among other drivers
and OS's,
so changing the code to read the sysfs instead of doing ioctls
is not possible.
Hm, so we are supposed to support, for forever, custom ioctls just
because another OS, that we care nothing about, supports it? Hm, I just
can't think this is a acceptable thing, sorry. Especially due to the
nasty 64/32 bit issues...
However, any *new* tools I write, will use sysfs, which is why
we need to have both the ioctl calls and sysfs files.
Please, no new ioctls, end of story.
The digi.h file has extra structures and ioctls that may not be
used in the driver, as that header
is shared among other drivers and OS's.
Please remove them as they are not needed in this OS, right? As you
already had to change the naming, structure definitions, and format, you
aren't sharing the file.
Removed the all things are not needed(digi.h) in this OS.
Attached the new patch6.jasmine for your reviewing.
Thanks,
wendy
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_driver.h
linux-2.6.11.new/drivers/serial/jsm/jsm_driver.h
--- linux-2.6.11.org/drivers/serial/jsm/jsm_driver.h1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_driver.h2005-03-09
09:43:11.995943064 -0600
@@ -0,0 +1,518 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau [EMAIL PROTECTED]
+ * Wendy Xiong [EMAIL PROTECTED]
+ *
+ ***/
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include linux/kernel.h
+#include linux/version.h
+#include linux/types.h /* To pick up the varions Linux types */
+#include linux/tty.h
+#include linux/serial_core.h
+#include linux/device.h
+#include linux/ioctl.h
+
+/*
+ * Debugging levels can be set using debug insmod variable
+ * They can also be compiled out completely.
+ */
+enum {
+ DBG_INIT= 0x01,
+ DBG_BASIC = 0x02,
+ DBG_CORE= 0x04,
+ DBG_OPEN= 0x08,
+ DBG_CLOSE = 0x10,
+ DBG_READ= 0x20,
+ DBG_WRITE = 0x40,
+ DBG_IOCTL = 0x80,
+ DBG_PROC= 0x100,
+ DBG_PARAM = 0x200,
+ DBG_PSCAN = 0x400,
+ DBG_EVENT = 0x800,
+ DBG_DRAIN = 0x1000,
+ DBG_MSIGS = 0x2000,
+ DBG_MGMT= 0x4000,
+ DBG_INTR= 0x8000,
+ DBG_CARR= 0x1,
+};
+
+#define jsm_printk(nlevel, klevel, pdev, fmt, args...) \
+ if ((DBG_##nlevel jsm_debug)) \
+ dev_printk(KERN_##klevel, pdev-dev, fmt, ## args)
+
+#define MAXPORTS 8
+#define MAX_STOPS_SENT 5
+
+/* Board type definitions */
+
+#define T_NEO
+#define T_CLASSIC 0001
+#define T_PCIBUS0400
+
+/* Board State Definitions */
+
+#define BD_RUNNING 0x0
+#define BD_REASON 0x7f
+#define BD_NOTFOUND 0x1
+#define BD_NOIOPORT 0x2
+#define BD_NOMEM0x3
+#define BD_NOBIOS 0x4
+#define BD_NOFEP0x5
+#define BD_FAILED 0x6
+#define BD_ALLOCATED0x7
+#define BD_TRIBOOT 0x8
+#define BD_BADKME 0x80
+
+/* 4 extra for alignment play space */
+#define WRITEBUFLEN((4096) + 4)
+#define MYFLIPLEN N_TTY_BUF_SIZE
+
+#define JSM_VERSIONjsm: 1.1-1-INKERNEL
+#define JSM_PARTNUM40002438_A-INKERNEL
+
+/*
+ * All the possible states the driver can be while being loaded.
+ */
+enum {
+ DRIVER_INITIALIZED = 0,
+ DRIVER_READY
+};
+
+/*
+ * All the possible states the board can be while booting up.
+ */
+enum {
+ BOARD_FAILED = 0,
+ BOARD_FOUND,
+ BOARD_READY
+};
+
+struct board_id {
+ u8 *name;
+ u32 maxports;
+};
+
+struct jsm_board;
+struct jsm_channel;
+
+/
+ * Per board operations
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Wed, Mar 09, 2005 at 10:50:04AM -0500, Wen Xiong wrote:
+/* Ioctls needed for dpa operation */
+#define DIGI_GETDD ('d'8) | 248 /* get driver info */
+#define DIGI_GETBD ('d'8) | 249 /* get board info */
+#define DIGI_GET_NI_INFO ('d'8) | 250 /* nonintelligent state
snfo */
Hm, new ioctls still?...
And the structures you are attempting to access through these ioctls are
incorrect, so if you are still insisting you need them, at least make
the code work properly :(
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Wed, Mar 09, 2005 at 10:50:04AM -0500, Wen Xiong wrote:
+/* Ioctls needed for dpa operation */
+#define DIGI_GETDD ('d'8) | 248 /* get driver info */
+#define DIGI_GETBD ('d'8) | 249 /* get board info */
+#define DIGI_GET_NI_INFO ('d'8) | 250 /* nonintelligent state snfo */
Hm, new ioctls still?...
And the structures you are attempting to access through these ioctls are
incorrect, so if you are still insisting you need them, at least make
the code work properly :(
thanks,
greg k-h
Hi Scott,
The above three ioctls are for dpa/managment. If I removed these ioctls,
I have to remove jsm_mgmt.c(patch5.jasmine).
Do you mind I remove jsm_mgmt.c code now? From my side, I don't think I
need them now.
Maybe we can have a solution in the kernel as Russell and Greg said later.
Thanks,
wendy
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [ patch 6/7] drivers/serial/jsm: new serial device driver
Hi Wendy, Greg, all,
If IBM intends on our DPA management program to work for the JSM
products,
the ioctls are needed.
DPA support is a requirement for all Digi drivers, so it would
not be possible for me to remove them from my dgnc version
of the driver.
For the JSM driver, its up to you whether you feel its needed or not.
However, I would like to mention that the DIGI drivers that currently
reside in the kernel sources *do* reserve that ioctl space,
and is acknowledged by Documentation/ioctl-number.txt:
d' F0-FF linux/digi1.h
I understand that the list is not a reservation list,
but a current list of potential ioctl conflicts...
But the JSM driver sure doesn't add any new ioctl ranges,
or cause any new conflicts that didn't already exist.
Scott
-Original Message-
From: Wen Xiong [mailto:[EMAIL PROTECTED]
Sent: Wednesday, March 09, 2005 11:32 AM
To: Greg KH
Cc: Kilau, Scott; linux-kernel@vger.kernel.org
Subject: Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Wed, Mar 09, 2005 at 10:50:04AM -0500, Wen Xiong wrote:
+/* Ioctls needed for dpa operation */
+#define DIGI_GETDD ('d'8) | 248 /* get driver info
*/
+#define DIGI_GETBD ('d'8) | 249 /* get board info
*/
+#define DIGI_GET_NI_INFO ('d'8) | 250 /*
nonintelligent state snfo */
Hm, new ioctls still?...
And the structures you are attempting to access through these ioctls
are
incorrect, so if you are still insisting you need them, at least make
the code work properly :(
thanks,
greg k-h
Hi Scott,
The above three ioctls are for dpa/managment. If I removed these ioctls,
I have to remove jsm_mgmt.c(patch5.jasmine).
Do you mind I remove jsm_mgmt.c code now? From my side, I don't think I
need them now.
Maybe we can have a solution in the kernel as Russell and Greg said
later.
Thanks,
wendy
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Wed, Mar 09, 2005 at 11:42:31AM -0600, Kilau, Scott wrote: Hi Wendy, Greg, all, If IBM intends on our DPA management program to work for the JSM products, the ioctls are needed. Wendy, what is IBM's stance on this? DPA support is a requirement for all Digi drivers, so it would not be possible for me to remove them from my dgnc version of the driver. requirement from whom and to who? The Linux kernel community? For the JSM driver, its up to you whether you feel its needed or not. However, I would like to mention that the DIGI drivers that currently reside in the kernel sources *do* reserve that ioctl space, and is acknowledged by Documentation/ioctl-number.txt: d' F0-FF linux/digi1.h I understand that the list is not a reservation list, but a current list of potential ioctl conflicts... It's not a reservation issue, it's the fact that we don't want to allow new ioctls, and if we do, they had better work properly (your implementation does not.) thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [ patch 6/7] drivers/serial/jsm: new serial device driver
DPA support is a requirement for all Digi drivers, so it would not be possible for me to remove them from my dgnc version of the driver. requirement from whom and to who? The Linux kernel community? From our customers who are moving from other OS's to Linux, and expect DPA support to be under Linux as well. It's not a reservation issue, it's the fact that we don't want to allow new ioctls, and if we do, they had better work properly (your implementation does not.) thanks, greg k-h Which is fine and I accept the blame for. This is something Wendy can change and fix. I am explaining why they exist today and my argument of why we need them to stay. As it stands today, your requirement appears to be that she needs to yank all diags ioctls and sysfs files before the driver can make it into the kernel sources. This is also fine, but Wendy and IBM will need to decide whether all our diags utilties are needed for the JSM driver or not. Scott - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Wed, Mar 09, 2005 at 01:35:41PM -0600, Kilau, Scott wrote: As it stands today, your requirement appears to be that she needs to yank all diags ioctls and sysfs files before the driver can make it into the kernel sources. Not all sysfs files, sysfs files are fine, as long as they are implemented properly, and are there for things that make sense. But yes, it should would be easier to accept the driver if the ioctls were not there :) thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote: On Wed, Mar 09, 2005 at 01:35:41PM -0600, Kilau, Scott wrote: As it stands today, your requirement appears to be that she needs to yank all diags ioctls and sysfs files before the driver can make it into the kernel sources. Not all sysfs files, sysfs files are fine, as long as they are implemented properly, and are there for things that make sense. But yes, it should would be easier to accept the driver if the ioctls were not there :) thanks, greg k-h Hi All, I think Digi's DPA magagement tool has very good user interfaces. I am going to change and fix the problem. Then Greg can decide if he want to pick it up or not. I will attatch the DPA graphic interface for you next time. Thanks, wendy - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
For the JSM driver, its up to you whether you feel its needed or not.
However, I would like to mention that the DIGI drivers that currently
reside in the kernel sources *do* reserve that ioctl space,
and is acknowledged by Documentation/ioctl-number.txt:
d' F0-FF linux/digi1.h
I understand that the list is not a reservation list,
but a current list of potential ioctl conflicts...
It's not a reservation issue, it's the fact that we don't want to allow
new ioctls, and if we do, they had better work properly (your
implementation does not.)
Hi Greg,
We understood that you don't want more new ioctls in the driver.
For DPA tool(it is very good for customers) works, I think first I want
to make this part code working, then you can decide if you pickup or not.
Old ioctls work with DPA tool. From your comments, looks old
implementation does not work.
Need I use register_ioctl32_convension and
unregister_ioctl32_convension?
Thank you so much!
wendy
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_mgmt.c
linux-2.6.11.new/drivers/serial/jsm/jsm_mgmt.c
--- linux-2.6.11.org/drivers/serial/jsm/jsm_mgmt.c 1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_mgmt.c 2005-03-04
11:28:39.420979504 -0600
@@ -0,0 +1,336 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau [EMAIL PROTECTED]
+ * Wendy Xiong [EMAIL PROTECTED]
+ *
+ ***/
+
+/
+ *
+ * This file implements the mgmt functionality for the
+ * Neo and ClassicBoard based product lines.
+ *
+
+ */
+#include linux/ctype.h
+#include linux/sched.h /* For jiffies, task states */
+#include linux/interrupt.h /* For tasklet and interrupt structs/defines */
+#include linux/termios.h
+#include asm/uaccess.h /* For copy_from_user/copy_to_user */
+
+#include jsm_driver.h
+
+/* Our in use variables, to enforce 1 open only */
+static int jsm_mgmt_in_use[8];
+spinlock_t jsm_global_lock = SPIN_LOCK_UNLOCKED;
+
+/*
+ * jsm_mgmt_open()
+ *
+ * Open the mgmt/downld/dpa device
+ */
+int jsm_mgmt_open(struct inode *inode, struct file *file)
+{
+ unsigned long lock_flags;
+ unsigned int minor = JSM_MINOR(inode);
+
+ DPRINTK(MGMT, INFO, start.\n);
+
+ spin_lock_irqsave(jsm_global_lock, lock_flags);
+
+ /* mgmt device */
+ if (minor 8) {
+ /* Only allow 1 open at a time on mgmt device */
+ if (jsm_mgmt_in_use[minor]) {
+ spin_unlock_irqrestore(jsm_global_lock, lock_flags);
+ return -EBUSY;
+ }
+ jsm_mgmt_in_use[minor]++;
+ }
+ else {
+ spin_unlock_irqrestore(jsm_global_lock, lock_flags);
+ return -ENXIO;
+ }
+
+ spin_unlock_irqrestore(jsm_global_lock, lock_flags);
+
+ DPRINTK(MGMT, INFO, finish.\n);
+
+ return 0;
+}
+
+/*
+ * jsm_mgmt_close()
+ *
+ * Open the mgmt/dpa device
+ */
+int jsm_mgmt_close(struct inode *inode, struct file *file)
+{
+ unsigned long lock_flags;
+ unsigned int minor = JSM_MINOR(inode);
+
+ DPRINTK(MGMT, INFO, start.\n);
+
+ spin_lock_irqsave(jsm_global_lock, lock_flags);
+
+ /* mgmt device */
+ if (minor 8) {
+ if (jsm_mgmt_in_use[minor])
+ jsm_mgmt_in_use[minor] = 0;
+ }
+ spin_unlock_irqrestore(jsm_global_lock, lock_flags);
+
+ DPRINTK(MGMT, INFO, finish.\n);
+
+ return 0;
+}
+
+/*
+ * jsm_mgmt_ioctl()
+ *
+ * ioctl the mgmt/dpa device
+ */
+
+int jsm_mgmt_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
unsigned long arg)
+{
+ unsigned long lock_flags;
+ void __user *uarg = (void __user *) arg;
+
+ DPRINTK(MGMT, INFO, start.\n);
+
+ switch (cmd) {
+
+ case DIGI_GETDD:
+ {
+
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Tue, Mar 08, 2005 at 01:42:31PM -0500, Wen Xiong wrote: > The following email I got from Scott Kilau in digi: > Scott Kilau wrote: > >The DPA program is very old, and is shared among other drivers > and OS's, >so changing the code to read the sysfs instead of doing ioctls > is not possible. Hm, so we are supposed to support, for forever, custom ioctls just because another OS, that we care nothing about, supports it? Hm, I just can't think this is a acceptable thing, sorry. Especially due to the nasty 64/32 bit issues... >However, any *new* tools I write, will use sysfs, which is why > we need to have both the ioctl calls and sysfs files. Please, no new ioctls, end of story. >The digi.h file has extra structures and ioctls that may not be > used in the driver, as that header >is shared among other drivers and OS's. Please remove them as they are not needed in this OS, right? As you already had to change the naming, structure definitions, and format, you aren't sharing the file. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Mon, Mar 07, 2005 at 05:48:56PM -0500, Wen Xiong wrote:
Since some tools in Digi company need these new ioctls to access device
driver. I still keep these new ioctls.
What tools? What are they used for? Why do they need them? Why can't
they just use the sysfs files?
As the driver isn't in the kernel tree, there should not be any users
expecting these ioctls to be around, right?
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Hi Greg,
I exchanged emails with digi developer, I scrubbed out some structs and
ioctls aren't being used by this driver.
Only keep some structures for management ports. After scrubbed the
digi.h, I combined two headers to one header.
Hope you like it this time.
Thanks,
wendy
The following email I got from Scott Kilau in digi:
Scott Kilau wrote:
The DPA program is very old, and is shared among other drivers
and OS's,
so changing the code to read the sysfs instead of doing ioctls
is not possible.
However, any *new* tools I write, will use sysfs, which is why
we need to have both the ioctl calls and sysfs files.
The digi.h file has extra structures and ioctls that may not be
used in the driver, as that header
is shared among other drivers and OS's.
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_driver.h
linux-2.6.11.new/drivers/serial/jsm/jsm_driver.h
--- linux-2.6.11.org/drivers/serial/jsm/jsm_driver.h1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_driver.h2005-03-08
12:19:05.204062432 -0600
@@ -0,0 +1,526 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau <[EMAIL PROTECTED]>
+ * Wendy Xiong <[EMAIL PROTECTED]>
+ *
+ ***/
+
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include
+#include
+#include/* To pick up the varions Linux types */
+#include
+#include
+#include
+#include
+
+/*
+ * Debugging levels can be set using debug insmod variable
+ * They can also be compiled out completely.
+ */
+enum {
+ DBG_INIT= 0x01,
+ DBG_BASIC = 0x02,
+ DBG_CORE= 0x04,
+ DBG_OPEN= 0x08,
+ DBG_CLOSE = 0x10,
+ DBG_READ= 0x20,
+ DBG_WRITE = 0x40,
+ DBG_IOCTL = 0x80,
+ DBG_PROC= 0x100,
+ DBG_PARAM = 0x200,
+ DBG_PSCAN = 0x400,
+ DBG_EVENT = 0x800,
+ DBG_DRAIN = 0x1000,
+ DBG_MSIGS = 0x2000,
+ DBG_MGMT= 0x4000,
+ DBG_INTR= 0x8000,
+ DBG_CARR= 0x1,
+};
+
+#define jsm_printk(nlevel, klevel, pdev, fmt, args...) \
+ if ((DBG_##nlevel & jsm_debug)) \
+ dev_printk(KERN_##klevel, pdev->dev, fmt, ## args)
+
+#define MAXPORTS 8
+#define MAX_STOPS_SENT 5
+
+/* Board type definitions */
+
+#define T_NEO
+#define T_CLASSIC 0001
+#define T_PCIBUS0400
+
+/* Board State Definitions */
+
+#define BD_RUNNING 0x0
+#define BD_REASON 0x7f
+#define BD_NOTFOUND 0x1
+#define BD_NOIOPORT 0x2
+#define BD_NOMEM0x3
+#define BD_NOBIOS 0x4
+#define BD_NOFEP0x5
+#define BD_FAILED 0x6
+#define BD_ALLOCATED0x7
+#define BD_TRIBOOT 0x8
+#define BD_BADKME 0x80
+
+/* 4 extra for alignment play space */
+#define WRITEBUFLEN((4096) + 4)
+#define MYFLIPLEN N_TTY_BUF_SIZE
+
+#define JSM_VERSION"jsm: 1.1-1-INKERNEL"
+#define JSM_PARTNUM"40002438_A-INKERNEL"
+
+/*
+ * All the possible states the driver can be while being loaded.
+ */
+enum {
+ DRIVER_INITIALIZED = 0,
+ DRIVER_READY
+};
+
+/*
+ * All the possible states the board can be while booting up.
+ */
+enum {
+ BOARD_FAILED = 0,
+ BOARD_FOUND,
+ BOARD_READY
+};
+
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Mon, Mar 07, 2005 at 05:48:56PM -0500, Wen Xiong wrote:
Since some tools in Digi company need these new ioctls to access device
driver. I still keep these new ioctls.
What tools? What are they used for? Why do they need them? Why can't
they just use the sysfs files?
As the driver isn't in the kernel tree, there should not be any users
expecting these ioctls to be around, right?
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Hi Greg,
I exchanged emails with digi developer, I scrubbed out some structs and
ioctls aren't being used by this driver.
Only keep some structures for management ports. After scrubbed the
digi.h, I combined two headers to one header.
Hope you like it this time.
Thanks,
wendy
The following email I got from Scott Kilau in digi:
Scott Kilau wrote:
The DPA program is very old, and is shared among other drivers
and OS's,
so changing the code to read the sysfs instead of doing ioctls
is not possible.
However, any *new* tools I write, will use sysfs, which is why
we need to have both the ioctl calls and sysfs files.
The digi.h file has extra structures and ioctls that may not be
used in the driver, as that header
is shared among other drivers and OS's.
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/jsm_driver.h
linux-2.6.11.new/drivers/serial/jsm/jsm_driver.h
--- linux-2.6.11.org/drivers/serial/jsm/jsm_driver.h1969-12-31
18:00:00.0 -0600
+++ linux-2.6.11.new/drivers/serial/jsm/jsm_driver.h2005-03-08
12:19:05.204062432 -0600
@@ -0,0 +1,526 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau [EMAIL PROTECTED]
+ * Wendy Xiong [EMAIL PROTECTED]
+ *
+ ***/
+
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include linux/kernel.h
+#include linux/version.h
+#include linux/types.h /* To pick up the varions Linux types */
+#include linux/tty.h
+#include linux/serial_core.h
+#include linux/device.h
+#include linux/ioctl.h
+
+/*
+ * Debugging levels can be set using debug insmod variable
+ * They can also be compiled out completely.
+ */
+enum {
+ DBG_INIT= 0x01,
+ DBG_BASIC = 0x02,
+ DBG_CORE= 0x04,
+ DBG_OPEN= 0x08,
+ DBG_CLOSE = 0x10,
+ DBG_READ= 0x20,
+ DBG_WRITE = 0x40,
+ DBG_IOCTL = 0x80,
+ DBG_PROC= 0x100,
+ DBG_PARAM = 0x200,
+ DBG_PSCAN = 0x400,
+ DBG_EVENT = 0x800,
+ DBG_DRAIN = 0x1000,
+ DBG_MSIGS = 0x2000,
+ DBG_MGMT= 0x4000,
+ DBG_INTR= 0x8000,
+ DBG_CARR= 0x1,
+};
+
+#define jsm_printk(nlevel, klevel, pdev, fmt, args...) \
+ if ((DBG_##nlevel jsm_debug)) \
+ dev_printk(KERN_##klevel, pdev-dev, fmt, ## args)
+
+#define MAXPORTS 8
+#define MAX_STOPS_SENT 5
+
+/* Board type definitions */
+
+#define T_NEO
+#define T_CLASSIC 0001
+#define T_PCIBUS0400
+
+/* Board State Definitions */
+
+#define BD_RUNNING 0x0
+#define BD_REASON 0x7f
+#define BD_NOTFOUND 0x1
+#define BD_NOIOPORT 0x2
+#define BD_NOMEM0x3
+#define BD_NOBIOS 0x4
+#define BD_NOFEP0x5
+#define BD_FAILED 0x6
+#define BD_ALLOCATED0x7
+#define BD_TRIBOOT 0x8
+#define BD_BADKME 0x80
+
+/* 4 extra for alignment play space */
+#define WRITEBUFLEN((4096) + 4)
+#define MYFLIPLEN N_TTY_BUF_SIZE
+
+#define JSM_VERSIONjsm: 1.1-1-INKERNEL
+#define JSM_PARTNUM40002438_A-INKERNEL
+
+/*
+ * All the possible states the driver can be while being loaded.
+ */
+enum {
+ DRIVER_INITIALIZED = 0,
+ DRIVER_READY
+};
+
+/*
+ * All the possible states the board can be while booting up.
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Tue, Mar 08, 2005 at 01:42:31PM -0500, Wen Xiong wrote: The following email I got from Scott Kilau in digi: Scott Kilau wrote: The DPA program is very old, and is shared among other drivers and OS's, so changing the code to read the sysfs instead of doing ioctls is not possible. Hm, so we are supposed to support, for forever, custom ioctls just because another OS, that we care nothing about, supports it? Hm, I just can't think this is a acceptable thing, sorry. Especially due to the nasty 64/32 bit issues... However, any *new* tools I write, will use sysfs, which is why we need to have both the ioctl calls and sysfs files. Please, no new ioctls, end of story. The digi.h file has extra structures and ioctls that may not be used in the driver, as that header is shared among other drivers and OS's. Please remove them as they are not needed in this OS, right? As you already had to change the naming, structure definitions, and format, you aren't sharing the file. thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Mon, Mar 07, 2005 at 05:48:56PM -0500, Wen Xiong wrote: > Since some tools in Digi company need these new ioctls to access device > driver. I still keep these new ioctls. What tools? What are they used for? Why do they need them? Why can't they just use the sysfs files? As the driver isn't in the kernel tree, there should not be any users expecting these ioctls to be around, right? thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Fri, Mar 04, 2005 at 04:08:44PM -0500, Wen Xiong wrote:
+/
+ * Structure used with ioctl commands for DIGI parameters.
+ /
+struct digi_t {
+ unsigned short digi_flags; /* Flags (see above) */
+ unsigned short digi_maxcps; /* Max printer CPS */
+ unsigned short digi_maxchar; /* Max chars in print queue */
+ unsigned short digi_bufsize; /* Buffer size */
+ unsigned char digi_onlen; /* Length of ON string */
+ unsigned char digi_offlen; /* Length of OFF string */
+ char digi_onstr[DIGI_PLEN]; /* Printer on string */
+ char digi_offstr[DIGI_PLEN]; /* Printer off string */
+ char digi_term[DIGI_TSIZ]; /* terminal string */
+};
Oops, don't use _t for a structure name please.
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include
+#include
+#include /* To pick up the varions Linux types */
+#include
+#include
+#include
+#include /* For irqreturn_t type */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
Don't put header files in header files if you can help it. It really
isn't needed here, and odds are, you are including files you don't
really need for each of the different driver .c files. The build will
go faster if you don't do that.
Also, check your ordering, some of those .h files already included the
ones above it.
+#include "digi.h" /* Digi specific ioctl header */
Why do you have your own ioctls? Please do not add any new ones to the
kernel.
+#define DRVSTR "jsm" /* Driver name string */
What is this for?
+#define DPRINTK(nlevel, klevel, fmt, args...) \
+ (void)((DBG_##nlevel & debug) && \
+ printk(KERN_##klevel "%s: " fmt, \
+ __FUNCTION__, ## args));
Please use dev_dbg() or at least dev_printk() for this. It provides
consistancy with the rest of the kernel, and it helps identify your
device much better.
+#define JSM_MAJOR(x) (imajor(x))
+#define JSM_MINOR(x) (iminor(x))
Not needed, please don't use.
+#ifndef _POSIX_VDISABLE
+#define _POSIX_VDISABLE '\0'
+#endif
What would have defined that before?
+/*
+ * Our Global Variables.
+ */
+extern struct uart_driver jsm_uart_driver;
+extern struct board_ops jsm_neo_ops;
+extern int debug;
+extern int rawreadok;
Both of these are bad global variable names.
+extern int jsm_driver_state; /* The state of the driver */
+extern char *jsm_driver_state_text[];/* Array of driver state text */
+
+extern spinlock_t jsm_board_head_lock;
+static LIST_HEAD(jsm_board_head);
Hm, static variable in a header file? bad...
+/*
+ *
+ * Prototypes for non-static functions used in more than one module
+ *
+ */
+extern char *jsm_ioctl_name(int cmd);
+extern int get_jsm_board_number(void);
Bad name for a global function, put the "jsm" at the front please.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Thanks for your reviewing!
Since some tools in Digi company need these new ioctls to access device
driver. I still keep these new ioctls.
Thanks,
wendy
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/digi.h
linux-2.6.11.new/drivers/serial/jsm/digi.h
--- linux-2.6.11.org/drivers/serial/jsm/digi.h 1969-12-31 18:00:00.0
-0600
+++ linux-2.6.11.new/drivers/serial/jsm/digi.h 2005-03-04 11:22:27.558933144
-0600
@@ -0,0 +1,387 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau <[EMAIL PROTECTED]>
+ * Wendy Xiong <[EMAIL PROTECTED]>
+ *
+ ***/
+
+#ifndef __DIGI_H
+#define __DIGI_H
+
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Fri, Mar 04, 2005 at 04:08:44PM -0500, Wen Xiong wrote:
+/
+ * Structure used with ioctl commands for DIGI parameters.
+ /
+struct digi_t {
+ unsigned short digi_flags; /* Flags (see above) */
+ unsigned short digi_maxcps; /* Max printer CPS */
+ unsigned short digi_maxchar; /* Max chars in print queue */
+ unsigned short digi_bufsize; /* Buffer size */
+ unsigned char digi_onlen; /* Length of ON string */
+ unsigned char digi_offlen; /* Length of OFF string */
+ char digi_onstr[DIGI_PLEN]; /* Printer on string */
+ char digi_offstr[DIGI_PLEN]; /* Printer off string */
+ char digi_term[DIGI_TSIZ]; /* terminal string */
+};
Oops, don't use _t for a structure name please.
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include linux/kernel.h
+#include linux/version.h
+#include linux/types.h /* To pick up the varions Linux types */
+#include linux/tty.h
+#include linux/serial_core.h
+#include linux/serial_reg.h
+#include linux/interrupt.h /* For irqreturn_t type */
+#include linux/module.h
+#include linux/moduleparam.h
+#include linux/kdev_t.h
+#include linux/pci.h
+#include linux/pci_ids.h
+#include linux/device.h
+#include linux/config.h
Don't put header files in header files if you can help it. It really
isn't needed here, and odds are, you are including files you don't
really need for each of the different driver .c files. The build will
go faster if you don't do that.
Also, check your ordering, some of those .h files already included the
ones above it.
+#include digi.h /* Digi specific ioctl header */
Why do you have your own ioctls? Please do not add any new ones to the
kernel.
+#define DRVSTR jsm /* Driver name string */
What is this for?
+#define DPRINTK(nlevel, klevel, fmt, args...) \
+ (void)((DBG_##nlevel debug) \
+ printk(KERN_##klevel %s: fmt, \
+ __FUNCTION__, ## args));
Please use dev_dbg() or at least dev_printk() for this. It provides
consistancy with the rest of the kernel, and it helps identify your
device much better.
+#define JSM_MAJOR(x) (imajor(x))
+#define JSM_MINOR(x) (iminor(x))
Not needed, please don't use.
+#ifndef _POSIX_VDISABLE
+#define _POSIX_VDISABLE '\0'
+#endif
What would have defined that before?
+/*
+ * Our Global Variables.
+ */
+extern struct uart_driver jsm_uart_driver;
+extern struct board_ops jsm_neo_ops;
+extern int debug;
+extern int rawreadok;
Both of these are bad global variable names.
+extern int jsm_driver_state; /* The state of the driver */
+extern char *jsm_driver_state_text[];/* Array of driver state text */
+
+extern spinlock_t jsm_board_head_lock;
+static LIST_HEAD(jsm_board_head);
Hm, static variable in a header file? bad...
+/*
+ *
+ * Prototypes for non-static functions used in more than one module
+ *
+ */
+extern char *jsm_ioctl_name(int cmd);
+extern int get_jsm_board_number(void);
Bad name for a global function, put the jsm at the front please.
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Thanks for your reviewing!
Since some tools in Digi company need these new ioctls to access device
driver. I still keep these new ioctls.
Thanks,
wendy
diff -Nuar linux-2.6.11.org/drivers/serial/jsm/digi.h
linux-2.6.11.new/drivers/serial/jsm/digi.h
--- linux-2.6.11.org/drivers/serial/jsm/digi.h 1969-12-31 18:00:00.0
-0600
+++ linux-2.6.11.new/drivers/serial/jsm/digi.h 2005-03-04 11:22:27.558933144
-0600
@@ -0,0 +1,387 @@
+/
+ * Copyright 2003 Digi International (www.digi.com)
+ *
+ * Copyright (C) 2004 IBM Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 * Temple Place - Suite 330, Boston,
+ * MA 02111-1307, USA.
+ *
+ * Contact Information:
+ * Scott H Kilau [EMAIL PROTECTED]
+ * Wendy Xiong [EMAIL
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Mon, Mar 07, 2005 at 05:48:56PM -0500, Wen Xiong wrote: Since some tools in Digi company need these new ioctls to access device driver. I still keep these new ioctls. What tools? What are they used for? Why do they need them? Why can't they just use the sysfs files? As the driver isn't in the kernel tree, there should not be any users expecting these ioctls to be around, right? thanks, greg k-h - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Fri, Mar 04, 2005 at 04:08:44PM -0500, Wen Xiong wrote:
> +/
> + * Structure used with ioctl commands for DIGI parameters.
> + /
> +struct digi_t {
> + unsigned short digi_flags; /* Flags (see above)*/
> + unsigned short digi_maxcps;/* Max printer CPS */
> + unsigned short digi_maxchar; /* Max chars in print queue */
> + unsigned short digi_bufsize; /* Buffer size */
> + unsigned char digi_onlen; /* Length of ON string */
> + unsigned char digi_offlen;/* Length of OFF string */
> + chardigi_onstr[DIGI_PLEN]; /* Printer on string*/
> + chardigi_offstr[DIGI_PLEN]; /* Printer off string */
> + chardigi_term[DIGI_TSIZ]; /* terminal string */
> +};
Oops, don't use _t for a structure name please.
> +#ifndef __JSM_DRIVER_H
> +#define __JSM_DRIVER_H
> +
> +#include
> +#include
> +#include /* To pick up the varions Linux types */
> +#include
> +#include
> +#include
> +#include /* For irqreturn_t type */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Don't put header files in header files if you can help it. It really
isn't needed here, and odds are, you are including files you don't
really need for each of the different driver .c files. The build will
go faster if you don't do that.
Also, check your ordering, some of those .h files already included the
ones above it.
> +#include "digi.h"/* Digi specific ioctl header */
Why do you have your own ioctls? Please do not add any new ones to the
kernel.
> +#define DRVSTR "jsm" /* Driver name string */
What is this for?
> +#define DPRINTK(nlevel, klevel, fmt, args...) \
> + (void)((DBG_##nlevel & debug) && \
> + printk(KERN_##klevel "%s: " fmt, \
> + __FUNCTION__, ## args));
Please use dev_dbg() or at least dev_printk() for this. It provides
consistancy with the rest of the kernel, and it helps identify your
device much better.
> +#define JSM_MAJOR(x) (imajor(x))
> +#define JSM_MINOR(x) (iminor(x))
Not needed, please don't use.
> +#ifndef _POSIX_VDISABLE
> +#define _POSIX_VDISABLE '\0'
> +#endif
What would have defined that before?
> +/*
> + * Our Global Variables.
> + */
> +extern structuart_driver jsm_uart_driver;
> +extern structboard_ops jsm_neo_ops;
> +extern int debug;
> +extern int rawreadok;
Both of these are bad global variable names.
> +extern int jsm_driver_state; /* The state of the driver */
> +extern char *jsm_driver_state_text[];/* Array of driver state text */
> +
> +extern spinlock_t jsm_board_head_lock;
> +static LIST_HEAD(jsm_board_head);
Hm, static variable in a header file? bad...
> +/*
> + *
> + * Prototypes for non-static functions used in more than one module
> + *
> + */
> +extern char *jsm_ioctl_name(int cmd);
> +extern int get_jsm_board_number(void);
Bad name for a global function, put the "jsm" at the front please.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Fri, Mar 04, 2005 at 04:08:44PM -0500, Wen Xiong wrote:
+/
+ * Structure used with ioctl commands for DIGI parameters.
+ /
+struct digi_t {
+ unsigned short digi_flags; /* Flags (see above)*/
+ unsigned short digi_maxcps;/* Max printer CPS */
+ unsigned short digi_maxchar; /* Max chars in print queue */
+ unsigned short digi_bufsize; /* Buffer size */
+ unsigned char digi_onlen; /* Length of ON string */
+ unsigned char digi_offlen;/* Length of OFF string */
+ chardigi_onstr[DIGI_PLEN]; /* Printer on string*/
+ chardigi_offstr[DIGI_PLEN]; /* Printer off string */
+ chardigi_term[DIGI_TSIZ]; /* terminal string */
+};
Oops, don't use _t for a structure name please.
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include linux/kernel.h
+#include linux/version.h
+#include linux/types.h /* To pick up the varions Linux types */
+#include linux/tty.h
+#include linux/serial_core.h
+#include linux/serial_reg.h
+#include linux/interrupt.h /* For irqreturn_t type */
+#include linux/module.h
+#include linux/moduleparam.h
+#include linux/kdev_t.h
+#include linux/pci.h
+#include linux/pci_ids.h
+#include linux/device.h
+#include linux/config.h
Don't put header files in header files if you can help it. It really
isn't needed here, and odds are, you are including files you don't
really need for each of the different driver .c files. The build will
go faster if you don't do that.
Also, check your ordering, some of those .h files already included the
ones above it.
+#include digi.h/* Digi specific ioctl header */
Why do you have your own ioctls? Please do not add any new ones to the
kernel.
+#define DRVSTR jsm /* Driver name string */
What is this for?
+#define DPRINTK(nlevel, klevel, fmt, args...) \
+ (void)((DBG_##nlevel debug) \
+ printk(KERN_##klevel %s: fmt, \
+ __FUNCTION__, ## args));
Please use dev_dbg() or at least dev_printk() for this. It provides
consistancy with the rest of the kernel, and it helps identify your
device much better.
+#define JSM_MAJOR(x) (imajor(x))
+#define JSM_MINOR(x) (iminor(x))
Not needed, please don't use.
+#ifndef _POSIX_VDISABLE
+#define _POSIX_VDISABLE '\0'
+#endif
What would have defined that before?
+/*
+ * Our Global Variables.
+ */
+extern structuart_driver jsm_uart_driver;
+extern structboard_ops jsm_neo_ops;
+extern int debug;
+extern int rawreadok;
Both of these are bad global variable names.
+extern int jsm_driver_state; /* The state of the driver */
+extern char *jsm_driver_state_text[];/* Array of driver state text */
+
+extern spinlock_t jsm_board_head_lock;
+static LIST_HEAD(jsm_board_head);
Hm, static variable in a header file? bad...
+/*
+ *
+ * Prototypes for non-static functions used in more than one module
+ *
+ */
+extern char *jsm_ioctl_name(int cmd);
+extern int get_jsm_board_number(void);
Bad name for a global function, put the jsm at the front please.
thanks,
greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Sun, Feb 27, 2005 at 06:40:20PM -0500, Wen Xiong wrote:
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/digi.h linux-2.6.9.new/drivers/serial/jsm/digi.h
Oh, and please diff against at least the latest kernel release, 2.6.9 is
old...
+ * $Id: digi.h,v 1.7 2004/09/23 16:08:30 scottk Exp $
+ *
+ * NOTE: THIS IS A SHARED HEADER. DO NOT CHANGE CODING STYLE!!!
Not true anymore, right?
+/
+ * Structure to get driver status information
+ /
+struct digi_dinfo {
+ unsigned int dinfo_nboards; /* # boards configured */
+ char dinfo_reserved[12]; /* for future expansion */
+ char dinfo_version[16]; /* driver version */
+};
+
+#define DIGI_GETDD ('d'<<8) | 248 /* get driver info */
All structures that are passed accross the ioctl interface, MUST use the
__u8, __u16, __u32, and friend definitions. unsigned int is not
allowed.
And why have all of these ioctls? Shouldn't most of this stuff be
availble in sysfs instead?
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include /* To pick up the varions Linux types */
+#include /* To pick up the various tty structs/defines */
+#include
+#include /* For irqreturn_t type */
+#include /* For irqreturn_t type */
That comment is incorrect.
+/*
+ * Driver identification, error and debugging statments
+ *
+ * In theory, you can change all occurances of "digi" in the next
+ * three lines, and the driver printk's will all automagically change.
+ *
+ * APR((fmt, args, ...)); Always prints message
+ * DPR((fmt, args, ...)); Only prints if JSM_TRACER is defined at
+ * compile time and jsm_debug!=0
+ *
+ * TRC_TO_CONSOLE:
+ * Setting this to 1 will turn on some general function tracing
+ * resulting in a bunch of extra debugging printks to the console
+ *
+ */
+
+#define PROCSTR "jsm" /* /proc entries */
+#define DEVSTR "/dev/dg/jsm" /* /dev entries */
+#define DRVSTR "jsm" /* Driver name string
+ * displayed by APR */
+#define APR(args) do {printk(DRVSTR": "); printk args;} while (0)
Ick. You _must_ use a KERN_ level for a printk, this is not allowed.
Please use the dev_printk and helper functions instead. It's not ok to
create new functions like this.
And again, what's with the double (( when you use this macro?
+#if TRC_TO_CONSOLE
+#define PRINTF_TO_CONSOLE(args) { printk(DRVSTR": "); printk args; }
+#else //!defined TRACE_TO_CONSOLE
+#define PRINTF_TO_CONSOLE(args)
+#endif
+
+#define TRC(args) { PRINTF_TO_CONSOLE(args) }
do { } while 0
+/* Our 3 magic numbers for our board, channel and unit structs */
+#define JSM_BOARD_MAGIC 0x5c6df104
+#define JSM_CHANNEL_MAGIC 0x6c6df104
+#define JSM_UNIT_MAGIC 0x7c6df104
Don't use magic numbers, they are not needed at all. Please just remove
them from the structures, and use the provided kernel slab debug
functions to catch errors that you might have been able to catch with
the magic values.
+ * This file is intended to contain all the kernel "differences" between the
+ * various kernels that we support.
No, please use this for your 2.4 code, not for your 2.6 driver version.
+# define JSM_MOD_INC_USE_COUNT(rtn) (rtn = 1)
+# define JSM_MOD_DEC_USE_COUNT
You shouldn't even be using these macros in your 2.4 code, so please
don't use it here.
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/jsm_mgmt.h linux-2.6.9.new/drivers/serial/jsm/jsm_mgmt.h
--- linux-2.6.9.orig/drivers/serial/jsm/jsm_mgmt.h 1969-12-31 18:00:00.0 -0600
+++ linux-2.6.9.new/drivers/serial/jsm/jsm_mgmt.h 2005-02-27 17:14:44.747952016 -0600
Do you really need all of these different header files? Why not just
put them all in 1?
+/
+ * Per channel/port NEO UART structure *
+
+ * Base Structure Entries Usage Meanings to Host *
+ * *
+ * W = read write R = read only*
+ * U = Unused. *
+ /
+
+struct neo_uart_struct {
+ volatile uchar txrx; /* WR RHR/THR - Holding Reg */
+ volatile uchar ier; /* WR IER - Interrupt Enable Reg */
+ volatile uchar isr_fcr; /* WR ISR/FCR - Interrupt Status Reg/Fifo Control Reg */
+ volatile uchar lcr; /* WR LCR - Line Control Reg */
+ volatile uchar mcr; /* WR MCR - Modem Control Reg */
+ volatile uchar lsr; /* WR LSR - Line Status Reg */
+ volatile uchar msr; /* WR MSR - Modem Status Reg */
+ volatile uchar spr; /* WR SPR - Scratch Pad Reg */
+ volatile uchar fctr; /* WR FCTR - Feature Control Reg */
+ volatile uchar efr; /* WR EFR - Enhanced Function Reg */
+ volatile uchar tfifo; /* WR TXCNT/TXTRG - Transmit FIFO Reg */
+ volatile uchar rfifo; /* WR RXCNT/RXTRG - Recieve FIFO Reg */
+ volatile uchar
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Greg KH wrote:
On Sun, Feb 27, 2005 at 06:40:20PM -0500, Wen Xiong wrote:
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/digi.h linux-2.6.9.new/drivers/serial/jsm/digi.h
Oh, and please diff against at least the latest kernel release, 2.6.9 is
old...
+ * $Id: digi.h,v 1.7 2004/09/23 16:08:30 scottk Exp $
+ *
+ * NOTE: THIS IS A SHARED HEADER. DO NOT CHANGE CODING STYLE!!!
Not true anymore, right?
+/
+ * Structure to get driver status information
+ /
+struct digi_dinfo {
+ unsigned int dinfo_nboards; /* # boards configured */
+ char dinfo_reserved[12]; /* for future expansion */
+ char dinfo_version[16]; /* driver version */
+};
+
+#define DIGI_GETDD ('d'8) | 248 /* get driver info */
All structures that are passed accross the ioctl interface, MUST use the
__u8, __u16, __u32, and friend definitions. unsigned int is not
allowed.
And why have all of these ioctls? Shouldn't most of this stuff be
availble in sysfs instead?
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include linux/types.h /* To pick up the varions Linux types */
+#include linux/tty.h /* To pick up the various tty structs/defines */
+#include linux/serial_core.h
+#include linux/interrupt.h /* For irqreturn_t type */
+#include linux/module.h /* For irqreturn_t type */
That comment is incorrect.
+/*
+ * Driver identification, error and debugging statments
+ *
+ * In theory, you can change all occurances of digi in the next
+ * three lines, and the driver printk's will all automagically change.
+ *
+ * APR((fmt, args, ...)); Always prints message
+ * DPR((fmt, args, ...)); Only prints if JSM_TRACER is defined at
+ * compile time and jsm_debug!=0
+ *
+ * TRC_TO_CONSOLE:
+ * Setting this to 1 will turn on some general function tracing
+ * resulting in a bunch of extra debugging printks to the console
+ *
+ */
+
+#define PROCSTR jsm /* /proc entries */
+#define DEVSTR /dev/dg/jsm /* /dev entries */
+#define DRVSTR jsm /* Driver name string
+ * displayed by APR */
+#define APR(args) do {printk(DRVSTR: ); printk args;} while (0)
Ick. You _must_ use a KERN_ level for a printk, this is not allowed.
Please use the dev_printk and helper functions instead. It's not ok to
create new functions like this.
And again, what's with the double (( when you use this macro?
+#if TRC_TO_CONSOLE
+#define PRINTF_TO_CONSOLE(args) { printk(DRVSTR: ); printk args; }
+#else //!defined TRACE_TO_CONSOLE
+#define PRINTF_TO_CONSOLE(args)
+#endif
+
+#define TRC(args) { PRINTF_TO_CONSOLE(args) }
do { } while 0
+/* Our 3 magic numbers for our board, channel and unit structs */
+#define JSM_BOARD_MAGIC 0x5c6df104
+#define JSM_CHANNEL_MAGIC 0x6c6df104
+#define JSM_UNIT_MAGIC 0x7c6df104
Don't use magic numbers, they are not needed at all. Please just remove
them from the structures, and use the provided kernel slab debug
functions to catch errors that you might have been able to catch with
the magic values.
+ * This file is intended to contain all the kernel differences between the
+ * various kernels that we support.
No, please use this for your 2.4 code, not for your 2.6 driver version.
+# define JSM_MOD_INC_USE_COUNT(rtn) (rtn = 1)
+# define JSM_MOD_DEC_USE_COUNT
You shouldn't even be using these macros in your 2.4 code, so please
don't use it here.
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/jsm_mgmt.h linux-2.6.9.new/drivers/serial/jsm/jsm_mgmt.h
--- linux-2.6.9.orig/drivers/serial/jsm/jsm_mgmt.h 1969-12-31 18:00:00.0 -0600
+++ linux-2.6.9.new/drivers/serial/jsm/jsm_mgmt.h 2005-02-27 17:14:44.747952016 -0600
Do you really need all of these different header files? Why not just
put them all in 1?
+/
+ * Per channel/port NEO UART structure *
+
+ * Base Structure Entries Usage Meanings to Host *
+ * *
+ * W = read write R = read only*
+ * U = Unused. *
+ /
+
+struct neo_uart_struct {
+ volatile uchar txrx; /* WR RHR/THR - Holding Reg */
+ volatile uchar ier; /* WR IER - Interrupt Enable Reg */
+ volatile uchar isr_fcr; /* WR ISR/FCR - Interrupt Status Reg/Fifo Control Reg */
+ volatile uchar lcr; /* WR LCR - Line Control Reg */
+ volatile uchar mcr; /* WR MCR - Modem Control Reg */
+ volatile uchar lsr; /* WR LSR - Line Status Reg */
+ volatile uchar msr; /* WR MSR - Modem Status Reg */
+ volatile uchar spr; /* WR SPR - Scratch Pad Reg */
+ volatile uchar fctr; /* WR FCTR - Feature Control Reg */
+ volatile uchar efr; /* WR EFR - Enhanced Function Reg */
+ volatile uchar tfifo; /* WR TXCNT/TXTRG - Transmit FIFO Reg */
+ volatile uchar rfifo;
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Sun, Feb 27, 2005 at 06:40:20PM -0500, Wen Xiong wrote:
>
> diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/digi.h
> linux-2.6.9.new/drivers/serial/jsm/digi.h
Oh, and please diff against at least the latest kernel release, 2.6.9 is
old...
> + * $Id: digi.h,v 1.7 2004/09/23 16:08:30 scottk Exp $
> + *
> + * NOTE: THIS IS A SHARED HEADER. DO NOT CHANGE CODING STYLE!!!
Not true anymore, right?
> +/
> + * Structure to get driver status information
> + /
> +struct digi_dinfo {
> + unsigned intdinfo_nboards; /* # boards configured */
> + chardinfo_reserved[12]; /* for future expansion */
> + chardinfo_version[16]; /* driver version */
> +};
> +
> +#define DIGI_GETDD ('d'<<8) | 248 /* get driver info
> */
All structures that are passed accross the ioctl interface, MUST use the
__u8, __u16, __u32, and friend definitions. unsigned int is not
allowed.
And why have all of these ioctls? Shouldn't most of this stuff be
availble in sysfs instead?
> +#ifndef __JSM_DRIVER_H
> +#define __JSM_DRIVER_H
> +
> +#include /* To pick up the varions Linux types */
> +#include/* To pick up the various tty
> structs/defines */
> +#include
> +#include /* For irqreturn_t type */
> +#include /* For irqreturn_t type */
That comment is incorrect.
> +/*
> + * Driver identification, error and debugging statments
> + *
> + * In theory, you can change all occurances of "digi" in the next
> + * three lines, and the driver printk's will all automagically change.
> + *
> + * APR((fmt, args, ...));Always prints message
> + * DPR((fmt, args, ...));Only prints if JSM_TRACER is defined at
> + * compile time and jsm_debug!=0
> + *
> + * TRC_TO_CONSOLE:
> + * Setting this to 1 will turn on some general function tracing
> + * resulting in a bunch of extra debugging printks to the console
> + *
> + */
> +
> +#define PROCSTR "jsm" /* /proc entries
> */
> +#define DEVSTR "/dev/dg/jsm" /* /dev entries
> */
> +#define DRVSTR "jsm" /* Driver name string
> + * displayed by APR */
> +#define APR(args) do {printk(DRVSTR": "); printk args;} while (0)
Ick. You _must_ use a KERN_ level for a printk, this is not allowed.
Please use the dev_printk and helper functions instead. It's not ok to
create new functions like this.
And again, what's with the double (( when you use this macro?
> +#if TRC_TO_CONSOLE
> +#define PRINTF_TO_CONSOLE(args) { printk(DRVSTR": "); printk args; }
> +#else //!defined TRACE_TO_CONSOLE
> +#define PRINTF_TO_CONSOLE(args)
> +#endif
> +
> +#define TRC(args) { PRINTF_TO_CONSOLE(args) }
do { } while 0
> +/* Our 3 magic numbers for our board, channel and unit structs */
> +#define JSM_BOARD_MAGIC 0x5c6df104
> +#define JSM_CHANNEL_MAGIC0x6c6df104
> +#define JSM_UNIT_MAGIC 0x7c6df104
Don't use magic numbers, they are not needed at all. Please just remove
them from the structures, and use the provided kernel slab debug
functions to catch errors that you might have been able to catch with
the magic values.
> + * This file is intended to contain all the kernel "differences" between the
> + * various kernels that we support.
No, please use this for your 2.4 code, not for your 2.6 driver version.
> +# define JSM_MOD_INC_USE_COUNT(rtn) (rtn = 1)
> +# define JSM_MOD_DEC_USE_COUNT
You shouldn't even be using these macros in your 2.4 code, so please
don't use it here.
> diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/jsm_mgmt.h
> linux-2.6.9.new/drivers/serial/jsm/jsm_mgmt.h
> --- linux-2.6.9.orig/drivers/serial/jsm/jsm_mgmt.h1969-12-31
> 18:00:00.0 -0600
> +++ linux-2.6.9.new/drivers/serial/jsm/jsm_mgmt.h 2005-02-27
> 17:14:44.747952016 -0600
Do you really need all of these different header files? Why not just
put them all in 1?
> +/
> + * Per channel/port NEO UART structure
> *
> +
> + * Base Structure Entries Usage Meanings to Host *
> + * *
> + * W = read write R = read only *
> + * U = Unused. *
> + /
> +
> +struct neo_uart_struct {
> + volatile uchar txrx;/* WR RHR/THR - Holding Reg */
> + volatile uchar ier; /* WR IER
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Sun, Feb 27, 2005 at 07:47:58PM -0500, Jeff Garzik wrote:
> >+struct shrink_buf_struct {
> >+unsigned intshrink_buf_vaddr; /* Virtual address of board
> >*/
> >+unsigned intshrink_buf_phys;/* Physical address of board
> >*/
>
> Major bug. These should be unsigned long.
not sure what they actually mean, but a virtual address should be
a void * and drivers shouldn't ever mess with physical addresses.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
> diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/jsm_driver.h > linux-2.6.9.new/drivers/serial/jsm/jsm_driver.h > --- linux-2.6.9.orig/drivers/serial/jsm/jsm_driver.h1969-12-31 > 18:00:00.0 -0600 > +++ linux-2.6.9.new/drivers/serial/jsm/jsm_driver.h 2005-02-27 > 17:14:44.747952016 -0600 > +#define jsm_jiffies_from_ms(a) (((a) * HZ) / 1000) Please use the existing msecs_to_jiffies(), which has both a more sensible name and is correct. Thanks, Nish - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Sun, 27 Feb 2005, Wen Xiong wrote: This patch is all headers for this device driver. Signed-off-by: Wen Xiong <[EMAIL PROTECTED]> +++ linux-2.6.9.new/drivers/serial/jsm/digi.h 2005-02-27 17:14:44.746952168 -0600 @@ -0,0 +1,416 @@ +/* + * Copyright 2003 Digi International (www.digi.com) + * Scott H Kilau + * Not signed off by the copyright holder. Not sure how much of a problem this is, but something to think about. -- "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it." - Brian W. Kernighan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Wen Xiong wrote:
+struct rw_t {
+ unsigned char rw_req; /* Request type */
+ unsigned char rw_board; /* Host Adapter board number*/
+ unsigned char rw_conc;/* Concentrator number */
+ unsigned char rw_reserved;/* Reserved for expansion */
+ unsigned intrw_addr;/* Address in concentrator */
+ unsigned short rw_size;/* Read/write request length*/
+ unsigned char rw_data[128]; /* Data to read/write */
+};
Invalid naming. Linux kernel style is "struct foo" not "foo_t" nor
"struct foo_t".
Also, "rw_t" is too short and ambiguous.
+
+/***
+ * Shrink Buffer and Board Information definitions and structures.
+
+ /
+ /* Board type return codes */
+#define PCXI_TYPE 1 /* Board type at the designated port is a PC/Xi */
+#define PCXM_TYPE 2 /* Board type at the designated port is a PC/Xm */
+#define PCXE_TYPE 3 /* Board type at the designated port is a PC/Xe */
+#define MCXI_TYPE 4 /* Board type at the designated port is a MC/Xi */
+#define COMXI_TYPE 5 /* Board type at the designated port is a COM/Xi */
+
+ /* Non-Zero Result codes. */
+#define RESULT_NOBDFND 1 /* A Digi product at that port is not config installed */
+#define RESULT_NODESCT 2 /* A memory descriptor was not obtainable */
+#define RESULT_NOOSSIG 3 /* FEP/OS signature was not detected on the board */
+#define RESULT_TOOSML 4 /* Too small an area to shrink. */
+#define RESULT_NOCHAN 5 /* Channel structure for the board was not found */
+
+struct shrink_buf_struct {
+ unsigned int shrink_buf_vaddr; /* Virtual address of board */
+ unsigned int shrink_buf_phys; /* Physical address of board */
Major bug. These should be unsigned long.
+ unsigned int shrink_buf_bseg; /* Amount of board memory */
+ unsigned int shrink_buf_hseg; /* '186 Begining of Dual-Port */
+
+ unsigned int shrink_buf_lseg; /* '186 Begining of freed memory */
+ unsigned int shrink_buf_mseg; /* Linear address from start of
+ dual-port were freed memory
+ begins, host viewpoint. */
+
+ unsigned int shrink_buf_bdparam; /* Parameter for xxmemon and
+ xxmemoff */
+
+ unsigned int shrink_buf_reserva; /* Reserved */
+ unsigned int shrink_buf_reservb; /* Reserved */
+ unsigned int shrink_buf_reservc; /* Reserved */
+ unsigned int shrink_buf_reservd; /* Reserved */
why do these exist?
+ unsigned char shrink_buf_result; /* Reason for call failing
+ Zero is Good return */
+ unsigned char shrink_buf_init; /* Non-Zero if it caused an
+ xxinit call. */
+
+ unsigned char shrink_buf_anports; /* Number of async ports */
+ unsigned char shrink_buf_snports; /* Number of sync ports */
+ unsigned char shrink_buf_type; /* Board type 1 = PC/Xi,
+2 = PC/Xm,
+3 = PC/Xe
+4 = MC/Xi
+5 = COMX/i */
+ unsigned char shrink_buf_card; /* Card number */
+
+};
+
+/
+ * Structure to get driver status information
+ /
+struct digi_dinfo {
+ unsigned int dinfo_nboards; /* # boards configured */
+ char dinfo_reserved[12]; /* for future expansion */
+ char dinfo_version[16]; /* driver version */
+};
+
+#define DIGI_GETDD ('d'<<8) | 248 /* get driver info */
+
+/
+ * Structure used with ioctl commands for per-board information
+ *
+ * physsize and memsize differ when board has "windowed" memory
+ /
+struct digi_info {
+ unsigned int info_bdnum; /* Board number (0 based) */
+ unsigned int info_ioport; /* io port address */
+ unsigned int info_physaddr; /* memory address */
+ unsigned int info_physsize; /* Size of host mem window */
+ unsigned int info_memsize; /* Amount of dual-port mem */
+ /* on board */
Major bugs. Use unsigned long for addresses.
Use "void __iomem *" for ioremap'd addresses.
+
+/*
+ * Driver identification, error and debugging statments
+ *
+ * In theory, you can change all occurances of "digi" in the next
+ * three lines, and the driver printk's will all automagically change.
+ *
+ * APR((fmt, args, ...)); Always prints message
+ * DPR((fmt, args, ...)); Only prints if JSM_TRACER is defined at
+ * compile time and jsm_debug!=0
+ *
+ * TRC_TO_CONSOLE:
+ * Setting this to 1 will turn on some general function tracing
+ * resulting in a bunch of extra debugging printks to the console
+ *
+ */
+
+#definePROCSTR "jsm" /* /proc entries
*/
+#defineDEVSTR "/dev/dg/jsm" /* /dev entries
*/
Never store
[ patch 6/7] drivers/serial/jsm: new serial device driver
This patch is all headers for this device driver.
Signed-off-by: Wen Xiong <[EMAIL PROTECTED]>
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/digi.h
linux-2.6.9.new/drivers/serial/jsm/digi.h
--- linux-2.6.9.orig/drivers/serial/jsm/digi.h 1969-12-31 18:00:00.0
-0600
+++ linux-2.6.9.new/drivers/serial/jsm/digi.h 2005-02-27 17:14:44.746952168
-0600
@@ -0,0 +1,416 @@
+/*
+ * Copyright 2003 Digi International (www.digi.com)
+ * Scott H Kilau
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * $Id: digi.h,v 1.7 2004/09/23 16:08:30 scottk Exp $
+ *
+ * NOTE: THIS IS A SHARED HEADER. DO NOT CHANGE CODING STYLE!!!
+ */
+
+#ifndef __DIGI_H
+#define __DIGI_H
+
+/
+ *** Definitions for Digi ditty(1) command.
+ /
+
+/*
+ * Copyright (c) 1988-96 Digi International Inc., All Rights Reserved.
+ */
+
+/
+ * This module provides application access to special Digi
+ * serial line enhancements which are not standard UNIX(tm) features.
+ /
+
+#if !defined(TIOCMODG)
+
+#defineTIOCMODG('d'<<8) | 250 /* get modem ctrl state
*/
+#defineTIOCMODS('d'<<8) | 251 /* set modem ctrl state
*/
+
+#ifndef TIOCM_LE
+#defineTIOCM_LE0x01/* line enable
*/
+#defineTIOCM_DTR 0x02/* data terminal ready
*/
+#defineTIOCM_RTS 0x04/* request to send
*/
+#defineTIOCM_ST0x08/* secondary transmit
*/
+#defineTIOCM_SR0x10/* secondary receive
*/
+#defineTIOCM_CTS 0x20/* clear to send
*/
+#defineTIOCM_CAR 0x40/* carrier detect
*/
+#defineTIOCM_RNG 0x80/* ring indicator
*/
+#defineTIOCM_DSR 0x100 /* data set ready
*/
+#defineTIOCM_RITIOCM_RNG /* ring (alternate)
*/
+#defineTIOCM_CDTIOCM_CAR /* carrier detect (alt)
*/
+#endif
+
+#endif
+
+#if !defined(TIOCMSET)
+#defineTIOCMSET('d'<<8) | 252 /* set modem ctrl state
*/
+#defineTIOCMGET('d'<<8) | 253 /* set modem ctrl state
*/
+#endif
+
+#if !defined(TIOCMBIC)
+#defineTIOCMBIC('d'<<8) | 254 /* set modem ctrl state
*/
+#defineTIOCMBIS('d'<<8) | 255 /* set modem ctrl state
*/
+#endif
+
+
+#if !defined(TIOCSDTR)
+#defineTIOCSDTR('e'<<8) | 0/* set DTR
*/
+#defineTIOCCDTR('e'<<8) | 1/* clear DTR
*/
+#endif
+
+/
+ * Ioctl command arguments for DIGI parameters.
+ /
+#define DIGI_GETA ('e'<<8) | 94 /* Read params */
+
+#define DIGI_SETA ('e'<<8) | 95 /* Set params */
+#define DIGI_SETAW ('e'<<8) | 96 /* Drain & set params */
+#define DIGI_SETAF ('e'<<8) | 97 /* Drain, flush & set params */
+
+#define DIGI_KME ('e'<<8) | 98 /* Read/Write Host */
+ /* Adapter Memory */
+
+#defineDIGI_GETFLOW('e'<<8) | 99 /* Get startc/stopc
flow */
+ /* control characters*/
+#defineDIGI_SETFLOW('e'<<8) | 100 /* Set startc/stopc
flow */
+ /* control characters*/
+#defineDIGI_GETAFLOW ('e'<<8) | 101 /* Get Aux.
startc/stopc */
+ /* flow control chars*/
+#defineDIGI_SETAFLOW ('e'<<8) | 102 /* Set Aux.
startc/stopc */
+ /* flow control chars*/
+
[ patch 6/7] drivers/serial/jsm: new serial device driver
This patch is all headers for this device driver.
Signed-off-by: Wen Xiong [EMAIL PROTECTED]
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/digi.h
linux-2.6.9.new/drivers/serial/jsm/digi.h
--- linux-2.6.9.orig/drivers/serial/jsm/digi.h 1969-12-31 18:00:00.0
-0600
+++ linux-2.6.9.new/drivers/serial/jsm/digi.h 2005-02-27 17:14:44.746952168
-0600
@@ -0,0 +1,416 @@
+/*
+ * Copyright 2003 Digi International (www.digi.com)
+ * Scott H Kilau Scott_Kilau at digi dot com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY, EXPRESS OR IMPLIED; without even the
+ * implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR
+ * PURPOSE. See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * $Id: digi.h,v 1.7 2004/09/23 16:08:30 scottk Exp $
+ *
+ * NOTE: THIS IS A SHARED HEADER. DO NOT CHANGE CODING STYLE!!!
+ */
+
+#ifndef __DIGI_H
+#define __DIGI_H
+
+/
+ *** Definitions for Digi ditty(1) command.
+ /
+
+/*
+ * Copyright (c) 1988-96 Digi International Inc., All Rights Reserved.
+ */
+
+/
+ * This module provides application access to special Digi
+ * serial line enhancements which are not standard UNIX(tm) features.
+ /
+
+#if !defined(TIOCMODG)
+
+#defineTIOCMODG('d'8) | 250 /* get modem ctrl state
*/
+#defineTIOCMODS('d'8) | 251 /* set modem ctrl state
*/
+
+#ifndef TIOCM_LE
+#defineTIOCM_LE0x01/* line enable
*/
+#defineTIOCM_DTR 0x02/* data terminal ready
*/
+#defineTIOCM_RTS 0x04/* request to send
*/
+#defineTIOCM_ST0x08/* secondary transmit
*/
+#defineTIOCM_SR0x10/* secondary receive
*/
+#defineTIOCM_CTS 0x20/* clear to send
*/
+#defineTIOCM_CAR 0x40/* carrier detect
*/
+#defineTIOCM_RNG 0x80/* ring indicator
*/
+#defineTIOCM_DSR 0x100 /* data set ready
*/
+#defineTIOCM_RITIOCM_RNG /* ring (alternate)
*/
+#defineTIOCM_CDTIOCM_CAR /* carrier detect (alt)
*/
+#endif
+
+#endif
+
+#if !defined(TIOCMSET)
+#defineTIOCMSET('d'8) | 252 /* set modem ctrl state
*/
+#defineTIOCMGET('d'8) | 253 /* set modem ctrl state
*/
+#endif
+
+#if !defined(TIOCMBIC)
+#defineTIOCMBIC('d'8) | 254 /* set modem ctrl state
*/
+#defineTIOCMBIS('d'8) | 255 /* set modem ctrl state
*/
+#endif
+
+
+#if !defined(TIOCSDTR)
+#defineTIOCSDTR('e'8) | 0/* set DTR
*/
+#defineTIOCCDTR('e'8) | 1/* clear DTR
*/
+#endif
+
+/
+ * Ioctl command arguments for DIGI parameters.
+ /
+#define DIGI_GETA ('e'8) | 94 /* Read params */
+
+#define DIGI_SETA ('e'8) | 95 /* Set params */
+#define DIGI_SETAW ('e'8) | 96 /* Drain set params */
+#define DIGI_SETAF ('e'8) | 97 /* Drain, flush set params */
+
+#define DIGI_KME ('e'8) | 98 /* Read/Write Host */
+ /* Adapter Memory */
+
+#defineDIGI_GETFLOW('e'8) | 99 /* Get startc/stopc
flow */
+ /* control characters*/
+#defineDIGI_SETFLOW('e'8) | 100 /* Set startc/stopc
flow */
+ /* control characters*/
+#defineDIGI_GETAFLOW ('e'8) | 101 /* Get Aux.
startc/stopc */
+ /* flow control chars*/
+#defineDIGI_SETAFLOW ('e'8) | 102 /* Set Aux.
startc/stopc */
+ /* flow control chars*/
+
+#define
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
Wen Xiong wrote:
+struct rw_t {
+ unsigned char rw_req; /* Request type */
+ unsigned char rw_board; /* Host Adapter board number*/
+ unsigned char rw_conc;/* Concentrator number */
+ unsigned char rw_reserved;/* Reserved for expansion */
+ unsigned intrw_addr;/* Address in concentrator */
+ unsigned short rw_size;/* Read/write request length*/
+ unsigned char rw_data[128]; /* Data to read/write */
+};
Invalid naming. Linux kernel style is struct foo not foo_t nor
struct foo_t.
Also, rw_t is too short and ambiguous.
+
+/***
+ * Shrink Buffer and Board Information definitions and structures.
+
+ /
+ /* Board type return codes */
+#define PCXI_TYPE 1 /* Board type at the designated port is a PC/Xi */
+#define PCXM_TYPE 2 /* Board type at the designated port is a PC/Xm */
+#define PCXE_TYPE 3 /* Board type at the designated port is a PC/Xe */
+#define MCXI_TYPE 4 /* Board type at the designated port is a MC/Xi */
+#define COMXI_TYPE 5 /* Board type at the designated port is a COM/Xi */
+
+ /* Non-Zero Result codes. */
+#define RESULT_NOBDFND 1 /* A Digi product at that port is not config installed */
+#define RESULT_NODESCT 2 /* A memory descriptor was not obtainable */
+#define RESULT_NOOSSIG 3 /* FEP/OS signature was not detected on the board */
+#define RESULT_TOOSML 4 /* Too small an area to shrink. */
+#define RESULT_NOCHAN 5 /* Channel structure for the board was not found */
+
+struct shrink_buf_struct {
+ unsigned int shrink_buf_vaddr; /* Virtual address of board */
+ unsigned int shrink_buf_phys; /* Physical address of board */
Major bug. These should be unsigned long.
+ unsigned int shrink_buf_bseg; /* Amount of board memory */
+ unsigned int shrink_buf_hseg; /* '186 Begining of Dual-Port */
+
+ unsigned int shrink_buf_lseg; /* '186 Begining of freed memory */
+ unsigned int shrink_buf_mseg; /* Linear address from start of
+ dual-port were freed memory
+ begins, host viewpoint. */
+
+ unsigned int shrink_buf_bdparam; /* Parameter for xxmemon and
+ xxmemoff */
+
+ unsigned int shrink_buf_reserva; /* Reserved */
+ unsigned int shrink_buf_reservb; /* Reserved */
+ unsigned int shrink_buf_reservc; /* Reserved */
+ unsigned int shrink_buf_reservd; /* Reserved */
why do these exist?
+ unsigned char shrink_buf_result; /* Reason for call failing
+ Zero is Good return */
+ unsigned char shrink_buf_init; /* Non-Zero if it caused an
+ xxinit call. */
+
+ unsigned char shrink_buf_anports; /* Number of async ports */
+ unsigned char shrink_buf_snports; /* Number of sync ports */
+ unsigned char shrink_buf_type; /* Board type 1 = PC/Xi,
+2 = PC/Xm,
+3 = PC/Xe
+4 = MC/Xi
+5 = COMX/i */
+ unsigned char shrink_buf_card; /* Card number */
+
+};
+
+/
+ * Structure to get driver status information
+ /
+struct digi_dinfo {
+ unsigned int dinfo_nboards; /* # boards configured */
+ char dinfo_reserved[12]; /* for future expansion */
+ char dinfo_version[16]; /* driver version */
+};
+
+#define DIGI_GETDD ('d'8) | 248 /* get driver info */
+
+/
+ * Structure used with ioctl commands for per-board information
+ *
+ * physsize and memsize differ when board has windowed memory
+ /
+struct digi_info {
+ unsigned int info_bdnum; /* Board number (0 based) */
+ unsigned int info_ioport; /* io port address */
+ unsigned int info_physaddr; /* memory address */
+ unsigned int info_physsize; /* Size of host mem window */
+ unsigned int info_memsize; /* Amount of dual-port mem */
+ /* on board */
Major bugs. Use unsigned long for addresses.
Use void __iomem * for ioremap'd addresses.
+
+/*
+ * Driver identification, error and debugging statments
+ *
+ * In theory, you can change all occurances of digi in the next
+ * three lines, and the driver printk's will all automagically change.
+ *
+ * APR((fmt, args, ...)); Always prints message
+ * DPR((fmt, args, ...)); Only prints if JSM_TRACER is defined at
+ * compile time and jsm_debug!=0
+ *
+ * TRC_TO_CONSOLE:
+ * Setting this to 1 will turn on some general function tracing
+ * resulting in a bunch of extra debugging printks to the console
+ *
+ */
+
+#definePROCSTR jsm /* /proc entries
*/
+#defineDEVSTR /dev/dg/jsm /* /dev entries
*/
Never store pathnames in a kernel
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Sun, 27 Feb 2005, Wen Xiong wrote: This patch is all headers for this device driver. Signed-off-by: Wen Xiong [EMAIL PROTECTED] +++ linux-2.6.9.new/drivers/serial/jsm/digi.h 2005-02-27 17:14:44.746952168 -0600 @@ -0,0 +1,416 @@ +/* + * Copyright 2003 Digi International (www.digi.com) + * Scott H Kilau Scott_Kilau at digi dot com + * Not signed off by the copyright holder. Not sure how much of a problem this is, but something to think about. -- Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. - Brian W. Kernighan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/jsm_driver.h linux-2.6.9.new/drivers/serial/jsm/jsm_driver.h --- linux-2.6.9.orig/drivers/serial/jsm/jsm_driver.h1969-12-31 18:00:00.0 -0600 +++ linux-2.6.9.new/drivers/serial/jsm/jsm_driver.h 2005-02-27 17:14:44.747952016 -0600 snip +#define jsm_jiffies_from_ms(a) (((a) * HZ) / 1000) Please use the existing msecs_to_jiffies(), which has both a more sensible name and is correct. Thanks, Nish - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Sun, Feb 27, 2005 at 07:47:58PM -0500, Jeff Garzik wrote:
+struct shrink_buf_struct {
+unsigned intshrink_buf_vaddr; /* Virtual address of board
*/
+unsigned intshrink_buf_phys;/* Physical address of board
*/
Major bug. These should be unsigned long.
not sure what they actually mean, but a virtual address should be
a void * and drivers shouldn't ever mess with physical addresses.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [ patch 6/7] drivers/serial/jsm: new serial device driver
On Sun, Feb 27, 2005 at 06:40:20PM -0500, Wen Xiong wrote:
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/digi.h
linux-2.6.9.new/drivers/serial/jsm/digi.h
Oh, and please diff against at least the latest kernel release, 2.6.9 is
old...
+ * $Id: digi.h,v 1.7 2004/09/23 16:08:30 scottk Exp $
+ *
+ * NOTE: THIS IS A SHARED HEADER. DO NOT CHANGE CODING STYLE!!!
Not true anymore, right?
+/
+ * Structure to get driver status information
+ /
+struct digi_dinfo {
+ unsigned intdinfo_nboards; /* # boards configured */
+ chardinfo_reserved[12]; /* for future expansion */
+ chardinfo_version[16]; /* driver version */
+};
+
+#define DIGI_GETDD ('d'8) | 248 /* get driver info
*/
All structures that are passed accross the ioctl interface, MUST use the
__u8, __u16, __u32, and friend definitions. unsigned int is not
allowed.
And why have all of these ioctls? Shouldn't most of this stuff be
availble in sysfs instead?
+#ifndef __JSM_DRIVER_H
+#define __JSM_DRIVER_H
+
+#include linux/types.h /* To pick up the varions Linux types */
+#include linux/tty.h /* To pick up the various tty
structs/defines */
+#include linux/serial_core.h
+#include linux/interrupt.h /* For irqreturn_t type */
+#include linux/module.h/* For irqreturn_t type */
That comment is incorrect.
+/*
+ * Driver identification, error and debugging statments
+ *
+ * In theory, you can change all occurances of digi in the next
+ * three lines, and the driver printk's will all automagically change.
+ *
+ * APR((fmt, args, ...));Always prints message
+ * DPR((fmt, args, ...));Only prints if JSM_TRACER is defined at
+ * compile time and jsm_debug!=0
+ *
+ * TRC_TO_CONSOLE:
+ * Setting this to 1 will turn on some general function tracing
+ * resulting in a bunch of extra debugging printks to the console
+ *
+ */
+
+#define PROCSTR jsm /* /proc entries
*/
+#define DEVSTR /dev/dg/jsm /* /dev entries
*/
+#define DRVSTR jsm /* Driver name string
+ * displayed by APR */
+#define APR(args) do {printk(DRVSTR: ); printk args;} while (0)
Ick. You _must_ use a KERN_ level for a printk, this is not allowed.
Please use the dev_printk and helper functions instead. It's not ok to
create new functions like this.
And again, what's with the double (( when you use this macro?
+#if TRC_TO_CONSOLE
+#define PRINTF_TO_CONSOLE(args) { printk(DRVSTR: ); printk args; }
+#else //!defined TRACE_TO_CONSOLE
+#define PRINTF_TO_CONSOLE(args)
+#endif
+
+#define TRC(args) { PRINTF_TO_CONSOLE(args) }
do { } while 0
+/* Our 3 magic numbers for our board, channel and unit structs */
+#define JSM_BOARD_MAGIC 0x5c6df104
+#define JSM_CHANNEL_MAGIC0x6c6df104
+#define JSM_UNIT_MAGIC 0x7c6df104
Don't use magic numbers, they are not needed at all. Please just remove
them from the structures, and use the provided kernel slab debug
functions to catch errors that you might have been able to catch with
the magic values.
+ * This file is intended to contain all the kernel differences between the
+ * various kernels that we support.
No, please use this for your 2.4 code, not for your 2.6 driver version.
+# define JSM_MOD_INC_USE_COUNT(rtn) (rtn = 1)
+# define JSM_MOD_DEC_USE_COUNT
You shouldn't even be using these macros in your 2.4 code, so please
don't use it here.
diff -Nuar linux-2.6.9.orig/drivers/serial/jsm/jsm_mgmt.h
linux-2.6.9.new/drivers/serial/jsm/jsm_mgmt.h
--- linux-2.6.9.orig/drivers/serial/jsm/jsm_mgmt.h1969-12-31
18:00:00.0 -0600
+++ linux-2.6.9.new/drivers/serial/jsm/jsm_mgmt.h 2005-02-27
17:14:44.747952016 -0600
Do you really need all of these different header files? Why not just
put them all in 1?
+/
+ * Per channel/port NEO UART structure
*
+
+ * Base Structure Entries Usage Meanings to Host *
+ * *
+ * W = read write R = read only *
+ * U = Unused. *
+ /
+
+struct neo_uart_struct {
+ volatile uchar txrx;/* WR RHR/THR - Holding Reg */
+ volatile uchar ier; /* WR IER - Interrupt Enable Reg
