* [PATCH v2 0/3] ARM: mxs: add recording support for saif
@ 2011-09-07 13:57 Dong Aisheng
2011-09-07 13:57 ` [PATCH v2 1/3] ARM: mx28evk: add platform data " Dong Aisheng
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Dong Aisheng @ 2011-09-07 13:57 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH v2 1/3] ARM: mx28evk: add platform data for saif 2011-09-07 13:57 [PATCH v2 0/3] ARM: mxs: add recording support for saif Dong Aisheng @ 2011-09-07 13:57 ` Dong Aisheng 2011-09-07 18:18 ` Uwe Kleine-König 2011-09-07 13:57 ` [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate " Dong Aisheng 2011-09-07 13:57 ` [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks Dong Aisheng 2 siblings, 1 reply; 15+ messages in thread From: Dong Aisheng @ 2011-09-07 13:57 UTC (permalink / raw) To: linux-arm-kernel 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, -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] ARM: mx28evk: add platform data for saif 2011-09-07 13:57 ` [PATCH v2 1/3] ARM: mx28evk: add platform data " Dong Aisheng @ 2011-09-07 18:18 ` Uwe Kleine-König 2011-09-08 7:52 ` Dong Aisheng-B29396 0 siblings, 1 reply; 15+ messages in thread From: Uwe Kleine-König @ 2011-09-07 18:18 UTC (permalink / raw) To: linux-arm-kernel 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 -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] ARM: mx28evk: add platform data for saif 2011-09-07 18:18 ` Uwe Kleine-König @ 2011-09-08 7:52 ` Dong Aisheng-B29396 2011-09-09 13:41 ` Wolfram Sang 0 siblings, 1 reply; 15+ messages in thread From: Dong Aisheng-B29396 @ 2011-09-08 7:52 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm- > kernel-bounces at 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 at alsa-project.org; s.hauer at pengutronix.de; > broonie at opensource.wolfsonmicro.com; w.sang at pengutronix.de; > kernel at pengutronix.de; lrg at ti.com; linux-arm-kernel at 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 at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] ARM: mx28evk: add platform data for saif 2011-09-08 7:52 ` Dong Aisheng-B29396 @ 2011-09-09 13:41 ` Wolfram Sang 2011-09-10 14:08 ` Uwe Kleine-König 0 siblings, 1 reply; 15+ messages in thread From: Wolfram Sang @ 2011-09-09 13:41 UTC (permalink / raw) To: 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 -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110909/76649b9c/attachment.sig> ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/3] ARM: mx28evk: add platform data for saif 2011-09-09 13:41 ` Wolfram Sang @ 2011-09-10 14:08 ` Uwe Kleine-König 0 siblings, 0 replies; 15+ messages in thread From: Uwe Kleine-König @ 2011-09-10 14:08 UTC (permalink / raw) To: linux-arm-kernel 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 -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif 2011-09-07 13:57 [PATCH v2 0/3] ARM: mxs: add recording support for saif Dong Aisheng 2011-09-07 13:57 ` [PATCH v2 1/3] ARM: mx28evk: add platform data " Dong Aisheng @ 2011-09-07 13:57 ` Dong Aisheng 2011-09-07 18:21 ` Uwe Kleine-König 2011-09-07 13:57 ` [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks Dong Aisheng 2 siblings, 1 reply; 15+ messages in thread From: Dong Aisheng @ 2011-09-07 13:57 UTC (permalink / raw) To: linux-arm-kernel 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); -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif 2011-09-07 13:57 ` [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate " Dong Aisheng @ 2011-09-07 18:21 ` Uwe Kleine-König 2011-09-08 8:03 ` Dong Aisheng-B29396 0 siblings, 1 reply; 15+ messages in thread From: Uwe Kleine-König @ 2011-09-07 18:21 UTC (permalink / raw) To: linux-arm-kernel 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); -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif 2011-09-07 18:21 ` Uwe Kleine-König @ 2011-09-08 8:03 ` Dong Aisheng-B29396 2011-09-08 8:56 ` Uwe Kleine-König 0 siblings, 1 reply; 15+ messages in thread From: Dong Aisheng-B29396 @ 2011-09-08 8:03 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: Uwe Kleine-K?nig [mailto:u.kleine-koenig at pengutronix.de] > Sent: Thursday, September 08, 2011 2:22 AM > To: Dong Aisheng-B29396 > Cc: alsa-devel at alsa-project.org; s.hauer at pengutronix.de; > broonie at opensource.wolfsonmicro.com; lrg at ti.com; linux-arm- > kernel at lists.infradead.org; w.sang at 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 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. 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif 2011-09-08 8:03 ` Dong Aisheng-B29396 @ 2011-09-08 8:56 ` Uwe Kleine-König 2011-09-08 9:06 ` Dong Aisheng-B29396 0 siblings, 1 reply; 15+ messages in thread From: Uwe Kleine-König @ 2011-09-08 8:56 UTC (permalink / raw) To: linux-arm-kernel 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 at pengutronix.de] > > Sent: Thursday, September 08, 2011 2:22 AM > > To: Dong Aisheng-B29396 > > Cc: alsa-devel at alsa-project.org; s.hauer at pengutronix.de; > > broonie at opensource.wolfsonmicro.com; lrg at ti.com; linux-arm- > > kernel at lists.infradead.org; w.sang at 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 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. > 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 -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate for saif 2011-09-08 8:56 ` Uwe Kleine-König @ 2011-09-08 9:06 ` Dong Aisheng-B29396 0 siblings, 0 replies; 15+ messages in thread From: Dong Aisheng-B29396 @ 2011-09-08 9:06 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: Uwe Kleine-K?nig [mailto:u.kleine-koenig at pengutronix.de] > Sent: Thursday, September 08, 2011 4:56 PM > To: Dong Aisheng-B29396 > Cc: alsa-devel at alsa-project.org; s.hauer at pengutronix.de; > broonie at opensource.wolfsonmicro.com; lrg at ti.com; linux-arm- > kernel at lists.infradead.org; w.sang at 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 at pengutronix.de] > > > Sent: Thursday, September 08, 2011 2:22 AM > > > To: Dong Aisheng-B29396 > > > Cc: alsa-devel at alsa-project.org; s.hauer at pengutronix.de; > > > broonie at opensource.wolfsonmicro.com; lrg at ti.com; linux-arm- > > > kernel at lists.infradead.org; w.sang at 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 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. > > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks 2011-09-07 13:57 [PATCH v2 0/3] ARM: mxs: add recording support for saif Dong Aisheng 2011-09-07 13:57 ` [PATCH v2 1/3] ARM: mx28evk: add platform data " Dong Aisheng 2011-09-07 13:57 ` [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate " Dong Aisheng @ 2011-09-07 13:57 ` Dong Aisheng 2011-09-07 18:24 ` Uwe Kleine-König 2 siblings, 1 reply; 15+ messages in thread From: Dong Aisheng @ 2011-09-07 13:57 UTC (permalink / raw) To: linux-arm-kernel 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 -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks 2011-09-07 13:57 ` [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks Dong Aisheng @ 2011-09-07 18:24 ` Uwe Kleine-König 2011-09-08 8:24 ` Dong Aisheng-B29396 0 siblings, 1 reply; 15+ messages in thread From: Uwe Kleine-König @ 2011-09-07 18:24 UTC (permalink / raw) To: linux-arm-kernel 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); \ > 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) 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 -- Pengutronix e.K. | Uwe Kleine-K?nig | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks 2011-09-07 18:24 ` Uwe Kleine-König @ 2011-09-08 8:24 ` Dong Aisheng-B29396 2011-09-09 13:42 ` Wolfram Sang 0 siblings, 1 reply; 15+ messages in thread From: Dong Aisheng-B29396 @ 2011-09-08 8:24 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: Uwe Kleine-K?nig [mailto:u.kleine-koenig at pengutronix.de] > Sent: Thursday, September 08, 2011 2:25 AM > To: Dong Aisheng-B29396 > Cc: alsa-devel at alsa-project.org; s.hauer at pengutronix.de; > broonie at opensource.wolfsonmicro.com; lrg at ti.com; linux-arm- > kernel at lists.infradead.org; w.sang at 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); \ > > 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) > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks 2011-09-08 8:24 ` Dong Aisheng-B29396 @ 2011-09-09 13:42 ` Wolfram Sang 0 siblings, 0 replies; 15+ messages in thread From: Wolfram Sang @ 2011-09-09 13:42 UTC (permalink / raw) To: linux-arm-kernel > 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 -- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110909/079d2209/attachment.sig> ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-09-10 14:08 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-07 13:57 [PATCH v2 0/3] ARM: mxs: add recording support for saif Dong Aisheng 2011-09-07 13:57 ` [PATCH v2 1/3] ARM: mx28evk: add platform data " Dong Aisheng 2011-09-07 18:18 ` Uwe Kleine-König 2011-09-08 7:52 ` Dong Aisheng-B29396 2011-09-09 13:41 ` Wolfram Sang 2011-09-10 14:08 ` Uwe Kleine-König 2011-09-07 13:57 ` [PATCH v2 2/3] ARM: mx28evk: set a initial clock rate " Dong Aisheng 2011-09-07 18:21 ` Uwe Kleine-König 2011-09-08 8:03 ` Dong Aisheng-B29396 2011-09-08 8:56 ` Uwe Kleine-König 2011-09-08 9:06 ` Dong Aisheng-B29396 2011-09-07 13:57 ` [PATCH v2 3/3] arm: mxs: disable clock-gates when setting saif-clocks Dong Aisheng 2011-09-07 18:24 ` Uwe Kleine-König 2011-09-08 8:24 ` Dong Aisheng-B29396 2011-09-09 13:42 ` Wolfram Sang
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).