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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
