From: Bjorn Helgaas <email@example.com> To: "Pali Rohár" <firstname.lastname@example.org> Cc: "Marek Behún" <email@example.com>, firstname.lastname@example.org, Rötti <email@example.com>, "Krzysztof Wilczyński" <firstname.lastname@example.org> 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  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.  https://email@example.com)
next prev parent 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: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH] PCI: Add Max Payload Size quirk for ASMedia ASM1062 SATA controller' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * 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).