Re: [PATCH v2 2/2] ASoC: codec: lpass-rx-macro: add suppor for 2.5 codec version

[Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>]

On Wed, 12 Jun 2024 at 20:09, Srinivas Kandagatla
<srinivas.kandagatla@xxxxxxxxxx> wrote:
>
>
>
> On 12/06/2024 17:52, Dmitry Baryshkov wrote:
> > On Wed, Jun 12, 2024 at 03:37:56PM +0100, Srinivas Kandagatla wrote:
> >>
> >>
> >> On 07/06/2024 12:03, Dmitry Baryshkov wrote:
> >>> On Thu, Jun 06, 2024 at 01:25:59PM +0100, srinivas.kandagatla@xxxxxxxxxx wrote:
> >>>> From: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> >>>>
> >>>> LPASS Codec v2.5 has significant changes in the rx register offsets.
> >>>> Due to this headset playback on SM8550, SM8650, x1e80100 and all SoCs
> >>>> after SM8450 have only Left working.
> >>>>
> >>>> This patch adjusts the registers to accomdate 2.5 changes. With this
> >>>> fixed now L and R are functional on Headset playback.
> >>>>
> >>>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> >>>> ---
> >>>>    sound/soc/codecs/lpass-rx-macro.c | 565 ++++++++++++++++++++++--------
> >>>>    1 file changed, 410 insertions(+), 155 deletions(-)
> >>>>
> >>>> diff --git a/sound/soc/codecs/lpass-rx-macro.c b/sound/soc/codecs/lpass-rx-macro.c
> >>>> index f35187d69cac..bb8ede0e7076 100644
> >>>> --- a/sound/soc/codecs/lpass-rx-macro.c
> >>>> +++ b/sound/soc/codecs/lpass-rx-macro.c
> >>>>    static int rx_macro_probe(struct platform_device *pdev)
> >>>>    {
> >>>> +  struct reg_default *reg_defaults;
> >>>>            struct device *dev = &pdev->dev;
> >>>>            kernel_ulong_t flags;
> >>>>            struct rx_macro *rx;
> >>>>            void __iomem *base;
> >>>> -  int ret;
> >>>> +  int ret, def_count;
> >>>>            flags = (kernel_ulong_t)device_get_match_data(dev);
> >>>> @@ -3567,6 +3793,33 @@ static int rx_macro_probe(struct platform_device *pdev)
> >>>>                    goto err;
> >>>>            }
> >>>> +  rx->codec_version = lpass_macro_get_codec_version();
> >>>
> >>> What guarantees that VA macro has been probed already? If I'm not
> >>> mistaken, we might easily get a default '0' here instead of a correct
> >>> version.
> >>
> >> fsgen(Frame sync gen) clk is derived from VA macro, so if we are here that
> >> means the va macro is probed.
> >
> > Is this written in stone or is it just a current way how these codecs
> > are connected?
>
> LPASS Codec IP which encompasses 5 other codec macros blocks (wsa, wsa2,
> tx, rx, va) all the codec macros receive framesync from VA codec block,
> this is the hw design.
>
> Not sure what do you mean by written in stone, but this is LPASS Codec
> design, at-least to the codec versions that this driver supports.

Ack. Please add a comment on top of it.

>
> >
> >>>> +  switch (rx->codec_version) {
> >>>> +  case LPASS_CODEC_VERSION_2_5 ... LPASS_CODEC_VERSION_2_8:
> >>>> +          rx->rxn_reg_offset = 0xc0;
> >>>> +          def_count = ARRAY_SIZE(rx_defaults) + ARRAY_SIZE(rx_2_5_defaults);
> >>>> +          reg_defaults = kmalloc_array(def_count, sizeof(struct reg_default), GFP_KERNEL);
> >>>> +          if (!reg_defaults)
> >>>> +                  return -ENOMEM;
> >>>> +          memcpy(&reg_defaults[0], rx_defaults, sizeof(rx_defaults));
> >>>> +          memcpy(&reg_defaults[ARRAY_SIZE(rx_defaults)],
> >>>> +                          rx_2_5_defaults, sizeof(rx_2_5_defaults));
> >>>> +          break;
> >>>> +  default:
> >>>> +          rx->rxn_reg_offset = 0x80;
> >>>> +          def_count = ARRAY_SIZE(rx_defaults) + ARRAY_SIZE(rx_pre_2_5_defaults);
> >>>> +          reg_defaults = kmalloc_array(def_count, sizeof(struct reg_default), GFP_KERNEL);
> >>>> +          if (!reg_defaults)
> >>>> +                  return -ENOMEM;
> >>>> +          memcpy(&reg_defaults[0], rx_defaults, sizeof(rx_defaults));
> >>>> +          memcpy(&reg_defaults[ARRAY_SIZE(rx_defaults)],
> >>>> +                          rx_pre_2_5_defaults, sizeof(rx_pre_2_5_defaults));
> >>>> +          break;
> >>>> +  }
> >>>> +
> >>>> +  rx_regmap_config.reg_defaults = reg_defaults,
> >>>> +  rx_regmap_config.num_reg_defaults = def_count;
> >>>> +
> >>>>            rx->regmap = devm_regmap_init_mmio(dev, base, &rx_regmap_config);
> >>>>            if (IS_ERR(rx->regmap)) {
> >>>>                    ret = PTR_ERR(rx->regmap);
> >>>> @@ -3629,6 +3882,7 @@ static int rx_macro_probe(struct platform_device *pdev)
> >>>>            if (ret)
> >>>>                    goto err_clkout;
> >>>> +  kfree(reg_defaults);
> >>>>            return 0;
> >>>>    err_clkout:
> >>>> @@ -3642,6 +3896,7 @@ static int rx_macro_probe(struct platform_device *pdev)
> >>>>    err_dcodec:
> >>>>            clk_disable_unprepare(rx->macro);
> >>>>    err:
> >>>> +  kfree(reg_defaults);
> >>>>            lpass_macro_pds_exit(rx->pds);
> >>>>            return ret;
> >>>> --
> >>>> 2.21.0
> >>>>
> >>>
> >



-- 
With best wishes
Dmitry