Re: [PATCH] staging: comedi: have comedi_set_spriv() allocate the memory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[Originally posted 2013-06-21.  Reposting to new driverdev-devel list.]

On 2013-06-20 18:07, H Hartley Sweeten wrote:
On Thursday, June 20, 2013 2:50 AM, Ian Abbott wrote:
On 2013/06/19 11:24 PM, H Hartley Sweeten wrote:
As suggested by Ian Abbott, comedi_set_spriv() can only be used to
set the subdevice->private pointer to something that can be kfree()'d.
Rename the function to comedi_alloc_spriv() and have it kzalloc() the
memory as well as set the private pointer. This saves a function call
in the drivers and avoids the possibility of incorrectly calling
comedi_set_spriv() for some pointer that is not meant to be kfree()'d.

Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
Cc: Ian Abbott <abbotti@xxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

Looks good.

Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>

Thanks.

Question.

In comedi_alloc_spriv() do you think the spinlock of the
runflags is necessary? This function should only be called
during the attach of a board so I feel setting SDF_FREE_SPRIV
should be safe without the spinlock.

It should be fine without the spinlock, as you say.

I would like to move this function from comedi_fops.c to
drivers.c but I don't want to unnecessarily expose the
comedi_set_subdevice_runflags() helper since only the
core should be messing with the runflags.

Does it really matter if it's in comedi_fops.c or drivers.c? Or are you planning to move other some other comedi driver core functions into drivers.c as well?

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux