[alsa-devel] [PATCH v4 0/7] ARM/ASoC: OMAP3: Fix McBSP2/3 sidetone support
Hi,
Changes since v3: - Rebased on next-20160525 [1] to apply cleanly - Fixed subject line for patch5 - Added Mark's acked-by to the ASoC patches
[1] "ASoC: omap-mcbsp: Enable/disable sidetone block auto clock gating for omap3" is gone missing from linux-next - it was in next-20160517 and I'm not sure when it got dropped.
Tony: since Paul did not replied to the series, I'm sending the v4 now, I hope he will have time to look at this.
Commit message from v3:
Based on the ongoing discussion on v2 (ARM: OMAP3: Fix McBSP2/3 hwmod setup for sidetone) I have dropped the removal of the sidetone hwmod and only corrected it. The series now includes all related changes needed to have correct sidetone support whenever we boot in legacy or in DT mode.
This time the ASoC part have dependency on earlier patches so they can not be applied separately.
Regards, Peter --- Peter Ujfalusi (7): ARM: OMAP3: hwmod data: Fix McBSP2/3 sidetone data ARM: dts: omap3: Add clocks to McBSP nodes ARM: OMAP3: McBSP: New callback for McBSP2/3 ICLK idle configuration ARM: OMAP3: pdata-quirks: Add support for McBSP2/3 sidetone handling ASoC: omap-mcbsp: Rename omap_mcbsp_sysfs_remove() to omap_mcbsp_cleanup() ASoC: omap-mcbsp: sidetone: Use the new callback for iclk handling ARM: OMAP2+: McBSP: Remove the old iclk allow/deny idle code
arch/arm/boot/dts/omap3.dtsi | 10 ++++++++++ arch/arm/mach-omap2/mcbsp.c | 31 ++++++++++++++++------------- arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 24 ++++------------------ arch/arm/mach-omap2/pdata-quirks.c | 18 +++++++++++++++++ include/linux/platform_data/asoc-ti-mcbsp.h | 4 +++- sound/soc/omap/mcbsp.c | 21 +++++++++++++------ sound/soc/omap/mcbsp.h | 3 ++- sound/soc/omap/omap-mcbsp.c | 5 ++++- 8 files changed, 73 insertions(+), 43 deletions(-)
The McBSPLP's sidetone main clock is the McBSPLP's ICLK, not FCLK as the sidetone only receives the ICLK from the main McBSP module. Since the McBSP and sidetone is using the very same clock from PRCM level the sidetone must not have the prcm section to check the clock status since the sidetone is only used when McBSP is already configured. If two separate hwmods looking at the same bit and they would use pm_runtime in nested way (as it must happen with McBSP and it's ST module) the hwmod would warn, because the idlest will not match what it is expected after enable/disable of the clocks.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c index 9869a75c5d96..d72ee6185d5e 100644 --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c @@ -1322,16 +1322,8 @@ static struct omap_hwmod omap3xxx_mcbsp2_sidetone_hwmod = { .name = "mcbsp2_sidetone", .class = &omap3xxx_mcbsp_sidetone_hwmod_class, .mpu_irqs = omap3xxx_mcbsp2_sidetone_irqs, - .main_clk = "mcbsp2_fck", - .prcm = { - .omap2 = { - .prcm_reg_id = 1, - .module_bit = OMAP3430_EN_MCBSP2_SHIFT, - .module_offs = OMAP3430_PER_MOD, - .idlest_reg_id = 1, - .idlest_idle_bit = OMAP3430_ST_MCBSP2_SHIFT, - }, - }, + .main_clk = "mcbsp2_ick", + .flags = HWMOD_NO_IDLEST, };
/* mcbsp3_sidetone */ @@ -1344,16 +1336,8 @@ static struct omap_hwmod omap3xxx_mcbsp3_sidetone_hwmod = { .name = "mcbsp3_sidetone", .class = &omap3xxx_mcbsp_sidetone_hwmod_class, .mpu_irqs = omap3xxx_mcbsp3_sidetone_irqs, - .main_clk = "mcbsp3_fck", - .prcm = { - .omap2 = { - .prcm_reg_id = 1, - .module_bit = OMAP3430_EN_MCBSP3_SHIFT, - .module_offs = OMAP3430_PER_MOD, - .idlest_reg_id = 1, - .idlest_idle_bit = OMAP3430_ST_MCBSP3_SHIFT, - }, - }, + .main_clk = "mcbsp3_ick", + .flags = HWMOD_NO_IDLEST, };
/* SR common */
Add clock properties to the McBSP nodes. McBSP2 and 3 need to have ick also since the Sidetone block of these modules are operating using the McBSP interface clock.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/boot/dts/omap3.dtsi | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi index 9fbda38528dc..4c3c471d2a83 100644 --- a/arch/arm/boot/dts/omap3.dtsi +++ b/arch/arm/boot/dts/omap3.dtsi @@ -493,6 +493,8 @@ dmas = <&sdma 31>, <&sdma 32>; dma-names = "tx", "rx"; + clocks = <&mcbsp1_fck>; + clock-names = "fck"; status = "disabled"; };
@@ -511,6 +513,8 @@ dmas = <&sdma 33>, <&sdma 34>; dma-names = "tx", "rx"; + clocks = <&mcbsp2_fck>, <&mcbsp2_ick>; + clock-names = "fck", "ick"; status = "disabled"; };
@@ -529,6 +533,8 @@ dmas = <&sdma 17>, <&sdma 18>; dma-names = "tx", "rx"; + clocks = <&mcbsp3_fck>, <&mcbsp3_ick>; + clock-names = "fck", "ick"; status = "disabled"; };
@@ -545,6 +551,8 @@ dmas = <&sdma 19>, <&sdma 20>; dma-names = "tx", "rx"; + clocks = <&mcbsp4_fck>; + clock-names = "fck"; status = "disabled"; };
@@ -561,6 +569,8 @@ dmas = <&sdma 21>, <&sdma 22>; dma-names = "tx", "rx"; + clocks = <&mcbsp5_fck>; + clock-names = "fck"; status = "disabled"; };
* Peter Ujfalusi peter.ujfalusi@ti.com [160525 07:24]:
Add clock properties to the McBSP nodes. McBSP2 and 3 need to have ick also since the Sidetone block of these modules are operating using the McBSP interface clock.
Sorry replied to the earlier thread putting this patch as the first patch when committing against v4.17-rc1.
Regards,
Tony
McBSP2/3 module's sidetone module operates using the module's ICLK clock. When the Sidetone is in use the interface clock of the module must not idle. The new callback expects to receive the *clk of the module's ick and not the id number of the McBSP. This will allow us more cleanups and going to simplify the ICLK handling.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/mcbsp.c | 12 ++++++++++++ include/linux/platform_data/asoc-ti-mcbsp.h | 1 + 2 files changed, 13 insertions(+)
diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c index b4ac3af1160c..959cb4cb1062 100644 --- a/arch/arm/mach-omap2/mcbsp.c +++ b/arch/arm/mach-omap2/mcbsp.c @@ -48,6 +48,17 @@ static int omap3_enable_st_clock(unsigned int id, bool enable) return omap2_clk_allow_idle(mcbsp_iclks[id]); }
+static int omap3_mcbsp_force_ick_on(struct clk *clk, bool force_on) +{ + if (!clk) + return 0; + + if (force_on) + return omap2_clk_deny_idle(clk); + else + return omap2_clk_allow_idle(clk); +} + static int __init omap_init_mcbsp(struct omap_hwmod *oh, void *unused) { int id, count = 1; @@ -97,6 +108,7 @@ static int __init omap_init_mcbsp(struct omap_hwmod *oh, void *unused) oh_device[1] = omap_hwmod_lookup(( (struct omap_mcbsp_dev_attr *)(oh->dev_attr))->sidetone); pdata->enable_st_clock = omap3_enable_st_clock; + pdata->force_ick_on = omap3_mcbsp_force_ick_on; sprintf(clk_name, "mcbsp%d_ick", id); mcbsp_iclks[id] = clk_get(NULL, clk_name); count++; diff --git a/include/linux/platform_data/asoc-ti-mcbsp.h b/include/linux/platform_data/asoc-ti-mcbsp.h index 3c73c045f8da..73e5e832fa23 100644 --- a/include/linux/platform_data/asoc-ti-mcbsp.h +++ b/include/linux/platform_data/asoc-ti-mcbsp.h @@ -45,6 +45,7 @@ struct omap_mcbsp_platform_data { bool has_wakeup; /* Wakeup capability */ bool has_ccr; /* Transceiver has configuration control registers */ int (*enable_st_clock)(unsigned int, bool); + int (*force_ick_on)(struct clk *clk, bool force_on); };
/**
McBSP2/3 module's sidetone module operates using the module's ICLK clock. When the Sidetone is in use the interface clock of the module must not idle. To prevent the iclk idling the driver expects to have pdata callback to call. With this patch the callback is going to be set up for DT boot also.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/mcbsp.c | 9 +++++++++ arch/arm/mach-omap2/pdata-quirks.c | 18 ++++++++++++++++++ include/linux/platform_data/asoc-ti-mcbsp.h | 2 ++ 3 files changed, 29 insertions(+)
diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c index 959cb4cb1062..edf4f41c0135 100644 --- a/arch/arm/mach-omap2/mcbsp.c +++ b/arch/arm/mach-omap2/mcbsp.c @@ -59,6 +59,15 @@ static int omap3_mcbsp_force_ick_on(struct clk *clk, bool force_on) return omap2_clk_allow_idle(clk); }
+void __init omap3_mcbsp_init_pdata_callback( + struct omap_mcbsp_platform_data *pdata) +{ + if (!pdata) + return; + + pdata->force_ick_on = omap3_mcbsp_force_ick_on; +} + static int __init omap_init_mcbsp(struct omap_hwmod *oh, void *unused) { int id, count = 1; diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c index 6571ad959908..ab2b2b2b90e5 100644 --- a/arch/arm/mach-omap2/pdata-quirks.c +++ b/arch/arm/mach-omap2/pdata-quirks.c @@ -26,6 +26,7 @@ #include <linux/platform_data/wkup_m3.h> #include <linux/platform_data/pwm_omap_dmtimer.h> #include <linux/platform_data/media/ir-rx51.h> +#include <linux/platform_data/asoc-ti-mcbsp.h> #include <plat/dmtimer.h>
#include "common.h" @@ -505,6 +506,16 @@ static struct platform_device __maybe_unused rx51_lirc_device = { }, };
+#if IS_ENABLED(CONFIG_SND_OMAP_SOC_MCBSP) +static struct omap_mcbsp_platform_data mcbsp_pdata; +static void __init omap3_mcbsp_init(void) +{ + omap3_mcbsp_init_pdata_callback(&mcbsp_pdata); +} +#else +static void __init omap3_mcbsp_init(void) {} +#endif + /* * Few boards still need auxdata populated before we populate * the dev entries in of_platform_populate(). @@ -536,6 +547,11 @@ static struct of_dev_auxdata omap_auxdata_lookup[] __initdata = { OF_DEV_AUXDATA("ti,davinci_mdio", 0x5c030000, "davinci_mdio.0", NULL), OF_DEV_AUXDATA("ti,am3517-emac", 0x5c000000, "davinci_emac.0", &am35xx_emac_pdata), + /* McBSP modules with sidetone core */ +#if IS_ENABLED(CONFIG_SND_OMAP_SOC_MCBSP) + OF_DEV_AUXDATA("ti,omap3-mcbsp", 0x49022000, "49022000.mcbsp", &mcbsp_pdata), + OF_DEV_AUXDATA("ti,omap3-mcbsp", 0x49024000, "49024000.mcbsp", &mcbsp_pdata), +#endif #endif #ifdef CONFIG_SOC_AM33XX OF_DEV_AUXDATA("ti,am3352-wkup-m3", 0x44d00000, "44d00000.wkup_m3", @@ -608,6 +624,8 @@ void __init pdata_quirks_init(const struct of_device_id *omap_dt_match_table) of_machine_is_compatible("ti,omap3")) omap_sdrc_init(NULL, NULL);
+ if (of_machine_is_compatible("ti,omap3")) + omap3_mcbsp_init(); pdata_quirks_check(auxdata_quirks); of_platform_populate(NULL, omap_dt_match_table, omap_auxdata_lookup, NULL); diff --git a/include/linux/platform_data/asoc-ti-mcbsp.h b/include/linux/platform_data/asoc-ti-mcbsp.h index 73e5e832fa23..5530971abf4d 100644 --- a/include/linux/platform_data/asoc-ti-mcbsp.h +++ b/include/linux/platform_data/asoc-ti-mcbsp.h @@ -56,4 +56,6 @@ struct omap_mcbsp_dev_attr { const char *sidetone; };
+void omap3_mcbsp_init_pdata_callback(struct omap_mcbsp_platform_data *pdata); + #endif
The function will do more then removing the sysfs files in the future.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Acked-by: Mark Brown broonie@kernel.org --- sound/soc/omap/mcbsp.c | 2 +- sound/soc/omap/mcbsp.h | 2 +- sound/soc/omap/omap-mcbsp.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c index c7563e230c7d..75cd1761d383 100644 --- a/sound/soc/omap/mcbsp.c +++ b/sound/soc/omap/mcbsp.c @@ -1080,7 +1080,7 @@ err_thres: return ret; }
-void omap_mcbsp_sysfs_remove(struct omap_mcbsp *mcbsp) +void omap_mcbsp_cleanup(struct omap_mcbsp *mcbsp) { if (mcbsp->pdata->buffer_size) sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group); diff --git a/sound/soc/omap/mcbsp.h b/sound/soc/omap/mcbsp.h index 96d1b086bcf8..ce6cbbf923a4 100644 --- a/sound/soc/omap/mcbsp.h +++ b/sound/soc/omap/mcbsp.h @@ -349,6 +349,6 @@ int omap_st_disable(struct omap_mcbsp *mcbsp); int omap_st_is_enabled(struct omap_mcbsp *mcbsp);
int omap_mcbsp_init(struct platform_device *pdev); -void omap_mcbsp_sysfs_remove(struct omap_mcbsp *mcbsp); +void omap_mcbsp_cleanup(struct omap_mcbsp *mcbsp);
#endif /* __ASOC_MCBSP_H */ diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index fd99d89de6a8..db07debb4a9c 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -832,7 +832,7 @@ static int asoc_mcbsp_remove(struct platform_device *pdev) if (mcbsp->pdata->ops && mcbsp->pdata->ops->free) mcbsp->pdata->ops->free(mcbsp->id);
- omap_mcbsp_sysfs_remove(mcbsp); + omap_mcbsp_cleanup(mcbsp);
clk_put(mcbsp->fclk);
The McBSP sidetone (in OMAP3 McBSP2 and 3 module) is working with the module's interface clock. When the sidetone is enabled the iclk must not idle because it will result in choppy sidetone. Switch to use the new callback for handling the iclk allow/deny idle configuration. For this the driver needs to get the module's ick clock and pass the clk pointer to the callback. In DT boot, the pdata-quirk is going to set up the callback for the driver so save it if it is set in the pdata of the device.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com Acked-by: Mark Brown broonie@kernel.org --- sound/soc/omap/mcbsp.c | 19 ++++++++++++++----- sound/soc/omap/mcbsp.h | 1 + sound/soc/omap/omap-mcbsp.c | 3 +++ 3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c index 75cd1761d383..5c3fdfe4c6b9 100644 --- a/sound/soc/omap/mcbsp.c +++ b/sound/soc/omap/mcbsp.c @@ -257,8 +257,8 @@ static void omap_st_on(struct omap_mcbsp *mcbsp) { unsigned int w;
- if (mcbsp->pdata->enable_st_clock) - mcbsp->pdata->enable_st_clock(mcbsp->id, 1); + if (mcbsp->pdata->force_ick_on) + mcbsp->pdata->force_ick_on(mcbsp->st_data->mcbsp_iclk, true);
/* Enable McBSP Sidetone */ w = MCBSP_READ(mcbsp, SSELCR); @@ -279,8 +279,8 @@ static void omap_st_off(struct omap_mcbsp *mcbsp) w = MCBSP_READ(mcbsp, SSELCR); MCBSP_WRITE(mcbsp, SSELCR, w & ~(SIDETONEEN));
- if (mcbsp->pdata->enable_st_clock) - mcbsp->pdata->enable_st_clock(mcbsp->id, 0); + if (mcbsp->pdata->force_ick_on) + mcbsp->pdata->force_ick_on(mcbsp->st_data->mcbsp_iclk, false); }
static void omap_st_fir_write(struct omap_mcbsp *mcbsp, s16 *fir) @@ -938,6 +938,13 @@ static int omap_st_add(struct omap_mcbsp *mcbsp, struct resource *res) if (!st_data) return -ENOMEM;
+ st_data->mcbsp_iclk = clk_get(mcbsp->dev, "ick"); + if (IS_ERR(st_data->mcbsp_iclk)) { + dev_warn(mcbsp->dev, + "Failed to get ick, sidetone might be broken\n"); + st_data->mcbsp_iclk = NULL; + } + st_data->io_base_st = devm_ioremap(mcbsp->dev, res->start, resource_size(res)); if (!st_data->io_base_st) @@ -1085,6 +1092,8 @@ void omap_mcbsp_cleanup(struct omap_mcbsp *mcbsp) if (mcbsp->pdata->buffer_size) sysfs_remove_group(&mcbsp->dev->kobj, &additional_attr_group);
- if (mcbsp->st_data) + if (mcbsp->st_data) { sysfs_remove_group(&mcbsp->dev->kobj, &sidetone_attr_group); + clk_put(mcbsp->st_data->mcbsp_iclk); + } } diff --git a/sound/soc/omap/mcbsp.h b/sound/soc/omap/mcbsp.h index ce6cbbf923a4..61e93b1c185d 100644 --- a/sound/soc/omap/mcbsp.h +++ b/sound/soc/omap/mcbsp.h @@ -280,6 +280,7 @@ struct omap_mcbsp_reg_cfg {
struct omap_mcbsp_st_data { void __iomem *io_base_st; + struct clk *mcbsp_iclk; bool running; bool enabled; s16 taps[128]; /* Sidetone filter coefficients */ diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index db07debb4a9c..d018e966e533 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -788,6 +788,7 @@ static int asoc_mcbsp_probe(struct platform_device *pdev) match = of_match_device(omap_mcbsp_of_match, &pdev->dev); if (match) { struct device_node *node = pdev->dev.of_node; + struct omap_mcbsp_platform_data *pdata_quirk = pdata; int buffer_size;
pdata = devm_kzalloc(&pdev->dev, @@ -799,6 +800,8 @@ static int asoc_mcbsp_probe(struct platform_device *pdev) memcpy(pdata, match->data, sizeof(*pdata)); if (!of_property_read_u32(node, "ti,buffer-size", &buffer_size)) pdata->buffer_size = buffer_size; + if (pdata_quirk) + pdata->force_ick_on = pdata_quirk->force_ick_on; } else if (!pdata) { dev_err(&pdev->dev, "missing platform data.\n"); return -EINVAL;
The new pdata callback (force_ick_on) is now used by the driver and the old callback related code can be removed.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/mcbsp.c | 18 ------------------ include/linux/platform_data/asoc-ti-mcbsp.h | 1 - 2 files changed, 19 deletions(-)
diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c index edf4f41c0135..fc04be74e064 100644 --- a/arch/arm/mach-omap2/mcbsp.c +++ b/arch/arm/mach-omap2/mcbsp.c @@ -34,20 +34,6 @@ #include "cm3xxx.h" #include "cm-regbits-34xx.h"
-static struct clk *mcbsp_iclks[5]; - -static int omap3_enable_st_clock(unsigned int id, bool enable) -{ - /* - * Sidetone uses McBSP ICLK - which must not idle when sidetones - * are enabled or sidetones start sounding ugly. - */ - if (enable) - return omap2_clk_deny_idle(mcbsp_iclks[id]); - else - return omap2_clk_allow_idle(mcbsp_iclks[id]); -} - static int omap3_mcbsp_force_ick_on(struct clk *clk, bool force_on) { if (!clk) @@ -75,7 +61,6 @@ static int __init omap_init_mcbsp(struct omap_hwmod *oh, void *unused) struct omap_hwmod *oh_device[2]; struct omap_mcbsp_platform_data *pdata = NULL; struct platform_device *pdev; - char clk_name[11];
sscanf(oh->name, "mcbsp%d", &id);
@@ -116,10 +101,7 @@ static int __init omap_init_mcbsp(struct omap_hwmod *oh, void *unused) if (oh->dev_attr) { oh_device[1] = omap_hwmod_lookup(( (struct omap_mcbsp_dev_attr *)(oh->dev_attr))->sidetone); - pdata->enable_st_clock = omap3_enable_st_clock; pdata->force_ick_on = omap3_mcbsp_force_ick_on; - sprintf(clk_name, "mcbsp%d_ick", id); - mcbsp_iclks[id] = clk_get(NULL, clk_name); count++; } pdev = omap_device_build_ss(name, id, oh_device, count, pdata, diff --git a/include/linux/platform_data/asoc-ti-mcbsp.h b/include/linux/platform_data/asoc-ti-mcbsp.h index 5530971abf4d..e684543254f3 100644 --- a/include/linux/platform_data/asoc-ti-mcbsp.h +++ b/include/linux/platform_data/asoc-ti-mcbsp.h @@ -44,7 +44,6 @@ struct omap_mcbsp_platform_data { /* McBSP platform and instance specific features */ bool has_wakeup; /* Wakeup capability */ bool has_ccr; /* Transceiver has configuration control registers */ - int (*enable_st_clock)(unsigned int, bool); int (*force_ick_on)(struct clk *clk, bool force_on); };
participants (2)
-
Peter Ujfalusi -
Tony Lindgren