* [PATCH 1/3] staging/slicoss: remove not-needed ASSERT @ 2012-07-09 17:34 Devendra Naga 2012-07-09 17:34 ` [PATCH 2/3] staging/slicoss: disable pci device at remove Devendra Naga 2012-07-09 17:34 ` [PATCH 3/3] staging/slicoss: return -ENODEV if no devid matches Devendra Naga 0 siblings, 2 replies; 7+ messages in thread From: Devendra Naga @ 2012-07-09 17:34 UTC (permalink / raw) To: Greg Kroah-Hartman, Lior Dotan, Christopher Harrer, linux-kernel, devel Cc: Devendra Naga As the private pointer is valid at the remove of driver, and remove wont' be called if probe fails, so no point for checking of ASSERT Signed-off-by: Devendra Naga <devendra.aaru@gmail.com> --- drivers/staging/slicoss/slicoss.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c index d2b82a7..a511a2b 100644 --- a/drivers/staging/slicoss/slicoss.c +++ b/drivers/staging/slicoss/slicoss.c @@ -3196,7 +3196,6 @@ static void __devexit slic_entry_remove(struct pci_dev *pcidev) struct sliccard *card; struct mcast_address *mcaddr, *mlist; - ASSERT(adapter); slic_adapter_freeresources(adapter); slic_unmap_mmio_space(adapter); unregister_netdev(dev); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] staging/slicoss: disable pci device at remove 2012-07-09 17:34 [PATCH 1/3] staging/slicoss: remove not-needed ASSERT Devendra Naga @ 2012-07-09 17:34 ` Devendra Naga 2012-07-09 20:06 ` Greg Kroah-Hartman 2012-07-09 17:34 ` [PATCH 3/3] staging/slicoss: return -ENODEV if no devid matches Devendra Naga 1 sibling, 1 reply; 7+ messages in thread From: Devendra Naga @ 2012-07-09 17:34 UTC (permalink / raw) To: Greg Kroah-Hartman, Lior Dotan, Christopher Harrer, linux-kernel, devel Cc: Devendra Naga at probe we enabled the device, and we should disable it at remove. Signed-off-by: Devendra Naga <devendra.aaru@gmail.com> --- drivers/staging/slicoss/slicoss.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c index a511a2b..5bd3825 100644 --- a/drivers/staging/slicoss/slicoss.c +++ b/drivers/staging/slicoss/slicoss.c @@ -3234,6 +3234,7 @@ static void __devexit slic_entry_remove(struct pci_dev *pcidev) } free_netdev(dev); pci_release_regions(pcidev); + pci_disable_device(pcidev); } static int slic_entry_halt(struct net_device *dev) -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] staging/slicoss: disable pci device at remove 2012-07-09 17:34 ` [PATCH 2/3] staging/slicoss: disable pci device at remove Devendra Naga @ 2012-07-09 20:06 ` Greg Kroah-Hartman 2012-07-09 20:09 ` Eric W. Biederman 0 siblings, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2012-07-09 20:06 UTC (permalink / raw) To: Devendra Naga; +Cc: Lior Dotan, Christopher Harrer, linux-kernel, devel On Mon, Jul 09, 2012 at 11:04:19PM +0530, Devendra Naga wrote: > at probe we enabled the device, and we should disable it at remove. > > Signed-off-by: Devendra Naga <devendra.aaru@gmail.com> > --- > drivers/staging/slicoss/slicoss.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c > index a511a2b..5bd3825 100644 > --- a/drivers/staging/slicoss/slicoss.c > +++ b/drivers/staging/slicoss/slicoss.c > @@ -3234,6 +3234,7 @@ static void __devexit slic_entry_remove(struct pci_dev *pcidev) > } > free_netdev(dev); > pci_release_regions(pcidev); > + pci_disable_device(pcidev); No, you really shouldn't do this, see the many times this has come up on the linux-kernel mailing list for why. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] staging/slicoss: disable pci device at remove 2012-07-09 20:06 ` Greg Kroah-Hartman @ 2012-07-09 20:09 ` Eric W. Biederman 2012-07-10 18:28 ` Greg Kroah-Hartman 0 siblings, 1 reply; 7+ messages in thread From: Eric W. Biederman @ 2012-07-09 20:09 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Devendra Naga, Lior Dotan, Christopher Harrer, linux-kernel, devel Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes: > On Mon, Jul 09, 2012 at 11:04:19PM +0530, Devendra Naga wrote: >> at probe we enabled the device, and we should disable it at remove. >> >> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com> >> --- >> drivers/staging/slicoss/slicoss.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c >> index a511a2b..5bd3825 100644 >> --- a/drivers/staging/slicoss/slicoss.c >> +++ b/drivers/staging/slicoss/slicoss.c >> @@ -3234,6 +3234,7 @@ static void __devexit slic_entry_remove(struct pci_dev *pcidev) >> } >> free_netdev(dev); >> pci_release_regions(pcidev); >> + pci_disable_device(pcidev); > > No, you really shouldn't do this, see the many times this has come up on > the linux-kernel mailing list for why. I haven't see this? Why don't you want to disable a device at remove time? Because we put the disable in the generic pci layer? Eric ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] staging/slicoss: disable pci device at remove 2012-07-09 20:09 ` Eric W. Biederman @ 2012-07-10 18:28 ` Greg Kroah-Hartman 2012-07-11 5:03 ` devendra.aaru 0 siblings, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2012-07-10 18:28 UTC (permalink / raw) To: Eric W. Biederman Cc: Devendra Naga, Lior Dotan, Christopher Harrer, linux-kernel, devel On Mon, Jul 09, 2012 at 01:09:23PM -0700, Eric W. Biederman wrote: > Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes: > > > On Mon, Jul 09, 2012 at 11:04:19PM +0530, Devendra Naga wrote: > >> at probe we enabled the device, and we should disable it at remove. > >> > >> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com> > >> --- > >> drivers/staging/slicoss/slicoss.c | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c > >> index a511a2b..5bd3825 100644 > >> --- a/drivers/staging/slicoss/slicoss.c > >> +++ b/drivers/staging/slicoss/slicoss.c > >> @@ -3234,6 +3234,7 @@ static void __devexit slic_entry_remove(struct pci_dev *pcidev) > >> } > >> free_netdev(dev); > >> pci_release_regions(pcidev); > >> + pci_disable_device(pcidev); > > > > No, you really shouldn't do this, see the many times this has come up on > > the linux-kernel mailing list for why. > > I haven't see this? Why don't you want to disable a device at remove > time? Because we put the disable in the generic pci layer? For some reason, I thought we didn't do this because of other "interfaces" on the same card might then be shut down. But I must have been thinking of something else, as lots of drivers do this, so adding it here looks to be correct. So, sorry Devendra, you were right, care to resend this so I can apply it? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] staging/slicoss: disable pci device at remove 2012-07-10 18:28 ` Greg Kroah-Hartman @ 2012-07-11 5:03 ` devendra.aaru 0 siblings, 0 replies; 7+ messages in thread From: devendra.aaru @ 2012-07-11 5:03 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Eric W. Biederman, Lior Dotan, Christopher Harrer, linux-kernel, devel On Wed, Jul 11, 2012 at 12:13 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: >> I haven't see this? Why don't you want to disable a device at remove >> time? Because we put the disable in the generic pci layer? > > For some reason, I thought we didn't do this because of other > "interfaces" on the same card might then be shut down. But I must have > been thinking of something else, as lots of drivers do this, so adding > it here looks to be correct. > > So, sorry Devendra, you were right, care to resend this so I can apply > it? > Hi Greg, I will send the patch out in few hrs, with V2 name. Actually i was referring to the discussion here in this link, https://lkml.org/lkml/2005/2/13/82 it says about if the request regions fails we do not disable the device, as the other driver may be using the device. since the remove function of the driver is calling, which means that probe succeeded, so i think its fine to call pci_disable_device at driver_remove. > thanks, > > greg k-h Thanks, devendra. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] staging/slicoss: return -ENODEV if no devid matches 2012-07-09 17:34 [PATCH 1/3] staging/slicoss: remove not-needed ASSERT Devendra Naga 2012-07-09 17:34 ` [PATCH 2/3] staging/slicoss: disable pci device at remove Devendra Naga @ 2012-07-09 17:34 ` Devendra Naga 1 sibling, 0 replies; 7+ messages in thread From: Devendra Naga @ 2012-07-09 17:34 UTC (permalink / raw) To: Greg Kroah-Hartman, Lior Dotan, Christopher Harrer, linux-kernel, devel Cc: Devendra Naga if no case matches we are simply asserting and doing break. and i think we may need to return that -ENODEV , no device is present, rather assert'ing. Signed-off-by: Devendra Naga <devendra.aaru@gmail.com> --- drivers/staging/slicoss/slicoss.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c index 5bd3825..56829fc 100644 --- a/drivers/staging/slicoss/slicoss.c +++ b/drivers/staging/slicoss/slicoss.c @@ -3746,8 +3746,7 @@ static u32 slic_card_locate(struct adapter *adapter) rdhostid_offset = SLIC_RDHOSTID_1GB; break; default: - ASSERT(0); - break; + return -ENODEV; } hostid_reg = -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-07-11 5:04 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-09 17:34 [PATCH 1/3] staging/slicoss: remove not-needed ASSERT Devendra Naga 2012-07-09 17:34 ` [PATCH 2/3] staging/slicoss: disable pci device at remove Devendra Naga 2012-07-09 20:06 ` Greg Kroah-Hartman 2012-07-09 20:09 ` Eric W. Biederman 2012-07-10 18:28 ` Greg Kroah-Hartman 2012-07-11 5:03 ` devendra.aaru 2012-07-09 17:34 ` [PATCH 3/3] staging/slicoss: return -ENODEV if no devid matches Devendra Naga
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.