From: Kirill Marinushkin <kmarinushkin@birdec.com>
To: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: broonie@kernel.org, lgirdwood@gmail.com,
codrin.ciubotariu@microchip.com, lars@metafoo.de,
cychiang@chromium.org, tzungbi@google.com, bleung@chromium.org,
matthias.bgg@gmail.com, oder_chiou@realtek.com,
steven.eckhoff.opensource@gmail.com,
srinivas.kandagatla@linaro.org, alexandre.belloni@bootlin.com,
kuninori.morimoto.gx@renesas.com, jiaxin.yu@mediatek.com,
alsa-devel@alsa-project.org, chrome-platform@lists.linux.dev,
linux-mediatek@lists.infradead.org,
patches@opensource.cirrus.com
Subject: Re: [PATCH 01/38] ASoC: soc-component: Add comment for the endianness flag
Date: Mon, 9 May 2022 21:22:42 +0200 [thread overview]
Message-ID: <901cb995-4a82-741e-00ea-a1c0b22ae749@birdec.com> (raw)
In-Reply-To: <20220509083729.GX38351@ediswmail.ad.cirrus.com>
Hello Charles,
On 5/9/22 10:37 AM, Charles Keepax wrote:
> On Sun, May 08, 2022 at 10:34:12PM +0200, Kirill Marinushkin wrote:
>> In the [PATCH 00/38] of this patch set, you write:
>>> 2) Devices behind non-audio buses, SPI just moves bits and doesn't
>>> really define an endian for audio data on the bus. Thus it seems the
>>> CODEC probably can care about the endian. The only devices that fall
>>> into this group (mostly for AoV) are: rt5514-spi.c, rt5677-spi.c,
>>> cros_ec_codec.c (only the AoV).
>> From my experience with some PCM codecs by TI, they can not care about the
>> endianness. For example, in driver [1], if I allow BE format as
>> well, and configure
>> the sound card to use BE, it will not work. I have no sound with BE.
>> In these cases, the codec HW supports *only* LE. That's why their
>> `snd_soc_dai_driver`
>> structures provide only LE in the `format` field.
>> If such drivers specify `endianness = 1`, then `soc-core` will
>> extend their supported
>> formats with BE counter-parts, see [2]. AFAIU, it will have the same
>> effect, as if the
>> drivers themselves provided their formats in both LE | BE.
>>
>> I don't think adding `endianness = 1` to such codecs will work as expected.
>> Unfortunately, I don't have an easy access to HW today, to confirm
>> or disprove
>> this understanding.
>>
> This sounds like an error on the CPU side of the DAI link rather
> than the CODEC side of the DAI link. The formats that will be
> supported on the link are the union of the CPU and CODEC supported
> formats, ie. a format must be supported on both for the DAI to
> support it.
Yes, agree, both sides of the DAI link should provide only endianness
they support.
This works currently, but, from my understending, it will break, after
we set `endianness = 1`.
As soon as we start setting `endianness = 1`, the function
`convert_endianness_formats()` will extend LE to (LE | BE), right?
If so, setting `endianness = 1` is the source of an error, right?
I could be missing something. Let's see what other reviewers think.
> The CPU I2S hardware should be sending out the bits in the same
> order regardless of if the data you feed it is BE or LE, as I2S
> specifies an ordering for the bits.
What does the endianness in the driver configure, then?
> If this is not the case then
> the host I2S controller is claiming to support an endian it does
> not, and the problem should be fixed on that side by removing the
> supported endian.
I think we have a misundersanding of my example.
In my example, i don't mean, that my CPU part of the DAI link is broken.
What i tried to demonstrate - is that if i set the unsupported
endianness, i wouldn't expect that "the CODEC probably can care about
the endian", as the message in [PATCH 00/38] suggests. I would expect,
that i will have no sound.
Best regards,
Kirill
>
> Thanks,
> Charles
WARNING: multiple messages have this Message-ID (diff)
From: Kirill Marinushkin <kmarinushkin@birdec.com>
To: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: oder_chiou@realtek.com, steven.eckhoff.opensource@gmail.com,
alexandre.belloni@bootlin.com, lars@metafoo.de,
kuninori.morimoto.gx@renesas.com,
chrome-platform@lists.linux.dev, patches@opensource.cirrus.com,
lgirdwood@gmail.com, jiaxin.yu@mediatek.com, tzungbi@google.com,
broonie@kernel.org, srinivas.kandagatla@linaro.org,
matthias.bgg@gmail.com, linux-mediatek@lists.infradead.org,
codrin.ciubotariu@microchip.com, alsa-devel@alsa-project.org,
bleung@chromium.org, cychiang@chromium.org
Subject: Re: [PATCH 01/38] ASoC: soc-component: Add comment for the endianness flag
Date: Mon, 9 May 2022 21:22:42 +0200 [thread overview]
Message-ID: <901cb995-4a82-741e-00ea-a1c0b22ae749@birdec.com> (raw)
In-Reply-To: <20220509083729.GX38351@ediswmail.ad.cirrus.com>
Hello Charles,
On 5/9/22 10:37 AM, Charles Keepax wrote:
> On Sun, May 08, 2022 at 10:34:12PM +0200, Kirill Marinushkin wrote:
>> In the [PATCH 00/38] of this patch set, you write:
>>> 2) Devices behind non-audio buses, SPI just moves bits and doesn't
>>> really define an endian for audio data on the bus. Thus it seems the
>>> CODEC probably can care about the endian. The only devices that fall
>>> into this group (mostly for AoV) are: rt5514-spi.c, rt5677-spi.c,
>>> cros_ec_codec.c (only the AoV).
>> From my experience with some PCM codecs by TI, they can not care about the
>> endianness. For example, in driver [1], if I allow BE format as
>> well, and configure
>> the sound card to use BE, it will not work. I have no sound with BE.
>> In these cases, the codec HW supports *only* LE. That's why their
>> `snd_soc_dai_driver`
>> structures provide only LE in the `format` field.
>> If such drivers specify `endianness = 1`, then `soc-core` will
>> extend their supported
>> formats with BE counter-parts, see [2]. AFAIU, it will have the same
>> effect, as if the
>> drivers themselves provided their formats in both LE | BE.
>>
>> I don't think adding `endianness = 1` to such codecs will work as expected.
>> Unfortunately, I don't have an easy access to HW today, to confirm
>> or disprove
>> this understanding.
>>
> This sounds like an error on the CPU side of the DAI link rather
> than the CODEC side of the DAI link. The formats that will be
> supported on the link are the union of the CPU and CODEC supported
> formats, ie. a format must be supported on both for the DAI to
> support it.
Yes, agree, both sides of the DAI link should provide only endianness
they support.
This works currently, but, from my understending, it will break, after
we set `endianness = 1`.
As soon as we start setting `endianness = 1`, the function
`convert_endianness_formats()` will extend LE to (LE | BE), right?
If so, setting `endianness = 1` is the source of an error, right?
I could be missing something. Let's see what other reviewers think.
> The CPU I2S hardware should be sending out the bits in the same
> order regardless of if the data you feed it is BE or LE, as I2S
> specifies an ordering for the bits.
What does the endianness in the driver configure, then?
> If this is not the case then
> the host I2S controller is claiming to support an endian it does
> not, and the problem should be fixed on that side by removing the
> supported endian.
I think we have a misundersanding of my example.
In my example, i don't mean, that my CPU part of the DAI link is broken.
What i tried to demonstrate - is that if i set the unsupported
endianness, i wouldn't expect that "the CODEC probably can care about
the endian", as the message in [PATCH 00/38] suggests. I would expect,
that i will have no sound.
Best regards,
Kirill
>
> Thanks,
> Charles
WARNING: multiple messages have this Message-ID (diff)
From: Kirill Marinushkin <kmarinushkin@birdec.com>
To: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: broonie@kernel.org, lgirdwood@gmail.com,
codrin.ciubotariu@microchip.com, lars@metafoo.de,
cychiang@chromium.org, tzungbi@google.com, bleung@chromium.org,
matthias.bgg@gmail.com, oder_chiou@realtek.com,
steven.eckhoff.opensource@gmail.com,
srinivas.kandagatla@linaro.org, alexandre.belloni@bootlin.com,
kuninori.morimoto.gx@renesas.com, jiaxin.yu@mediatek.com,
alsa-devel@alsa-project.org, chrome-platform@lists.linux.dev,
linux-mediatek@lists.infradead.org,
patches@opensource.cirrus.com
Subject: Re: [PATCH 01/38] ASoC: soc-component: Add comment for the endianness flag
Date: Mon, 9 May 2022 21:22:42 +0200 [thread overview]
Message-ID: <901cb995-4a82-741e-00ea-a1c0b22ae749@birdec.com> (raw)
In-Reply-To: <20220509083729.GX38351@ediswmail.ad.cirrus.com>
Hello Charles,
On 5/9/22 10:37 AM, Charles Keepax wrote:
> On Sun, May 08, 2022 at 10:34:12PM +0200, Kirill Marinushkin wrote:
>> In the [PATCH 00/38] of this patch set, you write:
>>> 2) Devices behind non-audio buses, SPI just moves bits and doesn't
>>> really define an endian for audio data on the bus. Thus it seems the
>>> CODEC probably can care about the endian. The only devices that fall
>>> into this group (mostly for AoV) are: rt5514-spi.c, rt5677-spi.c,
>>> cros_ec_codec.c (only the AoV).
>> From my experience with some PCM codecs by TI, they can not care about the
>> endianness. For example, in driver [1], if I allow BE format as
>> well, and configure
>> the sound card to use BE, it will not work. I have no sound with BE.
>> In these cases, the codec HW supports *only* LE. That's why their
>> `snd_soc_dai_driver`
>> structures provide only LE in the `format` field.
>> If such drivers specify `endianness = 1`, then `soc-core` will
>> extend their supported
>> formats with BE counter-parts, see [2]. AFAIU, it will have the same
>> effect, as if the
>> drivers themselves provided their formats in both LE | BE.
>>
>> I don't think adding `endianness = 1` to such codecs will work as expected.
>> Unfortunately, I don't have an easy access to HW today, to confirm
>> or disprove
>> this understanding.
>>
> This sounds like an error on the CPU side of the DAI link rather
> than the CODEC side of the DAI link. The formats that will be
> supported on the link are the union of the CPU and CODEC supported
> formats, ie. a format must be supported on both for the DAI to
> support it.
Yes, agree, both sides of the DAI link should provide only endianness
they support.
This works currently, but, from my understending, it will break, after
we set `endianness = 1`.
As soon as we start setting `endianness = 1`, the function
`convert_endianness_formats()` will extend LE to (LE | BE), right?
If so, setting `endianness = 1` is the source of an error, right?
I could be missing something. Let's see what other reviewers think.
> The CPU I2S hardware should be sending out the bits in the same
> order regardless of if the data you feed it is BE or LE, as I2S
> specifies an ordering for the bits.
What does the endianness in the driver configure, then?
> If this is not the case then
> the host I2S controller is claiming to support an endian it does
> not, and the problem should be fixed on that side by removing the
> supported endian.
I think we have a misundersanding of my example.
In my example, i don't mean, that my CPU part of the DAI link is broken.
What i tried to demonstrate - is that if i set the unsupported
endianness, i wouldn't expect that "the CODEC probably can care about
the endian", as the message in [PATCH 00/38] suggests. I would expect,
that i will have no sound.
Best regards,
Kirill
>
> Thanks,
> Charles
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2022-05-09 19:22 UTC|newest]
Thread overview: 141+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-04 17:08 [PATCH 00/38] Clean up usage of the endianness flag Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 01/38] ASoC: soc-component: Add comment for " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-08 20:34 ` Kirill Marinushkin
2022-05-08 20:34 ` Kirill Marinushkin
2022-05-08 20:34 ` Kirill Marinushkin
2022-05-09 8:37 ` Charles Keepax
2022-05-09 8:37 ` Charles Keepax
2022-05-09 8:37 ` Charles Keepax
2022-05-09 19:22 ` Kirill Marinushkin [this message]
2022-05-09 19:22 ` Kirill Marinushkin
2022-05-09 19:22 ` Kirill Marinushkin
2022-05-09 19:30 ` Mark Brown
2022-05-09 19:30 ` Mark Brown
2022-05-09 19:30 ` Mark Brown
2022-05-09 20:11 ` Kirill Marinushkin
2022-05-09 20:11 ` Kirill Marinushkin
2022-05-09 20:11 ` Kirill Marinushkin
2022-05-09 20:18 ` Mark Brown
2022-05-09 20:18 ` Mark Brown
2022-05-09 20:18 ` Mark Brown
2022-05-10 8:53 ` Charles Keepax
2022-05-10 8:53 ` Charles Keepax
2022-05-10 8:53 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 02/38] ASoC: atmel-pdmic: Remove endianness flag on pdmic component Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 03/38] ASoC: atmel-classd: Remove endianness flag on class d component Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 04/38] ASoC: cs4270: Remove redundant big endian formats Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 05/38] ASoC: cs42l51: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 06/38] ASoC: cs4349: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 07/38] ASoC: hdmi-codec: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 08/38] ASoC: sta32x: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 09/38] ASoC: sta350: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 10/38] ASoC: hdac_hda: Add endianness flag in snd_soc_component_driver Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 11/38] ASoC: max98504: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 12/38] ASoC: adau1372: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 13/38] ASoC: cs4234: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 14/38] ASoC: cs35l41: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 15/38] ASoC: cx2072x: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 16/38] ASoC: lochnagar: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 17/38] ASoC: mt6351: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 18/38] ASoC: mt6358: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 19/38] ASoC: mt6359: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 20/38] ASoC: mt6660: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 21/38] ASoC: pcm3060: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 22/38] ASoC: rt1019: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 23/38] ASoC: rt9120: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 24/38] ASoC: tlv320adc3xxx: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 25/38] ASoC: tscs454: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 26/38] ASoC: cros_ec_codec: Add endianness flag in i2s_rx_component_driver Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 27/38] ASoC: wcd934x: Add endianness flag in snd_soc_component_driver Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 28/38] ASoC: wcd9335: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 29/38] ASoC: rt700: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 30/38] ASoC: rt711: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 31/38] ASoC: rt711-sdca: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` [PATCH 32/38] ASoC: rt715: " Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:08 ` Charles Keepax
2022-05-04 17:09 ` [PATCH 33/38] ASoC: rt715-sdca: " Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` [PATCH 34/38] ASoC: rt1308-sdw: " Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` [PATCH 35/38] ASoC: rt1316-sdw: " Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` [PATCH 36/38] ASoC: wcd938x: " Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` [PATCH 37/38] ASoC: wsa881x: " Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` [PATCH 38/38] ASoC: sdw-mockup: " Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-04 17:09 ` Charles Keepax
2022-05-10 11:13 ` [PATCH 00/38] Clean up usage of the endianness flag Mark Brown
2022-05-10 11:13 ` Mark Brown
2022-05-10 11:13 ` Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=901cb995-4a82-741e-00ea-a1c0b22ae749@birdec.com \
--to=kmarinushkin@birdec.com \
--cc=alexandre.belloni@bootlin.com \
--cc=alsa-devel@alsa-project.org \
--cc=bleung@chromium.org \
--cc=broonie@kernel.org \
--cc=chrome-platform@lists.linux.dev \
--cc=ckeepax@opensource.cirrus.com \
--cc=codrin.ciubotariu@microchip.com \
--cc=cychiang@chromium.org \
--cc=jiaxin.yu@mediatek.com \
--cc=kuninori.morimoto.gx@renesas.com \
--cc=lars@metafoo.de \
--cc=lgirdwood@gmail.com \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=oder_chiou@realtek.com \
--cc=patches@opensource.cirrus.com \
--cc=srinivas.kandagatla@linaro.org \
--cc=steven.eckhoff.opensource@gmail.com \
--cc=tzungbi@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.