Re: [PATCH 01/23] devicetree: bindings: hisi_sas: add v2 HW bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux