Re: [PATCH v3 05/14] [media] cxd2880: Add tuner part of the driver

[Takiguchi, Yasunari]

Dear Mauro

Thanks for your review and reply.

We are going to discuss how to change our code with your comments internally.

I reply for your  2 comments,

>> [Change list]
>> Changes in V3
>>    drivers/media/dvb-frontends/cxd2880/cxd2880_dtv.h
>>       -removed code relevant to ISDB-T
> 
> Just curiosity here: why is it removed?
We decided to withhold the ISDB-T functionality as it contains some company 
proprietary code.


>> +    if (ret)
>> +            return ret;
>> +    if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl) {
>> +            data[0] = 0x01;
>> +            data[1] = 0x01;
>> +            data[2] = 0x01;
>> +            data[3] = 0x01;
>> +            data[4] = 0x01;
>> +            data[5] = 0x01;
>> +    } else {
>> +            data[0] = 0x00;
>> +            data[1] = 0x00;
>> +            data[2] = 0x00;
>> +            data[3] = 0x00;
>> +            data[4] = 0x00;
>> +            data[5] = 0x00;
>> +    }
> 
> Instead, just do:
> 
>       if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
>               memset(data, 0x01, sizeof(data));
>       else
>               memset(data, 0x00, sizeof(data));
> 
>> +    ret = tnr_dmd->io->write_regs(tnr_dmd->io,
>> +                                  CXD2880_IO_TGT_SYS,
>> +                                  0xef, data, 6);
>> +    if (ret)
>> +            return ret;
>> +
>> +    ret = tnr_dmd->io->write_reg(tnr_dmd->io,
>> +                                 CXD2880_IO_TGT_DMD,
>> +                                 0x00, 0x2d);
>> +    if (ret)
>> +            return ret;
> 
>> +    if ((sys == CXD2880_DTV_SYS_DVBT2) && en_fef_intmtnt_ctrl)
>> +            data[0] = 0x00;
>> +    else
>> +            data[0] = 0x01;
> 
> Not actually needed, as the previous logic already set data[0]
> accordingly.
> 
>> +    ret = tnr_dmd->io->write_reg(tnr_dmd->io,
>> +                                 CXD2880_IO_TGT_DMD,
>> +                                 0xb1, data[0]);

In this case、logic of data[0]( logic of if() ) is different from that of 
previous one.
And with setting register for address 0xb1, a bug might occur in the future, 
if our software specification (sequence) is changed.
So we would like to keep setting value of data[0] for address 0xb1.

Thanks,
Takiguchi