All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Peter De Schrijver
	<pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Mikko Perttunen
	<mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Laxman Dewangan
	<ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	"wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org"
	<wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>,
	"thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org"
	<thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	"linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH] i2c: i2c-tegra: Move clk_prepare/clk_set_rate to probe
Date: Fri, 15 Aug 2014 15:46:49 -0600	[thread overview]
Message-ID: <53EE7FC9.5010509@wwwdotorg.org> (raw)
In-Reply-To: <20140815213442.GJ1626-Rysk9IDjsxmJz7etNGeUX8VPkgjIgRvpAL8bYrjMMd8@public.gmane.org>

On 08/15/2014 03:34 PM, Peter De Schrijver wrote:
> On Fri, Aug 15, 2014 at 09:45:46PM +0200, Peter De Schrijver wrote:
>> On Fri, Aug 15, 2014 at 08:07:01PM +0200, Stephen Warren wrote:
>>>>> However, the new code sets the clock rate after the clock is prepared. I
>>>>> think the rate should be set first, then the clock prepared. While this
>>>>> likely doesn't apply to the Tegra clock controller, prepare() is allowed
>>>>> to enable the clock if enable() can't be implemented in an atomic
>>>>> fashion (in which case enable/disable would be no-ops), and we should
>>>>> make sure that the driver correctly configures the clock before
>>>>> potentially enabling it.
>>>>>
>>>>> I'm not sure if a similar change to our SPI drivers is possible; after
>>>>> all, the SPI transfer rate can vary per message, so if clk_set_rate()
>>>>> acquires a lock, it seems there's no way to avoid the issue there.
>>>>
>>>> Even for i2c this could be the case I think if you use the highspeed (3.4Mhz)
>>>> mode? From what I remember, a highspeed i2c transaction starts with a lower
>>>> speed preamble to make sure non highspeed slaves don't get confused? Which
>>>> means you could change the bus speed depending on the slave you're addressing.
>>>
>>> Since there's no separate chip-select for I2C, I believe all I2C devices
>>> need to be able to understand the entire transaction, so the I2C bus
>>> speed is fixed.
>>>
>>
>> Does it? I would assume the slave only needs to check if the address matches
>> its own address after a START condition and if not can just wait until the
>> STOP condition appears on the bus?
>>
>
> http://www.nxp.com/documents/user_manual/UM10204.pdf says you can mix them by
> using an interconnect bridge between the highspeed and the non-highspeed
> capable slaves. The bridge uses the special preamble to disconnect the non-
> highspeed part of the bus when a highspeed transaction is ongoing. It's afaics
> transparent to the master.

I expect that works by echoing the slow-speed pre-amble to the 
slow-speed bus segment, then emitting a stop and turning off the echo. 
For actual slow-speed transactions, the whole thing would be echo'd. 
That way the slow-speed devices don't ever see any high-speed pulses.

That all said, that does indeed imply that a master supporting the 
high-speed transactions would need to emit a varying-speed signal. My 
assumption would be that this happens inside the I2C HW, rather than 
under SW control though, since the transition would need to happen 
mid-protocol. Still, perhaps the selection between low-speed and 
high-speed-with-a-slow-preamble might need SW clock programming 
depending on the HW though... Who knows.

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren@wwwdotorg.org>
To: Peter De Schrijver <pdeschrijver@nvidia.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>,
	Laxman Dewangan <ldewangan@nvidia.com>,
	"wsa@the-dreams.de" <wsa@the-dreams.de>,
	"thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] i2c: i2c-tegra: Move clk_prepare/clk_set_rate to probe
Date: Fri, 15 Aug 2014 15:46:49 -0600	[thread overview]
Message-ID: <53EE7FC9.5010509@wwwdotorg.org> (raw)
In-Reply-To: <20140815213442.GJ1626@tbergstrom-lnx.Nvidia.com>

