[PATCH linux dev-4.7 0/2] drivers: fsi: Improve driver cleanup

[PATCH linux dev-4.7 0/2] drivers: fsi: Improve driver cleanup

[Eddie James]

From: "Edward A. James" <eajames@us.ibm.com>

Need to delete the fsi device during the "unscan" process. Also add remove
function for scom client driver, otherwise we end up with bad files in
/dev.

Edward A. James (2):
  drivers: fsi: delete device on unscan
  drivers: fsi: scom: Add remove function

 drivers/fsi/fsi-core.c |  1 +
 drivers/fsi/fsi-scom.c | 16 ++++++++++++++++
 2 files changed, 17 insertions(+)

-- 
1.8.3.1

[PATCH linux dev-4.7 1/2] drivers: fsi: delete device on unscan

[Eddie James]

From: "Edward A. James" <eajames@us.ibm.com>

need to delete device so that client driver "remove" is called.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/fsi/fsi-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 1e9c5a2..e8a3618 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -512,6 +512,7 @@ static void fsi_master_unscan(struct fsi_master *master)
 		list_for_each_entry_safe(fsi_dev, fsi_dev_tmp,
 					&slave->my_engines, link) {
 			list_del(&fsi_dev->link);
+			device_del(&fsi_dev->dev);
 			put_device(&fsi_dev->dev);
 		}
 		device_unregister(&slave->dev);
-- 
1.8.3.1

[PATCH linux dev-4.7 2/2] drivers: fsi: scom: Add remove function

[Eddie James]

From: "Edward A. James" <eajames@us.ibm.com>

driver wasn't cleaning up miscdevice

Signed-off-by: Edward A. James <eajames@us.ibm.com>
---
 drivers/fsi/fsi-scom.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
index 22d280a..a439a5e 100644
--- a/drivers/fsi/fsi-scom.c
+++ b/drivers/fsi/fsi-scom.c
@@ -199,6 +199,21 @@ static int scom_probe(struct device *dev)
 	return misc_register(&scom->mdev);
 }
 
+static int scom_remove(struct device *dev)
+{
+	struct scom_device *scom, *scom_tmp;
+	struct fsi_device *fsi_dev = to_fsi_dev(dev);
+	
+	list_for_each_entry_safe(scom, scom_tmp, &scom_devices, link) {
+		if (scom->fsi_dev == fsi_dev) {
+			list_del(&scom->link);
+			misc_deregister(&scom->mdev);
+		}
+	}
+
+	return 0;
+}
+
 static struct fsi_device_id scom_ids[] = {
 	{
 		.engine_type = FSI_ENGID_SCOM,
@@ -213,6 +228,7 @@ static struct fsi_driver scom_drv = {
 		.name = "scom",
 		.bus = &fsi_bus_type,
 		.probe = scom_probe,
+		.remove = scom_remove,
 	}
 };
 
-- 
1.8.3.1

Re: [PATCH linux dev-4.7 1/2] drivers: fsi: delete device on unscan

[Christopher Bostic]

Acked by: Christopher Bostic <cbostic@linux.vnet.ibm.com>


On 2/13/17 2:07 PM, Eddie James wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
>
> need to delete device so that client driver "remove" is called.
>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> ---
>   drivers/fsi/fsi-core.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 1e9c5a2..e8a3618 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -512,6 +512,7 @@ static void fsi_master_unscan(struct fsi_master *master)
>   		list_for_each_entry_safe(fsi_dev, fsi_dev_tmp,
>   					&slave->my_engines, link) {
>   			list_del(&fsi_dev->link);
> +			device_del(&fsi_dev->dev);
>   			put_device(&fsi_dev->dev);
>   		}
>   		device_unregister(&slave->dev);

Re: [PATCH linux dev-4.7 2/2] drivers: fsi: scom: Add remove function

[Christopher Bostic]

Acked by: Christopher Bostic <cbostic@linux.vnet.ibm.com>


On 2/13/17 2:08 PM, Eddie James wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
>
> driver wasn't cleaning up miscdevice
>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> ---
>   drivers/fsi/fsi-scom.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
> index 22d280a..a439a5e 100644
> --- a/drivers/fsi/fsi-scom.c
> +++ b/drivers/fsi/fsi-scom.c
> @@ -199,6 +199,21 @@ static int scom_probe(struct device *dev)
>   	return misc_register(&scom->mdev);
>   }
>
> +static int scom_remove(struct device *dev)
> +{
> +	struct scom_device *scom, *scom_tmp;
> +	struct fsi_device *fsi_dev = to_fsi_dev(dev);
> +	
> +	list_for_each_entry_safe(scom, scom_tmp, &scom_devices, link) {
> +		if (scom->fsi_dev == fsi_dev) {
> +			list_del(&scom->link);
> +			misc_deregister(&scom->mdev);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>   static struct fsi_device_id scom_ids[] = {
>   	{
>   		.engine_type = FSI_ENGID_SCOM,
> @@ -213,6 +228,7 @@ static struct fsi_driver scom_drv = {
>   		.name = "scom",
>   		.bus = &fsi_bus_type,
>   		.probe = scom_probe,
> +		.remove = scom_remove,
>   	}
>   };
>

Re: [PATCH linux dev-4.7 1/2] drivers: fsi: delete device on unscan

[Joel Stanley]

On Tue, Feb 14, 2017 at 6:37 AM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
>
> need to delete device so that client driver "remove" is called.
>
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> ---
>  drivers/fsi/fsi-core.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 1e9c5a2..e8a3618 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -512,6 +512,7 @@ static void fsi_master_unscan(struct fsi_master *master)
>                 list_for_each_entry_safe(fsi_dev, fsi_dev_tmp,
>                                         &slave->my_engines, link) {
>                         list_del(&fsi_dev->link);
> +                       device_del(&fsi_dev->dev);
>                         put_device(&fsi_dev->dev);

put_device calls kobject_put, but device_del has already called
kobject_del. Are you sure this is what you want to do here?


>                 }
>                 device_unregister(&slave->dev);
> --
> 1.8.3.1
>

Re: [PATCH linux dev-4.7 1/2] drivers: fsi: delete device on unscan

[Eddie James]

On 02/13/2017 06:18 PM, Joel Stanley wrote:
> On Tue, Feb 14, 2017 at 6:37 AM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
>> From: "Edward A. James" <eajames@us.ibm.com>
>>
>> need to delete device so that client driver "remove" is called.
>>
>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>> ---
>>   drivers/fsi/fsi-core.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>> index 1e9c5a2..e8a3618 100644
>> --- a/drivers/fsi/fsi-core.c
>> +++ b/drivers/fsi/fsi-core.c
>> @@ -512,6 +512,7 @@ static void fsi_master_unscan(struct fsi_master *master)
>>                  list_for_each_entry_safe(fsi_dev, fsi_dev_tmp,
>>                                          &slave->my_engines, link) {
>>                          list_del(&fsi_dev->link);
>> +                       device_del(&fsi_dev->dev);
>>                          put_device(&fsi_dev->dev);
> put_device calls kobject_put, but device_del has already called
> kobject_del. Are you sure this is what you want to do here?

Hmm, I was just doing what device_unregister() does. We manually call 
device_add() for these devices, so I understand from the documentation 
that device_del() should be called, but I think device_unregister would 
work as well. I can confirm this works and doesn't throw any warnings 
after repeated fsi scans.

>
>
>>                  }
>>                  device_unregister(&slave->dev);
>> --
>> 1.8.3.1
>>

Re: [PATCH linux dev-4.7 1/2] drivers: fsi: delete device on unscan

[Joel Stanley]

Hi Chris,

On Tue, Feb 14, 2017 at 7:30 AM, Christopher Bostic
<cbostic@linux.vnet.ibm.com> wrote:
> Acked by: Christopher Bostic <cbostic@linux.vnet.ibm.com>

Thanks for checking over this.

Next time you need to write "Acked-by" for patchwork to recognise your tag:

 http://patchwork.ozlabs.org/patch/727563/

Cheers,

Joel

>
>
>
> On 2/13/17 2:07 PM, Eddie James wrote:
>>
>> From: "Edward A. James" <eajames@us.ibm.com>
>>
>> need to delete device so that client driver "remove" is called.
>>
>> Signed-off-by: Edward A. James <eajames@us.ibm.com>
>> ---
>>   drivers/fsi/fsi-core.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>> index 1e9c5a2..e8a3618 100644
>> --- a/drivers/fsi/fsi-core.c
>> +++ b/drivers/fsi/fsi-core.c
>> @@ -512,6 +512,7 @@ static void fsi_master_unscan(struct fsi_master
>> *master)
>>                 list_for_each_entry_safe(fsi_dev, fsi_dev_tmp,
>>                                         &slave->my_engines, link) {
>>                         list_del(&fsi_dev->link);
>> +                       device_del(&fsi_dev->dev);
>>                         put_device(&fsi_dev->dev);
>>                 }
>>                 device_unregister(&slave->dev);
>
>

Re: [PATCH linux dev-4.7 1/2] drivers: fsi: delete device on unscan

[Joel Stanley]

On Tue, Feb 14, 2017 at 2:41 PM, Eddie James <eajames@linux.vnet.ibm.com> wrote:
> On 02/13/2017 06:18 PM, Joel Stanley wrote:
>>> @@ -512,6 +512,7 @@ static void fsi_master_unscan(struct fsi_master
>>> *master)
>>>                  list_for_each_entry_safe(fsi_dev, fsi_dev_tmp,
>>>                                          &slave->my_engines, link) {
>>>                          list_del(&fsi_dev->link);
>>> +                       device_del(&fsi_dev->dev);
>>>                          put_device(&fsi_dev->dev);
>>
>> put_device calls kobject_put, but device_del has already called
>> kobject_del. Are you sure this is what you want to do here?
>
>
> Hmm, I was just doing what device_unregister() does. We manually call
> device_add() for these devices, so I understand from the documentation that
> device_del() should be called, but I think device_unregister would work as
> well. I can confirm this works and doesn't throw any warnings after repeated
> fsi scans.

Thanks for looking into this.

I will merge this patch as-is, but please work with Chris to get this
change added to his upstream FSI patchset. The fix should be folded
into his series for his next submission (v4).

Cheers,

Joel