[PATCH 2/4] ASoC: fsi: Add FIFO size calculate

[PATCH 2/4] ASoC: fsi: Add FIFO size calculate

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 sound/soc/sh/fsi.c |   48 +++++++++++++++++++++++-------------------------
 1 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index db91349..cc41bae 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -46,8 +46,9 @@
 #define MUTE		0x020C
 #define CLK_RST		0x0210
 #define SOFT_RST	0x0214
+#define FIFO_SZ		0x0218
 #define MREG_START	INT_ST
-#define MREG_END	SOFT_RST
+#define MREG_END	FIFO_SZ
 
 /* DO_FMT */
 /* DI_FMT */
@@ -85,6 +86,11 @@
 #define IR		(1 <<  4) /* Interrupt Reset */
 #define FSISR		(1 <<  0) /* Software Reset */
 
+/* FIFO_SZ */
+#define OUT_SZ_MASK	0x7
+#define BO_SZ_SHIFT	8
+#define AO_SZ_SHIFT	0
+
 #define FSI_RATES SNDRV_PCM_RATE_8000_96000
 
 #define FSI_FMTS (SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE)
@@ -590,11 +596,12 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 			   struct snd_soc_dai *dai)
 {
 	struct fsi_priv *fsi = fsi_get_priv(substream);
+	struct fsi_master *master = fsi_get_master(fsi);
 	const char *msg;
 	u32 flags = fsi_get_info_flags(fsi);
 	u32 fmt;
 	u32 reg;
-	u32 data;
+	u32 data, i;
 	int is_play = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
 	int is_master;
 	int ret = 0;
@@ -668,31 +675,22 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 		dev_err(dai->dev, "unknown format.\n");
 		return -EINVAL;
 	}
-
-	switch (fsi->chan) {
-	case 1:
-		fsi->fifo_max = 256;
-		break;
-	case 2:
-		fsi->fifo_max = 128;
-		break;
-	case 3:
-	case 4:
-		fsi->fifo_max = 64;
-		break;
-	case 5:
-	case 6:
-	case 7:
-	case 8:
-		fsi->fifo_max = 32;
-		break;
-	default:
-		dev_err(dai->dev, "channel size error.\n");
-		return -EINVAL;
-	}
-
 	fsi_reg_write(fsi, reg, data);
 
+	/* calculate FIFO size */
+	data = fsi_master_read(master, FIFO_SZ);
+	data >>= fsi_is_port_a(fsi) ? AO_SZ_SHIFT : BO_SZ_SHIFT;
+	data &= OUT_SZ_MASK;
+	fsi->fifo_max = 256;
+	for (i = 0; i < data; i++)
+		fsi->fifo_max <<= 1;
+	dev_dbg(dai->dev, "fifo = %d words\n", fsi->fifo_max);
+
+	for (i = 1; i < fsi->chan; i *= 2)
+		if (fsi->chan > i)
+			fsi->fifo_max >>= 1;
+	dev_dbg(dai->dev, "%d channel %d store\n", fsi->chan, fsi->fifo_max);
+
 	/*
 	 * clear clk reset if master mode
 	 */
-- 
1.6.3.3

[PATCH 3/4] ASoC: fsi: IRQ related process had be united

[Kuninori Morimoto]

Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 sound/soc/sh/fsi.c |   53 ++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index cc41bae..b588d2d 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -328,7 +328,7 @@ static int fsi_get_fifo_residue(struct fsi_priv *fsi, int is_play)
 /************************************************************************
 
 
-		ctrl function
+		irq function
 
 
 ************************************************************************/
@@ -350,6 +350,35 @@ static void fsi_irq_disable(struct fsi_priv *fsi, int is_play)
 	fsi_master_mask_set(master, IEMSK, data, 0);
 }
 
+static u32 fsi_irq_get_status(struct fsi_master *master)
+{
+	return fsi_master_read(master, INT_ST);
+}
+
+static void fsi_irq_clear_all_status(struct fsi_master *master)
+{
+	fsi_master_write(master, INT_ST, 0x0000000);
+}
+
+static void fsi_irq_clear_status(struct fsi_priv *fsi)
+{
+	u32 data = 0;
+	struct fsi_master *master = fsi_get_master(fsi);
+
+	data |= fsi_port_ab_io_bit(fsi, 0);
+	data |= fsi_port_ab_io_bit(fsi, 1);
+
+	/* clear interrupt factor */
+	fsi_master_mask_set(master, INT_ST, data, 0);
+}
+
+/************************************************************************
+
+
+		ctrl function
+
+
+************************************************************************/
 static void fsi_clk_ctrl(struct fsi_priv *fsi, int enable)
 {
 	u32 val = fsi_is_port_a(fsi) ? (1 << 0) : (1 << 4);
@@ -361,25 +390,17 @@ static void fsi_clk_ctrl(struct fsi_priv *fsi, int enable)
 		fsi_master_mask_set(master, CLK_RST, val, 0);
 }
 
-static void fsi_irq_init(struct fsi_priv *fsi, int is_play)
+static void fsi_fifo_init(struct fsi_priv *fsi, int is_play)
 {
-	u32 data;
 	u32 ctrl;
 
-	data = fsi_port_ab_io_bit(fsi, is_play);
 	ctrl = is_play ? DOFF_CTL : DIFF_CTL;
 
-	/* set IMSK */
-	fsi_irq_disable(fsi, is_play);
-
 	/* set interrupt generation factor */
 	fsi_reg_write(fsi, ctrl, IRQ_HALF);
 
 	/* clear FIFO */
 	fsi_reg_mask_set(fsi, ctrl, FIFO_CLR, FIFO_CLR);
-
-	/* clear interrupt factor */
-	fsi_master_mask_set(fsi_get_master(fsi), INT_ST, data, 0);
 }
 
 static void fsi_soft_all_reset(struct fsi_master *master)
@@ -565,7 +586,7 @@ static int fsi_data_pop(struct fsi_priv *fsi, int startup)
 static irqreturn_t fsi_interrupt(int irq, void *data)
 {
 	struct fsi_master *master = data;
-	u32 int_st = fsi_master_read(master, INT_ST);
+	u32 int_st = fsi_irq_get_status(master);
 
 	/* clear irq status */
 	fsi_master_mask_set(master, SOFT_RST, IR, 0);
@@ -580,7 +601,7 @@ static irqreturn_t fsi_interrupt(int irq, void *data)
 	if (int_st & INT_B_IN)
 		fsi_data_pop(&master->fsib, 0);
 
-	fsi_master_write(master, INT_ST, 0x0000000);
+	fsi_irq_clear_all_status(master);
 
 	return IRQ_HANDLED;
 }
@@ -697,8 +718,12 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
 	if (is_master)
 		fsi_clk_ctrl(fsi, 1);
 
-	/* irq setting */
-	fsi_irq_init(fsi, is_play);
+	/* irq clear */
+	fsi_irq_disable(fsi, is_play);
+	fsi_irq_clear_status(fsi);
+
+	/* fifo init */
+	fsi_fifo_init(fsi, is_play);
 
 	return ret;
 }
-- 
1.6.3.3

[PATCH 0/4] ASoC: fsi: FSI2 device support

[Kuninori Morimoto]

Dear Mark

These patches are FSI2 support

Kuninori Morimoto (4):
      ASoC: fsi: ensures process inside master lock
      ASoC: fsi: Add FIFO size calculate
      ASoC: fsi: IRQ related process had be united
      ASoC: fsi: Add FSI2 device support

ARM SH-MOBILE series have FSI2/FMSI device which are
advance version of FSI.
Above patches add simple FSI2 support.
These patches are tested on AP4 board

Best regards
--
Kuninori Morimoto

[PATCH 1/4] ASoC: fsi: ensures process inside master lock

[Kuninori Morimoto]

Bit operation for fsi_master should be done inside master lock.
But soft-reset/interrupt operation were outside of it.
This patch modify this problem.
It still allow to INT_ST outside-operation on fsi_interrupt,
but it is not problem.
Because this register doesn't need the bit operation.

Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 sound/soc/sh/fsi.c |   22 +++++++++++-----------
 1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index 993abb7..db91349 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -79,6 +79,12 @@
 #define INT_A_IN	(1 << 4)
 #define INT_A_OUT	(1 << 0)
 
+/* SOFT_RST */
+#define PBSR		(1 << 12) /* Port B Software Reset */
+#define PASR		(1 <<  8) /* Port A Software Reset */
+#define IR		(1 <<  4) /* Interrupt Reset */
+#define FSISR		(1 <<  0) /* Software Reset */
+
 #define FSI_RATES SNDRV_PCM_RATE_8000_96000
 
 #define FSI_FMTS (SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE)
@@ -372,18 +378,13 @@ static void fsi_irq_init(struct fsi_priv *fsi, int is_play)
 
 static void fsi_soft_all_reset(struct fsi_master *master)
 {
-	u32 status = fsi_master_read(master, SOFT_RST);
-
 	/* port AB reset */
-	status &= 0x000000ff;
-	fsi_master_write(master, SOFT_RST, status);
+	fsi_master_mask_set(master, SOFT_RST, PASR | PBSR, 0);
 	mdelay(10);
 
 	/* soft reset */
-	status &= 0x000000f0;
-	fsi_master_write(master, SOFT_RST, status);
-	status |= 0x00000001;
-	fsi_master_write(master, SOFT_RST, status);
+	fsi_master_mask_set(master, SOFT_RST, FSISR, 0);
+	fsi_master_mask_set(master, SOFT_RST, FSISR, FSISR);
 	mdelay(10);
 }
 
@@ -558,12 +559,11 @@ static int fsi_data_pop(struct fsi_priv *fsi, int startup)
 static irqreturn_t fsi_interrupt(int irq, void *data)
 {
 	struct fsi_master *master = data;
-	u32 status = fsi_master_read(master, SOFT_RST) & ~0x00000010;
 	u32 int_st = fsi_master_read(master, INT_ST);
 
 	/* clear irq status */
-	fsi_master_write(master, SOFT_RST, status);
-	fsi_master_write(master, SOFT_RST, status | 0x00000010);
+	fsi_master_mask_set(master, SOFT_RST, IR, 0);
+	fsi_master_mask_set(master, SOFT_RST, IR, IR);
 
 	if (int_st & INT_A_OUT)
 		fsi_data_push(&master->fsia, 0);
-- 
1.6.3.3

[PATCH 4/4] ASoC: fsi: Add FSI2 device support

[Kuninori Morimoto]

ARM-SHMOBILE series have FIFO-buffered serial interface 2 (FSI2) device
which is advanced version of FSI.
This patch add simple support for it.

Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
---
 sound/soc/sh/Kconfig |    3 +-
 sound/soc/sh/fsi.c   |   56 +++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/sound/soc/sh/Kconfig b/sound/soc/sh/Kconfig
index f07f6d8..a1d14bc 100644
--- a/sound/soc/sh/Kconfig
+++ b/sound/soc/sh/Kconfig
@@ -1,5 +1,5 @@
 menu "SoC Audio support for SuperH"
-	depends on SUPERH
+	depends on SUPERH || ARCH_SHMOBILE
 
 config SND_SOC_PCM_SH7760
 	tristate "SoC Audio support for Renesas SH7760"
@@ -22,7 +22,6 @@ config SND_SOC_SH4_SSI
 
 config SND_SOC_SH4_FSI
 	tristate "SH4 FSI support"
-	depends on CPU_SUBTYPE_SH7724
 	help
 	  This option enables FSI sound support
 
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index b588d2d..6d90ea1 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -40,6 +40,10 @@
 #define MUTE_ST		0x0028
 #define REG_END		MUTE_ST
 
+
+#define CPU_INT_ST	0x01F4
+#define CPU_IEMSK	0x01F8
+#define CPU_IMSK	0x01FC
 #define INT_ST		0x0200
 #define IEMSK		0x0204
 #define IMSK		0x0208
@@ -47,7 +51,7 @@
 #define CLK_RST		0x0210
 #define SOFT_RST	0x0214
 #define FIFO_SZ		0x0218
-#define MREG_START	INT_ST
+#define MREG_START	CPU_INT_ST
 #define MREG_END	FIFO_SZ
 
 /* DO_FMT */
@@ -116,11 +120,18 @@ struct fsi_priv {
 	int periods;
 };
 
+struct fsi_regs {
+	u32 int_st;
+	u32 iemsk;
+	u32 imsk;
+};
+
 struct fsi_master {
 	void __iomem *base;
 	int irq;
 	struct fsi_priv fsia;
 	struct fsi_priv fsib;
+	struct fsi_regs *regs;
 	struct sh_fsi_platform_info *info;
 	spinlock_t lock;
 };
@@ -337,8 +348,8 @@ static void fsi_irq_enable(struct fsi_priv *fsi, int is_play)
 	u32 data = fsi_port_ab_io_bit(fsi, is_play);
 	struct fsi_master *master = fsi_get_master(fsi);
 
-	fsi_master_mask_set(master, IMSK,  data, data);
-	fsi_master_mask_set(master, IEMSK, data, data);
+	fsi_master_mask_set(master, master->regs->imsk,  data, data);
+	fsi_master_mask_set(master, master->regs->iemsk, data, data);
 }
 
 static void fsi_irq_disable(struct fsi_priv *fsi, int is_play)
