[PATCH] Use address mask in probe_jedec

[Carl-Daniel Hailfinger]

Use address mask in probe_jedec. This allows us to have one common
probe_jedec function instead of half a dozen wrappers.
The trick here is to have FEATURE_ADDR_FULL==0 and thus default to
unmasked addresses. That way, we only have to annotate chips which need
small address masks.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-probe_jedec_featurebits_addrmask/flash.h
===================================================================
--- flashrom-probe_jedec_featurebits_addrmask/flash.h	(Revision 836)
+++ flashrom-probe_jedec_featurebits_addrmask/flash.h	(Arbeitskopie)
@@ -149,8 +149,10 @@
  */
 #define NUM_ERASEFUNCTIONS 5

-#define FEATURE_REGISTERMAP (1 << 0)
-#define FEATURE_BYTEWRITES (1 << 1)
+#define FEATURE_REGISTERMAP	(1 << 0)
+#define FEATURE_BYTEWRITES	(1 << 1)
+#define FEATURE_ADDR_FULL	(0 << 2)
+#define FEATURE_ADDR_MASK	(3 << 2)

struct flashchip {
    const char *vendor;
Index: flashrom-probe_jedec_featurebits_addrmask/jedec.c
===================================================================
--- flashrom-probe_jedec_featurebits_addrmask/jedec.c	(Revision 836)
+++ flashrom-probe_jedec_featurebits_addrmask/jedec.c	(Arbeitskopie)
@@ -373,6 +373,19 @@
    return failed;
 }

+int getaddrmask(struct flashchip *flash)
+{
+	switch (flash->feature_bits & FEATURE_ADDR_MASK) {
+	case FEATURE_ADDR_FULL:
+		return MASK_FULL;
+		break;
+	default:
+		fprintf(stderr, "%s called with unknown mask\n", __func__);
+		return 0;
+		break;
+	}
+}
+
 int write_jedec(struct flashchip *flash, uint8_t *buf)
 {
    int i, failed = 0;
@@ -438,7 +451,10 @@

int probe_jedec(struct flashchip *flash)
 {
-	return probe_jedec_common(flash, MASK_FULL, 1);
+	int mask;
+
+	mask = getaddrmask(flash);
+	return probe_jedec_common(flash, mask, 1);
 }

int erase_sector_jedec(struct flashchip *flash, unsigned int page, unsigned int size)

[Sean Nelson]

On 1/7/2010 1:22 PM, Carl-Daniel Hailfinger wrote:
> int probe_jedec(struct flashchip *flash)
>   {
> return probe_jedec_common(flash, MASK_FULL, 1);
>
> int mask;
>
> mask = getaddrmask(flash);
>
> return probe_jedec_common(flash, mask, 1);
> }
>
> int erase_sector_jedec(struct flashchip *flash, unsigned int page, unsigned int size)

Instead of having this wrapper, we could move getaddrmask into 
probe_jedec_common and revert the name of probe_jedec_common too.

But, this is fine.
Acked-by: Sean Nelson

[Carl-Daniel Hailfinger]

On 09.01.2010 01:44, Sean Nelson wrote:
> On 1/7/2010 1:22 PM, Carl-Daniel Hailfinger wrote:
> > int probe_jedec(struct flashchip *flash)
> >   {
> > return probe_jedec_common(flash, MASK_FULL, 1);
> >
> > int mask;
> >
> > mask = getaddrmask(flash);
> >
> > return probe_jedec_common(flash, mask, 1);
> > }
> >
> > int erase_sector_jedec(struct flashchip *flash, unsigned int page,
> >
> > unsigned int size)
>
> Instead of having this wrapper, we could move getaddrmask into
> probe_jedec_common and revert the name of probe_jedec_common too.

My idea was to allow semi-converted flash chip drivers (drivers which
should use the JEDEC stuff but are instead calling the _jedec_common
functions with hand-crafted masks). You do have a point, but I didn't
just want to waltz in and revert parts of your _jedec_common infrastructure.
> But, this is fine.
> Acked-by: Sean Nelson

Thanks, r840.

Regards,
Carl-Daniel