[PATCH 0/3] PCI VPD changes

[PATCH 0/3] PCI VPD changes

[Stephen Hemminger]

Slightly revised from my last set. Allow mutex to be killed, and
just use yield.

-- 

[PATCH 0/4] PCI VPD changes

[Stephen Hemminger]

Update sky2 to use new VPD access routines.
The patches are against netdev-2.6 upstream-next branch,
and assume PCI changes submitted already.

-- 

[PATCH 1/4] sky2: set VPD size

[Stephen Hemminger]

[-- Attachment #1: sky2-vpd-size.patch --]
[-- Type: text/plain, Size: 2069 bytes --]

Read configuration register during probe and use it to size the
available VPD. Move existing code using same register slightly
earlier in probe handling.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/sky2.c	2008-09-04 11:27:34.000000000 -0700
+++ b/drivers/net/sky2.c	2008-09-04 11:44:49.000000000 -0700
@@ -4281,6 +4281,7 @@ static int __devinit sky2_probe(struct p
 	struct net_device *dev;
 	struct sky2_hw *hw;
 	int err, using_dac = 0, wol_default;
+	u32 reg;
 	char buf1[16];
 
 	err = pci_enable_device(pdev);
@@ -4314,6 +4315,34 @@ static int __devinit sky2_probe(struct p
 		}
 	}
 
+	/* Get configuration information
+	 * Note: only regular PCI config access once to test for HW issues
+	 *       other PCI access through shared memory for speed and to
+	 *	 avoid MMCONFIG problems.
+	 */
+	err = pci_read_config_dword(pdev, PCI_DEV_REG2, &reg);
+	if (err) {
+		dev_err(&pdev->dev, "PCI read config failed\n");
+		goto err_out_free_regions;
+	}
+
+	/* size of available VPD, only impact sysfs */
+	err = pci_vpd_size(pdev, 1ul << (((reg & PCI_VPD_ROM_SZ) >> 14) + 8));
+	if (err)
+		dev_warn(&pdev->dev, "Can't set VPD size\n");
+
+#ifdef __BIG_ENDIAN
+	/* The sk98lin vendor driver uses hardware byte swapping but
+	 * this driver uses software swapping.
+	 */
+	reg &= ~PCI_REV_DESC;
+	err = pci_write_config_dword(pdev,PCI_DEV_REG2, reg);
+	if (err) {
+		dev_err(&pdev->dev, "PCI write config failed\n");
+		goto err_out_free_regions;
+	}
+#endif
+
 	wol_default = pci_wake_enabled(pdev) ? WAKE_MAGIC : 0;
 
 	err = -ENOMEM;
@@ -4331,18 +4360,6 @@ static int __devinit sky2_probe(struct p
 		goto err_out_free_hw;
 	}
 
-#ifdef __BIG_ENDIAN
-	/* The sk98lin vendor driver uses hardware byte swapping but
-	 * this driver uses software swapping.
-	 */
-	{
-		u32 reg;
-		reg = sky2_pci_read32(hw, PCI_DEV_REG2);
-		reg &= ~PCI_REV_DESC;
-		sky2_pci_write32(hw, PCI_DEV_REG2, reg);
-	}
-#endif
-
 	/* ring for status responses */
 	hw->st_le = pci_alloc_consistent(pdev, STATUS_LE_BYTES, &hw->st_dma);
 	if (!hw->st_le)

-- 

[PATCH 2/4] sky2: add more generic ethtool hooks

[Stephen Hemminger]

[-- Attachment #1: sky2-ethtool-ops.patch --]
[-- Type: text/plain, Size: 889 bytes --]

Add support for some missing status queries.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/sky2.c	2008-09-04 11:48:30.000000000 -0700
+++ b/drivers/net/sky2.c	2008-09-04 11:48:31.000000000 -0700
@@ -3840,7 +3840,9 @@ static const struct ethtool_ops sky2_eth
 	.get_eeprom_len	= sky2_get_eeprom_len,
 	.get_eeprom	= sky2_get_eeprom,
 	.set_eeprom	= sky2_set_eeprom,
+	.get_sg		= ethtool_op_get_sg,
 	.set_sg 	= ethtool_op_set_sg,
+	.get_tx_csum	= ethtool_op_get_tx_csum,
 	.set_tx_csum	= sky2_set_tx_csum,
 	.set_tso	= sky2_set_tso,
 	.get_rx_csum	= sky2_get_rx_csum,
@@ -3855,6 +3857,8 @@ static const struct ethtool_ops sky2_eth
 	.phys_id	= sky2_phys_id,
 	.get_sset_count = sky2_get_sset_count,
 	.get_ethtool_stats = sky2_get_ethtool_stats,
+	.get_flags      = ethtool_op_get_flags,
+	.get_ufo	= ethtool_op_get_ufo,
 };
 
 #ifdef CONFIG_SKY2_DEBUG

-- 

[PATCH 3/4] sky2: move VPD display into debug interface

[Stephen Hemminger]

[-- Attachment #1: sky2-vpd-debug.patch --]
[-- Type: text/plain, Size: 4504 bytes --]

The VPD stuff has more data and isn't generally that useful, so move
it into the existing debugfs display and use the new PCI VPD
accessor routines.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/net/sky2.c	2008-09-04 13:47:04.000000000 -0700
+++ b/drivers/net/sky2.c	2008-09-04 13:47:24.000000000 -0700
@@ -3865,6 +3865,86 @@ static const struct ethtool_ops sky2_eth
 
 static struct dentry *sky2_debug;
 
+
+/*
+ * Read and parse the first part of Vital Product Data
+ */
+#define VPD_SIZE	128
+#define VPD_MAGIC	0x82
+
+static const struct vpd_tag {
+	char tag[2];
+	char *label;
+} vpd_tags[] = {
+	{ "PN",	"Part Number" },
+	{ "EC", "Engineering Level" },
+	{ "MN", "Manufacturer" },
+	{ "SN", "Serial Number" },
+	{ "YA", "Asset Tag" },
+	{ "VL", "First Error Log Message" },
+	{ "VF", "Second Error Log Message" },
+	{ "VB", "Boot Agent ROM Configuration" },
+	{ "VE", "EFI UNDI Configuration" },
+};
+
+static void sky2_show_vpd(struct seq_file *seq, struct sky2_hw *hw)
+{
+	size_t vpd_size;
+	loff_t offs;
+	u8 len;
+	unsigned char *buf;
+	u16 reg2;
+
+	reg2 = sky2_pci_read16(hw, PCI_DEV_REG2);
+	vpd_size = 1 << ( ((reg2 & PCI_VPD_ROM_SZ) >> 14) + 8);
+
+	seq_printf(seq, "%s Product Data\n", pci_name(hw->pdev));
+	buf = kmalloc(vpd_size, GFP_KERNEL);
+	if (!buf) {
+		seq_puts(seq, "no memory!\n");
+		return;
+	}
+
+	if (pci_read_vpd(hw->pdev, 0, vpd_size, buf) < 0) {
+		seq_puts(seq, "VPD read failed\n");
+		goto out;
+	}
+
+	if (buf[0] != VPD_MAGIC) {
+		seq_printf(seq, "VPD tag mismatch: %#x\n", buf[0]);
+		goto out;
+	}
+	len = buf[1];
+	if (len == 0 || len > vpd_size - 4) {
+		seq_printf(seq, "Invalid id length: %d\n", len);
+		goto out;
+	}
+
+	seq_printf(seq, "%.*s\n", len, buf + 3);
+	offs = len + 3;
+
+	while (offs < vpd_size - 4) {
+		int i;
+
+		if (!memcmp("RW", buf + offs, 2))	/* end marker */
+			break;
+		len = buf[offs + 2];
+		if (offs + len + 3 >= vpd_size)
+			break;
+
+		for (i = 0; i < ARRAY_SIZE(vpd_tags); i++) {
+			if (!memcmp(vpd_tags[i].tag, buf + offs, 2)) {
+				seq_printf(seq, " %s: %.*s\n",
+					   vpd_tags[i].label, len, buf + offs + 3);
+				break;
+			}
+		}
+		offs += len + 3;
+	}
+out:
+	kfree(buf);
+}
+
 static int sky2_debug_show(struct seq_file *seq, void *v)
 {
 	struct net_device *dev = seq->private;
@@ -3874,14 +3954,18 @@ static int sky2_debug_show(struct seq_fi
 	unsigned idx, last;
 	int sop;
 
-	if (!netif_running(dev))
-		return -ENETDOWN;
+	sky2_show_vpd(seq, hw);
 
-	seq_printf(seq, "IRQ src=%x mask=%x control=%x\n",
+	seq_printf(seq, "\nIRQ src=%x mask=%x control=%x\n",
 		   sky2_read32(hw, B0_ISRC),
 		   sky2_read32(hw, B0_IMSK),
 		   sky2_read32(hw, B0_Y2_SP_ICR));
 
+	if (!netif_running(dev)) {
+		seq_printf(seq, "network not running\n");
+		return 0;
+	}
+
 	napi_disable(&hw->napi);
 	last = sky2_read16(hw, STAT_PUT_IDX);
 
@@ -4195,69 +4279,6 @@ static int __devinit pci_wake_enabled(st
 	return value & PCI_PM_CTRL_PME_ENABLE;
 }
 
-/*
- * Read and parse the first part of Vital Product Data
- */
-#define VPD_SIZE	128
-#define VPD_MAGIC	0x82
-
-static void __devinit sky2_vpd_info(struct sky2_hw *hw)
-{
-	int cap = pci_find_capability(hw->pdev, PCI_CAP_ID_VPD);
-	const u8 *p;
-	u8 *vpd_buf = NULL;
-	u16 len;
-	static struct vpd_tag {
-		char tag[2];
-		char *label;
-	} vpd_tags[] = {
-		{ "PN",	"Part Number" },
-		{ "EC", "Engineering Level" },
-		{ "MN", "Manufacturer" },
-	};
-
-	if (!cap)
-		goto out;
-
-	vpd_buf = kmalloc(VPD_SIZE, GFP_KERNEL);
-	if (!vpd_buf)
-		goto out;
-
-	if (sky2_vpd_read(hw, cap, vpd_buf, 0, VPD_SIZE))
-		goto out;
-
-	if (vpd_buf[0] != VPD_MAGIC)
-		goto out;
-	len = vpd_buf[1];
-	if (len == 0 || len > VPD_SIZE - 4)
-		goto out;
-	p = vpd_buf + 3;
-	dev_info(&hw->pdev->dev, "%.*s\n", len, p);
-	p += len;
-
-	while (p < vpd_buf + VPD_SIZE - 4) {
-		int i;
-
-		if (!memcmp("RW", p, 2))	/* end marker */
-			break;
-
-		len = p[2];
-		if (len > (p - vpd_buf) - 4)
-			break;
-
-		for (i = 0; i < ARRAY_SIZE(vpd_tags); i++) {
-			if (!memcmp(vpd_tags[i].tag, p, 2)) {
-				printk(KERN_DEBUG " %s: %.*s\n",
-				       vpd_tags[i].label, len, p + 3);
-				break;
-			}
-		}
-		p += len + 3;
-	}
-out:
-	kfree(vpd_buf);
-}
-
 /* This driver supports yukon2 chipset only */
 static const char *sky2_name(u8 chipid, char *buf, int sz)
 {
@@ -4378,8 +4399,6 @@ static int __devinit sky2_probe(struct p
 
 	sky2_reset(hw);
 
-	sky2_vpd_info(hw);
-
 	dev = sky2_init_netdev(hw, 0, using_dac, wol_default);
 	if (!dev) {
 		err = -ENOMEM;

-- 

[PATCH 4/4] sky2: remove VPD eeprom

[Stephen Hemminger]

[-- Attachment #1: sky2-no-eeprom.patch --]
[-- Type: text/plain, Size: 3752 bytes --]

The VPD is not really the firmware EEPROM. The real EEPROM is behind
an SPI interface, that maybe supported in later version.
Programming the VPD through the ethtool interface doesn't work
(doesn't really change firmware). Now that the interesting bits are
available through the debug interface and raw data is available through sysfs,
the old interface can go.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/net/sky2.c	2008-09-04 11:49:12.000000000 -0700
+++ b/drivers/net/sky2.c	2008-09-04 11:49:28.000000000 -0700
@@ -75,8 +75,6 @@
 #define NAPI_WEIGHT		64
 #define PHY_RETRIES		1000
 
-#define SKY2_EEPROM_MAGIC	0x9955aabb
-
 
 #define RING_NEXT(x,s)	(((x)+1) & ((s)-1))
 
@@ -3722,109 +3720,6 @@ static int sky2_set_tso(struct net_devic
 	return ethtool_op_set_tso(dev, data);
 }
 
-static int sky2_get_eeprom_len(struct net_device *dev)
-{
-	struct sky2_port *sky2 = netdev_priv(dev);
-	struct sky2_hw *hw = sky2->hw;
-	u16 reg2;
-
-	reg2 = sky2_pci_read16(hw, PCI_DEV_REG2);
-	return 1 << ( ((reg2 & PCI_VPD_ROM_SZ) >> 14) + 8);
-}
-
-static int sky2_vpd_wait(const struct sky2_hw *hw, int cap, u16 busy)
-{
-	unsigned long start = jiffies;
-
-	while ( (sky2_pci_read16(hw, cap + PCI_VPD_ADDR) & PCI_VPD_ADDR_F) == busy) {
-		/* Can take up to 10.6 ms for write */
-		if (time_after(jiffies, start + HZ/4)) {
-			dev_err(&hw->pdev->dev, PFX "VPD cycle timed out");
-			return -ETIMEDOUT;
-		}
-		mdelay(1);
-	}
-
-	return 0;
-}
-
-static int sky2_vpd_read(struct sky2_hw *hw, int cap, void *data,
-			 u16 offset, size_t length)
-{
-	int rc = 0;
-
-	while (length > 0) {
-		u32 val;
-
-		sky2_pci_write16(hw, cap + PCI_VPD_ADDR, offset);
-		rc = sky2_vpd_wait(hw, cap, 0);
-		if (rc)
-			break;
-
-		val = sky2_pci_read32(hw, cap + PCI_VPD_DATA);
-
-		memcpy(data, &val, min(sizeof(val), length));
-		offset += sizeof(u32);
-		data += sizeof(u32);
-		length -= sizeof(u32);
-	}
-
-	return rc;
-}
-
-static int sky2_vpd_write(struct sky2_hw *hw, int cap, const void *data,
-			  u16 offset, unsigned int length)
-{
-	unsigned int i;
-	int rc = 0;
-
-	for (i = 0; i < length; i += sizeof(u32)) {
-		u32 val = *(u32 *)(data + i);
-
-		sky2_pci_write32(hw, cap + PCI_VPD_DATA, val);
-		sky2_pci_write32(hw, cap + PCI_VPD_ADDR, offset | PCI_VPD_ADDR_F);
-
-		rc = sky2_vpd_wait(hw, cap, PCI_VPD_ADDR_F);
-		if (rc)
-			break;
-	}
-	return rc;
-}
-
-static int sky2_get_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom,
-			   u8 *data)
-{
-	struct sky2_port *sky2 = netdev_priv(dev);
-	int cap = pci_find_capability(sky2->hw->pdev, PCI_CAP_ID_VPD);
-
-	if (!cap)
-		return -EINVAL;
-
-	eeprom->magic = SKY2_EEPROM_MAGIC;
-
-	return sky2_vpd_read(sky2->hw, cap, data, eeprom->offset, eeprom->len);
-}
-
-static int sky2_set_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom,
-			   u8 *data)
-{
-	struct sky2_port *sky2 = netdev_priv(dev);
-	int cap = pci_find_capability(sky2->hw->pdev, PCI_CAP_ID_VPD);
-
-	if (!cap)
-		return -EINVAL;
-
-	if (eeprom->magic != SKY2_EEPROM_MAGIC)
-		return -EINVAL;
-
-	/* Partial writes not supported */
-	if ((eeprom->offset & 3) || (eeprom->len & 3))
-		return -EINVAL;
-
-	return sky2_vpd_write(sky2->hw, cap, data, eeprom->offset, eeprom->len);
-}
-
-
 static const struct ethtool_ops sky2_ethtool_ops = {
 	.get_settings	= sky2_get_settings,
 	.set_settings	= sky2_set_settings,
@@ -3837,9 +3732,6 @@ static const struct ethtool_ops sky2_eth
 	.get_regs_len	= sky2_get_regs_len,
 	.get_regs	= sky2_get_regs,
 	.get_link	= ethtool_op_get_link,
-	.get_eeprom_len	= sky2_get_eeprom_len,
-	.get_eeprom	= sky2_get_eeprom,
-	.set_eeprom	= sky2_set_eeprom,
 	.get_sg		= ethtool_op_get_sg,
 	.set_sg 	= ethtool_op_set_sg,
 	.get_tx_csum	= ethtool_op_get_tx_csum,

-- 

[PATCH 1/3] PCI: vpd handle longer delays in access

[Stephen Hemminger]

[-- Attachment #1: vpd-delay.patch --]
[-- Type: text/plain, Size: 3108 bytes --]

Accessing the VPD area can take a long time.  The existing 
VPD access code fails consistently on my hardware. There are comments
in the SysKonnect vendor driver that it can take up to 13ms per word. 

Change the access routines to:
  * use a mutex rather than spinning with IRQ's disabled and lock held
  * have a longer timeout
  * call schedule while spinning to provide some responsivness

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/drivers/pci/access.c	2008-09-04 09:06:51.000000000 -0700
+++ b/drivers/pci/access.c	2008-09-04 10:16:52.000000000 -0700
@@ -133,7 +133,7 @@ PCI_USER_WRITE_CONFIG(dword, u32)
 
 struct pci_vpd_pci22 {
 	struct pci_vpd base;
-	spinlock_t lock; /* controls access to hardware and the flags */
+	struct mutex lock;
 	u8	cap;
 	bool	busy;
 	bool	flag; /* value of F bit to wait for */
@@ -144,29 +144,33 @@ static int pci_vpd_pci22_wait(struct pci
 {
 	struct pci_vpd_pci22 *vpd =
 		container_of(dev->vpd, struct pci_vpd_pci22, base);
-	u16 flag, status;
-	int wait;
+	u16 flag = vpd->flag ? PCI_VPD_ADDR_F : 0;
+	unsigned long timeout = jiffies + (vpd->flag ? HZ/50 : HZ/10);
+	u16 status;
 	int ret;
 
 	if (!vpd->busy)
 		return 0;
 
-	flag = vpd->flag ? PCI_VPD_ADDR_F : 0;
-	wait = vpd->flag ? 10 : 1000; /* read: 100 us; write: 10 ms */
 	for (;;) {
-		ret = pci_user_read_config_word(dev,
-						vpd->cap + PCI_VPD_ADDR,
+		ret = pci_user_read_config_word(dev, vpd->cap + PCI_VPD_ADDR,
 						&status);
 		if (ret < 0)
-			return ret;
+			break;
+
 		if ((status & PCI_VPD_ADDR_F) == flag) {
 			vpd->busy = false;
-			return 0;
+			break;
 		}
-		if (wait-- == 0)
+
+		if (time_after(jiffies, timeout))
 			return -ETIMEDOUT;
-		udelay(10);
+		if (signal_pending(current))
+			return -EINTR;
+		yield();
 	}
+
+	return ret;
 }
 
 static int pci_vpd_pci22_read(struct pci_dev *dev, int pos, int size,
@@ -183,7 +187,9 @@ static int pci_vpd_pci22_read(struct pci
 	if (size == 0)
 		return 0;
 
-	spin_lock_irq(&vpd->lock);
+	ret = mutex_lock_killable(&vpd->lock);
+	if (ret)
+		return ret;
 	ret = pci_vpd_pci22_wait(dev);
 	if (ret < 0)
 		goto out;
@@ -199,7 +205,7 @@ static int pci_vpd_pci22_read(struct pci
 	ret = pci_user_read_config_dword(dev, vpd->cap + PCI_VPD_DATA,
 					 &val);
 out:
-	spin_unlock_irq(&vpd->lock);
+	mutex_unlock(&vpd->lock);
 	if (ret < 0)
 		return ret;
 
@@ -231,7 +237,9 @@ static int pci_vpd_pci22_write(struct pc
 	val |= ((u8) *buf++) << 16;
 	val |= ((u32)(u8) *buf++) << 24;
 
-	spin_lock_irq(&vpd->lock);
+	ret = mutex_lock_killable(&vpd->lock);
+	if (ret)
+		return ret;
 	ret = pci_vpd_pci22_wait(dev);
 	if (ret < 0)
 		goto out;
@@ -247,7 +255,7 @@ static int pci_vpd_pci22_write(struct pc
 	vpd->flag = 0;
 	ret = pci_vpd_pci22_wait(dev);
 out:
-	spin_unlock_irq(&vpd->lock);
+	mutex_unlock(&vpd->lock);
 	if (ret < 0)
 		return ret;
 
@@ -279,7 +287,7 @@ int pci_vpd_pci22_init(struct pci_dev *d
 
 	vpd->base.len = PCI_VPD_PCI22_SIZE;
 	vpd->base.ops = &pci_vpd_pci22_ops;
-	spin_lock_init(&vpd->lock);
+	mutex_init(&vpd->lock);
 	vpd->cap = cap;
 	vpd->busy = false;
 	dev->vpd = &vpd->base;

-- 

Re: [PATCH 1/3] PCI: vpd handle longer delays in access

[Peter Zijlstra]

On Thu, 2008-09-04 at 13:56 -0700, Stephen Hemminger wrote:
> plain text document attachment (vpd-delay.patch)
> Accessing the VPD area can take a long time.  The existing 
> VPD access code fails consistently on my hardware. There are comments
> in the SysKonnect vendor driver that it can take up to 13ms per word. 
> 
> Change the access routines to:
>   * use a mutex rather than spinning with IRQ's disabled and lock held
>   * have a longer timeout
>   * call schedule while spinning to provide some responsivness
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> 
> --- a/drivers/pci/access.c	2008-09-04 09:06:51.000000000 -0700
> +++ b/drivers/pci/access.c	2008-09-04 10:16:52.000000000 -0700
> @@ -133,7 +133,7 @@ PCI_USER_WRITE_CONFIG(dword, u32)
>  
>  struct pci_vpd_pci22 {
>  	struct pci_vpd base;
> -	spinlock_t lock; /* controls access to hardware and the flags */
> +	struct mutex lock;
>  	u8	cap;
>  	bool	busy;
>  	bool	flag; /* value of F bit to wait for */
> @@ -144,29 +144,33 @@ static int pci_vpd_pci22_wait(struct pci
>  {
>  	struct pci_vpd_pci22 *vpd =
>  		container_of(dev->vpd, struct pci_vpd_pci22, base);
> -	u16 flag, status;
> -	int wait;
> +	u16 flag = vpd->flag ? PCI_VPD_ADDR_F : 0;
> +	unsigned long timeout = jiffies + (vpd->flag ? HZ/50 : HZ/10);
> +	u16 status;
>  	int ret;
>  
>  	if (!vpd->busy)
>  		return 0;
>  
> -	flag = vpd->flag ? PCI_VPD_ADDR_F : 0;
> -	wait = vpd->flag ? 10 : 1000; /* read: 100 us; write: 10 ms */
>  	for (;;) {
> -		ret = pci_user_read_config_word(dev,
> -						vpd->cap + PCI_VPD_ADDR,
> +		ret = pci_user_read_config_word(dev, vpd->cap + PCI_VPD_ADDR,
>  						&status);
>  		if (ret < 0)
> -			return ret;
> +			break;
> +
>  		if ((status & PCI_VPD_ADDR_F) == flag) {
>  			vpd->busy = false;
> -			return 0;
> +			break;
>  		}
> -		if (wait-- == 0)
> +
> +		if (time_after(jiffies, timeout))
>  			return -ETIMEDOUT;
> -		udelay(10);
> +		if (signal_pending(current))
> +			return -EINTR;
> +		yield();

At the very least put a big comment in here that we're polling the
hardware without completion event.

yield() is not good either, when used with a RT task (IMHO still the
only valid use of yield) it mostly doesn't spend any time away from this
task at all.

The other option, schedule_hrtimeout() isn't ideal either because on
crappy hardware it will fall back to jiffie based timeouts and I _hope_
that most hardware is less shitty than the 13ms reported in the
changelog.

Can we make this a per device delay that starts out small (the previous
10 us sound good) and gets doubled every once in a while when not enough
for a particular device?

That way we can - at some threshold - switch to sleeping delays..

>  	}
> +
> +	return ret;
>  }

Re: [PATCH 1/3] PCI: vpd handle longer delays in access

[Matthew Wilcox]

On Thu, Sep 04, 2008 at 01:56:37PM -0700, Stephen Hemminger wrote:
> -		udelay(10);
> +		if (signal_pending(current))
> +			return -EINTR;

If you're going to use _killable instead of _interruptible, this needs
to be fatal_signal_pending().  Otherwise the one who owns the lock can
be interrupted by _any_ signal while those waiting for the lock can only
be interrupted by fatal signals.  Which seems daft to me.

> -	spin_lock_irq(&vpd->lock);
> +	ret = mutex_lock_killable(&vpd->lock);
> +	if (ret)
> +		return ret;

What's wrong with the shorter:

	if (mutex_lock_killable(&vpd->lock))
		return -EINTR;
?

The actual error is irrelevant here since userspace will never consume it.

(I agree with Peter about use of yield())

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH 1/3] PCI: vpd handle longer delays in access

[Andrew Morton]

On Thu, 04 Sep 2008 13:56:37 -0700
Stephen Hemminger <shemminger@vyatta.com> wrote:

> Accessing the VPD area can take a long time.  The existing 
> VPD access code fails consistently on my hardware. There are comments
> in the SysKonnect vendor driver that it can take up to 13ms per word. 
> 
> Change the access routines to:
>   * use a mutex rather than spinning with IRQ's disabled and lock held
>   * have a longer timeout
>   * call schedule while spinning to provide some responsivness

It doesn't call schedule() - it calls yield().

yield() is pretty notoriously badly behaved in the presence of lots of
runnable tasks and there's been a general move to eradicate its
in-kernel callsites.

An alternative would be nice.

Re: [PATCH 1/3] PCI: vpd handle longer delays in access

[Stephen Hemminger]

On Mon, 8 Sep 2008 13:40:25 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 04 Sep 2008 13:56:37 -0700
> Stephen Hemminger <shemminger@vyatta.com> wrote:
> 
> > Accessing the VPD area can take a long time.  The existing 
> > VPD access code fails consistently on my hardware. There are comments
> > in the SysKonnect vendor driver that it can take up to 13ms per word. 
> > 
> > Change the access routines to:
> >   * use a mutex rather than spinning with IRQ's disabled and lock held
> >   * have a longer timeout
> >   * call schedule while spinning to provide some responsivness
> 
> It doesn't call schedule() - it calls yield().
> 
> yield() is pretty notoriously badly behaved in the presence of lots of
> runnable tasks and there's been a general move to eradicate its
> in-kernel callsites.
> 
> An alternative would be nice.

What is a good way to say "i am polling for a while"?

Re: [PATCH 1/3] PCI: vpd handle longer delays in access

[Arjan van de Ven]

On Mon, 8 Sep 2008 14:08:24 -0700
Stephen Hemminger <shemminger@vyatta.com> wrote:

> 
> What is a good way to say "i am polling for a while"?


can you take a fixed time delay?

or use cond_resched()


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH 1/3] PCI: vpd handle longer delays in access

[Stephen Hemminger]

On Mon, 8 Sep 2008 14:26:05 -0700
Arjan van de Ven <arjan@infradead.org> wrote:

> On Mon, 8 Sep 2008 14:08:24 -0700
> Stephen Hemminger <shemminger@vyatta.com> wrote:
> 
> > 
> > What is a good way to say "i am polling for a while"?
> 
> 
> can you take a fixed time delay?
> 
> or use cond_resched()
cond_resched sounds like the best suggestion.  The problem is that this
code needs to deal with hardware that could be fast, or slow depending
on how the device is implemented.  

Re: [PATCH 1/3] PCI: vpd handle longer delays in access

[Arjan van de Ven]

On Tue, 9 Sep 2008 09:55:48 -0700
Stephen Hemminger <shemminger@vyatta.com> wrote:

> On Mon, 8 Sep 2008 14:26:05 -0700
> Arjan van de Ven <arjan@infradead.org> wrote:
> 
> > On Mon, 8 Sep 2008 14:08:24 -0700
> > Stephen Hemminger <shemminger@vyatta.com> wrote:
> > 
> > > 
> > > What is a good way to say "i am polling for a while"?
> > 
> > 
> > can you take a fixed time delay?
> > 
> > or use cond_resched()
> cond_resched sounds like the best suggestion.  The problem is that
> this code needs to deal with hardware that could be fast, or slow
> depending on how the device is implemented.  

one option is poll fast for <X jiffies> and then back off with sleeping
delays



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

[PATCH 2/3] PCI: revise VPD access interface

[Stephen Hemminger]

[-- Attachment #1: vpd-ops.patch --]
[-- Type: text/plain, Size: 9079 bytes --]

Change PCI VPD API which was only used by sysfs to something usable
in drivers. 
   * move iteration over multiple words to the low level
   * cleanup types of arguments
   * add exportable wrapper

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/pci/access.c	2008-09-04 10:16:52.000000000 -0700
+++ b/drivers/pci/access.c	2008-09-04 10:19:47.000000000 -0700
@@ -66,6 +66,39 @@ EXPORT_SYMBOL(pci_bus_write_config_byte)
 EXPORT_SYMBOL(pci_bus_write_config_word);
 EXPORT_SYMBOL(pci_bus_write_config_dword);
 
+
+/**
+ * pci_read_vpd - Read one entry from Vital Product Data
+ * @dev:	pci device struct
+ * @pos:	offset in vpd space
+ * @count:	number of bytes to read
+ * @buf:	pointer to where to store result
+ *
+ */
+int pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf)
+{
+	if (!dev->vpd || !dev->vpd->ops)
+		return -ENODEV;
+	return dev->vpd->ops->read(dev, pos, count, buf);
+}
+EXPORT_SYMBOL(pci_read_vpd);
+
+/**
+ * pci_write_vpd - Write entry to Vital Product Data
+ * @dev:	pci device struct
+ * @pos:	offset in vpd space
+ * @count:	number of bytes to read
+ * @val:	value to write
+ *
+ */
+int pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf)
+{
+	if (!dev->vpd || !dev->vpd->ops)
+		return -ENODEV;
+	return dev->vpd->ops->write(dev, pos, count, buf);
+}
+EXPORT_SYMBOL(pci_write_vpd);
+
 /*
  * The following routines are to prevent the user from accessing PCI config
  * space when it's unsafe to do so.  Some devices require this during BIST and
@@ -173,93 +206,97 @@ static int pci_vpd_pci22_wait(struct pci
 	return ret;
 }
 
-static int pci_vpd_pci22_read(struct pci_dev *dev, int pos, int size,
-			      char *buf)
+static int pci_vpd_pci22_read(struct pci_dev *dev, loff_t pos, size_t count,
+			      void *buf)
 {
 	struct pci_vpd_pci22 *vpd =
 		container_of(dev->vpd, struct pci_vpd_pci22, base);
-	u32 val;
 	int ret;
-	int begin, end, i;
+	loff_t end = pos + count;
+	unsigned char *p = buf;
 
-	if (pos < 0 || pos > vpd->base.len || size > vpd->base.len  - pos)
+	if (pos < 0 || pos > vpd->base.len || count > vpd->base.len - pos)
 		return -EINVAL;
-	if (size == 0)
-		return 0;
 
 	ret = mutex_lock_killable(&vpd->lock);
 	if (ret)
 		return ret;
-	ret = pci_vpd_pci22_wait(dev);
-	if (ret < 0)
-		goto out;
-	ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
-					 pos & ~3);
-	if (ret < 0)
-		goto out;
-	vpd->busy = true;
-	vpd->flag = 1;
-	ret = pci_vpd_pci22_wait(dev);
-	if (ret < 0)
-		goto out;
-	ret = pci_user_read_config_dword(dev, vpd->cap + PCI_VPD_DATA,
-					 &val);
-out:
-	mutex_unlock(&vpd->lock);
-	if (ret < 0)
-		return ret;
 
-	/* Convert to bytes */
-	begin = pos & 3;
-	end = min(4, begin + size);
-	for (i = 0; i < end; ++i) {
-		if (i >= begin)
-			*buf++ = val;
-		val >>= 8;
+	while (pos < end) {
+		u32 val;
+		unsigned int i, skip;
+
+		ret = pci_vpd_pci22_wait(dev);
+		if (ret < 0)
+			break;
+
+		ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
+						 pos & ~3);
+		if (ret < 0)
+			break;
+		vpd->busy = true;
+		vpd->flag = 1;
+		ret = pci_vpd_pci22_wait(dev);
+		if (ret < 0)
+			break;
+
+		ret = pci_user_read_config_dword(dev, vpd->cap + PCI_VPD_DATA, &val);
+		if (ret < 0)
+			break;
+
+		skip = pos & 3;
+		for (i = 0;  i < sizeof(u32); i++) {
+			if (i >= skip) {
+				*p++ = val;
+				if (++pos == end)
+					break;
+			}
+			val >>= 8;
+		}
 	}
-	return end - begin;
+	mutex_unlock(&vpd->lock);
+	return ret ? ret : count;
 }
 
-static int pci_vpd_pci22_write(struct pci_dev *dev, int pos, int size,
-			       const char *buf)
+static int pci_vpd_pci22_write(struct pci_dev *dev, loff_t pos, size_t count,
+			       const void *buf)
 {
 	struct pci_vpd_pci22 *vpd =
 		container_of(dev->vpd, struct pci_vpd_pci22, base);
-	u32 val;
+	loff_t end = pos + count;
 	int ret;
 
-	if (pos < 0 || pos > vpd->base.len || pos & 3 ||
-	    size > vpd->base.len - pos || size < 4)
+	if (pos > vpd->base.len || pos & 3)
 		return -EINVAL;
 
-	val = (u8) *buf++;
-	val |= ((u8) *buf++) << 8;
-	val |= ((u8) *buf++) << 16;
-	val |= ((u32)(u8) *buf++) << 24;
-
 	ret = mutex_lock_killable(&vpd->lock);
 	if (ret)
 		return ret;
-	ret = pci_vpd_pci22_wait(dev);
-	if (ret < 0)
-		goto out;
-	ret = pci_user_write_config_dword(dev, vpd->cap + PCI_VPD_DATA,
-					  val);
-	if (ret < 0)
-		goto out;
-	ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
-					 pos | PCI_VPD_ADDR_F);
-	if (ret < 0)
-		goto out;
-	vpd->busy = true;
-	vpd->flag = 0;
-	ret = pci_vpd_pci22_wait(dev);
-out:
-	mutex_unlock(&vpd->lock);
-	if (ret < 0)
-		return ret;
 
-	return 4;
+	while (pos < end) {
+		u32 val;
+
+		ret = pci_vpd_pci22_wait(dev);
+		if (ret < 0)
+			break;
+		memcpy(&val, buf, sizeof(u32));
+
+		ret = pci_user_write_config_dword(dev, vpd->cap + PCI_VPD_DATA, val);
+		if (ret < 0)
+			break;
+		ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
+						 pos | PCI_VPD_ADDR_F);
+		if (ret < 0)
+			break;
+		vpd->busy = true;
+		vpd->flag = 0;
+		ret = pci_vpd_pci22_wait(dev);
+
+		buf += sizeof(u32);
+		pos += sizeof(u32);
+	}
+	mutex_unlock(&vpd->lock);
+	return ret ? ret : count;
 }
 
 static void pci_vpd_pci22_release(struct pci_dev *dev)
@@ -267,7 +304,7 @@ static void pci_vpd_pci22_release(struct
 	kfree(container_of(dev->vpd, struct pci_vpd_pci22, base));
 }
 
-static struct pci_vpd_ops pci_vpd_pci22_ops = {
+static const struct pci_vpd_ops pci_vpd_pci22_ops = {
 	.read = pci_vpd_pci22_read,
 	.write = pci_vpd_pci22_write,
 	.release = pci_vpd_pci22_release,
--- a/drivers/pci/pci.h	2008-09-04 10:14:59.000000000 -0700
+++ b/drivers/pci/pci.h	2008-09-04 10:17:01.000000000 -0700
@@ -44,14 +44,14 @@ extern int pci_user_write_config_word(st
 extern int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
 
 struct pci_vpd_ops {
-	int (*read)(struct pci_dev *dev, int pos, int size, char *buf);
-	int (*write)(struct pci_dev *dev, int pos, int size, const char *buf);
+	int (*read)(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
+	int (*write)(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
 	void (*release)(struct pci_dev *dev);
 };
 
 struct pci_vpd {
 	unsigned int len;
-	struct pci_vpd_ops *ops;
+	const struct pci_vpd_ops *ops;
 	struct bin_attribute *attr; /* descriptor for sysfs VPD entry */
 };
 
--- a/drivers/pci/pci-sysfs.c	2008-09-04 10:14:59.000000000 -0700
+++ b/drivers/pci/pci-sysfs.c	2008-09-04 10:17:01.000000000 -0700
@@ -360,55 +360,33 @@ pci_write_config(struct kobject *kobj, s
 }
 
 static ssize_t
-pci_read_vpd(struct kobject *kobj, struct bin_attribute *bin_attr,
-	     char *buf, loff_t off, size_t count)
+read_vpd_attr(struct kobject *kobj, struct bin_attribute *bin_attr,
+	      char *buf, loff_t off, size_t count)
 {
 	struct pci_dev *dev =
 		to_pci_dev(container_of(kobj, struct device, kobj));
-	int end;
-	int ret;
 
 	if (off > bin_attr->size)
 		count = 0;
 	else if (count > bin_attr->size - off)
 		count = bin_attr->size - off;
-	end = off + count;
 
-	while (off < end) {
-		ret = dev->vpd->ops->read(dev, off, end - off, buf);
-		if (ret < 0)
-			return ret;
-		buf += ret;
-		off += ret;
-	}
-
-	return count;
+	return pci_read_vpd(dev, off, count, buf);
 }
 
 static ssize_t
-pci_write_vpd(struct kobject *kobj, struct bin_attribute *bin_attr,
-	      char *buf, loff_t off, size_t count)
+write_vpd_attr(struct kobject *kobj, struct bin_attribute *bin_attr,
+	       char *buf, loff_t off, size_t count)
 {
 	struct pci_dev *dev =
 		to_pci_dev(container_of(kobj, struct device, kobj));
-	int end;
-	int ret;
 
 	if (off > bin_attr->size)
 		count = 0;
 	else if (count > bin_attr->size - off)
 		count = bin_attr->size - off;
-	end = off + count;
-
-	while (off < end) {
-		ret = dev->vpd->ops->write(dev, off, end - off, buf);
-		if (ret < 0)
-			return ret;
-		buf += ret;
-		off += ret;
-	}
 
-	return count;
+	return pci_write_vpd(dev, off, count, buf);
 }
 
 #ifdef HAVE_PCI_LEGACY
@@ -739,8 +717,8 @@ int __must_check pci_create_sysfs_dev_fi
 			attr->size = pdev->vpd->len;
 			attr->attr.name = "vpd";
 			attr->attr.mode = S_IRUSR | S_IWUSR;
-			attr->read = pci_read_vpd;
-			attr->write = pci_write_vpd;
+			attr->read = read_vpd_attr;
+			attr->write = write_vpd_attr;
 			retval = sysfs_create_bin_file(&pdev->dev.kobj, attr);
 			if (retval)
 				goto err_vpd;
--- a/include/linux/pci.h	2008-09-04 10:14:59.000000000 -0700
+++ b/include/linux/pci.h	2008-09-04 10:17:01.000000000 -0700
@@ -650,6 +650,10 @@ int pci_back_from_sleep(struct pci_dev *
 /* Functions for PCI Hotplug drivers to use */
 int pci_bus_find_capability(struct pci_bus *bus, unsigned int devfn, int cap);
 
+/* Vital product data routines */
+int pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
+int pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
+
 /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
 void pci_bus_assign_resources(struct pci_bus *bus);
 void pci_bus_size_bridges(struct pci_bus *bus);

-- 

Re: [PATCH 2/3] PCI: revise VPD access interface

[Ben Hutchings]

Stephen Hemminger wrote:
> Change PCI VPD API which was only used by sysfs to something usable
> in drivers. 
>    * move iteration over multiple words to the low level
>    * cleanup types of arguments
>    * add exportable wrapper
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> --- a/drivers/pci/access.c	2008-09-04 10:16:52.000000000 -0700
> +++ b/drivers/pci/access.c	2008-09-04 10:19:47.000000000 -0700
[...]
> -static int pci_vpd_pci22_write(struct pci_dev *dev, int pos, int size,
> -			       const char *buf)
> +static int pci_vpd_pci22_write(struct pci_dev *dev, loff_t pos, size_t count,
> +			       const void *buf)
>  {
>  	struct pci_vpd_pci22 *vpd =
>  		container_of(dev->vpd, struct pci_vpd_pci22, base);
> -	u32 val;
> +	loff_t end = pos + count;
>  	int ret;
>  
> -	if (pos < 0 || pos > vpd->base.len || pos & 3 ||
> -	    size > vpd->base.len - pos || size < 4)
> +	if (pos > vpd->base.len || pos & 3)
>  		return -EINVAL;
>  
> -	val = (u8) *buf++;
> -	val |= ((u8) *buf++) << 8;
> -	val |= ((u8) *buf++) << 16;
> -	val |= ((u32)(u8) *buf++) << 24;
> -
>  	ret = mutex_lock_killable(&vpd->lock);
>  	if (ret)
>  		return ret;
> -	ret = pci_vpd_pci22_wait(dev);
> -	if (ret < 0)
> -		goto out;
> -	ret = pci_user_write_config_dword(dev, vpd->cap + PCI_VPD_DATA,
> -					  val);
> -	if (ret < 0)
> -		goto out;
> -	ret = pci_user_write_config_word(dev, vpd->cap + PCI_VPD_ADDR,
> -					 pos | PCI_VPD_ADDR_F);
> -	if (ret < 0)
> -		goto out;
> -	vpd->busy = true;
> -	vpd->flag = 0;
> -	ret = pci_vpd_pci22_wait(dev);
> -out:
> -	mutex_unlock(&vpd->lock);
> -	if (ret < 0)
> -		return ret;
>  
> -	return 4;
> +	while (pos < end) {
> +		u32 val;
> +
> +		ret = pci_vpd_pci22_wait(dev);
> +		if (ret < 0)
> +			break;
> +		memcpy(&val, buf, sizeof(u32));
[...]

I'm not sure this is correct.  pci_user_write_config_dword() expects a
value in host byte order, but this memcpy() makes val always little-endian.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 2/3] PCI: revise VPD access interface

[Andrew Morton]

On Thu, 04 Sep 2008 13:56:38 -0700
Stephen Hemminger <shemminger@vyatta.com> wrote:

> Change PCI VPD API which was only used by sysfs to something usable
> in drivers. 
>    * move iteration over multiple words to the low level
>    * cleanup types of arguments
>    * add exportable wrapper
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> --- a/drivers/pci/access.c	2008-09-04 10:16:52.000000000 -0700
> +++ b/drivers/pci/access.c	2008-09-04 10:19:47.000000000 -0700
> @@ -66,6 +66,39 @@ EXPORT_SYMBOL(pci_bus_write_config_byte)
>  EXPORT_SYMBOL(pci_bus_write_config_word);
>  EXPORT_SYMBOL(pci_bus_write_config_dword);
>  
> +
> +/**
> + * pci_read_vpd - Read one entry from Vital Product Data
> + * @dev:	pci device struct
> + * @pos:	offset in vpd space
> + * @count:	number of bytes to read
> + * @buf:	pointer to where to store result
> + *
> + */
> +int pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf)
> +{
> +	if (!dev->vpd || !dev->vpd->ops)
> +		return -ENODEV;
> +	return dev->vpd->ops->read(dev, pos, count, buf);
> +}
> +EXPORT_SYMBOL(pci_read_vpd);

"read" functions normally return ssize_t, not int.

>
> ...
>  
>  static ssize_t
> -pci_read_vpd(struct kobject *kobj, struct bin_attribute *bin_attr,
> -	     char *buf, loff_t off, size_t count)
> +read_vpd_attr(struct kobject *kobj, struct bin_attribute *bin_attr,
> +	      char *buf, loff_t off, size_t count)
>  {

This returns size_t.

>  static ssize_t
> -pci_write_vpd(struct kobject *kobj, struct bin_attribute *bin_attr,
> -	      char *buf, loff_t off, size_t count)
> +write_vpd_attr(struct kobject *kobj, struct bin_attribute *bin_attr,
> +	       char *buf, loff_t off, size_t count)

as does this.

> @@ -739,8 +717,8 @@ int __must_check pci_create_sysfs_dev_fi
>  			attr->size = pdev->vpd->len;
>  			attr->attr.name = "vpd";
>  			attr->attr.mode = S_IRUSR | S_IWUSR;
> -			attr->read = pci_read_vpd;
> -			attr->write = pci_write_vpd;
> +			attr->read = read_vpd_attr;
> +			attr->write = write_vpd_attr;

But this (I think) is assigning a ssize_t-returning function to an
int-returning function pointer.

[PATCH 3/3] PCI: add interface to set visible size of VPD

[Stephen Hemminger]

[-- Attachment #1: vpd-size.patch --]
[-- Type: text/plain, Size: 1460 bytes --]

The VPD on all devices may not be 32K. Unfortunately, there is no
generic way to find the size, so this adds a simple API hook
to reset it.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

--- a/drivers/pci/access.c	2008-09-04 11:29:22.000000000 -0700
+++ b/drivers/pci/access.c	2008-09-04 11:38:29.000000000 -0700
@@ -332,6 +332,24 @@ int pci_vpd_pci22_init(struct pci_dev *d
 }
 
 /**
+ * pci_vpd_size - Set available Vital Product Data size
+ * @dev:	pci device struct
+ * @size:	available memory in bytes
+ *
+ * Adjust size of available VPD area.
+ */
+int pci_vpd_size(struct pci_dev *dev, size_t size)
+{
+	if (!dev->vpd)
+		return -EINVAL;
+	if (size > PCI_VPD_PCI22_SIZE)
+		return -EINVAL;
+	dev->vpd->len = size;
+	return 0;
+}
+EXPORT_SYMBOL(pci_vpd_size);
+
+/**
  * pci_block_user_cfg_access - Block userspace PCI config reads/writes
  * @dev:	pci device struct
  *
--- a/include/linux/pci.h	2008-09-04 11:28:45.000000000 -0700
+++ b/include/linux/pci.h	2008-09-04 11:29:15.000000000 -0700
@@ -653,6 +653,7 @@ int pci_bus_find_capability(struct pci_b
 /* Vital product data routines */
 int pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
 int pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
+int pci_vpd_size(struct pci_dev *dev, size_t size);
 
 /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
 void pci_bus_assign_resources(struct pci_bus *bus);

-- 

Re: [PATCH 3/3] PCI: add interface to set visible size of VPD

[Ben Hutchings]

Stephen Hemminger wrote:
> The VPD on all devices may not be 32K. Unfortunately, there is no
> generic way to find the size, so this adds a simple API hook
> to reset it.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
> 
> --- a/drivers/pci/access.c	2008-09-04 11:29:22.000000000 -0700
> +++ b/drivers/pci/access.c	2008-09-04 11:38:29.000000000 -0700
> @@ -332,6 +332,24 @@ int pci_vpd_pci22_init(struct pci_dev *d
>  }
>  
>  /**
> + * pci_vpd_size - Set available Vital Product Data size
> + * @dev:	pci device struct
> + * @size:	available memory in bytes
> + *
> + * Adjust size of available VPD area.
> + */
> +int pci_vpd_size(struct pci_dev *dev, size_t size)
> +{
> +	if (!dev->vpd)
> +		return -EINVAL;
> +	if (size > PCI_VPD_PCI22_SIZE)
> +		return -EINVAL;

This assumes that PCI 2.2 VPD is the only VPD implementation (which is true
at the moment, but at least PCI 2.1 VPD should be added).  Maybe this should
compare with the current length instead?

Ben.

> +	dev->vpd->len = size;
> +	return 0;
> +}
> +EXPORT_SYMBOL(pci_vpd_size);
> +
> +/**
>   * pci_block_user_cfg_access - Block userspace PCI config reads/writes
>   * @dev:	pci device struct
>   *
> --- a/include/linux/pci.h	2008-09-04 11:28:45.000000000 -0700
> +++ b/include/linux/pci.h	2008-09-04 11:29:15.000000000 -0700
> @@ -653,6 +653,7 @@ int pci_bus_find_capability(struct pci_b
>  /* Vital product data routines */
>  int pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
>  int pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
> +int pci_vpd_size(struct pci_dev *dev, size_t size);
>  
>  /* Helper functions for low-level code (drivers/pci/setup-[bus,res].c) */
>  void pci_bus_assign_resources(struct pci_bus *bus);
> 
> -- 
> 

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.