@@ -346,18 +357,18 @@ static void fsi_irq_disable(struct fsi_priv *fsi, int is_play)
 	u32 data = fsi_port_ab_io_bit(fsi, is_play);
 	struct fsi_master *master = fsi_get_master(fsi);
 
-	fsi_master_mask_set(master, IMSK,  data, 0);
-	fsi_master_mask_set(master, IEMSK, data, 0);
+	fsi_master_mask_set(master, master->regs->imsk,  data, 0);
+	fsi_master_mask_set(master, master->regs->iemsk, data, 0);
 }
 
 static u32 fsi_irq_get_status(struct fsi_master *master)
 {
-	return fsi_master_read(master, INT_ST);
+	return fsi_master_read(master, master->regs->int_st);
 }
 
 static void fsi_irq_clear_all_status(struct fsi_master *master)
 {
-	fsi_master_write(master, INT_ST, 0x0000000);
+	fsi_master_write(master, master->regs->int_st, 0x0000000);
 }
 
 static void fsi_irq_clear_status(struct fsi_priv *fsi)
@@ -369,7 +380,7 @@ static void fsi_irq_clear_status(struct fsi_priv *fsi)
 	data |= fsi_port_ab_io_bit(fsi, 1);
 
 	/* clear interrupt factor */
-	fsi_master_mask_set(master, INT_ST, data, 0);
+	fsi_master_mask_set(master, master->regs->int_st, data, 0);
 }
 
 /************************************************************************
@@ -935,6 +946,7 @@ EXPORT_SYMBOL_GPL(fsi_soc_platform);
 static int fsi_probe(struct platform_device *pdev)
 {
 	struct fsi_master *master;
+	const struct platform_device_id	*id_entry;
 	struct resource *res;
 	unsigned int irq;
 	int ret;
@@ -944,6 +956,12 @@ static int fsi_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	id_entry = pdev->id_entry;
+	if (!id_entry) {
+		dev_err(&pdev->dev, "unknown fsi device\n");
+		return -ENODEV;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
 	if (!res || (int)irq <= 0) {
@@ -972,6 +990,7 @@ static int fsi_probe(struct platform_device *pdev)
 	master->fsia.master	= master;
 	master->fsib.base	= master->base + 0x40;
 	master->fsib.master	= master;
+	master->regs		= (struct fsi_regs *)id_entry->driver_data;
 	spin_lock_init(&master->lock);
 
 	pm_runtime_enable(&pdev->dev);
@@ -984,7 +1003,8 @@ static int fsi_probe(struct platform_device *pdev)
 
 	fsi_soft_all_reset(master);
 
-	ret = request_irq(irq, &fsi_interrupt, IRQF_DISABLED, "fsi", master);
+	ret = request_irq(irq, &fsi_interrupt, IRQF_DISABLED,
+			  id_entry->name, master);
 	if (ret) {
 		dev_err(&pdev->dev, "irq request err\n");
 		goto exit_iounmap;
@@ -1051,6 +1071,23 @@ static struct dev_pm_ops fsi_pm_ops = {
 	.runtime_resume		= fsi_runtime_nop,
 };
 
+static struct fsi_regs fsi_regs = {
+	.int_st	= INT_ST,
+	.iemsk	= IEMSK,
+	.imsk	= IMSK,
+};
+
+static struct fsi_regs fsi2_regs = {
+	.int_st	= CPU_INT_ST,
+	.iemsk	= CPU_IEMSK,
+	.imsk	= CPU_IMSK,
+};
+
+static struct platform_device_id fsi_id_table[] = {
+	{ "sh_fsi",	(kernel_ulong_t)&fsi_regs },
+	{ "sh_fsi2",	(kernel_ulong_t)&fsi2_regs },
+};
+
 static struct platform_driver fsi_driver = {
 	.driver 	= {
 		.name	= "sh_fsi",
@@ -1058,6 +1095,7 @@ static struct platform_driver fsi_driver = {
 	},
 	.probe		= fsi_probe,
 	.remove		= fsi_remove,
+	.id_table	= fsi_id_table,
 };
 
 static int __init fsi_mobile_init(void)
-- 
1.6.3.3

Re: [PATCH 0/4] ASoC: fsi: FSI2 device support

[Liam Girdwood]

On Wed, 2010-03-24 at 15:26 +0900, Kuninori Morimoto wrote:
> Dear Mark
> 
> These patches are FSI2 support
> 
> Kuninori Morimoto (4):
>       ASoC: fsi: ensures process inside master lock
>       ASoC: fsi: Add FIFO size calculate
>       ASoC: fsi: IRQ related process had be united
>       ASoC: fsi: Add FSI2 device support
> 
> ARM SH-MOBILE series have FSI2/FMSI device which are
> advance version of FSI.
> Above patches add simple FSI2 support.
> These patches are tested on AP4 board

Btw, your clock appears to be a bit broken, patch 2/4 has a time stamp
from 7 days ago...

Liam 

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 2/4] ASoC: fsi: Add FIFO size calculate

[Liam Girdwood]

On Wed, 2010-03-17 at 14:26 +0900, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
> ---
>  sound/soc/sh/fsi.c |   48 +++++++++++++++++++++++-------------------------
>  1 files changed, 23 insertions(+), 25 deletions(-)
> 
> diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
> index db91349..cc41bae 100644
> --- a/sound/soc/sh/fsi.c
> +++ b/sound/soc/sh/fsi.c
> @@ -46,8 +46,9 @@
>  #define MUTE		0x020C
>  #define CLK_RST		0x0210
>  #define SOFT_RST	0x0214
> +#define FIFO_SZ		0x0218
>  #define MREG_START	INT_ST
> -#define MREG_END	SOFT_RST
> +#define MREG_END	FIFO_SZ
>  
>  /* DO_FMT */
>  /* DI_FMT */
> @@ -85,6 +86,11 @@
>  #define IR		(1 <<  4) /* Interrupt Reset */
>  #define FSISR		(1 <<  0) /* Software Reset */
>  
> +/* FIFO_SZ */
> +#define OUT_SZ_MASK	0x7
> +#define BO_SZ_SHIFT	8
> +#define AO_SZ_SHIFT	0
> +
>  #define FSI_RATES SNDRV_PCM_RATE_8000_96000
>  
>  #define FSI_FMTS (SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE)
> @@ -590,11 +596,12 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
>  			   struct snd_soc_dai *dai)
>  {
>  	struct fsi_priv *fsi = fsi_get_priv(substream);
> +	struct fsi_master *master = fsi_get_master(fsi);
>  	const char *msg;
>  	u32 flags = fsi_get_info_flags(fsi);
>  	u32 fmt;
>  	u32 reg;
> -	u32 data;
> +	u32 data, i;
>  	int is_play = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
>  	int is_master;
>  	int ret = 0;
> @@ -668,31 +675,22 @@ static int fsi_dai_startup(struct snd_pcm_substream *substream,
>  		dev_err(dai->dev, "unknown format.\n");
>  		return -EINVAL;
>  	}
> -
> -	switch (fsi->chan) {
> -	case 1:
> -		fsi->fifo_max = 256;
> -		break;
> -	case 2:
> -		fsi->fifo_max = 128;
> -		break;
> -	case 3:
> -	case 4:
> -		fsi->fifo_max = 64;
> -		break;
> -	case 5:
> -	case 6:
> -	case 7:
> -	case 8:
> -		fsi->fifo_max = 32;
> -		break;
> -	default:
> -		dev_err(dai->dev, "channel size error.\n");
> -		return -EINVAL;
> -	}
> -
>  	fsi_reg_write(fsi, reg, data);
>  
> +	/* calculate FIFO size */
> +	data = fsi_master_read(master, FIFO_SZ);
> +	data >>= fsi_is_port_a(fsi) ? AO_SZ_SHIFT : BO_SZ_SHIFT;
> +	data &= OUT_SZ_MASK;
> +	fsi->fifo_max = 256;
> +	for (i = 0; i < data; i++)
> +		fsi->fifo_max <<= 1;

You don't really need the for loop here if you calculate the shift size
and perform the shift in 1 operation. e.g.

fsi->fifo_max <<= data;

> +	dev_dbg(dai->dev, "fifo = %d words\n", fsi->fifo_max);
> +
> +	for (i = 1; i < fsi->chan; i *= 2)
> +		if (fsi->chan > i)
> +			fsi->fifo_max >>= 1;

Is the if really needed here. You may also want to calculate the shift
size rather than looping too.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 3/4] ASoC: fsi: IRQ related process had be united

