Re: [PATCH v2 00/22] Deal with exfat versus byteorder

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4768 bytes --]

On Thu, May 01, 2025 at 09:03:18AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 1 May 2025 at 08:17, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, May 01, 2025 at 06:07:15AM -0600, Simon Glass wrote:
> > > Hi Marek,
> > >
> > > On Wed, 30 Apr 2025 at 20:02, Marek Vasut <marek.vasut@mailbox.org> wrote:
> > > >
> > > > On 5/1/25 3:04 AM, Simon Glass wrote:
> > > > > Since exfat has its own byteorder functions we need to be careful about
> > > > > including the normal U-Boot byteorder headers in code included by exfat.
> > > >
> > > > ...
> > > >
> > > > >    net: Use a forward declaration for cmd_tbl in net-common.h
> > > > >    congatec: Include env.h to permit reading the environment
> > > > >    dhelectronics: Include env.h to permit reading the environment
> > > > >    imx8ulp_evk: Include env.h to permit reading the environment
> > > > >    venice: Include env.h to permit reading the environment
> > > > >    phytec: Include env.h to permit reading the environment
> > > > >    ronetix: Include env.h to permit reading the environment
> > > > >    toradex: Include env.h to permit reading the environment
> > > > >    advantech: Include env.h in imx8qm_dmsse20_a1
> > > > >    tegra: Include env.h to permit reading the environment
> > > > >    synology: Include env.h to permit reading the environment
> > > > >    amlogic: Include env.h to permit reading the environment
> > > > >    freescale: Include env.h to permit reading the environment
> > > > >    google: Include env.h to permit reading the environment
> > > > >    liebherr: Include env.h to permit reading the environment
> > > > >    technexion: Include env.h to permit reading the environment
> > > > >    elf: Only use network environment-variables if net enabled
> > > > >    net: Include byteorder in net6.h
> > > > >    net: Include string.h in net-legacy.h
> > > > >    net: Include env.h in pcap.c
> > > > >    net: dc2114x: Include env.h to permit reading the environment
> > > > >    net: Move env_get_ip() out of the header file
> > > > >
> > > > >   arch/arm/mach-tegra/dt-setup.c                |  1 +
> > > > >   board/Synology/ds414/ds414.c                  |  1 +
> > > > >   .../imx8qm_dmsse20_a1/imx8qm_dmsse20_a1.c     |  1 +
> > > > >   board/amlogic/jethub-j100/jethub-j100.c       |  1 +
> > > > >   board/congatec/cgtqmx8/cgtqmx8.c              |  3 +-
> > > > >   board/dhelectronics/common/dh_common.c        |  1 +
> > > > >   board/freescale/imx8ulp_evk/imx8ulp_evk.c     |  1 +
> > > > >   board/freescale/mx7dsabresd/mx7dsabresd.c     |  1 +
> > > > >   board/gateworks/venice/venice.c               |  1 +
> > > > >   board/google/chameleonv3/board.c              |  2 +
> > > > >   board/liebherr/xea/xea.c                      |  1 +
> > > > >   .../common/phytec_som_detection_blocks.c      |  1 +
> > > > >   board/ronetix/imx8mq-cm/imx8mq_cm.c           |  1 +
> > > > >   board/technexion/pico-imx7d/pico-imx7d.c      |  1 +
> > > > >   board/toradex/colibri_imx7/colibri_imx7.c     |  1 +
> > > > >   board/toradex/verdin-imx8mm/verdin-imx8mm.c   |  2 +-
> > > > >   cmd/elf.c                                     | 41 ++++++++++---------
> > > > >   drivers/net/dc2114x.c                         |  1 +
> > > > >   include/net-common.h                          |  9 ++--
> > > > >   include/net-legacy.h                          |  1 +
> > > > >   include/net6.h                                |  1 +
> > > > >   net/net-common.c                              |  6 +++
> > > > >   net/pcap.c                                    |  1 +
> > > > >   23 files changed, 53 insertions(+), 27 deletions(-)
> > > > There are no fs changes in this series , what does this series have to
> > > > do with exfat ?
> > >
> > > I found that in some situations, the exfat code includes
> > > linux/byteorder.h instead of its own local byteorder, which is
> > > implemented differently. This then causes build errors in the exfat
> > > code. One way to trigger it is by adding '#include net.h' to
> > > include/efi.h
> >
> > Setting aside if it's a good idea to have efi.h include net.h, we should
> > unwind what the heck is going on. The build error is on CLUSTER_INVALID
> > somehow becoming weird. Looking harder at this, the problem I see is
> > that include/fs.h includes <rtc.h> (which in turn pulls in many things)
> > when it only needs <rtc_defs.h> for struct rtc_time. This however
> > exposes some other implicit includes. I'll go and take a look at hunting
> > these all down.
> 
> OK. I wonder if we could drop the special byteorder macros in exfat?

Potentially. I'll leave it to Marek to comment on how much pain and
future pain that might cause.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]