Thanks, Chris & Christian. I think I got a better understanding of the issues now and found a simpler solution which is less intrusive. I will post a new patchset.
Best regards, Jan > -----Original Message----- > From: Chris Johns <chr...@rtems.org> > Sent: Tuesday, June 1, 2021 11:28 PM > To: Christian Mauderer <o...@c-mauderer.de>; Gedare Bloom > <ged...@rtems.org>; Sommer, Jan <jan.som...@dlr.de> > Cc: devel@rtems.org > Subject: Re: [PATCH v1 0/2] [libbsd] Install correct machine include headers > > On 2/6/21 4:20 am, Christian Mauderer wrote: > > On 01/06/2021 19:24, Gedare Bloom wrote: > >> On Mon, May 10, 2021 at 11:26 AM Jan Sommer <jan.som...@dlr.de> > wrote: > >>> > >>> Hello, > >>> > >>> This is a follow-up on this discussion regarding the installed > >>> header files in libbsd: > >>> https://lists.rtems.org/pipermail/devel/2021-April/066211.html > >>> > >>> The current situation is, that for example for all machines the > >>> bus.h is installed from within the rtemsbsd directory > >>> (https://git.rtems.org/rtems- > libbsd/tree/rtemsbsd/include/machine/bus.h). > >>> > >>> According to the file docu it originates from the amd64 version of this > file. > >>> It also has the following section in it which we ran into when > >>> compiling Chris' ptpd2 archive: > >>> > >>> #ifdef __i386__ > >>> #error "your include paths are wrong" > >>> #endif > >>> > >>> This patchset does the following: > >>> - Add the target dependent machine include directory to > >>> 'header-paths' in libbsd.py > >>> - Import (mostly) '_bus.h', 'bus.h' and 'cpufunc.h' for the targets > >>> from freebsd-org > >>> - Remove those header files from rtemsbsd directory > >>> > >>> As a result the matching versions for machine dependent header files > >>> are now installed for each BSP. > >>> Would this be an acceptable solution? > >>> > >>> So far I compiled some BSPs for i386, arm, aarch64, powerpc, riscv, > >>> sparc and > >>> sparc64 to check that they still compile after the changes. > >>> Are there any other architectures which should be included? > > > > I think the best starting point to find out the minimum supported > > platforms is the nexus-devices.h. At least the officially supported > > BSPs should have an entry there. That is: > > > > #if defined(LIBBSP_ARM_REALVIEW_PBX_A9_BSP_H) > > #elif defined(LIBBSP_ARM_BEAGLE_BSP_H) #elif > > defined(LIBBSP_ARM_LPC32XX_BSP_H) #elif > > defined(LIBBSP_M68K_GENMCF548X_BSP_H) > > #elif defined(LIBBSP_ARM_XILINX_ZYNQ_BSP_H) > > #elif defined(LIBBSP_AARCH64_XILINX_ZYNQMP_BSP_H) > > #elif defined(LIBBSP_ARM_ATSAM_BSP_H) > > #elif defined(LIBBSP_ARM_ALTERA_CYCLONE_V_BSP_H) > > #elif defined(LIBBSP_ARM_IMX_BSP_H) > > #elif defined(LIBBSP_ARM_IMXRT_BSP_H) > > #elif defined(LIBBSP_ARM_LPC24XX_BSP_H) #elif > > defined(LIBBSP_ARM_STM32H7_BSP_H) #elif > > defined(LIBBSP_I386_PC386_BSP_H) #elif > > defined(LIBBSP_POWERPC_QORIQ_BSP_H) > > #elif defined(LIBBSP_POWERPC_TQM8XX_BSP_H) > > #elif defined(LIBBSP_POWERPC_MOTOROLA_POWERPC_BSP_H) > > > > So I think you should have a look at m68k too. > > > > Did you try to run it on any of the platforms? > > > >>> > >>> I ran into one problem regarding the compilation of > >>> rtemsbsd/sys/dev/dw_mmc/dw_mmc.c:105 > >>> > >>>> return (bus_space_read_4(0, sc->bushandle, off)); > >>> > >>> The first parameter creates an error for riscv (I think because > >>> dereferencing a NULL pointer). > >>> Are there any suggestion how to solve it (I am not familiar with the > >>> bus space and there is a lot of macro magic going on)? > >>> > >> It looks like this will be a problem for any architecture. so should > >> the function that calls bus_space_write_4(0 ...) > >> The macro trail goes... > >> > >> #define __bs_rs(sz, t, h, o) > >> \ > >> (*(t)->__bs_opname(r,sz))((t)->bs_cookie, h, o) > >> > >> #define bus_space_read_4(t, h, o) __bs_rs(4,(t),(h),(o)) > >> > >> so t is dereferenced. It appears to be an error in the API usage by > >> the dw_mmc.c code to not specify a valid bus space. > > > > dw_mmc is only relevant for very few platforms. So in theory it could > > be limited to these. But most likely that won't really solve the problem. > > > > The right answer is that dw_mmc doesn't use the API like intended > > (like Gedare said). That could be a problem for more drivers that use > > stuff from bus.h in rtemsbsd. They never had to use that API > > correctly. Even worse: Some of the __rtems__ hacks in freebsd could have > that problem too. > > > > I think for this patch set we either need to have a thorough look at > > these locations or run (not only build) it on a number of platforms, > > especially the ones with special drivers in rtemsbsd or with heavily adapted > drivers. > > The only arch that supports a tag we current have is the x86 and that is for > IO > vs mem space. I posted some patches earlier this year I need to revisit for > the powerpc (mvme2700). If possible we prefer no tags and a single flat > address space that is cache coherent. In the x86 tags cannot be avoided and > supported drivers handle the tags correctly. > > Be-careful reviewing FreeBSD's bus space implementations. For example the > PowerPC support uses tables of calls and we do not need that overhead. > > The issue here is using this driver on the x86 platform. Until that happens I > am fine with the tag being 0. The API requires something even if it is not > used. > Thanks, Chris. No I understand the reason for the include guard in bus.h for i386. Maybe an overall simpler solutions is possible. I could add the bus.h for i386 to the existing machine directory in rtemsbsd/i386/include/machine And then change https://git.rtems.org/rtems-libbsd/tree/rtemsbsd/include/machine/bus.h#n126 To something like this: > Chris _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel