From: B47053@freescale.com (Li Xiubo)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver.
Date: Tue, 5 Nov 2013 03:50:07 +0000 [thread overview]
Message-ID: <1DD289F6464F0949A2FCA5AA6DC23F82875503@039-SN2MPN1-013.039d.mgd.msft.net> (raw)
In-Reply-To: <20131101102805.GG28401@MrMyself>
Hi Nicolin,
> > This is the SGTL5000 codec based audio driver supported with both
> > playback and capture dai link implemention.
> >
> > This implementation is only compatible with device tree definition.
> >
> > Signed-off-by: Alison Wang <b18965@freescale.com
> > Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> >
> > Conflicts:
> > sound/soc/fsl/Makefile
> > ---
> > sound/soc/fsl/Kconfig | 10 ++
> > sound/soc/fsl/Makefile | 2 +
>
> > sound/soc/fsl/fsl-sgtl5000-vf610.c | 208
> > +++++++++++++++++++++++++++++++++++++
>
> I just doubt if this file naming is appropriate. Even if we might not
> have rigor rule for the file names, according to existing ones, they are
> all in a same pattern: [SoC name]-[codec name].c
>
> "imx-sgtl5000.c" for example
>
> I think it would make user less confused about what this file exactly is
> if this machine driver also follow the pattern: vf610-sgtl5000.c
>
Yes, it looks nicer.
>
> @Shawn
>
> What do you think about the file name?
>
> > 3 files changed, 220 insertions(+)
> > create mode 100644 sound/soc/fsl/fsl-sgtl5000-vf610.c
> >
> > diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig index
> > 9a8851e..1b835ba 100644
> > --- a/sound/soc/fsl/Kconfig
> > +++ b/sound/soc/fsl/Kconfig
> > @@ -228,4 +228,14 @@ config SND_SOC_FSL_SAI
> > tristate
> > select SND_SOC_GENERIC_DMAENGINE_PCM
> >
> > +config SND_SOC_FSL_SGTL5000_VF610
>
> Same problem with the this define.
>
> > + tristate "SoC Audio support for FSL boards with sgtl5000"
>
> And 'FSL' here confuses me a lot. Because those boards based on i.MX
> series also could be called FSL boards.
>
Yes, this should be VF610.
> > + depends on OF && I2C
> > + select SND_SOC_FSL_SAI
> > + select SND_SOC_FSL_PCM
> > + select SND_SOC_SGTL5000
> > + help
> > + Say Y if you want to add support for SoC audio on an FSL board
> with
> > + a sgtl5000 codec.
> > +
> > endif # SND_FSL_SOC
> > diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile index
> > e5acc03..26fc551 100644
> > --- a/sound/soc/fsl/Makefile
> > +++ b/sound/soc/fsl/Makefile
> > @@ -59,5 +59,7 @@ obj-$(CONFIG_SND_SOC_IMX_MC13783) +=
> > snd-soc-imx-mc13783.o
> >
> > # FSL ARM SAI/SGT15000 Platform Support snd-soc-fsl-sai-objs :=
> > fsl-sai.o
> > +snd-soc-fsl-sgtl5000-vf610-objs := fsl-sgtl5000-vf610.o
> >
> > obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o
> > +obj-$(CONFIG_SND_SOC_FSL_SGTL5000_VF610) +=
> > +snd-soc-fsl-sgtl5000-vf610.o
> > diff --git a/sound/soc/fsl/fsl-sgtl5000-vf610.c
> > b/sound/soc/fsl/fsl-sgtl5000-vf610.c
> > new file mode 100644
> > index 0000000..f535b42
> > --- /dev/null
> > +++ b/sound/soc/fsl/fsl-sgtl5000-vf610.c
> > @@ -0,0 +1,208 @@
> > +/*
> > + * Freeacale ALSA SoC Audio using SGT1500 as codec.
> > + *
> > + * Copyright 2012-2013 Freescale Semiconductor, Inc.
> > + *
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
> > + *
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/i2c.h>
> > +#include <linux/clk.h>
> > +
> > +#include "../codecs/sgtl5000.h"
> > +#include "fsl-sai.h"
> > +
> > +static unsigned int sysclk_rate;
> > +
> > +static int fsl_sgtl5000_dai_init(struct snd_soc_pcm_runtime *rtd)
>
> Naming issue here again.
>
> At least from my point of view, if you actually merged imx-sgtl5000 with
> vf610-sgtl5000 and made it also compatible to other freescale SoCs, you
> could then fairly call it fsl_sgtl5000_xxxx.
>
> Well, I might be a little picky here because it's a static function and
> won't conflict others. Just the name here doesn't look so explicit to me.
>
> Please reconsider about this whole file's naming.
>
Yes, I still not very sure the names of the functions and files, from your replies,
I have got many information about the rules and others, I'll think it over and do some
research, Please see the next version.
Best regards,
Xiubo
next prev parent reply other threads:[~2013-11-05 3:50 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-01 7:04 No subject Xiubo Li
2013-11-01 7:04 ` [PATCHv2 1/8] ALSA: Add SAI SoC Digital Audio Interface driver Xiubo Li
2013-11-01 8:59 ` Nicolin Chen
2013-11-04 3:45 ` Li Xiubo
2013-11-04 4:33 ` Nicolin Chen
2013-11-20 3:37 ` Li Xiubo
2013-11-20 3:37 ` Nicolin Chen
2013-11-20 4:16 ` Li Xiubo
2013-11-05 13:26 ` Timur Tabi
2013-11-06 3:27 ` Li Xiubo
2013-11-06 3:31 ` Timur Tabi
2013-11-06 3:53 ` Li Xiubo
2013-11-06 8:12 ` Shawn Guo
2013-11-06 9:38 ` Li Xiubo
2013-11-01 18:25 ` Mark Brown
2013-11-04 7:35 ` Li Xiubo
2013-11-04 16:15 ` Mark Brown
2013-11-05 3:21 ` Li Xiubo
2013-11-06 9:53 ` Mark Brown
2013-11-01 7:04 ` [PATCHv2 2/8] ARM: dts: Add Freescale SAI ALSA SoC Digital Audio Interface node for VF610 Xiubo Li
2013-11-01 7:04 ` [PATCHv2 3/8] ARM: dts: Enables SAI ALSA SoC DAI device for Vybrid VF610 TOWER board Xiubo Li
2013-11-18 18:07 ` Bill Pringlemeir
2013-11-20 3:14 ` Li Xiubo
2013-11-20 16:04 ` Bill Pringlemeir
2013-11-21 2:58 ` Li Xiubo
2013-11-21 14:55 ` Bill Pringlemeir
2013-11-22 6:46 ` Li Xiubo
2013-11-22 15:09 ` Bill Pringlemeir
2013-11-28 7:45 ` Li Xiubo
2013-11-01 7:04 ` [PATCHv2 4/8] Documentation: Add device tree bindings for Freescale SAI Xiubo Li
2013-11-01 7:04 ` [PATCHv2 5/8] ASoC: SGTL5000: Enhance the SGTL5000 codec driver about regulator Xiubo Li
2013-11-01 10:02 ` Nicolin Chen
2013-11-01 18:50 ` Mark Brown
2013-11-06 8:59 ` Li Xiubo
2013-11-06 10:03 ` Mark Brown
2013-11-07 3:01 ` Li Xiubo
2013-11-07 20:38 ` Mark Brown
2013-11-01 7:04 ` [PATCHv2 6/8] ASoC: fsl: add SGTL5000 based audio machine driver Xiubo Li
2013-11-01 10:17 ` Oskar Schirmer
2013-11-05 3:26 ` Li Xiubo
2013-11-01 10:28 ` Nicolin Chen
2013-11-01 12:07 ` Shawn Guo
2013-11-05 6:17 ` Li Xiubo
2013-11-05 3:50 ` Li Xiubo [this message]
2013-11-01 18:40 ` Mark Brown
2013-11-04 9:52 ` Li Xiubo
2013-11-20 7:49 ` Li Xiubo
2013-11-01 7:04 ` [PATCHv2 7/8] ARM: dts: Enable SGTL5000 codec based audio driver node for VF610 Xiubo Li
2013-11-01 7:04 ` [PATCHv2 8/8] Documentation: Add device tree bindings for Freescale VF610 sound Xiubo Li
2013-11-01 7:52 ` [PATCHv1 0/8] ALSA: Add SAI driver and enable SGT15000 codec Li Xiubo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1DD289F6464F0949A2FCA5AA6DC23F82875503@039-SN2MPN1-013.039d.mgd.msft.net \
--to=b47053@freescale.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).