Patchwork patch Dynamic list Multiple chips

login
register
about
Submitter Николай Николаев
Date 2013-02-28 12:21:56
Message ID <CABSK2pkUFytBe5c2_nfAPALBzFEzrZHoB0DQXz6NhjdhTJm=cg@mail.gmail.com>
Download mbox | patch
Permalink /patch/3869/
State Superseded
Headers show

Comments

Николай Николаев - 2013-02-28 12:21:56
-- 
With best regards Nikolay Nikolaev
С Уважением Николаев Николай
commit 1a62a382170eb72f1337c4b73c3b991ab1448907
Author: Nikolay Nikolaev <evrinoma@gmail.com>
Date:   Thu Feb 28 15:50:03 2013 +0400

    If we detected Multiple flash chips than
    we can easy save all chips in dynamic list - pflashes.
    and we will not have a restrictions with number chips
    
    TODO:
    Sort Dynamic List by Volage. This chips has a equivalent the model_id
    but can have a different parameters like a name voltage probe function
    ...
    Probing
    ...
    Resolving chips names
    
    Signed-off-by:Nikolay Nikolaev evrinoma@gmail.com
Stefan Tauner - 2013-02-28 12:52:18
On Thu, 28 Feb 2013 16:21:56 +0400
Николай Николаев <evrinoma@gmail.com> wrote:

>     If we detected Multiple flash chips than
>     we can easy save all chips in dynamic list - pflashes.
>     and we will not have a restrictions with number chips
>     
>     TODO:
>     Sort Dynamic List by Volage. This chips has a equivalent the model_id
>     but can have a different parameters like a name voltage probe function
>     ...
>     Probing
>     ...
>     Resolving chips names

Hi,

thanks for the patch. Please do not leave the previous code as comments
in the patch. This makes it very unreadable and not directly applicable.
I have not looked at the changes in detail, but I did also not like the
limitation, so in general I like the idea.
I would like to continue to use "flashes" as the name of the
collection though (why the change?).

Eventually I would also like to see a general list implementation in
flashrom, maybe similar to something in the Linux kernel, because it
would really make life easier in some cases (like this one).
Николай Николаев - 2013-03-01 08:16:18
Hi
Why do you need to implement dynamic lists like a Linux kernel?
Stefan Tauner - 2013-03-01 19:12:30
On Fri, 1 Mar 2013 12:16:18 +0400
Николай Николаев <evrinoma@gmail.com> wrote:

> Hi
> Why do you need to implement dynamic lists like a Linux kernel?

I dont *need* to do it *exactly* as in the linux kernel, but it would
be good to have a generic implementation that allows common operations
and that can be tested... there is no reason to reinvent the wheel and
introduce bugs over and over again. :)

Can you please resend the patch without the superfluous comments?
Николай Николаев - 2013-03-02 05:42:51
hi           8)
Can you please resend the patch without the superfluous comments?

yes of course i will send him at next week
i want to improve them to add in structure dynamic list a pointer to header
flashes and i want to write function sort by voltage
Stefan Tauner - 2013-03-02 23:11:20
On Sat, 2 Mar 2013 09:42:51 +0400
Николай Николаев <evrinoma@gmail.com> wrote:

> hi           8)
> Can you please resend the patch without the superfluous comments?
> 
> yes of course i will send him at next week
> i want to improve them to add in structure dynamic list a pointer to header
> flashes and i want to write function sort by voltage
> 

Ok, thanks!
Please split those changes into multiple patches and don't send one
single patch then.

Patch

diff --git a/cli_classic.c b/cli_classic.c
index 14fb825..cb6c736 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -94,8 +94,11 @@  int main(int argc, char *argv[])
 	unsigned long size;
 	/* Probe for up to three flash chips. */
 	const struct flashchip *chip = NULL;
-	struct flashctx flashes[3] = {{0}};
-	struct flashctx *fill_flash;
+        struct flashctx flashes = {0};
+        struct flashctx *pflashes = NULL;
+        struct flashctx *paddress = NULL;
+/*	struct flashctx flashes[3] = {{0}};
+	struct flashctx *fill_flash;*/
 	const char *name;
 	int namelen, opt, i, j;
 	int startchip = -1, chipcount = 0, option_index = 0, force = 0;
