Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754161AbcCBJLe (ORCPT ); Wed, 2 Mar 2016 04:11:34 -0500 Received: from mout.kundenserver.de ([212.227.126.135]:62732 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750826AbcCBJL2 (ORCPT ); Wed, 2 Mar 2016 04:11:28 -0500 From: Arnd Bergmann To: Michal Simek Cc: Anurag Kumar Vulisha , Rob Herring , =?ISO-8859-1?Q?S=F6ren?= Brinkmann , "pawel.moll@arm.com" , "mark.rutland@arm.com" , "ijc+devicetree@hellion.org.uk" , "galak@codeaurora.org" , "tj@kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-ide@vger.kernel.org" , Anirudha Sarangi , Srikanth Vemula , Punnaiah Choudary Kalluri Subject: Re: [RFC PATCH] drivers: ata: Read Rx water mark value from device-tree Date: Wed, 02 Mar 2016 10:11:06 +0100 Message-ID: <3012728.7zDNPtQ7kR@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <56D69EDD.40400@xilinx.com> References: <1455974302-7082-1-git-send-email-anuragku@xilinx.com> <3802E9A6666DF54886E2B9CBF743BA9825E025F6@XAP-PVEXMBX01.xlnx.xilinx.com> <56D69EDD.40400@xilinx.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:7CjSrRIrw0xh+kLbJQrp1fRgdzmvUZ71vYdgoR9KyBbJAPVoIg8 zgAImMDzhWFCJI98b4s/Q5V3QQlNbLSltMrwFm0N+/7TZKjk8jh+VJ2Gc2/lnVFEi/fIR9d d/UTyejTQYtpXNZbkZeck+kDzjTry9qNhmUgTsn8oNKvBK8gOjRifWJNLTJZcKocr9/1e40 rTT2sdVVCvdpRViUJg4Dw== X-UI-Out-Filterresults: notjunk:1;V01:K0:QGn/IE1haWw=:wAapUiwmT0np0qRjUHlEtG BdoZb8KBb1hkFCCVS2boujPcgcEr7z0N4ab+IrCjqku1qd/dhi7iZUR1Gq2y3ywtzrOBJ9cmG KIPUwgoPock8aQRTMjtcKpNQCKdIPaBzPifKBI2bDp7swR8dj+DW6jRS/WgtCc+Yf8EB2BDTU 4f80UROB9RziOUUm79kr3ltklsDP42UV4y2SzOggxST5KlapRS9/1TIFD7tcggWgqtls7cHXl vas9SyXm2NgtVhwx8orIwDK88mSyOTXi0peVH5+tz3Zve9z3wWcssq/3m150buugZ8hkc/yv1 evSusOLbpQr3LjnqYs+kQTiO7H6wtfXUax1c0aw9bIgb4fFn9HXZEWUWmBl/jebVDimIJiJRb q8YNYGgMUvmdYZws2fqPisiTqt393y6qPF0srMSp67qrBVHIfCHMQjn9m416a81UrONP6+Bya chXddoxa4/FwfKG8viAiE8AIwH4j1xn+30S/8X0eaM6wLhpNUrbfhgALBbqUY+Ce99IkswS6w cLUke3GCz5gEnghcCKUdfgsysHYQK1T/fgGYZwJVlU22DaKEnLmkuzCSvxyzZaUs96RZDkCFB ixli/H8ZTZTsy8HsmFCm2oRzJz9wGX6c9kUfRRrdo27OmNZg5iPq6qq4mH5JuS9AcsSc9KSrN 8iA1+6maMJRlrHLNMYQNIpOqTNUPPVSd0PqslDnoTP4XmOJR0PByHRpJSG3h7skT8EYxCwdoE ga4V090ktdhu75Cz Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1825 Lines: 38 On Wednesday 02 March 2016 09:05:49 Michal Simek wrote: > On 2.3.2016 06:53, Anurag Kumar Vulisha wrote: > >>>>> > >>>>> I would probably make this dependent on the compatible string > >>>>> instead, and have a table in the device driver that uses a > >>>>> specific value for each variant of the device, but either way should be > >> fine. > >>>>> > >>>>> Having a separate property is most appropriate if for each > >>>>> hardware revision there is exactly one ideal value, while a table > >>>>> in the driver makes more sense if this takes a bit of tuning and > >>>>> the driver might choose to optimize it differently based on other > >>>>> constraints, such as its own interrupt handler implementation. > > that 0x40 is value choose based on testing that it is not causing any > visible problem and this is used as default value in the driver > (PTC_RX_WM_VAL - ahci_ceva.c) > > Values which you can setup are in range 0x0 - 0x7f (7bits). It means > hardware fifo size is probably 0x80. > > And this dt/module parameter is IMHO just sw setting setup by user. > It means I am not quite sure that this is DT parameter because it is > just SW setting. > I expect this range will be valid for all silicon revisions. > If happen that any silicon revision can't setup certain level because of > HW bug we can handle it via DT parameter or specific compatible string. > But setting up watermark SW level via DT doesn't look correct to me. > > Please let me know what you think. Ok, thanks for the background. I think we should just leave it to be set by the driver then. Please make sure that each SoC specific .dtsi file has a unique "compatible" string for the device though, so that the driver can later override it based on the specific variant if that ends up being necessary for performance or bug-avoidance. Arnd