At Wed, 7 Aug 2013 15:59:07 +0200,
Torsten Schenk wrote:
>
> On Wed, 07 Aug 2013 14:43:43 +0200
> Takashi Iwai <tiwai@xxxxxxx> wrote:
>
> > At Tue, 06 Aug 2013 14:53:24 +0300,
> > Jussi Kivilinna wrote:
> > >
> > > Patch fixes 6fire not to use stack as URB transfer_buffer. URB
> > > buffers need to be DMA-able, which stack is not. Furthermore,
> > > transfer_buffer should not be allocated as part of larger device
> > > structure because DMA coherency issues and patch fixes this issue
> > > too.
>
> Thanks for the information. There is another section where this applies
> in midi.c/midi.h. I can post a patch later.
Yes, please.
> > > Patch is only compile tested.
> >
> > The changes look OK, but I'd like to let it checked with a real
> > hardware before putting to stable kernel.
> >
> > Torsten, could you check this?
>
> The patch works nicely.
OK, I queued the patch to sound git tree now.
> I'm just wondering if instead of calling kmalloc() every time it'd be
> better to put a global sender_buffer and a mutex into the comm_runtime
> struct and lock it every time a writeX is performed. This also would
> have the advantage that message blocks would never overlap which
> currently is possible.
>
> I can prepare a patch on top of this one and send it later, if you
> agree.
Yes, it'd be much better.
thanks,
Takashi
>
> Thanks,
> Torsten
>
> >
> >
> > thanks,
> >
> > Takashi
> >
> > >
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@xxxxxx>
> > > ---
> > > sound/usb/6fire/comm.c | 38 ++++++++++++++++++++++++++++++++
> > > +----- sound/usb/6fire/comm.h | 2 +-
> > > 2 files changed, 34 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c
> > > index 9e6e3ff..23452ee 100644
> > > --- a/sound/usb/6fire/comm.c
> > > +++ b/sound/usb/6fire/comm.c
> > > @@ -110,19 +110,37 @@ static int usb6fire_comm_send_buffer(u8
> > > *buffer, struct usb_device *dev) static int usb6fire_comm_write8
> > > (struct comm_runtime *rt, u8 request, u8 reg, u8 value)
> > > {
> > > - u8 buffer[13]; /* 13: maximum length of message */
> > > + u8 *buffer;
> > > + int ret;
> > > +
> > > + /* 13: maximum length of message */
> > > + buffer = kmalloc(13, GFP_KERNEL);
> > > + if (!buffer)
> > > + return -ENOMEM;
> > >
> > > usb6fire_comm_init_buffer(buffer, 0x00, request, reg,
> > > value, 0x00);
> > > - return usb6fire_comm_send_buffer(buffer, rt->chip->dev);
> > > + ret = usb6fire_comm_send_buffer(buffer, rt->chip->dev);
> > > +
> > > + kfree(buffer);
> > > + return ret;
> > > }
> > >
> > > static int usb6fire_comm_write16(struct comm_runtime *rt, u8
> > > request, u8 reg, u8 vl, u8 vh)
> > > {
> > > - u8 buffer[13]; /* 13: maximum length of message */
> > > + u8 *buffer;
> > > + int ret;
> > > +
> > > + /* 13: maximum length of message */
> > > + buffer = kmalloc(13, GFP_KERNEL);
> > > + if (!buffer)
> > > + return -ENOMEM;
> > >
> > > usb6fire_comm_init_buffer(buffer, 0x00, request, reg, vl,
> > > vh);
> > > - return usb6fire_comm_send_buffer(buffer, rt->chip->dev);
> > > + ret = usb6fire_comm_send_buffer(buffer, rt->chip->dev);
> > > +
> > > + kfree(buffer);
> > > + return ret;
> > > }
> > >
> > > int usb6fire_comm_init(struct sfire_chip *chip)
> > > @@ -135,6 +153,12 @@ int usb6fire_comm_init(struct sfire_chip *chip)
> > > if (!rt)
> > > return -ENOMEM;
> > >
> > > + rt->receiver_buffer = kzalloc(COMM_RECEIVER_BUFSIZE,
> > > GFP_KERNEL);
> > > + if (!rt->receiver_buffer) {
> > > + kfree(rt);
> > > + return -ENOMEM;
> > > + }
> > > +
> > > urb = &rt->receiver;
> > > rt->serial = 1;
> > > rt->chip = chip;
> > > @@ -153,6 +177,7 @@ int usb6fire_comm_init(struct sfire_chip *chip)
> > > urb->interval = 1;
> > > ret = usb_submit_urb(urb, GFP_KERNEL);
> > > if (ret < 0) {
> > > + kfree(rt->receiver_buffer);
> > > kfree(rt);
> > > snd_printk(KERN_ERR PREFIX "cannot create comm
> > > data receiver."); return ret;
> > > @@ -171,6 +196,9 @@ void usb6fire_comm_abort(struct sfire_chip
> > > *chip)
> > > void usb6fire_comm_destroy(struct sfire_chip *chip)
> > > {
> > > - kfree(chip->comm);
> > > + struct comm_runtime *rt = chip->comm;
> > > +
> > > + kfree(rt->receiver_buffer);
> > > + kfree(rt);
> > > chip->comm = NULL;
> > > }
> > > diff --git a/sound/usb/6fire/comm.h b/sound/usb/6fire/comm.h
> > > index 6a0840b..780d5ed 100644
> > > --- a/sound/usb/6fire/comm.h
> > > +++ b/sound/usb/6fire/comm.h
> > > @@ -24,7 +24,7 @@ struct comm_runtime {
> > > struct sfire_chip *chip;
> > >
> > > struct urb receiver;
> > > - u8 receiver_buffer[COMM_RECEIVER_BUFSIZE];
> > > + u8 *receiver_buffer;
> > >
> > > u8 serial; /* urb serial */
> > >
> > >
> > > _______________________________________________
> > > Alsa-devel mailing list
> > > Alsa-devel@xxxxxxxxxxxxxxxx
> > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > >
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
