| Message ID | 20210327120630.92744-1-xypron.glpk@gmx.de |
|---|---|
| State | Accepted, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | [1/1] fs: fat: fix file_fat_detectfs() | expand |
On Sun, 28 Mar 2021 at 01:06, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > Up to now file_fat_detectfs() did not detect some interface types like > EFI, HOST, VIRTIO. > > Avoid duplicate code by calling blk_get_if_type_name(). > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > fs/fat/fat.c | 36 ++---------------------------------- > 1 file changed, 2 insertions(+), 34 deletions(-) This is lower case I think, but probably that is better anyway. Reviewed-by: Simon Glass <sjg@chromium.org>
On 3/28/21 4:05 AM, Simon Glass wrote: > On Sun, 28 Mar 2021 at 01:06, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> Up to now file_fat_detectfs() did not detect some interface types like >> EFI, HOST, VIRTIO. >> >> Avoid duplicate code by calling blk_get_if_type_name(). >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> fs/fat/fat.c | 36 ++---------------------------------- >> 1 file changed, 2 insertions(+), 34 deletions(-) > > This is lower case I think, but probably that is better anyway. > > Reviewed-by: Simon Glass <sjg@chromium.org> > Thanks for reviewing. Yes, if_typename_str[] is lowercase. So the case in the output changes. We should avoid different case in different use cases. So this change is favorable. There is nothing FAT specific in command 'fatinfo'. Shouldn't we better move it to cmd/fs.c and call it 'fsinfo'? Best regards Heinrich
Hi Heinrich, On Mon, 29 Mar 2021 at 01:24, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 3/28/21 4:05 AM, Simon Glass wrote: > > On Sun, 28 Mar 2021 at 01:06, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> Up to now file_fat_detectfs() did not detect some interface types like > >> EFI, HOST, VIRTIO. > >> > >> Avoid duplicate code by calling blk_get_if_type_name(). > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> --- > >> fs/fat/fat.c | 36 ++---------------------------------- > >> 1 file changed, 2 insertions(+), 34 deletions(-) > > > > This is lower case I think, but probably that is better anyway. > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > > > Thanks for reviewing. > > Yes, if_typename_str[] is lowercase. So the case in the output changes. > We should avoid different case in different use cases. So this change is > favorable. > > There is nothing FAT specific in command 'fatinfo'. Shouldn't we better > move it to cmd/fs.c and call it 'fsinfo'? Yes, ultimately the fs commands should use common code. It looks like that has happened with other FAT commands, but not this one. Regards, Simon
diff --git a/fs/fat/fat.c b/fs/fat/fat.c index ccba268f61..363e981fb2 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -1147,41 +1147,9 @@ int file_fat_detectfs(void) return 1; } -#if defined(CONFIG_IDE) || \ - defined(CONFIG_SATA) || \ - defined(CONFIG_SCSI) || \ - defined(CONFIG_CMD_USB) || \ - defined(CONFIG_MMC) - printf("Interface: "); - switch (cur_dev->if_type) { - case IF_TYPE_IDE: - printf("IDE"); - break; - case IF_TYPE_SATA: - printf("SATA"); - break; - case IF_TYPE_SCSI: - printf("SCSI"); - break; - case IF_TYPE_ATAPI: - printf("ATAPI"); - break; - case IF_TYPE_USB: - printf("USB"); - break; - case IF_TYPE_DOC: - printf("DOC"); - break; - case IF_TYPE_MMC: - printf("MMC"); - break; - default: - printf("Unknown"); - } - - printf("\n Device %d: ", cur_dev->devnum); + printf("Interface: %s\n", blk_get_if_type_name(cur_dev->if_type)); + printf(" Device %d: ", cur_dev->devnum); dev_print(cur_dev); -#endif if (read_bootsectandvi(&bs, &volinfo, &fatsize)) { printf("\nNo valid FAT fs found\n");
Up to now file_fat_detectfs() did not detect some interface types like EFI, HOST, VIRTIO. Avoid duplicate code by calling blk_get_if_type_name(). Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- fs/fat/fat.c | 36 ++---------------------------------- 1 file changed, 2 insertions(+), 34 deletions(-) -- 2.30.2