@@ -425,19 +428,48 @@  int main(int argc, char *argv[])
 
 	for (j = 0; j < registered_programmer_count; j++) {
 		startchip = 0;
-		while (chipcount < ARRAY_SIZE(flashes)) {
+                while (1){
+                        if (chip_to_probe && chipcount) break;
+                        startchip = probe_flash(&registered_programmers[j], startchip, &flashes, 0);
+			if (startchip == -1){
+                            break;
+                        }
+                        paddress = pflashes;                        
+                        pflashes = calloc(1, sizeof(struct flashctx));
+                        if (!pflashes) {
+                                msg_gerr("Out of memory!\n");
+                                exit(1);
+                        }
+                        memcpy(pflashes, &flashes, sizeof(struct flashctx));                       
+                        pflashes->last=paddress;
+                        pflashes->next=NULL;
+                        if (paddress!=NULL) {
+                            paddress->next=pflashes;
+                        }
+                        
+			chipcount++;    
+			startchip++;
+                }                
+		/*while (chipcount < ARRAY_SIZE(flashes)) {
 			startchip = probe_flash(&registered_programmers[j], startchip, &flashes[chipcount], 0);
 			if (startchip == -1)
 				break;
 			chipcount++;
 			startchip++;
-		}
+		}*/
 	}
 
 	if (chipcount > 1) {
-		msg_cinfo("Multiple flash chips were detected: \"%s\"", flashes[0].chip->name);
+                msg_cinfo("Multiple flash chips were detected: \n");
+                paddress=pflashes;                
+		while(pflashes!=NULL){                        
+                        (pflashes->last==NULL)?msg_cinfo("\"%s\"", pflashes->chip->name):msg_cinfo("\"%s\",", pflashes->chip->name);
+                        pflashes=pflashes->last;                        
+                }                
+                pflashes=paddress;                
+	/*	msg_cinfo("Multiple flash chips were detected: \"%s\"", flashes[0].chip->name);
 		for (i = 1; i < chipcount; i++)
-			msg_cinfo(", \"%s\"", flashes[i].chip->name);
+			msg_cinfo(", \"%s\"", flashes[i].chip->name);*/
 		msg_cinfo("\nPlease specify which chip to use with the -c <chipname> option.\n");
 		ret = 1;
 		goto out_shutdown;
@@ -468,7 +500,8 @@  int main(int argc, char *argv[])
 					  "chip, using the first one.\n");
 			for (j = 0; j < registered_programmer_count; j++) {
 				pgm = &registered_programmers[j];
-				startchip = probe_flash(pgm, 0, &flashes[0], 1);
+/*				startchip = probe_flash(pgm, 0, &flashes[0], 1);*/
+                                startchip = probe_flash(pgm, 0, pflashes, 1);
 				if (startchip != -1)
 					break;
 			}
@@ -479,20 +512,25 @@  int main(int argc, char *argv[])
 				goto out_shutdown;
 			}
 			msg_cinfo("Please note that forced reads most likely contain garbage.\n");
-			ret = read_flash_to_file(&flashes[0], filename);
-			free(flashes[0].chip);
+/*			ret = read_flash_to_file(&flashes[0], filename);
+			free(flashes[0].chip);*/
+                        ret = read_flash_to_file(pflashes, filename);
+			free(pflashes->chip);
 			goto out_shutdown;
 		}
 		ret = 1;
 		goto out_shutdown;
 	} else if (!chip_to_probe) {
 		/* repeat for convenience when looking at foreign logs */
-		tempstr = flashbuses_to_text(flashes[0].chip->bustype);
+/*		tempstr = flashbuses_to_text(flashes[0].chip->bustype);
+		msg_gdbg("Found %s flash chip \"%s\" (%d kB, %s).\n",
+			 flashes[0].chip->vendor, flashes[0].chip->name, flashes[0].chip->total_size, tempstr);*/
+                tempstr = flashbuses_to_text(pflashes->chip->bustype);    
 		msg_gdbg("Found %s flash chip \"%s\" (%d kB, %s).\n",
-			 flashes[0].chip->vendor, flashes[0].chip->name, flashes[0].chip->total_size, tempstr);
+			 pflashes->chip->vendor, pflashes->chip->name, pflashes->chip->total_size, tempstr);
 		free(tempstr);
 	}
-
+        /*
 	fill_flash = &flashes[0];
 
 	check_chip_supported(fill_flash->chip);
@@ -502,8 +540,17 @@  int main(int argc, char *argv[])
 		msg_cerr("Chip is too big for this programmer (-V gives details). Use --force to override.\n");
 		ret = 1;
 		goto out_shutdown;
-	}
+	}*/
+        
+        check_chip_supported(pflashes->chip);
 
+	size = pflashes->chip->total_size * 1024;
+	if (check_max_decode(pflashes->pgm->buses_supported & pflashes->chip->bustype, size) && (!force)) {
+		msg_cerr("Chip is too big for this programmer (-V gives details). Use --force to override.\n");
+		ret = 1;
+                goto out_shutdown;
+	}
+        
 	if (!(read_it | write_it | verify_it | erase_it)) {
 		msg_ginfo("No operations were specified.\n");
 		goto out_shutdown;
@@ -518,19 +565,22 @@  int main(int argc, char *argv[])
 	 * Give the chip time to settle.
 	 */
 	programmer_delay(100000);
-	ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it);
+/*	ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it);*/
+        ret |= doit(pflashes, force, filename, read_it, write_it, erase_it, verify_it);
 	/* Note: doit() already calls programmer_shutdown(). */
 	goto out;
 
 out_shutdown:
 	programmer_shutdown();
 out:
-	for (i = 0; i < chipcount; i++)
+/*	for (i = 0; i < chipcount; i++)
 		free(flashes[i].chip);
-
+*/
+        free(flashes.chip);
 	free(filename);
 	free(layoutfile);
 	free(pparam);
+        free(pflashes);
 	/* clean up global variables */
 	free((char *)chip_to_probe); /* Silence! Freeing is not modifying contents. */
 	chip_to_probe = NULL;
diff --git a/flash.h b/flash.h
index a479286..0b271f6 100644
--- a/flash.h
+++ b/flash.h
@@ -170,6 +170,9 @@  struct flashctx {
 	/* Some flash devices have an additional register space. */
 	chipaddr virtual_registers;
 	struct registered_programmer *pgm;
+        /*pointer dynamic list*/
+        struct flashctx *last;
+        struct flashctx *next;
 };
 
 #define TEST_UNTESTED	0