From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965157AbcALMR4 (ORCPT ); Tue, 12 Jan 2016 07:17:56 -0500 Received: from szxga01-in.huawei.com ([58.251.152.64]:17592 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934217AbcALMRy (ORCPT ); Tue, 12 Jan 2016 07:17:54 -0500 Subject: Re: [PATCH 01/23] devicetree: bindings: hisi_sas: add v2 HW bindings To: Mark Rutland References: <1452262542-64589-1-git-send-email-john.garry@huawei.com> <1452262542-64589-2-git-send-email-john.garry@huawei.com> <20160108145256.GG3097@leverpostej> <568FD281.4050207@huawei.com> <20160108151934.GA32692@leverpostej> <568FD70D.1090303@huawei.com> <20160108164910.GD32692@leverpostej> <5693B59A.6000705@huawei.com> CC: , , , , , , , , , , , , From: John Garry Message-ID: <5694EEA2.8060000@huawei.com> Date: Tue, 12 Jan 2016 12:16:34 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <5693B59A.6000705@huawei.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.203.181.160] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A090201.5694EEBB.00A7,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 75dbf9fe9ce31a5b9912dcdf0abaa26a Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/01/2016 14:00, John Garry wrote: >>> This is a specific issue for hip06 chipset. >> >> Ok. So is this: >> >> * a bug within the SAS controller in hip06, or: >> >> * a requirement/bug of an endpoint attached to the controller, or: >> >> * a requirement/bug of some interconnect between the controller and >> endpoint, or: >> >> * some other integration bug? >> > > This is related to how the SAS controller IP was integrated into the > chip. It is related to how many bursts are permitted for this controller > on the AXI bus. > >> Please describe what the issue is that you're trying to work around, not >> only your solution to it. >> >>> There is a bug in the HW on hip06 where controller #1 has to set to 2 >>> registers to non-default values to limit "am-max-transmissions". >> >> I got that. However, I have no idea what "am-max-transmissions" is, no >> idea why you need to limit it (hopefully you can describe that a little >> better above), nor what the semantics are for "limit". >> > > So am-max-transmissions is a SW configurable feature of the controller. > From a high-level, it means how many commands we can send in parallel > to the end device(s) without waiting for a response. It is dependent on > the chip bus design. > >> The description of the property is an imperative, which reads like a >> description of a specific driver behaviour rather than a property of the >> hardware that leads to that behaviour being necessary. That's a warning >> sign that the property is ill-defined, and we may encounter problems in >> future due to changes in Linux. >> > > Agreed. > >> Without knowing _why_ it's necessary to limit this, it's not possible to >> know if the property is both necessary and sufficient to solve the >> problem such that it doesn't rear its ugly head in future. >> >> For example, if this is simply one way to work around a hip06-specific >> integration bug that we cannot imagine occurring elsewhere, it may be >> better to instead key off of a platform-specific compatible string for >> the v2 SAS controller in hip06. That gives us more freedom to apply >> different workarounds if we have to. >> >> I see that the presence of this property will cause the driver to writes >> hard-coded values two two registers. Not knowing the format of those >> registers, their default values, nor how they respond to writes, I can't >> tell: >> > > As for writing hardcoded values, by default the related registers will > hold 0x40, which means we can have upto 64 outstanding requests on this > controller. Due to controller #1 integration restrictions, we can only > issue 32 requests. > >> * If the writes have other effects. >> >> * If the limit is a single bit being flipped (i.e. this is a boolean in >> hardware too). >> >> * If the limit is some arbitrary chosen value which is not described in >> the property or the binding, nor what that value is. If we encounter a >> similar bug requiring a different bound in future, it may be >> problematic to have chosen an arbitrary fixed value, and it may make >> more sense to describe the value in the DT. >> >> So, please: >> >> * Update the DT property description to describe the specific HW issue >> that needs to be worked around, with a full description in the commit >> message. >> >> * Add a comment to the driver to explain what the effect of the register >> writes is intended to be, i.e. what value am max transmissions is >> being set to, and why that value isn't arbitrary. >> > > As I understand, there are no more restictions/special requirements for > controller #1. This v2 controller IP will be used in other chips, so we > may have this issue again - I am seeking information from HW people. As > such, it may be useful to know this info before decided on how this is > decribed in the DT. This issue is specific to only SAS controller #1 in hip06. The controller is designed to permit upto 64, but, due to chip bus design, this specific controller is limited to support 32. So, after considering this, would a platform-specific compatible string be a better way of decribing this specific controller? > >>> This would not be a common SAS/SCSI controller property and is >>> specific to our HW. >> >> Ok. >> >> Thanks, >> Mark. >> > thanks, John > _______________________________________________ > linuxarm mailing list > linuxarm@huawei.com > http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm