From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753234AbbBWWHm (ORCPT ); Mon, 23 Feb 2015 17:07:42 -0500 Received: from pmta1.delivery10.ore.mailhop.org ([54.149.36.10]:36526 "EHLO pmta1.delivery10.ore.mailhop.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753112AbbBWWHk (ORCPT ); Mon, 23 Feb 2015 17:07:40 -0500 X-Mail-Handler: DuoCircle Outbound SMTP X-Originating-IP: 104.193.169.186 X-Report-Abuse-To: abuse@duocircle.com (see https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information for abuse reporting information) X-MHO-User: U2FsdGVkX19zQI1S/ijYW4s6xDWLAM+6 Date: Mon, 23 Feb 2015 14:03:23 -0800 From: Tony Lindgren To: Robert Abel Cc: Roger Quadros , khilman@deeprootsystems.com, linux@arm.linux.org.uk, linux-omap@vger.kernel.org, Linux Kernel Maling List Subject: Re: [PATCH 2/4] ARM OMAP2+ GPMC: always program GPMCFCLKDIVIDER Message-ID: <20150223220322.GI32521@atomide.com> References: <1424101741-24152-1-git-send-email-rabel@cit-ec.uni-bielefeld.de> <1424101741-24152-2-git-send-email-rabel@cit-ec.uni-bielefeld.de> <1424101741-24152-3-git-send-email-rabel@cit-ec.uni-bielefeld.de> <54E2F803.3070901@ti.com> <54E34786.1010102@ti.com> <54E34F73.2080108@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Robert Abel [150223 13:42]: > On Tue, Feb 17, 2015 at 3:25 PM, Roger Quadros wrote: > > one more thing to note is that just specifying sync-clk-ps in DT is not enough for > > asynchronous devices. > > > > The driver doesn't set gpmc_t->sync_clk if "gpmc,sync-read" or "gpmc,sync-write" > > was not set in the DT, which would be the case for asynchronous devices. > > You're mistaken. It's always parsed, no matter what. See [1]. But I > did implement your suggestion of computing the divider in the mean > time. I'm going to send the patches rebased to 3.19 tomorrow, after I > tested them. > > > What is your proposal to make things better? And what is your use case that doesn't > > work with existing setup? > > Well, the current setup was obviously inspired by an asynchronous-only > use-case, where all the timings are in seconds and get converted > on-the-fly. Of course, currently, there is no support to re-compute > them once the gpmc_fclk changes frequency, but I guess that's what the > TODO about the clock framework is about? > > Anyway, my synchronous use-case is inherently... synchronous. > Synchronous protocols shouldn't be specified in ns at all. They should > directly be specified in clock ticks. This is also advantageous, as > changes in gpmc_fclk don't need to propagate to the registers. > > My main grief with the current setup is its dependency on the > *unknown* gpmc_fclk period at startup when the DT is processed and > GPMC code is called. For instance, my private DT currently operates on > the assumption of 166 Mhz L3 clk (and therefore gpmc_fclk), so all my > timings are in multiples of 6 ns. Should now a colleague of mine think > it would be a splendid idea to change this frequency by means of > bootloader options [to save power, to process data even faster, etc], > everything would break, because the L3 might have to be clocked at 100 > MHz (due to divider limits along the clock tree for example) thus > making my gpmc_fclk period 10ns. Now all my synchronous timings are > wrong, because all DT values round to different clock ticks. > > One solution would obviously be very verbose: Split synchronous and > asynchronous timings completely. Have a time-ns and a time-tck (where > time is waitmonitoring, or readaccess etc) setting for different use > cases. This would work for truly asynchronous (read _and_ write > asynchronous) as well as truly synchronous (read _and_ write > synchronous) setups. > > This 'simple' model breaks for parts where one form of access is > synchronous while the other is asynchronous, e.g. Spansion WS/NS/VS > parts, see [2] pg. 10. There's no easy solution for mixed timings, > because some timing parameters are shared between synchronous and > asynchronous accesses (WAITMONITORINGTIME, CSONTIME, ADVONTIME, > WRDATAONADMUXBUS etc.). One obvious solution is to try to slow the > synchronous side down until asynchronous timings can be met, but that > would make for very slow accesses in most cases. > > For instance, I originally started out thinking the GPMC would run at > 100 MHz externally -- because it's the max frequency rating -- only to > be surprised when the clock looked weird on the GPMC_CLK pin, because > it was at 166 MHz. I'm not sure how to completely remedy this, but > specifying timings in ns in DT and then depending on the correct board > frequency is kind of shady... :( There have been few gpmc devices that were configured to work with changing L3 frequencies. For the related retime function, maybe take a look at the code removed for tusb_set_sync_mode() in commit 47acde167260. Just specifying the ticks is not enough there either.. Sorry I don't have a solution to how the timings should be specified, but both timings and ticks are needed it seems. Regards, Tony > [1]: http://lxr.free-electrons.com/source/drivers/memory/omap-gpmc.c#L1509 > [2]: http://processors.wiki.ti.com/images/5/56/SpansionNOR-AM35x.pdf ; > ("WS/NS/VS can be used for synchronous burst read / asynchronous > single write")