Re: [PATCHv2] Add Freescale MC44S803 tuner driver

[Jochen Friedrich <jochen@scram.de>]

Hi Alexey,

>> +/* This functions tries to identify a MC44S803 tuner by reading the ID
>> +   register. This is hasty. */
>> +struct dvb_frontend *mc44s803_attach(struct dvb_frontend *fe,
>> +        struct i2c_adapter *i2c, struct mc44s803_config *cfg)
>> +{
>> +       struct mc44s803_priv *priv = NULL;
> 
> Do you really need *priv set to NULL here ?

No, it's not needed. Will remove.

>> +       priv = kzalloc(sizeof(struct mc44s803_priv), GFP_KERNEL);
>> +       if (priv == NULL)
>> +               return NULL;
> 
> Maybe return -ENOMEM; ? I don't sure about return NULL, may be your
> variant is right.

NULL is correct here. All tuners are supposed to return NULL on an error during attach().

>> +       if (id != 0x14) {
>> +               printk(KERN_ERROR "MC44S803: unsupported ID "
> 
> You pass the name of driver directly to printk messages in few places.
> Is it better to use such approach:
> #define MC44S803_DRIVER_NAME "mc44s803"
> 
> printk (KERN_ERR MC44S803_DRIVER_NAME ": something\n");
> 
> ?
> What do you think?

You're right. 

Thanks for your comments.
Jochen