Patchwork Use correct programmer fallback functions

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2009-08-11 01:12:39
Message ID <4A80C587.9010005@gmx.net>
Download mbox | patch
Permalink /patch/103/
State Accepted
Commit r678
Headers show

Comments

Carl-Daniel Hailfinger - 2009-08-11 01:12:39
FT2232 and IT87 programmers used functions of the dummy programmer
instead of fallback functions. The dummy programmer is a "real"
programmer with possible side effects and its functions should not be
abused by other programmers. Make FT2232 and IT87 use official fallback
functions instead.
Create fallback_shutdown().
Create fallback_chip_writeb().
Convert the programmer #defines to an enum.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Reinauer - 2009-08-12 09:29:45
On 8/11/09 3:12 AM, Carl-Daniel Hailfinger wrote:
> FT2232 and IT87 programmers used functions of the dummy programmer
> instead of fallback functions. The dummy programmer is a "real"
> programmer with possible side effects and its functions should not be
> abused by other programmers. Make FT2232 and IT87 use official fallback
> functions instead.
> Create fallback_shutdown().
> Create fallback_chip_writeb().
> Convert the programmer #defines to an enum.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>   

We should drop the dummy programmer then... It's good for nothing

Acked-by: Stefan Reinauer <stepan@coresystems.de>

> Index: flashrom-fallback/flash.h
> ===================================================================
> --- flashrom-fallback/flash.h	(Revision 674)
> +++ flashrom-fallback/flash.h	(Arbeitskopie)
> @@ -80,15 +80,18 @@
>  
>  typedef unsigned long chipaddr;
>  
> -extern int programmer;
> -#define PROGRAMMER_INTERNAL	0x00
> -#define PROGRAMMER_DUMMY	0x01
> -#define PROGRAMMER_NIC3COM	0x02
> -#define PROGRAMMER_SATASII	0x03
> -#define PROGRAMMER_IT87SPI	0x04
> -#define PROGRAMMER_FT2232SPI	0x05
> -#define PROGRAMMER_SERPROG	0x06
> +enum programmer {
> +	PROGRAMMER_INTERNAL,
> +	PROGRAMMER_DUMMY,
> +	PROGRAMMER_NIC3COM,
> +	PROGRAMMER_SATASII,
> +	PROGRAMMER_IT87SPI,
> +	PROGRAMMER_FT2232SPI,
> +	PROGRAMMER_SERPROG,
> +};
>  
> +extern enum programmer programmer;
> +
>  struct programmer_entry {
>  	const char *vendor;
>  	const char *name;
> @@ -325,8 +328,10 @@
>  uint16_t mmio_readw(void *addr);
>  uint32_t mmio_readl(void *addr);
>  void internal_delay(int usecs);
> +int fallback_shutdown(void);
>  void *fallback_map(const char *descr, unsigned long phys_addr, size_t len);
>  void fallback_unmap(void *virt_addr, size_t len);
> +void fallback_chip_writeb(uint8_t val, chipaddr addr);
>  void fallback_chip_writew(uint16_t val, chipaddr addr);
>  void fallback_chip_writel(uint32_t val, chipaddr addr);
>  void fallback_chip_writen(uint8_t *buf, chipaddr addr, size_t len);
> @@ -375,7 +380,6 @@
>  int ft2232_spi_init(void);
>  int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
>  int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
> -int ft2232_spi_write1(struct flashchip *flash, uint8_t *buf);
>  int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf);
>  
>  /* flashrom.c */
> Index: flashrom-fallback/flashrom.c
> ===================================================================
> --- flashrom-fallback/flashrom.c	(Revision 674)
> +++ flashrom-fallback/flashrom.c	(Arbeitskopie)
> @@ -33,7 +33,7 @@
>  const char *flashrom_version = FLASHROM_VERSION;
>  char *chip_to_probe = NULL;
>  int verbose = 0;
> -int programmer = PROGRAMMER_INTERNAL;
> +enum programmer programmer = PROGRAMMER_INTERNAL;
>  
>  const struct programmer_entry programmer_table[] = {
>  	{
> @@ -102,14 +102,14 @@
>  
>  	{
>  		.init			= it87spi_init,
> -		.shutdown		= dummy_shutdown,
> -		.map_flash_region	= dummy_map,
> -		.unmap_flash_region	= dummy_unmap,
> +		.shutdown		= fallback_shutdown,
> +		.map_flash_region	= fallback_map,
> +		.unmap_flash_region	= fallback_unmap,
>  		.chip_readb		= dummy_chip_readb,
>  		.chip_readw		= fallback_chip_readw,
>  		.chip_readl		= fallback_chip_readl,
>  		.chip_readn		= fallback_chip_readn,
> -		.chip_writeb		= dummy_chip_writeb,
> +		.chip_writeb		= fallback_chip_writeb,
>  		.chip_writew		= fallback_chip_writew,
>  		.chip_writel		= fallback_chip_writel,
>  		.chip_writen		= fallback_chip_writen,
> @@ -118,19 +118,20 @@
>  
>  	{
>  		.init			= ft2232_spi_init,
> -		.shutdown		= dummy_shutdown,
> -		.map_flash_region	= dummy_map,
> -		.unmap_flash_region	= dummy_unmap,
> +		.shutdown		= fallback_shutdown,
> +		.map_flash_region	= fallback_map,
> +		.unmap_flash_region	= fallback_unmap,
>  		.chip_readb		= dummy_chip_readb,
>  		.chip_readw		= fallback_chip_readw,
>  		.chip_readl		= fallback_chip_readl,
>  		.chip_readn		= fallback_chip_readn,
> -		.chip_writeb		= dummy_chip_writeb,
> +		.chip_writeb		= fallback_chip_writeb,
>  		.chip_writew		= fallback_chip_writew,
>  		.chip_writel		= fallback_chip_writel,
>  		.chip_writen		= fallback_chip_writen,
>  		.delay			= internal_delay,
>  	},
> +
>  	{
>  		.init			= serprog_init,
>  		.shutdown		= serprog_shutdown,
> Index: flashrom-fallback/internal.c
> ===================================================================
> --- flashrom-fallback/internal.c	(Revision 674)
> +++ flashrom-fallback/internal.c	(Arbeitskopie)
> @@ -218,6 +218,12 @@
>  	}
>  }
>  
> +/* Fallback shutdown() for programmers which don't need special handling */
> +int fallback_shutdown(void)
> +{
> +	return 0;
> +}
> +
>  /* Fallback map() for programmers which don't need special handling */
>  void *fallback_map(const char *descr, unsigned long phys_addr, size_t len)
>  {
> @@ -229,6 +235,11 @@
>  {
>  }
>  
> +/* No-op fallback for drivers not supporting addr/data pair accesses */
> +void fallback_chip_writeb(uint8_t val, chipaddr addr)
> +{
> +}
> +
>  /* Little-endian fallback for drivers not supporting 16 bit accesses */
>  void fallback_chip_writew(uint16_t val, chipaddr addr)
>  {
>
>
>
Carl-Daniel Hailfinger - 2009-08-12 11:43:07
On 12.08.2009 11:29, Stefan Reinauer wrote:
> On 8/11/09 3:12 AM, Carl-Daniel Hailfinger wrote:
>   
>> FT2232 and IT87 programmers used functions of the dummy programmer
>> instead of fallback functions. The dummy programmer is a "real"
>> programmer with possible side effects and its functions should not be
>> abused by other programmers. Make FT2232 and IT87 use official fallback
>> functions instead.
>> Create fallback_shutdown().
>> Create fallback_chip_writeb().
>> Convert the programmer #defines to an enum.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>  
>>     
>
> We should drop the dummy programmer then... It's good for nothing
>   

It's essential for four reasons:
- Testing code flow as normal user without hardware for verification.
- Nondestructive experiments with flashrom.
- Machine and human readable flash access log, helped me find all those
finctions which copy probe_jedec instead of calling it, and it also
helped me find the incorrect programming sequence for 82802AB.
- Template to base new driver code on.

All the four use cases above were already seen in practice, and I'm not
the only one using them.

> Acked-by: Stefan Reinauer <stepan@coresystems.de>
>   

Thanks, r678.

Regards,
Carl-Daniel

Patch

Index: flashrom-fallback/flash.h
===================================================================
--- flashrom-fallback/flash.h	(Revision 674)
+++ flashrom-fallback/flash.h	(Arbeitskopie)
@@ -80,15 +80,18 @@ 
 
 typedef unsigned long chipaddr;
 
-extern int programmer;
-#define PROGRAMMER_INTERNAL	0x00
-#define PROGRAMMER_DUMMY	0x01
-#define PROGRAMMER_NIC3COM	0x02
-#define PROGRAMMER_SATASII	0x03
-#define PROGRAMMER_IT87SPI	0x04
-#define PROGRAMMER_FT2232SPI	0x05
-#define PROGRAMMER_SERPROG	0x06
+enum programmer {
+	PROGRAMMER_INTERNAL,
+	PROGRAMMER_DUMMY,
+	PROGRAMMER_NIC3COM,
+	PROGRAMMER_SATASII,
+	PROGRAMMER_IT87SPI,
+	PROGRAMMER_FT2232SPI,
+	PROGRAMMER_SERPROG,
+};
 
+extern enum programmer programmer;
+
 struct programmer_entry {
 	const char *vendor;
 	const char *name;
@@ -325,8 +328,10 @@ 
 uint16_t mmio_readw(void *addr);
 uint32_t mmio_readl(void *addr);
 void internal_delay(int usecs);
+int fallback_shutdown(void);
 void *fallback_map(const char *descr, unsigned long phys_addr, size_t len);
 void fallback_unmap(void *virt_addr, size_t len);
+void fallback_chip_writeb(uint8_t val, chipaddr addr);
 void fallback_chip_writew(uint16_t val, chipaddr addr);
 void fallback_chip_writel(uint32_t val, chipaddr addr);
 void fallback_chip_writen(uint8_t *buf, chipaddr addr, size_t len);
@@ -375,7 +380,6 @@ 
 int ft2232_spi_init(void);
 int ft2232_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr);
 int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int len);
-int ft2232_spi_write1(struct flashchip *flash, uint8_t *buf);
 int ft2232_spi_write_256(struct flashchip *flash, uint8_t *buf);
 
 /* flashrom.c */
Index: flashrom-fallback/flashrom.c
===================================================================
--- flashrom-fallback/flashrom.c	(Revision 674)
+++ flashrom-fallback/flashrom.c	(Arbeitskopie)
@@ -33,7 +33,7 @@ 
 const char *flashrom_version = FLASHROM_VERSION;
 char *chip_to_probe = NULL;
 int verbose = 0;
-int programmer = PROGRAMMER_INTERNAL;
+enum programmer programmer = PROGRAMMER_INTERNAL;
 
 const struct programmer_entry programmer_table[] = {
 	{
@@ -102,14 +102,14 @@ 
 
 	{
 		.init			= it87spi_init,
-		.shutdown		= dummy_shutdown,
-		.map_flash_region	= dummy_map,
-		.unmap_flash_region	= dummy_unmap,
+		.shutdown		= fallback_shutdown,
+		.map_flash_region	= fallback_map,
+		.unmap_flash_region	= fallback_unmap,
 		.chip_readb		= dummy_chip_readb,
 		.chip_readw		= fallback_chip_readw,
 		.chip_readl		= fallback_chip_readl,
 		.chip_readn		= fallback_chip_readn,
-		.chip_writeb		= dummy_chip_writeb,
+		.chip_writeb		= fallback_chip_writeb,
 		.chip_writew		= fallback_chip_writew,
 		.chip_writel		= fallback_chip_writel,
 		.chip_writen		= fallback_chip_writen,
@@ -118,19 +118,20 @@ 
 
 	{
 		.init			= ft2232_spi_init,
-		.shutdown		= dummy_shutdown,
-		.map_flash_region	= dummy_map,
-		.unmap_flash_region	= dummy_unmap,
+		.shutdown		= fallback_shutdown,
+		.map_flash_region	= fallback_map,
+		.unmap_flash_region	= fallback_unmap,
 		.chip_readb		= dummy_chip_readb,
 		.chip_readw		= fallback_chip_readw,
 		.chip_readl		= fallback_chip_readl,
 		.chip_readn		= fallback_chip_readn,
-		.chip_writeb		= dummy_chip_writeb,
+		.chip_writeb		= fallback_chip_writeb,
 		.chip_writew		= fallback_chip_writew,
 		.chip_writel		= fallback_chip_writel,
 		.chip_writen		= fallback_chip_writen,
 		.delay			= internal_delay,
 	},
+
 	{
 		.init			= serprog_init,
 		.shutdown		= serprog_shutdown,
Index: flashrom-fallback/internal.c
===================================================================
--- flashrom-fallback/internal.c	(Revision 674)
+++ flashrom-fallback/internal.c	(Arbeitskopie)
@@ -218,6 +218,12 @@ 
 	}
 }
 
+/* Fallback shutdown() for programmers which don't need special handling */
+int fallback_shutdown(void)
+{
+	return 0;
+}
+
 /* Fallback map() for programmers which don't need special handling */
 void *fallback_map(const char *descr, unsigned long phys_addr, size_t len)
 {
@@ -229,6 +235,11 @@ 
 {
 }
 
+/* No-op fallback for drivers not supporting addr/data pair accesses */
+void fallback_chip_writeb(uint8_t val, chipaddr addr)
+{
+}
+
 /* Little-endian fallback for drivers not supporting 16 bit accesses */
 void fallback_chip_writew(uint16_t val, chipaddr addr)
 {