Re: [PATCHv2 1/3] dt-bindings: spi: Add DT bindings for Synquacer

From: Trent Piepho <tpiepho-cgc2CodaaHDQT0dZR+AlfA@public.gmane.org>
To: "jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org"
	<jassisinghbrar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org"
	<robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: "linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"mark.rutland-5wv7dgnIgG8@public.gmane.org"
	<mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org"
	<ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org"
	<broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org"
	<masami.hiramatsu-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org"
	<jaswinder.singh-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCHv2 1/3] dt-bindings: spi: Add DT bindings for Synquacer
Date: Tue, 16 Jan 2018 20:02:06 +0000	[thread overview]
Message-ID: <1516132926.25398.98.camel@impinj.com> (raw)
In-Reply-To: <CABb+yY1f=NwB_UmGAogTj2AgA5stk5zic39HJgTSJcv1rdcQVw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2390 bytes --]

On Tue, 2018-01-16 at 08:59 +0530, Jassi Brar wrote:
> On Mon, Jan 15, 2018 at 10:47 PM, Rob Herring <robh+dt@kernel.org> wrote:
> > On Mon, Jan 15, 2018 at 9:53 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > > On Mon, Jan 15, 2018 at 8:45 PM, Rob Herring <robh+dt@kernel.org> wrote:
> > > > On Mon, Jan 15, 2018 at 7:05 AM,  <jassisinghbrar@gmail.com> wrote:
> > > > > 
> > > > > +- clocks: Must contain an entry for rate source clock(s).
> > > > > +- clock-names: Shall be "iHCLK" or "iPCLK". iHCLK is preferred over iPCLK
> > > > 
> > > > Huh? The clock binding should reflect all clocks connected to a block,
> > > > not a selection of which one you want to use.
> > > > 
> > > 
> > > Both the clocks are internal to the block and derived from the same source.
> > > Instead of defining a new "use-ipclk" property, the driver uses the
> > > clock-names to choose the appropriate divider.
> > > I am open to any better option.
> > 
> > If one is preferred, then why not always use it? Or how does one
> > decide which clock to use?
> > 
> 
> A slight correction, there is a mux inside the block which selects
> clock from two input ports (iPCLK and iHCLK) and send that to the
> divider. Depending upon the spi slave speed requirements the platform

It seems like the DT is a reasonable description of how the hardware is
designed.  It's possible to choose different clocks with different
rates to be wired to PCLK and/or HCLK, via some internal clock tree in
the SoC?  That's what the DT would indicate.

You could maybe allow one or both to be connected, and if both are
connected, have the driver prefer one based on some driver logic. 
Which would allow one to then...

> Now the idea dawns that DT provide both clocks and the driver select
> from the mux looking at speed requirements of the slave at runtime.

add this feature at some point.  It might be simpler to just prefer
HCLK if both are there.  The idea being that the DT should describe the
hardware rather than tell the driver what to do.

Speed can be changed on a xfer by xfer basis on a single message to a
single slave.  Trying to switch clocks during a message, if it requires
resetting the whole block, sounds rather difficult.

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i