Re: [PATCH 01/14] chardev: add helper function to register char devs with a struct device

2017-02-21 Thread Logan Gunthorpe 
Hey,

Ok, I'll do a v2 later this week with some of the feedback and tags?

Logan

On 21/02/17 11:18 AM, Dan Williams wrote:
> On Mon, Feb 20, 2017 at 10:35 PM, Logan Gunthorpe  wrote:
>>
>>
>> On 20/02/17 10:35 PM, Dmitry Torokhov wrote:
>>> My only objection is to this statement. There is absolutely nothing that
>>> prevents from calling device_unregister() first and cdev_del() later.
>>> Refcounting will sort it all out.
>>
>> Yeah, I was really trying to warn people against calling cdev_del within
>> the release function of the device. If you do that, then the cdev
>> reference will block the device from ever getting released.
>>
>> Certainly, you could call device_unregister followed by cdev_del. I
>> could reword this if you feel it necessary.
> 
> I agree with Dmitry, just delete the statement in parenthesis and the
> rest is fine. If you're modifying this patch it might be good to take
> the opportunity to add a WARN_ON(!parent->kobj.state_initialized) to
> catch attempts to call device_add_cdev() with an uninitialized device.
> 
> You can also add:
> 
> Signed-off-by: Dan Williams 
> 
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH 01/14] chardev: add helper function to register char devs with a struct device

2017-02-21 Thread Dan Williams 
On Mon, Feb 20, 2017 at 10:35 PM, Logan Gunthorpe  wrote:
>
>
> On 20/02/17 10:35 PM, Dmitry Torokhov wrote:
>> My only objection is to this statement. There is absolutely nothing that
>> prevents from calling device_unregister() first and cdev_del() later.
>> Refcounting will sort it all out.
>
> Yeah, I was really trying to warn people against calling cdev_del within
> the release function of the device. If you do that, then the cdev
> reference will block the device from ever getting released.
>
> Certainly, you could call device_unregister followed by cdev_del. I
> could reword this if you feel it necessary.

I agree with Dmitry, just delete the statement in parenthesis and the
rest is fine. If you're modifying this patch it might be good to take
the opportunity to add a WARN_ON(!parent->kobj.state_initialized) to
catch attempts to call device_add_cdev() with an uninitialized device.

You can also add:

Signed-off-by: Dan Williams 
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH 01/14] chardev: add helper function to register char devs with a struct device

2017-02-20 Thread Logan Gunthorpe 


On 20/02/17 10:35 PM, Dmitry Torokhov wrote:
> My only objection is to this statement. There is absolutely nothing that
> prevents from calling device_unregister() first and cdev_del() later.
> Refcounting will sort it all out.

Yeah, I was really trying to warn people against calling cdev_del within
the release function of the device. If you do that, then the cdev
reference will block the device from ever getting released.

Certainly, you could call device_unregister followed by cdev_del. I
could reword this if you feel it necessary.

Logan


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm