Re: [PATCH 04/11] ASoC: sun4i-spdif: Support SPDIF output on A523 family

[Chen-Yu Tsai <wens@kernel.org>]

On Tue, Oct 21, 2025 at 1:49 AM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Hi,
>
> Dne ponedeljek, 20. oktober 2025 ob 19:10:50 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> > The TX side of the SPDIF block on the A523 is almost the same the
> > previous generations, the only difference being that it has separate
> > module clock inputs for the TX and RX side.
> >
> > Since this driver currently only supports TX, add support for a
> > different clock name so that TX and RX clocks can be separated
> > if RX support is ever added. Then add support for the A523.
> >
> > Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
> > ---
> >  sound/soc/sunxi/sun4i-spdif.c | 28 +++++++++++++++++++++++++---
> >  1 file changed, 25 insertions(+), 3 deletions(-)
> >
> > diff --git a/sound/soc/sunxi/sun4i-spdif.c b/sound/soc/sunxi/sun4i-spdif.c
> > index 34e5bd94e9af..6a58dc4311de 100644
> > --- a/sound/soc/sunxi/sun4i-spdif.c
> > +++ b/sound/soc/sunxi/sun4i-spdif.c
> > @@ -177,6 +177,7 @@ struct sun4i_spdif_quirks {
> >       bool has_reset;
> >       unsigned int val_fctl_ftx;
> >       unsigned int mclk_multiplier;
> > +     const char *tx_clk_name;
> >  };
> >
> >  struct sun4i_spdif_dev {
> > @@ -323,6 +324,7 @@ static int sun4i_spdif_hw_params(struct snd_pcm_substream *substream,
> >       }
> >       mclk *= host->quirks->mclk_multiplier;
> >
> > +     dev_info(&pdev->dev, "Setting SPDIF clock rate to %u\n", mclk);
> >       ret = clk_set_rate(host->spdif_clk, mclk);
> >       if (ret < 0) {
> >               dev_err(&pdev->dev,
> > @@ -542,7 +544,6 @@ static struct snd_soc_dai_driver sun4i_spdif_dai = {
> >               .formats = SUN4I_FORMATS,
> >       },
> >       .ops = &sun4i_spdif_dai_ops,
> > -     .name = "spdif",
>
> Why this change?

Now that you mention it, this looks bogus to me as well. I'll drop it.

> >  };
> >
> >  static const struct sun4i_spdif_quirks sun4i_a10_spdif_quirks = {
> > @@ -572,6 +573,14 @@ static const struct sun4i_spdif_quirks sun50i_h6_spdif_quirks = {
> >       .mclk_multiplier = 1,
> >  };
> >
> > +static const struct sun4i_spdif_quirks sun55i_a523_spdif_quirks = {
> > +     .reg_dac_txdata = SUN8I_SPDIF_TXFIFO,
> > +     .val_fctl_ftx   = SUN50I_H6_SPDIF_FCTL_FTX,
> > +     .has_reset      = true,
> > +     .mclk_multiplier = 1,
> > +     .tx_clk_name    = "tx",
> > +};
> > +
> >  static const struct of_device_id sun4i_spdif_of_match[] = {
> >       {
> >               .compatible = "allwinner,sun4i-a10-spdif",
> > @@ -594,6 +603,15 @@ static const struct of_device_id sun4i_spdif_of_match[] = {
> >               /* Essentially the same as the H6, but without RX */
> >               .data = &sun50i_h6_spdif_quirks,
> >       },
> > +     {
> > +             .compatible = "allwinner,sun55i-a523-spdif",
> > +             /*
> > +              * Almost the same as H6, but has split the TX and RX clocks,
> > +              * has a separate reset bit for the RX side, and has some
> > +              * expanded features for the RX side.
> > +              */
> > +             .data = &sun55i_a523_spdif_quirks,
> > +     },
> >       { /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, sun4i_spdif_of_match);
> > @@ -635,6 +653,7 @@ static int sun4i_spdif_probe(struct platform_device *pdev)
> >       const struct sun4i_spdif_quirks *quirks;
> >       int ret;
> >       void __iomem *base;
> > +     const char *tx_clk_name = "spdif";
>
> Reverse tree?

I think that only applies to the network tree.

> Otherwise it looks good.

Thanks!

ChenYu

> Best regards,
> Jernej
>
> >
> >       dev_dbg(&pdev->dev, "Entered %s\n", __func__);
> >
> > @@ -671,9 +690,12 @@ static int sun4i_spdif_probe(struct platform_device *pdev)
> >               return PTR_ERR(host->apb_clk);
> >       }
> >
> > -     host->spdif_clk = devm_clk_get(&pdev->dev, "spdif");
> > +     if (quirks->tx_clk_name)
> > +             tx_clk_name = quirks->tx_clk_name;
> > +     host->spdif_clk = devm_clk_get(&pdev->dev, tx_clk_name);
> >       if (IS_ERR(host->spdif_clk)) {
> > -             dev_err(&pdev->dev, "failed to get a spdif clock.\n");
> > +             dev_err(&pdev->dev, "failed to get the \"%s\" clock.\n",
> > +                     tx_clk_name);
> >               return PTR_ERR(host->spdif_clk);
> >       }
> >
> >
>
>
>
>