[nvo] Remove the non-volatile options fragment list

Since its implementation several years ago, no driver has used a
fragment list containing more than a single fragment.  Simplify the
NVO core and the drivers that use it by removing the whole concept of
the fragment list, and using a simple (address,length) pair instead.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/core/nvo.c

@@ -43,7 +43,7 @@ static unsigned int nvo_checksum ( struct nvo_block *nvo ) {
 	uint8_t sum = 0;
 	unsigned int i;
 
-	for ( i = 0 ; i < nvo->total_len ; i++ ) {
+	for ( i = 0 ; i < nvo->len ; i++ ) {
 		sum += *(data++);
 	}
 	return sum;
@@ -56,20 +56,15 @@ static unsigned int nvo_checksum ( struct nvo_block *nvo ) {
  * @ret rc		Return status code
  */
 static int nvo_load ( struct nvo_block *nvo ) {
-	void *data = nvo->data;
-	struct nvo_fragment *frag;
 	int rc;
 
-	/* Read data a fragment at a time */
+	/* Read data */
-	for ( frag = nvo->fragments ; frag->len ; frag++ ) {
+	if ( ( rc = nvs_read ( nvo->nvs, nvo->address, nvo->data,
-		if ( ( rc = nvs_read ( nvo->nvs, frag->address, data,
+			       nvo->len ) ) != 0 ) {
-				       frag->len ) ) != 0 ) {
+		DBGC ( nvo, "NVO %p could not read %zd bytes at %#04x: %s\n",
-			DBGC ( nvo, "NVO %p could not read %zd bytes at "
+		       nvo, nvo->len, nvo->address, strerror ( rc ) );
-			       "%#04x\n", nvo, frag->len, frag->address );
 		return rc;
 	}
-		data += frag->len;
-	}
 
 	DBGC ( nvo, "NVO %p loaded from non-volatile storage\n", nvo );
 	return 0;
@@ -82,24 +77,19 @@ static int nvo_load ( struct nvo_block *nvo ) {
  * @ret rc		Return status code
  */
 static int nvo_save ( struct nvo_block *nvo ) {
-	void *data = nvo->data;
+	uint8_t *checksum = nvo->data;
-	uint8_t *checksum = data;
-	struct nvo_fragment *frag;
 	int rc;
 
 	/* Recalculate checksum */
 	*checksum -= nvo_checksum ( nvo );
 
-	/* Write data a fragment at a time */
+	/* Write data */
-	for ( frag = nvo->fragments ; frag->len ; frag++ ) {
+	if ( ( rc = nvs_write ( nvo->nvs, nvo->address, nvo->data,
-		if ( ( rc = nvs_write ( nvo->nvs, frag->address, data,
+				nvo->len ) ) != 0 ) {
-					frag->len ) ) != 0 ) {
+		DBGC ( nvo, "NVO %p could not write %zd bytes at %#04x: %s\n",
-			DBGC ( nvo, "NVO %p could not write %zd bytes at "
+		       nvo, nvo->len, nvo->address, strerror ( rc ) );
-			       "%#04x\n", nvo, frag->len, frag->address );
 		return rc;
 	}
-		data += frag->len;
-	}
 
 	DBGC ( nvo, "NVO %p saved to non-volatile storage\n", nvo );
 	return 0;
@@ -120,7 +110,7 @@ static void nvo_init_dhcpopts ( struct nvo_block *nvo ) {
 
 	/* Steal one byte for the checksum */
 	options_data = ( nvo->data + 1 );
-	options_len = ( nvo->total_len - 1 );
+	options_len = ( nvo->len - 1 );
 
 	/* If checksum fails, or options data starts with a zero,
 	 * assume the whole block is invalid.  This should capture the
@@ -130,7 +120,7 @@ static void nvo_init_dhcpopts ( struct nvo_block *nvo ) {
 		DBGC ( nvo, "NVO %p has checksum %02x and initial byte %02x; "
 		       "assuming empty\n", nvo, nvo_checksum ( nvo ),
 		       options_data[0] );
-		memset ( nvo->data, 0, nvo->total_len );
+		memset ( nvo->data, 0, nvo->len );
 	}
 
 	dhcpopt_init ( &nvo->dhcpopts, options_data, options_len,
@@ -198,13 +188,15 @@ static struct settings_operations nvo_settings_operations = {
  *
  * @v nvo		Non-volatile options block
  * @v nvs		Underlying non-volatile storage device
- * @v fragments		List of option-containing fragments, or NULL
+ * @v address		Address within NVS device
+ * @v len		Length of non-volatile options data
  * @v refcnt		Containing object reference counter, or NULL
  */
 void nvo_init ( struct nvo_block *nvo, struct nvs_device *nvs,
-		struct nvo_fragment *fragments, struct refcnt *refcnt ) {
+		size_t address, size_t len, struct refcnt *refcnt ) {
 	nvo->nvs = nvs;
-	nvo->fragments = fragments;
+	nvo->address = address;
+	nvo->len = len;
 	settings_init ( &nvo->settings, &nvo_settings_operations, refcnt, 0 );
 }
 
@@ -216,34 +208,17 @@ void nvo_init ( struct nvo_block *nvo, struct nvs_device *nvs,
  * @ret rc		Return status code
  */
 int register_nvo ( struct nvo_block *nvo, struct settings *parent ) {
-	struct nvo_fragment *fragment = nvo->fragments;
 	int rc;
 
-	/* Calculate total length of all fragments, if applicable */
+	/* Allocate memory for options */
-	if ( fragment ) {
+	nvo->data = zalloc ( nvo->len );
-		for ( ; fragment->len ; fragment++ )
-			nvo->total_len += fragment->len;
-	} else {
-		nvo->total_len = nvo->nvs->size;
-	}
-
-	/* Allocate memory for options (and fragment list, if applicable) */
-	nvo->data = zalloc ( nvo->total_len +
-			     ( fragment ? 0 : ( 2 * sizeof ( *fragment ) ) ) );
 	if ( ! nvo->data ) {
 		DBGC ( nvo, "NVO %p could not allocate %zd bytes\n",
-		       nvo, nvo->total_len );
+		       nvo, nvo->len );
 		rc = -ENOMEM;
 		goto err_malloc;
 	}
 
-	/* Create fragment list, if applicable */
-	if ( ! fragment ) {
-		fragment = ( nvo->data + nvo->total_len );
-		fragment->len = nvo->total_len;
-		nvo->fragments = fragment;
-	}
-
 	/* Read data from NVS */
 	if ( ( rc = nvo_load ( nvo ) ) != 0 )
 		goto err_load;

src/drivers/net/etherfabric.c

@@ -1492,12 +1492,6 @@ fail1:
 	return rc;
 }
 
-/** Portion of EEPROM available for non-volatile options */
-static struct nvo_fragment falcon_nvo_fragments[] = {
-	{ 0x100, 0xf0 },
-	{ 0, 0 }
-};
-
 /*******************************************************************************
  *
  *
@@ -3277,9 +3271,10 @@ falcon_probe_spi ( struct efab_nic *efab )
 	}
 
 	/* If the device has EEPROM attached, then advertise NVO space */
-	if ( has_eeprom )
+	if ( has_eeprom ) {
-		nvo_init ( &efab->nvo, &efab->spi_eeprom.nvs, falcon_nvo_fragments,
+		nvo_init ( &efab->nvo, &efab->spi_eeprom.nvs, 0x100, 0xf0,
 			   &efab->netdev->refcnt );
+	}
 
 	return 0;
 }

src/drivers/net/myri10ge.c

@@ -183,8 +183,8 @@ struct myri10ge_private
 	 */
 
 	struct nvs_device	nvs;
-	struct nvo_fragment	nvo_fragment[2];
 	struct nvo_block	nvo;
+	unsigned int		nvo_registered;
 
 	/* Cached PCI capability locations. */
 
@@ -727,28 +727,20 @@ static int myri10ge_nv_init ( struct myri10ge_private *priv )
 	priv->nvs.read		= myri10ge_nvs_read;
 	priv->nvs.write		= myri10ge_nvs_write;
 
-	/* Build the NonVolatile storage fragment list.  We would like
-	   to use the whole last EEPROM block for this, but we must
-	   reduce the block size lest malloc fail in
-	   src/core/nvo.o. */
-
-	priv->nvo_fragment[0].address = nvo_fragment_pos;
-	priv->nvo_fragment[0].len     = 0x200;
-
 	/* Register the NonVolatile Options storage. */
 
 	nvo_init ( &priv->nvo,
 		   &priv->nvs,
-		   priv->nvo_fragment,
+		   nvo_fragment_pos, 0x200,
 		   & myri10ge_netdev (priv) -> refcnt );
 	rc = register_nvo ( &priv->nvo,
 			    netdev_settings ( myri10ge_netdev ( priv ) ) );
 	if ( rc ) {
 		DBG ("register_nvo failed");
-		priv->nvo_fragment[0].len = 0;
 		return rc;
 	}
 
+	priv->nvo_registered = 1;
 	DBG2 ( "NVO supported\n" );
 	return 0;
 }
@@ -758,7 +750,7 @@ myri10ge_nv_fini ( struct myri10ge_private *priv )
 {
 	/* Simply return if nonvolatile access is not supported. */
 
-	if ( 0 == priv->nvo_fragment[0].len )
+	if ( 0 == priv->nvo_registered )
 		return;
 
 	unregister_nvo ( &priv->nvo );

src/drivers/net/natsemi.c

@@ -130,15 +130,6 @@ static struct bit_basher_operations natsemi_basher_ops = {
 	.write = natsemi_spi_write_bit,
 };
 
-/* It looks that this portion of EEPROM can be used for 
- * non-volatile stored options. Data sheet does not talk about this region.
- * Currently it is not working. But with some efforts it can.
- */
-static struct nvo_fragment natsemi_nvo_fragments[] = {
-	{ 0x0c, 0x68 },
-	{ 0, 0 }
-};
-
 /*
  * Set up for EEPROM access
  *
@@ -157,8 +148,13 @@ static void natsemi_init_eeprom ( struct natsemi_private *np ) {
 	 */
 	init_at93c46 ( &np->eeprom, 16 );
 	np->eeprom.bus = &np->spibit.bus;
-	np->nvo.nvs = &np->eeprom.nvs;
+
-	np->nvo.fragments = natsemi_nvo_fragments;
+	/* It looks that this portion of EEPROM can be used for
+	 * non-volatile stored options. Data sheet does not talk about
+	 * this region.  Currently it is not working. But with some
+	 * efforts it can.
+	 */
+	nvo_init ( &np->nvo, &np->eeprom.nvs, 0x0c, 0x68, NULL );
 }
 
 /**

src/drivers/net/rtl8139.c

@@ -251,17 +251,6 @@ static struct bit_basher_operations rtl_basher_ops = {
 	.write = rtl_spi_write_bit,
 };
 
-/** Portion of EEPROM available for non-volatile stored options
- *
- * We use offset 0x40 (i.e. address 0x20), length 0x40.  This block is
- * marked as VPD in the rtl8139 datasheets, so we use it only if we
- * detect that the card is not supporting VPD.
- */
-static struct nvo_fragment rtl_nvo_fragments[] = {
-	{ 0x20, 0x40 },
-	{ 0, 0 }
-};
-
 /**
  * Set up for EEPROM access
  *
@@ -288,13 +277,18 @@ static void rtl_init_eeprom ( struct net_device *netdev ) {
 	}
 	rtl->eeprom.bus = &rtl->spibit.bus;
 
-	/* Initialise space for non-volatile options, if available */
+	/* Initialise space for non-volatile options, if available
+	 *
+	 * We use offset 0x40 (i.e. address 0x20), length 0x40.  This
+	 * block is marked as VPD in the rtl8139 datasheets, so we use
+	 * it only if we detect that the card is not supporting VPD.
+	 */
 	vpd = ( inw ( rtl->ioaddr + Config1 ) & VPDEnable );
 	if ( vpd ) {
 		DBGC ( rtl, "rtl8139 %p EEPROM in use for VPD; cannot use "
 		       "for options\n", rtl );
 	} else {
-		nvo_init ( &rtl->nvo, &rtl->eeprom.nvs, rtl_nvo_fragments,
+		nvo_init ( &rtl->nvo, &rtl->eeprom.nvs, 0x20, 0x40,
 			   &netdev->refcnt );
 	}
 }

src/include/ipxe/nvo.h

@@ -16,16 +16,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 struct nvs_device;
 struct refcnt;
 
-/**
- * A fragment of a non-volatile storage device used for stored options
- */
-struct nvo_fragment {
-	/** Starting address of fragment within NVS device */
-	unsigned int address;
-	/** Length of fragment */
-	size_t len;
-};
-
 /**
  * A block of non-volatile stored options
  */
@@ -34,13 +24,10 @@ struct nvo_block {
 	struct settings settings;
 	/** Underlying non-volatile storage device */
 	struct nvs_device *nvs;
-	/** List of option-containing fragments
+	/** Address within NVS device */
-	 *
+	unsigned int address;
-	 * The list is terminated by a fragment with a length of zero.
+	/** Length of options data */
-	 */
+	size_t len;
-	struct nvo_fragment *fragments;
-	/** Total length of option-containing fragments */
-	size_t total_len;
 	/** Option-containing data */
 	void *data;
 	/** DHCP options block */
@@ -48,7 +35,7 @@ struct nvo_block {
 };
 
 extern void nvo_init ( struct nvo_block *nvo, struct nvs_device *nvs,
-		       struct nvo_fragment *fragments, struct refcnt *refcnt );
+		       size_t address, size_t len, struct refcnt *refcnt );
 extern int register_nvo ( struct nvo_block *nvo, struct settings *parent );
 extern void unregister_nvo ( struct nvo_block *nvo );