| Submitter | Carl-Daniel Hailfinger |
|---|---|
| Date | 2010-01-07 21:22:46 |
| Message ID | <4B4650A6.5030700@gmx.net> |
| Download | mbox | patch |
| Permalink | /patch/748/ |
| State | Accepted |
| Commit | r840 |
| Headers | show |
Comments
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 <audiohacked@gmail.com>
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 <audiohacked@gmail.com> Thanks, r840. Regards, Carl-Daniel
Patch
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)
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 <c-d.hailfinger.devel.2006@gmx.net>