Hi Heinrich, Alex,
On Tue, Sep 04, 2018 at 12:46:58PM +0200, Heinrich Schuchardt wrote:
On 09/04/2018 10:52 AM, Alexander Graf wrote:
On 04.09.18 09:49, AKASHI Takahiro wrote:
The whole content of include/fat.h is private to FAT implementation and then should be guarded with CONFIG_FS_FAT.
Hello Takahiro,
doesn't this imply that building common/spl/spl_sata.c without FAT will fail for CONFIG_SPL_SATA_SUPPORT=y (e.g. cm_t54_defconfig with FAT disabled).
Yes, you're right, but this is a matter of cm_t54_defconfig. spl_sta.c uses spl_load_image_fat[_os]() which are implemented in spl_fat.c which replies on CONFIG_SPL_FAT_SUPPORT, which will in turn select CONFIG_FS_FAT. The problem is that cm_t54_defconfig doesn't enable CONFIG_SPL_FAT_SUPPORT.
To fix this issue, we should add "depends on SPL_FAT_SUPPORT" to SPL_SATA_SUPPORT (and SPL_USB_SUPPORT) as well.
Did you run Travis CI on your patch series?
No. Is everybody required to run Travis CI for u-boot before any submission?
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
include/fat.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/fat.h b/include/fat.h index 09e142368585..c02839dcb040 100644 --- a/include/fat.h +++ b/include/fat.h @@ -9,6 +9,8 @@ #ifndef _FAT_H_ #define _FAT_H_
+#ifdef CONFIG_FS_FAT
Isn't this missing an include of at least common.h to actually propagate the config define?
Also, you want to use #if CONFIG_IS_ENABLED(...) here to guard against SPL builds too.
Probably not:
As I'm saying, CONFIG_SPL_FAT_SUPPORT selects CONFIG_FS_FAT and so either "ifdef CONFIG_FS_FAT" or "if CONFIG_IS_ENABLED(FS_FAT)" will turn into True although CONFIG_SPL_FS_FAT actually doesn't exist.
Therefore, I don't think we need a change Alex suggested.
Thanks, -Takahiro AKASHI
common/spl/spl_fat.c:14:#include <fat.h>
Best regards
Heinrich Schuchardt
However, I don't fully grasp why you need this patch. Please describe in your commit message what the incentive is for creating it. What errors are you trying to protect against?
Alex
#include <asm/byteorder.h> #include <fs.h>
@@ -202,4 +204,5 @@ int fat_opendir(const char *filename, struct fs_dir_stream **dirsp); int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp); void fat_closedir(struct fs_dir_stream *dirs); void fat_close(void); +#endif /* CONFIG_FS_FAT */ #endif /* _FAT_H_ */