Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1218232

Re: [PATCH 03/10] ALSA: add AXD Audio Processing IP alsa driver

From Mark Brown <broonie@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH 03/10] ALSA: add AXD Audio Processing IP alsa driver
Date 2015-09-03 14:50 +0200
Message-ID <q4C7L-3PO-7@gated-at.bofh.it> (permalink)
References (1 earlier) <q0ZcE-7Np-39@gated-at.bofh.it> <q1NM6-5sE-35@gated-at.bofh.it> <q24jT-4aB-3@gated-at.bofh.it> <q27rr-8j-5@gated-at.bofh.it> <q2o8V-7z5-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Fri, Aug 28, 2015 at 10:22:57AM +0100, Qais Yousef wrote:
> On 08/27/2015 04:32 PM, Mark Brown wrote:
> >On Thu, Aug 27, 2015 at 01:15:51PM +0100, Qais Yousef wrote:

> >>>>+#define AXD_BASE_VADDR		0xD0000000

> >>>This sounds like something that is going to be platform dependant,
> >>>should this be supplied from board configuration?

> >>I don't expect this to change. Can we add the configuration later if we hit
> >>the need to change it?

> >It should be trivial to make things configurable shouldn't it?

> Yes and I am all with configurability but I don't think it makes sense here.
> AXD will always have its own MMU and will not share virtual address space,
> so the possibility of us wanting to move this somewhere else is really very
> thin. Also I don't think this is the kind of detail we need to concern the
> user with. I'll see if I can make the binary header parsing more flexible so
> we can add more info like this and the one above in the future and be more
> future proof.

So this is a virtual address in the memory map of the DSP?  That's not
what I thought it was.

> >>>>+	memcpy_toio((void *)cached_fw_base, fw->data, fw->size);

> >>>Why the cast here?  I'm also not seeing where we handled the copying to
> >>>I/O in the decompression case?

> >>I couldn't avoid the cast. If cached_fw_base is 'void *' I'll get a warning
> >>when initialising cached_fw_base from CAC_ADDR().

> >Why do you get a warning from that?  Perhaps the warnings are trying to
> >tell us something...

> Because we try to assign an int to a pointer. So the error is 'makes pointer
> from integer without a cast'. To convert an address from uncached to cached
> we need to convert to an int as in MIPS it's a case of adding or subtracting
> a value then convert this value back to it's original form.
> I'll see if I can find a better way to fix the coherency issue when we copy
> through uncached.

Why can't you just use pointer arithmmetic?

> >>Good point. When decompressing crypto_comp_decompress() will write directly
> >>to the memory. It is safe but it doesn't go through the correct API. Not
> >>sure what I can do here.

> >Uncompress to a buffer then write that buffer to the final destination?

> Yes but the binary could be multi MiB so we can't get a temp buffer that
> large. If the crypto API allows decompressing in steps we can use a small
> buffer to move the data iteratively. I'll have a look.

A few megabytes doesn't seem like that big an ask (it's not *nice* but
it's doable with vmalloc()).  Iteratively copying is nicer though.

> >>>>+	/* wake up any task sleeping on command response */
> >>>>+	wake_up(&axd->cmd.wait);
> >>>>+	/* give chance to user land tasks to react to the crash */
> >>>>+	ssleep(2);
> >>>This looks horribly racy, I'd expect us to be trashing and/or killing
> >>>off any active work and resources here.
> >>OK. I was trying to play nicely by giving the chance to userland to repond
> >>to -ERESTART which would be sent from aborting any pending reads/writes.
> >>Are you suggesting to send SIGKILL using force_sig()?
> >No, I'm suggesting tearing down the kernel side of any work and kicking
> >errors back to userspace if it continues to interact with anything that
> >was ongoing.

> OK. This is what we do (see my other email about abort). I'll have a think
> for a way to get rid of the ssleep(). Any ideas are welcome.

