Re: [PATCH 3/3] radio-tea5761: Update driver

2008-06-10 Thread Eduardo Valentin
Hi Balbi,

On Mon, Jun 9, 2008 at 10:07 PM, Felipe Balbi [EMAIL PROTECTED] wrote:
 On Mon, Jun 09, 2008 at 09:14:21PM -0400, Eduardo Valentin wrote:
 If I understood correctly what you said, the same comment I said
 before also applies here.
 Eventhough it prints that the probe
 failed, this way I sent it says: The probe failed and The driver
 registration failed. More easy
 to debug the code when in a error situation.

 But if probe fails, driver registration failed. The same comment Jean
 Delvare gave me when I was sending some isp1301 patches upstream, so
 later on, when this driver goes to mailine, it'll probably, if Jean
 happens to see this one, get commented on this point as well.

 If you look at drivers/i2c/chips/*.c you'll see that only in two cases
 they are printing error if i2c_add_driver() fails and those are really
 necessary cases (check those drivers).

 But anyway, if you think it's really necessary.


Ok. I've just resent the whole series with this message removed. :)

 --
 Best Regards,

 Felipe Balbi
 [EMAIL PROTECTED]
 http://blog.felipebalbi.com




-- 
Eduardo Bezerra Valentin
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] radio-tea5761: Update driver

2008-06-09 Thread Felipe Balbi
On Mon, Jun 09, 2008 at 04:05:43PM -0400, Eduardo Valentin wrote:
 +static void tea5761_set_audout_mode(struct tea5761_device *tea, int audmode)
  {
 - struct tea5761_regs *r = tea-regs;
 -
 - if (!(r-tnctrl  TEA5761_TNCTRL_PUPD0)) {
 - r-tnctrl = ~(TEA5761_TNCTRL_AFM | TEA5761_TNCTRL_MU |
 -TEA5761_TNCTRL_HLSI);
 - r-testreg |= TEA5761_TESTREG_TRIGFR;
 - r-tnctrl |= TEA5761_TNCTRL_PUPD0;
 - return tea5761_write_regs(tea);
 + struct dvb_frontend *fe = tea-fe;
 + struct dvb_tuner_ops *fe_tuner_ops = fe-ops.tuner_ops;
 + struct analog_parameters params = {
 + .mode   = V4L2_TUNER_RADIO,
 + .frequency  = tea-freq,
 + .audmode= audmode,
 + };
 +
 + if (NULL == fe_tuner_ops-set_analog_params) {
 + dev_warn(tea-dev,
 + Tuner frontend module has no way to set frequency\n);
 + return;
   }
 + if (!fe_tuner_ops-set_analog_params(fe, params))
 + tea-audmode = audmode;

instead of both ifs, how about:

if (fe_tuner_ops-set_analog_params) {
tea-audmode =
fe_tuner_ops-set_analog_params(fe, params) ?
audmode : 0;
}
  }

 +static void tea5761_mute(struct tea5761_device *tea, int on)
 +{
 + struct dvb_frontend *fe = tea-fe;
 + struct dvb_tuner_ops *fe_tuner_ops = fe-ops.tuner_ops;
 + struct analog_parameters params = {
 + .mode   = on ? T_STANDBY : V4L2_TUNER_RADIO,
 + .frequency  = tea-freq,
 + .audmode= tea-audmode,
 + };
 +
 + if (NULL == fe_tuner_ops-set_analog_params) {
 + dev_warn(tea-dev,
 + Tuner frontend module has no way to set frequency\n);
 + return;
   }
 + if (!fe_tuner_ops-set_analog_params(fe, params))
 + tea-mute = on;

samething here.

  }

  static int tea5761_i2c_driver_probe(struct i2c_client *client,
 @@ -422,12 +407,24 @@ static int tea5761_i2c_driver_probe(struct i2c_client 
 *client,
  
   mutex_init(tea-mutex);
  
 - tea-i2c_dev = client;
 + /* Tuner attach */
 + if (!dvb_attach(tea5761_attach, tea-fe, client-adapter,
 + client-addr)) {
 + dev_err(client-dev, Could not attach tuner\n);
 + err = -ENODEV;
 + goto exit;
 + }
 +
 + /* initialize and power off the chip */
 + tea5761_power_up(tea);
 + tea5761_set_audout_mode(tea, V4L2_TUNER_MODE_STEREO);
 + tea5761_mute(tea, 0);
 + tea5761_power_down(tea);
  
   /* V4L initialization */
   video_dev = video_device_alloc();
   if (video_dev == NULL) {

if (!video_dev)

 - dev_err(client-dev, couldn't allocate memory\n);
 + dev_err(client-dev, Could not allocate memory\n);
   err = -ENOMEM;
   goto exit;
   }
 @@ -436,25 +433,15 @@ static int tea5761_i2c_driver_probe(struct i2c_client 
 *client,
   *video_dev = tea5761_video_device;
   video_dev-dev = client-dev;
   i2c_set_clientdata(client, video_dev);
 -
 - /* initialize and power off the chip */
 - tea5761_read_regs(tea);
 - tea5761_set_audout_mode(tea, V4L2_TUNER_MODE_STEREO);
 - tea5761_mute(tea, 0);
 - tea5761_power_down(tea);
 -
 - tea5761.video_dev = video_dev;
 - tea5761.i2c_dev = client;
 + tea-video_dev = video_dev;
 + tea-dev = client-dev;
  
   err = video_register_device(video_dev, VFL_TYPE_RADIO, radio_nr);
   if (err) {
 - dev_err(client-dev, couldn't register video device\n);
 + dev_err(client-dev, Could not register video device\n);
   goto err_video_alloc;
   }
  
 - dev_info(client-dev, tea5761 (version %d) detected\n,
 - (tea-regs.manid  12)  0xf);
 -
   return 0;
  
  err_video_alloc:
 @@ -492,7 +479,8 @@ static int __init tea5761_init(void)
  {
   int res;
  
 - if ((res = i2c_add_driver(tea5761_driver))) {
 + res = i2c_add_driver(tea5761_driver);
 + if (res) {
   printk(KERN_ERR DRIVER_NAME : driver registration failed\n);
   return res;

not needed, return i2c_add_driver(tea5761_driver); is enough as i2c
subsystem already prints out error messages in case of failed probe.

   }

-- 
Best Regards,

Felipe Balbi
[EMAIL PROTECTED]
http://blog.felipebalbi.com
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html