archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <>
To: "Pali Rohár" <>
Cc: "Marek Behún" <>,,
	Rötti <>,
	"Krzysztof Wilczyński" <>
Subject: Re: [PATCH] PCI: Add Max Payload Size quirk for ASMedia ASM1062 SATA controller
Date: Tue, 1 Jun 2021 12:09:07 -0500	[thread overview]
Message-ID: <20210601170907.GA1949035@bjorn-Precision-5520> (raw)
In-Reply-To: <20210528001208.z4g4f7wlu65dxypj@pali>

[+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

  - "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.


  reply	other threads:[~2021-06-01 17:09 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-17 11:59 Marek Behún
2021-03-17 18:46 ` Pali Rohár
2021-03-17 22:45 ` Bjorn Helgaas
2021-03-17 22:55   ` Pali Rohár
2021-03-17 23:03     ` Bjorn Helgaas
2021-03-19 19:02       ` Pali Rohár
2021-03-21 15:09         ` Rötti
2021-05-30 10:21           ` Pali Rohár
2021-08-26 11:00           ` Rötti
2021-03-17 23:09   ` Marek Behún
2021-04-16 13:54   ` Marek Behún
2021-04-25 15:29     ` Pali Rohár
2021-05-11 16:16 ` Marek Behún
2021-05-28  0:12   ` Pali Rohár
2021-06-01 17:09     ` Bjorn Helgaas [this message]
2021-06-01 21:15       ` Bjorn Helgaas
2021-06-01 11:36 ` Krzysztof Wilczyński

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210601170907.GA1949035@bjorn-Precision-5520 \ \ \ \ \ \ \
    --subject='Re: [PATCH] PCI: Add Max Payload Size quirk for ASMedia ASM1062 SATA controller' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).