From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933109AbcAKOCP (ORCPT ); Mon, 11 Jan 2016 09:02:15 -0500 Received: from szxga02-in.huawei.com ([119.145.14.65]:14644 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760227AbcAKOCK (ORCPT ); Mon, 11 Jan 2016 09:02:10 -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> CC: , , , , , , , , , , , , , From: John Garry Message-ID: <5693B59A.6000705@huawei.com> Date: Mon, 11 Jan 2016 14:00:58 +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: <20160108164910.GD32692@leverpostej> 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.0A020202.5693B5B2.0202,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: 9c552173c0206cc0cb614a1a9b064281 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> 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 would not be a common SAS/SCSI controller property and is >> specific to our HW. > > Ok. > > Thanks, > Mark. > Cheers, John