On Mon, 15 Jun 2020 at 22:23, Allan Peramaki <apera...@pp1.inet.fi> wrote:
>
> Fix audio on software that accesses DRAM above 64k via register peek/poke
> and some cases when more than 16 voices are used.
>
> Fixes: 135f5ae1974c ("audio: GUSsample is int16_t")
> Signed-off-by: Allan Peramaki <apera...@pp1.inet.fi>This patch is quite difficult to read because it mixes some whitespace only changes with some actual changes of behaviour. > -#define GUSregb(position) (* (gusptr+(position))) > -#define GUSregw(position) (*(uint16_t *) (gusptr+(position))) > -#define GUSregd(position) (*(uint16_t *)(gusptr+(position))) > +#define GUSregb(position) (*(gusptr + (position))) > +#define GUSregw(position) (*(uint16_t *)(gusptr + (position))) > +#define GUSregd(position) (*(uint32_t *)(gusptr + (position))) So, I think the actual bugfix change here is just the changing of uint16_t to uint32_t in the GUSregd definition... > -#define GUSregb(position) (* (gusptr+(position))) > -#define GUSregw(position) (*(uint16_t *) (gusptr+(position))) > -#define GUSregd(position) (*(uint16_t *)(gusptr+(position))) > +#define GUSregb(position) (*(gusptr + (position))) > +#define GUSregw(position) (*(uint16_t *)(gusptr + (position))) > +#define GUSregd(position) (*(uint32_t *)(gusptr + (position))) ...and similarly here, and all the other changes are purely cleaning up the spaces. Is that right? > -#define GUSvoice(position) (*(uint16_t *)(voiceptr+(position))) > +#define GUSvoice(position) (*(uint16_t *)(voiceptr + (position))) Are these accesses all guaranteed to be correctly aligned to be 16 or 32 bit loads/stores ? Otherwise it would be better to use the ldl_p/stl_p/ldw_p/stw_p/etc accessors, which correctly handle possibly misaligned pointers. thanks -- PMM
