| Message ID | 20211202113457.24011-1-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| Headers | show |
| Series | firewire: assist unit driver to compute packet timestamp | expand |
Hi Stefan, Thank you for your long effort to maintain Linux FireWire subsystem. I'd like to use the timestamp function for my integration in ALSA firewire stack planned at next version of Linux kernel. I'm glad if getting to your help for upstreaming. On Thu, Dec 02, 2021 at 08:34:54PM +0900, Takashi Sakamoto wrote: > Hi, > > In 1394 OHCI specification, each descriptor of IR/IT/AR/AT DMA context > has timeStamp field. The value of timeStamp field express the time in > which the controller accept packet. The resolution of value is isochronous > cycle count (8,000 Hz) with second up to 7. > > I have a plan to use the value of timeStamp field for ALSA firewire stack > so that userspace ALSA PCM/Rawmidi applications can get converted timestamp > (ktime) for PCM frame/MIDI message. The timestamp can ideally express > finer granularity than the time to invoke IRQ handler (and co). > > Current implementation of Linux FireWire subsystem delivers the value of > timeStamp field to unit driver for IR/IT/AT DMA context, but not for AR > DMA context. Additionally, the way to refer to Isochronous Cycle Timer > Register in MMIO region of 1394 OHCI controller is transaction to local > node. It includes overhead of transaction and it's preferable to add > less-overhead way available in any type of IRQ context. > > This patchset adds two functions exposed in kernel space: > > * fw_card_read_cycle_time() > * allow unit driver to access to CYCLE_TIME register in MMIO region > without initiate transaction > * fw_request_get_timestamp() > * allow unit driver to get timestamp of request packet inner request > handler > > I note that Hector Martin found kernel null pointer dereference during > process to remove PCI card and has posted a patch: > > * https://lore.kernel.org/lkml/20211027113130.8802-1-marcan@marcan.st/ > > His patch is included in the series with my comment for relevant commit > 20802224298c ("firewire: core: add forgotten dummy driver methods, remove > unused ones"). The patch is required since unit driver can refer to dummy > driver between removal callback of PCI subsystem and removal callback of > FireWire subsystem. > > Hector Martin (1): > firewire: Add dummy read_csr/write_csr functions > > Takashi Sakamoto (2): > firewire: add kernel API to access CYCLE_TIME register > firewire: add kernel API to access packet structure in request > structure for AR context > > drivers/firewire/core-card.c | 39 +++++++++++++++++++++++++++++ > drivers/firewire/core-cdev.c | 6 +++-- > drivers/firewire/core-transaction.c | 18 +++++++++++++ > include/linux/firewire.h | 3 +++ > 4 files changed, 64 insertions(+), 2 deletions(-) > > -- > 2.32.0 Sincerely yours Takashi Sakamoto
Hi Stefan, Wishing you a happy new year. We are in the last week for release of v5.16 kernel, and soon merge window for v5.17 kernel will be opened if thing goes well. I wish any action for the review process to merge these patches into upstream. Thanks Takashi Sakamoto On Tue, Dec 21, 2021 at 07:54:42PM +0900, Takashi Sakamoto wrote: > Hi Stefan, > > Thank you for your long effort to maintain Linux FireWire subsystem. I'd > like to use the timestamp function for my integration in ALSA firewire > stack planned at next version of Linux kernel. I'm glad if getting to > your help for upstreaming. > > On Thu, Dec 02, 2021 at 08:34:54PM +0900, Takashi Sakamoto wrote: > > Hi, > > > > In 1394 OHCI specification, each descriptor of IR/IT/AR/AT DMA context > > has timeStamp field. The value of timeStamp field express the time in > > which the controller accept packet. The resolution of value is isochronous > > cycle count (8,000 Hz) with second up to 7. > > > > I have a plan to use the value of timeStamp field for ALSA firewire stack > > so that userspace ALSA PCM/Rawmidi applications can get converted timestamp > > (ktime) for PCM frame/MIDI message. The timestamp can ideally express > > finer granularity than the time to invoke IRQ handler (and co). > > > > Current implementation of Linux FireWire subsystem delivers the value of > > timeStamp field to unit driver for IR/IT/AT DMA context, but not for AR > > DMA context. Additionally, the way to refer to Isochronous Cycle Timer > > Register in MMIO region of 1394 OHCI controller is transaction to local > > node. It includes overhead of transaction and it's preferable to add > > less-overhead way available in any type of IRQ context. > > > > This patchset adds two functions exposed in kernel space: > > > > * fw_card_read_cycle_time() > > * allow unit driver to access to CYCLE_TIME register in MMIO region > > without initiate transaction > > * fw_request_get_timestamp() > > * allow unit driver to get timestamp of request packet inner request > > handler > > > > I note that Hector Martin found kernel null pointer dereference during > > process to remove PCI card and has posted a patch: > > > > * https://lore.kernel.org/lkml/20211027113130.8802-1-marcan@marcan.st/ > > > > His patch is included in the series with my comment for relevant commit > > 20802224298c ("firewire: core: add forgotten dummy driver methods, remove > > unused ones"). The patch is required since unit driver can refer to dummy > > driver between removal callback of PCI subsystem and removal callback of > > FireWire subsystem. > > > > Hector Martin (1): > > firewire: Add dummy read_csr/write_csr functions > > > > Takashi Sakamoto (2): > > firewire: add kernel API to access CYCLE_TIME register > > firewire: add kernel API to access packet structure in request > > structure for AR context > > > > drivers/firewire/core-card.c | 39 +++++++++++++++++++++++++++++ > > drivers/firewire/core-cdev.c | 6 +++-- > > drivers/firewire/core-transaction.c | 18 +++++++++++++ > > include/linux/firewire.h | 3 +++ > > 4 files changed, 64 insertions(+), 2 deletions(-) > > > > -- > > 2.32.0 > > > Sincerely yours > > Takashi Sakamoto
Hi Stefan, I'm sorry to post messages several times for the patchset if you are still busy. But I'm still waiting for any reaction. I note that Linus have announced merge window for v5.17 kernel. * https://lore.kernel.org/lkml/CAHk-=wgUkBrUVhjixy4wvrUhPbW-DTgtQubJWVOoLW=O0wRKMA@mail.gmail.com/T/#u I'm glad if seeing your action for pull request as a response to the window. Kind Regards Takashi Sakamoto On Fri, Jan 07, 2022 at 08:01:18PM +0900, Takashi Sakamoto wrote: > Hi Stefan, > > Wishing you a happy new year. > > We are in the last week for release of v5.16 kernel, and soon merge > window for v5.17 kernel will be opened if thing goes well. I wish any > action for the review process to merge these patches into upstream. > > > Thanks > > Takashi Sakamoto > > On Tue, Dec 21, 2021 at 07:54:42PM +0900, Takashi Sakamoto wrote: > > Hi Stefan, > > > > Thank you for your long effort to maintain Linux FireWire subsystem. I'd > > like to use the timestamp function for my integration in ALSA firewire > > stack planned at next version of Linux kernel. I'm glad if getting to > > your help for upstreaming. > > > > On Thu, Dec 02, 2021 at 08:34:54PM +0900, Takashi Sakamoto wrote: > > > Hi, > > > > > > In 1394 OHCI specification, each descriptor of IR/IT/AR/AT DMA context > > > has timeStamp field. The value of timeStamp field express the time in > > > which the controller accept packet. The resolution of value is isochronous > > > cycle count (8,000 Hz) with second up to 7. > > > > > > I have a plan to use the value of timeStamp field for ALSA firewire stack > > > so that userspace ALSA PCM/Rawmidi applications can get converted timestamp > > > (ktime) for PCM frame/MIDI message. The timestamp can ideally express > > > finer granularity than the time to invoke IRQ handler (and co). > > > > > > Current implementation of Linux FireWire subsystem delivers the value of > > > timeStamp field to unit driver for IR/IT/AT DMA context, but not for AR > > > DMA context. Additionally, the way to refer to Isochronous Cycle Timer > > > Register in MMIO region of 1394 OHCI controller is transaction to local > > > node. It includes overhead of transaction and it's preferable to add > > > less-overhead way available in any type of IRQ context. > > > > > > This patchset adds two functions exposed in kernel space: > > > > > > * fw_card_read_cycle_time() > > > * allow unit driver to access to CYCLE_TIME register in MMIO region > > > without initiate transaction > > > * fw_request_get_timestamp() > > > * allow unit driver to get timestamp of request packet inner request > > > handler > > > > > > I note that Hector Martin found kernel null pointer dereference during > > > process to remove PCI card and has posted a patch: > > > > > > * https://lore.kernel.org/lkml/20211027113130.8802-1-marcan@marcan.st/ > > > > > > His patch is included in the series with my comment for relevant commit > > > 20802224298c ("firewire: core: add forgotten dummy driver methods, remove > > > unused ones"). The patch is required since unit driver can refer to dummy > > > driver between removal callback of PCI subsystem and removal callback of > > > FireWire subsystem. > > > > > > Hector Martin (1): > > > firewire: Add dummy read_csr/write_csr functions > > > > > > Takashi Sakamoto (2): > > > firewire: add kernel API to access CYCLE_TIME register > > > firewire: add kernel API to access packet structure in request > > > structure for AR context > > > > > > drivers/firewire/core-card.c | 39 +++++++++++++++++++++++++++++ > > > drivers/firewire/core-cdev.c | 6 +++-- > > > drivers/firewire/core-transaction.c | 18 +++++++++++++ > > > include/linux/firewire.h | 3 +++ > > > 4 files changed, 64 insertions(+), 2 deletions(-) > > > > > > -- > > > 2.32.0 > > > > > > Sincerely yours > > > > Takashi Sakamoto
On 2022/01/12 12:51, Takashi Sakamoto wrote: > Hi Stefan, > > I'm sorry to post messages several times for the patchset if you are > still busy. But I'm still waiting for any reaction. > > I note that Linus have announced merge window for v5.17 kernel. > * https://lore.kernel.org/lkml/CAHk-=wgUkBrUVhjixy4wvrUhPbW-DTgtQubJWVOoLW=O0wRKMA@mail.gmail.com/T/#u > > I'm glad if seeing your action for pull request as a response to the > window. Hi Tahashi, Just FYI, I think a lot of maintainers have been off or doing less work over December/the holidays; I also have some things that didn't make it into subsystem trees for 5.17. So I'm guessing this patchset will have to wait for 5.18. AIUI most maintainers don't merge things into subsystem trees after the upstream merge window opens. I've also been meaning to test your Firewire improvements again (and also with Pipewire), but I've been pretty busy lately... hopefully I'll get a chance soon. When I tried the first round of improvements that got merged (the sequence replay stuff) I noticed it fixed the glitchy audio problem, but the minimum stable period size with JACK+ALSA was still higher than with JACK+FFADO, and Pipewire also had even higher latencies. So I'm back using FFADO but I hope I can switch to ALSA soon :)
Hi, In 1394 OHCI specification, each descriptor of IR/IT/AR/AT DMA context has timeStamp field. The value of timeStamp field express the time in which the controller accept packet. The resolution of value is isochronous cycle count (8,000 Hz) with second up to 7. I have a plan to use the value of timeStamp field for ALSA firewire stack so that userspace ALSA PCM/Rawmidi applications can get converted timestamp (ktime) for PCM frame/MIDI message. The timestamp can ideally express finer granularity than the time to invoke IRQ handler (and co). Current implementation of Linux FireWire subsystem delivers the value of timeStamp field to unit driver for IR/IT/AT DMA context, but not for AR DMA context. Additionally, the way to refer to Isochronous Cycle Timer Register in MMIO region of 1394 OHCI controller is transaction to local node. It includes overhead of transaction and it's preferable to add less-overhead way available in any type of IRQ context. This patchset adds two functions exposed in kernel space: * fw_card_read_cycle_time() * allow unit driver to access to CYCLE_TIME register in MMIO region without initiate transaction * fw_request_get_timestamp() * allow unit driver to get timestamp of request packet inner request handler I note that Hector Martin found kernel null pointer dereference during process to remove PCI card and has posted a patch: * https://lore.kernel.org/lkml/20211027113130.8802-1-marcan@marcan.st/ His patch is included in the series with my comment for relevant commit 20802224298c ("firewire: core: add forgotten dummy driver methods, remove unused ones"). The patch is required since unit driver can refer to dummy driver between removal callback of PCI subsystem and removal callback of FireWire subsystem. Hector Martin (1): firewire: Add dummy read_csr/write_csr functions Takashi Sakamoto (2): firewire: add kernel API to access CYCLE_TIME register firewire: add kernel API to access packet structure in request structure for AR context drivers/firewire/core-card.c | 39 +++++++++++++++++++++++++++++ drivers/firewire/core-cdev.c | 6 +++-- drivers/firewire/core-transaction.c | 18 +++++++++++++ include/linux/firewire.h | 3 +++ 4 files changed, 64 insertions(+), 2 deletions(-)