[PATCH 00/24] ALSA: Generic PCM copy ops using sockptr_t
Hi,
this is a patch set to clean up the PCM copy ops using sockptr_t as a "universal" pointer, inspired by the recent patch from Andy Shevchenko: https://lore.kernel.org/r/20230721100146.67293-1-andriy.shevchenko@linux.int...
Even though it sounds a bit weird, sockptr_t is a generic type that is used already in wide ranges, and it can fit our purpose, too. With sockptr_t, the former split of copy_user and copy_kernel PCM ops can be unified again gracefully.
The patch set introduces the new PCM ops, converting users, and drops the old PCM ops. Most of conversions are straightforward, simply replacing copy_*_user() with copy_*_sockptr() variants.
Note that the conversion in ASoC will fix a potential problem of ASoC PCM that has been for long time. Since ASoC component takes care of only copy_user, the conversion form/to kernel space might have been missing. With this patch set, both cases are handled with sockptr_t by a single callback.
The patches are lightly tested (with a faked PCM copy implementation on HD-audio), while most of patches are only compile-tested.
Takashi
===
Cc: Andy Shevchenko andriy.shevchenko@linux.intel.com Cc: Andrey Utkin andrey_utkin@fastmail.com Cc: Anton Sviridenko anton@corp.bluecherry.net Cc: Arnaud Pouliquen arnaud.pouliquen@foss.st.com Cc: Banajit Goswami bgoswami@quicinc.com Cc: Bluecherry Maintainers maintainers@bluecherrydvr.com Cc: Claudiu Beznea claudiu.beznea@microchip.com Cc: Ismael Luceno ismael@iodev.co.uk Cc: Lars-Peter Clausen lars@metafoo.de Cc: Mark Brown broonie@kernel.org Cc: Mauro Carvalho Chehab mchehab@kernel.org Cc: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com Cc: Olivier Moysan olivier.moysan@foss.st.com Cc: Srinivas Kandagatla srinivas.kandagatla@linaro.org Cc: linux-media@vger.kernel.org Cc: xen-devel@lists.xenproject.org
===
Takashi Iwai (24): ALSA: pcm: Add copy ops with universal sockptr_t ALSA: core: Add memory copy helpers between sockptr and iomem ALSA: dummy: Convert to generic PCM copy ops ALSA: gus: Convert to generic PCM copy ops ALSA: emu8000: Convert to generic PCM copy ops ALSA: es1938: Convert to generic PCM copy ops ALSA: korg1212: Convert to generic PCM copy ops ALSA: nm256: Convert to generic PCM copy ops ALSA: rme32: Convert to generic PCM copy ops ALSA: rme96: Convert to generic PCM copy ops ALSA: hdsp: Convert to generic PCM copy ops ALSA: rme9652: Convert to generic PCM copy ops ALSA: sh: Convert to generic PCM copy ops ALSA: xen: Convert to generic PCM copy ops ALSA: pcmtest: Update comment about PCM copy ops media: solo6x10: Convert to generic PCM copy ops ASoC: component: Add generic PCM copy ops ASoC: mediatek: Convert to generic PCM copy ops ASoC: qcom: Convert to generic PCM copy ops ASoC: dmaengine: Convert to generic PCM copy ops ASoC: dmaengine: Use sockptr_t for process callback, too ALSA: doc: Update description for the new PCM copy ops ASoC: pcm: Drop obsoleted PCM copy_user ops ALSA: pcm: Drop obsoleted PCM copy_user and copy_kernel ops
.../kernel-api/writing-an-alsa-driver.rst | 59 +++++--------- drivers/media/pci/solo6x10/solo6x10-g723.c | 41 ++-------- include/sound/dmaengine_pcm.h | 2 +- include/sound/pcm.h | 12 +-- include/sound/soc-component.h | 14 ++-- sound/core/memory.c | 39 +++++++++ sound/core/pcm_lib.c | 81 +++++++++---------- sound/core/pcm_native.c | 2 +- sound/drivers/dummy.c | 12 +-- sound/drivers/pcmtest.c | 2 +- sound/isa/gus/gus_pcm.c | 23 +----- sound/isa/sb/emu8000_pcm.c | 79 +++++------------- sound/pci/es1938.c | 31 ++----- sound/pci/korg1212/korg1212.c | 46 +++-------- sound/pci/nm256/nm256.c | 42 ++-------- sound/pci/rme32.c | 50 +++--------- sound/pci/rme96.c | 48 +++-------- sound/pci/rme9652/hdsp.c | 42 ++-------- sound/pci/rme9652/rme9652.c | 46 ++--------- sound/sh/sh_dac_audio.c | 25 +----- sound/soc/atmel/mchp-pdmc.c | 2 +- sound/soc/mediatek/common/mtk-btcvsd.c | 22 ++--- sound/soc/qcom/lpass-platform.c | 12 +-- sound/soc/soc-component.c | 10 +-- sound/soc/soc-generic-dmaengine-pcm.c | 18 ++--- sound/soc/soc-pcm.c | 4 +- sound/soc/stm/stm32_sai_sub.c | 2 +- sound/xen/xen_snd_front_alsa.c | 55 +++---------- 28 files changed, 251 insertions(+), 570 deletions(-)
sockptr is a universal pointer that can represent both kernel- and user-space pointers. Despite of its name, sockptr is already widely used in quite a few subsystems as a generic solution, and it fits pretty well for ALSA PCM copy ops, too; we had to split to copy_user and copy_kernel, and those can be unified to a single ops with sockptr_t.
So here it is: this patch adds a new PCM copy ops that passes sockptr_t pointer for copying both kernel and user-space more consistently. This patch touches only the ALSA PCM core part, and the actual users will be replaced in the following patches.
As of now, the old copy_user and copy_kernel ops are still kept. Once after all users are converted, we'll drop the old copy_user and copy_kernel ops, too.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 3 ++ sound/core/pcm_lib.c | 97 ++++++++++++++++++++++------------------- sound/core/pcm_native.c | 2 +- 3 files changed, 57 insertions(+), 45 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 19f564606ac4..04b935200d0e 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -16,6 +16,7 @@ #include <linux/bitops.h> #include <linux/pm_qos.h> #include <linux/refcount.h> +#include <linux/sockptr.h>
#define snd_pcm_substream_chip(substream) ((substream)->private_data) #define snd_pcm_chip(pcm) ((pcm)->private_data) @@ -68,6 +69,8 @@ struct snd_pcm_ops { struct snd_pcm_audio_tstamp_report *audio_tstamp_report); int (*fill_silence)(struct snd_pcm_substream *substream, int channel, unsigned long pos, unsigned long bytes); + int (*copy)(struct snd_pcm_substream *substream, int channel, + unsigned long pos, sockptr_t buf, unsigned long bytes); int (*copy_user)(struct snd_pcm_substream *substream, int channel, unsigned long pos, void __user *buf, unsigned long bytes); diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 9c121a921b04..1dd630cd22a7 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1973,10 +1973,11 @@ static int wait_for_avail(struct snd_pcm_substream *substream, typedef int (*pcm_transfer_f)(struct snd_pcm_substream *substream, int channel, unsigned long hwoff, - void *buf, unsigned long bytes); + sockptr_t buf, unsigned long bytes);
typedef int (*pcm_copy_f)(struct snd_pcm_substream *, snd_pcm_uframes_t, void *, - snd_pcm_uframes_t, snd_pcm_uframes_t, pcm_transfer_f); + snd_pcm_uframes_t, snd_pcm_uframes_t, pcm_transfer_f, + bool);
/* calculate the target DMA-buffer position to be written/read */ static void *get_dma_ptr(struct snd_pcm_runtime *runtime, @@ -1986,32 +1987,23 @@ static void *get_dma_ptr(struct snd_pcm_runtime *runtime, channel * (runtime->dma_bytes / runtime->channels); }
-/* default copy_user ops for write; used for both interleaved and non- modes */ +/* default copy ops for write; used for both interleaved and non- modes */ static int default_write_copy(struct snd_pcm_substream *substream, int channel, unsigned long hwoff, - void *buf, unsigned long bytes) + sockptr_t buf, unsigned long bytes) { - if (copy_from_user(get_dma_ptr(substream->runtime, channel, hwoff), - (void __user *)buf, bytes)) + if (copy_from_sockptr(get_dma_ptr(substream->runtime, channel, hwoff), + buf, bytes)) return -EFAULT; return 0; }
-/* default copy_kernel ops for write */ -static int default_write_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long hwoff, - void *buf, unsigned long bytes) -{ - memcpy(get_dma_ptr(substream->runtime, channel, hwoff), buf, bytes); - return 0; -} - /* fill silence instead of copy data; called as a transfer helper * from __snd_pcm_lib_write() or directly from noninterleaved_copy() when * a NULL buffer is passed */ static int fill_silence(struct snd_pcm_substream *substream, int channel, - unsigned long hwoff, void *buf, unsigned long bytes) + unsigned long hwoff, sockptr_t buf, unsigned long bytes) { struct snd_pcm_runtime *runtime = substream->runtime;
@@ -2027,27 +2019,42 @@ static int fill_silence(struct snd_pcm_substream *substream, int channel, return 0; }
-/* default copy_user ops for read; used for both interleaved and non- modes */ +/* default copy ops for read; used for both interleaved and non- modes */ static int default_read_copy(struct snd_pcm_substream *substream, int channel, unsigned long hwoff, - void *buf, unsigned long bytes) + sockptr_t buf, unsigned long bytes) { - if (copy_to_user((void __user *)buf, - get_dma_ptr(substream->runtime, channel, hwoff), - bytes)) + if (copy_to_sockptr(buf, + get_dma_ptr(substream->runtime, channel, hwoff), + bytes)) return -EFAULT; return 0; }
-/* default copy_kernel ops for read */ -static int default_read_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long hwoff, - void *buf, unsigned long bytes) +/* a wrapper for calling old copy_kernel or copy_user ops */ +static int call_old_copy(struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + sockptr_t buf, unsigned long bytes) { - memcpy(buf, get_dma_ptr(substream->runtime, channel, hwoff), bytes); - return 0; + if (sockptr_is_kernel(buf)) + return substream->ops->copy_kernel(substream, channel, hwoff, + buf.kernel, bytes); + else + return substream->ops->copy_user(substream, channel, hwoff, + buf.user, bytes); }
+/* create a sockptr_t */ +static inline sockptr_t make_sockptr(void *p, bool in_kernel) +{ + if (in_kernel) + return KERNEL_SOCKPTR(p); + else + return USER_SOCKPTR((void __user *)p); +} + +#define NULL_SOCKPTR USER_SOCKPTR(NULL) + /* call transfer function with the converted pointers and sizes; * for interleaved mode, it's one shot for all samples */ @@ -2055,7 +2062,8 @@ static int interleaved_copy(struct snd_pcm_substream *substream, snd_pcm_uframes_t hwoff, void *data, snd_pcm_uframes_t off, snd_pcm_uframes_t frames, - pcm_transfer_f transfer) + pcm_transfer_f transfer, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime;
@@ -2063,7 +2071,8 @@ static int interleaved_copy(struct snd_pcm_substream *substream, hwoff = frames_to_bytes(runtime, hwoff); off = frames_to_bytes(runtime, off); frames = frames_to_bytes(runtime, frames); - return transfer(substream, 0, hwoff, data + off, frames); + return transfer(substream, 0, hwoff, + make_sockptr(data + off, in_kernel), frames); }
/* call transfer function with the converted pointers and sizes for each @@ -2073,7 +2082,8 @@ static int noninterleaved_copy(struct snd_pcm_substream *substream, snd_pcm_uframes_t hwoff, void *data, snd_pcm_uframes_t off, snd_pcm_uframes_t frames, - pcm_transfer_f transfer) + pcm_transfer_f transfer, + bool in_kernel) { struct snd_pcm_runtime *runtime = substream->runtime; int channels = runtime->channels; @@ -2089,9 +2099,11 @@ static int noninterleaved_copy(struct snd_pcm_substream *substream, hwoff = samples_to_bytes(runtime, hwoff); for (c = 0; c < channels; ++c, ++bufs) { if (!data || !*bufs) - err = fill_silence(substream, c, hwoff, NULL, frames); + err = fill_silence(substream, c, hwoff, + NULL_SOCKPTR, frames); else - err = transfer(substream, c, hwoff, *bufs + off, + err = transfer(substream, c, hwoff, + make_sockptr(*bufs + off, in_kernel), frames); if (err < 0) return err; @@ -2108,10 +2120,10 @@ static int fill_silence_frames(struct snd_pcm_substream *substream, if (substream->runtime->access == SNDRV_PCM_ACCESS_RW_INTERLEAVED || substream->runtime->access == SNDRV_PCM_ACCESS_MMAP_INTERLEAVED) return interleaved_copy(substream, off, NULL, 0, frames, - fill_silence); + fill_silence, true); else return noninterleaved_copy(substream, off, NULL, 0, frames, - fill_silence); + fill_silence, true); }
/* sanity-check for read/write methods */ @@ -2121,7 +2133,7 @@ static int pcm_sanity_check(struct snd_pcm_substream *substream) if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; runtime = substream->runtime; - if (snd_BUG_ON(!substream->ops->copy_user && !runtime->dma_area)) + if (snd_BUG_ON(!substream->ops->copy && !runtime->dma_area)) return -EINVAL; if (runtime->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; @@ -2226,15 +2238,12 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, transfer = fill_silence; else return -EINVAL; - } else if (in_kernel) { - if (substream->ops->copy_kernel) - transfer = substream->ops->copy_kernel; - else - transfer = is_playback ? - default_write_copy_kernel : default_read_copy_kernel; } else { - if (substream->ops->copy_user) - transfer = (pcm_transfer_f)substream->ops->copy_user; + if (substream->ops->copy) + transfer = substream->ops->copy; + else if ((in_kernel && substream->ops->copy_kernel) || + (!in_kernel && substream->ops->copy_user)) + transfer = call_old_copy; else transfer = is_playback ? default_write_copy : default_read_copy; @@ -2307,7 +2316,7 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, if (!is_playback) snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_CPU); err = writer(substream, appl_ofs, data, offset, frames, - transfer); + transfer, in_kernel); if (is_playback) snd_pcm_dma_buffer_sync(substream, SNDRV_DMA_SYNC_DEVICE); snd_pcm_stream_lock_irq(substream); diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 95fc56e403b1..34efd4d198d6 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -809,7 +809,7 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, runtime->boundary *= 2;
/* clear the buffer for avoiding possible kernel info leaks */ - if (runtime->dma_area && !substream->ops->copy_user) { + if (runtime->dma_area && !substream->ops->copy && !substream->ops->copy_user) { size_t size = runtime->dma_bytes;
if (runtime->info & SNDRV_PCM_INFO_MMAP)
Add two more helpers for copying memory between sockptr and iomem, which will be used by the new PCM copy ops in a few drivers. It's just bridging to the existing helpers.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 4 ++++ sound/core/memory.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 04b935200d0e..9b7054f0cea0 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -1559,6 +1559,10 @@ static inline u64 pcm_format_to_bits(snd_pcm_format_t pcm_format) #define pcm_dbg(pcm, fmt, args...) \ dev_dbg((pcm)->card->dev, fmt, ##args)
+/* helpers for copying between sockptr and iomem */ +int copy_to_sockptr_fromio(sockptr_t dst, const void __iomem *src, size_t count); +int copy_from_sockptr_toio(void __iomem *dst, sockptr_t src, size_t count); + struct snd_pcm_status64 { snd_pcm_state_t state; /* stream state */ u8 rsvd[4]; diff --git a/sound/core/memory.c b/sound/core/memory.c index 5d894dc32f7d..ff4a9e0064a2 100644 --- a/sound/core/memory.c +++ b/sound/core/memory.c @@ -9,6 +9,7 @@ #include <linux/io.h> #include <linux/uaccess.h> #include <sound/core.h> +#include <sound/pcm.h>
/** * copy_to_user_fromio - copy data from mmio-space to user-space @@ -73,3 +74,41 @@ int copy_from_user_toio(volatile void __iomem *dst, const void __user *src, size #endif } EXPORT_SYMBOL(copy_from_user_toio); + +/** + * copy_to_sockptr_fromio - copy data from mmio-space to sockptr + * @dst: the destination pointer + * @src: the source pointer on mmio + * @count: the data size to copy in bytes + * + * Copies the data from mmio-space to the generic pointer address + * + * Return: Zero if successful, or -EFAULT on failure + */ +int copy_to_sockptr_fromio(sockptr_t dst, const void __iomem *src, size_t count) +{ + if (!sockptr_is_kernel(dst)) + return copy_to_user_fromio(dst.user, src, count); + memcpy_fromio(dst.kernel, src, count); + return 0; +} +EXPORT_SYMBOL(copy_to_sockptr_fromio); + +/** + * copy_from_sockptr_toio - copy data from sockptr to mmio-space + * @dst: the destination pointer on mmio-space + * @src: the source pointer + * @count: the data size to copy in bytes + * + * Copies the data from the generic pointer address to mmio-space. + * + * Return: Zero if successful, or -EFAULT on failure + */ +int copy_from_sockptr_toio(void __iomem *dst, sockptr_t src, size_t count) +{ + if (!sockptr_is_kernel(src)) + return copy_from_user_toio(dst, src.user, count); + memcpy_toio(dst, src.kernel, count); + return 0; +} +EXPORT_SYMBOL(copy_from_sockptr_toio);
This patch converts the dummy driver code to use the new unified PCM copy callback. As dummy driver doesn't do anything in the callback, it's just a simple replacement.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/drivers/dummy.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c index 9c17b49a2ae1..eef71535e4d0 100644 --- a/sound/drivers/dummy.c +++ b/sound/drivers/dummy.c @@ -626,14 +626,7 @@ static int alloc_fake_buffer(void)
static int dummy_pcm_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long bytes) -{ - return 0; /* do nothing */ -} - -static int dummy_pcm_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long bytes) + sockptr_t dst, unsigned long bytes) { return 0; /* do nothing */ } @@ -667,8 +660,7 @@ static const struct snd_pcm_ops dummy_pcm_ops_no_buf = { .prepare = dummy_pcm_prepare, .trigger = dummy_pcm_trigger, .pointer = dummy_pcm_pointer, - .copy_user = dummy_pcm_copy, - .copy_kernel = dummy_pcm_copy_kernel, + .copy = dummy_pcm_copy, .fill_silence = dummy_pcm_silence, .page = dummy_pcm_page, };
This patch converts the GUS driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/isa/gus/gus_pcm.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/sound/isa/gus/gus_pcm.c b/sound/isa/gus/gus_pcm.c index 388db5fb65bd..4056961230ad 100644 --- a/sound/isa/gus/gus_pcm.c +++ b/sound/isa/gus/gus_pcm.c @@ -369,7 +369,7 @@ static int playback_copy_ack(struct snd_pcm_substream *substream,
static int snd_gf1_pcm_playback_copy(struct snd_pcm_substream *substream, int voice, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct gus_pcm_private *pcmp = runtime->private_data; @@ -379,27 +379,11 @@ static int snd_gf1_pcm_playback_copy(struct snd_pcm_substream *substream, bpos = get_bpos(pcmp, voice, pos, len); if (bpos < 0) return pos; - if (copy_from_user(runtime->dma_area + bpos, src, len)) + if (copy_from_sockptr(runtime->dma_area + bpos, src, len)) return -EFAULT; return playback_copy_ack(substream, bpos, len); }
-static int snd_gf1_pcm_playback_copy_kernel(struct snd_pcm_substream *substream, - int voice, unsigned long pos, - void *src, unsigned long count) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - struct gus_pcm_private *pcmp = runtime->private_data; - unsigned int len = count; - int bpos; - - bpos = get_bpos(pcmp, voice, pos, len); - if (bpos < 0) - return pos; - memcpy(runtime->dma_area + bpos, src, len); - return playback_copy_ack(substream, bpos, len); -} - static int snd_gf1_pcm_playback_silence(struct snd_pcm_substream *substream, int voice, unsigned long pos, unsigned long count) @@ -830,8 +814,7 @@ static const struct snd_pcm_ops snd_gf1_pcm_playback_ops = { .prepare = snd_gf1_pcm_playback_prepare, .trigger = snd_gf1_pcm_playback_trigger, .pointer = snd_gf1_pcm_playback_pointer, - .copy_user = snd_gf1_pcm_playback_copy, - .copy_kernel = snd_gf1_pcm_playback_copy_kernel, + .copy = snd_gf1_pcm_playback_copy, .fill_silence = snd_gf1_pcm_playback_silence, };
This patch converts the SB Emu8000 driver code to use the new unified PCM copy callback. The conversion is a bit complicated because of many open code in emu8000_pcm.c. GET_VAL() and LOOP_WRITE() macros were rewritten / simplified with copy_from_sockptr_offset().
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/isa/sb/emu8000_pcm.c | 79 ++++++++++---------------------------- 1 file changed, 21 insertions(+), 58 deletions(-)
diff --git a/sound/isa/sb/emu8000_pcm.c b/sound/isa/sb/emu8000_pcm.c index c8afc4347c54..31a5e5fa278d 100644 --- a/sound/isa/sb/emu8000_pcm.c +++ b/sound/isa/sb/emu8000_pcm.c @@ -409,40 +409,28 @@ do { \ return -EAGAIN;\ } while (0)
-enum { - COPY_USER, COPY_KERNEL, FILL_SILENCE, -}; - -#define GET_VAL(sval, buf, mode) \ +#define GET_VAL(sval, buf, offset) \ do { \ - switch (mode) { \ - case FILL_SILENCE: \ + if (sockptr_is_null(buf)) \ sval = 0; \ - break; \ - case COPY_KERNEL: \ - sval = *buf++; \ - break; \ - default: \ - if (get_user(sval, (unsigned short __user *)buf)) \ - return -EFAULT; \ - buf++; \ - break; \ - } \ + else if (copy_from_sockptr_offset(&sval, buf, offset, 2)) \ + return -EFAULT; \ } while (0)
#ifdef USE_NONINTERLEAVE
-#define LOOP_WRITE(rec, offset, _buf, count, mode) \ +#define LOOP_WRITE(rec, offset, buf, count) \ do { \ struct snd_emu8000 *emu = (rec)->emu; \ - unsigned short *buf = (__force unsigned short *)(_buf); \ + unsigned int offset = 0; \ snd_emu8000_write_wait(emu, 1); \ EMU8000_SMALW_WRITE(emu, offset); \ while (count > 0) { \ unsigned short sval; \ CHECK_SCHEDULER(); \ - GET_VAL(sval, buf, mode); \ + GET_VAL(sval, buf, offset); \ EMU8000_SMLD_WRITE(emu, sval); \ + offset += 2; \ count--; \ } \ } while (0) @@ -450,27 +438,14 @@ enum { /* copy one channel block */ static int emu8k_pcm_copy(struct snd_pcm_substream *subs, int voice, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { struct snd_emu8k_pcm *rec = subs->runtime->private_data;
/* convert to word unit */ pos = (pos << 1) + rec->loop_start[voice]; count <<= 1; - LOOP_WRITE(rec, pos, src, count, COPY_USER); - return 0; -} - -static int emu8k_pcm_copy_kernel(struct snd_pcm_substream *subs, - int voice, unsigned long pos, - void *src, unsigned long count) -{ - struct snd_emu8k_pcm *rec = subs->runtime->private_data; - - /* convert to word unit */ - pos = (pos << 1) + rec->loop_start[voice]; - count <<= 1; - LOOP_WRITE(rec, pos, src, count, COPY_KERNEL); + LOOP_WRITE(rec, pos, src, count); return 0; }
@@ -483,16 +458,16 @@ static int emu8k_pcm_silence(struct snd_pcm_substream *subs, /* convert to word unit */ pos = (pos << 1) + rec->loop_start[voice]; count <<= 1; - LOOP_WRITE(rec, pos, NULL, count, FILL_SILENCE); + LOOP_WRITE(rec, pos, USER_SOCKPTR(NULL), count); return 0; }
#else /* interleave */
-#define LOOP_WRITE(rec, pos, _buf, count, mode) \ +#define LOOP_WRITE(rec, pos, buf, count) \ do { \ struct snd_emu8000 *emu = rec->emu; \ - unsigned short *buf = (__force unsigned short *)(_buf); \ + unsigned int offset = 0; \ snd_emu8000_write_wait(emu, 1); \ EMU8000_SMALW_WRITE(emu, pos + rec->loop_start[0]); \ if (rec->voices > 1) \ @@ -500,12 +475,14 @@ static int emu8k_pcm_silence(struct snd_pcm_substream *subs, while (count > 0) { \ unsigned short sval; \ CHECK_SCHEDULER(); \ - GET_VAL(sval, buf, mode); \ + GET_VAL(sval, buf, offset); \ EMU8000_SMLD_WRITE(emu, sval); \ + offset += 2; \ if (rec->voices > 1) { \ CHECK_SCHEDULER(); \ - GET_VAL(sval, buf, mode); \ + GET_VAL(sval, buf, offset); \ EMU8000_SMRD_WRITE(emu, sval); \ + offset += 2; \ } \ count--; \ } \ @@ -518,27 +495,14 @@ static int emu8k_pcm_silence(struct snd_pcm_substream *subs, */ static int emu8k_pcm_copy(struct snd_pcm_substream *subs, int voice, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { struct snd_emu8k_pcm *rec = subs->runtime->private_data;
/* convert to frames */ pos = bytes_to_frames(subs->runtime, pos); count = bytes_to_frames(subs->runtime, count); - LOOP_WRITE(rec, pos, src, count, COPY_USER); - return 0; -} - -static int emu8k_pcm_copy_kernel(struct snd_pcm_substream *subs, - int voice, unsigned long pos, - void *src, unsigned long count) -{ - struct snd_emu8k_pcm *rec = subs->runtime->private_data; - - /* convert to frames */ - pos = bytes_to_frames(subs->runtime, pos); - count = bytes_to_frames(subs->runtime, count); - LOOP_WRITE(rec, pos, src, count, COPY_KERNEL); + LOOP_WRITE(rec, pos, src, count); return 0; }
@@ -550,7 +514,7 @@ static int emu8k_pcm_silence(struct snd_pcm_substream *subs, /* convert to frames */ pos = bytes_to_frames(subs->runtime, pos); count = bytes_to_frames(subs->runtime, count); - LOOP_WRITE(rec, pos, NULL, count, FILL_SILENCE); + LOOP_WRITE(rec, pos, USER_SOCKPTR(NULL), count); return 0; } #endif @@ -666,8 +630,7 @@ static const struct snd_pcm_ops emu8k_pcm_ops = { .prepare = emu8k_pcm_prepare, .trigger = emu8k_pcm_trigger, .pointer = emu8k_pcm_pointer, - .copy_user = emu8k_pcm_copy, - .copy_kernel = emu8k_pcm_copy_kernel, + .copy = emu8k_pcm_copy, .fill_silence = emu8k_pcm_silence, };
This patch converts the es1938 driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants in most parts. The only no 1:1 conversion is put_user() for a byte write, which was replaced with copy_to_sockptr*(), too.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/es1938.c | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-)
diff --git a/sound/pci/es1938.c b/sound/pci/es1938.c index e34ec6f89e7e..1fddbca395d0 100644 --- a/sound/pci/es1938.c +++ b/sound/pci/es1938.c @@ -824,7 +824,7 @@ static snd_pcm_uframes_t snd_es1938_playback_pointer(struct snd_pcm_substream *s
static int snd_es1938_capture_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long count) + sockptr_t dst, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct es1938 *chip = snd_pcm_substream_chip(substream); @@ -832,36 +832,18 @@ static int snd_es1938_capture_copy(struct snd_pcm_substream *substream, if (snd_BUG_ON(pos + count > chip->dma1_size)) return -EINVAL; if (pos + count < chip->dma1_size) { - if (copy_to_user(dst, runtime->dma_area + pos + 1, count)) + if (copy_to_sockptr(dst, runtime->dma_area + pos + 1, count)) return -EFAULT; } else { - if (copy_to_user(dst, runtime->dma_area + pos + 1, count - 1)) + if (copy_to_sockptr(dst, runtime->dma_area + pos + 1, count - 1)) return -EFAULT; - if (put_user(runtime->dma_area[0], - ((unsigned char __user *)dst) + count - 1)) + if (copy_to_sockptr_offset(dst, count - 1, + runtime->dma_area, 1)) return -EFAULT; } return 0; }
-static int snd_es1938_capture_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long count) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - struct es1938 *chip = snd_pcm_substream_chip(substream); - - if (snd_BUG_ON(pos + count > chip->dma1_size)) - return -EINVAL; - if (pos + count < chip->dma1_size) { - memcpy(dst, runtime->dma_area + pos + 1, count); - } else { - memcpy(dst, runtime->dma_area + pos + 1, count - 1); - runtime->dma_area[0] = *((unsigned char *)dst + count - 1); - } - return 0; -} - /* ---------------------------------------------------------------------- * Audio1 Capture (ADC) * ----------------------------------------------------------------------*/ @@ -987,8 +969,7 @@ static const struct snd_pcm_ops snd_es1938_capture_ops = { .prepare = snd_es1938_capture_prepare, .trigger = snd_es1938_capture_trigger, .pointer = snd_es1938_capture_pointer, - .copy_user = snd_es1938_capture_copy, - .copy_kernel = snd_es1938_capture_copy_kernel, + .copy = snd_es1938_capture_copy, };
static int snd_es1938_new_pcm(struct es1938 *chip, int device)
This patch converts the korg1212 driver code to use the new unified PCM copy callback. The open-coded conditional memory copies are replaced with simpler copy_from/to_sockptr_offset() calls.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/korg1212/korg1212.c | 46 ++++++++--------------------------- 1 file changed, 10 insertions(+), 36 deletions(-)
diff --git a/sound/pci/korg1212/korg1212.c b/sound/pci/korg1212/korg1212.c index 33b4f95d65b3..644f8b392c54 100644 --- a/sound/pci/korg1212/korg1212.c +++ b/sound/pci/korg1212/korg1212.c @@ -1285,8 +1285,7 @@ static int snd_korg1212_silence(struct snd_korg1212 *korg1212, int pos, int coun }
static int snd_korg1212_copy_to(struct snd_pcm_substream *substream, - void __user *dst, int pos, int count, - bool in_kernel) + sockptr_t dst, int pos, int count) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_korg1212 *korg1212 = snd_pcm_substream_chip(substream); @@ -1310,20 +1309,16 @@ static int snd_korg1212_copy_to(struct snd_pcm_substream *substream, return -EFAULT; } #endif - if (in_kernel) - memcpy((__force void *)dst, src, size); - else if (copy_to_user(dst, src, size)) + if (copy_to_sockptr_offset(dst, i * size, src, size)) return -EFAULT; src++; - dst += size; }
return 0; }
static int snd_korg1212_copy_from(struct snd_pcm_substream *substream, - void __user *src, int pos, int count, - bool in_kernel) + sockptr_t src, int pos, int count) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_korg1212 *korg1212 = snd_pcm_substream_chip(substream); @@ -1349,12 +1344,9 @@ static int snd_korg1212_copy_from(struct snd_pcm_substream *substream, return -EFAULT; } #endif - if (in_kernel) - memcpy(dst, (__force void *)src, size); - else if (copy_from_user(dst, src, size)) + if (copy_from_sockptr_offset(dst, src, i * size, size)) return -EFAULT; dst++; - src += size; }
return 0; @@ -1642,17 +1634,9 @@ static snd_pcm_uframes_t snd_korg1212_capture_pointer(struct snd_pcm_substream *
static int snd_korg1212_playback_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { - return snd_korg1212_copy_from(substream, src, pos, count, false); -} - -static int snd_korg1212_playback_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *src, unsigned long count) -{ - return snd_korg1212_copy_from(substream, (void __user *)src, - pos, count, true); + return snd_korg1212_copy_from(substream, src, pos, count); }
static int snd_korg1212_playback_silence(struct snd_pcm_substream *substream, @@ -1670,17 +1654,9 @@ static int snd_korg1212_playback_silence(struct snd_pcm_substream *substream,
static int snd_korg1212_capture_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long count) + sockptr_t dst, unsigned long count) { - return snd_korg1212_copy_to(substream, dst, pos, count, false); -} - -static int snd_korg1212_capture_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long count) -{ - return snd_korg1212_copy_to(substream, (void __user *)dst, - pos, count, true); + return snd_korg1212_copy_to(substream, dst, pos, count); }
static const struct snd_pcm_ops snd_korg1212_playback_ops = { @@ -1691,8 +1667,7 @@ static const struct snd_pcm_ops snd_korg1212_playback_ops = { .prepare = snd_korg1212_prepare, .trigger = snd_korg1212_trigger, .pointer = snd_korg1212_playback_pointer, - .copy_user = snd_korg1212_playback_copy, - .copy_kernel = snd_korg1212_playback_copy_kernel, + .copy = snd_korg1212_playback_copy, .fill_silence = snd_korg1212_playback_silence, };
@@ -1704,8 +1679,7 @@ static const struct snd_pcm_ops snd_korg1212_capture_ops = { .prepare = snd_korg1212_prepare, .trigger = snd_korg1212_trigger, .pointer = snd_korg1212_capture_pointer, - .copy_user = snd_korg1212_capture_copy, - .copy_kernel = snd_korg1212_capture_copy_kernel, + .copy = snd_korg1212_capture_copy, };
/*
This patch converts the nm256 driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/nm256/nm256.c | 42 ++++++----------------------------------- 1 file changed, 6 insertions(+), 36 deletions(-)
diff --git a/sound/pci/nm256/nm256.c b/sound/pci/nm256/nm256.c index f99a1e96e923..136fd3fe3005 100644 --- a/sound/pci/nm256/nm256.c +++ b/sound/pci/nm256/nm256.c @@ -691,26 +691,12 @@ snd_nm256_playback_silence(struct snd_pcm_substream *substream, static int snd_nm256_playback_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct nm256_stream *s = runtime->private_data;
- if (copy_from_user_toio(s->bufptr + pos, src, count)) - return -EFAULT; - return 0; -} - -static int -snd_nm256_playback_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *src, unsigned long count) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - struct nm256_stream *s = runtime->private_data; - - memcpy_toio(s->bufptr + pos, src, count); - return 0; + return copy_from_sockptr_toio(s->bufptr + pos, src, count); }
/* @@ -719,26 +705,12 @@ snd_nm256_playback_copy_kernel(struct snd_pcm_substream *substream, static int snd_nm256_capture_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long count) + sockptr_t dst, unsigned long count) { struct snd_pcm_runtime *runtime = substream->runtime; struct nm256_stream *s = runtime->private_data;
- if (copy_to_user_fromio(dst, s->bufptr + pos, count)) - return -EFAULT; - return 0; -} - -static int -snd_nm256_capture_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long count) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - struct nm256_stream *s = runtime->private_data; - - memcpy_fromio(dst, s->bufptr + pos, count); - return 0; + return copy_to_sockptr_fromio(dst, s->bufptr + pos, count); }
#endif /* !__i386__ */ @@ -909,8 +881,7 @@ static const struct snd_pcm_ops snd_nm256_playback_ops = { .trigger = snd_nm256_playback_trigger, .pointer = snd_nm256_playback_pointer, #ifndef __i386__ - .copy_user = snd_nm256_playback_copy, - .copy_kernel = snd_nm256_playback_copy_kernel, + .copy = snd_nm256_playback_copy, .fill_silence = snd_nm256_playback_silence, #endif .mmap = snd_pcm_lib_mmap_iomem, @@ -924,8 +895,7 @@ static const struct snd_pcm_ops snd_nm256_capture_ops = { .trigger = snd_nm256_capture_trigger, .pointer = snd_nm256_capture_pointer, #ifndef __i386__ - .copy_user = snd_nm256_capture_copy, - .copy_kernel = snd_nm256_capture_copy_kernel, + .copy = snd_nm256_capture_copy, #endif .mmap = snd_pcm_lib_mmap_iomem, };
This patch converts the rme32 driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/rme32.c | 50 +++++++++++------------------------------------ 1 file changed, 11 insertions(+), 39 deletions(-)
diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c index 9c0ac025e143..295c13829521 100644 --- a/sound/pci/rme32.c +++ b/sound/pci/rme32.c @@ -252,48 +252,24 @@ static int snd_rme32_playback_silence(struct snd_pcm_substream *substream, /* copy callback for halfduplex mode */ static int snd_rme32_playback_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { struct rme32 *rme32 = snd_pcm_substream_chip(substream);
- if (copy_from_user_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos, - src, count)) - return -EFAULT; - return 0; -} - -static int snd_rme32_playback_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *src, unsigned long count) -{ - struct rme32 *rme32 = snd_pcm_substream_chip(substream); - - memcpy_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos, src, count); - return 0; + return copy_from_sockptr_toio(rme32->iobase + RME32_IO_DATA_BUFFER + pos, + src, count); }
/* copy callback for halfduplex mode */ static int snd_rme32_capture_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long count) + sockptr_t dst, unsigned long count) { struct rme32 *rme32 = snd_pcm_substream_chip(substream);
- if (copy_to_user_fromio(dst, - rme32->iobase + RME32_IO_DATA_BUFFER + pos, - count)) - return -EFAULT; - return 0; -} - -static int snd_rme32_capture_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long count) -{ - struct rme32 *rme32 = snd_pcm_substream_chip(substream); - - memcpy_fromio(dst, rme32->iobase + RME32_IO_DATA_BUFFER + pos, count); - return 0; + return copy_to_sockptr_fromio(dst, + rme32->iobase + RME32_IO_DATA_BUFFER + pos, + count); }
/* @@ -1194,8 +1170,7 @@ static const struct snd_pcm_ops snd_rme32_playback_spdif_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer, - .copy_user = snd_rme32_playback_copy, - .copy_kernel = snd_rme32_playback_copy_kernel, + .copy = snd_rme32_playback_copy, .fill_silence = snd_rme32_playback_silence, .mmap = snd_pcm_lib_mmap_iomem, }; @@ -1207,8 +1182,7 @@ static const struct snd_pcm_ops snd_rme32_capture_spdif_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer, - .copy_user = snd_rme32_capture_copy, - .copy_kernel = snd_rme32_capture_copy_kernel, + .copy = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem, };
@@ -1219,8 +1193,7 @@ static const struct snd_pcm_ops snd_rme32_playback_adat_ops = { .prepare = snd_rme32_playback_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_playback_pointer, - .copy_user = snd_rme32_playback_copy, - .copy_kernel = snd_rme32_playback_copy_kernel, + .copy = snd_rme32_playback_copy, .fill_silence = snd_rme32_playback_silence, .mmap = snd_pcm_lib_mmap_iomem, }; @@ -1232,8 +1205,7 @@ static const struct snd_pcm_ops snd_rme32_capture_adat_ops = { .prepare = snd_rme32_capture_prepare, .trigger = snd_rme32_pcm_trigger, .pointer = snd_rme32_capture_pointer, - .copy_user = snd_rme32_capture_copy, - .copy_kernel = snd_rme32_capture_copy_kernel, + .copy = snd_rme32_capture_copy, .mmap = snd_pcm_lib_mmap_iomem, };
This patch converts the rme96 driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/rme96.c | 48 +++++++++++------------------------------------ 1 file changed, 11 insertions(+), 37 deletions(-)
diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index bccb7e0d3d11..fd22aaca7a4b 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -320,46 +320,24 @@ snd_rme96_playback_silence(struct snd_pcm_substream *substream, static int snd_rme96_playback_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { struct rme96 *rme96 = snd_pcm_substream_chip(substream);
- return copy_from_user_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, - src, count); -} - -static int -snd_rme96_playback_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *src, unsigned long count) -{ - struct rme96 *rme96 = snd_pcm_substream_chip(substream); - - memcpy_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, src, count); - return 0; + return copy_from_sockptr_toio(rme96->iobase + RME96_IO_PLAY_BUFFER + pos, + src, count); }
static int snd_rme96_capture_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long count) + sockptr_t dst, unsigned long count) { struct rme96 *rme96 = snd_pcm_substream_chip(substream);
- return copy_to_user_fromio(dst, - rme96->iobase + RME96_IO_REC_BUFFER + pos, - count); -} - -static int -snd_rme96_capture_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long count) -{ - struct rme96 *rme96 = snd_pcm_substream_chip(substream); - - memcpy_fromio(dst, rme96->iobase + RME96_IO_REC_BUFFER + pos, count); - return 0; + return copy_to_sockptr_fromio(dst, + rme96->iobase + RME96_IO_REC_BUFFER + pos, + count); }
/* @@ -1518,8 +1496,7 @@ static const struct snd_pcm_ops snd_rme96_playback_spdif_ops = { .prepare = snd_rme96_playback_prepare, .trigger = snd_rme96_playback_trigger, .pointer = snd_rme96_playback_pointer, - .copy_user = snd_rme96_playback_copy, - .copy_kernel = snd_rme96_playback_copy_kernel, + .copy = snd_rme96_playback_copy, .fill_silence = snd_rme96_playback_silence, .mmap = snd_pcm_lib_mmap_iomem, }; @@ -1531,8 +1508,7 @@ static const struct snd_pcm_ops snd_rme96_capture_spdif_ops = { .prepare = snd_rme96_capture_prepare, .trigger = snd_rme96_capture_trigger, .pointer = snd_rme96_capture_pointer, - .copy_user = snd_rme96_capture_copy, - .copy_kernel = snd_rme96_capture_copy_kernel, + .copy = snd_rme96_capture_copy, .mmap = snd_pcm_lib_mmap_iomem, };
@@ -1543,8 +1519,7 @@ static const struct snd_pcm_ops snd_rme96_playback_adat_ops = { .prepare = snd_rme96_playback_prepare, .trigger = snd_rme96_playback_trigger, .pointer = snd_rme96_playback_pointer, - .copy_user = snd_rme96_playback_copy, - .copy_kernel = snd_rme96_playback_copy_kernel, + .copy = snd_rme96_playback_copy, .fill_silence = snd_rme96_playback_silence, .mmap = snd_pcm_lib_mmap_iomem, }; @@ -1556,8 +1531,7 @@ static const struct snd_pcm_ops snd_rme96_capture_adat_ops = { .prepare = snd_rme96_capture_prepare, .trigger = snd_rme96_capture_trigger, .pointer = snd_rme96_capture_pointer, - .copy_user = snd_rme96_capture_copy, - .copy_kernel = snd_rme96_capture_copy_kernel, + .copy = snd_rme96_capture_copy, .mmap = snd_pcm_lib_mmap_iomem, };
This patch converts the hdsp driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/rme9652/hdsp.c | 42 ++++++---------------------------------- 1 file changed, 6 insertions(+), 36 deletions(-)
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c index 65add92c88aa..6d8410ea4244 100644 --- a/sound/pci/rme9652/hdsp.c +++ b/sound/pci/rme9652/hdsp.c @@ -3961,7 +3961,7 @@ static signed char *hdsp_channel_buffer_location(struct hdsp *hdsp,
static int snd_hdsp_playback_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { struct hdsp *hdsp = snd_pcm_substream_chip(substream); signed char *channel_buf; @@ -3972,28 +3972,14 @@ static int snd_hdsp_playback_copy(struct snd_pcm_substream *substream, channel_buf = hdsp_channel_buffer_location (hdsp, substream->pstr->stream, channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - if (copy_from_user(channel_buf + pos, src, count)) + if (copy_from_sockptr(channel_buf + pos, src, count)) return -EFAULT; return 0; }
-static int snd_hdsp_playback_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *src, unsigned long count) -{ - struct hdsp *hdsp = snd_pcm_substream_chip(substream); - signed char *channel_buf; - - channel_buf = hdsp_channel_buffer_location(hdsp, substream->pstr->stream, channel); - if (snd_BUG_ON(!channel_buf)) - return -EIO; - memcpy(channel_buf + pos, src, count); - return 0; -} - static int snd_hdsp_capture_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long count) + sockptr_t dst, unsigned long count) { struct hdsp *hdsp = snd_pcm_substream_chip(substream); signed char *channel_buf; @@ -4004,25 +3990,11 @@ static int snd_hdsp_capture_copy(struct snd_pcm_substream *substream, channel_buf = hdsp_channel_buffer_location (hdsp, substream->pstr->stream, channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - if (copy_to_user(dst, channel_buf + pos, count)) + if (copy_to_sockptr(dst, channel_buf + pos, count)) return -EFAULT; return 0; }
-static int snd_hdsp_capture_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long count) -{ - struct hdsp *hdsp = snd_pcm_substream_chip(substream); - signed char *channel_buf; - - channel_buf = hdsp_channel_buffer_location(hdsp, substream->pstr->stream, channel); - if (snd_BUG_ON(!channel_buf)) - return -EIO; - memcpy(dst, channel_buf + pos, count); - return 0; -} - static int snd_hdsp_hw_silence(struct snd_pcm_substream *substream, int channel, unsigned long pos, unsigned long count) @@ -4950,8 +4922,7 @@ static const struct snd_pcm_ops snd_hdsp_playback_ops = { .prepare = snd_hdsp_prepare, .trigger = snd_hdsp_trigger, .pointer = snd_hdsp_hw_pointer, - .copy_user = snd_hdsp_playback_copy, - .copy_kernel = snd_hdsp_playback_copy_kernel, + .copy = snd_hdsp_playback_copy, .fill_silence = snd_hdsp_hw_silence, };
@@ -4963,8 +4934,7 @@ static const struct snd_pcm_ops snd_hdsp_capture_ops = { .prepare = snd_hdsp_prepare, .trigger = snd_hdsp_trigger, .pointer = snd_hdsp_hw_pointer, - .copy_user = snd_hdsp_capture_copy, - .copy_kernel = snd_hdsp_capture_copy_kernel, + .copy = snd_hdsp_capture_copy, };
static int snd_hdsp_create_hwdep(struct snd_card *card, struct hdsp *hdsp)
This patch converts the rme9652 driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/pci/rme9652/rme9652.c | 46 +++++-------------------------------- 1 file changed, 6 insertions(+), 40 deletions(-)
diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c index e7c320afefe8..a1f844799466 100644 --- a/sound/pci/rme9652/rme9652.c +++ b/sound/pci/rme9652/rme9652.c @@ -1844,7 +1844,7 @@ static signed char *rme9652_channel_buffer_location(struct snd_rme9652 *rme9652,
static int snd_rme9652_playback_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); signed char *channel_buf; @@ -1857,30 +1857,14 @@ static int snd_rme9652_playback_copy(struct snd_pcm_substream *substream, channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - if (copy_from_user(channel_buf + pos, src, count)) + if (copy_from_sockptr(channel_buf + pos, src, count)) return -EFAULT; return 0; }
-static int snd_rme9652_playback_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *src, unsigned long count) -{ - struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); - signed char *channel_buf; - - channel_buf = rme9652_channel_buffer_location(rme9652, - substream->pstr->stream, - channel); - if (snd_BUG_ON(!channel_buf)) - return -EIO; - memcpy(channel_buf + pos, src, count); - return 0; -} - static int snd_rme9652_capture_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long count) + sockptr_t dst, unsigned long count) { struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); signed char *channel_buf; @@ -1893,27 +1877,11 @@ static int snd_rme9652_capture_copy(struct snd_pcm_substream *substream, channel); if (snd_BUG_ON(!channel_buf)) return -EIO; - if (copy_to_user(dst, channel_buf + pos, count)) + if (copy_to_sockptr(dst, channel_buf + pos, count)) return -EFAULT; return 0; }
-static int snd_rme9652_capture_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long count) -{ - struct snd_rme9652 *rme9652 = snd_pcm_substream_chip(substream); - signed char *channel_buf; - - channel_buf = rme9652_channel_buffer_location(rme9652, - substream->pstr->stream, - channel); - if (snd_BUG_ON(!channel_buf)) - return -EIO; - memcpy(dst, channel_buf + pos, count); - return 0; -} - static int snd_rme9652_hw_silence(struct snd_pcm_substream *substream, int channel, unsigned long pos, unsigned long count) @@ -2370,8 +2338,7 @@ static const struct snd_pcm_ops snd_rme9652_playback_ops = { .prepare = snd_rme9652_prepare, .trigger = snd_rme9652_trigger, .pointer = snd_rme9652_hw_pointer, - .copy_user = snd_rme9652_playback_copy, - .copy_kernel = snd_rme9652_playback_copy_kernel, + .copy = snd_rme9652_playback_copy, .fill_silence = snd_rme9652_hw_silence, };
@@ -2383,8 +2350,7 @@ static const struct snd_pcm_ops snd_rme9652_capture_ops = { .prepare = snd_rme9652_prepare, .trigger = snd_rme9652_trigger, .pointer = snd_rme9652_hw_pointer, - .copy_user = snd_rme9652_capture_copy, - .copy_kernel = snd_rme9652_capture_copy_kernel, + .copy = snd_rme9652_capture_copy, };
static int snd_rme9652_create_pcm(struct snd_card *card,
This patch converts the sh_dac_audio driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/sh/sh_dac_audio.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-)
diff --git a/sound/sh/sh_dac_audio.c b/sound/sh/sh_dac_audio.c index 8cf571955c9d..40e783219b44 100644 --- a/sound/sh/sh_dac_audio.c +++ b/sound/sh/sh_dac_audio.c @@ -158,12 +158,12 @@ static int snd_sh_dac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *src, unsigned long count) + sockptr_t src, unsigned long count) { /* channel is not used (interleaved data) */ struct snd_sh_dac *chip = snd_pcm_substream_chip(substream);
- if (copy_from_user_toio(chip->data_buffer + pos, src, count)) + if (copy_from_sockptr_toio(chip->data_buffer + pos, src, count)) return -EFAULT; chip->buffer_end = chip->data_buffer + pos + count;
@@ -175,24 +175,6 @@ static int snd_sh_dac_pcm_copy(struct snd_pcm_substream *substream, return 0; }
-static int snd_sh_dac_pcm_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *src, unsigned long count) -{ - /* channel is not used (interleaved data) */ - struct snd_sh_dac *chip = snd_pcm_substream_chip(substream); - - memcpy_toio(chip->data_buffer + pos, src, count); - chip->buffer_end = chip->data_buffer + pos + count; - - if (chip->empty) { - chip->empty = 0; - dac_audio_start_timer(chip); - } - - return 0; -} - static int snd_sh_dac_pcm_silence(struct snd_pcm_substream *substream, int channel, unsigned long pos, unsigned long count) @@ -227,8 +209,7 @@ static const struct snd_pcm_ops snd_sh_dac_pcm_ops = { .prepare = snd_sh_dac_pcm_prepare, .trigger = snd_sh_dac_pcm_trigger, .pointer = snd_sh_dac_pcm_pointer, - .copy_user = snd_sh_dac_pcm_copy, - .copy_kernel = snd_sh_dac_pcm_copy_kernel, + .copy = snd_sh_dac_pcm_copy, .fill_silence = snd_sh_dac_pcm_silence, .mmap = snd_pcm_lib_mmap_iomem, };
This patch converts the xen frontend driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Cc: Oleksandr Andrushchenko oleksandr_andrushchenko@epam.com Cc: xen-devel@lists.xenproject.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/xen/xen_snd_front_alsa.c | 55 +++++++--------------------------- 1 file changed, 10 insertions(+), 45 deletions(-)
diff --git a/sound/xen/xen_snd_front_alsa.c b/sound/xen/xen_snd_front_alsa.c index 7a3dfce97c15..a7e2cba3ff4e 100644 --- a/sound/xen/xen_snd_front_alsa.c +++ b/sound/xen/xen_snd_front_alsa.c @@ -602,38 +602,24 @@ static snd_pcm_uframes_t alsa_pointer(struct snd_pcm_substream *substream) return (snd_pcm_uframes_t)atomic_read(&stream->hw_ptr); }
-static int alsa_pb_copy_user(struct snd_pcm_substream *substream, - int channel, unsigned long pos, void __user *src, - unsigned long count) +static int alsa_pb_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, sockptr_t src, + unsigned long count) { struct xen_snd_front_pcm_stream_info *stream = stream_get(substream);
if (unlikely(pos + count > stream->buffer_sz)) return -EINVAL;
- if (copy_from_user(stream->buffer + pos, src, count)) + if (copy_from_sockptr(stream->buffer + pos, src, count)) return -EFAULT;
return xen_snd_front_stream_write(&stream->evt_pair->req, pos, count); }
-static int alsa_pb_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, void *src, - unsigned long count) -{ - struct xen_snd_front_pcm_stream_info *stream = stream_get(substream); - - if (unlikely(pos + count > stream->buffer_sz)) - return -EINVAL; - - memcpy(stream->buffer + pos, src, count); - - return xen_snd_front_stream_write(&stream->evt_pair->req, pos, count); -} - -static int alsa_cap_copy_user(struct snd_pcm_substream *substream, - int channel, unsigned long pos, void __user *dst, - unsigned long count) +static int alsa_cap_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, sockptr_t dst, + unsigned long count) { struct xen_snd_front_pcm_stream_info *stream = stream_get(substream); int ret; @@ -645,29 +631,10 @@ static int alsa_cap_copy_user(struct snd_pcm_substream *substream, if (ret < 0) return ret;
- return copy_to_user(dst, stream->buffer + pos, count) ? + return copy_to_sockptr(dst, stream->buffer + pos, count) ? -EFAULT : 0; }
-static int alsa_cap_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, void *dst, - unsigned long count) -{ - struct xen_snd_front_pcm_stream_info *stream = stream_get(substream); - int ret; - - if (unlikely(pos + count > stream->buffer_sz)) - return -EINVAL; - - ret = xen_snd_front_stream_read(&stream->evt_pair->req, pos, count); - if (ret < 0) - return ret; - - memcpy(dst, stream->buffer + pos, count); - - return 0; -} - static int alsa_pb_fill_silence(struct snd_pcm_substream *substream, int channel, unsigned long pos, unsigned long count) @@ -697,8 +664,7 @@ static const struct snd_pcm_ops snd_drv_alsa_playback_ops = { .prepare = alsa_prepare, .trigger = alsa_trigger, .pointer = alsa_pointer, - .copy_user = alsa_pb_copy_user, - .copy_kernel = alsa_pb_copy_kernel, + .copy = alsa_pb_copy, .fill_silence = alsa_pb_fill_silence, };
@@ -710,8 +676,7 @@ static const struct snd_pcm_ops snd_drv_alsa_capture_ops = { .prepare = alsa_prepare, .trigger = alsa_trigger, .pointer = alsa_pointer, - .copy_user = alsa_cap_copy_user, - .copy_kernel = alsa_cap_copy_kernel, + .copy = alsa_cap_copy, };
static int new_pcm_instance(struct xen_snd_front_card_info *card_info,
Just an update of a comment mentioning the old PCM callbacks to correct to the new PCM copy ops.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/drivers/pcmtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/drivers/pcmtest.c b/sound/drivers/pcmtest.c index 08e14b5eb772..e4dd65e5cd86 100644 --- a/sound/drivers/pcmtest.c +++ b/sound/drivers/pcmtest.c @@ -230,7 +230,7 @@ static void check_buf_block(struct pcmtst_buf_iter *v_iter, struct snd_pcm_runti
/* * Fill buffer in the non-interleaved mode. The order of samples is C0, ..., C0, C1, ..., C1, C2... - * The channel buffers lay in the DMA buffer continuously (see default copy_user and copy_kernel + * The channel buffers lay in the DMA buffer continuously (see default copy * handlers in the pcm_lib.c file). * * Here we increment the DMA buffer position every time we write a byte to any channel 'buffer'.
This patch converts the solo6x10 driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Cc: Bluecherry Maintainers maintainers@bluecherrydvr.com Cc: Anton Sviridenko anton@corp.bluecherry.net Cc: Andrey Utkin andrey_utkin@fastmail.com Cc: Ismael Luceno ismael@iodev.co.uk Cc: Mauro Carvalho Chehab mchehab@kernel.org Cc: linux-media@vger.kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- drivers/media/pci/solo6x10/solo6x10-g723.c | 41 +++++----------------- 1 file changed, 8 insertions(+), 33 deletions(-)
diff --git a/drivers/media/pci/solo6x10/solo6x10-g723.c b/drivers/media/pci/solo6x10/solo6x10-g723.c index 6cebad665565..cf134810b8ec 100644 --- a/drivers/media/pci/solo6x10/solo6x10-g723.c +++ b/drivers/media/pci/solo6x10/solo6x10-g723.c @@ -204,12 +204,13 @@ static snd_pcm_uframes_t snd_solo_pcm_pointer(struct snd_pcm_substream *ss) return idx * G723_FRAMES_PER_PAGE; }
-static int snd_solo_pcm_copy_user(struct snd_pcm_substream *ss, int channel, - unsigned long pos, void __user *dst, - unsigned long count) +static int snd_solo_pcm_copy(struct snd_pcm_substream *ss, int channel, + unsigned long pos, sockptr_t dst, + unsigned long count) { struct solo_snd_pcm *solo_pcm = snd_pcm_substream_chip(ss); struct solo_dev *solo_dev = solo_pcm->solo_dev; + unsigned int off = 0; int err, i;
for (i = 0; i < (count / G723_FRAMES_PER_PAGE); i++) { @@ -223,35 +224,10 @@ static int snd_solo_pcm_copy_user(struct snd_pcm_substream *ss, int channel, if (err) return err;
- if (copy_to_user(dst, solo_pcm->g723_buf, G723_PERIOD_BYTES)) + if (copy_to_sockptr_offset(dst, off, + solo_pcm->g723_buf, G723_PERIOD_BYTES)) return -EFAULT; - dst += G723_PERIOD_BYTES; - } - - return 0; -} - -static int snd_solo_pcm_copy_kernel(struct snd_pcm_substream *ss, int channel, - unsigned long pos, void *dst, - unsigned long count) -{ - struct solo_snd_pcm *solo_pcm = snd_pcm_substream_chip(ss); - struct solo_dev *solo_dev = solo_pcm->solo_dev; - int err, i; - - for (i = 0; i < (count / G723_FRAMES_PER_PAGE); i++) { - int page = (pos / G723_FRAMES_PER_PAGE) + i; - - err = solo_p2m_dma_t(solo_dev, 0, solo_pcm->g723_dma, - SOLO_G723_EXT_ADDR(solo_dev) + - (page * G723_PERIOD_BLOCK) + - (ss->number * G723_PERIOD_BYTES), - G723_PERIOD_BYTES, 0, 0); - if (err) - return err; - - memcpy(dst, solo_pcm->g723_buf, G723_PERIOD_BYTES); - dst += G723_PERIOD_BYTES; + off += G723_PERIOD_BYTES; }
return 0; @@ -263,8 +239,7 @@ static const struct snd_pcm_ops snd_solo_pcm_ops = { .prepare = snd_solo_pcm_prepare, .trigger = snd_solo_pcm_trigger, .pointer = snd_solo_pcm_pointer, - .copy_user = snd_solo_pcm_copy_user, - .copy_kernel = snd_solo_pcm_copy_kernel, + .copy = snd_solo_pcm_copy, };
static int snd_solo_capture_volume_info(struct snd_kcontrol *kcontrol,
For following the ALSA PCM core change, a new PCM copy ops is added toe ASoC component framework: snd_soc_component_driver receives the copy ops, and snd_soc_pcm_component_copy() helper is provided.
This also fixes a long-standing potential bug where the ASoC driver covers only copy_user PCM callback and misses the copy from kernel pointers (such as OSS PCM layer), too.
As of this patch, the old copy_user is still kept, but it'll be dropped later after all drivers are converted.
Cc: Mark Brown broonie@kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/soc-component.h | 7 +++++++ sound/soc/soc-component.c | 20 ++++++++++++++++++++ sound/soc/soc-pcm.c | 4 +++- 3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index 87f248a06271..d9388eac82ee 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -141,6 +141,10 @@ struct snd_soc_component_driver { struct snd_pcm_substream *substream, int channel, unsigned long pos, void __user *buf, unsigned long bytes); + int (*copy)(struct snd_soc_component *component, + struct snd_pcm_substream *substream, int channel, + unsigned long pos, sockptr_t buf, + unsigned long bytes); struct page *(*page)(struct snd_soc_component *component, struct snd_pcm_substream *substream, unsigned long offset); @@ -512,6 +516,9 @@ int snd_soc_pcm_component_sync_stop(struct snd_pcm_substream *substream); int snd_soc_pcm_component_copy_user(struct snd_pcm_substream *substream, int channel, unsigned long pos, void __user *buf, unsigned long bytes); +int snd_soc_pcm_component_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + sockptr_t buf, unsigned long bytes); struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream, unsigned long offset); int snd_soc_pcm_component_mmap(struct snd_pcm_substream *substream, diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index 4356cc320fea..efae007f0f82 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -1052,6 +1052,26 @@ int snd_soc_pcm_component_sync_stop(struct snd_pcm_substream *substream) return 0; }
+int snd_soc_pcm_component_copy(struct snd_pcm_substream *substream, + int channel, unsigned long pos, + sockptr_t buf, unsigned long bytes) +{ + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); + struct snd_soc_component *component; + int i; + + /* FIXME. it returns 1st copy now */ + for_each_rtd_components(rtd, i, component) + if (component->driver->copy) + return soc_component_ret( + component, + component->driver->copy( + component, substream, channel, + pos, buf, bytes)); + + return -EINVAL; +} + int snd_soc_pcm_component_copy_user(struct snd_pcm_substream *substream, int channel, unsigned long pos, void __user *buf, unsigned long bytes) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 8896227e4fb7..71403da28d37 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2973,7 +2973,9 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) rtd->ops.ioctl = snd_soc_pcm_component_ioctl; if (drv->sync_stop) rtd->ops.sync_stop = snd_soc_pcm_component_sync_stop; - if (drv->copy_user) + if (drv->copy) + rtd->ops.copy = snd_soc_pcm_component_copy; + else if (drv->copy_user) rtd->ops.copy_user = snd_soc_pcm_component_copy_user; if (drv->page) rtd->ops.page = snd_soc_pcm_component_page;
This patch converts the mediatek BT SCO driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Cc: Mark Brown broonie@kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/soc/mediatek/common/mtk-btcvsd.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/sound/soc/mediatek/common/mtk-btcvsd.c b/sound/soc/mediatek/common/mtk-btcvsd.c index 1ba0633e542f..d54752baf338 100644 --- a/sound/soc/mediatek/common/mtk-btcvsd.c +++ b/sound/soc/mediatek/common/mtk-btcvsd.c @@ -696,7 +696,7 @@ static int wait_for_bt_irq(struct mtk_btcvsd_snd *bt, }
static ssize_t mtk_btcvsd_snd_read(struct mtk_btcvsd_snd *bt, - char __user *buf, + sockptr_t buf, size_t count) { ssize_t read_size = 0, read_count = 0, cur_read_idx, cont; @@ -743,9 +743,9 @@ static ssize_t mtk_btcvsd_snd_read(struct mtk_btcvsd_snd *bt, if (read_size > cont) read_size = cont;
- if (copy_to_user(buf + cur_buf_ofs, - bt->rx_packet_buf + cur_read_idx, - read_size)) { + if (copy_to_sockptr_offset(buf, cur_buf_ofs, + bt->rx_packet_buf + cur_read_idx, + read_size)) { dev_warn(bt->dev, "%s(), copy_to_user fail\n", __func__); return -EFAULT; @@ -777,7 +777,7 @@ static ssize_t mtk_btcvsd_snd_read(struct mtk_btcvsd_snd *bt, }
static ssize_t mtk_btcvsd_snd_write(struct mtk_btcvsd_snd *bt, - char __user *buf, + sockptr_t buf, size_t count) { int written_size = count, avail, cur_write_idx, write_size, cont; @@ -835,10 +835,10 @@ static ssize_t mtk_btcvsd_snd_write(struct mtk_btcvsd_snd *bt, if (write_size > cont) write_size = cont;
- if (copy_from_user(bt->tx_packet_buf + - cur_write_idx, - buf + cur_buf_ofs, - write_size)) { + if (copy_from_sockptr_offset(bt->tx_packet_buf + + cur_write_idx, + buf, cur_buf_ofs, + write_size)) { dev_warn(bt->dev, "%s(), copy_from_user fail\n", __func__); return -EFAULT; @@ -1033,7 +1033,7 @@ static snd_pcm_uframes_t mtk_pcm_btcvsd_pointer( static int mtk_pcm_btcvsd_copy(struct snd_soc_component *component, struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *buf, unsigned long count) + sockptr_t buf, unsigned long count) { struct mtk_btcvsd_snd *bt = snd_soc_component_get_drvdata(component);
@@ -1274,7 +1274,7 @@ static const struct snd_soc_component_driver mtk_btcvsd_snd_platform = { .prepare = mtk_pcm_btcvsd_prepare, .trigger = mtk_pcm_btcvsd_trigger, .pointer = mtk_pcm_btcvsd_pointer, - .copy_user = mtk_pcm_btcvsd_copy, + .copy = mtk_pcm_btcvsd_copy, };
static int mtk_btcvsd_snd_probe(struct platform_device *pdev)
This patch converts the qcom lpass driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
Cc: Srinivas Kandagatla srinivas.kandagatla@linaro.org Cc: Banajit Goswami bgoswami@quicinc.com Cc: Mark Brown broonie@kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/soc/qcom/lpass-platform.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c index ef5cb40b2d9b..78bf15e768af 100644 --- a/sound/soc/qcom/lpass-platform.c +++ b/sound/soc/qcom/lpass-platform.c @@ -1219,7 +1219,7 @@ static int lpass_platform_pcmops_resume(struct snd_soc_component *component)
static int lpass_platform_copy(struct snd_soc_component *component, struct snd_pcm_substream *substream, int channel, - unsigned long pos, void __user *buf, unsigned long bytes) + unsigned long pos, sockptr_t buf, unsigned long bytes) { struct snd_pcm_runtime *rt = substream->runtime; unsigned int dai_id = component->id; @@ -1230,16 +1230,16 @@ static int lpass_platform_copy(struct snd_soc_component *component,
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { if (is_cdc_dma_port(dai_id)) { - ret = copy_from_user_toio(dma_buf, buf, bytes); + ret = copy_from_sockptr_toio(dma_buf, buf, bytes); } else { - if (copy_from_user((void __force *)dma_buf, buf, bytes)) + if (copy_from_sockptr((void __force *)dma_buf, buf, bytes)) ret = -EFAULT; } } else if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) { if (is_cdc_dma_port(dai_id)) { - ret = copy_to_user_fromio(buf, dma_buf, bytes); + ret = copy_to_sockptr_fromio(buf, dma_buf, bytes); } else { - if (copy_to_user(buf, (void __force *)dma_buf, bytes)) + if (copy_to_sockptr(buf, (void __force *)dma_buf, bytes)) ret = -EFAULT; } } @@ -1260,7 +1260,7 @@ static const struct snd_soc_component_driver lpass_component_driver = { .pcm_construct = lpass_platform_pcm_new, .suspend = lpass_platform_pcmops_suspend, .resume = lpass_platform_pcmops_resume, - .copy_user = lpass_platform_copy, + .copy = lpass_platform_copy,
};
This patch converts the ASoC dmaenging driver code to use the new unified PCM copy callback. It's a straightforward conversion from *_user() to *_sockptr() variants.
The process callback is still using the direct pointer as of now, but it'll be converted in the next patch.
Cc: Lars-Peter Clausen lars@metafoo.de Cc: Mark Brown broonie@kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/soc/soc-generic-dmaengine-pcm.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index 3b99f619e37e..25b8de7e9bb2 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -287,10 +287,10 @@ static snd_pcm_uframes_t dmaengine_pcm_pointer( return snd_dmaengine_pcm_pointer(substream); }
-static int dmaengine_copy_user(struct snd_soc_component *component, - struct snd_pcm_substream *substream, - int channel, unsigned long hwoff, - void __user *buf, unsigned long bytes) +static int dmaengine_copy(struct snd_soc_component *component, + struct snd_pcm_substream *substream, + int channel, unsigned long hwoff, + sockptr_t buf, unsigned long bytes) { struct snd_pcm_runtime *runtime = substream->runtime; struct dmaengine_pcm *pcm = soc_component_to_pcm(component); @@ -302,17 +302,17 @@ static int dmaengine_copy_user(struct snd_soc_component *component, channel * (runtime->dma_bytes / runtime->channels);
if (is_playback) - if (copy_from_user(dma_ptr, buf, bytes)) + if (copy_from_sockptr(dma_ptr, buf, bytes)) return -EFAULT;
if (process) { - int ret = process(substream, channel, hwoff, (__force void *)buf, bytes); + int ret = process(substream, channel, hwoff, buf.kernel, bytes); if (ret < 0) return ret; }
if (!is_playback) - if (copy_to_user(buf, dma_ptr, bytes)) + if (copy_to_sockptr(buf, dma_ptr, bytes)) return -EFAULT;
return 0; @@ -337,7 +337,7 @@ static const struct snd_soc_component_driver dmaengine_pcm_component_process = { .hw_params = dmaengine_pcm_hw_params, .trigger = dmaengine_pcm_trigger, .pointer = dmaengine_pcm_pointer, - .copy_user = dmaengine_copy_user, + .copy = dmaengine_copy, .pcm_construct = dmaengine_pcm_new, };
Along with the conversion to PCM copy ops, use the sockptr_t for the pointer to be passed to the dmaengine process callback, too. It avoids the direct reference of sockptr.kernel field, and it can potentially help for the drivers to access memory properly (although both atmel and stm drivers don't use the given buffer address at all for now).
Cc: Lars-Peter Clausen lars@metafoo.de Cc: Claudiu Beznea claudiu.beznea@microchip.com Cc: Mark Brown broonie@kernel.org Cc: Olivier Moysan olivier.moysan@foss.st.com Cc: Arnaud Pouliquen arnaud.pouliquen@foss.st.com Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/dmaengine_pcm.h | 2 +- sound/soc/atmel/mchp-pdmc.c | 2 +- sound/soc/soc-generic-dmaengine-pcm.c | 4 ++-- sound/soc/stm/stm32_sai_sub.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h index 2df54cf02cb3..86af624f3aea 100644 --- a/include/sound/dmaengine_pcm.h +++ b/include/sound/dmaengine_pcm.h @@ -142,7 +142,7 @@ struct snd_dmaengine_pcm_config { struct snd_pcm_substream *substream); int (*process)(struct snd_pcm_substream *substream, int channel, unsigned long hwoff, - void *buf, unsigned long bytes); + sockptr_t buf, unsigned long bytes); dma_filter_fn compat_filter_fn; struct device *dma_dev; const char *chan_names[SNDRV_PCM_STREAM_LAST + 1]; diff --git a/sound/soc/atmel/mchp-pdmc.c b/sound/soc/atmel/mchp-pdmc.c index 1a069f4cdcda..0e12be712996 100644 --- a/sound/soc/atmel/mchp-pdmc.c +++ b/sound/soc/atmel/mchp-pdmc.c @@ -954,7 +954,7 @@ static int mchp_pdmc_dt_init(struct mchp_pdmc *dd) /* used to clean the channel index found on RHR's MSB */ static int mchp_pdmc_process(struct snd_pcm_substream *substream, int channel, unsigned long hwoff, - void *buf, unsigned long bytes) + sockptr_t buf, unsigned long bytes) { struct snd_pcm_runtime *runtime = substream->runtime; u8 *dma_ptr = runtime->dma_area + hwoff + diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c index 25b8de7e9bb2..a68029d16ccd 100644 --- a/sound/soc/soc-generic-dmaengine-pcm.c +++ b/sound/soc/soc-generic-dmaengine-pcm.c @@ -296,7 +296,7 @@ static int dmaengine_copy(struct snd_soc_component *component, struct dmaengine_pcm *pcm = soc_component_to_pcm(component); int (*process)(struct snd_pcm_substream *substream, int channel, unsigned long hwoff, - void *buf, unsigned long bytes) = pcm->config->process; + sockptr_t buf, unsigned long bytes) = pcm->config->process; bool is_playback = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; void *dma_ptr = runtime->dma_area + hwoff + channel * (runtime->dma_bytes / runtime->channels); @@ -306,7 +306,7 @@ static int dmaengine_copy(struct snd_soc_component *component, return -EFAULT;
if (process) { - int ret = process(substream, channel, hwoff, buf.kernel, bytes); + int ret = process(substream, channel, hwoff, buf, bytes); if (ret < 0) return ret; } diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c index 271ec5b3378d..a61cca86ee31 100644 --- a/sound/soc/stm/stm32_sai_sub.c +++ b/sound/soc/stm/stm32_sai_sub.c @@ -1233,7 +1233,7 @@ static const struct snd_soc_dai_ops stm32_sai_pcm_dai_ops = {
static int stm32_sai_pcm_process_spdif(struct snd_pcm_substream *substream, int channel, unsigned long hwoff, - void *buf, unsigned long bytes) + sockptr_t buf, unsigned long bytes) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
Update the documentation about the PCM copy callbacks. The update was kept minimalistic, just correcting the use of copy_user ops with the single copy ops, and drop/update the text mentioning the copy_kernel.
Signed-off-by: Takashi Iwai tiwai@suse.de --- .../kernel-api/writing-an-alsa-driver.rst | 59 +++++++------------ 1 file changed, 20 insertions(+), 39 deletions(-)
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst index 4335c98b3d82..2ecd6992b796 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -2018,8 +2018,8 @@ sleeping poll threads, etc.
This callback is also atomic by default.
-copy_user, copy_kernel and fill_silence ops -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +copy and fill_silence ops +~~~~~~~~~~~~~~~~~~~~~~~~~
These callbacks are not mandatory, and can be omitted in most cases. These callbacks are used when the hardware buffer cannot be in the @@ -3444,8 +3444,8 @@ external hardware buffer in interrupts (or in tasklets, preferably).
The first case works fine if the external hardware buffer is large enough. This method doesn't need any extra buffers and thus is more -efficient. You need to define the ``copy_user`` and ``copy_kernel`` -callbacks for the data transfer, in addition to the ``fill_silence`` +efficient. You need to define the ``copy`` callback +for the data transfer, in addition to the ``fill_silence`` callback for playback. However, there is a drawback: it cannot be mmapped. The examples are GUS's GF1 PCM or emu8000's wavetable PCM.
@@ -3458,22 +3458,22 @@ Another case is when the chip uses a PCI memory-map region for the buffer instead of the host memory. In this case, mmap is available only on certain architectures like the Intel one. In non-mmap mode, the data cannot be transferred as in the normal way. Thus you need to define the -``copy_user``, ``copy_kernel`` and ``fill_silence`` callbacks as well, +``copy`` and ``fill_silence`` callbacks as well, as in the cases above. Examples are found in ``rme32.c`` and ``rme96.c``.
-The implementation of the ``copy_user``, ``copy_kernel`` and +The implementation of the ``copy`` and ``silence`` callbacks depends upon whether the hardware supports -interleaved or non-interleaved samples. The ``copy_user`` callback is +interleaved or non-interleaved samples. The ``copy`` callback is defined like below, a bit differently depending on whether the direction is playback or capture::
- static int playback_copy_user(struct snd_pcm_substream *substream, + static int playback_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *src, unsigned long count); - static int capture_copy_user(struct snd_pcm_substream *substream, + sockptr_t src, unsigned long count); + static int capture_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, - void __user *dst, unsigned long count); + sockptr_t dst, unsigned long count);
In the case of interleaved samples, the second argument (``channel``) is not used. The third argument (``pos``) specifies the position in bytes. @@ -3490,18 +3490,18 @@ of data (``count``) at the specified pointer (``src``) to the specified offset (``pos``) in the hardware buffer. When coded like memcpy-like way, the copy would look like::
- my_memcpy_from_user(my_buffer + pos, src, count); + my_memcpy_from_sockptr(my_buffer + pos, src, count);
For the capture direction, you copy the given amount of data (``count``) at the specified offset (``pos``) in the hardware buffer to the specified pointer (``dst``)::
- my_memcpy_to_user(dst, my_buffer + pos, count); + my_memcpy_to_sockptr(dst, my_buffer + pos, count);
-Here the functions are named ``from_user`` and ``to_user`` because -it's the user-space buffer that is passed to these callbacks. That -is, the callback is supposed to copy data from/to the user-space -directly to/from the hardware buffer. +The given ``src`` or ``dst`` pointer is a ``sockptr_t`` type +containing a universal pointer that can be either a user-space and +kernel pointer. Use the existing helpers to copy or access the data +as defined in ``linux/sockptr.h``.
Careful readers might notice that these callbacks receive the arguments in bytes, not in frames like other callbacks. It's because @@ -3519,25 +3519,6 @@ the given user-space buffer, but only for the given channel. For details, please check ``isa/gus/gus_pcm.c`` or ``pci/rme9652/rme9652.c`` as examples.
-The above callbacks are the copies from/to the user-space buffer. There -are some cases where we want to copy from/to the kernel-space buffer -instead. In such a case, the ``copy_kernel`` callback is called. It'd -look like:: - - static int playback_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *src, unsigned long count); - static int capture_copy_kernel(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void *dst, unsigned long count); - -As found easily, the only difference is that the buffer pointer is -without a ``__user`` prefix; that is, a kernel-buffer pointer is passed -in the fourth argument. Correspondingly, the implementation would be -a version without the user-copy, such as:: - - my_memcpy(my_buffer + pos, src, count); - Usually for the playback, another callback ``fill_silence`` is defined. It's implemented in a similar way as the copy callbacks above:: @@ -3545,10 +3526,10 @@ above:: static int silence(struct snd_pcm_substream *substream, int channel, unsigned long pos, unsigned long count);
-The meanings of arguments are the same as in the ``copy_user`` and -``copy_kernel`` callbacks, although there is no buffer pointer +The meanings of arguments are the same as in the ``copy`` callback, +although there is no buffer pointer argument. In the case of interleaved samples, the channel argument has -no meaning, as for the ``copy_*`` callbacks. +no meaning, as for the ``copy`` callback.
The role of the ``fill_silence`` callback is to set the given amount (``count``) of silence data at the specified offset (``pos``) in the
Now all ASoC users have been replaced to use the new PCM copy ops, let's drop the obsoleted copy_user ops and its helper function.
Cc: Mark Brown broonie@kernel.org Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/soc-component.h | 7 ------- sound/soc/soc-component.c | 20 -------------------- sound/soc/soc-pcm.c | 2 -- 3 files changed, 29 deletions(-)
diff --git a/include/sound/soc-component.h b/include/sound/soc-component.h index d9388eac82ee..8351e0c8e277 100644 --- a/include/sound/soc-component.h +++ b/include/sound/soc-component.h @@ -137,10 +137,6 @@ struct snd_soc_component_driver { struct timespec64 *audio_ts, struct snd_pcm_audio_tstamp_config *audio_tstamp_config, struct snd_pcm_audio_tstamp_report *audio_tstamp_report); - int (*copy_user)(struct snd_soc_component *component, - struct snd_pcm_substream *substream, int channel, - unsigned long pos, void __user *buf, - unsigned long bytes); int (*copy)(struct snd_soc_component *component, struct snd_pcm_substream *substream, int channel, unsigned long pos, sockptr_t buf, @@ -513,9 +509,6 @@ int snd_soc_pcm_component_pointer(struct snd_pcm_substream *substream); int snd_soc_pcm_component_ioctl(struct snd_pcm_substream *substream, unsigned int cmd, void *arg); int snd_soc_pcm_component_sync_stop(struct snd_pcm_substream *substream); -int snd_soc_pcm_component_copy_user(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void __user *buf, unsigned long bytes); int snd_soc_pcm_component_copy(struct snd_pcm_substream *substream, int channel, unsigned long pos, sockptr_t buf, unsigned long bytes); diff --git a/sound/soc/soc-component.c b/sound/soc/soc-component.c index efae007f0f82..94802aada00f 100644 --- a/sound/soc/soc-component.c +++ b/sound/soc/soc-component.c @@ -1072,26 +1072,6 @@ int snd_soc_pcm_component_copy(struct snd_pcm_substream *substream, return -EINVAL; }
-int snd_soc_pcm_component_copy_user(struct snd_pcm_substream *substream, - int channel, unsigned long pos, - void __user *buf, unsigned long bytes) -{ - struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream); - struct snd_soc_component *component; - int i; - - /* FIXME. it returns 1st copy now */ - for_each_rtd_components(rtd, i, component) - if (component->driver->copy_user) - return soc_component_ret( - component, - component->driver->copy_user( - component, substream, channel, - pos, buf, bytes)); - - return -EINVAL; -} - struct page *snd_soc_pcm_component_page(struct snd_pcm_substream *substream, unsigned long offset) { diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 71403da28d37..ae02d1d80c88 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2975,8 +2975,6 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) rtd->ops.sync_stop = snd_soc_pcm_component_sync_stop; if (drv->copy) rtd->ops.copy = snd_soc_pcm_component_copy; - else if (drv->copy_user) - rtd->ops.copy_user = snd_soc_pcm_component_copy_user; if (drv->page) rtd->ops.page = snd_soc_pcm_component_page; if (drv->mmap)
Finally all users have been converted to the new PCM copy ops, let's drop the obsoleted copy_kernel and copy_user ops completely.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/pcm.h | 5 ----- sound/core/pcm_lib.c | 16 ---------------- sound/core/pcm_native.c | 2 +- 3 files changed, 1 insertion(+), 22 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index 9b7054f0cea0..2700ca1c9db8 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -71,11 +71,6 @@ struct snd_pcm_ops { unsigned long pos, unsigned long bytes); int (*copy)(struct snd_pcm_substream *substream, int channel, unsigned long pos, sockptr_t buf, unsigned long bytes); - int (*copy_user)(struct snd_pcm_substream *substream, int channel, - unsigned long pos, void __user *buf, - unsigned long bytes); - int (*copy_kernel)(struct snd_pcm_substream *substream, int channel, - unsigned long pos, void *buf, unsigned long bytes); struct page *(*page)(struct snd_pcm_substream *substream, unsigned long offset); int (*mmap)(struct snd_pcm_substream *substream, struct vm_area_struct *vma); diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 1dd630cd22a7..1d10de7eeac6 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2031,19 +2031,6 @@ static int default_read_copy(struct snd_pcm_substream *substream, return 0; }
-/* a wrapper for calling old copy_kernel or copy_user ops */ -static int call_old_copy(struct snd_pcm_substream *substream, - int channel, unsigned long hwoff, - sockptr_t buf, unsigned long bytes) -{ - if (sockptr_is_kernel(buf)) - return substream->ops->copy_kernel(substream, channel, hwoff, - buf.kernel, bytes); - else - return substream->ops->copy_user(substream, channel, hwoff, - buf.user, bytes); -} - /* create a sockptr_t */ static inline sockptr_t make_sockptr(void *p, bool in_kernel) { @@ -2241,9 +2228,6 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, } else { if (substream->ops->copy) transfer = substream->ops->copy; - else if ((in_kernel && substream->ops->copy_kernel) || - (!in_kernel && substream->ops->copy_user)) - transfer = call_old_copy; else transfer = is_playback ? default_write_copy : default_read_copy; diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 34efd4d198d6..bd9ddf412b46 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -809,7 +809,7 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, runtime->boundary *= 2;
/* clear the buffer for avoiding possible kernel info leaks */ - if (runtime->dma_area && !substream->ops->copy && !substream->ops->copy_user) { + if (runtime->dma_area && !substream->ops->copy) { size_t size = runtime->dma_bytes;
if (runtime->info & SNDRV_PCM_INFO_MMAP)
On Mon, Jul 31, 2023 at 05:46:54PM +0200, Takashi Iwai wrote:
this is a patch set to clean up the PCM copy ops using sockptr_t as a "universal" pointer, inspired by the recent patch from Andy Shevchenko: https://lore.kernel.org/r/20230721100146.67293-1-andriy.shevchenko@linux.int...
Even though it sounds a bit weird, sockptr_t is a generic type that is used already in wide ranges, and it can fit our purpose, too. With sockptr_t, the former split of copy_user and copy_kernel PCM ops can be unified again gracefully.
It really feels like we ought to rename, or add an alias for, the type if we're going to start using it more widely - it's not helping to make the code clearer.
On Mon, 31 Jul 2023 19:20:54 +0200, Mark Brown wrote:
On Mon, Jul 31, 2023 at 05:46:54PM +0200, Takashi Iwai wrote:
this is a patch set to clean up the PCM copy ops using sockptr_t as a "universal" pointer, inspired by the recent patch from Andy Shevchenko: https://lore.kernel.org/r/20230721100146.67293-1-andriy.shevchenko@linux.int...
Even though it sounds a bit weird, sockptr_t is a generic type that is used already in wide ranges, and it can fit our purpose, too. With sockptr_t, the former split of copy_user and copy_kernel PCM ops can be unified again gracefully.
It really feels like we ought to rename, or add an alias for, the type if we're going to start using it more widely - it's not helping to make the code clearer.
That was my very first impression, too, but I changed my mind after seeing the already used code. An alias might work, either typedef or define genptr_t or such as sockptr_t. But we'll need to copy the bunch of helper functions, too...
Takashi
On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
It really feels like we ought to rename, or add an alias for, the type if we're going to start using it more widely - it's not helping to make the code clearer.
That was my very first impression, too, but I changed my mind after seeing the already used code. An alias might work, either typedef or define genptr_t or such as sockptr_t. But we'll need to copy the bunch of helper functions, too...
I would predict that if the type becomes more widely used that'll happen eventually and the longer it's left the more work it'll be.
On Mon, 31 Jul 2023 21:40:20 +0200, Mark Brown wrote:
On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
It really feels like we ought to rename, or add an alias for, the type if we're going to start using it more widely - it's not helping to make the code clearer.
That was my very first impression, too, but I changed my mind after seeing the already used code. An alias might work, either typedef or define genptr_t or such as sockptr_t. But we'll need to copy the bunch of helper functions, too...
I would predict that if the type becomes more widely used that'll happen eventually and the longer it's left the more work it'll be.
That's true. The question is how more widely it'll be used, then.
Is something like below what you had in mind, too?
Takashi
-- 8< -- From: Takashi Iwai tiwai@suse.de Subject: [PATCH] Introduce uniptr_t as replacement of sockptr_t
Although sockptr_t is used already in several places as a "universal" pointer, it's still too confusing as if were related with network stuff.
Make a more generic type, uniptr_t, that does exactly as same as sockptr_t for a wider use. sockptr_t becomes now alias to uniptr_t.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/linux/sockptr.h | 124 +++++----------------------------------- include/linux/uniptr.h | 117 +++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 109 deletions(-) create mode 100644 include/linux/uniptr.h
diff --git a/include/linux/sockptr.h b/include/linux/sockptr.h index bae5e2369b4f..dc803989a4d6 100644 --- a/include/linux/sockptr.h +++ b/include/linux/sockptr.h @@ -1,118 +1,24 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2020 Christoph Hellwig. - * - * Support for "universal" pointers that can point to either kernel or userspace - * memory. + * Aliases for the old sockptr_t and its helpers for the new uniptr_t */ #ifndef _LINUX_SOCKPTR_H #define _LINUX_SOCKPTR_H
-#include <linux/slab.h> -#include <linux/uaccess.h> +#include <linux/uniptr.h>
-typedef struct { - union { - void *kernel; - void __user *user; - }; - bool is_kernel : 1; -} sockptr_t; - -static inline bool sockptr_is_kernel(sockptr_t sockptr) -{ - return sockptr.is_kernel; -} - -static inline sockptr_t KERNEL_SOCKPTR(void *p) -{ - return (sockptr_t) { .kernel = p, .is_kernel = true }; -} - -static inline sockptr_t USER_SOCKPTR(void __user *p) -{ - return (sockptr_t) { .user = p }; -} - -static inline bool sockptr_is_null(sockptr_t sockptr) -{ - if (sockptr_is_kernel(sockptr)) - return !sockptr.kernel; - return !sockptr.user; -} - -static inline int copy_from_sockptr_offset(void *dst, sockptr_t src, - size_t offset, size_t size) -{ - if (!sockptr_is_kernel(src)) - return copy_from_user(dst, src.user + offset, size); - memcpy(dst, src.kernel + offset, size); - return 0; -} - -static inline int copy_from_sockptr(void *dst, sockptr_t src, size_t size) -{ - return copy_from_sockptr_offset(dst, src, 0, size); -} - -static inline int copy_to_sockptr_offset(sockptr_t dst, size_t offset, - const void *src, size_t size) -{ - if (!sockptr_is_kernel(dst)) - return copy_to_user(dst.user + offset, src, size); - memcpy(dst.kernel + offset, src, size); - return 0; -} - -static inline int copy_to_sockptr(sockptr_t dst, const void *src, size_t size) -{ - return copy_to_sockptr_offset(dst, 0, src, size); -} - -static inline void *memdup_sockptr(sockptr_t src, size_t len) -{ - void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN); - - if (!p) - return ERR_PTR(-ENOMEM); - if (copy_from_sockptr(p, src, len)) { - kfree(p); - return ERR_PTR(-EFAULT); - } - return p; -} - -static inline void *memdup_sockptr_nul(sockptr_t src, size_t len) -{ - char *p = kmalloc_track_caller(len + 1, GFP_KERNEL); - - if (!p) - return ERR_PTR(-ENOMEM); - if (copy_from_sockptr(p, src, len)) { - kfree(p); - return ERR_PTR(-EFAULT); - } - p[len] = '\0'; - return p; -} - -static inline long strncpy_from_sockptr(char *dst, sockptr_t src, size_t count) -{ - if (sockptr_is_kernel(src)) { - size_t len = min(strnlen(src.kernel, count - 1) + 1, count); - - memcpy(dst, src.kernel, len); - return len; - } - return strncpy_from_user(dst, src.user, count); -} - -static inline int check_zeroed_sockptr(sockptr_t src, size_t offset, - size_t size) -{ - if (!sockptr_is_kernel(src)) - return check_zeroed_user(src.user + offset, size); - return memchr_inv(src.kernel + offset, 0, size) == NULL; -} +#define sockptr_t uniptr_t +#define sockptr_is_kernel uniptr_is_kernel +#define KERNEL_SOCKPTR KERNEL_UNIPTR +#define USER_SOCKPTR USER_UNIPTR +#define sockptr_is_null uniptr_is_null +#define copy_from_sockptr_offset copy_from_uniptr_offset +#define copy_from_sockptr copy_from_uniptr +#define copy_to_sockptr_offset copy_to_uniptr_offset +#define copy_to_sockptr copy_to_uniptr +#define memdup_sockptr memdup_uniptr +#define memdup_sockptr_nul memdup_uniptr_nul +#define strncpy_from_sockptr strncpy_from_uniptr +#define check_zeroed_sockptr check_zeroed_uniptr
#endif /* _LINUX_SOCKPTR_H */ diff --git a/include/linux/uniptr.h b/include/linux/uniptr.h new file mode 100644 index 000000000000..3ca9fc8eab4e --- /dev/null +++ b/include/linux/uniptr.h @@ -0,0 +1,117 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2020 Christoph Hellwig. + * + * Support for "universal" pointers that can point to either kernel or userspace + * memory. + */ +#ifndef _LINUX_UNIPTR_H +#define _LINUX_UNIPTR_H + +#include <linux/slab.h> +#include <linux/uaccess.h> + +typedef struct { + union { + void *kernel; + void __user *user; + }; + bool is_kernel : 1; +} uniptr_t; + +static inline bool uniptr_is_kernel(uniptr_t uniptr) +{ + return uniptr.is_kernel; +} + +static inline uniptr_t KERNEL_UNIPTR(void *p) +{ + return (uniptr_t) { .kernel = p, .is_kernel = true }; +} + +static inline uniptr_t USER_UNIPTR(void __user *p) +{ + return (uniptr_t) { .user = p }; +} + +static inline bool uniptr_is_null(uniptr_t uniptr) +{ + if (uniptr_is_kernel(uniptr)) + return !uniptr.kernel; + return !uniptr.user; +} + +static inline int copy_from_uniptr_offset(void *dst, uniptr_t src, + size_t offset, size_t size) +{ + if (!uniptr_is_kernel(src)) + return copy_from_user(dst, src.user + offset, size); + memcpy(dst, src.kernel + offset, size); + return 0; +} + +static inline int copy_from_uniptr(void *dst, uniptr_t src, size_t size) +{ + return copy_from_uniptr_offset(dst, src, 0, size); +} + +static inline int copy_to_uniptr_offset(uniptr_t dst, size_t offset, + const void *src, size_t size) +{ + if (!uniptr_is_kernel(dst)) + return copy_to_user(dst.user + offset, src, size); + memcpy(dst.kernel + offset, src, size); + return 0; +} + +static inline int copy_to_uniptr(uniptr_t dst, const void *src, size_t size) +{ + return copy_to_uniptr_offset(dst, 0, src, size); +} + +static inline void *memdup_uniptr(uniptr_t src, size_t len) +{ + void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN); + + if (!p) + return ERR_PTR(-ENOMEM); + if (copy_from_uniptr(p, src, len)) { + kfree(p); + return ERR_PTR(-EFAULT); + } + return p; +} + +static inline void *memdup_uniptr_nul(uniptr_t src, size_t len) +{ + char *p = kmalloc_track_caller(len + 1, GFP_KERNEL); + + if (!p) + return ERR_PTR(-ENOMEM); + if (copy_from_uniptr(p, src, len)) { + kfree(p); + return ERR_PTR(-EFAULT); + } + p[len] = '\0'; + return p; +} + +static inline long strncpy_from_uniptr(char *dst, uniptr_t src, size_t count) +{ + if (uniptr_is_kernel(src)) { + size_t len = min(strnlen(src.kernel, count - 1) + 1, count); + + memcpy(dst, src.kernel, len); + return len; + } + return strncpy_from_user(dst, src.user, count); +} + +static inline int check_zeroed_uniptr(uniptr_t src, size_t offset, size_t size) +{ + if (!uniptr_is_kernel(src)) + return check_zeroed_user(src.user + offset, size); + return memchr_inv(src.kernel + offset, 0, size) == NULL; +} + +#endif /* _LINUX_UNIPTR_H */
On Tue, Aug 01, 2023 at 02:54:45PM +0200, Takashi Iwai wrote:
That's true. The question is how more widely it'll be used, then.
Indeed.
Is something like below what you had in mind, too?
Yes, or Andy's suggestion of just copying without trying to put a redirect in works for me too. I imagine there might be some bikeshedding of the name, your proposal seems fine to me.
On Tue, Aug 01, 2023 at 02:54:45PM +0200, Takashi Iwai wrote:
On Mon, 31 Jul 2023 21:40:20 +0200, Mark Brown wrote:
On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
It really feels like we ought to rename, or add an alias for, the type if we're going to start using it more widely - it's not helping to make the code clearer.
That was my very first impression, too, but I changed my mind after seeing the already used code. An alias might work, either typedef or define genptr_t or such as sockptr_t. But we'll need to copy the bunch of helper functions, too...
I would predict that if the type becomes more widely used that'll happen eventually and the longer it's left the more work it'll be.
That's true. The question is how more widely it'll be used, then.
Is something like below what you had in mind, too?
I agree with your proposal (uniptr_t also works for me), but see below.
...
+#include <linux/slab.h> +#include <linux/uaccess.h>
But let make the list of the headers right this time.
It seems to me that
err.h minmax.h // maybe not, see a remark at the bottom string.h types.h
are missing.
More below.
...
- void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN);
- if (!p)
return ERR_PTR(-ENOMEM);
This can use cleanup.h.
- if (copy_from_uniptr(p, src, len)) {
kfree(p);return ERR_PTR(-EFAULT);- }
- return p;
+}
+static inline void *memdup_uniptr_nul(uniptr_t src, size_t len) +{
- char *p = kmalloc_track_caller(len + 1, GFP_KERNEL);
Ditto.
- if (!p)
return ERR_PTR(-ENOMEM);- if (copy_from_uniptr(p, src, len)) {
kfree(p);return ERR_PTR(-EFAULT);- }
- p[len] = '\0';
- return p;
+}
...
+static inline long strncpy_from_uniptr(char *dst, uniptr_t src, size_t count) +{
- if (uniptr_is_kernel(src)) {
size_t len = min(strnlen(src.kernel, count - 1) + 1, count);
I didn't get why do we need min()? To check the count == 0 case?
Wouldn't
size_t len;
len = strnlen(src.kernel, count); if (len == 0) return 0;
/* Copy a trailing NUL if found */ if (len < count) len++;
be a good equivalent?
memcpy(dst, src.kernel, len);return len;- }
- return strncpy_from_user(dst, src.user, count);
+}
...
+static inline int check_zeroed_uniptr(uniptr_t src, size_t offset, size_t size) +{
- if (!uniptr_is_kernel(src))
Why not to align all the functions to use same conditional (either always positive or negative)?
return check_zeroed_user(src.user + offset, size);- return memchr_inv(src.kernel + offset, 0, size) == NULL;
+}
...
Taking all remarks into account I would rather go with sockptr.h being untouched for now, just a big
/* DO NOT USE, it's obsolete, use uniptr.h instead! */
to be added.
On Tue, 01 Aug 2023 19:51:39 +0200, Andy Shevchenko wrote:
On Tue, Aug 01, 2023 at 02:54:45PM +0200, Takashi Iwai wrote:
On Mon, 31 Jul 2023 21:40:20 +0200, Mark Brown wrote:
On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote:
Mark Brown wrote:
It really feels like we ought to rename, or add an alias for, the type if we're going to start using it more widely - it's not helping to make the code clearer.
That was my very first impression, too, but I changed my mind after seeing the already used code. An alias might work, either typedef or define genptr_t or such as sockptr_t. But we'll need to copy the bunch of helper functions, too...
I would predict that if the type becomes more widely used that'll happen eventually and the longer it's left the more work it'll be.
That's true. The question is how more widely it'll be used, then.
Is something like below what you had in mind, too?
I agree with your proposal (uniptr_t also works for me), but see below.
...
+#include <linux/slab.h> +#include <linux/uaccess.h>
But let make the list of the headers right this time.
It seems to me that
err.h minmax.h // maybe not, see a remark at the bottom string.h types.h
are missing.
OK, makes sense to add them.
More below.
...
- void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN);
- if (!p)
return ERR_PTR(-ENOMEM);This can use cleanup.h.
Hm, I don't think it can be replaced with that. There may be more use of cleanup.h, but it's no direct alternative for kmalloc_track_caller()...
- if (copy_from_uniptr(p, src, len)) {
kfree(p);return ERR_PTR(-EFAULT);- }
- return p;
+}
+static inline void *memdup_uniptr_nul(uniptr_t src, size_t len) +{
- char *p = kmalloc_track_caller(len + 1, GFP_KERNEL);
Ditto.
- if (!p)
return ERR_PTR(-ENOMEM);- if (copy_from_uniptr(p, src, len)) {
kfree(p);return ERR_PTR(-EFAULT);- }
- p[len] = '\0';
- return p;
+}
...
+static inline long strncpy_from_uniptr(char *dst, uniptr_t src, size_t count) +{
- if (uniptr_is_kernel(src)) {
size_t len = min(strnlen(src.kernel, count - 1) + 1, count);I didn't get why do we need min()? To check the count == 0 case?
Wouldn't
size_t len; len = strnlen(src.kernel, count); if (len == 0) return 0; /* Copy a trailing NUL if found */ if (len < count) len++;be a good equivalent?
A good question. I rather wonder why it can't be simple strncpy().
memcpy(dst, src.kernel, len);return len;- }
- return strncpy_from_user(dst, src.user, count);
+}
...
+static inline int check_zeroed_uniptr(uniptr_t src, size_t offset, size_t size) +{
- if (!uniptr_is_kernel(src))
Why not to align all the functions to use same conditional (either always positive or negative)?
A different person, a different taste :) But it's trivial to fix.
return check_zeroed_user(src.user + offset, size);- return memchr_inv(src.kernel + offset, 0, size) == NULL;
+}
...
Taking all remarks into account I would rather go with sockptr.h being untouched for now, just a big
/* DO NOT USE, it's obsolete, use uniptr.h instead! */
to be added.
Possibly that's a safer choice. But the biggest question is whether we want a generic type or not. Let's try to ask it first.
Interestingly, this file doesn't belong to any subsystem in MAINTAINERS, so I'm not sure who to be Cc'ed. Chirstoph as the original author and net dev, maybe.
thanks,
Takashi
On Mon, Aug 07, 2023 at 05:22:18PM +0200, Takashi Iwai wrote:
On Tue, 01 Aug 2023 19:51:39 +0200, Andy Shevchenko wrote:
On Tue, Aug 01, 2023 at 02:54:45PM +0200, Takashi Iwai wrote:
...
I rather wonder why it can't be simple strncpy().
This is obvious. To avoid compiler warning about 0 (possible) truncation.
...
Taking all remarks into account I would rather go with sockptr.h being untouched for now, just a big
/* DO NOT USE, it's obsolete, use uniptr.h instead! */
to be added.
Possibly that's a safer choice. But the biggest question is whether we want a generic type or not. Let's try to ask it first.
Interestingly, this file doesn't belong to any subsystem in MAINTAINERS, so I'm not sure who to be Cc'ed. Chirstoph as the original author and net dev, maybe.
Yes, but actually it's fine to just copy and change sockptr.h to say "that's blablabla for net subsystem" (maybe this is implied by the name?). In that case we just introduce our copy and can do whatever modifications we want (see previous reply).
On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote:
On Mon, 31 Jul 2023 19:20:54 +0200, Mark Brown wrote:
On Mon, Jul 31, 2023 at 05:46:54PM +0200, Takashi Iwai wrote:
this is a patch set to clean up the PCM copy ops using sockptr_t as a "universal" pointer, inspired by the recent patch from Andy Shevchenko: https://lore.kernel.org/r/20230721100146.67293-1-andriy.shevchenko@linux.int...
Even though it sounds a bit weird, sockptr_t is a generic type that is used already in wide ranges, and it can fit our purpose, too. With sockptr_t, the former split of copy_user and copy_kernel PCM ops can be unified again gracefully.
It really feels like we ought to rename, or add an alias for, the type if we're going to start using it more widely - it's not helping to make the code clearer.
That was my very first impression, too, but I changed my mind after seeing the already used code. An alias might work, either typedef or define genptr_t or such as sockptr_t. But we'll need to copy the bunch of helper functions, too...
Maybe we should define a genptr_t (in genptr.h) and convert sockptr infra to use it (in sockptr.h)? This will leave network and other existing users to convert to it step-by-step.
Another approach is to simply copy sockptr.h to genptr.h with changed naming scheme and add a deprecation note to the former.
Thank you, Takashi, for doing this!
participants (3)
-
Andy Shevchenko -
Mark Brown -
Takashi Iwai