| Message ID | 20210319092919.21218-1-srinivas.kandagatla@linaro.org |
|---|---|
| Headers | show |
| Series | ASoC: codecs: add wcd938x support | expand |
On 3/19/21 4:29 AM, Srinivas Kandagatla wrote: > This patchset adds support for Qualcomm WCD938X codec. > > Qualcomm WCD9380/WCD9385 Codec is a standalone Hi-Fi audio codec IC > connected over SoundWire. This device has two SoundWire devices, RX and > TX respectively supporting 4 x ADCs, ClassH, Ear, Aux PA, 2xHPH, > 7 x TX diff inputs, 8 DMICs and MBHC. > > Even though this device has two SoundWire devices, only tx device has > access to main codec Control/Status Registers! That part is a new concept we haven't seen so far with SoundWire support, and I added a number of comments in the patches. It would really help if you could add more explanations on how regmap/pm_runtime/gpios/regulators/interrupts are supposed to work with such a functional split. Thanks! > This patchset along with other SoundWire patches on the list > have been tested on SM8250 MTP device. > > Am planning to send support for MBHC once this driver gets accepted! > > Thanks, > srini > > Many thanks for reviewing v2. > > > Changes since v2: > - fixed dt_binding_check error > > > Srinivas Kandagatla (7): > ASoC: dt-bindings: wcd938x: add bindings for wcd938x > ASoC: codecs: wcd-clsh: add new version support > ASoC: codecs: wcd938x: add basic driver > ASoC: codecs: wcd938x: add basic controls > ASoC: codecs: wcd938x: add playback dapm widgets > ASoC: codecs: wcd938x: add capture dapm widgets > ASoC: codecs: wcd938x: add audio routing > > .../bindings/sound/qcom,wcd938x.yaml | 165 + > sound/soc/codecs/Kconfig | 9 + > sound/soc/codecs/Makefile | 2 + > sound/soc/codecs/wcd-clsh-v2.c | 350 +- > sound/soc/codecs/wcd-clsh-v2.h | 16 + > sound/soc/codecs/wcd938x-sdw.c | 291 ++ > sound/soc/codecs/wcd938x.c | 3623 +++++++++++++++++ > sound/soc/codecs/wcd938x.h | 676 +++ > 8 files changed, 5122 insertions(+), 10 deletions(-) > create mode 100644 Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml > create mode 100644 sound/soc/codecs/wcd938x-sdw.c > create mode 100644 sound/soc/codecs/wcd938x.c > create mode 100644 sound/soc/codecs/wcd938x.h >
On 19/03/2021 15:23, Pierre-Louis Bossart wrote: > >> +static void wcd_clsh_v3_set_hph_mode(struct snd_soc_component >> *component, >> + int mode) >> +{ >> + u8 val = 0; > > initialization not needed. > I agree, will remove this in next spin. >> + >> + switch (mode) { >> + case CLS_H_NORMAL: >> + val = 0x00; >> + break; >> + case CLS_AB: >> + case CLS_H_ULP: >> + val = 0x0C; >> + break; >> + case CLS_AB_HIFI: >> + case CLS_H_HIFI: >> + val = 0x08; >> + break; >> + case CLS_H_LP: >> + case CLS_H_LOHIFI: >> + case CLS_AB_LP: >> + case CLS_AB_LOHIFI: >> + val = 0x04; >> + break; >> + default: >> + dev_err(component->dev, "%s:Invalid mode %d\n", __func__, mode); >> + return; >> + }; >> + >> + snd_soc_component_update_bits(component, WCD9XXX_ANA_HPH, 0x0C, >> val); >> +} >> + >> + > > extra line > will remove this in next spin. >> +void wcd_clsh_set_hph_mode(struct wcd_clsh_ctrl *ctrl, int mode) >> +{ >> + struct snd_soc_component *comp = ctrl->comp; >> + >> + if (ctrl->codec_version >= WCD937X) >> + wcd_clsh_v3_set_hph_mode(comp, mode); >> + else >> + wcd_clsh_v2_set_hph_mode(comp, mode); >> + >> +} >> + >> static void wcd_clsh_set_flyback_current(struct snd_soc_component >> *comp, >> int mode) >> { >> @@ -289,6 +388,130 @@ static void >> wcd_clsh_set_buck_regulator_mode(struct snd_soc_component *comp, >> WCD9XXX_A_ANA_RX_REGULATOR_MODE_CLS_H); >> } >> +static void wcd_clsh_v3_set_buck_regulator_mode(struct >> snd_soc_component *component, >> + int mode) >> +{ >> + snd_soc_component_update_bits(component, WCD9XXX_ANA_RX_SUPPLIES, >> + 0x02, 0x00); >> +} >> + >> +static inline void wcd_clsh_v3_set_flyback_mode(struct >> snd_soc_component *component, >> + int mode) >> +{ >> + if (mode == CLS_H_HIFI || mode == CLS_H_LOHIFI || >> + mode == CLS_AB_HIFI || mode == CLS_AB_LOHIFI) { >> + snd_soc_component_update_bits(component, >> + WCD9XXX_ANA_RX_SUPPLIES, >> + 0x04, 0x04); >> + snd_soc_component_update_bits(component, >> + WCD9XXX_FLYBACK_VNEG_CTRL_4, >> + 0xF0, 0x80); >> + } else { >> + snd_soc_component_update_bits(component, >> + WCD9XXX_ANA_RX_SUPPLIES, >> + 0x04, 0x00); /* set to Default */ >> + snd_soc_component_update_bits(component, >> + WCD9XXX_FLYBACK_VNEG_CTRL_4, >> + 0xF0, 0x70); >> + } >> +} >> + >> +static inline void wcd_clsh_v3_force_iq_ctl(struct snd_soc_component >> *component, >> + int mode, bool enable) >> +{ >> + if (enable) { >> + snd_soc_component_update_bits(component, >> + WCD9XXX_FLYBACK_VNEGDAC_CTRL_2, >> + 0xE0, 0xA0); >> + /* 100usec delay is needed as per HW requirement */ >> + usleep_range(100, 110); >> + snd_soc_component_update_bits(component, >> + WCD9XXX_CLASSH_MODE_3, >> + 0x02, 0x02); >> + snd_soc_component_update_bits(component, >> + WCD9XXX_CLASSH_MODE_2, >> + 0xFF, 0x1C); >> + if (mode == CLS_H_LOHIFI || mode == CLS_AB_LOHIFI) { >> + snd_soc_component_update_bits(component, >> + WCD9XXX_HPH_NEW_INT_PA_MISC2, >> + 0x20, 0x20); >> + snd_soc_component_update_bits(component, >> + WCD9XXX_RX_BIAS_HPH_LOWPOWER, >> + 0xF0, 0xC0); >> + snd_soc_component_update_bits(component, >> + WCD9XXX_HPH_PA_CTL1, >> + 0x0E, 0x02); >> + } >> + } else { >> + snd_soc_component_update_bits(component, >> + WCD9XXX_HPH_NEW_INT_PA_MISC2, >> + 0x20, 0x00); >> + snd_soc_component_update_bits(component, >> + WCD9XXX_RX_BIAS_HPH_LOWPOWER, >> + 0xF0, 0x80); >> + snd_soc_component_update_bits(component, >> + WCD9XXX_HPH_PA_CTL1, >> + 0x0E, 0x06); >> + } >> +} > > do you need the inline for the two functions above? yes, these are totally unnecessary. --srini > >
Many thanks Pierre for reviewing the patches, On 19/03/2021 16:09, Pierre-Louis Bossart wrote: > > > On 3/19/21 4:29 AM, Srinivas Kandagatla wrote: >> This patchset adds support for Qualcomm WCD938X codec. >> >> Qualcomm WCD9380/WCD9385 Codec is a standalone Hi-Fi audio codec IC >> connected over SoundWire. This device has two SoundWire devices, RX and >> TX respectively supporting 4 x ADCs, ClassH, Ear, Aux PA, 2xHPH, >> 7 x TX diff inputs, 8 DMICs and MBHC. >> >> Even though this device has two SoundWire devices, only tx device has >> access to main codec Control/Status Registers! > > That part is a new concept we haven't seen so far with SoundWire > support, and I added a number of comments in the patches. > > It would really help if you could add more explanations on how > regmap/pm_runtime/gpios/regulators/interrupts are supposed to work with > such a functional split. Thanks! codec regmap for Control and status registers are only possible via TX Soundwire device as per h.w wiring. In the existing code we take care of this in common code (wcd938x.c) shared between TX and RX devices. pm runtime is also handled in the common code, Ex. resume on rx dev will make sure that tx dev is resumed first, and suspend on rx is nop. I have tested basic pm runtime auto-suspend cases with this. same with reset gpios/regulators and regulators. SDW Interrupts are also via tx device. > >> This patchset along with other SoundWire patches on the list >> have been tested on SM8250 MTP device. >> >> Am planning to send support for MBHC once this driver gets accepted! >> >> Thanks, >> srini >> >> Many thanks for reviewing v2. >> >> >> Changes since v2: >> - fixed dt_binding_check error >> >> >> Srinivas Kandagatla (7): >> ASoC: dt-bindings: wcd938x: add bindings for wcd938x >> ASoC: codecs: wcd-clsh: add new version support >> ASoC: codecs: wcd938x: add basic driver >> ASoC: codecs: wcd938x: add basic controls >> ASoC: codecs: wcd938x: add playback dapm widgets >> ASoC: codecs: wcd938x: add capture dapm widgets >> ASoC: codecs: wcd938x: add audio routing >> >> .../bindings/sound/qcom,wcd938x.yaml | 165 + >> sound/soc/codecs/Kconfig | 9 + >> sound/soc/codecs/Makefile | 2 + >> sound/soc/codecs/wcd-clsh-v2.c | 350 +- >> sound/soc/codecs/wcd-clsh-v2.h | 16 + >> sound/soc/codecs/wcd938x-sdw.c | 291 ++ >> sound/soc/codecs/wcd938x.c | 3623 +++++++++++++++++ >> sound/soc/codecs/wcd938x.h | 676 +++ >> 8 files changed, 5122 insertions(+), 10 deletions(-) >> create mode 100644 >> Documentation/devicetree/bindings/sound/qcom,wcd938x.yaml >> create mode 100644 sound/soc/codecs/wcd938x-sdw.c >> create mode 100644 sound/soc/codecs/wcd938x.c >> create mode 100644 sound/soc/codecs/wcd938x.h >>