Re: [Patch v2 0/2] Move Digi Neo card support from dgnc to jsm
On 09/08/2014 04:17 PM, Greg KH wrote: On Sat, Aug 30, 2014 at 04:35:56PM -0400, wf...@worldbroken.com wrote: From: Bill Pemberton The jsm driver (drivers/tty/serial/jsm) already supports the Digi Neo cards that dgnc (staging) supports. In fact, it appears that jsm was based on dgnc. The dgnc driver has PCI ids for more cards than jsm, this patch moves support for all the Neo cards to jsm. The result of this will be that dgnc will only support the Classic cards since the jsm driver doesn't support those cards yet. Bill Pemberton (2): jsm: add support for additional Neo cards staging: dgnc: remove Neo card ids from device table drivers/staging/dgnc/dgnc_driver.c | 13 - drivers/tty/serial/jsm/jsm.h| 10 ++ drivers/tty/serial/jsm/jsm_driver.c | 38 + 3 files changed, 44 insertions(+), 17 deletions(-) These are now applied, thanks. Do you think we can delete the dgnc driver now, or are there still some odd cards out there that it only supports? What would it take to move that support to the jsm driver instead? The jsm driver still doesn't support the Classic cards, just the Neo cards so dgnc would still be needed to support those cards. I think the right thing would be to put the Classic support into jsm. I've started looking at that. Dgnc and jsm are obviously from the same base, so dgnc can be used as guide on what to do. However, I don't have a Classic card so I wouldn't be able to test any of the Classic support. -- Bill -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Patch v2 0/2] Move Digi Neo card support from dgnc to jsm
On 09/08/2014 04:17 PM, Greg KH wrote: On Sat, Aug 30, 2014 at 04:35:56PM -0400, wf...@worldbroken.com wrote: From: Bill Pemberton wf...@worldbroken.com The jsm driver (drivers/tty/serial/jsm) already supports the Digi Neo cards that dgnc (staging) supports. In fact, it appears that jsm was based on dgnc. The dgnc driver has PCI ids for more cards than jsm, this patch moves support for all the Neo cards to jsm. The result of this will be that dgnc will only support the Classic cards since the jsm driver doesn't support those cards yet. Bill Pemberton (2): jsm: add support for additional Neo cards staging: dgnc: remove Neo card ids from device table drivers/staging/dgnc/dgnc_driver.c | 13 - drivers/tty/serial/jsm/jsm.h| 10 ++ drivers/tty/serial/jsm/jsm_driver.c | 38 + 3 files changed, 44 insertions(+), 17 deletions(-) These are now applied, thanks. Do you think we can delete the dgnc driver now, or are there still some odd cards out there that it only supports? What would it take to move that support to the jsm driver instead? The jsm driver still doesn't support the Classic cards, just the Neo cards so dgnc would still be needed to support those cards. I think the right thing would be to put the Classic support into jsm. I've started looking at that. Dgnc and jsm are obviously from the same base, so dgnc can be used as guide on what to do. However, I don't have a Classic card so I wouldn't be able to test any of the Classic support. -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] jsm: add support for additional Neo cards
On 08/30/2014 02:59 PM, Greg KH wrote:
On Sat, Aug 30, 2014 at 02:54:36PM -0400, wf...@worldbroken.com wrote:
From: Bill Pemberton
Add device ids for additional Neo cards. The ids come from the dgnc
driver.
Signed-off-by: Bill Pemberton
---
drivers/tty/serial/jsm/jsm_driver.c | 38 +
include/linux/pci_ids.h | 8
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/jsm/jsm_driver.c
b/drivers/tty/serial/jsm/jsm_driver.c
index a47d882d6743..d2885a7bb090 100644
--- a/drivers/tty/serial/jsm/jsm_driver.c
+++ b/drivers/tty/serial/jsm/jsm_driver.c
@@ -93,12 +93,34 @@ static int jsm_probe_one(struct pci_dev *pdev, const struct
pci_device_id *ent)
/* store the info for the board we've found */
brd->boardnum = adapter_count++;
brd->pci_dev = pdev;
- if (pdev->device == PCIE_DEVICE_ID_NEO_4_IBM)
+
+ switch (pdev->device) {
+
+ case PCI_DEVICE_ID_NEO_2DB9:
+ case PCI_DEVICE_ID_NEO_2DB9PRI:
+ case PCI_DEVICE_ID_NEO_2RJ45:
+ case PCI_DEVICE_ID_NEO_2RJ45PRI:
+ case PCI_DEVICE_ID_NEO_2_422_485:
+ brd->maxports = 2;
+ break;
+
+ case PCI_DEVICE_ID_NEO_4:
+ case PCIE_DEVICE_ID_NEO_4:
+ case PCIE_DEVICE_ID_NEO_4RJ45:
+ case PCIE_DEVICE_ID_NEO_4_IBM:
brd->maxports = 4;
- else if (pdev->device == PCI_DEVICE_ID_DIGI_NEO_8)
+ break;
+
+ case PCI_DEVICE_ID_DIGI_NEO_8:
+ case PCIE_DEVICE_ID_NEO_8:
+ case PCIE_DEVICE_ID_NEO_8RJ45:
brd->maxports = 8;
- else
- brd->maxports = 2;
+ break;
+
+ default:
+ brd->maxports = 1;
+ break;
+ }
spin_lock_init(>bd_intr_lock);
@@ -209,6 +231,14 @@ static struct pci_device_id jsm_pci_tbl[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45PRI), 0, 0, 3 },
{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4_IBM), 0, 0, 4 },
{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_DIGI_NEO_8), 0, 0, 5 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_4), 0, 0, 6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422), 0, 0, 7 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422_485), 0, 0, 8
},
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2_422_485), 0, 0, 9
},
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8), 0, 0, 10 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4), 0, 0, 11 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4RJ45), 0, 0, 12 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8RJ45), 0, 0, 13 },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, jsm_pci_tbl);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 6ed0bb73a864..f0f610c94bb2 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1516,11 +1516,19 @@
#define PCI_DEVICE_ID_DIGI_DF_M_E 0x0071
#define PCI_DEVICE_ID_DIGI_DF_M_IOM2_A0x0072
#define PCI_DEVICE_ID_DIGI_DF_M_A 0x0073
+#define PCI_DEVICE_ID_NEO_4 0x00B0
#define PCI_DEVICE_ID_DIGI_NEO_8 0x00B1
#define PCI_DEVICE_ID_NEO_2DB9 0x00C8
#define PCI_DEVICE_ID_NEO_2DB9PRI 0x00C9
#define PCI_DEVICE_ID_NEO_2RJ45 0x00CA
#define PCI_DEVICE_ID_NEO_2RJ45PRI 0x00CB
+#define PCI_DEVICE_ID_NEO_1_422 0x00CC
+#define PCI_DEVICE_ID_NEO_1_422_485 0x00CD
+#define PCI_DEVICE_ID_NEO_2_422_485 0x00CE
+#define PCIE_DEVICE_ID_NEO_80x00F0
+#define PCIE_DEVICE_ID_NEO_40x00F1
+#define PCIE_DEVICE_ID_NEO_4RJ450x00F2
+#define PCIE_DEVICE_ID_NEO_8RJ450x00F3
#define PCIE_DEVICE_ID_NEO_4_IBM0x00F4
As per the information in the top of this file, we only add new ids when
they cross multiple files. Right now these values you are adding are
only for a single driver, so they don't need to be added here.
Can you redo this series without touching the pci_ids.h file?
Ah, my apologies, obviously I didn't read the top of the pci_ids.h.
I'll redo these.
--
Bill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] jsm: add support for additional Neo cards
On 08/30/2014 02:59 PM, Greg KH wrote:
On Sat, Aug 30, 2014 at 02:54:36PM -0400, wf...@worldbroken.com wrote:
From: Bill Pemberton wf...@worldbroken.com
Add device ids for additional Neo cards. The ids come from the dgnc
driver.
Signed-off-by: Bill Pemberton wf...@worldbroken.com
---
drivers/tty/serial/jsm/jsm_driver.c | 38 +
include/linux/pci_ids.h | 8
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/jsm/jsm_driver.c
b/drivers/tty/serial/jsm/jsm_driver.c
index a47d882d6743..d2885a7bb090 100644
--- a/drivers/tty/serial/jsm/jsm_driver.c
+++ b/drivers/tty/serial/jsm/jsm_driver.c
@@ -93,12 +93,34 @@ static int jsm_probe_one(struct pci_dev *pdev, const struct
pci_device_id *ent)
/* store the info for the board we've found */
brd-boardnum = adapter_count++;
brd-pci_dev = pdev;
- if (pdev-device == PCIE_DEVICE_ID_NEO_4_IBM)
+
+ switch (pdev-device) {
+
+ case PCI_DEVICE_ID_NEO_2DB9:
+ case PCI_DEVICE_ID_NEO_2DB9PRI:
+ case PCI_DEVICE_ID_NEO_2RJ45:
+ case PCI_DEVICE_ID_NEO_2RJ45PRI:
+ case PCI_DEVICE_ID_NEO_2_422_485:
+ brd-maxports = 2;
+ break;
+
+ case PCI_DEVICE_ID_NEO_4:
+ case PCIE_DEVICE_ID_NEO_4:
+ case PCIE_DEVICE_ID_NEO_4RJ45:
+ case PCIE_DEVICE_ID_NEO_4_IBM:
brd-maxports = 4;
- else if (pdev-device == PCI_DEVICE_ID_DIGI_NEO_8)
+ break;
+
+ case PCI_DEVICE_ID_DIGI_NEO_8:
+ case PCIE_DEVICE_ID_NEO_8:
+ case PCIE_DEVICE_ID_NEO_8RJ45:
brd-maxports = 8;
- else
- brd-maxports = 2;
+ break;
+
+ default:
+ brd-maxports = 1;
+ break;
+ }
spin_lock_init(brd-bd_intr_lock);
@@ -209,6 +231,14 @@ static struct pci_device_id jsm_pci_tbl[] = {
{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2RJ45PRI), 0, 0, 3 },
{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4_IBM), 0, 0, 4 },
{ PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_DIGI_NEO_8), 0, 0, 5 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_4), 0, 0, 6 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422), 0, 0, 7 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_1_422_485), 0, 0, 8
},
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCI_DEVICE_ID_NEO_2_422_485), 0, 0, 9
},
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8), 0, 0, 10 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4), 0, 0, 11 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_4RJ45), 0, 0, 12 },
+ { PCI_DEVICE(PCI_VENDOR_ID_DIGI, PCIE_DEVICE_ID_NEO_8RJ45), 0, 0, 13 },
{ 0, }
};
MODULE_DEVICE_TABLE(pci, jsm_pci_tbl);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 6ed0bb73a864..f0f610c94bb2 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -1516,11 +1516,19 @@
#define PCI_DEVICE_ID_DIGI_DF_M_E 0x0071
#define PCI_DEVICE_ID_DIGI_DF_M_IOM2_A0x0072
#define PCI_DEVICE_ID_DIGI_DF_M_A 0x0073
+#define PCI_DEVICE_ID_NEO_4 0x00B0
#define PCI_DEVICE_ID_DIGI_NEO_8 0x00B1
#define PCI_DEVICE_ID_NEO_2DB9 0x00C8
#define PCI_DEVICE_ID_NEO_2DB9PRI 0x00C9
#define PCI_DEVICE_ID_NEO_2RJ45 0x00CA
#define PCI_DEVICE_ID_NEO_2RJ45PRI 0x00CB
+#define PCI_DEVICE_ID_NEO_1_422 0x00CC
+#define PCI_DEVICE_ID_NEO_1_422_485 0x00CD
+#define PCI_DEVICE_ID_NEO_2_422_485 0x00CE
+#define PCIE_DEVICE_ID_NEO_80x00F0
+#define PCIE_DEVICE_ID_NEO_40x00F1
+#define PCIE_DEVICE_ID_NEO_4RJ450x00F2
+#define PCIE_DEVICE_ID_NEO_8RJ450x00F3
#define PCIE_DEVICE_ID_NEO_4_IBM0x00F4
As per the information in the top of this file, we only add new ids when
they cross multiple files. Right now these values you are adding are
only for a single driver, so they don't need to be added here.
Can you redo this series without touching the pci_ids.h file?
Ah, my apologies, obviously I didn't read the top of the pci_ids.h.
I'll redo these.
--
Bill
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the tty tree
Jiri Slaby writes: > > On 03/20/2013 03:42 PM, Bill Pemberton wrote: > > Ok, for the unopened ports there *should* never be any actual data to > > push so the push is really doing nothing anyhow in these cases. It's > > coming from the device sending an initial change port command. > > > > Anyhow, so my patch adding more is_open logic can be dropped and then > > yours will apply fine. What's the best way for me to handle this? > > Send a revert for my patch so yours will apply or send an updated > > version of your patch that removes my additions? > > Asking Greg to revert should suffice. I commented on that patch, but in > a different thread, so Greg missed the comment the patch is not needed > IIRC. What was the title of the patch, I cannot find it immediately :/? > "USB: quatech2: only write to the tty if the port is open." (commit 27b351c in v3.9-rc3). -- Bill -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the tty tree
Jiri Slaby writes:
>
> On 03/20/2013 01:51 PM, Bill Pemberton wrote:
> > Jiri Slaby writes:
> >>
> >>> drivers/usb/serial/quatech2.c: In function 'qt2_process_read_urb':
> >>> drivers/usb/serial/quatech2.c:661:18: error: 'struct
> >>> qt2_port_private' has no member named 'is_open'
> >>> drivers/usb/serial/quatech2.c:705:15: error: 'struct
> >>> qt2_port_private' has no member named 'is_open'
> >>>
> >>> Caused by commit e4408ce3c23f ("TTY: quatech2, remove unneeded
> >>> is_open"). grep is your friend (or searching while editting the
> >>> file).
> >>
> >> I think this is caused by another commit. Bill added a check for
> >> is_open meanwhile I removed that member completely. Bill, could you
> >> fix that up, please?
> >>
> >
> > Is it legal to call tty_flip_buffer_push() without an open tty? If
> > so, I can get rid of the is_open logic completely. Otherwise, I need
> > to either keep is_open or otherwise check for a valid tty before
> > calling tty_flip_buffer_push().
>
> Yeah, it is legal to call that, as long as you have a valid tty_port.
> Now, the data are queued and later flushed. The plan is to throw the
> data directly on ingress in a way the drivers don't need to care.
>
Ok, for the unopened ports there *should* never be any actual data to
push so the push is really doing nothing anyhow in these cases. It's
coming from the device sending an initial change port command.
Anyhow, so my patch adding more is_open logic can be dropped and then
yours will apply fine. What's the best way for me to handle this?
Send a revert for my patch so yours will apply or send an updated
version of your patch that removes my additions?
--
Bill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the tty tree
Jiri Slaby writes:
>
> > drivers/usb/serial/quatech2.c: In function 'qt2_process_read_urb':
> > drivers/usb/serial/quatech2.c:661:18: error: 'struct
> > qt2_port_private' has no member named 'is_open'
> > drivers/usb/serial/quatech2.c:705:15: error: 'struct
> > qt2_port_private' has no member named 'is_open'
> >
> > Caused by commit e4408ce3c23f ("TTY: quatech2, remove unneeded
> > is_open"). grep is your friend (or searching while editting the
> > file).
>
> I think this is caused by another commit. Bill added a check for
> is_open meanwhile I removed that member completely. Bill, could you
> fix that up, please?
>
Is it legal to call tty_flip_buffer_push() without an open tty? If
so, I can get rid of the is_open logic completely. Otherwise, I need
to either keep is_open or otherwise check for a valid tty before
calling tty_flip_buffer_push().
--
Bill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the tty tree
Jiri Slaby writes: drivers/usb/serial/quatech2.c: In function 'qt2_process_read_urb': drivers/usb/serial/quatech2.c:661:18: error: 'struct qt2_port_private' has no member named 'is_open' drivers/usb/serial/quatech2.c:705:15: error: 'struct qt2_port_private' has no member named 'is_open' Caused by commit e4408ce3c23f (TTY: quatech2, remove unneeded is_open). grep is your friend (or searching while editting the file). I think this is caused by another commit. Bill added a check for is_open meanwhile I removed that member completely. Bill, could you fix that up, please? Is it legal to call tty_flip_buffer_push() without an open tty? If so, I can get rid of the is_open logic completely. Otherwise, I need to either keep is_open or otherwise check for a valid tty before calling tty_flip_buffer_push(). -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the tty tree
Jiri Slaby writes: On 03/20/2013 01:51 PM, Bill Pemberton wrote: Jiri Slaby writes: drivers/usb/serial/quatech2.c: In function 'qt2_process_read_urb': drivers/usb/serial/quatech2.c:661:18: error: 'struct qt2_port_private' has no member named 'is_open' drivers/usb/serial/quatech2.c:705:15: error: 'struct qt2_port_private' has no member named 'is_open' Caused by commit e4408ce3c23f (TTY: quatech2, remove unneeded is_open). grep is your friend (or searching while editting the file). I think this is caused by another commit. Bill added a check for is_open meanwhile I removed that member completely. Bill, could you fix that up, please? Is it legal to call tty_flip_buffer_push() without an open tty? If so, I can get rid of the is_open logic completely. Otherwise, I need to either keep is_open or otherwise check for a valid tty before calling tty_flip_buffer_push(). Yeah, it is legal to call that, as long as you have a valid tty_port. Now, the data are queued and later flushed. The plan is to throw the data directly on ingress in a way the drivers don't need to care. Ok, for the unopened ports there *should* never be any actual data to push so the push is really doing nothing anyhow in these cases. It's coming from the device sending an initial change port command. Anyhow, so my patch adding more is_open logic can be dropped and then yours will apply fine. What's the best way for me to handle this? Send a revert for my patch so yours will apply or send an updated version of your patch that removes my additions? -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: build failure after merge of the tty tree
Jiri Slaby writes: On 03/20/2013 03:42 PM, Bill Pemberton wrote: Ok, for the unopened ports there *should* never be any actual data to push so the push is really doing nothing anyhow in these cases. It's coming from the device sending an initial change port command. Anyhow, so my patch adding more is_open logic can be dropped and then yours will apply fine. What's the best way for me to handle this? Send a revert for my patch so yours will apply or send an updated version of your patch that removes my additions? Asking Greg to revert should suffice. I commented on that patch, but in a different thread, so Greg missed the comment the patch is not needed IIRC. What was the title of the patch, I cannot find it immediately :/? USB: quatech2: only write to the tty if the port is open. (commit 27b351c in v3.9-rc3). -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/16] TTY: quatech2, remove unneeded is_open
Jiri Slaby writes: > > On 03/13/2013 02:46 PM, Bill Pemberton wrote: > > Jiri Slaby writes: > >> > >> tty->ops->break_ctl cannot be called outside the gap between open and > >> close. So there is no need to check whether the port is open in > >> break_ctl in quatech2. Remove the check and also that member > >> completely. > >> > > > > We can't get rid of is_open. The devices use 1 read urb for all ports > > and will send various things about ports that haven't actually been > > opened. So the driver needs to know if a port has actually been > > opened or not. In fact, I was about to send a patch that fixes a > > warning caused by commit 2e124b4a390ca85325fae75764bef92f0547fa25 > > causing the driver to try to write to ttys that weren't actually > > opened. > > As long as tty_port exists for the port, calling tty buffer functions is > OK. The warning you mention is now bogus and there is a patch flying > around to disable that at the moment. > Ah, ok, I assumed the warning was telling me the driver was doing something stupid by calling tty_flip_buffer_push() on a port that wasn't opened (which did sound like a stupid thing to do to me). If that's actually harmless, then yes, the is_open stuff can be dropped and my recent patch to check is_open before calling tty_flip_buffer_push() can be ignored. > It is also that is_open was completely racy, right? > Does it simply need a lock around it or is there something else I'm missing? In any event, if it can go, that's great -- it's only used for the above "don't call tty_flip_buffer_push() on an unopened port" logic. -- Bill -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/16] TTY: quatech2, remove unneeded is_open
Jiri Slaby writes: > > tty->ops->break_ctl cannot be called outside the gap between open and > close. So there is no need to check whether the port is open in > break_ctl in quatech2. Remove the check and also that member > completely. > We can't get rid of is_open. The devices use 1 read urb for all ports and will send various things about ports that haven't actually been opened. So the driver needs to know if a port has actually been opened or not. In fact, I was about to send a patch that fixes a warning caused by commit 2e124b4a390ca85325fae75764bef92f0547fa25 causing the driver to try to write to ttys that weren't actually opened. The guard in qt2_break_ctl() can still be removed. -- Bill -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/16] TTY: quatech2, remove unneeded is_open
Jiri Slaby writes: tty-ops-break_ctl cannot be called outside the gap between open and close. So there is no need to check whether the port is open in break_ctl in quatech2. Remove the check and also that member completely. We can't get rid of is_open. The devices use 1 read urb for all ports and will send various things about ports that haven't actually been opened. So the driver needs to know if a port has actually been opened or not. In fact, I was about to send a patch that fixes a warning caused by commit 2e124b4a390ca85325fae75764bef92f0547fa25 causing the driver to try to write to ttys that weren't actually opened. The guard in qt2_break_ctl() can still be removed. -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 07/16] TTY: quatech2, remove unneeded is_open
Jiri Slaby writes: On 03/13/2013 02:46 PM, Bill Pemberton wrote: Jiri Slaby writes: tty-ops-break_ctl cannot be called outside the gap between open and close. So there is no need to check whether the port is open in break_ctl in quatech2. Remove the check and also that member completely. We can't get rid of is_open. The devices use 1 read urb for all ports and will send various things about ports that haven't actually been opened. So the driver needs to know if a port has actually been opened or not. In fact, I was about to send a patch that fixes a warning caused by commit 2e124b4a390ca85325fae75764bef92f0547fa25 causing the driver to try to write to ttys that weren't actually opened. As long as tty_port exists for the port, calling tty buffer functions is OK. The warning you mention is now bogus and there is a patch flying around to disable that at the moment. Ah, ok, I assumed the warning was telling me the driver was doing something stupid by calling tty_flip_buffer_push() on a port that wasn't opened (which did sound like a stupid thing to do to me). If that's actually harmless, then yes, the is_open stuff can be dropped and my recent patch to check is_open before calling tty_flip_buffer_push() can be ignored. It is also that is_open was completely racy, right? Does it simply need a lock around it or is there something else I'm missing? In any event, if it can go, that's great -- it's only used for the above don't call tty_flip_buffer_push() on an unopened port logic. -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel
H. Peter Anvin writes: > > On 11/29/2012 10:43 AM, Greg Kroah-Hartman wrote: > > From: Greg Kroah-Hartman > > > > With the recent work to remove CONFIG_HOTPLUG, we are starting to get a > > bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being > > enabled. So, stop marking the sections entirely, by defining them away > > the section markings in init.h > > > > Could we run a sed script over the kernel tree and generate patches to > remove the markings completely? > That's how I've been generating the patches to remove the markings -- except it's a perl script. -- Bill -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] init.h: Remove __dev* sections from the kernel
H. Peter Anvin writes: On 11/29/2012 10:43 AM, Greg Kroah-Hartman wrote: From: Greg Kroah-Hartman gre...@linuxfoundation.org With the recent work to remove CONFIG_HOTPLUG, we are starting to get a bunch of __devinit section warnings, despite CONFIG_HOTPLUG always being enabled. So, stop marking the sections entirely, by defining them away the section markings in init.h Could we run a sed script over the kernel tree and generate patches to remove the markings completely? That's how I've been generating the patches to remove the markings -- except it's a perl script. -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: new warnings from partial __devexit removal
gre...@linuxfoundation.org writes: > > On Mon, Nov 26, 2012 at 08:38:32PM +, Arnd Bergmann wrote: > > This is probably known and fixed already, but in case it's not, let me just > > mention that I saw two new warnings with ARM allyesconfig about a > > __devexit being removed but the __devexit_p() still being there: > > > > drivers/tty/serial/xilinx_uartps.c:1000:12: error: 'xuartps_remove' defined > > but not used [-Werror=unused-function] > > drivers/usb/gadget/mv_udc_core.c:2131:12: error: 'mv_udc_remove' defined > > but not used [-Werror=unused-function] > > What platform has -Werror=unused-function set? > > Bill, care to send follow-on patches to resolve these warnings? > I don't understand how my changes would causing the warning. Both functions are put in the platform_driver struct using __exit_p, not __devexit_p. It seems like you should get the warning with or without my changes as long as MODULE is not defined. -- Bill -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: new warnings from partial __devexit removal
gre...@linuxfoundation.org writes: On Mon, Nov 26, 2012 at 08:38:32PM +, Arnd Bergmann wrote: This is probably known and fixed already, but in case it's not, let me just mention that I saw two new warnings with ARM allyesconfig about a __devexit being removed but the __devexit_p() still being there: drivers/tty/serial/xilinx_uartps.c:1000:12: error: 'xuartps_remove' defined but not used [-Werror=unused-function] drivers/usb/gadget/mv_udc_core.c:2131:12: error: 'mv_udc_remove' defined but not used [-Werror=unused-function] What platform has -Werror=unused-function set? Bill, care to send follow-on patches to resolve these warnings? I don't understand how my changes would causing the warning. Both functions are put in the platform_driver struct using __exit_p, not __devexit_p. It seems like you should get the warning with or without my changes as long as MODULE is not defined. -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 000/493] remove CONFIG_HOTPLUG as an option
Grant Likely writes:
>
> You mean this series wasn't created with a script? You did this by
> hand? If so then I must say kudos on your dedication!
>
> But it makes me more nervous about the series. Too easy to fat
> finger many things when touching that many files.
>
No, I didn't do them by hand, it was a script. Originally, it was a
couple, all basically the same, but removing each __dev*. Then I'd do
a word diff to eyeball them to make sure the script didn't do
something goofy.
The whack-a-mole part came along because I was working against
linux-next and whatever patch series was right for one day wouldn't be
right for the next day because of some of the faster moving trees.
> Please do write a script and post that for review.
>
The all-in-one version of the script:
#! /usr/bin/perl
use strict;
use IO::InSitu;
sub processfile
{
my $fn = shift;
my ($in, $out) = open_rw($fn, $fn);
while (<$in>) {
s|__devexit_p\(([^)]+)\)|$1|;
s|\s__devexit\b||;
s|\s__devinitconst\b||;
s|\s__devinitdata\b||;
s|\s__devinit\b||;
print $out $_;
}
close $in;
close $out;
}
foreach my $fn (@ARGV) {
processfile($fn);
}
--
Bill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 000/493] remove CONFIG_HOTPLUG as an option
Grant Likely writes:
You mean this series wasn't created with a script? You did this by
hand? If so then I must say kudos on your dedication!
But it makes me more nervous about the series. Too easy to fat
finger many things when touching that many files.
No, I didn't do them by hand, it was a script. Originally, it was a
couple, all basically the same, but removing each __dev*. Then I'd do
a word diff to eyeball them to make sure the script didn't do
something goofy.
The whack-a-mole part came along because I was working against
linux-next and whatever patch series was right for one day wouldn't be
right for the next day because of some of the faster moving trees.
Please do write a script and post that for review.
The all-in-one version of the script:
#! /usr/bin/perl
use strict;
use IO::InSitu;
sub processfile
{
my $fn = shift;
my ($in, $out) = open_rw($fn, $fn);
while ($in) {
s|__devexit_p\(([^)]+)\)|$1|;
s|\s__devexit\b||;
s|\s__devinitconst\b||;
s|\s__devinitdata\b||;
s|\s__devinit\b||;
print $out $_;
}
close $in;
close $out;
}
foreach my $fn (@ARGV) {
processfile($fn);
}
--
Bill
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 000/493] remove CONFIG_HOTPLUG as an option
Andrew Morton writes: > > On Tue, 20 Nov 2012 10:46:11 + Grant Likely > wrote: > > > On Sat, Nov 17, 2012 at 12:19 AM, Bill Pemberton wrote: > > > CONFIG_HOTPLUG is no longer an optional setting. In order to remove > > > it as on option code paths that check CONFIG_HOTPLUG will removed > > > along with the attributes __devexit_p, __devexit, __devinitconst, and > > > __devinitdata. > > > > > > I'll save the list from the mailbomb of this huge patchset. The > > > patches themselves are going to Greg KH for the driver core tree. > > > > > > > > > Bill Pemberton (493): > > [...] > > > 2942 files changed, 11645 insertions(+), 12116 deletions(-) > > > > So, I've got no problem with the reason for the change and I don't > > even think you need my ack for the bits that I maintain (though you > > have it if you want it). However, this looks like it is going to be > > /painful/. First of all it will touch a huge number of files in the > > tree. Yes the change is trivial, but it will require manual fixups on > > a lot of patches. > > Yeah, this is dopey. Send the script to Linus and ask him to run it > seven seconds before he releases -rc1, when everyone's trees are > empty(ish). Or send him a single megapatch at that time. > I like the script idea for removing all the __dev markings. Creating the patches in the first place was a game of whack-a-mole as various trees changed. -- Bill -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 000/493] remove CONFIG_HOTPLUG as an option
Andrew Morton writes: On Tue, 20 Nov 2012 10:46:11 + Grant Likely grant.lik...@secretlab.ca wrote: On Sat, Nov 17, 2012 at 12:19 AM, Bill Pemberton wf...@virginia.edu wrote: CONFIG_HOTPLUG is no longer an optional setting. In order to remove it as on option code paths that check CONFIG_HOTPLUG will removed along with the attributes __devexit_p, __devexit, __devinitconst, and __devinitdata. I'll save the list from the mailbomb of this huge patchset. The patches themselves are going to Greg KH for the driver core tree. Bill Pemberton (493): [...] 2942 files changed, 11645 insertions(+), 12116 deletions(-) So, I've got no problem with the reason for the change and I don't even think you need my ack for the bits that I maintain (though you have it if you want it). However, this looks like it is going to be /painful/. First of all it will touch a huge number of files in the tree. Yes the change is trivial, but it will require manual fixups on a lot of patches. Yeah, this is dopey. Send the script to Linus and ask him to run it seven seconds before he releases -rc1, when everyone's trees are empty(ish). Or send him a single megapatch at that time. I like the script idea for removing all the __dev markings. Creating the patches in the first place was a game of whack-a-mole as various trees changed. -- Bill -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
