[alsa-devel] [PATCH v2 0/3] ARM: mxs: add recording support for saif
The first series does not have a cover letter. Add it now for v2.
The main changes are move mach-specific code(clkmux in DIGCTL) from saif driver to mach-specific layer based on Wolfram's suggestion.
Note that the last patch is a RFC patch and sent out for testing since without that patch the saif may not work.
Dong Aisheng (2): ARM: mx28evk: add platform data for saif ARM: mx28evk: set a initial clock rate for saif
Wolfram Sang (1): arm: mxs: disable clock-gates when setting saif-clocks
arch/arm/mach-mxs/clock-mx28.c | 63 ++++++++++++++++++++++- arch/arm/mach-mxs/devices-mx28.h | 4 +- arch/arm/mach-mxs/devices/platform-mxs-saif.c | 5 +- arch/arm/mach-mxs/include/mach/common.h | 2 + arch/arm/mach-mxs/include/mach/devices-common.h | 4 +- arch/arm/mach-mxs/include/mach/digctl.h | 18 +++++++ arch/arm/mach-mxs/mach-mx28evk.c | 15 +++++- arch/arm/mach-mxs/regs-clkctrl-mx28.h | 2 + 8 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 arch/arm/mach-mxs/include/mach/digctl.h
This is for supporting saif record function.
Signed-off-by: Dong Aisheng b29396@freescale.com Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Wolfram Sang w.sang@pengutronix.de Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@ti.com
--- changes since v1: * move saif clkmux code into mach-specific part --- arch/arm/mach-mxs/clock-mx28.c | 47 +++++++++++++++++++++++ arch/arm/mach-mxs/devices-mx28.h | 4 +- arch/arm/mach-mxs/devices/platform-mxs-saif.c | 5 +- arch/arm/mach-mxs/include/mach/common.h | 2 + arch/arm/mach-mxs/include/mach/devices-common.h | 4 +- arch/arm/mach-mxs/include/mach/digctl.h | 18 +++++++++ arch/arm/mach-mxs/mach-mx28evk.c | 15 ++++++- 7 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index 7954013..f0c7cb6 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -29,6 +29,7 @@ #include <mach/mx28.h> #include <mach/common.h> #include <mach/clock.h> +#include <mach/digctl.h>
#include "regs-clkctrl-mx28.h"
@@ -43,6 +44,52 @@ static struct clk emi_clk; static struct clk saif0_clk; static struct clk saif1_clk; static struct clk clk32k_clk; +static unsigned int saif_clkmux; + +/* + * HW_SAIF_CLKMUX_SEL: + * DIRECT(0x0): SAIF0 clock pins selected for SAIF0 input clocks, and SAIF1 + * clock pins selected for SAIF1 input clocks. + * CROSSINPUT(0x1): SAIF1 clock inputs selected for SAIF0 input clocks, and + * SAIF0 clock inputs selected for SAIF1 input clocks. + * EXTMSTR0(0x2): SAIF0 clock pin selected for both SAIF0 and SAIF1 input + * clocks. + * EXTMSTR1(0x3): SAIF1 clock pin selected for both SAIF0 and SAIF1 input + * clocks. + */ +int mxs_saif_clkmux_select(unsigned int clkmux) +{ + if (clkmux > 0x3) + return -EINVAL; + + __raw_writel(clkmux << 10, DIGCTRL_BASE_ADDR + MXS_SET_ADDR); + saif_clkmux = clkmux; + return 0; +} + +int mxs_get_saif_clk_master_id(unsigned int saif_id) +{ + unsigned int master_id; + + switch (saif_clkmux) { + case MXS_DIGCTL_SAIF_CLKMUX_DIRECT: + master_id = saif_id; + break; + case MXS_DIGCTL_SAIF_CLKMUX_CROSSINPUT: + master_id = saif_id ? 0 : 1; + break; + case MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0: + master_id = 0; + break; + case MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR1: + master_id = 1; + break; + default: + return -EINVAL; + } + + return master_id; +}
static int _raw_clk_enable(struct clk *clk) { diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h index c888710..40be649 100644 --- a/arch/arm/mach-mxs/devices-mx28.h +++ b/arch/arm/mach-mxs/devices-mx28.h @@ -47,6 +47,8 @@ struct platform_device *__init mx28_add_mxsfb( const struct mxsfb_platform_data *pdata);
extern const struct mxs_saif_data mx28_saif_data[] __initconst; -#define mx28_add_saif(id) mxs_add_saif(&mx28_saif_data[id]) +#define mx28_add_saif(id, pdata) \ + mxs_add_saif(&mx28_saif_data[id], pdata) +
struct platform_device *__init mx28_add_rtc_stmp3xxx(void); diff --git a/arch/arm/mach-mxs/devices/platform-mxs-saif.c b/arch/arm/mach-mxs/devices/platform-mxs-saif.c index 1ec965e..f6e3a60 100644 --- a/arch/arm/mach-mxs/devices/platform-mxs-saif.c +++ b/arch/arm/mach-mxs/devices/platform-mxs-saif.c @@ -32,7 +32,8 @@ const struct mxs_saif_data mx28_saif_data[] __initconst = { }; #endif
-struct platform_device *__init mxs_add_saif(const struct mxs_saif_data *data) +struct platform_device *__init mxs_add_saif(const struct mxs_saif_data *data, + const struct mxs_saif_platform_data *pdata) { struct resource res[] = { { @@ -56,5 +57,5 @@ struct platform_device *__init mxs_add_saif(const struct mxs_saif_data *data) };
return mxs_add_platform_device("mxs-saif", data->id, res, - ARRAY_SIZE(res), NULL, 0); + ARRAY_SIZE(res), pdata, sizeof(*pdata)); } diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h index 635bb5d..bf91a10 100644 --- a/arch/arm/mach-mxs/include/mach/common.h +++ b/arch/arm/mach-mxs/include/mach/common.h @@ -29,4 +29,6 @@ extern void mx28_init_irq(void);
extern void icoll_init_irq(void);
+extern int mxs_saif_clkmux_select(unsigned int clkmux); +extern int mxs_get_saif_clk_master_id(unsigned int saif_id); #endif /* __MACH_MXS_COMMON_H__ */ diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h b/arch/arm/mach-mxs/include/mach/devices-common.h index a8080f4..dc369c1 100644 --- a/arch/arm/mach-mxs/include/mach/devices-common.h +++ b/arch/arm/mach-mxs/include/mach/devices-common.h @@ -94,6 +94,7 @@ struct platform_device *__init mxs_add_mxs_pwm( resource_size_t iobase, int id);
/* saif */ +#include <sound/saif.h> struct mxs_saif_data { int id; resource_size_t iobase; @@ -103,4 +104,5 @@ struct mxs_saif_data { };
struct platform_device *__init mxs_add_saif( - const struct mxs_saif_data *data); + const struct mxs_saif_data *data, + const struct mxs_saif_platform_data *pdata); diff --git a/arch/arm/mach-mxs/include/mach/digctl.h b/arch/arm/mach-mxs/include/mach/digctl.h new file mode 100644 index 0000000..6231e10 --- /dev/null +++ b/arch/arm/mach-mxs/include/mach/digctl.h @@ -0,0 +1,18 @@ +/* + * Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved. + * + * 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 __MACH_DIGCTL_H__ +#define __MACH_DIGCTL_H__ + +/* MXS DIGCTL SAIF CLKMUX */ +#define MXS_DIGCTL_SAIF_CLKMUX_DIRECT 0x0 +#define MXS_DIGCTL_SAIF_CLKMUX_CROSSINPUT 0x1 +#define MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0 0x2 +#define MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR1 0x3 + +#endif diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c index 4a3cca3..6affe5d 100644 --- a/arch/arm/mach-mxs/mach-mx28evk.c +++ b/arch/arm/mach-mxs/mach-mx28evk.c @@ -28,6 +28,7 @@
#include <mach/common.h> #include <mach/iomux-mx28.h> +#include <mach/digctl.h>
#include "devices-mx28.h"
@@ -417,6 +418,16 @@ static void __init mx28evk_add_regulators(void) static void __init mx28evk_add_regulators(void) {} #endif
+static int mx28evk_mxs_saif_pinit(void) +{ + return mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0); +} + +struct mxs_saif_platform_data mx28evk_mxs_saif_pdata __initdata = { + .init = mx28evk_mxs_saif_pinit, + .get_master_id = mxs_get_saif_clk_master_id, +}; + static void __init mx28evk_init(void) { int ret; @@ -457,8 +468,8 @@ static void __init mx28evk_init(void)
mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
- mx28_add_saif(0); - mx28_add_saif(1); + mx28_add_saif(0, &mx28evk_mxs_saif_pdata); + mx28_add_saif(1, &mx28evk_mxs_saif_pdata);
mx28_add_mxs_i2c(0); i2c_register_board_info(0, mxs_i2c0_board_info,
Hello,
On Wed, Sep 07, 2011 at 09:57:36PM +0800, Dong Aisheng wrote:
This is for supporting saif record function.
Signed-off-by: Dong Aisheng b29396@freescale.com Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Wolfram Sang w.sang@pengutronix.de Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@ti.com
changes since v1:
- move saif clkmux code into mach-specific part
arch/arm/mach-mxs/clock-mx28.c | 47 +++++++++++++++++++++++ arch/arm/mach-mxs/devices-mx28.h | 4 +- arch/arm/mach-mxs/devices/platform-mxs-saif.c | 5 +- arch/arm/mach-mxs/include/mach/common.h | 2 + arch/arm/mach-mxs/include/mach/devices-common.h | 4 +- arch/arm/mach-mxs/include/mach/digctl.h | 18 +++++++++ arch/arm/mach-mxs/mach-mx28evk.c | 15 ++++++- 7 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index 7954013..f0c7cb6 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -29,6 +29,7 @@ #include <mach/mx28.h> #include <mach/common.h> #include <mach/clock.h> +#include <mach/digctl.h>
#include "regs-clkctrl-mx28.h"
@@ -43,6 +44,52 @@ static struct clk emi_clk; static struct clk saif0_clk; static struct clk saif1_clk; static struct clk clk32k_clk; +static unsigned int saif_clkmux;
+/*
- HW_SAIF_CLKMUX_SEL:
- DIRECT(0x0): SAIF0 clock pins selected for SAIF0 input clocks, and SAIF1
clock pins selected for SAIF1 input clocks.
- CROSSINPUT(0x1): SAIF1 clock inputs selected for SAIF0 input clocks, and
SAIF0 clock inputs selected for SAIF1 input clocks.
- EXTMSTR0(0x2): SAIF0 clock pin selected for both SAIF0 and SAIF1 input
clocks.
- EXTMSTR1(0x3): SAIF1 clock pin selected for both SAIF0 and SAIF1 input
clocks.- */
+int mxs_saif_clkmux_select(unsigned int clkmux) +{
- if (clkmux > 0x3)
return -EINVAL;- __raw_writel(clkmux << 10, DIGCTRL_BASE_ADDR + MXS_SET_ADDR);
hmm, in my version of the mx28 reference the module is named DIGCTL?!
shouldn't that be something like:
DIGCTRL_BASE_ADDR + DIGCTRL_CTRL + MXS_SET_ADDR?
and is it intended that if you do:
mxs_saif_clkmux_select(1); mxs_saif_clkmux_select(2);
the SAIF_CLKMUX_SEL bitfield ends up being 3?
- saif_clkmux = clkmux;
- return 0;
+}
+int mxs_get_saif_clk_master_id(unsigned int saif_id) +{
- unsigned int master_id;
- switch (saif_clkmux) {
- case MXS_DIGCTL_SAIF_CLKMUX_DIRECT:
master_id = saif_id;break;- case MXS_DIGCTL_SAIF_CLKMUX_CROSSINPUT:
master_id = saif_id ? 0 : 1;break;- case MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0:
master_id = 0;break;- case MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR1:
master_id = 1;break;- default:
return -EINVAL;- }
- return master_id;
+}
These two functions are only supposed to be called by arch code right? (If not, you might need to export them.)
static int _raw_clk_enable(struct clk *clk) { diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h index c888710..40be649 100644 --- a/arch/arm/mach-mxs/devices-mx28.h +++ b/arch/arm/mach-mxs/devices-mx28.h @@ -47,6 +47,8 @@ struct platform_device *__init mx28_add_mxsfb( const struct mxsfb_platform_data *pdata);
extern const struct mxs_saif_data mx28_saif_data[] __initconst; -#define mx28_add_saif(id) mxs_add_saif(&mx28_saif_data[id]) +#define mx28_add_saif(id, pdata) \
- mxs_add_saif(&mx28_saif_data[id], pdata)
should it be a seperate patch to add pdata for saif devices?
undesirable empty line.
struct platform_device *__init mx28_add_rtc_stmp3xxx(void); diff --git a/arch/arm/mach-mxs/devices/platform-mxs-saif.c b/arch/arm/mach-mxs/devices/platform-mxs-saif.c index 1ec965e..f6e3a60 100644 --- a/arch/arm/mach-mxs/devices/platform-mxs-saif.c +++ b/arch/arm/mach-mxs/devices/platform-mxs-saif.c @@ -32,7 +32,8 @@ const struct mxs_saif_data mx28_saif_data[] __initconst = { }; #endif
-struct platform_device *__init mxs_add_saif(const struct mxs_saif_data *data) +struct platform_device *__init mxs_add_saif(const struct mxs_saif_data *data,
const struct mxs_saif_platform_data *pdata){ struct resource res[] = { { @@ -56,5 +57,5 @@ struct platform_device *__init mxs_add_saif(const struct mxs_saif_data *data) };
return mxs_add_platform_device("mxs-saif", data->id, res,
ARRAY_SIZE(res), NULL, 0);
ARRAY_SIZE(res), pdata, sizeof(*pdata));} diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h index 635bb5d..bf91a10 100644 --- a/arch/arm/mach-mxs/include/mach/common.h +++ b/arch/arm/mach-mxs/include/mach/common.h @@ -29,4 +29,6 @@ extern void mx28_init_irq(void);
extern void icoll_init_irq(void);
+extern int mxs_saif_clkmux_select(unsigned int clkmux); +extern int mxs_get_saif_clk_master_id(unsigned int saif_id); #endif /* __MACH_MXS_COMMON_H__ */ diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h b/arch/arm/mach-mxs/include/mach/devices-common.h index a8080f4..dc369c1 100644 --- a/arch/arm/mach-mxs/include/mach/devices-common.h +++ b/arch/arm/mach-mxs/include/mach/devices-common.h @@ -94,6 +94,7 @@ struct platform_device *__init mxs_add_mxs_pwm( resource_size_t iobase, int id);
/* saif */ +#include <sound/saif.h> struct mxs_saif_data { int id; resource_size_t iobase; @@ -103,4 +104,5 @@ struct mxs_saif_data { };
struct platform_device *__init mxs_add_saif(
const struct mxs_saif_data *data);
const struct mxs_saif_data *data,const struct mxs_saif_platform_data *pdata);diff --git a/arch/arm/mach-mxs/include/mach/digctl.h b/arch/arm/mach-mxs/include/mach/digctl.h new file mode 100644 index 0000000..6231e10 --- /dev/null +++ b/arch/arm/mach-mxs/include/mach/digctl.h @@ -0,0 +1,18 @@ +/*
- Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
- 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 __MACH_DIGCTL_H__ +#define __MACH_DIGCTL_H__
+/* MXS DIGCTL SAIF CLKMUX */ +#define MXS_DIGCTL_SAIF_CLKMUX_DIRECT 0x0 +#define MXS_DIGCTL_SAIF_CLKMUX_CROSSINPUT 0x1 +#define MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0 0x2 +#define MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR1 0x3
+#endif diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c index 4a3cca3..6affe5d 100644 --- a/arch/arm/mach-mxs/mach-mx28evk.c +++ b/arch/arm/mach-mxs/mach-mx28evk.c @@ -28,6 +28,7 @@
#include <mach/common.h> #include <mach/iomux-mx28.h> +#include <mach/digctl.h>
#include "devices-mx28.h"
@@ -417,6 +418,16 @@ static void __init mx28evk_add_regulators(void) static void __init mx28evk_add_regulators(void) {} #endif
+static int mx28evk_mxs_saif_pinit(void) +{
- return mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
+}
+struct mxs_saif_platform_data mx28evk_mxs_saif_pdata __initdata = {
const + __initconst?
- .init = mx28evk_mxs_saif_pinit,
- .get_master_id = mxs_get_saif_clk_master_id,
+};
static void __init mx28evk_init(void) { int ret; @@ -457,8 +468,8 @@ static void __init mx28evk_init(void)
mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
- mx28_add_saif(0);
- mx28_add_saif(1);
mx28_add_saif(0, &mx28evk_mxs_saif_pdata);
mx28_add_saif(1, &mx28evk_mxs_saif_pdata);
mx28_add_mxs_i2c(0); i2c_register_board_info(0, mxs_i2c0_board_info,
-- 1.7.0.4
Best regards Uwe
-----Original Message----- From: linux-arm-kernel-bounces@lists.infradead.org [mailto:linux-arm- kernel-bounces@lists.infradead.org] On Behalf Of Uwe Kleine-K?nig Sent: Thursday, September 08, 2011 2:18 AM To: Dong Aisheng-B29396 Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de; broonie@opensource.wolfsonmicro.com; w.sang@pengutronix.de; kernel@pengutronix.de; lrg@ti.com; linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 1/3] ARM: mx28evk: add platform data for saif
Hello,
On Wed, Sep 07, 2011 at 09:57:36PM +0800, Dong Aisheng wrote:
This is for supporting saif record function.
Signed-off-by: Dong Aisheng b29396@freescale.com Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Wolfram Sang w.sang@pengutronix.de Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@ti.com
changes since v1:
- move saif clkmux code into mach-specific part
arch/arm/mach-mxs/clock-mx28.c | 47
+++++++++++++++++++++++
arch/arm/mach-mxs/devices-mx28.h | 4 +- arch/arm/mach-mxs/devices/platform-mxs-saif.c | 5 +- arch/arm/mach-mxs/include/mach/common.h | 2 + arch/arm/mach-mxs/include/mach/devices-common.h | 4 +- arch/arm/mach-mxs/include/mach/digctl.h | 18 +++++++++ arch/arm/mach-mxs/mach-mx28evk.c | 15 ++++++- 7 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index 7954013..f0c7cb6 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -29,6 +29,7 @@ #include <mach/mx28.h> #include <mach/common.h> #include <mach/clock.h> +#include <mach/digctl.h>
#include "regs-clkctrl-mx28.h"
@@ -43,6 +44,52 @@ static struct clk emi_clk; static struct clk saif0_clk; static struct clk saif1_clk; static struct clk clk32k_clk; +static unsigned int saif_clkmux;
+/*
- HW_SAIF_CLKMUX_SEL:
- DIRECT(0x0): SAIF0 clock pins selected for SAIF0 input clocks, and
SAIF1
clock pins selected for SAIF1 input clocks.
- CROSSINPUT(0x1): SAIF1 clock inputs selected for SAIF0 input
clocks, and
SAIF0 clock inputs selected for SAIF1 input clocks.
- EXTMSTR0(0x2): SAIF0 clock pin selected for both SAIF0 and SAIF1
input
clocks.
- EXTMSTR1(0x3): SAIF1 clock pin selected for both SAIF0 and SAIF1
input
clocks.- */
+int mxs_saif_clkmux_select(unsigned int clkmux) {
- if (clkmux > 0x3)
return -EINVAL;- __raw_writel(clkmux << 10, DIGCTRL_BASE_ADDR + MXS_SET_ADDR);
hmm, in my version of the mx28 reference the module is named DIGCTL?!
shouldn't that be something like:
DIGCTRL_BASE_ADDR + DIGCTRL_CTRL + MXS_SET_ADDR?
Yes, DIGCTRL_CTRL is still not defined. So for conveniently I just used DIGCTRL_BASE_ADDR. I'm going to add it in digctl.h. If it's not the right place please let me know.
and is it intended that if you do:
mxs_saif_clkmux_select(1); mxs_saif_clkmux_select(2);
the SAIF_CLKMUX_SEL bitfield ends up being 3?
Yes, it ends up being 3.
- saif_clkmux = clkmux;
- return 0;
+}
+int mxs_get_saif_clk_master_id(unsigned int saif_id) {
- unsigned int master_id;
- switch (saif_clkmux) {
- case MXS_DIGCTL_SAIF_CLKMUX_DIRECT:
master_id = saif_id;break;- case MXS_DIGCTL_SAIF_CLKMUX_CROSSINPUT:
master_id = saif_id ? 0 : 1;break;- case MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0:
master_id = 0;break;- case MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR1:
master_id = 1;break;- default:
return -EINVAL;- }
- return master_id;
+}
These two functions are only supposed to be called by arch code right? (If not, you might need to export them.)
Yes, it's supposed to be called by arch code right now.
static int _raw_clk_enable(struct clk *clk) { diff --git a/arch/arm/mach-mxs/devices-mx28.h b/arch/arm/mach-mxs/devices-mx28.h index c888710..40be649 100644 --- a/arch/arm/mach-mxs/devices-mx28.h +++ b/arch/arm/mach-mxs/devices-mx28.h @@ -47,6 +47,8 @@ struct platform_device *__init mx28_add_mxsfb( const struct mxsfb_platform_data *pdata);
extern const struct mxs_saif_data mx28_saif_data[] __initconst; -#define mx28_add_saif(id)
mxs_add_saif(&mx28_saif_data[id])
+#define mx28_add_saif(id, pdata) \
- mxs_add_saif(&mx28_saif_data[id], pdata)
should it be a seperate patch to add pdata for saif devices?
How do you suggest to separate? All changes are mainly to add saif pdata. You mean to move DIGCTL related saif pdata function to another patch?
undesirable empty line.
Got it. I will remove.
struct platform_device *__init mx28_add_rtc_stmp3xxx(void); diff --git a/arch/arm/mach-mxs/devices/platform-mxs-saif.c b/arch/arm/mach-mxs/devices/platform-mxs-saif.c index 1ec965e..f6e3a60 100644 --- a/arch/arm/mach-mxs/devices/platform-mxs-saif.c +++ b/arch/arm/mach-mxs/devices/platform-mxs-saif.c @@ -32,7 +32,8 @@ const struct mxs_saif_data mx28_saif_data[] __initconst = { }; #endif
-struct platform_device *__init mxs_add_saif(const struct mxs_saif_data *data) +struct platform_device *__init mxs_add_saif(const struct mxs_saif_data
*data,
const struct mxs_saif_platform_data *pdata){ struct resource res[] = { { @@ -56,5 +57,5 @@ struct platform_device *__init mxs_add_saif(const
struct mxs_saif_data *data)
};
return mxs_add_platform_device("mxs-saif", data->id, res,
ARRAY_SIZE(res), NULL, 0);
ARRAY_SIZE(res), pdata, sizeof(*pdata));} diff --git a/arch/arm/mach-mxs/include/mach/common.h b/arch/arm/mach-mxs/include/mach/common.h index 635bb5d..bf91a10 100644 --- a/arch/arm/mach-mxs/include/mach/common.h +++ b/arch/arm/mach-mxs/include/mach/common.h @@ -29,4 +29,6 @@ extern void mx28_init_irq(void);
extern void icoll_init_irq(void);
+extern int mxs_saif_clkmux_select(unsigned int clkmux); extern int +mxs_get_saif_clk_master_id(unsigned int saif_id); #endif /* __MACH_MXS_COMMON_H__ */ diff --git a/arch/arm/mach-mxs/include/mach/devices-common.h b/arch/arm/mach-mxs/include/mach/devices-common.h index a8080f4..dc369c1 100644 --- a/arch/arm/mach-mxs/include/mach/devices-common.h +++ b/arch/arm/mach-mxs/include/mach/devices-common.h @@ -94,6 +94,7 @@ struct platform_device *__init mxs_add_mxs_pwm( resource_size_t iobase, int id);
/* saif */ +#include <sound/saif.h> struct mxs_saif_data { int id; resource_size_t iobase; @@ -103,4 +104,5 @@ struct mxs_saif_data { };
struct platform_device *__init mxs_add_saif(
const struct mxs_saif_data *data);
const struct mxs_saif_data *data,const struct mxs_saif_platform_data *pdata);diff --git a/arch/arm/mach-mxs/include/mach/digctl.h b/arch/arm/mach-mxs/include/mach/digctl.h new file mode 100644 index 0000000..6231e10 --- /dev/null +++ b/arch/arm/mach-mxs/include/mach/digctl.h @@ -0,0 +1,18 @@ +/*
- Copyright 2011 Freescale Semiconductor, Inc. All Rights Reserved.
- 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 __MACH_DIGCTL_H__ +#define __MACH_DIGCTL_H__
+/* MXS DIGCTL SAIF CLKMUX */ +#define MXS_DIGCTL_SAIF_CLKMUX_DIRECT 0x0 +#define MXS_DIGCTL_SAIF_CLKMUX_CROSSINPUT 0x1 +#define MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0 0x2 +#define MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR1 0x3
+#endif diff --git a/arch/arm/mach-mxs/mach-mx28evk.c b/arch/arm/mach-mxs/mach-mx28evk.c index 4a3cca3..6affe5d 100644 --- a/arch/arm/mach-mxs/mach-mx28evk.c +++ b/arch/arm/mach-mxs/mach-mx28evk.c @@ -28,6 +28,7 @@
#include <mach/common.h> #include <mach/iomux-mx28.h> +#include <mach/digctl.h>
#include "devices-mx28.h"
@@ -417,6 +418,16 @@ static void __init mx28evk_add_regulators(void) static void __init mx28evk_add_regulators(void) {} #endif
+static int mx28evk_mxs_saif_pinit(void) {
- return mxs_saif_clkmux_select(MXS_DIGCTL_SAIF_CLKMUX_EXTMSTR0);
+}
+struct mxs_saif_platform_data mx28evk_mxs_saif_pdata __initdata = {
const + __initconst?
Yes, will change like that.
- .init = mx28evk_mxs_saif_pinit,
- .get_master_id = mxs_get_saif_clk_master_id, };
static void __init mx28evk_init(void) { int ret; @@ -457,8 +468,8 @@ static void __init mx28evk_init(void)
mx28_add_mxsfb(&mx28evk_mxsfb_pdata);
- mx28_add_saif(0);
- mx28_add_saif(1);
mx28_add_saif(0, &mx28evk_mxs_saif_pdata);
mx28_add_saif(1, &mx28evk_mxs_saif_pdata);
mx28_add_mxs_i2c(0); i2c_register_board_info(0, mxs_i2c0_board_info,
-- 1.7.0.4
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
+static unsigned int saif_clkmux;
Can't we just read the register in get_master and spare the static variable?
and is it intended that if you do:
mxs_saif_clkmux_select(1); mxs_saif_clkmux_select(2);
the SAIF_CLKMUX_SEL bitfield ends up being 3?
Yes, it ends up being 3.
Needs to be fixed, of course. The function will need locking then.
extern const struct mxs_saif_data mx28_saif_data[] __initconst; -#define mx28_add_saif(id)
mxs_add_saif(&mx28_saif_data[id])
+#define mx28_add_saif(id, pdata) \
- mxs_add_saif(&mx28_saif_data[id], pdata)
should it be a seperate patch to add pdata for saif devices?
How do you suggest to separate? All changes are mainly to add saif pdata. You mean to move DIGCTL related saif pdata function to another patch?
I think he meant one patch for adding the functions in clock-mx28.c and one patch to use the stuff when adding the pdata. Uwe?
Wolfram
Hello,
On Fri, Sep 09, 2011 at 03:41:05PM +0200, Wolfram Sang wrote:
extern const struct mxs_saif_data mx28_saif_data[] __initconst; -#define mx28_add_saif(id)
mxs_add_saif(&mx28_saif_data[id])
+#define mx28_add_saif(id, pdata) \
- mxs_add_saif(&mx28_saif_data[id], pdata)
should it be a seperate patch to add pdata for saif devices?
How do you suggest to separate? All changes are mainly to add saif pdata. You mean to move DIGCTL related saif pdata function to another patch?
I think he meant one patch for adding the functions in clock-mx28.c and one patch to use the stuff when adding the pdata. Uwe?
yeah.
Uwe
Signed-off-by: Dong Aisheng b29396@freescale.com Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Wolfram Sang w.sang@pengutronix.de Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@ti.com
--- No changes since v1.
Note this patch still needs another patch from Wolfram to work. I send out it in this series as [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
As discussed with Wolfram, that patch may still have some later work to do. However, we just send out it for testers to work first. --- arch/arm/mach-mxs/clock-mx28.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index f0c7cb6..33cc2ff 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void) clk_set_parent(&saif0_clk, &pll0_clk); clk_set_parent(&saif1_clk, &pll0_clk);
+ /* + * Set an initial clk rate for saif's internal logic to work properly, + * this is especially for the saif working on EXTMASTER mode that who + * uses other saif's BITCLK&LRCLK but it still needs a basic clk which + * should be bigger enough for its internal logic. + */ + clk_set_rate(&saif0_clk, 24000000); + clk_set_rate(&saif1_clk, 24000000); + clkdev_add_table(lookups, ARRAY_SIZE(lookups));
mxs_timer_init(&clk32k_clk, MX28_INT_TIMER0);
On Wed, Sep 07, 2011 at 09:57:37PM +0800, Dong Aisheng wrote:
Signed-off-by: Dong Aisheng b29396@freescale.com Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Wolfram Sang w.sang@pengutronix.de Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@ti.com
No changes since v1.
Note this patch still needs another patch from Wolfram to work. I send out it in this series as [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
If your patch 2 depends on patch 3 there is something fishy.
As discussed with Wolfram, that patch may still have some later work to do. However, we just send out it for testers to work first.
arch/arm/mach-mxs/clock-mx28.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index f0c7cb6..33cc2ff 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void) clk_set_parent(&saif0_clk, &pll0_clk); clk_set_parent(&saif1_clk, &pll0_clk);
- /*
* Set an initial clk rate for saif's internal logic to work properly,* this is especially for the saif working on EXTMASTER mode that who* uses other saif's BITCLK&LRCLK but it still needs a basic clk which* should be bigger enough for its internal logic.
This is not a proper (German) English sentence. Maybe this is better?:
Set an initial clock rate for the saif internal logic to work properly. This is important when working in EXTMASTER mode that uses the other saif's BITCLK&LRCLK but it still needs a basic clock which should be fast enough for the internal logic.
*/- clk_set_rate(&saif0_clk, 24000000);
- clk_set_rate(&saif1_clk, 24000000);
Do you need to check for clk_set_rate returning -ESOMETHING here?
Uwe
clkdev_add_table(lookups, ARRAY_SIZE(lookups));
mxs_timer_init(&clk32k_clk, MX28_INT_TIMER0);
-----Original Message----- From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de] Sent: Thursday, September 08, 2011 2:22 AM To: Dong Aisheng-B29396 Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de; broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm- kernel@lists.infradead.org; w.sang@pengutronix.de Subject: Re: [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif
On Wed, Sep 07, 2011 at 09:57:37PM +0800, Dong Aisheng wrote:
Signed-off-by: Dong Aisheng b29396@freescale.com Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Wolfram Sang w.sang@pengutronix.de Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@ti.com
No changes since v1.
Note this patch still needs another patch from Wolfram to work. I send out it in this series as [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
If your patch 2 depends on patch 3 there is something fishy.
Yes, because the original saif clk setting code has some issues Which can be fixed in patch 3.
As discussed with Wolfram, that patch may still have some later work to do. However, we just send out it for testers to work first.
arch/arm/mach-mxs/clock-mx28.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index f0c7cb6..33cc2ff 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void) clk_set_parent(&saif0_clk, &pll0_clk); clk_set_parent(&saif1_clk, &pll0_clk);
- /*
* Set an initial clk rate for saif's internal logic to workproperly,
* this is especially for the saif working on EXTMASTER mode thatwho
* uses other saif's BITCLK&LRCLK but it still needs a basic clkwhich
* should be bigger enough for its internal logic.This is not a proper (German) English sentence. Maybe this is better?:
Set an initial clock rate for the saif internal logic to work properly. This is important when working in EXTMASTER mode that uses the other saif's BITCLK&LRCLK but it still needs a basic clock which should be fast enough for the internal logic.
Yes, I think it's better. :) I will change like that.
*/- clk_set_rate(&saif0_clk, 24000000);
- clk_set_rate(&saif1_clk, 24000000);
Do you need to check for clk_set_rate returning -ESOMETHING here?
Basically this rate setting will not fail in clock_init. I noticed arch/arm/mach-mx5/clock-mx51-mx53.c also used like this. Should I add an error checking here?
Regards Dong Aisheng
Hello (must I say Dong or Aisheng here?)
On Thu, Sep 08, 2011 at 08:03:17AM +0000, Dong Aisheng-B29396 wrote:
-----Original Message----- From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de] Sent: Thursday, September 08, 2011 2:22 AM To: Dong Aisheng-B29396 Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de; broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm- kernel@lists.infradead.org; w.sang@pengutronix.de Subject: Re: [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif
On Wed, Sep 07, 2011 at 09:57:37PM +0800, Dong Aisheng wrote:
Signed-off-by: Dong Aisheng b29396@freescale.com Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Wolfram Sang w.sang@pengutronix.de Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@ti.com
No changes since v1.
Note this patch still needs another patch from Wolfram to work. I send out it in this series as [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
If your patch 2 depends on patch 3 there is something fishy.
Yes, because the original saif clk setting code has some issues Which can be fixed in patch 3.
So patch 3 should come before patch 2 in the series.
As discussed with Wolfram, that patch may still have some later work to do. However, we just send out it for testers to work first.
arch/arm/mach-mxs/clock-mx28.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index f0c7cb6..33cc2ff 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void) clk_set_parent(&saif0_clk, &pll0_clk); clk_set_parent(&saif1_clk, &pll0_clk);
- /*
* Set an initial clk rate for saif's internal logic to workproperly,
* this is especially for the saif working on EXTMASTER mode thatwho
* uses other saif's BITCLK&LRCLK but it still needs a basic clkwhich
* should be bigger enough for its internal logic.This is not a proper (German) English sentence. Maybe this is better?:
Set an initial clock rate for the saif internal logic to work properly. This is important when working in EXTMASTER mode that uses the other saif's BITCLK&LRCLK but it still needs a basic clock which should be fast enough for the internal logic.
Yes, I think it's better. :) I will change like that.
*/- clk_set_rate(&saif0_clk, 24000000);
- clk_set_rate(&saif1_clk, 24000000);
Do you need to check for clk_set_rate returning -ESOMETHING here?
Basically this rate setting will not fail in clock_init. I noticed arch/arm/mach-mx5/clock-mx51-mx53.c also used like this. Should I add an error checking here?
Probably it's OK as is then.
Uwe
-----Original Message----- From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de] Sent: Thursday, September 08, 2011 4:56 PM To: Dong Aisheng-B29396 Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de; broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm- kernel@lists.infradead.org; w.sang@pengutronix.de Subject: Re: [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif
Hello (must I say Dong or Aisheng here?)
Both are ok. If conveniently, I prefer Aisheng. :)
On Thu, Sep 08, 2011 at 08:03:17AM +0000, Dong Aisheng-B29396 wrote:
-----Original Message----- From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de] Sent: Thursday, September 08, 2011 2:22 AM To: Dong Aisheng-B29396 Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de; broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm- kernel@lists.infradead.org; w.sang@pengutronix.de Subject: Re: [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif
On Wed, Sep 07, 2011 at 09:57:37PM +0800, Dong Aisheng wrote:
Signed-off-by: Dong Aisheng b29396@freescale.com Cc: Sascha Hauer s.hauer@pengutronix.de Cc: Wolfram Sang w.sang@pengutronix.de Cc: Mark Brown broonie@opensource.wolfsonmicro.com Cc: Liam Girdwood lrg@ti.com
No changes since v1.
Note this patch still needs another patch from Wolfram to work. I send out it in this series as [PATCH 3/3] arm: mxs: disable clock-gates when setting saif-clocks
If your patch 2 depends on patch 3 there is something fishy.
Yes, because the original saif clk setting code has some issues Which can be fixed in patch 3.
So patch 3 should come before patch 2 in the series.
Question is that do you think the patch could be accepted? If yes, I will change it as patch 2. Or it may still be patch3 just for testing and wait for the further fixing.
As discussed with Wolfram, that patch may still have some later work to do. However, we just send out it for testers to work first.
arch/arm/mach-mxs/clock-mx28.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index f0c7cb6..33cc2ff 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -826,6 +826,15 @@ int __init mx28_clocks_init(void) clk_set_parent(&saif0_clk, &pll0_clk); clk_set_parent(&saif1_clk, &pll0_clk);
- /*
* Set an initial clk rate for saif's internal logic to workproperly,
* this is especially for the saif working on EXTMASTER mode+that
who
* uses other saif's BITCLK&LRCLK but it still needs a basicclk
which
* should be bigger enough for its internal logic.This is not a proper (German) English sentence. Maybe this is better?:
Set an initial clock rate for the saif internal logic to work properly. This is important when working in EXTMASTER mode that uses the other saif's BITCLK&LRCLK but it still needs a basic clock which should be fast enough for the internal logic.
Yes, I think it's better. :) I will change like that.
*/- clk_set_rate(&saif0_clk, 24000000);
- clk_set_rate(&saif1_clk, 24000000);
Do you need to check for clk_set_rate returning -ESOMETHING here?
Basically this rate setting will not fail in clock_init. I noticed arch/arm/mach-mx5/clock-mx51-mx53.c also used like this. Should I add an error checking here?
Probably it's OK as is then.
Ok, thanks.
Regards Dong Aisheng
From: Wolfram Sang w.sang@pengutronix.de
New divides should only be written when gates are off.
Reported-by: Dong Aisheng b29396@freescale.com Signed-off-by: Wolfram Sang w.sang@pengutronix.de
--- Changes since v1: * Fix author name. It should be Wolfram Sang. :)
BTW, i did a minus change based on wolfram's patch or the saif will not work.
Change + __raw_writel(clkgate, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs##_SET); \ to + __raw_writel(reg & ~clkgate, \ CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \
It seemed HW_CLKCTRL_##rs##_SET did not work well. (i did not find HW_CLKCTRL_SAIFx_SET in spec). --- arch/arm/mach-mxs/clock-mx28.c | 7 +++++-- arch/arm/mach-mxs/regs-clkctrl-mx28.h | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index 33cc2ff..c9d0548 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -476,7 +476,7 @@ _CLK_SET_RATE1(xbus_clk, XBUS) static int name##_set_rate(struct clk *clk, unsigned long rate) \ { \ u16 div; \ - u32 reg; \ + u32 reg, clkgate; \ u64 lrate; \ unsigned long parent_rate; \ int i; \ @@ -493,7 +493,8 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \ return -EINVAL; \ \ reg = __raw_readl(CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \ - reg &= ~BM_CLKCTRL_##rs##_DIV; \ + clkgate = reg & BM_CLKCTRL_##rs##_CLKGATE; \ + reg &= ~(BM_CLKCTRL_##rs##_DIV | BM_CLKCTRL_##rs##_CLKGATE); \ reg |= div << BP_CLKCTRL_##rs##_DIV; \ __raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \ \ @@ -506,6 +507,8 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \ return -ETIMEDOUT; \ } \ \ + __raw_writel(reg & ~clkgate, \ + CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \ return 0; \ }
diff --git a/arch/arm/mach-mxs/regs-clkctrl-mx28.h b/arch/arm/mach-mxs/regs-clkctrl-mx28.h index 7d1b061..08749b3 100644 --- a/arch/arm/mach-mxs/regs-clkctrl-mx28.h +++ b/arch/arm/mach-mxs/regs-clkctrl-mx28.h @@ -285,6 +285,7 @@ (((v) << 0) & BM_CLKCTRL_EMI_DIV_EMI)
#define HW_CLKCTRL_SAIF0 (0x00000100) +#define HW_CLKCTRL_SAIF0_SET (0x00000104)
#define BP_CLKCTRL_SAIF0_CLKGATE 31 #define BM_CLKCTRL_SAIF0_CLKGATE 0x80000000 @@ -296,6 +297,7 @@ (((v) << 0) & BM_CLKCTRL_SAIF0_DIV)
#define HW_CLKCTRL_SAIF1 (0x00000110) +#define HW_CLKCTRL_SAIF1_SET (0x00000114)
#define BP_CLKCTRL_SAIF1_CLKGATE 31 #define BM_CLKCTRL_SAIF1_CLKGATE 0x80000000
On Wed, Sep 07, 2011 at 09:57:38PM +0800, Dong Aisheng wrote:
From: Wolfram Sang w.sang@pengutronix.de
New divides should only be written when gates are off.
Reported-by: Dong Aisheng b29396@freescale.com Signed-off-by: Wolfram Sang w.sang@pengutronix.de
Changes since v1:
- Fix author name. It should be Wolfram Sang. :)
BTW, i did a minus change based on wolfram's patch or the saif will not work.
Change
__raw_writel(clkgate, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs##_SET); \to
__raw_writel(reg & ~clkgate, \ CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \
IMHO this is worth to be noted in the commit log.
It seemed HW_CLKCTRL_##rs##_SET did not work well. (i did not find HW_CLKCTRL_SAIFx_SET in spec).
arch/arm/mach-mxs/clock-mx28.c | 7 +++++-- arch/arm/mach-mxs/regs-clkctrl-mx28.h | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index 33cc2ff..c9d0548 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -476,7 +476,7 @@ _CLK_SET_RATE1(xbus_clk, XBUS) static int name##_set_rate(struct clk *clk, unsigned long rate) \ { \ u16 div; \
- u32 reg; \
- u32 reg, clkgate; \ u64 lrate; \ unsigned long parent_rate; \ int i; \
@@ -493,7 +493,8 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \ return -EINVAL; \ \ reg = __raw_readl(CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \
- reg &= ~BM_CLKCTRL_##rs##_DIV; \
- clkgate = reg & BM_CLKCTRL_##rs##_CLKGATE; \
- reg &= ~(BM_CLKCTRL_##rs##_DIV | BM_CLKCTRL_##rs##_CLKGATE); \ reg |= div << BP_CLKCTRL_##rs##_DIV; \ __raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \ \
@@ -506,6 +507,8 @@ static int name##_set_rate(struct clk *clk, unsigned long rate) \ return -ETIMEDOUT; \ } \ \
- __raw_writel(reg & ~clkgate, \
CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \}
diff --git a/arch/arm/mach-mxs/regs-clkctrl-mx28.h b/arch/arm/mach-mxs/regs-clkctrl-mx28.h index 7d1b061..08749b3 100644 --- a/arch/arm/mach-mxs/regs-clkctrl-mx28.h +++ b/arch/arm/mach-mxs/regs-clkctrl-mx28.h @@ -285,6 +285,7 @@ (((v) << 0) & BM_CLKCTRL_EMI_DIV_EMI)
#define HW_CLKCTRL_SAIF0 (0x00000100) +#define HW_CLKCTRL_SAIF0_SET (0x00000104)
Is this still needed?
#define BP_CLKCTRL_SAIF0_CLKGATE 31 #define BM_CLKCTRL_SAIF0_CLKGATE 0x80000000 @@ -296,6 +297,7 @@ (((v) << 0) & BM_CLKCTRL_SAIF0_DIV)
#define HW_CLKCTRL_SAIF1 (0x00000110) +#define HW_CLKCTRL_SAIF1_SET (0x00000114)
#define BP_CLKCTRL_SAIF1_CLKGATE 31 #define BM_CLKCTRL_SAIF1_CLKGATE 0x80000000
Best regards Uwe
-----Original Message----- From: Uwe Kleine-König [mailto:u.kleine-koenig@pengutronix.de] Sent: Thursday, September 08, 2011 2:25 AM To: Dong Aisheng-B29396 Cc: alsa-devel@alsa-project.org; s.hauer@pengutronix.de; broonie@opensource.wolfsonmicro.com; lrg@ti.com; linux-arm- kernel@lists.infradead.org; w.sang@pengutronix.de Subject: Re: [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks
On Wed, Sep 07, 2011 at 09:57:38PM +0800, Dong Aisheng wrote:
From: Wolfram Sang w.sang@pengutronix.de
New divides should only be written when gates are off.
Reported-by: Dong Aisheng b29396@freescale.com Signed-off-by: Wolfram Sang w.sang@pengutronix.de
Changes since v1:
- Fix author name. It should be Wolfram Sang. :)
BTW, i did a minus change based on wolfram's patch or the saif will not work.
Change
__raw_writel(clkgate, CLKCTRL_BASE_ADDR +- HW_CLKCTRL_##rs##_SET); \
to
__raw_writel(reg & ~clkgate, \ CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \IMHO this is worth to be noted in the commit log.
Yes.
It seemed HW_CLKCTRL_##rs##_SET did not work well. (i did not find HW_CLKCTRL_SAIFx_SET in spec).
arch/arm/mach-mxs/clock-mx28.c | 7 +++++-- arch/arm/mach-mxs/regs-clkctrl-mx28.h | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-mxs/clock-mx28.c b/arch/arm/mach-mxs/clock-mx28.c index 33cc2ff..c9d0548 100644 --- a/arch/arm/mach-mxs/clock-mx28.c +++ b/arch/arm/mach-mxs/clock-mx28.c @@ -476,7 +476,7 @@ _CLK_SET_RATE1(xbus_clk, XBUS) static int name##_set_rate(struct clk *clk, unsigned long rate)
\
{ \ u16 div; \
- u32 reg; \
- u32 reg, clkgate; \ u64 lrate; \ unsigned long parent_rate; \ int i; \
@@ -493,7 +493,8 @@ static int name##_set_rate(struct clk *clk,
unsigned long rate) \
return -EINVAL; \ \reg = __raw_readl(CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \
- reg &= ~BM_CLKCTRL_##rs##_DIV; \
- clkgate = reg & BM_CLKCTRL_##rs##_CLKGATE; \
- reg &= ~(BM_CLKCTRL_##rs##_DIV | BM_CLKCTRL_##rs##_CLKGATE); \ reg |= div << BP_CLKCTRL_##rs##_DIV; \ __raw_writel(reg, CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \ \
@@ -506,6 +507,8 @@ static int name##_set_rate(struct clk *clk,
unsigned long rate) \
return -ETIMEDOUT; \} \ \
- __raw_writel(reg & ~clkgate, \
CLKCTRL_BASE_ADDR + HW_CLKCTRL_##rs); \}
diff --git a/arch/arm/mach-mxs/regs-clkctrl-mx28.h b/arch/arm/mach-mxs/regs-clkctrl-mx28.h index 7d1b061..08749b3 100644 --- a/arch/arm/mach-mxs/regs-clkctrl-mx28.h +++ b/arch/arm/mach-mxs/regs-clkctrl-mx28.h @@ -285,6 +285,7 @@ (((v) << 0) & BM_CLKCTRL_EMI_DIV_EMI)
#define HW_CLKCTRL_SAIF0 (0x00000100) +#define HW_CLKCTRL_SAIF0_SET (0x00000104)
Is this still needed?
No, we may not need this.
For this patch, as discussed with Wolfram before, since we also find Some other devices have the same issue as SAIF(e.g. SSP), it looks we may have further work to do on clock-mx28.c.
However the original purpose of sending this patch is for poeple to easily test.
Regards Dong Aisheng
For this patch, as discussed with Wolfram before, since we also find Some other devices have the same issue as SAIF(e.g. SSP), it looks we may have further work to do on clock-mx28.c.
However the original purpose of sending this patch is for poeple to easily test.
I will update this patch later on the train :)
Regards,
Wolfram
participants (4)
-
Dong Aisheng -
Dong Aisheng-B29396
-
Uwe Kleine-König -
Wolfram Sang