[PATCH 1/3] staging/slicoss: remove not-needed ASSERT

[PATCH 1/3] staging/slicoss: remove not-needed ASSERT

[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

[PATCH 2/3] staging/slicoss: disable pci device at remove

[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

Re: [PATCH 2/3] staging/slicoss: disable pci device at remove

[Greg Kroah-Hartman]

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

Re: [PATCH 2/3] staging/slicoss: disable pci device at remove

[Eric W. Biederman]

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

Re: [PATCH 2/3] staging/slicoss: disable pci device at remove

[Greg Kroah-Hartman]

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

Re: [PATCH 2/3] staging/slicoss: disable pci device at remove

[devendra.aaru]

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.

[PATCH 3/3] staging/slicoss: return -ENODEV if no devid matches

[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