Re: [PATCH v2 07/10] dt-bindings: adv748x: add information about serial audio interface (I2S/TDM)

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Alex Riesen <alexander.riesen@cetitec.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	 Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	 Kieran Bingham <kieran.bingham@ideasonboard.com>,
	 Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	 Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	 Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	driverdevel <devel@driverdev.osuosl.org>,
	 Linux Media Mailing List <linux-media@vger.kernel.org>,
	 Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	 "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>,
	 Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH v2 07/10] dt-bindings: adv748x: add information about serial audio interface (I2S/TDM)
Date: Fri, 20 Mar 2020 10:15:17 +0100	[thread overview]
Message-ID: <CAMuHMdVtsdNg0s5fio8GAhHGV9H+1J=xvuCXj5VdZ6gwqxGrZw@mail.gmail.com> (raw)
In-Reply-To: <20200320090339.GD4344@pflmari>

Hi Alex,

On Fri, Mar 20, 2020 at 10:03 AM Alex Riesen
<alexander.riesen@cetitec.com> wrote:
> Geert Uytterhoeven, Fri, Mar 20, 2020 09:48:14 +0100:
> > On Fri, Mar 20, 2020 at 9:44 AM Alex Riesen <alexander.riesen@cetitec.com> wrote:
> > > Laurent Pinchart, Thu, Mar 19, 2020 19:01:25 +0100:
> > > > On Thu, Mar 19, 2020 at 06:42:36PM +0100, Alex Riesen wrote:
> > > > > As the driver has some support for the audio interface of the device,
> > > > > the bindings file should mention it.
> >
> > > > > @@ -16,6 +18,8 @@ Required Properties:
> > > > >      slave device on the I2C bus. The main address is mandatory, others are
> > > > >      optional and remain at default values if not specified.
> > > > >
> > > > > +  - #clock-cells: must be <0> if the I2S port is used
> > > >
> > > > Wouldn't it be simpler to set it to 0 unconditionally ?
> > >
> > > Would it? If the port itself is optional, shouldn't the clock be an option
> > > too?
> >
> > You'd be surprised how many board designers would consider this a cheap
> > 12.288 MHz clock source, without using the I2S port ;-)
>
> Well, I am :-)
>
> Especially considering that the driver will not switch the MCLK pin aktive
> (all I2S-related pins are tristate by default).

OK, didn't consider that.  But that still won't stop the hardware designer.
E.g. on Lager, the clock input of the PMIC is tied to the clock line of an SPI
bus, so to use that feature, the SPI clock must be kept running all the time,
not just when doing a transfer.

> And how do I require it to be set unconditionally? By just removing the
> "if ..." part of the statement?

Indeed.  This is still the plain text binding, not yaml.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel