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