Re: [PATCH v5 1/2] PCI: iproc: Retry request when CRS returned from EP

[Oza Oza]

On Fri, Aug 4, 2017 at 6:57 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> On Fri, Aug 04, 2017 at 11:40:46AM +0530, Oza Oza wrote:
>> On Fri, Aug 4, 2017 at 11:29 AM, Oza Oza <oza.oza@xxxxxxxxxxxx> wrote:
>> > On Fri, Aug 4, 2017 at 12:27 AM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>> >> On Thu, Aug 03, 2017 at 01:50:29PM +0530, Oza Oza wrote:
>> >>> On Thu, Aug 3, 2017 at 2:34 AM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>> >>> > On Thu, Jul 06, 2017 at 08:39:41AM +0530, Oza Pawandeep wrote:
> ...
>
>> >>> > What about CRS status for a config *write*?  There's nothing here to
>> >>> > reissue those.
>> >>>
>> >>> No, we do not need there, because read will always be issued first
>> >>> before any write.
>> >>> so we do not need to implement write.
>> >>
>> >> How so?  As far as I know, there's nothing in the spec that requires
>> >> the first config access to a device to be a read, and there are
>> >> reasons why we might want to do a write first:
>> >> http://lkml.kernel.org/r/5952D144.8060609@xxxxxxxxxx
>> >>
>> >
>> > I understand your point here. my thinking was during enumeration
>> > process first read will always be issued
>> > such as vendor/device id.
>> > I will extend this implementation for write.
>>
>> I am sorry, but I just released that, it is not possible to implement
>> retry for write.
>> the reason is:
>>
>> we have indirect way of accessing configuration space access.
>> for e.g.
>> for config write:
>>
>> A) write to to addr register.
>> B) write to data register
>>
>> now above those 2 registers are implemented by host bridge (not in
>> PCIe core IP).
>> there is no way of knowing for software, if write has to be retried.
>>
>> e.g. I can not read data register (step B) to check if write was successful.
>> I have double checked this with internal ASIC team here.
>
> The bottom line is that you're saying this hardware cannot correctly
> support CRS.  Maybe the workaround you're proposing will work in many
> cases, but we need to acknowledge in the code and changelog that there
> are issues we might trip over.

yes this is precisely right.

1) I will have to add notes in the code as you are suggesting.
2) I will add documentation notes in the Change-log.

But even going forward, we will still have one more separate register
in host bridge,
which will be dedicated to CRS. but again to a very limited extent.
because CRS software visibility bit will not have any effect, (e.g. HW
is not going to consider it).

Regards,
Oza.