Re: [PATCH v3] isl6421.c - added tone control and temporary diseqc overcurrent

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

 



HoP wrote:
> Hi Mauro,
> 
> Not to hassle you, I'm sure you're very busy.
> 
> But I'm not yet received a response from you on mail with corrected patch.
> 
> Your attention would be appreciated

Hi Honza,

The patch looks correct to me, but, as I previously mentioned, our policy is
to add new features at the kernel driver only together with a driver that
actually requires it. This helps to avoid increasing the kernel without need.

So, please re-submit it when you have your driver requiring the isl6421
changes ready for submission, on the same patch series.

Cheers,
Mauro.
> 
> Regards
> 
> /Honza
> 
> 2009/12/16 HoP <jpetrous@xxxxxxxxx>:
>> Hi Mauro,
>>
>> 2009/12/15 Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>:
>> [snip]
>>
>>> I'm still missing a driver or a board entry that requires those
>>> changes. Could you please send it together with this patch series?
>>>
>> We are using it in our project. Currently we are in very early
>> stage of it. We still have some serious issue, what not allows
>> us sending such code for mainlining.
>>
>> Anyway, I don't think it can block accepting current patchset.
>> Isl6421 driver is already in tree, we only want to add some
>> features, which can be or can not be interesting for others.
>>
>> I believe extending of usability of current drivers is correct
>> way.
>>
>>> Also, you forgot to send your Signed-off-By. This is required for
>>> patch submission.
>>>
>>>> Regards
>>>>
>>>> /Honza
>>>>
>>>> ---
>>>>
>>>> isl6421.c - added tone control and temporary diseqc overcurrent
>>> Please, always send patches in-lined. makes easier for commenting.
>>>
>> OK.
>>
>>>> diff -r 79fc32bba0a0 linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c
>>>> --- a/linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c Mon Dec 14 17:43:13 2009 -0200
>>>> +++ b/linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c Tue Dec 15 16:36:14 2009 +0100
>>>> @@ -302,6 +302,12 @@ static struct itd1000_config skystar2_re
>>>>       .i2c_address = 0x61,
>>>>  };
>>>>
>>>> +static struct isl6421_config skystar2_rev2_7_isl6421_config = {
>>>> +     .i2c_address = 0x08,
>>>> +     .override_set = 0x01,
>>>> +     .override_clear = 0x01,
>>>> +};
>>>> +
>>>>  static int skystar2_rev27_attach(struct flexcop_device *fc,
>>>>       struct i2c_adapter *i2c)
>>>>  {
>>>> @@ -325,7 +331,7 @@ static int skystar2_rev27_attach(struct
>>>>       /* enable no_base_addr - no repeated start when reading */
>>>>       fc->fc_i2c_adap[2].no_base_addr = 1;
>>>>       if (!dvb_attach(isl6421_attach, fc->fe, &fc->fc_i2c_adap[2].i2c_adap,
>>>> -                     0x08, 1, 1)) {
>>>> +                     &skystar2_rev2_7_isl6421_config)) {
>>>>               err("ISL6421 could NOT be attached");
>>>>               goto fail_isl;
>>>>       }
>>>> @@ -368,6 +374,12 @@ static const struct cx24113_config skyst
>>>>       .xtal_khz = 10111,
>>>>  };
>>>>
>>>> +static struct isl6421_config skystar2_rev2_8_isl6421_config = {
>>>> +     .i2c_address = 0x08,
>>>> +     .override_set = 0x00,
>>>> +     .override_clear = 0x00,
>>> Please, do not set any static value to zero. Kernel module support already
>>> does that, and this will just add uneeded stuff into BSS.
>>>
>> Done.
>>
>>>> +};
>>>> +
>>>>  static int skystar2_rev28_attach(struct flexcop_device *fc,
>>>>       struct i2c_adapter *i2c)
>>>>  {
>>>> @@ -391,7 +403,7 @@ static int skystar2_rev28_attach(struct
>>>>
>>>>       fc->fc_i2c_adap[2].no_base_addr = 1;
>>>>       if (!dvb_attach(isl6421_attach, fc->fe, &fc->fc_i2c_adap[2].i2c_adap,
>>>> -                     0x08, 0, 0)) {
>>>> +                     &skystar2_rev2_8_isl6421_config)) {
>>>>               err("ISL6421 could NOT be attached");
>>>>               fc->fc_i2c_adap[2].no_base_addr = 0;
>>>>               return 0;
>>>> diff -r 79fc32bba0a0 linux/drivers/media/dvb/frontends/isl6421.c
>>>> --- a/linux/drivers/media/dvb/frontends/isl6421.c     Mon Dec 14 17:43:13 2009 -0200
>>>> +++ b/linux/drivers/media/dvb/frontends/isl6421.c     Tue Dec 15 16:36:14 2009 +0100
>>>> @@ -3,6 +3,9 @@
>>>>   *
>>>>   * Copyright (C) 2006 Andrew de Quincey
>>>>   * Copyright (C) 2006 Oliver Endriss
>>>> + * Copyright (C) 2009 Ales Jurik and Jan Petrous (added optional 22k tone
>>>> + *                    support and temporary diseqc overcurrent enable until
>>>> + *                    next command - set voltage or tone)
>>>>   *
>>>>   * This program is free software; you can redistribute it and/or
>>>>   * modify it under the terms of the GNU General Public License
>>>> @@ -36,37 +39,88 @@
>>>>  #include "isl6421.h"
>>>>
>>>>  struct isl6421 {
>>>> -     u8                      config;
>>>> -     u8                      override_or;
>>>> -     u8                      override_and;
>>>> -     struct i2c_adapter      *i2c;
>>>> -     u8                      i2c_addr;
>>>> +     const struct isl6421_config     *config;
>>>> +     u8                              reg1;
>>> reg1 seems a very bad name. Based on the datasheet, maybe
>>> you could call it as sys_config or sys_reg_config.
>>>
>> Renamed to sys_reg.
>>
>>>> +
>>>> +     struct i2c_adapter *i2c;
>>>> +
>>>> +     int (*diseqc_send_master_cmd_orig)(struct dvb_frontend *fe,
>>>> +                     struct dvb_diseqc_master_cmd *cmd);
>>>>  };
>>>>
>>>>  static int isl6421_set_voltage(struct dvb_frontend *fe, fe_sec_voltage_t voltage)
>>>>  {
>>>>       struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>>>> -     struct i2c_msg msg = {  .addr = isl6421->i2c_addr, .flags = 0,
>>>> -                             .buf = &isl6421->config,
>>>> -                             .len = sizeof(isl6421->config) };
>>>> +     struct i2c_msg msg = {  .addr = isl6421->config->i2c_addr, .flags = 0,
>>>> +                             .buf = &isl6421->reg1,
>>>> +                             .len = sizeof(isl6421->reg1) };
>>>>
>>>> -     isl6421->config &= ~(ISL6421_VSEL1 | ISL6421_EN1);
>>>> +     isl6421->reg1 &= ~(ISL6421_VSEL1 | ISL6421_EN1);
>>>>
>>>>       switch(voltage) {
>>>>       case SEC_VOLTAGE_OFF:
>>>>               break;
>>>>       case SEC_VOLTAGE_13:
>>>> -             isl6421->config |= ISL6421_EN1;
>>>> +             isl6421->reg1 |= ISL6421_EN1;
>>>>               break;
>>>>       case SEC_VOLTAGE_18:
>>>> -             isl6421->config |= (ISL6421_EN1 | ISL6421_VSEL1);
>>>> +             isl6421->reg1 |= (ISL6421_EN1 | ISL6421_VSEL1);
>>>>               break;
>>>>       default:
>>>>               return -EINVAL;
>>>>       };
>>>>
>>>> -     isl6421->config |= isl6421->override_or;
>>>> -     isl6421->config &= isl6421->override_and;
>>>> +     isl6421->reg1 |= isl6421->config->override_set;
>>>> +     isl6421->reg1 &= ~isl6421->config->override_clear;
>>>> +
>>>> +     return (i2c_transfer(isl6421->i2c, &msg, 1) == 1) ? 0 : -EIO;
>>>> +}
>>>> +
>>>> +static int isl6421_send_diseqc(struct dvb_frontend *fe,
>>>> +                             struct dvb_diseqc_master_cmd *cmd)
>>> Please add a comment explaining that this function is only called
>>> when diseqc_send_master_cmd_orig() is defined. On a first look, it seemed
>> OK, similar note added.
>>
>>> to me that you would cause a crash by not checking if diseqc_send_master_cmd_orig()
>>> is not null.
>>>
>> Your are right! Thanks. Fixed.
>>
>>>> +{
>>>> +     struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>>>> +     struct i2c_msg msg = {  .addr = isl6421->config->i2c_addr, .flags = 0,
>>>> +                             .buf = &isl6421->reg1,
>>>> +                             .len = sizeof(isl6421->reg1) };
>>>> +
>>>> +     isl6421->reg1 |= ISL6421_DCL;
>>>> +
>>>> +     isl6421->reg1 |= isl6421->config->override_set;
>>>> +     isl6421->reg1 &= ~isl6421->config->override_clear;
>>>> +
>>>> +     if (i2c_transfer(isl6421->i2c, &msg, 1) != 1)
>>>> +             return -EIO;
>>>> +
>>>> +     isl6421->reg1 &= ~ISL6421_DCL;
>>>> +
>>>> +     return isl6421->diseqc_send_master_cmd_orig(fe, cmd);
>>>> +}
>>>> +
>>>> +static int isl6421_set_tone(struct dvb_frontend *fe, fe_sec_tone_mode_t tone)
>>>> +{
>>>> +     struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>>>> +     struct i2c_msg msg = {  .addr = isl6421->config->i2c_addr, .flags = 0,
>>>> +                             .buf = &isl6421->reg1,
>>>> +                             .len = sizeof(isl6421->reg1) };
>>>> +
>>>> +     isl6421->reg1 &= ~(ISL6421_ENT1);
>>>> +
>>>> +     printk(KERN_INFO "%s: %s\n", __func__, ((tone == SEC_TONE_OFF) ?
>>>> +                             "Off" : "On"));
>>>> +
>>>> +     switch (tone) {
>>>> +     case SEC_TONE_ON:
>>>> +             isl6421->reg1 |= ISL6421_ENT1;
>>>> +             break;
>>>> +     case SEC_TONE_OFF:
>>>> +             break;
>>>> +     default:
>>>> +             return -EINVAL;
>>>> +     };
>>>> +
>>>> +     isl6421->reg1 |= isl6421->config->override_set;
>>>> +     isl6421->reg1 &= ~isl6421->config->override_clear;
>>>>
>>>>       return (i2c_transfer(isl6421->i2c, &msg, 1) == 1) ? 0 : -EIO;
>>>>  }
>>>> @@ -74,49 +128,52 @@ static int isl6421_enable_high_lnb_volta
>>>>  static int isl6421_enable_high_lnb_voltage(struct dvb_frontend *fe, long arg)
>>>>  {
>>>>       struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>>>> -     struct i2c_msg msg = {  .addr = isl6421->i2c_addr, .flags = 0,
>>>> -                             .buf = &isl6421->config,
>>>> -                             .len = sizeof(isl6421->config) };
>>>> +     struct i2c_msg msg = {  .addr = isl6421->config->i2c_addr, .flags = 0,
>>>> +                             .buf = &isl6421->reg1,
>>>> +                             .len = sizeof(isl6421->reg1) };
>>>>
>>>>       if (arg)
>>>> -             isl6421->config |= ISL6421_LLC1;
>>>> +             isl6421->reg1 |= ISL6421_LLC1;
>>>>       else
>>>> -             isl6421->config &= ~ISL6421_LLC1;
>>>> +             isl6421->reg1 &= ~ISL6421_LLC1;
>>>>
>>>> -     isl6421->config |= isl6421->override_or;
>>>> -     isl6421->config &= isl6421->override_and;
>>>> +     isl6421->reg1 |= isl6421->config->override_set;
>>>> +     isl6421->reg1 &= ~isl6421->config->override_clear;
>>>>
>>>>       return (i2c_transfer(isl6421->i2c, &msg, 1) == 1) ? 0 : -EIO;
>>>>  }
>>>>
>>>>  static void isl6421_release(struct dvb_frontend *fe)
>>>>  {
>>>> +     struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>>>> +
>>>>       /* power off */
>>>>       isl6421_set_voltage(fe, SEC_VOLTAGE_OFF);
>>>> +
>>>> +     if (isl6421->config->disable_overcurrent_protection)
>>>> +             fe->ops.diseqc_send_master_cmd =
>>>> +                     isl6421->diseqc_send_master_cmd_orig;
>>> You need to test if this function pointer were defined or not at the config struct.
>>>
>> Why? If original value was NULL, then there should be no problem
>> to reassign back to NULL.
>>
>>>>       /* free */
>>>>       kfree(fe->sec_priv);
>>>>       fe->sec_priv = NULL;
>>>>  }
>>>>
>>>> -struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe, struct i2c_adapter *i2c, u8 i2c_addr,
>>>> -                u8 override_set, u8 override_clear)
>>>> +struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe,
>>>> +                                        struct i2c_adapter *i2c,
>>>> +                                        const struct isl6421_config *config)
>>>>  {
>>>>       struct isl6421 *isl6421 = kmalloc(sizeof(struct isl6421), GFP_KERNEL);
>>>> +
>>>>       if (!isl6421)
>>>>               return NULL;
>>>>
>>>> -     /* default configuration */
>>>> -     isl6421->config = ISL6421_ISEL1;
>>>> +     isl6421->config = config;
>>>>       isl6421->i2c = i2c;
>>>> -     isl6421->i2c_addr = i2c_addr;
>>>>       fe->sec_priv = isl6421;
>>>>
>>>> -     /* bits which should be forced to '1' */
>>>> -     isl6421->override_or = override_set;
>>>> -
>>>> -     /* bits which should be forced to '0' */
>>>> -     isl6421->override_and = ~override_clear;
>>>> +     /* default configuration */
>>>> +     isl6421->reg1 = ISL6421_ISEL1;
>>>>
>>>>       /* detect if it is present or not */
>>>>       if (isl6421_set_voltage(fe, SEC_VOLTAGE_OFF)) {
>>>> @@ -131,11 +188,38 @@ struct dvb_frontend *isl6421_attach(stru
>>>>       /* override frontend ops */
>>>>       fe->ops.set_voltage = isl6421_set_voltage;
>>>>       fe->ops.enable_high_lnb_voltage = isl6421_enable_high_lnb_voltage;
>>>> +     if (config->tone_control)
>>>> +             fe->ops.set_tone = isl6421_set_tone;
>>>> +
>>>> +     printk(KERN_INFO "ISL6421 attached on addr=%x\n", config->i2c_addr);
>>>> +
>>>> +     if (config->disable_overcurrent_protection) {
>>>> +             if ((config->override_set & ISL6421_DCL) ||
>>>> +                             (config->override_clear & ISL6421_DCL)) {
>>>> +                     /* there is no sense to use overcurrent_enable
>>>> +                      * with DCL bit set in any override byte */
>>>> +                     if (config->override_set & ISL6421_DCL)
>>>> +                             printk(KERN_WARNING "ISL6421 overcurrent_enable"
>>>> +                                             " with DCL bit in override_set,"
>>>> +                                             " overcurrent_enable ignored\n");
>>>> +                     if (config->override_clear & ISL6421_DCL)
>>>> +                             printk(KERN_WARNING "ISL6421 overcurrent_enable"
>>>> +                                             " with DCL bit in override_clear,"
>>>> +                                             " overcurrent_enable ignored\n");
>>>> +             } else {
>>>> +                     printk(KERN_WARNING "ISL6421 overcurrent_enable "
>>>> +                                     " activated. WARNING: it can be "
>>>> +                                     " dangerous for your hardware!");
>>>> +                     isl6421->diseqc_send_master_cmd_orig =
>>>> +                             fe->ops.diseqc_send_master_cmd;
>>>> +                     fe->ops.diseqc_send_master_cmd = isl6421_send_diseqc;
>>>> +             }
>>>> +     }
>>>>
>>>>       return fe;
>>>>  }
>>>>  EXPORT_SYMBOL(isl6421_attach);
>>>>
>>>>  MODULE_DESCRIPTION("Driver for lnb supply and control ic isl6421");
>>>> -MODULE_AUTHOR("Andrew de Quincey & Oliver Endriss");
>>>> +MODULE_AUTHOR("Andrew de Quincey,Oliver Endriss,Ales Jurik,Jan Petrous");
>>>>  MODULE_LICENSE("GPL");
>>>> diff -r 79fc32bba0a0 linux/drivers/media/dvb/frontends/isl6421.h
>>>> --- a/linux/drivers/media/dvb/frontends/isl6421.h     Mon Dec 14 17:43:13 2009 -0200
>>>> +++ b/linux/drivers/media/dvb/frontends/isl6421.h     Tue Dec 15 16:36:14 2009 +0100
>>>> @@ -39,14 +39,40 @@
>>>>  #define ISL6421_ISEL1        0x20
>>>>  #define ISL6421_DCL  0x40
>>>>
>>>> +struct isl6421_config {
>>>> +     /* I2C address */
>>>> +     u8 i2c_addr;
>>>> +
>>>> +     /* Enable DISEqC tone control mode */
>>>> +     bool tone_control;
>>>> +
>>>> +     /*
>>>> +      * Disable isl6421 overcurrent protection.
>>>> +      *
>>>> +      * WARNING: Don't disable the protection unless you are 100% sure about
>>>> +      *          what you're doing, otherwise you may damage your board.
>>>> +      *          Only a few designs require to disable the protection, since
>>>> +      *          the hardware designer opted to use a hardware protection instead
>>>> +      */
>>>> +     bool disable_overcurrent_protection;
>>>> +
>>>> +     /* bits which should be forced to '1' */
>>>> +     u8 override_set;
>>>> +
>>>> +     /* bits which should be forced to '0' */
>>>> +     u8 override_clear;
>>>> +};
>>>> +
>>>> +
>>>>  #if defined(CONFIG_DVB_ISL6421) || (defined(CONFIG_DVB_ISL6421_MODULE) && defined(MODULE))
>>>>  /* override_set and override_clear control which system register bits (above) to always set & clear */
>>>> -extern struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe, struct i2c_adapter *i2c, u8 i2c_addr,
>>>> -                       u8 override_set, u8 override_clear);
>>>> +extern struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe,
>>>> +                                        struct i2c_adapter *i2c,
>>>> +                                        const struct isl6421_config *config);
>>>>  #else
>>>> -static inline struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe, struct i2c_adapter *i2c, u8 i2c_addr,
>>>> -                                               u8 override_set, u8 override_clear)
>>>> -{
>>>> +static struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe,
>>>> +                                        struct i2c_adapter *i2c,
>>>> +                                        const struct isl6421_config *config);
>>>>       printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
>>>>       return NULL;
>>>>  }
>>>> diff -r 79fc32bba0a0 linux/drivers/media/video/cx88/cx88-dvb.c
>>>> --- a/linux/drivers/media/video/cx88/cx88-dvb.c       Mon Dec 14 17:43:13 2009 -0200
>>>> +++ b/linux/drivers/media/video/cx88/cx88-dvb.c       Tue Dec 15 16:36:14 2009 +0100
>>>> @@ -456,6 +456,12 @@ static struct cx24123_config hauppauge_n
>>>>       .set_ts_params = cx24123_set_ts_param,
>>>>  };
>>>>
>>>> +static struct isl6421_config hauppauge_novas_isl6421_config = {
>>>> +     .i2c_address = 0x08,
>>>> +     .override_set = ISL6421_DCL,
>>>> +     .override_clear = 0x00,
>>> Don't initialize a value with zero.
>>>
>> Done.
>>
>>>> +};
>>>> +
>>>>  static struct cx24123_config kworld_dvbs_100_config = {
>>>>       .demod_address = 0x15,
>>>>       .set_ts_params = cx24123_set_ts_param,
>>>> @@ -614,6 +620,12 @@ static struct cx24116_config hauppauge_h
>>>>       .reset_device           = cx24116_reset_device,
>>>>  };
>>>>
>>>> +static struct isl6421_config hauppauge_hvr4000_isl6421_config = {
>>>> +     .i2c_address = 0x08,
>>>> +     .override_set = ISL6421_DCL,
>>>> +     .override_clear = 0x00,
>>> Don't initialize a value with zero.
>>>
>> Done.
>>
>>>> +};
>>>> +
>>>>  static struct cx24116_config tevii_s460_config = {
>>>>       .demod_address = 0x55,
>>>>       .set_ts_params = cx24116_set_ts_param,
>>>> @@ -757,7 +769,7 @@ static int dvb_register(struct cx8802_de
>>>>                       if (!dvb_attach(isl6421_attach,
>>>>                                       fe0->dvb.frontend,
>>>>                                       &dev->core->i2c_adap,
>>>> -                                     0x08, ISL6421_DCL, 0x00))
>>>> +                                     &hauppauge_novas_isl6421_config))
>>>>                               goto frontend_detach;
>>>>               }
>>>>               /* MFE frontend 2 */
>>>> @@ -995,7 +1007,8 @@ static int dvb_register(struct cx8802_de
>>>>                                              &core->i2c_adap);
>>>>               if (fe0->dvb.frontend) {
>>>>                       if (!dvb_attach(isl6421_attach, fe0->dvb.frontend,
>>>> -                                     &core->i2c_adap, 0x08, ISL6421_DCL, 0x00))
>>>> +                                     &core->i2c_adap,
>>>> +                                     &hauppauge_novas_isl6421_config))
>>>>                               goto frontend_detach;
>>>>               }
>>>>               break;
>>>> @@ -1100,7 +1113,7 @@ static int dvb_register(struct cx8802_de
>>>>                       if (!dvb_attach(isl6421_attach,
>>>>                                       fe0->dvb.frontend,
>>>>                                       &dev->core->i2c_adap,
>>>> -                                     0x08, ISL6421_DCL, 0x00))
>>>> +                                     &hauppauge_hvr4000_isl6421_config))
>>>>                               goto frontend_detach;
>>>>               }
>>>>               /* MFE frontend 2 */
>>>> @@ -1128,7 +1141,7 @@ static int dvb_register(struct cx8802_de
>>>>                       if (!dvb_attach(isl6421_attach,
>>>>                                       fe0->dvb.frontend,
>>>>                                       &dev->core->i2c_adap,
>>>> -                                     0x08, ISL6421_DCL, 0x00))
>>>> +                                     &hauppauge_hvr4000_isl6421_config))
>>>>                               goto frontend_detach;
>>>>               }
>>>>               break;
>>>> diff -r 79fc32bba0a0 linux/drivers/media/video/saa7134/saa7134-dvb.c
>>>> --- a/linux/drivers/media/video/saa7134/saa7134-dvb.c Mon Dec 14 17:43:13 2009 -0200
>>>> +++ b/linux/drivers/media/video/saa7134/saa7134-dvb.c Tue Dec 15 16:36:14 2009 +0100
>>>> @@ -716,6 +716,12 @@ static struct tda1004x_config lifeview_t
>>>>       .request_firmware = philips_tda1004x_request_firmware
>>>>  };
>>>>
>>>> +static struct isl6421_config lifeview_trio_isl6421_config = {
>>>> +     .i2c_address = 0x08,
>>>> +     .override_set = 0x00,
>>>> +     .override_clear = 0x00,
>>> Don't initialize a value with zero.
>>>
>> Done.
>>
>>>> +};
>>>> +
>>>>  static struct tda1004x_config tevion_dvbt220rf_config = {
>>>>       .demod_address = 0x08,
>>>>       .invert        = 1,
>>>> @@ -895,6 +901,12 @@ static struct tda10086_config flydvbs =
>>>>       .invert = 0,
>>>>       .diseqc_tone = 0,
>>>>       .xtal_freq = TDA10086_XTAL_16M,
>>>> +};
>>>> +
>>>> +static struct isl6421_config flydvbs_isl6421_config = {
>>>> +     .i2c_address = 0x08,
>>>> +     .override_set = 0x00,
>>>> +     .override_clear = 0x00,
>>> Don't initialize a value with zero.
>>>
>> Done.
>>
>>>>  };
>>>>
>>>>  static struct tda10086_config sd1878_4m = {
>>>> @@ -1248,7 +1260,7 @@ static int dvb_init(struct saa7134_dev *
>>>>                                       goto dettach_frontend;
>>>>                               }
>>>>                               if (dvb_attach(isl6421_attach, fe0->dvb.frontend, &dev->i2c_adap,
>>>> -                                                                             0x08, 0, 0) == NULL) {
>>>> +                                                                     &lifeview_trio_isl6421_config) == NULL) {
>>>>                                       wprintk("%s: Lifeview Trio, No ISL6421 found!\n", __func__);
>>>>                                       goto dettach_frontend;
>>>>                               }
>>>> @@ -1349,7 +1361,8 @@ static int dvb_init(struct saa7134_dev *
>>>>                               goto dettach_frontend;
>>>>                       }
>>>>                       if (dvb_attach(isl6421_attach, fe0->dvb.frontend,
>>>> -                                    &dev->i2c_adap, 0x08, 0, 0) == NULL) {
>>>> +                                    &dev->i2c_adap,
>>>> +                                    &flydvbs_isl6421_config) == NULL) {
>>>>                               wprintk("%s: No ISL6421 found!\n", __func__);
>>>>                               goto dettach_frontend;
>>>>                       }
>>>
>>>
>> I hope mailer not squash patch somehow
>>
>> Regards
>>
>> /Honza
>>
>> ---
>>
>> isl6421.c - added tone control and temporary diseqc overcurrent
>>
>> Signed-off-by: Jan Petrous <jpetrous@xxxxxxxxx>
>> Signed-off-by: Ales Jurik <ajurik@xxxxxxxx>
>>
>> diff -r 79fc32bba0a0 linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c
>> --- a/linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c   Mon Dec 14
>> 17:43:13 2009 -0200
>> +++ b/linux/drivers/media/dvb/b2c2/flexcop-fe-tuner.c   Wed Dec 16
>> 01:08:05 2009 +0100
>> @@ -302,6 +302,12 @@ static struct itd1000_config skystar2_re
>>        .i2c_address = 0x61,
>>  };
>>
>> +static struct isl6421_config skystar2_rev2_7_isl6421_config = {
>> +       .i2c_address = 0x08,
>> +       .override_set = 0x01,
>> +       .override_clear = 0x01,
>> +};
>> +
>>  static int skystar2_rev27_attach(struct flexcop_device *fc,
>>        struct i2c_adapter *i2c)
>>  {
>> @@ -325,7 +331,7 @@ static int skystar2_rev27_attach(struct
>>        /* enable no_base_addr - no repeated start when reading */
>>        fc->fc_i2c_adap[2].no_base_addr = 1;
>>        if (!dvb_attach(isl6421_attach, fc->fe, &fc->fc_i2c_adap[2].i2c_adap,
>> -                       0x08, 1, 1)) {
>> +                       &skystar2_rev2_7_isl6421_config)) {
>>                err("ISL6421 could NOT be attached");
>>                goto fail_isl;
>>        }
>> @@ -368,6 +374,10 @@ static const struct cx24113_config skyst
>>        .xtal_khz = 10111,
>>  };
>>
>> +static struct isl6421_config skystar2_rev2_8_isl6421_config = {
>> +       .i2c_address = 0x08,
>> +};
>> +
>>  static int skystar2_rev28_attach(struct flexcop_device *fc,
>>        struct i2c_adapter *i2c)
>>  {
>> @@ -391,7 +401,7 @@ static int skystar2_rev28_attach(struct
>>
>>        fc->fc_i2c_adap[2].no_base_addr = 1;
>>        if (!dvb_attach(isl6421_attach, fc->fe, &fc->fc_i2c_adap[2].i2c_adap,
>> -                       0x08, 0, 0)) {
>> +                       &skystar2_rev2_8_isl6421_config)) {
>>                err("ISL6421 could NOT be attached");
>>                fc->fc_i2c_adap[2].no_base_addr = 0;
>>                return 0;
>> diff -r 79fc32bba0a0 linux/drivers/media/dvb/frontends/isl6421.c
>> --- a/linux/drivers/media/dvb/frontends/isl6421.c       Mon Dec 14 17:43:13 2009 -0200
>> +++ b/linux/drivers/media/dvb/frontends/isl6421.c       Wed Dec 16 01:08:05 2009 +0100
>> @@ -3,6 +3,9 @@
>>  *
>>  * Copyright (C) 2006 Andrew de Quincey
>>  * Copyright (C) 2006 Oliver Endriss
>> + * Copyright (C) 2009 Ales Jurik and Jan Petrous (added optional 22k tone
>> + *                    support and temporary diseqc overcurrent enable until
>> + *                    next command - set voltage or tone)
>>  *
>>  * This program is free software; you can redistribute it and/or
>>  * modify it under the terms of the GNU General Public License
>> @@ -36,37 +39,91 @@
>>  #include "isl6421.h"
>>
>>  struct isl6421 {
>> -       u8                      config;
>> -       u8                      override_or;
>> -       u8                      override_and;
>> -       struct i2c_adapter      *i2c;
>> -       u8                      i2c_addr;
>> +       const struct isl6421_config     *config;
>> +       u8                              sys_reg;
>> +
>> +       struct i2c_adapter *i2c;
>> +
>> +       int (*diseqc_send_master_cmd_orig)(struct dvb_frontend *fe,
>> +                       struct dvb_diseqc_master_cmd *cmd);
>>  };
>>
>>  static int isl6421_set_voltage(struct dvb_frontend *fe,
>> fe_sec_voltage_t voltage)
>>  {
>>        struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>> -       struct i2c_msg msg = {  .addr = isl6421->i2c_addr, .flags = 0,
>> -                               .buf = &isl6421->config,
>> -                               .len = sizeof(isl6421->config) };
>> +       struct i2c_msg msg = {  .addr = isl6421->config->i2c_addr, .flags = 0,
>> +                               .buf = &isl6421->sys_reg,
>> +                               .len = sizeof(isl6421->sys_reg) };
>>
>> -       isl6421->config &= ~(ISL6421_VSEL1 | ISL6421_EN1);
>> +       isl6421->sys_reg &= ~(ISL6421_VSEL1 | ISL6421_EN1);
>>
>>        switch(voltage) {
>>        case SEC_VOLTAGE_OFF:
>>                break;
>>        case SEC_VOLTAGE_13:
>> -               isl6421->config |= ISL6421_EN1;
>> +               isl6421->sys_reg |= ISL6421_EN1;
>>                break;
>>        case SEC_VOLTAGE_18:
>> -               isl6421->config |= (ISL6421_EN1 | ISL6421_VSEL1);
>> +               isl6421->sys_reg |= (ISL6421_EN1 | ISL6421_VSEL1);
>>                break;
>>        default:
>>                return -EINVAL;
>>        };
>>
>> -       isl6421->config |= isl6421->override_or;
>> -       isl6421->config &= isl6421->override_and;
>> +       isl6421->sys_reg |= isl6421->config->override_set;
>> +       isl6421->sys_reg &= ~isl6421->config->override_clear;
>> +
>> +       return (i2c_transfer(isl6421->i2c, &msg, 1) == 1) ? 0 : -EIO;
>> +}
>> +
>> +/* Hooked to fe->ops.diseqc_send_master_cmd() only
>> +   when disable_overcurrent_protection=1 */
>> +static int isl6421_send_diseqc(struct dvb_frontend *fe,
>> +                               struct dvb_diseqc_master_cmd *cmd)
>> +{
>> +       struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>> +       struct i2c_msg msg = {  .addr = isl6421->config->i2c_addr, .flags = 0,
>> +                               .buf = &isl6421->sys_reg,
>> +                               .len = sizeof(isl6421->sys_reg) };
>> +
>> +       isl6421->sys_reg |= ISL6421_DCL;
>> +
>> +       isl6421->sys_reg |= isl6421->config->override_set;
>> +       isl6421->sys_reg &= ~isl6421->config->override_clear;
>> +
>> +       if (i2c_transfer(isl6421->i2c, &msg, 1) != 1)
>> +               return -EIO;
>> +
>> +       isl6421->sys_reg &= ~ISL6421_DCL;
>> +
>> +       return isl6421->diseqc_send_master_cmd_orig ?
>> +               isl6421->diseqc_send_master_cmd_orig(fe, cmd) : -EIO;
>> +}
>> +
>> +static int isl6421_set_tone(struct dvb_frontend *fe, fe_sec_tone_mode_t tone)
>> +{
>> +       struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>> +       struct i2c_msg msg = {  .addr = isl6421->config->i2c_addr, .flags = 0,
>> +                               .buf = &isl6421->sys_reg,
>> +                               .len = sizeof(isl6421->sys_reg) };
>> +
>> +       isl6421->sys_reg &= ~(ISL6421_ENT1);
>> +
>> +       printk(KERN_INFO "%s: %s\n", __func__, ((tone == SEC_TONE_OFF) ?
>> +                               "Off" : "On"));
>> +
>> +       switch (tone) {
>> +       case SEC_TONE_ON:
>> +               isl6421->sys_reg |= ISL6421_ENT1;
>> +               break;
>> +       case SEC_TONE_OFF:
>> +               break;
>> +       default:
>> +               return -EINVAL;
>> +       };
>> +
>> +       isl6421->sys_reg |= isl6421->config->override_set;
>> +       isl6421->sys_reg &= ~isl6421->config->override_clear;
>>
>>        return (i2c_transfer(isl6421->i2c, &msg, 1) == 1) ? 0 : -EIO;
>>  }
>> @@ -74,49 +131,52 @@ static int isl6421_enable_high_lnb_volta
>>  static int isl6421_enable_high_lnb_voltage(struct dvb_frontend *fe, long arg)
>>  {
>>        struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>> -       struct i2c_msg msg = {  .addr = isl6421->i2c_addr, .flags = 0,
>> -                               .buf = &isl6421->config,
>> -                               .len = sizeof(isl6421->config) };
>> +       struct i2c_msg msg = {  .addr = isl6421->config->i2c_addr, .flags = 0,
>> +                               .buf = &isl6421->sys_reg,
>> +                               .len = sizeof(isl6421->sys_reg) };
>>
>>        if (arg)
>> -               isl6421->config |= ISL6421_LLC1;
>> +               isl6421->sys_reg |= ISL6421_LLC1;
>>        else
>> -               isl6421->config &= ~ISL6421_LLC1;
>> +               isl6421->sys_reg &= ~ISL6421_LLC1;
>>
>> -       isl6421->config |= isl6421->override_or;
>> -       isl6421->config &= isl6421->override_and;
>> +       isl6421->sys_reg |= isl6421->config->override_set;
>> +       isl6421->sys_reg &= ~isl6421->config->override_clear;
>>
>>        return (i2c_transfer(isl6421->i2c, &msg, 1) == 1) ? 0 : -EIO;
>>  }
>>
>>  static void isl6421_release(struct dvb_frontend *fe)
>>  {
>> +       struct isl6421 *isl6421 = (struct isl6421 *) fe->sec_priv;
>> +
>>        /* power off */
>>        isl6421_set_voltage(fe, SEC_VOLTAGE_OFF);
>> +
>> +       if (isl6421->config->disable_overcurrent_protection)
>> +               fe->ops.diseqc_send_master_cmd =
>> +                       isl6421->diseqc_send_master_cmd_orig;
>>
>>        /* free */
>>        kfree(fe->sec_priv);
>>        fe->sec_priv = NULL;
>>  }
>>
>> -struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe, struct
>> i2c_adapter *i2c, u8 i2c_addr,
>> -                  u8 override_set, u8 override_clear)
>> +struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe,
>> +                                          struct i2c_adapter *i2c,
>> +                                          const struct isl6421_config *config)
>>  {
>>        struct isl6421 *isl6421 = kmalloc(sizeof(struct isl6421), GFP_KERNEL);
>> +
>>        if (!isl6421)
>>                return NULL;
>>
>> -       /* default configuration */
>> -       isl6421->config = ISL6421_ISEL1;
>> +       isl6421->config = config;
>>        isl6421->i2c = i2c;
>> -       isl6421->i2c_addr = i2c_addr;
>>        fe->sec_priv = isl6421;
>>
>> -       /* bits which should be forced to '1' */
>> -       isl6421->override_or = override_set;
>> -
>> -       /* bits which should be forced to '0' */
>> -       isl6421->override_and = ~override_clear;
>> +       /* default configuration */
>> +       isl6421->sys_reg = ISL6421_ISEL1;
>>
>>        /* detect if it is present or not */
>>        if (isl6421_set_voltage(fe, SEC_VOLTAGE_OFF)) {
>> @@ -131,11 +191,38 @@ struct dvb_frontend *isl6421_attach(stru
>>        /* override frontend ops */
>>        fe->ops.set_voltage = isl6421_set_voltage;
>>        fe->ops.enable_high_lnb_voltage = isl6421_enable_high_lnb_voltage;
>> +       if (config->tone_control)
>> +               fe->ops.set_tone = isl6421_set_tone;
>> +
>> +       printk(KERN_INFO "ISL6421 attached on addr=%x\n", config->i2c_addr);
>> +
>> +       if (config->disable_overcurrent_protection) {
>> +               if ((config->override_set & ISL6421_DCL) ||
>> +                               (config->override_clear & ISL6421_DCL)) {
>> +                       /* there is no sense to use overcurrent_enable
>> +                        * with DCL bit set in any override byte */
>> +                       if (config->override_set & ISL6421_DCL)
>> +                               printk(KERN_WARNING "ISL6421 overcurrent_enable"
>> +                                               " with DCL bit in override_set,"
>> +                                               " overcurrent_enable ignored\n");
>> +                       if (config->override_clear & ISL6421_DCL)
>> +                               printk(KERN_WARNING "ISL6421 overcurrent_enable"
>> +                                               " with DCL bit in override_clear,"
>> +                                               " overcurrent_enable ignored\n");
>> +               } else {
>> +                       printk(KERN_WARNING "ISL6421 overcurrent_enable "
>> +                                       " activated. WARNING: it can be "
>> +                                       " dangerous for your hardware!");
>> +                       isl6421->diseqc_send_master_cmd_orig =
>> +                               fe->ops.diseqc_send_master_cmd;
>> +                       fe->ops.diseqc_send_master_cmd = isl6421_send_diseqc;
>> +               }
>> +       }
>>
>>        return fe;
>>  }
>>  EXPORT_SYMBOL(isl6421_attach);
>>
>>  MODULE_DESCRIPTION("Driver for lnb supply and control ic isl6421");
>> -MODULE_AUTHOR("Andrew de Quincey & Oliver Endriss");
>> +MODULE_AUTHOR("Andrew de Quincey,Oliver Endriss,Ales Jurik,Jan Petrous");
>>  MODULE_LICENSE("GPL");
>> diff -r 79fc32bba0a0 linux/drivers/media/dvb/frontends/isl6421.h
>> --- a/linux/drivers/media/dvb/frontends/isl6421.h       Mon Dec 14 17:43:13 2009 -0200
>> +++ b/linux/drivers/media/dvb/frontends/isl6421.h       Wed Dec 16 01:08:05 2009 +0100
>> @@ -39,14 +39,40 @@
>>  #define ISL6421_ISEL1  0x20
>>  #define ISL6421_DCL    0x40
>>
>> +struct isl6421_config {
>> +       /* I2C address */
>> +       u8 i2c_addr;
>> +
>> +       /* Enable DISEqC tone control mode */
>> +       bool tone_control;
>> +
>> +       /*
>> +        * Disable isl6421 overcurrent protection.
>> +        *
>> +        * WARNING: Don't disable the protection unless you are 100% sure about
>> +        *          what you're doing, otherwise you may damage your board.
>> +        *          Only a few designs require to disable the protection, since
>> +        *          the hardware designer opted to use a hardware protection instead
>> +        */
>> +       bool disable_overcurrent_protection;
>> +
>> +       /* bits which should be forced to '1' */
>> +       u8 override_set;
>> +
>> +       /* bits which should be forced to '0' */
>> +       u8 override_clear;
>> +};
>> +
>> +
>>  #if defined(CONFIG_DVB_ISL6421) ||
>> (defined(CONFIG_DVB_ISL6421_MODULE) && defined(MODULE))
>>  /* override_set and override_clear control which system register bits
>> (above) to always set & clear */
>> -extern struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe,
>> struct i2c_adapter *i2c, u8 i2c_addr,
>> -                         u8 override_set, u8 override_clear);
>> +extern struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe,
>> +                                          struct i2c_adapter *i2c,
>> +                                          const struct isl6421_config *config);
>>  #else
>> -static inline struct dvb_frontend *isl6421_attach(struct dvb_frontend
>> *fe, struct i2c_adapter *i2c, u8 i2c_addr,
>> -                                                 u8 override_set, u8 override_clear)
>> -{
>> +static struct dvb_frontend *isl6421_attach(struct dvb_frontend *fe,
>> +                                          struct i2c_adapter *i2c,
>> +                                          const struct isl6421_config *config);
>>        printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
>>        return NULL;
>>  }
>> diff -r 79fc32bba0a0 linux/drivers/media/video/cx88/cx88-dvb.c
>> --- a/linux/drivers/media/video/cx88/cx88-dvb.c Mon Dec 14 17:43:13 2009 -0200
>> +++ b/linux/drivers/media/video/cx88/cx88-dvb.c Wed Dec 16 01:08:05 2009 +0100
>> @@ -456,6 +456,11 @@ static struct cx24123_config hauppauge_n
>>        .set_ts_params = cx24123_set_ts_param,
>>  };
>>
>> +static struct isl6421_config hauppauge_novas_isl6421_config = {
>> +       .i2c_address = 0x08,
>> +       .override_set = ISL6421_DCL,
>> +};
>> +
>>  static struct cx24123_config kworld_dvbs_100_config = {
>>        .demod_address = 0x15,
>>        .set_ts_params = cx24123_set_ts_param,
>> @@ -614,6 +619,11 @@ static struct cx24116_config hauppauge_h
>>        .reset_device           = cx24116_reset_device,
>>  };
>>
>> +static struct isl6421_config hauppauge_hvr4000_isl6421_config = {
>> +       .i2c_address = 0x08,
>> +       .override_set = ISL6421_DCL,
>> +};
>> +
>>  static struct cx24116_config tevii_s460_config = {
>>        .demod_address = 0x55,
>>        .set_ts_params = cx24116_set_ts_param,
>> @@ -757,7 +767,7 @@ static int dvb_register(struct cx8802_de
>>                        if (!dvb_attach(isl6421_attach,
>>                                        fe0->dvb.frontend,
>>                                        &dev->core->i2c_adap,
>> -                                       0x08, ISL6421_DCL, 0x00))
>> +                                       &hauppauge_novas_isl6421_config))
>>                                goto frontend_detach;
>>                }
>>                /* MFE frontend 2 */
>> @@ -995,7 +1005,8 @@ static int dvb_register(struct cx8802_de
>>                                               &core->i2c_adap);
>>                if (fe0->dvb.frontend) {
>>                        if (!dvb_attach(isl6421_attach, fe0->dvb.frontend,
>> -                                       &core->i2c_adap, 0x08, ISL6421_DCL, 0x00))
>> +                                       &core->i2c_adap,
>> +                                       &hauppauge_novas_isl6421_config))
>>                                goto frontend_detach;
>>                }
>>                break;
>> @@ -1100,7 +1111,7 @@ static int dvb_register(struct cx8802_de
>>                        if (!dvb_attach(isl6421_attach,
>>                                        fe0->dvb.frontend,
>>                                        &dev->core->i2c_adap,
>> -                                       0x08, ISL6421_DCL, 0x00))
>> +                                       &hauppauge_hvr4000_isl6421_config))
>>                                goto frontend_detach;
>>                }
>>                /* MFE frontend 2 */
>> @@ -1128,7 +1139,7 @@ static int dvb_register(struct cx8802_de
>>                        if (!dvb_attach(isl6421_attach,
>>                                        fe0->dvb.frontend,
>>                                        &dev->core->i2c_adap,
>> -                                       0x08, ISL6421_DCL, 0x00))
>> +                                       &hauppauge_hvr4000_isl6421_config))
>>                                goto frontend_detach;
>>                }
>>                break;
>> diff -r 79fc32bba0a0 linux/drivers/media/video/saa7134/saa7134-dvb.c
>> --- a/linux/drivers/media/video/saa7134/saa7134-dvb.c   Mon Dec 14
>> 17:43:13 2009 -0200
>> +++ b/linux/drivers/media/video/saa7134/saa7134-dvb.c   Wed Dec 16
>> 01:08:05 2009 +0100
>> @@ -716,6 +716,10 @@ static struct tda1004x_config lifeview_t
>>        .request_firmware = philips_tda1004x_request_firmware
>>  };
>>
>> +static struct isl6421_config lifeview_trio_isl6421_config = {
>> +       .i2c_address = 0x08,
>> +};
>> +
>>  static struct tda1004x_config tevion_dvbt220rf_config = {
>>        .demod_address = 0x08,
>>        .invert        = 1,
>> @@ -895,6 +899,10 @@ static struct tda10086_config flydvbs =
>>        .invert = 0,
>>        .diseqc_tone = 0,
>>        .xtal_freq = TDA10086_XTAL_16M,
>> +};
>> +
>> +static struct isl6421_config flydvbs_isl6421_config = {
>> +       .i2c_address = 0x08,
>>  };
>>
>>  static struct tda10086_config sd1878_4m = {
>> @@ -1248,7 +1256,7 @@ static int dvb_init(struct saa7134_dev *
>>                                        goto dettach_frontend;
>>                                }
>>                                if (dvb_attach(isl6421_attach, fe0->dvb.frontend, &dev->i2c_adap,
>> -                                                                               0x08, 0, 0) == NULL) {
>> +                                                                       &lifeview_trio_isl6421_config) == NULL) {
>>                                        wprintk("%s: Lifeview Trio, No ISL6421 found!\n", __func__);
>>                                        goto dettach_frontend;
>>                                }
>> @@ -1349,7 +1357,8 @@ static int dvb_init(struct saa7134_dev *
>>                                goto dettach_frontend;
>>                        }
>>                        if (dvb_attach(isl6421_attach, fe0->dvb.frontend,
>> -                                      &dev->i2c_adap, 0x08, 0, 0) == NULL) {
>> +                                      &dev->i2c_adap,
>> +                                      &flydvbs_isl6421_config) == NULL) {
>>                                wprintk("%s: No ISL6421 found!\n", __func__);
>>                                goto dettach_frontend;
>>                        }
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux