Le 23/01/2023 à 09:56, Herve Codina a écrit :
>>> +
>>> +static int idt821034_set_channel_power(struct idt821034 *idt821034, u8 ch, u8 power)
>>> +{
>>> + u8 conf;
>>> + int ret;
>>> +
>>> + dev_dbg(&idt821034->spi->dev, "set_channel_power(%u, 0x%x)\n", ch, power);
>>> +
>>> + conf = IDT821034_MODE_CODEC(ch) | idt821034->cache.codec_conf;
>>> +
>>> + if (power & IDT821034_CONF_PWRUP_RX) {
>>> + ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_RX);
>>> + if (ret)
>>> + return ret;
>>> + ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].rx_slot);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> + if (power & IDT821034_CONF_PWRUP_TX) {
>>> + ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_TX);
>>> + if (ret)
>>> + return ret;
>>> + ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].tx_slot);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> + if (!(power & (IDT821034_CONF_PWRUP_TX | IDT821034_CONF_PWRUP_RX))) {
>>> + ret = idt821034_8bit_write(idt821034, conf);
>>> + if (ret)
>>> + return ret;
>>> + ret = idt821034_8bit_write(idt821034, 0x00);
>>> + if (ret)
>>> + return ret;
>>> + }
>>
>> Can we refactor the three actions with an helper, that could also be
>> reused for idt821034_set_codec_conf() and idt821034_set_channel_ts() and
>> idt821034_set_slic_conf() and idt821034_write_slic_raw() and
>> idt821034_set_gain_channel, something like for instance:
>>
>> static int idt821034_set_conf(struct idt821034 *idt821034, u8 conf, u8 val)
>> {
>> ret = idt821034_8bit_write(idt821034, conf);
>> if (ret)
>> return ret;
>> return idt821034_8bit_write(idt821034, val);
>> }
>
> It can be changed.
> The function name will not be idt821034_set_conf() as it is not the same
> kind of funtion as the idt821031_set_*() already available in the code.
> What do you think about:
> static int idt821034_2x8bit_write(struct idt821034 *idt821034, u8 val1, u8 val2)
> or
> static int idt821034_conf_write(struct idt821034 *idt821034, u8 conf, u8 val)
>
> I prefer the first one but it is only a personal preference.
> On your side, what do you prefer ?
idt821034_2x8bit_write() looks good to me.
Christophe
