public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: m-karicheri2@ti•com (Murali Karicheri)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2] PCI: keystone: add a pci quirk to limit mrrs
Date: Fri, 5 Sep 2014 16:30:12 -0400	[thread overview]
Message-ID: <540A1D54.9030103@ti.com> (raw)
In-Reply-To: <20140905192329.GE8080@google.com>

On 09/05/2014 03:23 PM, Bjorn Helgaas wrote:
> On Fri, Aug 08, 2014 at 03:45:32PM -0400, Murali Karicheri wrote:
>> Keystone PCI controller has a limitation that memory read request
>> size must not exceed 256 bytes. This is a hardware limitation and
>> add a quirk to force this limit on all downstream devices by
>> updating mrrs.
>>
>> Signed-off-by: Murali Karicheri<m-karicheri2@ti•com>
>> ---
>>   -v2: made the quirk happens after tuning
>>   -v1: changed printk to indicate PCI bdf
>>   This applies on top of the Keystone PCI controller patch series
>>   at http://thread.gmane.org/gmane.linux.kernel.pci/33523
>>   drivers/pci/host/pci-keystone.c |   40 +++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 40 insertions(+)
>>
>> diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
>> index c1cfaef..a132622 100644
>> --- a/drivers/pci/host/pci-keystone.c
>> +++ b/drivers/pci/host/pci-keystone.c
>> @@ -42,8 +42,48 @@
>>   /* DEV_STAT_CTRL */
>>   #define PCIE_CAP_BASE		0x70
>>
>> +/* PCIE controller device IDs */
>> +#define PCIE_RC_K2HK		0xb008
>> +#define PCIE_RC_K2E		0xb009
>> +#define PCIE_RC_K2L		0xb00a
>
> A Root Complex doesn't appear in config space as a PCI device, so I think
> these are actually Root Port devices, aren't they?

Yes.

>
> I suppose the spec would allow Root Complex Integrated Endpoints, on which
> MRRS could also be set, but I think your quirk ignores them because they
> would appear on the root bus.  Is ignoring them OK, or would you want to
> set MRRS for those as well?

How does the Root Complex integrated endpoints are designed? Our PCI can 
act as a RC or EP and not both at the same time. So I guess this is not 
applicable.

>
>> +
>>   #define to_keystone_pcie(x)	container_of(x, struct keystone_pcie, pp)
>>
>> +static void quirk_limit_mrrs(struct pci_dev *dev)
>> +{
>> +	struct pci_bus *bus = dev->bus;
>> +	struct pci_dev *bridge = bus->self;
>> +
>> +	if (pci_is_root_bus(bus))
>> +		return;
>> +
>> +	/* look for the host bridge */
>> +	while (!pci_is_root_bus(bus)) {
>> +		bridge = bus->self;
>> +		bus = bus->parent;
>> +	}
>> +
>> +	if (bridge) {
>> +		u16 id;
>> +
>> +		/*
>> +		 * Keystone PCI controller has a h/w limitation of
>> +		 * 256 bytes maximum read request size. It can't handle
>> +		 * anything higher than this. So force this limit on
>> +		 * all downstream devices
>> +		 */
>> +		pci_read_config_word(bridge, PCI_DEVICE_ID,&id);
>
> I think you should check the Vendor ID here as well.  Otherwise, there's a
> possibility of applying this quirk to devices under a bridge with the same
> Device ID but different Vendor ID.
>
> And I think you should be able to use the bridge->vendor and bridge->device
> fields instead of reading them from config space.  Actually, this might be
> a good place to use pci_match_id().

Ok.

>
>> +		if ((id == PCIE_RC_K2HK) || (id == PCIE_RC_K2E) ||
>> +		    (id == PCIE_RC_K2L)) {
>> +			if (pcie_get_readrq(dev)>  256) {
>> +				dev_info(&dev->dev, "limiting mrrs to 256\n");
>> +				pcie_set_readrq(dev, 256);
>> +			}
>> +		}
>> +	}
>> +}
>> +DECLARE_PCI_FIXUP_ENABLE(PCI_ANY_ID, PCI_ANY_ID, quirk_limit_mrrs);
>> +
>>   static int ks_pcie_establish_link(struct keystone_pcie *ks_pcie)
>>   {
>>   	struct pcie_port *pp =&ks_pcie->pp;
>> --
>> 1.7.9.5
>>

  reply	other threads:[~2014-09-05 20:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-08 19:45 [PATCH v2] PCI: keystone: add a pci quirk to limit mrrs Murali Karicheri
2014-09-05 19:23 ` Bjorn Helgaas
2014-09-05 20:30   ` Murali Karicheri [this message]
2014-09-05 20:40     ` Bjorn Helgaas
2014-09-05 20:43       ` Murali Karicheri

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=540A1D54.9030103@ti.com \
    --to=m-karicheri2@ti$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox