[PATCH] Set PCIE maxpayload for card during hotplug insertion

[PATCH] Set PCIE maxpayload for card during hotplug insertion

[Jordan Hargrave]

Reference: http://marc.info/?l=linux-hotplug&m\x128932324625063&w=2

The following patch sets the MaxPayload setting to match the parent reading when inserting
a PCIE card into a hotplug slot.  On our system, the upstream bridge is set to 256, but when
inserting a card, the card setting defaults to 128.  As soon as I/O is performed to the card
it starts receiving errors since the payload size is too small.

Signed-off-by: Jordan_Hargrave@dell.com

---
 drivers/pci/hotplug/pcihp_slot.c |   45 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/hotplug/pcihp_slot.c b/drivers/pci/hotplug/pcihp_slot.c
index 80b461c..912d55b 100644
--- a/drivers/pci/hotplug/pcihp_slot.c
+++ b/drivers/pci/hotplug/pcihp_slot.c
@@ -158,6 +158,47 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
+/* Program PCIE MaxPayload setting on device: ensure parent maxpayload <= device */
+static int pci_set_payload(struct pci_dev *dev)
+{
+	int pos, ppos;
+	u16 pctl, psz;
+	u16 dctl, dsz, dcap, dmax;
+	struct pci_dev *parent;
+
+	parent = dev->bus->self;
+	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
+	if (!pos)
+		return 0;
+
+	/* Read Device MaxPayload capability and setting */
+	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &dctl);
+	pci_read_config_word(dev, pos + PCI_EXP_DEVCAP, &dcap);
+	dsz = (dctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5;
+	dmax = (dcap & PCI_EXP_DEVCAP_PAYLOAD);
+
+	/* Read Parent MaxPayload setting */
+	ppos = pci_find_capability(parent, PCI_CAP_ID_EXP);
+	if (!ppos)
+		return 0;
+	pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL, &pctl);
+	psz = (pctl &  PCI_EXP_DEVCTL_PAYLOAD) >> 5;
+
+	/* If parent payload > device max payload -> error
+	 * If parent payload > device payload -> set speed
+	 * If parent payload <= device payload -> do nothing
+	 */
+	if (psz > dmax)
+		return -1;
+	else if (psz > dsz) {
+		dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128 << psz);
+		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, 
+				      (dctl & ~PCI_EXP_DEVCTL_PAYLOAD) + 
+				      (psz << 5)); 
+	}
+	return 0;
+}
+
 void pci_configure_slot(struct pci_dev *dev)
 {
 	struct pci_dev *cdev;
@@ -169,6 +210,10 @@ void pci_configure_slot(struct pci_dev *dev)
 			(dev->class >> 8) = PCI_CLASS_BRIDGE_PCI)))
 		return;
 
+	ret = pci_set_payload(dev);
+	if (ret)
+		dev_warn(&dev->dev, "could not set device max payload\n");
+
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
 	if (ret)
-- 
1.7.3.4

Re: [PATCH] Set PCIE maxpayload for card during hotplug insertion

[Kenji Kaneshige]

(2011/03/25 7:21), Jordan Hargrave wrote:
> Reference: http://marc.info/?l=linux-hotplug&m\x128932324625063&w=2
>
> The following patch sets the MaxPayload setting to match the parent reading when inserting
> a PCIE card into a hotplug slot.  On our system, the upstream bridge is set to 256, but when
> inserting a card, the card setting defaults to 128.  As soon as I/O is performed to the card
> it starts receiving errors since the payload size is too small.
>
> Signed-off-by: Jordan_Hargrave@dell.com
>
> ---
>   drivers/pci/hotplug/pcihp_slot.c |   45 ++++++++++++++++++++++++++++++++++++++
>   1 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/hotplug/pcihp_slot.c b/drivers/pci/hotplug/pcihp_slot.c
> index 80b461c..912d55b 100644
> --- a/drivers/pci/hotplug/pcihp_slot.c
> +++ b/drivers/pci/hotplug/pcihp_slot.c
> @@ -158,6 +158,47 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
>   	 */
>   }
>
> +/* Program PCIE MaxPayload setting on device: ensure parent maxpayload<= device */
> +static int pci_set_payload(struct pci_dev *dev)
> +{
> +	int pos, ppos;
> +	u16 pctl, psz;
> +	u16 dctl, dsz, dcap, dmax;
> +	struct pci_dev *parent;
> +
> +	parent = dev->bus->self;
> +	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> +	if (!pos)
> +		return 0;

You can use pci_pcie_cap(), which just returns dev->pcie_cap, instead of pci_find_capability().

> +
> +	/* Read Device MaxPayload capability and setting */
> +	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&dctl);
> +	pci_read_config_word(dev, pos + PCI_EXP_DEVCAP,&dcap);
> +	dsz = (dctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> +	dmax = (dcap&  PCI_EXP_DEVCAP_PAYLOAD);
> +
> +	/* Read Parent MaxPayload setting */
> +	ppos = pci_find_capability(parent, PCI_CAP_ID_EXP);
> +	if (!ppos)
> +		return 0;

Ditto.

> +	pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL,&pctl);
> +	psz = (pctl&   PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> +
> +	/* If parent payload>  device max payload ->  error
> +	 * If parent payload>  device payload ->  set speed
> +	 * If parent payload<= device payload ->  do nothing
> +	 */
> +	if (psz>  dmax)
> +		return -1;
> +	else if (psz>  dsz) {

Maybe just "if" (not "else if") here is easier to read.

> +		dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128<<  psz);
> +		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL,
> +				      (dctl&  ~PCI_EXP_DEVCTL_PAYLOAD) +
> +				      (psz<<  5));

What about

	dctrl = (dctrl & ~PCI_EXP_DEVCTL_PAYLOAD) | (psz << 5);
	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, dctrl);

?

Regards,
Kenji Kaneshige

RE: [PATCH] Set PCIE maxpayload for card during hotplug insertion

[Jordan_Hargrave]

> -----Original Message-----
> From: Kenji Kaneshige [mailto:kaneshige.kenji@jp.fujitsu.com]
> Sent: Thursday, March 24, 2011 8:07 PM
> To: Hargrave, Jordan
> Cc: linux-hotplug@vger.kernel.org; linux-pci@vger.kernel.org;
> jbarnes@virtuousgeek.org
> Subject: Re: [PATCH] Set PCIE maxpayload for card during hotplug
> insertion
> 
> (2011/03/25 7:21), Jordan Hargrave wrote:
> > Reference: http://marc.info/?l=linux-hotplug&m\x128932324625063&w=2
> >
> > The following patch sets the MaxPayload setting to match the parent
> reading when inserting
> > a PCIE card into a hotplug slot.  On our system, the upstream bridge
> is set to 256, but when
> > inserting a card, the card setting defaults to 128.  As soon as I/O
> is performed to the card
> > it starts receiving errors since the payload size is too small.
> >
> > Signed-off-by: Jordan_Hargrave@dell.com
> >
> > ---
> >   drivers/pci/hotplug/pcihp_slot.c |   45
> ++++++++++++++++++++++++++++++++++++++
> >   1 files changed, 45 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/pci/hotplug/pcihp_slot.c
> b/drivers/pci/hotplug/pcihp_slot.c
> > index 80b461c..912d55b 100644
> > --- a/drivers/pci/hotplug/pcihp_slot.c
> > +++ b/drivers/pci/hotplug/pcihp_slot.c
> > @@ -158,6 +158,47 @@ static void program_hpp_type2(struct pci_dev
> *dev, struct hpp_type2 *hpp)
> >   	 */
> >   }
> >
> > +/* Program PCIE MaxPayload setting on device: ensure parent
> maxpayload<= device */
> > +static int pci_set_payload(struct pci_dev *dev)
> > +{
> > +	int pos, ppos;
> > +	u16 pctl, psz;
> > +	u16 dctl, dsz, dcap, dmax;
> > +	struct pci_dev *parent;
> > +
> > +	parent = dev->bus->self;
> > +	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> > +	if (!pos)
> > +		return 0;
> 
> You can use pci_pcie_cap(), which just returns dev->pcie_cap, instead
> of pci_find_capability().
> 
> > +
> > +	/* Read Device MaxPayload capability and setting */
> > +	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&dctl);
> > +	pci_read_config_word(dev, pos + PCI_EXP_DEVCAP,&dcap);
> > +	dsz = (dctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> > +	dmax = (dcap&  PCI_EXP_DEVCAP_PAYLOAD);
> > +
> > +	/* Read Parent MaxPayload setting */
> > +	ppos = pci_find_capability(parent, PCI_CAP_ID_EXP);
> > +	if (!ppos)
> > +		return 0;
> 
> Ditto.
> 
> > +	pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL,&pctl);
> > +	psz = (pctl&   PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> > +
> > +	/* If parent payload>  device max payload ->  error
> > +	 * If parent payload>  device payload ->  set speed
> > +	 * If parent payload<= device payload ->  do nothing
> > +	 */
> > +	if (psz>  dmax)
> > +		return -1;
> > +	else if (psz>  dsz) {
> 
> Maybe just "if" (not "else if") here is easier to read.
> 
> > +		dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128<<
> psz);
> > +		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL,
> > +				      (dctl&  ~PCI_EXP_DEVCTL_PAYLOAD) +
> > +				      (psz<<  5));
> 
> What about
> 
> 	dctrl = (dctrl & ~PCI_EXP_DEVCTL_PAYLOAD) | (psz << 5);
> 	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, dctrl);
> 
> ?
> 
> Regards,
> Kenji Kaneshige

Signed-off-by: Jordan Hargrave <Jordan_Hargrave@dell.com>
---
 drivers/pci/hotplug/pcihp_slot.c |   44 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/hotplug/pcihp_slot.c b/drivers/pci/hotplug/pcihp_slot.c
index 80b461c..3b628c5 100644
--- a/drivers/pci/hotplug/pcihp_slot.c
+++ b/drivers/pci/hotplug/pcihp_slot.c
@@ -158,6 +158,46 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
+/* Program PCIE MaxPayload setting on device: ensure parent maxpayload 
+<= device */ static int pci_set_payload(struct pci_dev *dev) {
+	int pos, ppos;
+	u16 pctl, psz;
+	u16 dctl, dsz, dcap, dmax;
+	struct pci_dev *parent;
+
+	parent = dev->bus->self;
+	pos = pci_pcie_cap(dev);
+	if (!pos)
+		return 0;
+
+	/* Read Device MaxPayload capability and setting */
+	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &dctl);
+	pci_read_config_word(dev, pos + PCI_EXP_DEVCAP, &dcap);
+	dsz = (dctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5;
+	dmax = (dcap & PCI_EXP_DEVCAP_PAYLOAD);
+
+	/* Read Parent MaxPayload setting */
+	ppos = pci_pcie_cap(parent);
+	if (!ppos)
+		return 0;
+	pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL, &pctl);
+	psz = (pctl &  PCI_EXP_DEVCTL_PAYLOAD) >> 5;
+
+	/* If parent payload > device max payload -> error
+	 * If parent payload > device payload -> set speed
+	 * If parent payload <= device payload -> do nothing
+	 */
+	if (psz > dmax)
+		return -1;
+	if (psz > dsz) {
+		dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128 << psz);
+		dctl = (dctl & ~PCI_EXP_DEVCTL_PAYLOAD) + (psz << 5);
+		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, dctl);
+	}
+	return 0;
+}
+
 void pci_configure_slot(struct pci_dev *dev)  {
 	struct pci_dev *cdev;
@@ -169,6 +209,10 @@ void pci_configure_slot(struct pci_dev *dev)
 			(dev->class >> 8) = PCI_CLASS_BRIDGE_PCI)))
 		return;
 
+	ret = pci_set_payload(dev);
+	if (ret)
+		dev_warn(&dev->dev, "could not set device max payload\n");
+
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
 	if (ret)
--
1.7.4.1

RE: [PATCH] Set PCIE maxpayload for card during hotplug insertion

[Jordan_Hargrave]

Are there any further comments on this diff?

Would it be better to have a separate pcie_get_maxpayload/pcie_set_maxpayload function? 

What about cards that fail the 'if (psz > dmax)' test?  Technically the code should walk up to the root complex and set the speed there to the least common denominator speed of all children.  However if that happens, the performance of other cards/PCI devices in the system maybe adversely affected.  Any ideas on how to implement this or if this is desired?

--jordan hargrave
Dell Enterprise Linux Engineering

> -----Original Message-----
> From: Hargrave, Jordan
> Sent: Monday, March 28, 2011 2:53 AM
> Cc: 'linux-hotplug@vger.kernel.org'; 'linux-pci@vger.kernel.org'
> Subject: RE: [PATCH] Set PCIE maxpayload for card during hotplug
> insertion
> 
> > -----Original Message-----
> > From: Kenji Kaneshige [mailto:kaneshige.kenji@jp.fujitsu.com]
> > Sent: Thursday, March 24, 2011 8:07 PM
> > To: Hargrave, Jordan
> > Cc: linux-hotplug@vger.kernel.org; linux-pci@vger.kernel.org;
> > jbarnes@virtuousgeek.org
> > Subject: Re: [PATCH] Set PCIE maxpayload for card during hotplug
> > insertion
> >
> > (2011/03/25 7:21), Jordan Hargrave wrote:
> > > Reference: http://marc.info/?l=linux-hotplug&m\x128932324625063&w=2
> > >
> > > The following patch sets the MaxPayload setting to match the parent
> > reading when inserting
> > > a PCIE card into a hotplug slot.  On our system, the upstream
> bridge
> > is set to 256, but when
> > > inserting a card, the card setting defaults to 128.  As soon as I/O
> > is performed to the card
> > > it starts receiving errors since the payload size is too small.
> > >
> > > Signed-off-by: Jordan_Hargrave@dell.com
> > >
> > > ---
> > >   drivers/pci/hotplug/pcihp_slot.c |   45
> > ++++++++++++++++++++++++++++++++++++++
> > >   1 files changed, 45 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/pci/hotplug/pcihp_slot.c
> > b/drivers/pci/hotplug/pcihp_slot.c
> > > index 80b461c..912d55b 100644
> > > --- a/drivers/pci/hotplug/pcihp_slot.c
> > > +++ b/drivers/pci/hotplug/pcihp_slot.c
> > > @@ -158,6 +158,47 @@ static void program_hpp_type2(struct pci_dev
> > *dev, struct hpp_type2 *hpp)
> > >   	 */
> > >   }
> > >
> > > +/* Program PCIE MaxPayload setting on device: ensure parent
> > maxpayload<= device */
> > > +static int pci_set_payload(struct pci_dev *dev)
> > > +{
> > > +	int pos, ppos;
> > > +	u16 pctl, psz;
> > > +	u16 dctl, dsz, dcap, dmax;
> > > +	struct pci_dev *parent;
> > > +
> > > +	parent = dev->bus->self;
> > > +	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> > > +	if (!pos)
> > > +		return 0;
> >
> > You can use pci_pcie_cap(), which just returns dev->pcie_cap, instead
> > of pci_find_capability().
> >
> > > +
> > > +	/* Read Device MaxPayload capability and setting */
> > > +	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&dctl);
> > > +	pci_read_config_word(dev, pos + PCI_EXP_DEVCAP,&dcap);
> > > +	dsz = (dctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> > > +	dmax = (dcap&  PCI_EXP_DEVCAP_PAYLOAD);
> > > +
> > > +	/* Read Parent MaxPayload setting */
> > > +	ppos = pci_find_capability(parent, PCI_CAP_ID_EXP);
> > > +	if (!ppos)
> > > +		return 0;
> >
> > Ditto.
> >
> > > +	pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL,&pctl);
> > > +	psz = (pctl&   PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> > > +
> > > +	/* If parent payload>  device max payload ->  error
> > > +	 * If parent payload>  device payload ->  set speed
> > > +	 * If parent payload<= device payload ->  do nothing
> > > +	 */
> > > +	if (psz>  dmax)
> > > +		return -1;
> > > +	else if (psz>  dsz) {
> >
> > Maybe just "if" (not "else if") here is easier to read.
> >
> > > +		dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128<<
> > psz);
> > > +		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL,
> > > +				      (dctl&  ~PCI_EXP_DEVCTL_PAYLOAD) +
> > > +				      (psz<<  5));
> >
> > What about
> >
> > 	dctrl = (dctrl & ~PCI_EXP_DEVCTL_PAYLOAD) | (psz << 5);
> > 	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, dctrl);
> >
> > ?
> >
> > Regards,
> > Kenji Kaneshige

Re: [PATCH] Set PCIE maxpayload for card during hotplug insertion

[Jesse Barnes]

Kenji-san, does the most recent version look ok?  An added tested-by to
confirm the payload size is set correctly would be a bonus.

Thanks,
Jesse

On Tue, 29 Mar 2011 16:42:25 -0500
<Jordan_Hargrave@Dell.com> wrote:

> Are there any further comments on this diff?
> 
> Would it be better to have a separate pcie_get_maxpayload/pcie_set_maxpayload function? 
> 
> What about cards that fail the 'if (psz > dmax)' test?  Technically the code should walk up to the root complex and set the speed there to the least common denominator speed of all children.  However if that happens, the performance of other cards/PCI devices in the system maybe adversely affected.  Any ideas on how to implement this or if this is desired?
> 
> --jordan hargrave
> Dell Enterprise Linux Engineering
> 
> > -----Original Message-----
> > From: Hargrave, Jordan
> > Sent: Monday, March 28, 2011 2:53 AM
> > Cc: 'linux-hotplug@vger.kernel.org'; 'linux-pci@vger.kernel.org'
> > Subject: RE: [PATCH] Set PCIE maxpayload for card during hotplug
> > insertion
> > 
> > > -----Original Message-----
> > > From: Kenji Kaneshige [mailto:kaneshige.kenji@jp.fujitsu.com]
> > > Sent: Thursday, March 24, 2011 8:07 PM
> > > To: Hargrave, Jordan
> > > Cc: linux-hotplug@vger.kernel.org; linux-pci@vger.kernel.org;
> > > jbarnes@virtuousgeek.org
> > > Subject: Re: [PATCH] Set PCIE maxpayload for card during hotplug
> > > insertion
> > >
> > > (2011/03/25 7:21), Jordan Hargrave wrote:
> > > > Reference: http://marc.info/?l=linux-hotplug&m\x128932324625063&w=2
> > > >
> > > > The following patch sets the MaxPayload setting to match the parent
> > > reading when inserting
> > > > a PCIE card into a hotplug slot.  On our system, the upstream
> > bridge
> > > is set to 256, but when
> > > > inserting a card, the card setting defaults to 128.  As soon as I/O
> > > is performed to the card
> > > > it starts receiving errors since the payload size is too small.
> > > >
> > > > Signed-off-by: Jordan_Hargrave@dell.com
> > > >
> > > > ---
> > > >   drivers/pci/hotplug/pcihp_slot.c |   45
> > > ++++++++++++++++++++++++++++++++++++++
> > > >   1 files changed, 45 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/hotplug/pcihp_slot.c
> > > b/drivers/pci/hotplug/pcihp_slot.c
> > > > index 80b461c..912d55b 100644
> > > > --- a/drivers/pci/hotplug/pcihp_slot.c
> > > > +++ b/drivers/pci/hotplug/pcihp_slot.c
> > > > @@ -158,6 +158,47 @@ static void program_hpp_type2(struct pci_dev
> > > *dev, struct hpp_type2 *hpp)
> > > >   	 */
> > > >   }
> > > >
> > > > +/* Program PCIE MaxPayload setting on device: ensure parent
> > > maxpayload<= device */
> > > > +static int pci_set_payload(struct pci_dev *dev)
> > > > +{
> > > > +	int pos, ppos;
> > > > +	u16 pctl, psz;
> > > > +	u16 dctl, dsz, dcap, dmax;
> > > > +	struct pci_dev *parent;
> > > > +
> > > > +	parent = dev->bus->self;
> > > > +	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> > > > +	if (!pos)
> > > > +		return 0;
> > >
> > > You can use pci_pcie_cap(), which just returns dev->pcie_cap, instead
> > > of pci_find_capability().
> > >
> > > > +
> > > > +	/* Read Device MaxPayload capability and setting */
> > > > +	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&dctl);
> > > > +	pci_read_config_word(dev, pos + PCI_EXP_DEVCAP,&dcap);
> > > > +	dsz = (dctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> > > > +	dmax = (dcap&  PCI_EXP_DEVCAP_PAYLOAD);
> > > > +
> > > > +	/* Read Parent MaxPayload setting */
> > > > +	ppos = pci_find_capability(parent, PCI_CAP_ID_EXP);
> > > > +	if (!ppos)
> > > > +		return 0;
> > >
> > > Ditto.
> > >
> > > > +	pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL,&pctl);
> > > > +	psz = (pctl&   PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> > > > +
> > > > +	/* If parent payload>  device max payload ->  error
> > > > +	 * If parent payload>  device payload ->  set speed
> > > > +	 * If parent payload<= device payload ->  do nothing
> > > > +	 */
> > > > +	if (psz>  dmax)
> > > > +		return -1;
> > > > +	else if (psz>  dsz) {
> > >
> > > Maybe just "if" (not "else if") here is easier to read.
> > >
> > > > +		dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128<<
> > > psz);
> > > > +		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL,
> > > > +				      (dctl&  ~PCI_EXP_DEVCTL_PAYLOAD) +
> > > > +				      (psz<<  5));
> > >
> > > What about
> > >
> > > 	dctrl = (dctrl & ~PCI_EXP_DEVCTL_PAYLOAD) | (psz << 5);
> > > 	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, dctrl);
> > >
> > > ?
> > >
> > > Regards,
> > > Kenji Kaneshige
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Jesse Barnes, Intel Open Source Technology Center

RE: [PATCH] Set PCIE maxpayload for card during hotplug insertion

[Jordan_Hargrave]

I have tested and verified that the patch works.
Tested-by: jordan_hargrave@dell.com

--jordan hargrave
Dell Enterprise Linux Engineering

> -----Original Message-----
> From: Hargrave, Jordan
> Sent: Tuesday, March 29, 2011 4:43 PM
> To: Iyer, Shyam; Hargrave, Jordan
> Subject: [PATCH] Set PCIE maxpayload for card during hotplug insertion
> 
> Signed-off-by: Jordan Hargrave <Jordan_Hargrave@dell.com>
> ---
>  drivers/pci/hotplug/pcihp_slot.c |   44
> ++++++++++++++++++++++++++++++++++++++
>  1 files changed, 44 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pcihp_slot.c
> b/drivers/pci/hotplug/pcihp_slot.c
> index 80b461c..3b628c5 100644
> --- a/drivers/pci/hotplug/pcihp_slot.c
> +++ b/drivers/pci/hotplug/pcihp_slot.c
> @@ -158,6 +158,46 @@ static void program_hpp_type2(struct pci_dev *dev,
> struct hpp_type2 *hpp)
>  	 */
>  }
> 
> +/* Program PCIE MaxPayload setting on device: ensure parent maxpayload
> <= device */
> +static int pci_set_payload(struct pci_dev *dev)
> +{
> +	int pos, ppos;
> +	u16 pctl, psz;
> +	u16 dctl, dsz, dcap, dmax;
> +	struct pci_dev *parent;
> +
> +	parent = dev->bus->self;
> +	pos = pci_pcie_cap(dev);
> +	if (!pos)
> +		return 0;
> +
> +	/* Read Device MaxPayload capability and setting */
> +	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &dctl);
> +	pci_read_config_word(dev, pos + PCI_EXP_DEVCAP, &dcap);
> +	dsz = (dctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5;
> +	dmax = (dcap & PCI_EXP_DEVCAP_PAYLOAD);
> +
> +	/* Read Parent MaxPayload setting */
> +	ppos = pci_pcie_cap(parent);
> +	if (!ppos)
> +		return 0;
> +	pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL, &pctl);
> +	psz = (pctl &  PCI_EXP_DEVCTL_PAYLOAD) >> 5;
> +
> +	/* If parent payload > device max payload -> error
> +	 * If parent payload > device payload -> set speed
> +	 * If parent payload <= device payload -> do nothing
> +	 */
> +	if (psz > dmax)
> +		return -1;
> +	if (psz > dsz) {
> +		dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128 <<
> psz);
> +		dctl = (dctl & ~PCI_EXP_DEVCTL_PAYLOAD) + (psz << 5);
> +		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, dctl);
> +	}
> +	return 0;
> +}
> +
>  void pci_configure_slot(struct pci_dev *dev)
>  {
>  	struct pci_dev *cdev;
> @@ -169,6 +209,10 @@ void pci_configure_slot(struct pci_dev *dev)
>  			(dev->class >> 8) = PCI_CLASS_BRIDGE_PCI)))
>  		return;
> 
> +	ret = pci_set_payload(dev);
> +	if (ret)
> +		dev_warn(&dev->dev, "could not set device max payload\n");
> +
>  	memset(&hpp, 0, sizeof(hpp));
>  	ret = pci_get_hp_params(dev, &hpp);
>  	if (ret)
> --
> 1.7.4.1

Re: [PATCH] Set PCIE maxpayload for card during hotplug insertion

[Jesse Barnes]

On Wed, 4 May 2011 13:42:44 -0500
<Jordan_Hargrave@Dell.com> wrote:

> I have tested and verified that the patch works.
> Tested-by: jordan_hargrave@dell.com
> 

Great, thanks.  Can you re-send against my linux-next branch?  The last
patch seems to have been corrupted.

Kenji-san, can I add your reviewed-by?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] Set PCIE maxpayload for card during hotplug insertion

[Kenji Kaneshige]

(2011/05/10 4:18), Jesse Barnes wrote:
> On Wed, 4 May 2011 13:42:44 -0500
> <Jordan_Hargrave@Dell.com>  wrote:
> 
>> I have tested and verified that the patch works.
>> Tested-by: jordan_hargrave@dell.com
>>
> 
> Great, thanks.  Can you re-send against my linux-next branch?  The last
> patch seems to have been corrupted.
> 
> Kenji-san, can I add your reviewed-by?

Sure.

Thanks
Kenji Kaneshige