| Message ID | 20200218083843.3855-1-rasmus.villemoes@prevas.dk |
|---|---|
| State | Accepted |
| Commit | 682fef9ff6b464602b35e4fcc0cca83568ad2ffa |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | include/eeprom.h: fix build errors | expand |
On Tue, Feb 18, 2020 at 08:39:42AM +0000, Rasmus Villemoes wrote: > CMD_EEPROM and ENV_IS_IN_EEPROM can be selected independently, and > cmd/eeprom.o gets built in either case, so whether to declare the real > prototypes needs to follow the same logic as whether cmd/eeprom.c is > built. Otherwise a ENV_IS_IN_EEPROM=y, CMD_EEPROM=n build fails > > cmd/eeprom.c:73:1: error: expected identifier or ‘(’ before ‘{’ token > { > > While at it, fix the dummy replacements (at least assuming they are > meant to allow the code to compile) - they need to have the same type > as the expression they replace, or one gets errors such as > > env/eeprom.c: In function ‘eeprom_bus_read’: > env/eeprom.c:37:8: error: void value not ignored as it ought to be > rcode = eeprom_read(dev_addr, offset, buffer, cnt); > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > --- > include/eeprom.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/eeprom.h b/include/eeprom.h > index 79118eb83d..6820844cea 100644 > --- a/include/eeprom.h > +++ b/include/eeprom.h > @@ -7,7 +7,7 @@ > #ifndef __EEPROM_LEGACY_H > #define __EEPROM_LEGACY_H > > -#ifdef CONFIG_CMD_EEPROM > +#if defined(CONFIG_CMD_EEPROM) || defined(CONFIG_ENV_IS_IN_EEPROM) > void eeprom_init(int bus); > int eeprom_read(uint dev_addr, uint offset, uchar *buffer, uint cnt); > int eeprom_write(uint dev_addr, uint offset, uchar *buffer, uint cnt); > @@ -17,8 +17,8 @@ int eeprom_write(uint dev_addr, uint offset, uchar *buffer, uint cnt); > * some macros here so we don't have to touch every one of those uses > */ > #define eeprom_init(bus) > -#define eeprom_read(dev_addr, offset, buffer, cnt) ((void)-ENOSYS) > -#define eeprom_write(dev_addr, offset, buffer, cnt) ((void)-ENOSYS) > +#define eeprom_read(dev_addr, offset, buffer, cnt) (-ENOSYS) > +#define eeprom_write(dev_addr, offset, buffer, cnt) (-ENOSYS) > #endif > > #if !defined(CONFIG_ENV_EEPROM_IS_ON_I2C) && defined(CONFIG_SYS_I2C_EEPROM_ADDR) Reviewed-by: Tom Rini <trini@konsulko.com> But while you're in here, can you make a follow-up patch that splits the common parts of cmd/eeprom.c out and in to common/eeprom/eeprom.c or something better named? Thanks!
On Tue, Feb 18, 2020 at 08:39:42AM +0000, Rasmus Villemoes wrote: > CMD_EEPROM and ENV_IS_IN_EEPROM can be selected independently, and > cmd/eeprom.o gets built in either case, so whether to declare the real > prototypes needs to follow the same logic as whether cmd/eeprom.c is > built. Otherwise a ENV_IS_IN_EEPROM=y, CMD_EEPROM=n build fails > > cmd/eeprom.c:73:1: error: expected identifier or ‘(’ before ‘{’ token > { > > While at it, fix the dummy replacements (at least assuming they are > meant to allow the code to compile) - they need to have the same type > as the expression they replace, or one gets errors such as > > env/eeprom.c: In function ‘eeprom_bus_read’: > env/eeprom.c:37:8: error: void value not ignored as it ought to be > rcode = eeprom_read(dev_addr, offset, buffer, cnt); > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > Reviewed-by: Tom Rini <trini@konsulko.com> [I had asked for a follow-up cleanup, but this is still worth taking, so...] Applied to u-boot/master, thanks!
On 07/05/2020 15.03, Tom Rini wrote: > On Tue, Feb 18, 2020 at 08:39:42AM +0000, Rasmus Villemoes wrote: > >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> >> Reviewed-by: Tom Rini <trini@konsulko.com> > > [I had asked for a follow-up cleanup, but this is still worth taking, > so...] Yeah, sorry, been rather busy, dunno when or if I'll find time to look at that extra cleanup. Thanks, Rasmus
diff --git a/include/eeprom.h b/include/eeprom.h index 79118eb83d..6820844cea 100644 --- a/include/eeprom.h +++ b/include/eeprom.h @@ -7,7 +7,7 @@ #ifndef __EEPROM_LEGACY_H #define __EEPROM_LEGACY_H -#ifdef CONFIG_CMD_EEPROM +#if defined(CONFIG_CMD_EEPROM) || defined(CONFIG_ENV_IS_IN_EEPROM) void eeprom_init(int bus); int eeprom_read(uint dev_addr, uint offset, uchar *buffer, uint cnt); int eeprom_write(uint dev_addr, uint offset, uchar *buffer, uint cnt); @@ -17,8 +17,8 @@ int eeprom_write(uint dev_addr, uint offset, uchar *buffer, uint cnt); * some macros here so we don't have to touch every one of those uses */ #define eeprom_init(bus) -#define eeprom_read(dev_addr, offset, buffer, cnt) ((void)-ENOSYS) -#define eeprom_write(dev_addr, offset, buffer, cnt) ((void)-ENOSYS) +#define eeprom_read(dev_addr, offset, buffer, cnt) (-ENOSYS) +#define eeprom_write(dev_addr, offset, buffer, cnt) (-ENOSYS) #endif #if !defined(CONFIG_ENV_EEPROM_IS_ON_I2C) && defined(CONFIG_SYS_I2C_EEPROM_ADDR)
CMD_EEPROM and ENV_IS_IN_EEPROM can be selected independently, and cmd/eeprom.o gets built in either case, so whether to declare the real prototypes needs to follow the same logic as whether cmd/eeprom.c is built. Otherwise a ENV_IS_IN_EEPROM=y, CMD_EEPROM=n build fails cmd/eeprom.c:73:1: error: expected identifier or ‘(’ before ‘{’ token { While at it, fix the dummy replacements (at least assuming they are meant to allow the code to compile) - they need to have the same type as the expression they replace, or one gets errors such as env/eeprom.c: In function ‘eeprom_bus_read’: env/eeprom.c:37:8: error: void value not ignored as it ought to be rcode = eeprom_read(dev_addr, offset, buffer, cnt); Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> --- include/eeprom.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)