On Fri, Feb 01, 2013 at 02:54:59PM -0600, H Hartley Sweeten wrote:
> On Friday, February 01, 2013 1:50 PM, Dan Carpenter wrote:
> > On Fri, Feb 01, 2013 at 02:43:17PM -0600, H Hartley Sweeten wrote:
> >> On Friday, February 01, 2013 6:23 AM, Ian Abbott wrote:
> >>> diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h
> >>> index f4541ae..33207fb 100644
> >>> --- a/drivers/staging/comedi/comedidev.h
> >>> +++ b/drivers/staging/comedi/comedidev.h
> >>> @@ -324,14 +324,7 @@ static inline unsigned int bytes_per_sample(const struct comedi_subdevice *subd)
> >>> * known bus type. Set automatically for auto-configured devices.
> >>> * Automatically set to NULL when detaching hardware device.
> >>> */
> >>> -static inline void comedi_set_hw_dev(struct comedi_device *dev,
> >>> - struct device *hw_dev)
> >>> -{
> >>> - struct device *old_hw_dev = dev->hw_dev;
> >>> -
> >>> - dev->hw_dev = get_device(hw_dev);
> >>> - put_device(old_hw_dev);
> >>> -}
> >>> +int comedi_set_hw_dev(struct comedi_device *dev, struct device *hw_dev);
> >>
> >> Minor nitpick...
> >>
> >> The variable names are not needed in the prototype of a function
> >> in a header.
> >
> > It's better to have variable names because they act as a kind of
> > documentation.
>
> Like I said, minor nitpick.
>
> Regarding the documentation usage.
>
> Since both variables are pointers to named structs they are basically
> already "documented".
That's because you already know the code inside and out... :/
For me, it's helpful to know which side is the hw_dev.
My nit-pick on this patch would be that the function comment should
be in the .c file and not the .h file.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