[Liam Girdwood]

On Tue, 2010-03-23 at 11:47 +0900, Kuninori Morimoto wrote:
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
> ---
>  sound/soc/sh/fsi.c |   53 ++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 39 insertions(+), 14 deletions(-)

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 1/4] ASoC: fsi: ensures process inside master lock

[Liam Girdwood]

On Wed, 2010-03-24 at 15:27 +0900, Kuninori Morimoto wrote:
> Bit operation for fsi_master should be done inside master lock.
> But soft-reset/interrupt operation were outside of it.
> This patch modify this problem.
> It still allow to INT_ST outside-operation on fsi_interrupt,
> but it is not problem.
> Because this register doesn't need the bit operation.
> 
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>
> ---
>  sound/soc/sh/fsi.c |   22 +++++++++++-----------
>  1 files changed, 11 insertions(+), 11 deletions(-)
> 
Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 4/4] ASoC: fsi: Add FSI2 device support

[Liam Girdwood]

On Wed, 2010-03-24 at 15:27 +0900, Kuninori Morimoto wrote:
> ARM-SHMOBILE series have FIFO-buffered serial interface 2 (FSI2) device
> which is advanced version of FSI.
> This patch add simple support for it.
> 
> Signed-off-by: Kuninori Morimoto <morimoto.kuninori@renesas.com>

Acked-by: Liam Girdwood <lrg@slimlogic.co.uk>

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 0/4] ASoC: fsi: FSI2 device support

[Mark Brown]

On Wed, Mar 24, 2010 at 03:26:55PM +0900, Kuninori Morimoto wrote:

> These patches are FSI2 support

> Kuninori Morimoto (4):
>       ASoC: fsi: ensures process inside master lock
>       ASoC: fsi: Add FIFO size calculate
>       ASoC: fsi: IRQ related process had be united
>       ASoC: fsi: Add FSI2 device support

Applied patches 1 and 3, patch 4 was OK but won't apply without the FIFO
size change (and presumably depends on that anyway).  As Liam said it'd
be good if you could check what's going on with these old dates.

Thanks.

Re: [PATCH 2/4] ASoC: fsi: Add FIFO size calculate

[Kuninori Morimoto]

Dear Liam, Mark

Thank you for checking patch

> > +	/* calculate FIFO size */
> > +	data = fsi_master_read(master, FIFO_SZ);
> > +	data >>= fsi_is_port_a(fsi) ? AO_SZ_SHIFT : BO_SZ_SHIFT;
> > +	data &= OUT_SZ_MASK;
> > +	fsi->fifo_max = 256;
> > +	for (i = 0; i < data; i++)
> > +		fsi->fifo_max <<= 1;
> 
> You don't really need the for loop here if you calculate the shift size
> and perform the shift in 1 operation. e.g.

Indeed
Thank you. I will modify this.

> > +	dev_dbg(dai->dev, "fifo = %d words\n", fsi->fifo_max);
> > +
> > +	for (i = 1; i < fsi->chan; i *= 2)
> > +		if (fsi->chan > i)
> > +			fsi->fifo_max >>= 1;
> 
> Is the if really needed here. You may also want to calculate the shift
> size rather than looping too.

This calculate number of sample data.
And it is depend on channel size.
If chip fifo size is 256, the number of sample data will be...

1 chan : 256 (256 x 1 = 256)
2 chan : 128 (128 x 2 = 256)
3 chan :  64 ( 64 x 3 = 192)
4 chan :  64 ( 64 x 4 = 256)
5 chan :  32 ( 32 x 5 = 160)
6 chan :  32 ( 32 x 6 = 192)
7 chan :  32 ( 32 x 7 = 224)
8 chan :  32 ( 32 x 8 = 256)

Above calculation is needed. but yes un-understandable.
I don't know a method computable by one line.
So, I will add tiny comment/document here.
OK ?

> As Liam said it'd
> be good if you could check what's going on with these old dates.

Sorry so much

Best regards
--
Kuninori Morimoto

Re: [PATCH 2/4] ASoC: fsi: Add FIFO size calculate

[Liam Girdwood]

On Thu, 2010-03-25 at 11:39 +0900, Kuninori Morimoto wrote:

> 
> > > +	dev_dbg(dai->dev, "fifo = %d words\n", fsi->fifo_max);
> > > +
> > > +	for (i = 1; i < fsi->chan; i *= 2)
> > > +		if (fsi->chan > i)
> > > +			fsi->fifo_max >>= 1;
> > 
> > Is the if really needed here. You may also want to calculate the shift
> > size rather than looping too.
> 
> This calculate number of sample data.
> And it is depend on channel size.
> If chip fifo size is 256, the number of sample data will be...
> 
> 1 chan : 256 (256 x 1 = 256)
> 2 chan : 128 (128 x 2 = 256)
> 3 chan :  64 ( 64 x 3 = 192)
> 4 chan :  64 ( 64 x 4 = 256)
> 5 chan :  32 ( 32 x 5 = 160)
> 6 chan :  32 ( 32 x 6 = 192)
> 7 chan :  32 ( 32 x 7 = 224)
> 8 chan :  32 ( 32 x 8 = 256)
> 
> Above calculation is needed. but yes un-understandable.
> I don't know a method computable by one line.
> So, I will add tiny comment/document here.
> OK ?
> 

Yes, this sounds fine :)

It wasn't understandable from the original code.

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk