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

Reply via email to