[+cc Krzysztof]
On Fri, May 28, 2021 at 02:12:08AM +0200, Pali Rohár wrote:
> On Tuesday 11 May 2021 18:16:12 Marek Behún wrote:
> > Ping? :)
> >
> > Marek
>
> Bjorn: Gentle reminder :)
The current patch [1] doesn't look mergeable as-is.
- "ASM1062 SATA controller causes an External Abort on controllers
which support Max Payload Size >= 512" doesn't sound like a
correct description.
That description suggests the problem is with the *PCI
controller*, not with the ASM1062. I think that's incorrect; I
think the problem is with the ASM1062.
I would expect something like "ASM1062 advertises Max_Payload_Size
Supported of 512, but in fact it cannot handle TLPs with payload
size of 512."
- Also, "External Abort" is an arch-specific description. Ideally
we would know the PCIe error. Probably ASM1062 reports a
Malformed TLP error when it receives a data payload of 512 bytes,
and Aardvark, DesignWare, etc convert this to an arm64 External
Abort.
- "this problem first appeared ... after 8a3ebd8de328 ..." seems
only marginally relevant. It seems to have exposed the latent
ASM1062 defect. We *could* say something about how the defect was
masked by the fact that many PCI bridges only support MPS <= 256,
and 8a3ebd8de328 would then be relevant because it effectively
limits MPS.
- A bugzilla link is in the email thread; thanks for that.
Somebody needs to include it in a revised commit log.
- The "For some reason DECLARE_PCI_FIXUP_HEADER does not work"
comment in the code needs explanation. If we need to change all
the quirks to EARLY quirks, we should do that. If this one needs
to be different from the others, we need to explain *why*, not
just "for some reason."
I know some of these were addressed in the email thread, and I could
synthesize some of those responses into a commit log, but it would be
better if you collected things up into a v2 posting.
[1] https://lore.kernel.org/r/20210317115924.31885-1-kabel@xxxxxxxxxx)
