On 11/03/2015 09:13 AM, Steven Kipisz wrote:
On 11/03/2015 07:16 AM, Igor Grinberg wrote:
Hi Steve,
On 11/03/15 14:22, Steve Kipisz wrote:
From: Lokesh Vutla lokeshvutla@ti.com
[...]
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com Signed-off-by: Steve Kipisz s-kipisz2@ti.com
v2 Based on master a6104737 ARM: at91: sama5: change the environment address to 0x6000
Changes in v2 (since v1) - make the EEPROM code mor generic for TI EVMs - rename structures/subroutines to ti_am_xxxxx - add routines to access the EEPROM data - redo commit message to be more clear
v1: http://marc.info/?t=144608007900001&r=1&w=2 (mailing list squashed original submission)
arch/arm/cpu/armv7/omap-common/Makefile | 1 + arch/arm/cpu/armv7/omap-common/ti-i2c-eeprom.c | 148 +++++++++++++++++++++++++ arch/arm/include/asm/omap_common.h | 130 +++++++++++++++++++++- 3 files changed, 278 insertions(+), 1 deletion(-) create mode 100644 arch/arm/cpu/armv7/omap-common/ti-i2c-eeprom.c
diff --git a/arch/arm/cpu/armv7/omap-common/Makefile b/arch/arm/cpu/armv7/omap-common/Makefile index 464a5d1d732a..53a9fdb81100 100644 --- a/arch/arm/cpu/armv7/omap-common/Makefile +++ b/arch/arm/cpu/armv7/omap-common/Makefile @@ -15,6 +15,7 @@ obj-y += clocks-common.o obj-y += emif-common.o obj-y += vc.o obj-y += abb.o +obj-$(CONFIG_I2C) += ti-i2c-eeprom.o
This makes this module compile on all TI SoC based boards enabling I2C. AFAIU, this is a separate chip (not inside the SoC), so this module will also compile on non-TI boards that do not have this EEPROM. I think, it should be more fine grained (e.g. have its own symbol).
Can you give a suggestion?
Are you sure this will be built into non-ti SoCs with I2C enabled if you are not using the function? I assume __maybe_unused should take care of that, no - let the compiler do the gc anyways?
It should have been documented on commit message as well.
- @revision: Revision of the board
- @serial: Serial Number of the board
- In case of NULL revision or serial information "unknown" is setup.
- If name is NULL, default_name is used.
- */
+void set_board_info_env(char *name, char *default_name,
char *revision, char *serial);+/**
- board_am_is() - Generic Board detection logic
- @name_tag: Tag used in eeprom for the board
- Return: false if board information does not match OR eeprom
was'nt read.
true otherwise- */
+static inline bool board_am_is(char *name_tag) +{
- struct ti_am_eeprom *ep = TI_AM_EEPROM_DATA;
- if (ep->header != TI_EEPROM_HEADER_MAGIC)
return false;- return !strncmp(ep->name, name_tag, TI_EEPROM_HDR_NAME_LEN);
+}
Why do you need to place non trivial function implementation inside the header file?
+/**
- board_am_rev_is() - Compare board revision
- @rev_tag: Revision tag to check in eeprom
- @cmp_len: How many chars to compare?
- NOTE: revision information is often messed up (hence the str len
match) :(
- Return: false if board information does not match OR eeprom
was'nt read.
true otherwise- */
+static inline bool board_am_rev_is(char *rev_tag, int cmp_len) +{
- struct ti_am_eeprom *ep = TI_AM_EEPROM_DATA;
- int l;
- if (ep->header != TI_EEPROM_HEADER_MAGIC)
return false;- l = cmp_len > TI_EEPROM_HDR_REV_LEN ? TI_EEPROM_HDR_NAME_LEN :
cmp_len;
- return !strncmp(ep->version, rev_tag, l);
+}
Same here.
I thought by making them static inline would save space.
I prefer that myself as well.