USB: Remove special case for iVersion string descriptor

Previously the version string was special cased in the USB stack
because the build system prevented the inclusion of ec_version.h in
any file other than common/version.c.  This lead to common/version.c
being the only place that the USB version string could be computed
and thus the special case of filling in the version string descriptor
at run time.  This made the USB stack more complex, and lead to the
common/version.c file including usb.h, which is actually STM32
specific.

Now, the portion of ec_version.h that is deterministic is only
updated when something in the tree actually changes (by way of a
conditional in the makefile), and ec_version.h no longer has to
depend on all object files (other than the special version.o).
This allows anyone to include ec_version.h as needed.  In particular,
each board that wants to define a USB version string can directly
include ec_version.h and do so.

Signed-off-by: Anton Staaf <robotboy@chromium.org>

BRANCH=None
BUG=None
TEST=make buildall -j
     touch files and verify rebuilds happen correctly

Change-Id: Ic84d0b9da90f82ebb4630fb550ec841071e25a49
Reviewed-on: https://chromium-review.googlesource.com/227211
Tested-by: Anton Staaf <robotboy@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Commit-Queue: Anton Staaf <robotboy@chromium.org>

11 files changed, 54 insertions, 22 deletions

diff --git a/Makefile.rules b/Makefile.rules
index 205c89a983..3a1b415ad3 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -52,6 +52,7 @@ cmd_c_to_build = $(BUILDCC) $(BUILD_CFLAGS) \
 cmd_c_to_host = $(HOSTCC) $(HOST_CFLAGS) -MMD -MF $@.d  -o $@ \
 	         $(sort $(foreach c,$($(*F)-objs),util/$(c:%.o=%.c)) $*.c)
 cmd_host_test = ./util/run_host_test $* $(silent)
+cmd_date = ./util/getdate.sh > $@
 cmd_version = ./util/getversion.sh > $@
 cmd_mv_from_tmp = mv $(out)/$*.bin.tmp $(out)/$*.bin
 cmd_extractrw-y = dd if=$(out)/$(PROJECT).bin.tmp of=$(out)/$(PROJECT).RW.bin \
@@ -179,10 +180,33 @@ $(out)/vboot/%.o:$(VBOOT_SOURCE)/%.c
 $(out)/%.o:%.S
 	$(call quiet,c_to_o,AS     )
 
-$(out)/common/version.o: $(out)/ec_version.h
-$(out)/ec_version.h: $(filter-out $(out)/common/version.o,$(objs))
+# Conditionally force the rebuilding of ec_version.h only if it would be
+# changed.
+old_version_hash := $(shell cat $(out)/ec_version.h 2> /dev/null | md5sum -)
+new_version_hash := $(shell BOARD=$(BOARD) ./util/getversion.sh | md5sum -)
+
+ifneq ($(old_version_hash),$(new_version_hash))
+.PHONY: $(out)/ec_version.h
+endif
+
+# All of the objects have an order only dependency on the ec_version header.
+# This ensures that if ec_version.h needs to be build (because it was marked
+# PHONY above) then it will be rebuilt before any objects.  This is important
+# because some source files will include ev_version.h and fail to compile if
+# it doesn't already exist.  This dependency shouldn't be a normal dependency
+# because that would cause every object to be rebuilt when ec_version.h
+# changes, instead of just the ones that actually depend on it.  The objects
+# that truly depend on ec_version.h will have that information encoded in their
+# .d file.
+$(objs): | $(out)/ec_version.h
+
+$(out)/ec_version.h:
 	$(call quiet,version,VERSION)
 
+$(out)/common/version.o: $(out)/ec_date.h
+$(out)/ec_date.h: $(filter-out $(out)/common/version.o,$(objs))
+	$(call quiet,date,DATE   )
+
 $(out)/gen_pub_key.h: $(PEM)
 	$(call quiet,pubkey,PUBKEY )
 
diff --git a/board/dingdong/board.c b/board/dingdong/board.c
index a6e1723a9b..867eb1a4ed 100644
--- a/board/dingdong/board.c
+++ b/board/dingdong/board.c
@@ -7,6 +7,7 @@
 #include "adc.h"
 #include "adc_chip.h"
 #include "common.h"
+#include "ec_version.h"
 #include "gpio.h"
 #include "hooks.h"
 #include "registers.h"
@@ -118,7 +119,7 @@ const void * const usb_strings[] = {
 	[USB_STR_DESC] = usb_string_desc,
 	[USB_STR_VENDOR] = USB_STRING_DESC("Google Inc."),
 	[USB_STR_PRODUCT] = USB_STRING_DESC("Dingdong"),
-	[USB_STR_VERSION] = NULL /* filled at runtime */,
+	[USB_STR_VERSION] = USB_STRING_DESC(CROS_EC_VERSION32),
 	[USB_STR_BB_URL] = USB_STRING_DESC(USB_GOOGLE_TYPEC_URL),
 };
 BUILD_ASSERT(ARRAY_SIZE(usb_strings) == USB_STR_COUNT);
diff --git a/board/discovery-stm32f072/board.c b/board/discovery-stm32f072/board.c
index 2d9d15005a..8beae33e74 100644
--- a/board/discovery-stm32f072/board.c
+++ b/board/discovery-stm32f072/board.c
@@ -5,6 +5,7 @@
 /* STM32F072-discovery board configuration */
 
 #include "common.h"
+#include "ec_version.h"
 #include "gpio.h"
 #include "hooks.h"
 #include "registers.h"
@@ -45,7 +46,7 @@ const void *const usb_strings[] = {
 	[USB_STR_DESC]         = usb_string_desc,
 	[USB_STR_VENDOR]       = USB_STRING_DESC("Google Inc."),
 	[USB_STR_PRODUCT]      = USB_STRING_DESC("discovery-stm32f072"),
-	[USB_STR_VERSION]      = NULL /* filled at runtime */,
+	[USB_STR_VERSION]      = USB_STRING_DESC(CROS_EC_VERSION32),
 	[USB_STR_STREAM_NAME]  = USB_STRING_DESC("Echo"),
 	[USB_STR_CONSOLE_NAME] = USB_STRING_DESC("Shell"),
 };
diff --git a/board/fruitpie/board.c b/board/fruitpie/board.c
index cd733c6c8f..f03d8546db 100644
--- a/board/fruitpie/board.c
+++ b/board/fruitpie/board.c
@@ -8,6 +8,7 @@
 #include "adc_chip.h"
 #include "common.h"
 #include "console.h"
+#include "ec_version.h"
 #include "gpio.h"
 #include "hooks.h"
 #include "i2c.h"
@@ -67,7 +68,7 @@ const void * const usb_strings[] = {
 	[USB_STR_DESC] = usb_string_desc,
 	[USB_STR_VENDOR] = USB_STRING_DESC("Google Inc."),
 	[USB_STR_PRODUCT] = USB_STRING_DESC("FruitPie"),
-	[USB_STR_VERSION] = NULL /* filled at runtime */,
+	[USB_STR_VERSION] = USB_STRING_DESC(CROS_EC_VERSION32),
 };
 BUILD_ASSERT(ARRAY_SIZE(usb_strings) == USB_STR_COUNT);
 
diff --git a/board/hoho/board.c b/board/hoho/board.c
index f7ce18da4b..56ad6af81e 100644
--- a/board/hoho/board.c
+++ b/board/hoho/board.c
@@ -7,6 +7,7 @@
 #include "adc.h"
 #include "adc_chip.h"
 #include "common.h"
+#include "ec_version.h"
 #include "gpio.h"
 #include "hooks.h"
 #include "i2c.h"
@@ -163,7 +164,7 @@ const void * const usb_strings[] = {
 	[USB_STR_DESC] = usb_string_desc,
 	[USB_STR_VENDOR] = USB_STRING_DESC("Google Inc."),
 	[USB_STR_PRODUCT] = USB_STRING_DESC("Hoho"),
-	[USB_STR_VERSION] = NULL /* filled at runtime */,
+	[USB_STR_VERSION] = USB_STRING_DESC(CROS_EC_VERSION32),
 	[USB_STR_BB_URL] = USB_STRING_DESC(USB_GOOGLE_TYPEC_URL),
 };
 BUILD_ASSERT(ARRAY_SIZE(usb_strings) == USB_STR_COUNT);
diff --git a/board/twinkie/board.c b/board/twinkie/board.c
index bfd7e078b9..efdf16d7b7 100644
--- a/board/twinkie/board.c
+++ b/board/twinkie/board.c
@@ -8,6 +8,7 @@
 #include "adc_chip.h"
 #include "common.h"
 #include "console.h"
