Hi Morimoto-san,
Thanks for your work on this!
On Wed, Aug 19, 2009 at 8:25 PM, Kuninori
Morimoto<morimoto.kuninori@xxxxxxxxxxx> wrote:
> This driver is very simple.
> It support playback only now.
> It is tested by ms7724se board.
>
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@xxxxxxxxxxx>
> ---
[snip]
> --- /dev/null
> +++ b/include/sound/fsi.h
How about using sh_fsi.h or sh_mobile_fsi.h? I think
include/sound/fsi.h is polluting the name space.
> --- /dev/null
> +++ b/sound/soc/sh/fsi.c
[snip]
> +struct fsi_master {
> + void __iomem *base;
> + atomic_t user;
> + int irq;
> + struct clk *clk;
> + struct fsi_priv fsia;
> + struct fsi_priv fsib;
> + struct sh_fsi_platform_info *info;
> +};
> +
> +struct fsi_master *master;
Is it really necessary to have "master" as a global variable? Maybe
this global variable is there to work around some framework issue?
If possible please try to avoid using a global variable like this.
Future processors may come with multiple FSI blocks.
Right now I'm quite happy that the CEU driver does not use global
variables. Remember the case with a single CEU in sh7722 and now we
have two CEU blocks in sh7724. It was easy to support sh7724 because I
avoided using global variables when I initially wrote the driver for
sh7722.
Cheers,
/ magnus
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