Just delete it?

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/10] Add support for img AXD audio hardware decoder Qais Yousef <qais.yousef@imgtec.com> - 2015-08-24 14:40 +0200
  [PATCH 04/10] ALSA: axd: add fw binary header manipulation files Qais Yousef <qais.yousef@imgtec.com> - 2015-08-24 14:40 +0200
  [PATCH 09/10] ALSA: axd: add alsa compress offload operations Qais Yousef <qais.yousef@imgtec.com> - 2015-08-24 14:50 +0200
  [PATCH 05/10] ALSA: axd: add buffers manipulation files Qais Yousef <qais.yousef@imgtec.com> - 2015-08-24 14:50 +0200
    Re: [PATCH 05/10] ALSA: axd: add buffers manipulation files Mark Brown <broonie@kernel.org> - 2015-08-26 20:50 +0200
      Re: [PATCH 05/10] ALSA: axd: add buffers manipulation files Qais Yousef <qais.yousef@imgtec.com> - 2015-08-27 16:30 +0200
        Re: [PATCH 05/10] ALSA: axd: add buffers manipulation files Mark Brown <broonie@kernel.org> - 2015-08-29 11:50 +0200
          Re: [PATCH 05/10] ALSA: axd: add buffers manipulation files Qais Yousef <qais.yousef@imgtec.com> - 2015-09-01 12:10 +0200
            Re: [PATCH 05/10] ALSA: axd: add buffers manipulation files Mark Brown <broonie@kernel.org> - 2015-09-03 14:40 +0200
  [PATCH 08/10] ALSA: axd: add low level AXD platform setup files Qais Yousef <qais.yousef@imgtec.com> - 2015-08-24 14:50 +0200
  [PATCH 10/10] ALSA: axd: add Makefile Qais Yousef <qais.yousef@imgtec.com> - 2015-08-24 14:50 +0200
  Re: [PATCH 00/10] Add support for img AXD audio hardware decoder Mark Brown <broonie@kernel.org> - 2015-08-26 20:10 +0200
    Re: [PATCH 00/10] Add support for img AXD audio hardware decoder Qais Yousef <qais.yousef@imgtec.com> - 2015-08-27 11:10 +0200
  Re: [PATCH 03/10] ALSA: add AXD Audio Processing IP alsa driver Mark Brown <broonie@kernel.org> - 2015-08-26 20:40 +0200
    Re: [PATCH 03/10] ALSA: add AXD Audio Processing IP alsa driver Qais Yousef <qais.yousef@imgtec.com> - 2015-08-27 14:20 +0200
      Re: [PATCH 03/10] ALSA: add AXD Audio Processing IP alsa driver Mark Brown <broonie@kernel.org> - 2015-08-27 17:40 +0200
        Re: [PATCH 03/10] ALSA: add AXD Audio Processing IP alsa driver Qais Yousef <qais.yousef@imgtec.com> - 2015-08-28 11:30 +0200
          Re: [PATCH 03/10] ALSA: add AXD Audio Processing IP alsa driver Mark Brown <broonie@kernel.org> - 2015-09-03 14:50 +0200
  Re: [PATCH 06/10] ALSA: axd: add basic files for sending/receiving  axd cmds Mark Brown <broonie@kernel.org> - 2015-08-26 21:20 +0200
    Re: [PATCH 06/10] ALSA: axd: add basic files for sending/receiving  axd cmds Qais Yousef <qais.yousef@imgtec.com> - 2015-08-27 17:50 +0200
      Re: [PATCH 06/10] ALSA: axd: add basic files for sending/receiving  axd cmds Mark Brown <broonie@kernel.org> - 2015-08-29 12:20 +0200
        Re: [PATCH 06/10] ALSA: axd: add basic files for sending/receiving  axd cmds Qais Yousef <qais.yousef@imgtec.com> - 2015-09-01 12:50 +0200
          Re: [PATCH 06/10] ALSA: axd: add basic files for sending/receiving  axd cmds Mark Brown <broonie@kernel.org> - 2015-09-03 14:50 +0200

csiph-web