+#include "ec_version.h"
 #include "gpio.h"
 #include "hooks.h"
 #include "i2c.h"
@@ -74,7 +75,7 @@ const void * const usb_strings[] = {
 	[USB_STR_DESC] = usb_string_desc,
 	[USB_STR_VENDOR] = USB_STRING_DESC("Google Inc."),
 	[USB_STR_PRODUCT] = USB_STRING_DESC("Twinkie"),
-	[USB_STR_VERSION] = NULL /* filled at runtime */,
+	[USB_STR_VERSION] = USB_STRING_DESC(CROS_EC_VERSION32),
 	[USB_STR_SNIFFER] = USB_STRING_DESC("USB-PD Sniffer"),
 	[USB_STR_CONSOLE_NAME] = USB_STRING_DESC("Shell"),
 };
diff --git a/chip/stm32/usb.c b/chip/stm32/usb.c
index 8cb01a002b..18193d4ffa 100644
--- a/chip/stm32/usb.c
+++ b/chip/stm32/usb.c
@@ -125,12 +125,6 @@ static void ep0_rx(void)
 			if (idx >= USB_STR_COUNT)
 				/* The string does not exist : STALL */
 				goto unknown_req;
-			if (idx == USB_STR_VERSION) {
-				/* use the generated firmware version string */
-				desc = usb_fw_version;
-				len = desc[0];
-				break;
-			}
 			desc = usb_strings[idx];
 			len = desc[0];
 			break;
diff --git a/common/version.c b/common/version.c
index a80d843580..b6cc8e1d8b 100644
--- a/common/version.c
+++ b/common/version.c
@@ -7,8 +7,8 @@
 
 #include <stdint.h>
 #include "common.h"
+#include "ec_date.h"
 #include "ec_version.h"
-#include "usb.h"
 #include "version.h"
 
 const struct version_struct version_data
@@ -21,11 +21,6 @@ const struct version_struct version_data
 const char build_info[] __attribute__((section(".rodata.buildinfo")))  =
 	CROS_EC_VERSION " " DATE " " BUILDER;
 
-#ifdef CONFIG_USB
-/* UTF-16 encoded USB string descriptor */
-const void * const usb_fw_version = USB_STRING_DESC(CROS_EC_VERSION32);
-#endif
-
 uint32_t ver_get_numcommits(void)
 {
 	int i;
diff --git a/include/usb.h b/include/usb.h
index 65dc8236f4..0b108b19fc 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -228,9 +228,12 @@ struct usb_endpoint_descriptor {
 /* primitive to access the words in USB RAM */
 #if defined(CHIP_FAMILY_STM32F0) || defined(CHIP_FAMILY_STM32F3)
 typedef uint16_t usb_uint;
+#elif defined(CHIP_FAMILY_STM32F) || defined(CHIP_FAMILY_STM32L)
+typedef uint16_t usb_uint;
+#elif defined(CHIP_HOST)
+typedef unsigned int usb_uint;
 #else
-/* older chips use a weird addressing were 16-bit words are 32-bit appart */
-typedef uint32_t usb_uint;
+#warn "usb_uint not defined for this chip family"
 #endif
 
 struct stm32_endpoint {
diff --git a/util/getdate.sh b/util/getdate.sh
new file mode 100755
index 0000000000..8099b398e8
--- /dev/null
+++ b/util/getdate.sh
@@ -0,0 +1,12 @@
+#!/bin/bash
+#
+# Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+#
+# Generate build date information for the EC binary
+
+echo "/* This file is generated by util/getdate.sh */"
+
+echo "/* DATE is used to form build info string in common/version.c. */"
+echo "#define DATE \"$(date '+%F %T')\""
diff --git a/util/getversion.sh b/util/getversion.sh
index 4d1931d4a2..c1da044690 100755
--- a/util/getversion.sh
+++ b/util/getversion.sh
@@ -46,5 +46,4 @@ echo "#define CROS_EC_VERSION32 \"${ver:0:31}\""
 echo "/* Sub-fields for use in Makefile.rules and to form build info string"
 echo " * in common/version.c. */"
 echo "#define VERSION \"${ver}\""
-echo "#define DATE \"`date '+%F %T'`\""
 echo "#define BUILDER \"${USER}@`hostname`\""