Hi, I'm sorry to be late but I was stuck at my work for ALSA control service programs for audio and music units on IEEE 1394 bus[1]. On Sat, May 30, 2020 at 11:08:39AM +0200, Oscar Carter wrote: > In 1394 OHCI specification, Isochronous Receive DMA context has several > modes. One of mode is 'BufferFill' and Linux FireWire stack uses it to > receive isochronous packets for multiple isochronous channel as > FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL. > > The mode is not used by in-kernel driver, while it's available for > userspace. The character device driver in firewire-core includes > cast of function callback for the mode since the type of callback > function is different from the other modes. The case is inconvenient > to effort of Control Flow Integrity builds due to > -Wcast-function-type warning. > > This commit removes the cast. A static helper function is newly added > to initialize isochronous context for the mode. The helper function > arranges isochronous context to assign specific callback function > after call of existent kernel API. It's noticeable that the number of > isochronous channel, speed, and the size of header are not required for > the mode. The helper function is used for the mode by character device > driver instead of direct call of existent kernel API. > > The same goal can be achieved (in the ioctl_create_iso_context function) > without this helper function as follows: > - Call the fw_iso_context_create function passing NULL to the callback > parameter. > - Then setting the context->callback.sc or context->callback.mc > variables based on the a->type value. > > However using the helper function created in this patch makes code more > clear and declarative. This way avoid the call to a function with one > purpose to achieved another one. > > Co-developed-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx> > Signed-off-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx> > Co-developed-by: Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx> > Signed-off-by: Stefan Richter <stefanr@xxxxxxxxxxxxxxxxx> > Signed-off-by: Oscar Carter <oscar.carter@xxxxxxx> > --- > Hi, > > this is another proposal to achieved the goal of remove function callback > cast start by me with the first [1] and second [2] versions, and followed > by the work of Takashi Sakamoto with his first [3] and second [4] versions, > and the code of Stefan Richter [5]. > > The purpose of this third version is to put together all the work done > until now following the comments of all reviewed patches. > > I've added the "Co-developed-by" and "Signed-off-by" tags to give credit to > Takashi Sakamoto and Stefan Richter if there are no objections. In my opinion, it's no need to add my and Stefan's sign-off tag to patch in which you firstly wrote even if it includes ideas from the others ;) > Changelog v1->v2 > -Set explicity to NULL the "ctx->callback.sc" variable and return an error > code in "fw_iso_context_create" function if both callback parameters are > NULL as Lev R. Oshvang suggested. > -Modify the commit changelog accordingly. > > Changelog v2->v3 > -Put togeher all the work done in different patches by different authors. > -Modify the previous work following the comments in the reviewed patches. > > [1] https://lore.kernel.org/lkml/20200516173934.31527-1-oscar.carter@xxxxxxx/ > [2] https://lore.kernel.org/lkml/20200519173425.4724-1-oscar.carter@xxxxxxx/ > [3] https://lore.kernel.org/lkml/20200520064726.31838-1-o-takashi@xxxxxxxxxxxxx/ > [4] https://lore.kernel.org/lkml/20200524132048.243223-1-o-takashi@xxxxxxxxxxxxx/ > [5] https://lore.kernel.org/lkml/20200525015532.0055f9df@kant/ > > drivers/firewire/core-cdev.c | 32 ++++++++++++++++++++++++++------ > include/linux/firewire.h | 11 +++++++---- > 2 files changed, 33 insertions(+), 10 deletions(-) Anyway this patch looks good to me. I test this patch with libhinoko and find no regression. Reviewed-by: Takashi Sakamoto <o-takashi@xxxxxxxxxxxxx> Testeb-by: Takashi Sakamoto<o-takashi@xxxxxxxxxxxxx> [1] [RFT] ALSA control service programs for Digidesign Digi 002/003 family and Tascam FireWire series https://mailman.alsa-project.org/pipermail/alsa-devel/2020-July/170331.html Thanks Takashi Sakamoto