* [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier) @ 2007-02-23 20:42 Mike Miller (OS Dev) 2007-02-23 21:32 ` Greg KH 2007-02-27 20:26 ` Andrew Morton 0 siblings, 2 replies; 5+ messages in thread From: Mike Miller (OS Dev) @ 2007-02-23 20:42 UTC (permalink / raw) To: akpm, jens.axboe; +Cc: linux-kernel, linux-scsi, gregkh Patch 2/2 This patch supercedes yesterdays cciss-shutdown patch. The primary difference is removing __devexit from cciss_remove_one. Instead of create another function I'd rather use the code that was intended to perform the cleanup and cache flush. I've tested as a loadable module and statically linked without error. Please consider this for inclusion. Signed-off-by: Mike Miller <mike.miller@hp.com> ------------------------------------------------------------------------------------------ diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c index 9d84ab3..b16f48c 100644 --- a/drivers/block/cciss.c +++ b/drivers/block/cciss.c @@ -3404,7 +3404,7 @@ #endif return -1; } -static void __devexit cciss_remove_one(struct pci_dev *pdev) +static void cciss_remove_one(struct pci_dev *pdev) { ctlr_info_t *tmp_ptr; int i, j; @@ -3428,9 +3428,10 @@ static void __devexit cciss_remove_one(s memset(flush_buf, 0, 4); return_code = sendcmd(CCISS_CACHE_FLUSH, i, flush_buf, 4, 0, 0, 0, NULL, TYPE_CMD); - if (return_code != IO_OK) { - printk(KERN_WARNING "Error Flushing cache on controller %d\n", - i); + if (return_code == IO_OK) { + printk(KERN_WARNING "Completed flushing cache on controller %d\n", i); + } else { + printk(KERN_WARNING "Error flushing cache on controller %d\n", i); } free_irq(hba[i]->intr[2], hba[i]); @@ -3481,6 +3482,7 @@ static struct pci_driver cciss_pci_drive .probe = cciss_init_one, .remove = __devexit_p(cciss_remove_one), .id_table = cciss_pci_device_id, /* id_table */ + .shutdown = cciss_remove_one, }; /* ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier) 2007-02-23 20:42 [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier) Mike Miller (OS Dev) @ 2007-02-23 21:32 ` Greg KH 2007-02-23 21:44 ` Mike Miller (OS Dev) 2007-02-27 20:26 ` Andrew Morton 1 sibling, 1 reply; 5+ messages in thread From: Greg KH @ 2007-02-23 21:32 UTC (permalink / raw) To: Mike Miller (OS Dev); +Cc: akpm, jens.axboe, linux-kernel, linux-scsi On Fri, Feb 23, 2007 at 02:42:39PM -0600, Mike Miller (OS Dev) wrote: > Patch 2/2 > > This patch supercedes yesterdays cciss-shutdown patch. The primary difference is > removing __devexit from cciss_remove_one. Instead of create another function I'd > rather use the code that was intended to perform the cleanup and cache flush. I've > tested as a loadable module and statically linked without error. > Please consider this for inclusion. > > Signed-off-by: Mike Miller <mike.miller@hp.com> > ------------------------------------------------------------------------------------------ > diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c > index 9d84ab3..b16f48c 100644 > --- a/drivers/block/cciss.c > +++ b/drivers/block/cciss.c > @@ -3404,7 +3404,7 @@ #endif > return -1; > } > > -static void __devexit cciss_remove_one(struct pci_dev *pdev) > +static void cciss_remove_one(struct pci_dev *pdev) > { > ctlr_info_t *tmp_ptr; > int i, j; > @@ -3428,9 +3428,10 @@ static void __devexit cciss_remove_one(s > memset(flush_buf, 0, 4); > return_code = sendcmd(CCISS_CACHE_FLUSH, i, flush_buf, 4, 0, 0, 0, NULL, > TYPE_CMD); > - if (return_code != IO_OK) { > - printk(KERN_WARNING "Error Flushing cache on controller %d\n", > - i); > + if (return_code == IO_OK) { > + printk(KERN_WARNING "Completed flushing cache on controller %d\n", i); Why do you want the world to know this on every shutdown? At the least, use dev_warn() to show the proper device and driver that this is happening to, but does it really need to go to the warning log? It sounds like a normal operation to me. How about dev_dbg() instead? thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier) 2007-02-23 21:32 ` Greg KH @ 2007-02-23 21:44 ` Mike Miller (OS Dev) 0 siblings, 0 replies; 5+ messages in thread From: Mike Miller (OS Dev) @ 2007-02-23 21:44 UTC (permalink / raw) To: Greg KH; +Cc: akpm, jens.axboe, linux-kernel, linux-scsi On Fri, Feb 23, 2007 at 01:32:36PM -0800, Greg KH wrote: > On Fri, Feb 23, 2007 at 02:42:39PM -0600, Mike Miller (OS Dev) wrote: > > Patch 2/2 > > > > This patch supercedes yesterdays cciss-shutdown patch. The primary difference is > > removing __devexit from cciss_remove_one. Instead of create another function I'd > > rather use the code that was intended to perform the cleanup and cache flush. I've > > tested as a loadable module and statically linked without error. > > Please consider this for inclusion. > > > > Signed-off-by: Mike Miller <mike.miller@hp.com> > > ------------------------------------------------------------------------------------------ > > diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c > > index 9d84ab3..b16f48c 100644 > > --- a/drivers/block/cciss.c > > +++ b/drivers/block/cciss.c > > @@ -3404,7 +3404,7 @@ #endif > > return -1; > > } > > > > -static void __devexit cciss_remove_one(struct pci_dev *pdev) > > +static void cciss_remove_one(struct pci_dev *pdev) > > { > > ctlr_info_t *tmp_ptr; > > int i, j; > > @@ -3428,9 +3428,10 @@ static void __devexit cciss_remove_one(s > > memset(flush_buf, 0, 4); > > return_code = sendcmd(CCISS_CACHE_FLUSH, i, flush_buf, 4, 0, 0, 0, NULL, > > TYPE_CMD); > > - if (return_code != IO_OK) { > > - printk(KERN_WARNING "Error Flushing cache on controller %d\n", > > - i); > > + if (return_code == IO_OK) { > > + printk(KERN_WARNING "Completed flushing cache on controller %d\n", i); > > Why do you want the world to know this on every shutdown? > > At the least, use dev_warn() to show the proper device and driver that > this is happening to, but does it really need to go to the warning log? > It sounds like a normal operation to me. How about dev_dbg() instead? > > thanks, > > greg k-h OK, here's a version that only prints if a problem occurred. That's the way the code was originally written. -- mikem ------------------------------------------------------------------------------------------ diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c index 9d84ab3..c50c09b 100644 --- a/drivers/block/cciss.c +++ b/drivers/block/cciss.c @@ -3404,7 +3404,7 @@ #endif return -1; } -static void __devexit cciss_remove_one(struct pci_dev *pdev) +static void cciss_remove_one(struct pci_dev *pdev) { ctlr_info_t *tmp_ptr; int i, j; @@ -3429,8 +3429,7 @@ static void __devexit cciss_remove_one(s return_code = sendcmd(CCISS_CACHE_FLUSH, i, flush_buf, 4, 0, 0, 0, NULL, TYPE_CMD); if (return_code != IO_OK) { - printk(KERN_WARNING "Error Flushing cache on controller %d\n", - i); + printk(KERN_WARNING "Error flushing cache on controller %d\n", i); } free_irq(hba[i]->intr[2], hba[i]); @@ -3481,6 +3480,7 @@ static struct pci_driver cciss_pci_drive .probe = cciss_init_one, .remove = __devexit_p(cciss_remove_one), .id_table = cciss_pci_device_id, /* id_table */ + .shutdown = cciss_remove_one, }; /* ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier) 2007-02-23 20:42 [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier) Mike Miller (OS Dev) 2007-02-23 21:32 ` Greg KH @ 2007-02-27 20:26 ` Andrew Morton 2007-03-05 22:06 ` [Patch 2/2] cciss: add struct pci_driver " Mike Miller (OS Dev) 1 sibling, 1 reply; 5+ messages in thread From: Andrew Morton @ 2007-02-27 20:26 UTC (permalink / raw) To: Mike Miller (OS Dev); +Cc: jens.axboe, linux-kernel, linux-scsi, gregkh > On Fri, 23 Feb 2007 14:42:39 -0600 "Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote: > This patch supercedes yesterdays cciss-shutdown patch. The primary difference is > removing __devexit from cciss_remove_one. Instead of create another function I'd > rather use the code that was intended to perform the cleanup and cache flush. I've > tested as a loadable module and statically linked without error. > Please consider this for inclusion. Please don't document patches like this. The entirety of your changelog and the Subject: are relative to a patch which will never hit the mainline git tree. Put yourself in the position of someone reading the git changelogs in a year's time. They're going to be left scratching their heads at the above, aren't they? Always include a complete and standalone, not-referential-to-an-old-patch changelog in each iteration of a patch. Always choose a suitable Subject: Yes, it's good to tell us things about how this patch differs from the previous one. That info can be placed after the ^--- which comes after your signed-off-by:, or can be placed at the top of the email, as long as the full permanent changelog is there too. Bottom line: you are submitting code and its documentation into the permanent kernel record, not just the mailing list. Try to make it appropriate, thanks. Please send a new Subject: and changlog for this patch. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch 2/2] cciss: add struct pci_driver shutdown support (replaces reboot notifier) 2007-02-27 20:26 ` Andrew Morton @ 2007-03-05 22:06 ` Mike Miller (OS Dev) 0 siblings, 0 replies; 5+ messages in thread From: Mike Miller (OS Dev) @ 2007-03-05 22:06 UTC (permalink / raw) To: Andrew Morton; +Cc: jens.axboe, linux-kernel, linux-scsi, gregkh On Tue, Feb 27, 2007 at 12:26:34PM -0800, Andrew Morton wrote: > > On Fri, 23 Feb 2007 14:42:39 -0600 "Mike Miller (OS Dev)" <mikem@beardog.cca.cpqcorp.net> wrote: > > This patch supercedes yesterdays cciss-shutdown patch. The primary difference is > > removing __devexit from cciss_remove_one. Instead of create another function I'd > > rather use the code that was intended to perform the cleanup and cache flush. I've > > tested as a loadable module and statically linked without error. > > Please consider this for inclusion. > > Please don't document patches like this. > > The entirety of your changelog and the Subject: are relative to a patch > which will never hit the mainline git tree. Put yourself in the position > of someone reading the git changelogs in a year's time. They're going to > be left scratching their heads at the above, aren't they? > > Always include a complete and standalone, not-referential-to-an-old-patch > changelog in each iteration of a patch. > > Always choose a suitable Subject: > > Yes, it's good to tell us things about how this patch differs from the > previous one. That info can be placed after the ^--- which comes after > your signed-off-by:, or can be placed at the top of the email, as long as > the full permanent changelog is there too. > > Bottom line: you are submitting code and its documentation into the > permanent kernel record, not just the mailing list. Try to make it > appropriate, thanks. > > > Please send a new Subject: and changlog for this patch. > Patch 2/2 This patch adds support for the struct pci_driver shutdown method to cciss. We require notification of an impending reboot or shutdown so that we can flush the battery backed write cache (BBWC) on the Smart Array controller. Please consider this for inclusion. Signed-off-by: Mike Miller <mike.miller@hp.com> ------------------------------------------------------------------------------------------ diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c index 9d84ab3..b16f48c 100644 --- a/drivers/block/cciss.c +++ b/drivers/block/cciss.c @@ -3404,7 +3404,7 @@ #endif return -1; } -static void __devexit cciss_remove_one(struct pci_dev *pdev) +static void cciss_remove_one(struct pci_dev *pdev) { ctlr_info_t *tmp_ptr; int i, j; @@ -3428,9 +3428,10 @@ static void __devexit cciss_remove_one(s memset(flush_buf, 0, 4); return_code = sendcmd(CCISS_CACHE_FLUSH, i, flush_buf, 4, 0, 0, 0, NULL, TYPE_CMD); - if (return_code != IO_OK) { - printk(KERN_WARNING "Error Flushing cache on controller %d\n", - i); + if (return_code == IO_OK) { + printk(KERN_INFO "Completed flushing cache on controller %d\n", i); + } else { + printk(KERN_WARNING "Error flushing cache on controller %d\n", i); } free_irq(hba[i]->intr[2], hba[i]); @@ -3481,6 +3482,7 @@ static struct pci_driver cciss_pci_drive .probe = cciss_init_one, .remove = __devexit_p(cciss_remove_one), .id_table = cciss_pci_device_id, /* id_table */ + .shutdown = cciss_remove_one, }; /* ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-03-05 22:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-02-23 20:42 [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier) Mike Miller (OS Dev) 2007-02-23 21:32 ` Greg KH 2007-02-23 21:44 ` Mike Miller (OS Dev) 2007-02-27 20:26 ` Andrew Morton 2007-03-05 22:06 ` [Patch 2/2] cciss: add struct pci_driver " Mike Miller (OS Dev)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.