On 25 February 2016 at 22:43, Mike Holmes <mike.hol...@linaro.org> wrote:
> On 25 February 2016 at 08:17, Christophe Milard < > christophe.mil...@linaro.org> wrote: > >> I guess this is stuck untill BKK discussion...? >> > > I hope not. > > >> >> On 18 February 2016 at 12:37, Christophe Milard < >> christophe.mil...@linaro.org> wrote: >> >>> A complete -partly working- driver prototype was sent as in RFC: >>> https://lists.linaro.org/pipermail/lng-odp/2016-January/019162.html >>> This approach of the driver interface "extending" the application >>> interface was refused and we decided that the driver would have its own >>> separate definition... in its own directory. >>> so now the choice are: >>> 1) Let the driver use both the north application interface and its own >>> south interface (do we really want that)? >>> 2) let the driver interface have some common functions (to the >>> application interface), i.e. redefining these common functions in both >>> interfaces, and when possible having the common "body" in the com >>> directory) - this patch >>> >>> Christophe. >>> >>> >>> >>> On 18 February 2016 at 11:44, Maxim Uvarov <maxim.uva...@linaro.org> >>> wrote: >>> >>>> I really don't like what we are doing here. We should try to avoid any >>>> code duplication. >>>> Add odpdrv_ prefix to all odp functions and move them to other place is >>>> not thing we should to do. >>>> >>>> And also, in my understanding, we should not implement our own >>>> programming language for odp drivers. >>>> >>>> So my point is: >>>> 1) odp driver and odp application should operate the same type of >>>> variables and structs. >>>> No any drv_magic_atomic, drv_LITTLE_ENDIAN and etc. >>>> >>> > We have already agreed there will be two interfaces that are independent, > one for applications and one for drivers, so I do not agree they use the > same same struct, can you convince us why we would change our position with > a good example on the problem it will cause. > We agreed before that the interface independance was worth a little > complexity, I do not see a reason to change that decision yet. > Agreed. Changing a decision that has been taken should require: 1) Identifying a new problem which wasn't known by the time the initial decision was taken 2) having a better solution at least fixing the problem identified in 1) I don't see this here. > > > >> >>>> 2) What is odp driver? Is it some linux kernel driver adopted to ODP or >>>> freebsd driver adopted to ODP? >>>> If it's adopted driver than we need have as less changes related to odp >>>> there as we can. >>>> >>> > No, it is a driver specifically written for ODP to gain access to a PCIe > or simple case platform NIC with the highest possible performance from > userspace. Maybe Christophe can give another talk in the ARCH call to make > sure we are all solving the same problem when we look at this proposal. > I don't mind trying to explain my view at the next arch call. The problem I am trying to address is to enable a set of user space programs (the drivers), interfacing ODP (through the interface we are now trying to define, called drv here). These "drivers" (which I think should be libs eventually) would enable a new packetio type at the application level, to send/receive packet from the NIC. The RFC I sent early january was an example of that. -At init time, each driver would register to ODP. (the RFC contained a single driver for intel ixgbe). So a single driver registered. When registering, the driver gave a pointer to its probe() function to ODP. -The ODP application could create a new packetio (named "nic:<pci_address>". A wrapper could be created for other kind of interface specification later, but these board spec will always boil down to a PCI address at the end, so it was the simplest case to start with)). -when pktio_open() was called, ODP called the registered driver probe() one by one. The first driver answering "I can handle this board" was selected. -Any packet sent to this packet io was passed to the NIC and sent on the wire, -Any pacet received by the board was presented as received packet on the packet_io > >>>> 3) Work flow here should be opposite. Driver should be implemented (or >>>> ported) first. Then you should >>>> consider which parts needed to be implemented. Patch should be linked >>>> to implementation patch which >>>> shows the actual reason for doing this. But now it looks like double >>>> defining everything what we have in >>>> odp api and then we will think what to do with it. >>>> >>> > Another RFC is no bad thing, it does help show how things are to be used. > But what I have witnessed is a couple of large effort RFCs that flip flop > on fundamentals and that is a waste of effort. We must stick to our > decisions and progress unless a new argument shows we need to change course > or we will never get far enough to really understand the problem. > The RFC sent in january was 31 patches. only patch 1 recieved comments. Even though the interface specification (which was a .h file then) has slightly changed, most of the concepts address by this patch set remains valid. Christophe > > > >> >>>> Maxim. >>>> >>>> >>>> >>>> On 02/17/16 17:36, Christophe Milard wrote: >>>> >>>>> Signed-off-by: Christophe Milard <christophe.mil...@linaro.org> >>>>> --- >>>>> include/odp/drv/spec/byteorder.h | 176 >>>>> +++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 176 insertions(+) >>>>> create mode 100644 include/odp/drv/spec/byteorder.h >>>>> >>>>> diff --git a/include/odp/drv/spec/byteorder.h >>>>> b/include/odp/drv/spec/byteorder.h >>>>> new file mode 100644 >>>>> index 0000000..2543016 >>>>> --- /dev/null >>>>> +++ b/include/odp/drv/spec/byteorder.h >>>>> @@ -0,0 +1,176 @@ >>>>> +/* Copyright (c) 2016, Linaro Limited >>>>> + * All rights reserved. >>>>> + * >>>>> + * SPDX-License-Identifier: BSD-3-Clause >>>>> + */ >>>>> + >>>>> +/** >>>>> + * @file >>>>> + * >>>>> + * ODP byteorder >>>>> + */ >>>>> + >>>>> +#ifndef ODPDRV_BYTEORDER_H_ >>>>> +#define ODPDRV_BYTEORDER_H_ >>>>> + >>>>> +#ifdef __cplusplus >>>>> +extern "C" { >>>>> +#endif >>>>> + >>>>> +/** @addtogroup odpdrv_compiler_optim ODPDRV COMPILER / OPTIMIZATION >>>>> + * Macros that check byte order and operations for byte order >>>>> conversion. >>>>> + * @{ >>>>> + */ >>>>> + >>>>> +/** >>>>> + * @def ODPDRV_BIG_ENDIAN >>>>> + * Big endian byte order >>>>> + * >>>>> + * @def ODPDRV_LITTLE_ENDIAN >>>>> + * Little endian byte order >>>>> + * >>>>> + * @def ODPDRV_BIG_ENDIAN_BITFIELD >>>>> + * Big endian bit field >>>>> + * >>>>> + * @def ODPDRV_LITTLE_ENDIAN_BITFIELD >>>>> + * Little endian bit field >>>>> + * >>>>> + * @def ODPDRV_BYTE_ORDER >>>>> + * Selected byte order >>>>> + */ >>>>> + >>>>> +/** >>>>> + * @typedef odpdrv_u16le_t >>>>> + * unsigned 16bit little endian >>>>> + * >>>>> + * @typedef odpdrv_u16be_t >>>>> + * unsigned 16bit big endian >>>>> + * >>>>> + * @typedef odpdrv_u32le_t >>>>> + * unsigned 32bit little endian >>>>> + * >>>>> + * @typedef odpdrv_u32be_t >>>>> + * unsigned 32bit big endian >>>>> + * >>>>> + * @typedef odpdrv_u64le_t >>>>> + * unsigned 64bit little endian >>>>> + * >>>>> + * @typedef odpdrv_u64be_t >>>>> + * unsigned 64bit big endian >>>>> + * >>>>> + * @typedef odpdrv_u16sum_t >>>>> + * unsigned 16bit bitwise >>>>> + * >>>>> + * @typedef odpdrv_u32sum_t >>>>> + * unsigned 32bit bitwise >>>>> + */ >>>>> + >>>>> +/* >>>>> + * Big Endian -> CPU byte order: >>>>> + */ >>>>> + >>>>> +/** >>>>> + * Convert 16bit big endian to cpu native uint16_t >>>>> + * @param be16 big endian 16bit >>>>> + * @return cpu native uint16_t >>>>> + */ >>>>> +uint16_t odpdrv_be_to_cpu_16(odpdrv_u16be_t be16); >>>>> + >>>>> +/** >>>>> + * Convert 32bit big endian to cpu native uint32_t >>>>> + * @param be32 big endian 32bit >>>>> + * @return cpu native uint32_t >>>>> + */ >>>>> +uint32_t odpdrv_be_to_cpu_32(odpdrv_u32be_t be32); >>>>> + >>>>> +/** >>>>> + * Convert 64bit big endian to cpu native uint64_t >>>>> + * @param be64 big endian 64bit >>>>> + * @return cpu native uint64_t >>>>> + */ >>>>> +uint64_t odpdrv_be_to_cpu_64(odpdrv_u64be_t be64); >>>>> + >>>>> +/* >>>>> + * CPU byte order -> Big Endian: >>>>> + */ >>>>> + >>>>> +/** >>>>> + * Convert cpu native uint16_t to 16bit big endian >>>>> + * @param cpu16 uint16_t in cpu native format >>>>> + * @return big endian 16bit >>>>> + */ >>>>> +odpdrv_u16be_t odpdrv_cpu_to_be_16(uint16_t cpu16); >>>>> + >>>>> +/** >>>>> + * Convert cpu native uint32_t to 32bit big endian >>>>> + * @param cpu32 uint32_t in cpu native format >>>>> + * @return big endian 32bit >>>>> + */ >>>>> +odpdrv_u32be_t odpdrv_cpu_to_be_32(uint32_t cpu32); >>>>> + >>>>> +/** >>>>> + * Convert cpu native uint64_t to 64bit big endian >>>>> + * @param cpu64 uint64_t in cpu native format >>>>> + * @return big endian 64bit >>>>> + */ >>>>> +odpdrv_u64be_t odpdrv_cpu_to_be_64(uint64_t cpu64); >>>>> + >>>>> +/* >>>>> + * Little Endian -> CPU byte order: >>>>> + */ >>>>> + >>>>> +/** >>>>> + * Convert 16bit little endian to cpu native uint16_t >>>>> + * @param le16 little endian 16bit >>>>> + * @return cpu native uint16_t >>>>> + */ >>>>> +uint16_t odpdrv_le_to_cpu_16(odpdrv_u16le_t le16); >>>>> + >>>>> +/** >>>>> + * Convert 32bit little endian to cpu native uint32_t >>>>> + * @param le32 little endian 32bit >>>>> + * @return cpu native uint32_t >>>>> + */ >>>>> +uint32_t odpdrv_le_to_cpu_32(odpdrv_u32le_t le32); >>>>> + >>>>> +/** >>>>> + * Convert 64bit little endian to cpu native uint64_t >>>>> + * @param le64 little endian 64bit >>>>> + * @return cpu native uint64_t >>>>> + */ >>>>> +uint64_t odpdrv_le_to_cpu_64(odpdrv_u64le_t le64); >>>>> + >>>>> +/* >>>>> + * CPU byte order -> Little Endian: >>>>> + */ >>>>> + >>>>> +/** >>>>> + * Convert cpu native uint16_t to 16bit little endian >>>>> + * @param cpu16 uint16_t in cpu native format >>>>> + * @return little endian 16bit >>>>> + */ >>>>> +odpdrv_u16le_t odpdrv_cpu_to_le_16(uint16_t cpu16); >>>>> + >>>>> +/** >>>>> + * Convert cpu native uint32_t to 32bit little endian >>>>> + * @param cpu32 uint32_t in cpu native format >>>>> + * @return little endian 32bit >>>>> + */ >>>>> +odpdrv_u32le_t odpdrv_cpu_to_le_32(uint32_t cpu32); >>>>> + >>>>> +/** >>>>> + * Convert cpu native uint64_t to 64bit little endian >>>>> + * @param cpu64 uint64_t in cpu native format >>>>> + * @return little endian 64bit >>>>> + */ >>>>> +odpdrv_u64le_t odpdrv_cpu_to_le_64(uint64_t cpu64); >>>>> + >>>>> +/** >>>>> + * @} >>>>> + */ >>>>> + >>>>> +#ifdef __cplusplus >>>>> +} >>>>> +#endif >>>>> + >>>>> +#endif >>>>> >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> https://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>> >>> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs > "Work should be fun and collborative, the rest follows" > > >
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
