[alsa-devel] [PATCH] sh: add SuperH DAC audio driver for ALSA V3

[Takashi Iwai]

At Mon, 2 Nov 2009 14:03:42 -0300,
Rafael Ignacio Zurita wrote:
> 
> Hello Takashi,
>   thanks for the review in your previous mail.
> 
> On Fri, Oct 30, 2009 at 08:59:56PM +0900, Paul Mundt wrote:
> > On Fri, Oct 30, 2009 at 12:24:03PM +0100, Takashi Iwai wrote:
> > > At Thu, 22 Oct 2009 17:25:06 -0300,
> > > Rafael Ignacio Zurita wrote:
> > > > --- /dev/null
> > > > +++ b/include/sound/sh_dac_audio.h
> > > > @@ -0,0 +1,21 @@
> > > > +/*
> > > > + * SH_DAC specific configuration, for the dac_audio platform_device
> > > > + *
> > > > + * Copyright (C) 2009 Rafael Ignacio Zurita <rizurita at yahoo.com>
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify it
> > > > + * under the terms of the GNU General Public License version 2 as published
> > > > + * by the Free Software Foundation.
> > > > + */
> > > > +
> > > > +#ifndef __INCLUDE_SH_DAC_AUDIO_H
> > > > +#define __INCLUDE_SH_DAC_AUDIO_H
> > > > +
> > > > +struct dac_audio_pdata {
> > > > +	int buffer_size;
> > > > +	int channel;
> > > > +	void (*start)(struct dac_audio_pdata *pd);
> > > > +	void (*stop)(struct dac_audio_pdata *pd);
> > > > +};
> > > > +
> > > > +#endif /* __INCLUDE_SH_DAC_AUDIO_H */
> > > 
> > > Isn't it better to include this piece into mach/hp6xx.h or so?
> > > Since it's pretty specific to the platform, it's not necessarily in
> > > the common sound include path.
> > > 
> > It's not really platform specific, it's a generic driver for the SH
> > on-chip DAC. hp6xx is the only platform that is using this at the moment,
> > but it's forseeable that other SH-3 platforms will enable this, too.
> > 
> > Of course we can hide this header in the sh includes somewhere if you
> > don't want it in include/sound, but include/sound certainly seems like a
> > reasonable place for it.
> 
> After Pauls comments I would like to ask :
> Is the sh_dac_audio.h file in include/sound okey with you?
> 
> If so, then I am going to send a new V4 of the patch, with the proper 
> snd-* driver file name.

It's fine to have it in include/sound.  I just wondered it because
this is used only in two places, and it's not necessarily to be
exported to the user-space, too.  But, include/sound is a reasonable
place, that's pretty OK.  (And, if any, we can move the code later on
:)

Please go ahead.  I'll apply the patch after reviewing.


thanks,

Takashi