| Message ID | 20230612085614.3039498-1-mengyingkun@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Loongson I2S controller support | expand |
Hey! On Mon, Jun 12, 2023 at 04:56:14PM +0800, YingKun Meng wrote: > From: Yingkun Meng <mengyingkun@loongson.cn> > > The audio card uses loongson I2S controller present in > 7axxx/2kxxx chips to transfer audio data. > > On loongson platform, the chip has only one I2S controller. > > Signed-off-by: Yingkun Meng <mengyingkun@loongson.cn> I got 2 copies of this patch, but none of the rest of the series appears to be threaded with it. > .../sound/loongson,ls-audio-card.yaml | 70 +++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml > > diff --git a/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml > new file mode 100644 > index 000000000000..61e8babed402 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml > @@ -0,0 +1,70 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/loongson,ls-audio-card.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Loongson 7axxx/2kxxx ASoC audio sound card driver > + > +maintainers: > + - Yingkun Meng <mengyingkun@loongson.cn> > + > +description: > + The binding describes the sound card present in loongson > + 7axxx/2kxxx platform. The sound card is an ASoC component > + which uses Loongson I2S controller to transfer the audio data. > + > +properties: > + compatible: > + const: loongson,ls-audio-card Reviewing sound stuff is beyond my pay grade, so forgive me if I am off the rails here, but this (and the "x"s in the description) look a bit odd. Recently, we've noticed quite a few loongson dt-bindings attempting to use a single compatible for many different chips. Usually you have individual compatibles for the various SoCs with this core, which can fall back to a generic one, rather than just adding a generic compatible for all devices. As far as I know, there's several SoCs fitting 2kxxx, and the format being used elsewhere is "loongson,ls2k1000" etc. Cheers, Conor.
On 2023/6/13 01:24, Conor Dooley wrote: > Hey! > > On Mon, Jun 12, 2023 at 04:56:14PM +0800, YingKun Meng wrote: >> From: Yingkun Meng <mengyingkun@loongson.cn> >> >> The audio card uses loongson I2S controller present in >> 7axxx/2kxxx chips to transfer audio data. >> >> On loongson platform, the chip has only one I2S controller. >> >> Signed-off-by: Yingkun Meng <mengyingkun@loongson.cn> > I got 2 copies of this patch, but none of the rest of the series appears > to be threaded with it. > >> .../sound/loongson,ls-audio-card.yaml | 70 +++++++++++++++++++ >> 1 file changed, 70 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml >> >> diff --git a/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml >> new file mode 100644 >> index 000000000000..61e8babed402 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml >> @@ -0,0 +1,70 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/sound/loongson,ls-audio-card.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Loongson 7axxx/2kxxx ASoC audio sound card driver >> + >> +maintainers: >> + - Yingkun Meng <mengyingkun@loongson.cn> >> + >> +description: >> + The binding describes the sound card present in loongson >> + 7axxx/2kxxx platform. The sound card is an ASoC component >> + which uses Loongson I2S controller to transfer the audio data. >> + >> +properties: >> + compatible: >> + const: loongson,ls-audio-card > Reviewing sound stuff is beyond my pay grade, so forgive me if I am off > the rails here, but this (and the "x"s in the description) look a bit > odd. Recently, we've noticed quite a few loongson dt-bindings attempting > to use a single compatible for many different chips. > Usually you have individual compatibles for the various SoCs with this > core, which can fall back to a generic one, rather than just adding a > generic compatible for all devices. > As far as I know, there's several SoCs fitting 2kxxx, and the format > being used elsewhere is "loongson,ls2k1000" etc. Currently, Loongson has 2K0500/2K1000LA/2K1500/2K2000 chips. Here, its' possible to use a single compatible for different chips, as the audio device is a logical device, not dependent on chip model. > Cheers, > Conor. >
On Tue, Jun 13, 2023 at 08:23:58PM +0800, Yingkun Meng wrote: > > On 2023/6/13 01:24, Conor Dooley wrote: > > Hey! > > > > On Mon, Jun 12, 2023 at 04:56:14PM +0800, YingKun Meng wrote: > > > From: Yingkun Meng <mengyingkun@loongson.cn> > > > > > > The audio card uses loongson I2S controller present in > > > 7axxx/2kxxx chips to transfer audio data. > > > > > > On loongson platform, the chip has only one I2S controller. > > > > > > Signed-off-by: Yingkun Meng <mengyingkun@loongson.cn> > > I got 2 copies of this patch, but none of the rest of the series appears > > to be threaded with it. > > > > > .../sound/loongson,ls-audio-card.yaml | 70 +++++++++++++++++++ > > > 1 file changed, 70 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml > > > new file mode 100644 > > > index 000000000000..61e8babed402 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml > > > @@ -0,0 +1,70 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/sound/loongson,ls-audio-card.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Loongson 7axxx/2kxxx ASoC audio sound card driver > > > + > > > +maintainers: > > > + - Yingkun Meng <mengyingkun@loongson.cn> > > > + > > > +description: > > > + The binding describes the sound card present in loongson > > > + 7axxx/2kxxx platform. The sound card is an ASoC component > > > + which uses Loongson I2S controller to transfer the audio data. > > > + > > > +properties: > > > + compatible: > > > + const: loongson,ls-audio-card > > Reviewing sound stuff is beyond my pay grade, so forgive me if I am off > > the rails here, but this (and the "x"s in the description) look a bit > > odd. Recently, we've noticed quite a few loongson dt-bindings attempting > > to use a single compatible for many different chips. > > Usually you have individual compatibles for the various SoCs with this > > core, which can fall back to a generic one, rather than just adding a > > generic compatible for all devices. > > As far as I know, there's several SoCs fitting 2kxxx, and the format > > being used elsewhere is "loongson,ls2k1000" etc. > > > Currently, Loongson has 2K0500/2K1000LA/2K1500/2K2000 chips. > > Here, its' possible to use a single compatible for different chips, > > as the audio device is a logical device, not dependent on chip model. What, may I ask, is a "logical device"?
On 2023/6/13 20:28, Conor Dooley wrote: > On Tue, Jun 13, 2023 at 08:23:58PM +0800, Yingkun Meng wrote: >> On 2023/6/13 01:24, Conor Dooley wrote: >>> Hey! >>> >>> On Mon, Jun 12, 2023 at 04:56:14PM +0800, YingKun Meng wrote: >>>> From: Yingkun Meng <mengyingkun@loongson.cn> >>>> >>>> The audio card uses loongson I2S controller present in >>>> 7axxx/2kxxx chips to transfer audio data. >>>> >>>> On loongson platform, the chip has only one I2S controller. >>>> >>>> Signed-off-by: Yingkun Meng <mengyingkun@loongson.cn> >>> I got 2 copies of this patch, but none of the rest of the series appears >>> to be threaded with it. >>> >>>> .../sound/loongson,ls-audio-card.yaml | 70 +++++++++++++++++++ >>>> 1 file changed, 70 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml >>>> new file mode 100644 >>>> index 000000000000..61e8babed402 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml >>>> @@ -0,0 +1,70 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/sound/loongson,ls-audio-card.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Loongson 7axxx/2kxxx ASoC audio sound card driver >>>> + >>>> +maintainers: >>>> + - Yingkun Meng <mengyingkun@loongson.cn> >>>> + >>>> +description: >>>> + The binding describes the sound card present in loongson >>>> + 7axxx/2kxxx platform. The sound card is an ASoC component >>>> + which uses Loongson I2S controller to transfer the audio data. >>>> + >>>> +properties: >>>> + compatible: >>>> + const: loongson,ls-audio-card >>> Reviewing sound stuff is beyond my pay grade, so forgive me if I am off >>> the rails here, but this (and the "x"s in the description) look a bit >>> odd. Recently, we've noticed quite a few loongson dt-bindings attempting >>> to use a single compatible for many different chips. >>> Usually you have individual compatibles for the various SoCs with this >>> core, which can fall back to a generic one, rather than just adding a >>> generic compatible for all devices. >>> As far as I know, there's several SoCs fitting 2kxxx, and the format >>> being used elsewhere is "loongson,ls2k1000" etc. >> >> Currently, Loongson has 2K0500/2K1000LA/2K1500/2K2000 chips. >> >> Here, its' possible to use a single compatible for different chips, >> >> as the audio device is a logical device, not dependent on chip model. > What, may I ask, is a "logical device"? I means it's not a physical one, like "platform bus".
On Tue, Jun 13, 2023 at 08:38:59PM +0800, Yingkun Meng wrote: > On 2023/6/13 20:28, Conor Dooley wrote: > > On Tue, Jun 13, 2023 at 08:23:58PM +0800, Yingkun Meng wrote: > > > On 2023/6/13 01:24, Conor Dooley wrote: > > > > On Mon, Jun 12, 2023 at 04:56:14PM +0800, YingKun Meng wrote: > > > > > From: Yingkun Meng <mengyingkun@loongson.cn> > > > > > > > > > > The audio card uses loongson I2S controller present in > > > > > 7axxx/2kxxx chips to transfer audio data. > > > > > > > > > > On loongson platform, the chip has only one I2S controller. > > > > > +description: > > > > > + The binding describes the sound card present in loongson > > > > > + 7axxx/2kxxx platform. The sound card is an ASoC component > > > > > + which uses Loongson I2S controller to transfer the audio data. > > > > > + > > > > > +properties: > > > > > + compatible: > > > > > + const: loongson,ls-audio-card > > > > Reviewing sound stuff is beyond my pay grade, so forgive me if I am off > > > > the rails here, but this (and the "x"s in the description) look a bit > > > > odd. Recently, we've noticed quite a few loongson dt-bindings attempting > > > > to use a single compatible for many different chips. > > > > Usually you have individual compatibles for the various SoCs with this > > > > core, which can fall back to a generic one, rather than just adding a > > > > generic compatible for all devices. > > > > As far as I know, there's several SoCs fitting 2kxxx, and the format > > > > being used elsewhere is "loongson,ls2k1000" etc. > > > > > > Currently, Loongson has 2K0500/2K1000LA/2K1500/2K2000 chips. > > > > > > Here, its' possible to use a single compatible for different chips, > > > > > > as the audio device is a logical device, not dependent on chip model. > > What, may I ask, is a "logical device"? > > > I means it's not a physical one, like "platform bus". So it is entirely a software construct? Why does it need a dt-binding then? Your commit message says the controller is present on the device! Confused, Conor.
On 2023/6/13 20:46, Conor Dooley wrote: > On Tue, Jun 13, 2023 at 08:38:59PM +0800, Yingkun Meng wrote: >> On 2023/6/13 20:28, Conor Dooley wrote: >>> On Tue, Jun 13, 2023 at 08:23:58PM +0800, Yingkun Meng wrote: >>>> On 2023/6/13 01:24, Conor Dooley wrote: >>>>> On Mon, Jun 12, 2023 at 04:56:14PM +0800, YingKun Meng wrote: >>>>>> From: Yingkun Meng <mengyingkun@loongson.cn> >>>>>> >>>>>> The audio card uses loongson I2S controller present in >>>>>> 7axxx/2kxxx chips to transfer audio data. >>>>>> >>>>>> On loongson platform, the chip has only one I2S controller. >>>>>> +description: >>>>>> + The binding describes the sound card present in loongson >>>>>> + 7axxx/2kxxx platform. The sound card is an ASoC component >>>>>> + which uses Loongson I2S controller to transfer the audio data. >>>>>> + >>>>>> +properties: >>>>>> + compatible: >>>>>> + const: loongson,ls-audio-card >>>>> Reviewing sound stuff is beyond my pay grade, so forgive me if I am off >>>>> the rails here, but this (and the "x"s in the description) look a bit >>>>> odd. Recently, we've noticed quite a few loongson dt-bindings attempting >>>>> to use a single compatible for many different chips. >>>>> Usually you have individual compatibles for the various SoCs with this >>>>> core, which can fall back to a generic one, rather than just adding a >>>>> generic compatible for all devices. >>>>> As far as I know, there's several SoCs fitting 2kxxx, and the format >>>>> being used elsewhere is "loongson,ls2k1000" etc. >>>> Currently, Loongson has 2K0500/2K1000LA/2K1500/2K2000 chips. >>>> >>>> Here, its' possible to use a single compatible for different chips, >>>> >>>> as the audio device is a logical device, not dependent on chip model. >>> What, may I ask, is a "logical device"? >> >> I means it's not a physical one, like "platform bus". > So it is entirely a software construct? Why does it need a dt-binding > then? Your commit message says the controller is present on the device! It's not. The audio device consists of an i2s controller and codec. The dt-binding is for the audio device, not for i2s controller. > Confused, > Conor.
On Tue, Jun 13, 2023 at 01:46:41PM +0100, Conor Dooley wrote: > So it is entirely a software construct? Why does it need a dt-binding > then? Your commit message says the controller is present on the device! A typical embedded (or power efficient laptop) audio design will consist of multiple devices connected together (frequently via non-control buses) together with system level passive components and plastics which are also important to the audio configuration. A card binding describes the interconections between the devices in the system and provides identification information for the audio subsystem. This system level audio integration is a physical thing that can be pointed at that requires real software control. Like I said before please look at the existing audio card bindings.
On Tue, Jun 13, 2023 at 02:54:36PM +0100, Mark Brown wrote: > On Tue, Jun 13, 2023 at 01:46:41PM +0100, Conor Dooley wrote: > > > So it is entirely a software construct? Why does it need a dt-binding > > then? Your commit message says the controller is present on the device! > > A typical embedded (or power efficient laptop) audio design will consist > of multiple devices connected together (frequently via non-control > buses) together with system level passive components and plastics which > are also important to the audio configuration. A card binding describes > the interconections between the devices in the system and provides > identification information for the audio subsystem. This system level > audio integration is a physical thing that can be pointed at that > requires real software control. The bit you were responding to with that was a disingenuous question. Probably not fair of me to ask one of a non-native speaker like that, when all I wanted to know was whether it was appropriate not to add a more specific compatible, or whether this was something invariant. > Like I said before please look at the existing audio card bindings. Yah, ofc I did that to see if there were other similar bindings that used specific compatibles...
On Tue, Jun 13, 2023 at 03:29:33PM +0100, Conor Dooley wrote: > On Tue, Jun 13, 2023 at 02:54:36PM +0100, Mark Brown wrote: > > are also important to the audio configuration. A card binding describes > > the interconections between the devices in the system and provides > > identification information for the audio subsystem. This system level > > audio integration is a physical thing that can be pointed at that > > requires real software control. > The bit you were responding to with that was a disingenuous question. > Probably not fair of me to ask one of a non-native speaker like that, > when all I wanted to know was whether it was appropriate not to add > a more specific compatible, or whether this was something invariant. I don't think that's particularly useful - you'd end up adding a compatible for every single board which we've got no real intention of adding except in the cases where we end up being able to merge a more specific machine driver into a generic one. It'd be more likely to lead to bikeshedding than anything useful I think.
diff --git a/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml new file mode 100644 index 000000000000..61e8babed402 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/loongson,ls-audio-card.yaml @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/loongson,ls-audio-card.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Loongson 7axxx/2kxxx ASoC audio sound card driver + +maintainers: + - Yingkun Meng <mengyingkun@loongson.cn> + +description: + The binding describes the sound card present in loongson + 7axxx/2kxxx platform. The sound card is an ASoC component + which uses Loongson I2S controller to transfer the audio data. + +properties: + compatible: + const: loongson,ls-audio-card + + model: + $ref: /schemas/types.yaml#/definitions/string + description: User specified audio sound card name + + mclk-fs: + $ref: simple-card.yaml#/definitions/mclk-fs + + cpu: + description: Holds subnode which indicates cpu dai. + type: object + additionalProperties: false + properties: + sound-dai: + maxItems: 1 + required: + - sound-dai + + codec: + description: Holds subnode which indicates codec dai. + type: object + additionalProperties: false + properties: + sound-dai: + maxItems: 1 + required: + - sound-dai + +required: + - compatible + - model + - mclk-fs + - cpu + - codec + +additionalProperties: false + +examples: + - | + sound { + compatible = "loongson,ls-audio-card"; + model = "loongson-audio"; + mclk-fs = <512>; + + cpu { + sound-dai = <&i2s>; + }; + codec { + sound-dai = <&es8323>; + }; + };