Re: [PATCH rtems-lwip 4/5] lwip.py: Add STM32 lwIP port to build

2022-09-04 Thread Duc Doan 
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

Re: [PATCH rtems-lwip 4/5] lwip.py: Add STM32 lwIP port to build

2022-09-04 Thread Chris Johns 
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

[PATCH rtems-lwip 4/5] lwip.py: Add STM32 lwIP port to build

2022-09-03 Thread Duc Doan 
---
 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'
+]
+
 
 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'):
+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',
 target='lwip_obj',
 cflags='-g -Wall -O0',
 includes=' '.join(lwip_obj_incl),
-- 
2.37.2

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel