LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Rob Herring <robh@kernel.org>
To: "Krzysztof Hałasa" <khalasa@piap.pl>
Cc: "Bjorn Helgaas" <bhelgaas@google.com>,
	PCI <linux-pci@vger.kernel.org>,
	"Artem Lapkin" <email2tema@gmail.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Huacai Chen" <chenhuacai@gmail.com>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Richard Zhu" <hongxing.zhu@nxp.com>,
	"Lucas Stach" <l.stach@pengutronix.de>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] PCIe: limit Max Read Request Size on i.MX to 512 bytes
Date: Mon, 16 Aug 2021 14:34:00 -0500	[thread overview]
Message-ID: <CAL_Jsq+M7xdqf8bVhs-isHoGCGjLhi6N2q+tm7msWLBy52OsMw@mail.gmail.com> (raw)
In-Reply-To: <m3k0klzl1x.fsf@t19.piap.pl>

On Mon, Aug 16, 2021 at 6:27 AM Krzysztof Hałasa <khalasa@piap.pl> wrote:
>
> DWC PCIe controller imposes limits on the Read Request Size that it can
> handle. For i.MX6 family it's fixed at 512 bytes by default.
>
> If a memory read larger than the limit is requested, the CPU responds
> with Completer Abort (CA) (on i.MX6 Unsupported Request (UR) is returned
> instead due to a design error).
>
> The i.MX6 documentation states that the limit can be changed by writing
> to the PCIE_PL_MRCCR0 register, however there is a fixed (and
> undocumented) maximum (CX_REMOTE_RD_REQ_SIZE constant). Tests indicate
> that values larger than 512 bytes don't work, though.
>
> This patch makes the RTL8111 work on i.MX6.
>
> Signed-off-by: Krzysztof Hałasa <khalasa@piap.pl>
> ---
> This version drops CONFIG_NEED_PCIE_MAX_MRRS.
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80fc98acf097..225380e75fff 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1148,6 +1148,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>                 imx6_pcie->vph = NULL;
>         }
>
> +       max_pcie_mrrs = 512;
>         platform_set_drvdata(pdev, imx6_pcie);
>
>         ret = imx6_pcie_attach_pd(dev);
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index aacf575c15cf..44815af4ad85 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -112,6 +112,10 @@ enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_PEER2PEER;
>  enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_DEFAULT;
>  #endif
>
> +#ifdef CONFIG_ARM
> +u16 max_pcie_mrrs = 4096; // no limit - needed mostly for DWC PCIe
> +#endif
> +
>  /*
>   * The default CLS is used if arch didn't set CLS explicitly and not
>   * all pci devices agree on the same value.  Arch can override either
> @@ -5816,6 +5820,11 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
>                         rq = mps;
>         }
>
> +#ifdef CONFIG_ARM
> +       if (rq > max_pcie_mrrs)
> +               rq = max_pcie_mrrs;
> +#endif

My objection wasn't having another kconfig option so much as I don't
think we need one at all here unless Bjorn feels otherwise. It's 2
bytes of data and about 3 instructions (load, cmp, store).

If we do have a config option, using or basing on the arch is wrong.
Has nothing to do with the arch. Are the other platforms needing this
arm32 as well?

Also, when you do use kconfig options, use IS_ENABLED() whenever possible.

Rob

  reply	other threads:[~2021-08-16 19:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-16 11:27 Krzysztof Hałasa
2021-08-16 19:34 ` Rob Herring [this message]
2021-08-17  4:29   ` Krzysztof Hałasa

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=CAL_Jsq+M7xdqf8bVhs-isHoGCGjLhi6N2q+tm7msWLBy52OsMw@mail.gmail.com \
    --to=robh@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=chenhuacai@gmail.com \
    --cc=email2tema@gmail.com \
    --cc=hongxing.zhu@nxp.com \
    --cc=khalasa@piap.pl \
    --cc=kw@linux.com \
    --cc=l.stach@pengutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=narmstrong@baylibre.com \
    --subject='Re: [PATCH v2] PCIe: limit Max Read Request Size on i.MX to 512 bytes' \
    /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).