mbox series

[PATCHv2,00/10] ASoC: fsl-asoc-card: compatibility integration of a generic codec use case for use with S/PDIF controller

Message ID 20231027144734.3654829-1-elinor.montmasson@savoirfairelinux.com
Headers show
Series ASoC: fsl-asoc-card: compatibility integration of a generic codec use case for use with S/PDIF controller | expand

Message

Elinor Montmasson Oct. 27, 2023, 2:47 p.m. UTC
Hello,

This is the v2 of the series of patch aiming to make the machine driver fsl-asoc-card compatible with use cases where there is no real codec driver. It proposes to use the spdif_receiver and spdif_transmitter drivers instead of the dummy codec.
This is a first step in using the S/PDIF controller with the ASRC.

The five first patches add compatibility with the pair of codecs spdif_receiver and spdif_transmitter with a new compatible, "fsl,imx-audio-generic". Codec parameters are set with default values.

The remaining patches add configuration options for the device tree. They configure the CPU DAI when using "fsl,imx-audio-generic". These are usually hard-coded in fsl-asoc-card for each audio codec. Because the generic codec could be used with other CPU DAI than the S/PDIF controller, setting these parameters could be required.

This series of patch was successfully built for arm64 and x86 on top of the latest for-next branch of the ASoC git tree on the 24/10.
These modifications have also been tested on an i.MX8MN evaluation board, with a linux kernel RT v6.1.26-rt8.

We also have a few questions :

* We named the compatible "fsl,imx-audio-generic" as, for the moment, it could work with any CPU DAI, even if it uses S/PDIF codecs. Is it preferable to keep these modifications specific to the S/PDIF, thus specifying "spdif" in the compatible ? Or is it okay to keep the actual name of the compatible, and the generic purpose of this compatible ?

* Part of the dai_fmt variable hold information on the codec provider or consumer status for bit/frame clocks. In patch 03/10, as we add compatibility for multiple codecs, we make the test about bit/frame clock provider only check with codec[0]. That way we assure compatibility with all already existing compatibles. As it was never intended before to have multiple codecs for this test, is there a better way to handle it ? Should we make this test check if any codec is clock provider ? Or should we let codec[0] be the default possibility ? That way, future compatibles that could encounter this specific case with multi-codecs should adapt the test for their needs.

Best regards,
Elinor Montmasson

Changelog:
v1 -> v2:
* replace use of the dummy codec by the pair of codecs spdif_receiver / spdif_transmitter
* adapt how dai links codecs are used to take into account the possibility for multiple codecs per link
* change compatible name
* adapt driver to be able to register two codecs given in the device tree
* v1 patch series at:
https://lore.kernel.org/alsa-devel/20230901144550.520072-1-elinor.montmasson@savoirfairelinux.com/

Elinor Montmasson (10):
  ASoC: fsl-asoc-card: add support for dai links with multiple codecs
  ASoC: fsl-asoc-card: add second dai link component for codecs
  ASoC: fsl-asoc-card: add compatibility to use 2 codecs from device
    tree
  ASoC: fsl-asoc-card: add new compatible for a generic codec use case
  ASoC: fsl-asoc-card: set generic codec as clock provider
  ASoC: fsl-asoc-card: add dts property "cpu-slot-width"
  ASoC: fsl-asoc-card: add dts property "cpu-slot-num"
  ASoC: fsl-asoc-card: add dts properties "cpu-sysclk-freq"
  ASoC: fsl-asoc-card: add dts properties "cpu-sysclk-dir-out"
  Documentation: fsl-asoc-card: add documentation for generic codec case

 .../bindings/sound/fsl-asoc-card.txt          |  26 +++-
 sound/soc/fsl/fsl-asoc-card.c                 | 128 ++++++++++++------
 2 files changed, 114 insertions(+), 40 deletions(-)

Comments

Mark Brown Nov. 20, 2023, 2:23 p.m. UTC | #1
On Fri, Oct 27, 2023 at 04:47:24PM +0200, Elinor Montmasson wrote:
> Hello,
> 
> This is the v2 of the series of patch aiming to make the machine driver fsl-asoc-card compatible with use cases where there is no real codec driver. It proposes to use the spdif_receiver and spdif_transmitter drivers instead of the dummy codec.

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> * Part of the dai_fmt variable hold information on the codec provider
> or consumer status for bit/frame clocks. In patch 03/10, as we add
> compatibility for multiple codecs, we make the test about bit/frame
> clock provider only check with codec[0]. That way we assure
> compatibility with all already existing compatibles. As it was never
> intended before to have multiple codecs for this test, is there a
> better way to handle it ? Should we make this test check if any codec
> is clock provider ? Or should we let codec[0] be the default
> possibility ? That way, future compatibles that could encounter this
> specific case with multi-codecs should adapt the test for their needs.

Yes, we should be checking all the CODECs - existing bindings that only
have a single CODEC should work fine since they shouldn't have
additional CODECs but it's possible that a device other than the first
one found may be providing the clocks.

>   Documentation: fsl-asoc-card: add documentation for generic codec case

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.