On 08/15/2014 03:34 PM, Peter De Schrijver wrote:
> On Fri, Aug 15, 2014 at 09:45:46PM +0200, Peter De Schrijver wrote:
>> On Fri, Aug 15, 2014 at 08:07:01PM +0200, Stephen Warren wrote:
>>>>> However, the new code sets the clock rate after the clock is prepared. I
>>>>> think the rate should be set first, then the clock prepared. While this
>>>>> likely doesn't apply to the Tegra clock controller, prepare() is allowed
>>>>> to enable the clock if enable() can't be implemented in an atomic
>>>>> fashion (in which case enable/disable would be no-ops), and we should
>>>>> make sure that the driver correctly configures the clock before
>>>>> potentially enabling it.
>>>>>
>>>>> I'm not sure if a similar change to our SPI drivers is possible; after
>>>>> all, the SPI transfer rate can vary per message, so if clk_set_rate()
>>>>> acquires a lock, it seems there's no way to avoid the issue there.
>>>>
>>>> Even for i2c this could be the case I think if you use the highspeed (3.4Mhz)
>>>> mode? From what I remember, a highspeed i2c transaction starts with a lower
>>>> speed preamble to make sure non highspeed slaves don't get confused? Which
>>>> means you could change the bus speed depending on the slave you're addressing.
>>>
>>> Since there's no separate chip-select for I2C, I believe all I2C devices
>>> need to be able to understand the entire transaction, so the I2C bus
>>> speed is fixed.
>>>
>>
>> Does it? I would assume the slave only needs to check if the address matches
>> its own address after a START condition and if not can just wait until the
>> STOP condition appears on the bus?
>>
>
> http://www.nxp.com/documents/user_manual/UM10204.pdf says you can mix them by
> using an interconnect bridge between the highspeed and the non-highspeed
> capable slaves. The bridge uses the special preamble to disconnect the non-
> highspeed part of the bus when a highspeed transaction is ongoing. It's afaics
> transparent to the master.

I expect that works by echoing the slow-speed pre-amble to the 
slow-speed bus segment, then emitting a stop and turning off the echo. 
For actual slow-speed transactions, the whole thing would be echo'd. 
That way the slow-speed devices don't ever see any high-speed pulses.

That all said, that does indeed imply that a master supporting the 
high-speed transactions would need to emit a varying-speed signal. My 
assumption would be that this happens inside the I2C HW, rather than 
under SW control though, since the transition would need to happen 
mid-protocol. Still, perhaps the selection between low-speed and 
high-speed-with-a-slow-preamble might need SW clock programming 
depending on the HW though... Who knows.


  parent reply	other threads:[~2014-08-15 21:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-15  9:47 [PATCH] i2c: i2c-tegra: Move clk_prepare/clk_set_rate to probe Mikko Perttunen
2014-08-15  9:47 ` Mikko Perttunen
     [not found] ` <1408096034-17270-1-git-send-email-mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-08-15 16:18   ` Stephen Warren
2014-08-15 16:18     ` Stephen Warren
     [not found]     ` <53EE32C7.6000500-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2014-08-15 18:02       ` Peter De Schrijver
2014-08-15 18:02         ` Peter De Schrijver
     [not found]         ` <20140815180218.GH1626-Rysk9IDjsxmJz7etNGeUX8VPkgjIgRvpAL8bYrjMMd8@public.gmane.org>
2014-08-15 18:07           ` Stephen Warren
2014-08-15 18:07             ` Stephen Warren
     [not found]             ` <53EE4C45.5080805-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2014-08-15 19:45               ` Peter De Schrijver
2014-08-15 19:45                 ` Peter De Schrijver
     [not found]                 ` <20140815194546.GI1626-Rysk9IDjsxmJz7etNGeUX8VPkgjIgRvpAL8bYrjMMd8@public.gmane.org>
2014-08-15 21:34                   ` Peter De Schrijver
2014-08-15 21:34                     ` Peter De Schrijver
     [not found]                     ` <20140815213442.GJ1626-Rysk9IDjsxmJz7etNGeUX8VPkgjIgRvpAL8bYrjMMd8@public.gmane.org>
2014-08-15 21:46                       ` Stephen Warren [this message]
2014-08-15 21:46                         ` Stephen Warren
     [not found]                         ` <53EE7FC9.5010509-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
2014-08-15 22:18                           ` Peter De Schrijver
2014-08-15 22:18                             ` Peter De Schrijver
2014-09-02 11:56       ` Wolfram Sang
2014-09-02 11:56         ` Wolfram Sang
2014-09-02 15:17         ` Stephen Warren
2014-09-02 15:17           ` Stephen Warren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53EE7FC9.5010509@wwwdotorg.org \
    --to=swarren-3lzwwm7+weoh9zmkesr00q@public.gmane.org \
    --cc=ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mperttunen-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org \
    --cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.