Re: [RFC PATCH 1/2] dt-bindings: media: snps,dw-hdmi-rx: add #sound-dai-cells

[Igor Paunovic]

Hello Sebastian,

> It should be 1 from the start as the DT binding is supposed to
> describe the hardware and be stable.

Thanks -- keeping const: 1 then.

> But in the next version you should look into updating the commit
> descriptions to be less about the kernel driver and more about
> the hardware.

Will do. I'll use your wording as the base for the v2 commit
message, document that argument 0 is the I2S and 1 the S/PDIF
interface in the property description, and move the consumer
reference (sound-dai = <&hdmi_receiver 0>) into the example
section.

For completeness: the driver part of the series currently wires
up only the I2S interface (DAI 0); the binding now describes both
interfaces as per the hardware.

Thanks for the review!

Igor

On Wed, Jul 15, 2026 at 3:00 PM Sebastian Reichel
<sebastian.reichel@xxxxxxxxxxxxx> wrote:
>
> Hello Igor,
>
> On Wed, Jul 15, 2026 at 08:55:37AM +0200, Royal Net wrote:
> > > Should the constraint be set to 0 to follow standard device tree
> > > norms?
> >
> > Fair point. The current RFC wires up a single I2S DAI, and for a
> > single-DAI provider 0 is indeed the norm -- rockchip,dw-hdmi does
> > exactly that for its one DAI on the TX side.
> >
> > The reason I picked 1 is that the controller also has an S/PDIF
> > audio output path (the vendor driver exposes both I2S and S/PDIF
> > DAIs through hdmi-codec), so a DAI index becomes meaningful if that
> > second path is wired up later.
> >
> > Unless the DT maintainers prefer keeping 1 from the start to avoid
> > changing the cell count if/when the S/PDIF DAI is added, I'll switch
> > to const: 0 in the next revision -- please let me know which you'd
> > prefer.
>
> It should be 1 from the start as the DT binding is supposed to
> describe the hardware and be stable. But in the next version you
> should look into updating the commit descriptions to be less about
> the kernel driver and more about the hardware. Maybe something like
>
> > The HDMI RX controller has two digital audio interfaces, one for
> > I2S and one for S/PDIF. Allow referencing connections from audio
> > codecs to these interfaces by documenting the missing #sound-dai-cells
> > property.
>
> Apart from that it makes sense to document that argument 0 means
> I2S and argument 1 means S/PDIF interface in the property description.
>
> Last but not least drop "A value of 1 lets a sound card reference
> the audio DAI as <&hdmirx 0>.". Instead you can add this to the
> example section at the end of the file.
>
> Greetings,
>
> -- Sebastian
>
> >
> > Igor
> >
> > On Wed, Jul 15, 2026 at 7:54 AM <sashiko-bot@xxxxxxxxxx> wrote:
> > >
> > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > - [Medium] Incorrect cell count constraint for single-DAI provider.
> > > --
> > >
> > > --- Patch [1]: [RFC PATCH 1/2] dt-bindings: media: snps,dw-hdmi-rx: add #sound-dai-cells ---
> > > commit e6b0bac0b97f568ce749084e6c1e728bcd258b10
> > > Author: Igor Paunovic <royalnet026@xxxxxxxxx>
> > > dt-bindings: media: snps,dw-hdmi-rx: add #sound-dai-cells
> > >
> > > This commit adds the #sound-dai-cells property to the HDMI RX controller
> > > bindings, enabling a generic audio codec to capture incoming embedded audio.
> > >
> > > > diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> > > > index b7f6c87d0e06a..83c10d9997c8e 100644
> > > > --- a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> > > > +++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> > > > @@ -78,6 +78,13 @@ properties:
> > > >        The phandle of the syscon node for the Video Output GRF register
> > > >        to enable EDID transfer through SDAIN and SCLIN.
> > > >
> > > > +  "#sound-dai-cells":
> > > > +    const: 1
> > > > +    description:
> > > > +      The HDMI RX controller registers a generic HDMI audio codec so the
> > > > +      audio embedded in the incoming stream can be captured. A value of 1
> > > > +      lets a sound card reference the audio DAI as <&hdmirx 0>.
> > >
> > > [Severity: Medium]
> > > Is this cell count constraint correct for a single-DAI provider?
> > >
> > > Looking at properties:#sound-dai-cells, the hardware exposes only a single
> > > generic audio DAI. Requiring a cell count of 1 forces device tree nodes to
> > > use a non-standard syntax like <&hdmirx 0> instead of just <&hdmirx> for a
> > > single resource.
> > >
> > > Should the constraint be set to 0 to follow standard device tree norms?
> > >
> > > --
> > > Sashiko AI review · https://sashiko.dev/#/patchset/20260715051939.64652-1-royalnet026@xxxxxxxxx?part=1