> On 19 Jul 2017, at 6:25, Honnappa Nagarahalli > <honnappa.nagaraha...@linaro.org> wrote: > > On 18 July 2017 at 06:37, Elo, Matias (Nokia - FI/Espoo) > <matias....@nokia.com> wrote: >> >>> On 18 Jul 2017, at 6:58, Honnappa Nagarahalli >>> <honnappa.nagaraha...@linaro.org> wrote: >>> >>> On 17 July 2017 at 04:23, Elo, Matias (Nokia - FI/Espoo) >>> <matias....@nokia.com> wrote: >>>> Does this patch fix some real problem? At leas for me it only makes the >>>> scheduler interface harder to follow by spreading the functions into >>>> multiple headers. >>> >>> I have said this again and again. odp_schedule_if.h is a scheduler >>> interface file. i.e this file is supposed to contain >>> services/functions provided by scheduler to other components in ODP >>> (similar to what has been done in odp_queue_if.h - appreciate if a >>> closer attention is paid to this). Now, this file contains functions >>> provided by packet I/O (among other things). Appreciate if you could >>> explain why this file should contain these functions? >>> >>> Also, Petri has understood what this patch does, can you check with him? >> >> These functions are used by the schedulers to interface with other ODP >> components, so the scheduler_if.h is a logical place to define them. When >> implementing a new scheduler it's helpful to see all available functions >> from one >> place. I'm not fundamentally against this patch, but it's the task of the >> patch >> submitter to justify why a change is needed, not the other way around. >> >> Petri was originally opposed to moving these functions into xyz_internal.h >> headers, >> and only approved moving the functions into xyz_if.h files if it must be >> done. >> >> I'm just trying to understand why this change is necessary. I patch like this >> would be a lot easier to justify if it was sent as a part of the patch set >> which requires >> this change. Without that, a more comprehensive commit log would be helpful. > > Any suggestions on the commit message? > > Does adding the following sentence help? > > "odp_schedule_if.h is the scheduler interface file. i.e this file is > supposed to contain services/functions provided by scheduler to other > components in ODP"
Based on your older message the motivation for moving these functions is that they are related to the default scheduler and queue implementations. This would be a good point to mention. > >> >> The naming of the odp_packet_io_if.h is now a bit confusing as we already >> have the >> pktio_if_ops_t struct in odp_packet_io_internal.h, which is the actual pktio >> interface. > > I definitely agree with you on this. That's how v1 of the patch was. > It is changed after Petri's suggestion to move it to > odp_packet_io_if.h. In a previous email Petri suggested filename odp_queue_sched_if.h. Extrapolating from this, odp_packet_io_if.h should be named odp_packet_io_sched_if.h.
