[PATCH 5/6] staging: ozwpan: udev support

Rupesh Gujare rgujare at ozmodevices.com
Tue Jun 26 09:39:47 UTC 2012


On 23/06/12 10:46, Dan Carpenter wrote:
> On Wed, Jun 20, 2012 at 01:36:10PM +0100, Rupesh Gujare wrote:
>> Register ozmo_wpan class with sysfs & support for udev
>> to create device node.
>>
>> Signed-off-by: Rupesh Gujare <rgujare at ozmodevices.com>
>> ---
>>   drivers/staging/ozwpan/ozcdev.c |   24 +++++++++++++++++++++++-
>>   1 files changed, 23 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
>> index 27325f7..929756a 100644
>> --- a/drivers/staging/ozwpan/ozcdev.c
>> +++ b/drivers/staging/ozwpan/ozcdev.c
>> @@ -330,10 +330,12 @@ const struct file_operations oz_fops = {
>>   int oz_cdev_register(void)
>>   {
>>   	int err;
>> +	struct class *cl;
>> +	struct device *dev;
> Put a blank line between declarations and statements.
>
>>   	memset(&g_cdev, 0, sizeof(g_cdev));
>>   	err = alloc_chrdev_region(&g_cdev.devnum, 0, 1, "ozwpan");
>>   	if (err < 0)
>> -		return err;
>> +		goto out3;
> Just return directly here.  Don't use GW-BASIC names for labels.
> The label name should reflect the what happens at the label.  For
> example:  goto unregister; would meant that you unregister after the
> goto.
>
>>   	oz_trace("Alloc dev number %d:%d\n", MAJOR(g_cdev.devnum),
>>   			MINOR(g_cdev.devnum));
>>   	cdev_init(&g_cdev.cdev, &oz_fops);
>> @@ -342,7 +344,27 @@ int oz_cdev_register(void)
>>   	spin_lock_init(&g_cdev.lock);
>>   	init_waitqueue_head(&g_cdev.rdq);
>>   	err = cdev_add(&g_cdev.cdev, g_cdev.devnum, 1);
>> +	if (err < 0) {
>> +		oz_trace("Failed to add cdev\n");
>> +		goto out2;
>> +	}
>> +	cl = class_create(THIS_MODULE, "ozmo_wpan");
>> +	if (IS_ERR(cl)) {
> Needs a:
> 		err = PTR_ERR(cl);
>
>> +		oz_trace("Failed to register ozmo_wpan class\n");
>> +		goto out1;
>> +	}
>> +	dev = device_create(cl, NULL, g_cdev.devnum, NULL, "ozwpan");
>> +	if (IS_ERR(dev)) {
>> +		oz_trace("Failed to create sysfs entry for cdev\n");
> Needs a:
> 		err = PTR_ERR(dev);
>
>
>> +		goto out1;
>> +	}
>>   	return 0;
> Blank line here probably.
>
> regards,
> dan carpenter
>
>> +out1:
>> +	cdev_del(&g_cdev.cdev);
>> +out2:
>> +	unregister_chrdev_region(g_cdev.devnum, 1);
>> +out3:
>> +	return err;
>>   }
>>   /*------------------------------------------------------------------------------
>>    * Context: process
>> -- 
>>
Thanks Dan,

Sounds reasonable.
I will include these changes in next patch series.

-- 
Regards,
Rupesh Gujare





More information about the devel mailing list