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

Chen-Yu Tsai
2025-10-21T03:51:03Z

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);
> > }
> >
> >
>
>
>
>