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