| Message ID | 20200730055319.1522-5-michael.wei.hong.sit@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ASoC: Intel: KMB: TDM Enablement patches | expand |
On Thu, Jul 30, 2020 at 01:53:19PM +0800, Michael Sit Wei Hong wrote: > Add a property that configures the interface to the desired max number > of capture channels. The platform may have multiple interfaces with > different number of capture channels. Why? None of the other platforms which support many channels need this defining and the constraint code and/or machine driver would normally be where this would be handled.
> -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Thursday, 30 July, 2020 7:30 PM > To: Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com> > Cc: alsa-devel@alsa-project.org; tiwai@suse.com; pierre- > louis.bossart@linux.intel.com; Rojewski, Cezary > <cezary.rojewski@intel.com>; Shevchenko, Andriy > <andriy.shevchenko@intel.com>; liam.r.girdwood@linux.intel.com; Sia, > Jee Heng <jee.heng.sia@intel.com> > Subject: Re: [PATCH 4/4] dt-bindings: sound: intel,keembay-i2s: Add > channel-max property > > On Thu, Jul 30, 2020 at 01:53:19PM +0800, Michael Sit Wei Hong wrote: > > > Add a property that configures the interface to the desired max number > > of capture channels. The platform may have multiple interfaces with > > different number of capture channels. > > Why? None of the other platforms which support many channels need > this defining and the constraint code and/or machine driver would > normally be where this would be handled. The platform uses the audio-graph-card to create the dai-links, and doesn't use a specific machine driver code. The platform also has 2 different interfaces which have different supported max-channels. Using this value in the device-tree to determine the maximum supported channel of the interface.
On Mon, Aug 03, 2020 at 01:59:10AM +0000, Sit, Michael Wei Hong wrote: > > On Thu, Jul 30, 2020 at 01:53:19PM +0800, Michael Sit Wei Hong wrote: > > > Add a property that configures the interface to the desired max number > > > of capture channels. The platform may have multiple interfaces with > > > different number of capture channels. > > Why? None of the other platforms which support many channels need > > this defining and the constraint code and/or machine driver would > > normally be where this would be handled. > The platform uses the audio-graph-card to create the dai-links, and doesn't use a specific machine driver code. The audio-graph-card is very flexible and if it doesn't support something which it is useful to configure per platform then that's the place to add the extension, not some DAI specific driver. > The platform also has 2 different interfaces which have different supported max-channels. > Using this value in the device-tree to determine the maximum supported channel of the interface. These should have different compatible strings, there are likely further differences between them (even if they are not currently documented). 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.
> -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Monday, 3 August, 2020 6:49 PM > To: Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com> > Cc: alsa-devel@alsa-project.org; tiwai@suse.com; pierre- > louis.bossart@linux.intel.com; Rojewski, Cezary > <cezary.rojewski@intel.com>; Shevchenko, Andriy > <andriy.shevchenko@intel.com>; liam.r.girdwood@linux.intel.com; > Sia, Jee Heng <jee.heng.sia@intel.com> > Subject: Re: [PATCH 4/4] dt-bindings: sound: intel,keembay-i2s: Add > channel-max property > > On Mon, Aug 03, 2020 at 01:59:10AM +0000, Sit, Michael Wei Hong > wrote: > > > On Thu, Jul 30, 2020 at 01:53:19PM +0800, Michael Sit Wei Hong > wrote: > > > > > Add a property that configures the interface to the desired max > > > > number of capture channels. The platform may have multiple > > > > interfaces with different number of capture channels. > > > > Why? None of the other platforms which support many > channels need > > > this defining and the constraint code and/or machine driver > would > > > normally be where this would be handled. > > > The platform uses the audio-graph-card to create the dai-links, > and doesn't use a specific machine driver code. > > The audio-graph-card is very flexible and if it doesn't support > something which it is useful to configure per platform then that's > the place to add the extension, not some DAI specific driver. > > > The platform also has 2 different interfaces which have different > supported max-channels. > > Using this value in the device-tree to determine the maximum > supported channel of the interface. > > These should have different compatible strings, there are likely > further differences between them (even if they are not currently > documented). > The 2 different I2S ports are from the same SoC which supports different number of channels, do we need different compatible strings for this? Considering the only difference is the maximum supported channels is 8 and 2? > 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.
On Tue, Aug 04, 2020 at 01:57:23AM +0000, Sit, Michael Wei Hong wrote: > > > The platform also has 2 different interfaces which have different > > supported max-channels. > > > Using this value in the device-tree to determine the maximum > > supported channel of the interface. > > These should have different compatible strings, there are likely > > further differences between them (even if they are not currently > > documented). > The 2 different I2S ports are from the same SoC which supports different > number of channels, do we need different compatible strings for this? > Considering the only difference is the maximum supported channels is 8 and 2? Are you *sure* that's the only difference, or is that just the only difference you know about right now?
> -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Tuesday, 4 August, 2020 7:50 PM > To: Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com> > Cc: alsa-devel@alsa-project.org; tiwai@suse.com; pierre- > louis.bossart@linux.intel.com; Rojewski, Cezary > <cezary.rojewski@intel.com>; Shevchenko, Andriy > <andriy.shevchenko@intel.com>; liam.r.girdwood@linux.intel.com; > Sia, Jee Heng <jee.heng.sia@intel.com> > Subject: Re: [PATCH 4/4] dt-bindings: sound: intel,keembay-i2s: Add > channel-max property > > On Tue, Aug 04, 2020 at 01:57:23AM +0000, Sit, Michael Wei Hong > wrote: > > > > > The platform also has 2 different interfaces which have > different > > > supported max-channels. > > > > Using this value in the device-tree to determine the maximum > > > supported channel of the interface. > > > > These should have different compatible strings, there are likely > > > further differences between them (even if they are not currently > > > documented). > > > The 2 different I2S ports are from the same SoC which supports > > different number of channels, do we need different compatible > strings for this? > > Considering the only difference is the maximum supported > channels is 8 and 2? > > Are you *sure* that's the only difference, or is that just the only > difference you know about right now? Yes, I am fairy sure that is the only difference according to the design, as per the platform use case.
On Wed, Aug 05, 2020 at 06:21:14AM +0000, Sit, Michael Wei Hong wrote: > > Are you *sure* that's the only difference, or is that just the only > > difference you know about right now? > Yes, I am fairy sure that is the only difference according to the design, as per the platform use case. It would still be safer to have a separate compatible, it wouldn't be the first time that additional changes that the hardware people had failed to communicate were discovered. 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.
> -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Friday, 7 August, 2020 10:31 PM > To: Sit, Michael Wei Hong <michael.wei.hong.sit@intel.com> > Cc: alsa-devel@alsa-project.org; tiwai@suse.com; pierre- > louis.bossart@linux.intel.com; Rojewski, Cezary > <cezary.rojewski@intel.com>; Shevchenko, Andriy > <andriy.shevchenko@intel.com>; liam.r.girdwood@linux.intel.com; > Sia, Jee Heng <jee.heng.sia@intel.com> > Subject: Re: [PATCH 4/4] dt-bindings: sound: intel,keembay-i2s: Add > channel-max property > > On Wed, Aug 05, 2020 at 06:21:14AM +0000, Sit, Michael Wei Hong > wrote: > > > > Are you *sure* that's the only difference, or is that just the only > > > difference you know about right now? > > > Yes, I am fairy sure that is the only difference according to the > design, as per the platform use case. > > It would still be safer to have a separate compatible, it wouldn't be > the first time that additional changes that the hardware people had > failed to communicate were discovered. > I will create a separate compatible string in the next revision of the patch. Since part of the patch series has already been merged, do I send in the new patch as a new review or on top of these patch series? > 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.
On Mon, Aug 10, 2020 at 09:47:15AM +0000, Sit, Michael Wei Hong wrote: > Since part of the patch series has already been merged, do I send in the new > patch as a new review or on top of these patch series? Only send the patch that hasn't been applied yet, it should be based on top of the current code (including the patches that have already been applied) but no need to resend anything that was already applied.
diff --git a/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml b/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml index 175e4fb0c315..9345fc3a2b95 100644 --- a/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml +++ b/Documentation/devicetree/bindings/sound/intel,keembay-i2s.yaml @@ -43,6 +43,10 @@ properties: - const: osc - const: apb_clk + channel-max: + items: + - description: Maximum audio input channels + required: - compatible - "#sound-dai-cells" @@ -51,6 +55,9 @@ required: - clock-names - interrupts +optional: + - channel-max + examples: - | #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -65,4 +72,5 @@ examples: interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>; clock-names = "osc", "apb_clk"; clocks = <&scmi_clk KEEM_BAY_PSS_AUX_I2S3>, <&scmi_clk KEEM_BAY_PSS_I2S3>; + channel-max = <2>; };