Hello Chris,
Thank you for pointing them out. I'll fix them in the next patch set.
Best,
Duc
On Mon, 2022-09-05 at 09:05 +1000, Chris Johns wrote:
> On 4/9/2022 11:25 am, Duc Doan wrote:
> > ---
> > lwip.py | 20 +++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/lwip.py b/lwip.py
> > index 84eef2c..d806b64 100644
> > --- a/lwip.py
> > +++ b/lwip.py
> > @@ -92,6 +92,17 @@ common_source_files = [
> > 'rtemslwip/bsd_compat/rtems-kernel-program.c'
> > ]
> >
> > +stm32f4_drv_incl = [
> > + 'rtemslwip/stm32f4',
> > + 'stm32',
> > + 'stm32/driver'
> > +]
> > +
> > +stm32f4_drv_src = [
> > + 'stm32',
> > + 'stm32/driver'
> > +]
> > +
>
> Please group the BSP specific file sets together rather than
> spreading them
> across the common source files. I suggest after the Xilinx ones.
>
> > def build(bld):
> > source_files = []
> > @@ -150,13 +161,20 @@ def build(bld):
> >
> > driver_source.extend(walk_sources('rtemslwip/zynqmp_hardware'))
> > driver_source.extend(xilinx_aarch64_driver_source)
> > drv_incl.extend(xilinx_aarch64_drv_incl)
> > +
> > + # These files will only compile for STM32F4 BSPs
> > + if bld.env.RTEMS_ARCH_BSP.startswith('arm-rtems6-stm32f4'):
>
> This is wrong because the version is hard coded. I see you have
> copied what is
> in the file and they are also wrong.
>
> The formal arch and bsp format is arch/bsp and these tests expose an
> internal
> implementation detail from rtems_waf.
>
> I suggest the code be changed to add:
>
> arch = rtems.arch(bld.env.RTEMS_ARCH_BSP)
> bsp = rtems.bsp(bld.env.RTEMS_ARCH_BSP)
>
> and all the tests changed to:
>
> if arch == 'arm' and bsp == 'stm32f4':
>
> > + driver_source.extend(walk_sources('rtemslwip/stm32f4'))
> > + drv_incl.extend(stm32f4_drv_incl)
> > + for s in stm32f4_drv_src:
> > + driver_source.extend(walk_sources(s))
> >
> > lwip_obj_incl = []
> > lwip_obj_incl.extend(drv_incl)
> > lwip_obj_incl.extend(bsd_compat_incl)
> > lwip_obj_incl.extend(common_includes)
> >
> > - bld(features='c',
> > + bld(features ='c',
>
> The coding standard tool has the previous format.
>
> > target='lwip_obj',
> > cflags='-g -Wall -O0',
> > includes=' '.join(lwip_obj_incl),
>
> Thanks
> Chris
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
