We need to pull this work into the full ODP repo structure.

It should be in a new repo here https://git.linaro.org/lng/odp-d01.git

And then we can hook it into the website and other ODP tools.

On 24 November 2014 05:00, Ciprian Barbu <[email protected]> wrote:

> Hi,
>
> Here are a few comments from my side:
>
> minor nit: platform/linux-d01/Makefile.am : preserve the order of
> __LIB__libodp_la_SOURCES relative to linux-generic. odp_packet_flags.c
> is placed at the end of the list. It's just aesthetics but I wouldn't
> mind if you fixed it. Also compared to linux-generic Makefile.am you
> are missing $(top_srcdir)/helper/include/odph_icmp.h and
> $(top_srcdir)/helper/include/odph_ipsec.h
>
> platform/linux-d01/include/odp_d01.h - maybe you should rename it to
> odp_d01_internal.h. Inside this file, there is a comment for uint32_t
> reserve[8] that does not help much, maybe you should document it
> better, other fields as well.
>
> platform/linux-d01/include/odp_packet_io_internal.h still contains
> #ifdef ODP_HAVE_NETMAP. Also struct pktio_entry_s has been renamed in
> platform/linux-generic/include/odp_packet_io_internal.h to struct
> pktio_entry
>
> platform/linux-d01/include/api/odp_buffer.h - check against the
> linux-generic version, you are missing some doxygen tags. Also you
> introduced a new API, odp_buffer_paddr. It's not ok to introduce new
> API's because the implementation will not be compatible anymore. I
> couldn't find a use for this API, is it valid anymore? If not, please
> remove it.
>
> platform/linux-d01/odp_buffer_pool.c - pool_handle_to_index return
> clause was changed compared to linux-generic without reason. Did
> checkpatch complain about it? If not please restore it.
>
> platform/linux-d01/odp_shared_memory.c - #define SHM_FLAGS is not
> used. At line 200 the comment has no spaces between /* and */
>
> platform/linux-d01/README is an exact copy of the linux-generic
> README. Please provide some instructions on how to build and run ODP
> on linux-d01.
>
> It might also be a good idea to set the default platform to linux-d01,
> in configure.ac, but you need to make that clear in the README.
>
> I didn't spend too much time looking at the platform specific
> implementation, it's quite a major task understanding how it all
> works. I need some time to try and tackle that.
>
> /Ciprian
>
> On Mon, Nov 17, 2014 at 11:12 AM, Weilong Chen <[email protected]>
> wrote:
> > Refresh the git repo:ssh://
> [email protected]/people/weilong.chen/d01.git
> > Fix some errors by Anders reviews.
> > Thanks!
> >
> >
> >
> > On 10 November 2014 11:52, Weilong Chen <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> I refresh the git repo.
> >> Fix the send packet bug.
> >> Remove net d01 code
> >> Fix copyright year and commit message
> >>
> >> Please review it,
> >>
> >> Thanks
> >>
> >> On 23 October 2014 20:14, Anders Roxell <[email protected]>
> wrote:
> >>>
> >>> On 2014-10-22 16:58, Weilong Chen wrote:
> >>> > Hi Anders
> >>> >
> >>> > I push linux-d01 to git.linaro.org/people/weilong.chen/linux-d01.git
> >>> > There's still some bugs to workout.
> >>> > If you want to have a quit look at it, you can get it.
> >>> >
> >>> > Thanks,
> >>> > Weilong
> >>>
> >>> And I guess you have to change the copyright year as well on the new
> >>> files...
> >>> Maybe fix the commit message to something more sensible.
> >>>
> >>> Cheers,
> >>> Anders
> >>
> >>
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > [email protected]
> > http://lists.linaro.org/mailman/listinfo/lng-odp
> >
>
> _______________________________________________
> lng-odp mailing list
> [email protected]
> http://lists.linaro.org/mailman/listinfo/lng-odp
>



-- 
*Mike Holmes*
Linaro  Sr Technical Manager
LNG - ODP
_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to