Re: [PATCH] Add TEA5764 radio driver

["Fábio Belavenuto" <belavenuto@gmail.com>]

Alexey Klimov escreveu:
> Hello, Fabio
> May i tell some suggestion here ?
>
>   
Yes, I appreciate.
> So, you used printk + KBUILD_MODNAME here and then you used it in code.
> As i know(may be i'm wrong) that using KBUILD_MODNAME here is bad.
> Better way is doing something like this:
>
> #define TEA5764_DRIVER_NAME "radio-tea5764"
>
> (defined driver name should be unique)
>
> #define PINFO(format, ...)					\
> 	printk(KERN_INFO TEA5764_DRIVER_NAME": " 		\
> 		DRIVER_VERSION ": " format "\n", ## __VA_ARGS__)
> Later, you can(should?) use TEA5764_DRIVER_NAME in static const struct
> i2c_device_id tea5764_id and in static struct i2c_driver
> tea5764_i2c_driver.
>
> But, really good way is using dev_info, dev_warn, dev_err macros. You
> can look for examples in dsbr100.c or radio-mr800.c. Or in other
> drivers.
>
>   
Thank you, I will use this constant and macros dev_*.
>  
>   
> It will be really nice to hear what other developers think about making
> header file for structures and a lot of defines. Should it go to
> separate .h file ?
>
>
>   
I study on the use of headers, I thought it was complicated to create 
more headlines.
>
> Here. You can insert TEA5764_DRIVER_NAME instead of "radio-tea5764".
>
>   
Ok.
> Please, reformat the patch. All rest code looks good for my eyes.
>
>
>   
Thank you, I will.


--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list