[RFC 0/12] McBSP cleanup and generalization

[RFC 0/12] McBSP cleanup and generalization

[Jarkko Nikula]

Hi

Some cleanups to McBSP code and attempts to make it more generic. I'll be out
of office in coming days but I'm happy to hear how does this set work on
OMAP1 Amstrad Delta as there are changes around register definitions and
access. I've tested this on 2420 and 3430 and build tested with omap1_defconfig
and omap2plus_defconfig.

I added Paul's build fix first as it seems to got lost in l-o master during
recent devel branch merges and my patches are made on top of it. Paul's
patch can be found from commit 7150dfee80a1e7ed5064730994b704495a8c6b99.

For is_omap removal I didn't invent any other new than reg_size and reg_step
to platform data but in the future I think mcbsp_config_type is better to
replace with some feature flags that come from hwmod so that there is no
need to modify code for any new omap version.

Long term plan is still bit open. If we go to merge McBSP with ASoC omap-mcbsp
we can drop quite much of special API here as well as omap-mcbsp-dai
platform devices.

-- 
Jarkko

[RFC 10/12] omap: mcbsp: Move sidetone clock management to mach-omap2/mcbsp.c

[Paul Walmsley]

+ Beno?t

Hello Jarkko

On Fri, 1 Jul 2011, Jarkko Nikula wrote:

> Active sidetone requires that McBSP interface clock doesn't idle and there
> is no mechanism in hwmod to turn autoidling on/off in runtime. McBSP2 and 3
> in OMAP34xx share their interface clock with McBSP sidetone module and
> that interface clock must be active when the sidetone is operating.
> 
> Sidetone has its own autoidle bit which should keep the interface clock
> active but it is broken. Putting the McBSP core to no-idle mode when the
> sidetone is active is no good either since it results to higher power
> consumption when using the threshold based DMA transfers.

In the hwmod code/data, we've got the OCPIF_SWSUP_IDLE flag that can be 
set on a struct omap_hwmod_ocp_if.  I think this is probably what's needed 
here.  The only problem is that we haven't linked that to the clock code 
to deny idle on the interface clock yet (see omap_hwmod.c:_setup()).  
Adding that code in, plus adding that OCPIF_SWSUP_IDLE flag to the 
McBSP2/3 data, seems like the right approach here.

I guess we also will need some basic usecounting for denying idle in the 
clock code.

Otherwise these direct register manipulations of clock registers, outside 
the clock code, could turn into a mess :-(

At least it makes sense to move those out of the driver and into the 
arch/arm/*omap* code...


- Paul

[RFC 10/12] omap: mcbsp: Move sidetone clock management to mach-omap2/mcbsp.c

[Cousson, Benoit]

On 7/1/2011 11:23 AM, Paul Walmsley wrote:
> + Beno?t
>
> Hello Jarkko
>
> On Fri, 1 Jul 2011, Jarkko Nikula wrote:
>
>> Active sidetone requires that McBSP interface clock doesn't idle and there
>> is no mechanism in hwmod to turn autoidling on/off in runtime. McBSP2 and 3
>> in OMAP34xx share their interface clock with McBSP sidetone module and
>> that interface clock must be active when the sidetone is operating.
>>
>> Sidetone has its own autoidle bit which should keep the interface clock
>> active but it is broken. Putting the McBSP core to no-idle mode when the
>> sidetone is active is no good either since it results to higher power
>> consumption when using the threshold based DMA transfers.
>
> In the hwmod code/data, we've got the OCPIF_SWSUP_IDLE flag that can be
> set on a struct omap_hwmod_ocp_if.  I think this is probably what's needed
> here.  The only problem is that we haven't linked that to the clock code
> to deny idle on the interface clock yet (see omap_hwmod.c:_setup()).
> Adding that code in, plus adding that OCPIF_SWSUP_IDLE flag to the
> McBSP2/3 data, seems like the right approach here.
>
> I guess we also will need some basic usecounting for denying idle in the
> clock code.
>
> Otherwise these direct register manipulations of clock registers, outside
> the clock code, could turn into a mess :-(

AFAIR Kishon did submit some patches to expose this feature to the 
driver through omap_device API. The point is that other broken IP like 
SDMA of USB will require such feature.

Didn't we pull them?

Benoit

[RFC 10/12] omap: mcbsp: Move sidetone clock management to mach-omap2/mcbsp.c

[Paul Walmsley]

cc'ing Kishon 

On Fri, 1 Jul 2011, Cousson, Benoit wrote:

> On 7/1/2011 11:23 AM, Paul Walmsley wrote:
> > On Fri, 1 Jul 2011, Jarkko Nikula wrote:
> > 
> > > Active sidetone requires that McBSP interface clock doesn't idle and there
> > > is no mechanism in hwmod to turn autoidling on/off in runtime. McBSP2 and
> > > 3
> > > in OMAP34xx share their interface clock with McBSP sidetone module and
> > > that interface clock must be active when the sidetone is operating.
> > > 
> > > Sidetone has its own autoidle bit which should keep the interface clock
> > > active but it is broken. Putting the McBSP core to no-idle mode when the
> > > sidetone is active is no good either since it results to higher power
> > > consumption when using the threshold based DMA transfers.
> > 
> > In the hwmod code/data, we've got the OCPIF_SWSUP_IDLE flag that can be
> > set on a struct omap_hwmod_ocp_if.  I think this is probably what's needed
> > here.  The only problem is that we haven't linked that to the clock code
> > to deny idle on the interface clock yet (see omap_hwmod.c:_setup()).
> > Adding that code in, plus adding that OCPIF_SWSUP_IDLE flag to the
> > McBSP2/3 data, seems like the right approach here.
> > 
> > I guess we also will need some basic usecounting for denying idle in the
> > clock code.
> > 
> > Otherwise these direct register manipulations of clock registers, outside
> > the clock code, could turn into a mess :-(
> 
> AFAIR Kishon did submit some patches to expose this feature to the driver
> through omap_device API. The point is that other broken IP like SDMA of USB
> will require such feature.
> 
> Didn't we pull them?

You sent him some comments on March 1 but it looks like the series never 
got updated and reposted, at least not that I can find in my mail 
archive.  Kishon?

Anyway, those patches won't help in this case if the sidetone AUTOIDLE bit 
is broken - looks like the interface clock autoidle bit is what needs to 
change.


- Paul

[RFC 10/12] omap: mcbsp: Move sidetone clock management to mach-omap2/mcbsp.c

[ABRAHAM, KISHON VIJAY]

On Fri, Jul 1, 2011 at 3:04 PM, Paul Walmsley <paul@pwsan.com> wrote:
> cc'ing Kishon
>
> On Fri, 1 Jul 2011, Cousson, Benoit wrote:
>
>> On 7/1/2011 11:23 AM, Paul Walmsley wrote:
>> > On Fri, 1 Jul 2011, Jarkko Nikula wrote:
>> >
>> > > Active sidetone requires that McBSP interface clock doesn't idle and there
>> > > is no mechanism in hwmod to turn autoidling on/off in runtime. McBSP2 and
>> > > 3
>> > > in OMAP34xx share their interface clock with McBSP sidetone module and
>> > > that interface clock must be active when the sidetone is operating.
>> > >
>> > > Sidetone has its own autoidle bit which should keep the interface clock
>> > > active but it is broken. Putting the McBSP core to no-idle mode when the
>> > > sidetone is active is no good either since it results to higher power
>> > > consumption when using the threshold based DMA transfers.
>> >
>> > In the hwmod code/data, we've got the OCPIF_SWSUP_IDLE flag that can be
>> > set on a struct omap_hwmod_ocp_if. ?I think this is probably what's needed
>> > here. ?The only problem is that we haven't linked that to the clock code
>> > to deny idle on the interface clock yet (see omap_hwmod.c:_setup()).
>> > Adding that code in, plus adding that OCPIF_SWSUP_IDLE flag to the
>> > McBSP2/3 data, seems like the right approach here.
>> >
>> > I guess we also will need some basic usecounting for denying idle in the
>> > clock code.
>> >
>> > Otherwise these direct register manipulations of clock registers, outside
>> > the clock code, could turn into a mess :-(
>>
>> AFAIR Kishon did submit some patches to expose this feature to the driver
>> through omap_device API. The point is that other broken IP like SDMA of USB
>> will require such feature.
>>
>> Didn't we pull them?

I'll repost by early next week.

>
> You sent him some comments on March 1 but it looks like the series never
> got updated and reposted, at least not that I can find in my mail
> archive. ?Kishon?
>
> Anyway, those patches won't help in this case if the sidetone AUTOIDLE bit
> is broken - looks like the interface clock autoidle bit is what needs to
> change.
>
>
> - Paul
>

[RFC 10/12] omap: mcbsp: Move sidetone clock management to mach-omap2/mcbsp.c

[Jarkko Nikula]

On Fri, 1 Jul 2011 03:23:49 -0600 (MDT)
Paul Walmsley <paul@pwsan.com> wrote:

> In the hwmod code/data, we've got the OCPIF_SWSUP_IDLE flag that can be 
> set on a struct omap_hwmod_ocp_if.  I think this is probably what's needed 
> here.  The only problem is that we haven't linked that to the clock code 
> to deny idle on the interface clock yet (see omap_hwmod.c:_setup()).  
> Adding that code in, plus adding that OCPIF_SWSUP_IDLE flag to the 
> McBSP2/3 data, seems like the right approach here.
> 
I understood from the code that OCPIF_SWSUP_IDLE with
clkops_omap2_dflt_wait in clock data means that then ICLK won't
autoidle at all when clock is enabled? Normally, when not using the
sidetone we want to have ICLK autoidling since then omap can idle when
using threshold based transfers and McBSP FIFO.

> Otherwise these direct register manipulations of clock registers, outside 
> the clock code, could turn into a mess :-(
> 
I definitely agree. I was thinking some virtual clock for sidetone but
that didn't sound good either since then both McBSP ICLK and virtual
sidetone clock would be modifying the same register. Anyway, some omap
device API for runtime autoidle bit setting sounds much better approach
in enable_st_clock callback than hacking with clock registers.

-- 
Jarkko

[RFC 11/12] omap: mcbsp: Remove conditional compilation for OMAP3

[Varadarajan, Charulatha]

Jarkko,

On Fri, Jul 1, 2011 at 14:22, Jarkko Nikula <jhnikula@gmail.com> wrote:
> Although this increases the build size a little for !OMAP3 builds it makes
> the code one step more generic. For instance the threshold based DMA
> transfers seems to be possible with OMAP4 too from quick look of the TRM.
>
> Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
> ---
> ?arch/arm/plat-omap/include/plat/mcbsp.h | ? 35 ++++--------------------------
> ?arch/arm/plat-omap/mcbsp.c ? ? ? ? ? ? ?| ? 13 -----------
> ?2 files changed, 5 insertions(+), 43 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
> index 2a7e8d5..720461c 100644
> --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> @@ -26,9 +26,6 @@
>
> ?#include <linux/spinlock.h>
>
> -#include <mach/hardware.h>
> -#include <plat/clock.h>
> -
> ?/* macro for building platform_device for McBSP ports */
> ?#define OMAP_MCBSP_PLATFORM_DEVICE(port_nr) ? ? ? ? ? ?\
> ?static struct platform_device omap_mcbsp##port_nr = { ?\
> @@ -314,11 +311,11 @@ struct omap_mcbsp_platform_data {
> ? ? ? ?u8 dma_rx_sync, dma_tx_sync;
> ? ? ? ?u16 rx_irq, tx_irq;
> ? ? ? ?struct omap_mcbsp_ops *ops;
> -#ifdef CONFIG_ARCH_OMAP3
> +
> ? ? ? ?/* Sidetone block for McBSP 2 and 3 */
> ? ? ? ?unsigned long phys_base_st;
> ? ? ? ?int (*enable_st_clock)(unsigned int, bool);
> -#endif
> +
> ? ? ? ?u16 buffer_size;
> ? ? ? ?unsigned int mcbsp_config_type;
> ? ? ? ?u8 reg_size;
> @@ -354,12 +351,12 @@ struct omap_mcbsp {
> ? ? ? ?spinlock_t lock;
> ? ? ? ?struct omap_mcbsp_platform_data *pdata;
> ? ? ? ?struct clk *fclk;
> -#ifdef CONFIG_ARCH_OMAP3
> +
> ? ? ? ?struct omap_mcbsp_st_data *st_data;
> ? ? ? ?int dma_op_mode;
> ? ? ? ?u16 max_tx_thres;
> ? ? ? ?u16 max_rx_thres;
> -#endif
> +
> ? ? ? ?void *reg_cache;
> ? ? ? ?unsigned int mcbsp_config_type;
> ?};
> @@ -380,7 +377,6 @@ extern int omap_mcbsp_count, omap_mcbsp_cache_size;
>
> ?int omap_mcbsp_init(void);
> ?void omap_mcbsp_config(unsigned int id, const struct omap_mcbsp_reg_cfg * config);
> -#ifdef CONFIG_ARCH_OMAP3
> ?void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold);
> ?void omap_mcbsp_set_rx_threshold(unsigned int id, u16 threshold);
> ?u16 omap_mcbsp_get_max_tx_threshold(unsigned int id);
> @@ -389,18 +385,7 @@ u16 omap_mcbsp_get_fifo_size(unsigned int id);
> ?u16 omap_mcbsp_get_tx_delay(unsigned int id);
> ?u16 omap_mcbsp_get_rx_delay(unsigned int id);
> ?int omap_mcbsp_get_dma_op_mode(unsigned int id);
> -#else
> -static inline void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
> -{ }
> -static inline void omap_mcbsp_set_rx_threshold(unsigned int id, u16 threshold)
> -{ }
> -static inline u16 omap_mcbsp_get_max_tx_threshold(unsigned int id) { return 0; }
> -static inline u16 omap_mcbsp_get_max_rx_threshold(unsigned int id) { return 0; }
> -static inline u16 omap_mcbsp_get_fifo_size(unsigned int id) { return 0; }
> -static inline u16 omap_mcbsp_get_tx_delay(unsigned int id) { return 0; }
> -static inline u16 omap_mcbsp_get_rx_delay(unsigned int id) { return 0; }
> -static inline int omap_mcbsp_get_dma_op_mode(unsigned int id) { return 0; }
> -#endif
> +
> ?int omap_mcbsp_request(unsigned int id);
> ?void omap_mcbsp_free(unsigned int id);
> ?void omap_mcbsp_start(unsigned int id, int tx, int rx);
> @@ -416,21 +401,11 @@ void omap2_mcbsp1_mux_fsr_src(u8 mux);
> ?int omap_mcbsp_dma_ch_params(unsigned int id, unsigned int stream);
> ?int omap_mcbsp_dma_reg_params(unsigned int id, unsigned int stream);
>
> -#ifdef CONFIG_ARCH_OMAP3
> ?/* Sidetone specific API */
> ?int omap_st_set_chgain(unsigned int id, int channel, s16 chgain);
> ?int omap_st_get_chgain(unsigned int id, int channel, s16 *chgain);
> ?int omap_st_enable(unsigned int id);
> ?int omap_st_disable(unsigned int id);
> ?int omap_st_is_enabled(unsigned int id);
> -#else
> -static inline int omap_st_set_chgain(unsigned int id, int channel,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?s16 chgain) { return 0; }
> -static inline int omap_st_get_chgain(unsigned int id, int channel,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?s16 *chgain) { return 0; }
> -static inline int omap_st_enable(unsigned int id) { return 0; }
> -static inline int omap_st_disable(unsigned int id) { return 0; }
> -static inline int omap_st_is_enabled(unsigned int id) { ?return 0; }
> -#endif
>
> ?#endif
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index 1a7cfb3..d565b36 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -63,7 +63,6 @@ static int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
> ? ? ? ?}
> ?}
>
> -#ifdef CONFIG_ARCH_OMAP3

Are there any plans to move the functions  omap_mcbsp_st_*
to mach-omap2/* as they are specific to OMAP3 only?

-V Charulatha

> ?static void omap_mcbsp_st_write(struct omap_mcbsp *mcbsp, u16 reg, u32 val)
> ?{
> ? ? ? ?__raw_writel(val, mcbsp->st_data->io_base_st + reg);
> @@ -73,7 +72,6 @@ static int omap_mcbsp_st_read(struct omap_mcbsp *mcbsp, u16 reg)
> ?{
> ? ? ? ?return __raw_readl(mcbsp->st_data->io_base_st + reg);
> ?}
> -#endif
>
> ?#define MCBSP_READ(mcbsp, reg) \
> ? ? ? ? ? ? ? ?omap_mcbsp_read(mcbsp, OMAP_MCBSP_REG_##reg, 0)
> @@ -258,7 +256,6 @@ int omap_mcbsp_dma_reg_params(unsigned int id, unsigned int stream)
> ?}
> ?EXPORT_SYMBOL(omap_mcbsp_dma_reg_params);
>
> -#ifdef CONFIG_ARCH_OMAP3
> ?static void omap_st_on(struct omap_mcbsp *mcbsp)
> ?{
> ? ? ? ?unsigned int w;
> @@ -657,11 +654,6 @@ int omap_mcbsp_get_dma_op_mode(unsigned int id)
> ?}
> ?EXPORT_SYMBOL(omap_mcbsp_get_dma_op_mode);
>
> -#else
> -static inline void omap_st_start(struct omap_mcbsp *mcbsp) {}
> -static inline void omap_st_stop(struct omap_mcbsp *mcbsp) {}
> -#endif
> -
> ?int omap_mcbsp_request(unsigned int id)
> ?{
> ? ? ? ?struct omap_mcbsp *mcbsp;
> @@ -935,7 +927,6 @@ void omap2_mcbsp1_mux_fsr_src(u8 mux)
> ?}
> ?#endif
>
> -#ifdef CONFIG_ARCH_OMAP3
> ?#define max_thres(m) ? ? ? ? ? ? ? ? ? (mcbsp->pdata->buffer_size)
> ?#define valid_threshold(m, val) ? ? ? ? ? ? ? ?((val) <= max_thres(m))
> ?#define THRESHOLD_PROP_BUILDER(prop) ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> @@ -1205,10 +1196,6 @@ static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp)
> ? ? ? ? ? ? ? ? ? ? ? ?omap_st_remove(mcbsp);
> ? ? ? ?}
> ?}
> -#else
> -static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp) {}
> -static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp) {}
> -#endif /* CONFIG_ARCH_OMAP3 */
>
> ?/*
> ?* McBSP1 and McBSP3 are directly mapped on 1610 and 1510.
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

[RFC 11/12] omap: mcbsp: Remove conditional compilation for OMAP3

[Jarkko Nikula]

On Fri, 1 Jul 2011 16:30:09 +0530
"Varadarajan, Charulatha" <charu@ti.com> wrote:

> > --- a/arch/arm/plat-omap/mcbsp.c
> > +++ b/arch/arm/plat-omap/mcbsp.c
> > @@ -63,7 +63,6 @@ static int omap_mcbsp_read(struct omap_mcbsp *mcbsp, u16 reg, bool from_cache)
> > ? ? ? ?}
> > ?}
> >
> > -#ifdef CONFIG_ARCH_OMAP3
> 
> Are there any plans to move the functions  omap_mcbsp_st_*
> to mach-omap2/* as they are specific to OMAP3 only?
> 
I was thinking that for saving code space for non !OMAP3 builds but
thats again is not generic code if sidetone IP gets ever reused in
another silicon. Another platform device for sidetone could be one
solution but I would like to preserve sidetone sysfs nodes to get
registered to McBSP port for keeping the userspace API the same during
the transition (or over a few kernel releases).

-- 
Jarkko

[RFC 07/12] omap: mcbsp: Get rid of remaining is_omap tests

[Varadarajan, Charulatha]

Jarkko,

On Fri, Jul 1, 2011 at 14:22, Jarkko Nikula <jhnikula@gmail.com> wrote:
> We can get rid of remaining is_omap tests in the driver by using
> mcbsp_config_type that is set in omap hwmod data for 2430, 34xx and 44xx.
>
> Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
> ---
> ?arch/arm/plat-omap/mcbsp.c | ? 30 ++++++++++++++----------------
> ?1 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index 92d6d4f..47720b1 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -193,7 +193,7 @@ void omap_mcbsp_config(unsigned int id, const struct omap_mcbsp_reg_cfg *config)
> ? ? ? ?MCBSP_WRITE(mcbsp, MCR2, config->mcr2);
> ? ? ? ?MCBSP_WRITE(mcbsp, MCR1, config->mcr1);
> ? ? ? ?MCBSP_WRITE(mcbsp, PCR0, config->pcr0);
> - ? ? ? if (cpu_is_omap2430() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
> + ? ? ? if (mcbsp->mcbsp_config_type >= MCBSP_CONFIG_TYPE2) {
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, XCCR, config->xccr);
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, RCCR, config->rccr);
> ? ? ? ?}
> @@ -531,14 +531,13 @@ void omap_mcbsp_set_tx_threshold(unsigned int id, u16 threshold)
> ?{
> ? ? ? ?struct omap_mcbsp *mcbsp;
>
> - ? ? ? if (!cpu_is_omap34xx() && !cpu_is_omap44xx())
> - ? ? ? ? ? ? ? return;
> -
> ? ? ? ?if (!omap_mcbsp_check_valid_id(id)) {
> ? ? ? ? ? ? ? ?printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> ? ? ? ? ? ? ? ?return;
> ? ? ? ?}
> ? ? ? ?mcbsp = id_to_mcbsp_ptr(id);
> + ? ? ? if (mcbsp->mcbsp_config_type < MCBSP_CONFIG_TYPE3)
> + ? ? ? ? ? ? ? return;
>
> ? ? ? ?if (threshold && threshold <= mcbsp->max_tx_thres)
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, THRSH2, threshold - 1);
> @@ -554,14 +553,13 @@ void omap_mcbsp_set_rx_threshold(unsigned int id, u16 threshold)
> ?{
> ? ? ? ?struct omap_mcbsp *mcbsp;
>
> - ? ? ? if (!cpu_is_omap34xx() && !cpu_is_omap44xx())
> - ? ? ? ? ? ? ? return;
> -
> ? ? ? ?if (!omap_mcbsp_check_valid_id(id)) {
> ? ? ? ? ? ? ? ?printk(KERN_ERR "%s: Invalid id (%d)\n", __func__, id + 1);
> ? ? ? ? ? ? ? ?return;
> ? ? ? ?}
> ? ? ? ?mcbsp = id_to_mcbsp_ptr(id);
> + ? ? ? if (mcbsp->mcbsp_config_type < MCBSP_CONFIG_TYPE3)
> + ? ? ? ? ? ? ? return;
>
> ? ? ? ?if (threshold && threshold <= mcbsp->max_rx_thres)
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, THRSH1, threshold - 1);
> @@ -698,7 +696,7 @@ static inline void omap34xx_mcbsp_request(struct omap_mcbsp *mcbsp)
> ? ? ? ? * Enable wakup behavior, smart idle and all wakeups
> ? ? ? ? * REVISIT: some wakeups may be unnecessary
> ? ? ? ? */
> - ? ? ? if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> + ? ? ? if (mcbsp->mcbsp_config_type >= MCBSP_CONFIG_TYPE3) {
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, WAKEUPEN, XRDYEN | RRDYEN);
> ? ? ? ?}
> ?}
> @@ -712,7 +710,7 @@ static inline void omap34xx_mcbsp_free(struct omap_mcbsp *mcbsp)
> ? ? ? ?/*
> ? ? ? ? * Disable wakup behavior, smart idle and all wakeups
> ? ? ? ? */
> - ? ? ? if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> + ? ? ? if (mcbsp->mcbsp_config_type >= MCBSP_CONFIG_TYPE3) {


I think that it would be a good idea to not use
MCBSP_CONFIG_TYPE* in plat-omap/* as it is the .rev info.
At later point of time, this might make the code look uglier
as it happened with GPIO_METHOD_* flags in OMAP GPIO driver.
Instead use this .rev info in mach-omap*/ to fill the pdata and pass it
to driver.

-V Charulatha

> ? ? ? ? ? ? ? ?/*
> ? ? ? ? ? ? ? ? * HW bug workaround - If no_idle mode is taken, we need to
> ? ? ? ? ? ? ? ? * go to smart_idle before going to always_idle, or the
> @@ -872,7 +870,7 @@ void omap_mcbsp_start(unsigned int id, int tx, int rx)
> ? ? ? ?}
> ? ? ? ?mcbsp = id_to_mcbsp_ptr(id);
>
> - ? ? ? if (cpu_is_omap34xx())
> + ? ? ? if (mcbsp->mcbsp_config_type == MCBSP_CONFIG_TYPE3)
> ? ? ? ? ? ? ? ?omap_st_start(mcbsp);
>
> ? ? ? ?/* Only enable SRG, if McBSP is master */
> @@ -910,7 +908,7 @@ void omap_mcbsp_start(unsigned int id, int tx, int rx)
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, SPCR2, w | (1 << 7));
> ? ? ? ?}
>
> - ? ? ? if (cpu_is_omap2430() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
> + ? ? ? if (mcbsp->mcbsp_config_type >= MCBSP_CONFIG_TYPE2) {
> ? ? ? ? ? ? ? ?/* Release the transmitter and receiver */
> ? ? ? ? ? ? ? ?w = MCBSP_READ_CACHE(mcbsp, XCCR);
> ? ? ? ? ? ? ? ?w &= ~(tx ? XDISABLE : 0);
> @@ -940,7 +938,7 @@ void omap_mcbsp_stop(unsigned int id, int tx, int rx)
>
> ? ? ? ?/* Reset transmitter */
> ? ? ? ?tx &= 1;
> - ? ? ? if (cpu_is_omap2430() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
> + ? ? ? if (mcbsp->mcbsp_config_type >= MCBSP_CONFIG_TYPE2) {
> ? ? ? ? ? ? ? ?w = MCBSP_READ_CACHE(mcbsp, XCCR);
> ? ? ? ? ? ? ? ?w |= (tx ? XDISABLE : 0);
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, XCCR, w);
> @@ -950,7 +948,7 @@ void omap_mcbsp_stop(unsigned int id, int tx, int rx)
>
> ? ? ? ?/* Reset receiver */
> ? ? ? ?rx &= 1;
> - ? ? ? if (cpu_is_omap2430() || cpu_is_omap34xx() || cpu_is_omap44xx()) {
> + ? ? ? if (mcbsp->mcbsp_config_type >= MCBSP_CONFIG_TYPE2) {
> ? ? ? ? ? ? ? ?w = MCBSP_READ_CACHE(mcbsp, RCCR);
> ? ? ? ? ? ? ? ?w |= (rx ? RDISABLE : 0);
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, RCCR, w);
> @@ -967,7 +965,7 @@ void omap_mcbsp_stop(unsigned int id, int tx, int rx)
> ? ? ? ? ? ? ? ?MCBSP_WRITE(mcbsp, SPCR2, w & ~(1 << 6));
> ? ? ? ?}
>
> - ? ? ? if (cpu_is_omap34xx())
> + ? ? ? if (mcbsp->mcbsp_config_type == MCBSP_CONFIG_TYPE3)
> ? ? ? ? ? ? ? ?omap_st_stop(mcbsp);
> ?}
> ?EXPORT_SYMBOL(omap_mcbsp_stop);
> @@ -1230,7 +1228,7 @@ static void __devexit omap_st_remove(struct omap_mcbsp *mcbsp)
> ?static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp)
> ?{
> ? ? ? ?mcbsp->dma_op_mode = MCBSP_DMA_MODE_ELEMENT;
> - ? ? ? if (cpu_is_omap34xx()) {
> + ? ? ? if (mcbsp->mcbsp_config_type == MCBSP_CONFIG_TYPE3) {
> ? ? ? ? ? ? ? ?/*
> ? ? ? ? ? ? ? ? * Initially configure the maximum thresholds to a safe value.
> ? ? ? ? ? ? ? ? * The McBSP FIFO usage with these values should not go under
> @@ -1262,7 +1260,7 @@ static inline void __devinit omap34xx_device_init(struct omap_mcbsp *mcbsp)
>
> ?static inline void __devexit omap34xx_device_exit(struct omap_mcbsp *mcbsp)
> ?{
> - ? ? ? if (cpu_is_omap34xx()) {
> + ? ? ? if (mcbsp->mcbsp_config_type == MCBSP_CONFIG_TYPE3) {
> ? ? ? ? ? ? ? ?omap_additional_remove(mcbsp->dev);
>
> ? ? ? ? ? ? ? ?if (mcbsp->id == 2 || mcbsp->id == 3)
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

[RFC 07/12] omap: mcbsp: Get rid of remaining is_omap tests

[Jarkko Nikula]

Hi

On Fri, 1 Jul 2011 16:41:23 +0530
"Varadarajan, Charulatha" <charu@ti.com> wrote:

> > @@ -712,7 +710,7 @@ static inline void omap34xx_mcbsp_free(struct omap_mcbsp *mcbsp)
> > ? ? ? ?/*
> > ? ? ? ? * Disable wakup behavior, smart idle and all wakeups
> > ? ? ? ? */
> > - ? ? ? if (cpu_is_omap34xx() || cpu_is_omap44xx()) {
> > + ? ? ? if (mcbsp->mcbsp_config_type >= MCBSP_CONFIG_TYPE3) {
> 
> 
> I think that it would be a good idea to not use
> MCBSP_CONFIG_TYPE* in plat-omap/* as it is the .rev info.
> At later point of time, this might make the code look uglier
> as it happened with GPIO_METHOD_* flags in OMAP GPIO driver.
> Instead use this .rev info in mach-omap*/ to fill the pdata and pass it
> to driver.
> 
Exactly and this is already a little uggly here. Kind of simple
replacement for cpu_is_omap tests and definitely must be replaced with
some feature flags before moving code outside of plat-omap.

-- 
Jarkko

[RFC 04/12] omap: mcbsp: Merge OMAP1 and OMAP2+ McBSP register definitions

[Janusz Krzysztofik]

On Fri, 1 Jul 2011 at 10:52:28 Jarkko Nikula wrote:
> There is no need to have separate register definitions for OMAP1
> since they are ordered similarly than in OMAP2+ and thus register
> address offsets can be calculated from OMAP2+ register address
> offsets.

Hi Jarkko,
This patch is missing a similiar change to the calculation of register 
address returned by omap_mcbsp_dma_reg_params(), rendering it broken on 
OMAP1. Otherwise, the whole series works for me on my Amstrad Delta. 

For me, it would be more elegant to redefine all these symbols as 
register serial numbers, not offsets, then multiply them by sub-platform 
specific register width to get their offsets, instead of having offsets 
already defined for one sub-platfrom only and mangle them to get correct 
values on others.

Anyway, with the omap_mcbsp_dma_reg_params() fixed to return correct 
address on OMAP1 as well, you can add my Tested-by: or Acked-by:, which 
one you find more appropriate.

Thanks,
Janusz

> Signed-off-by: Jarkko Nikula <jhnikula@gmail.com>
> ---
>  arch/arm/plat-omap/include/plat/mcbsp.h |   50
> ++++--------------------------- arch/arm/plat-omap/mcbsp.c          
>    |    2 +
>  2 files changed, 8 insertions(+), 44 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h
> b/arch/arm/plat-omap/include/plat/mcbsp.h index 9882c65..6db02b3
> 100644
> --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> @@ -51,54 +51,16 @@ static struct platform_device omap_mcbsp##port_nr
> = {	\ #define OMAP1610_MCBSP2_BASE	0xfffb1000
>  #define OMAP1610_MCBSP3_BASE	0xe1017000
> 
> -#ifdef CONFIG_ARCH_OMAP1
> -
> -#define OMAP_MCBSP_REG_DRR2	0x00
> -#define OMAP_MCBSP_REG_DRR1	0x02
> -#define OMAP_MCBSP_REG_DXR2	0x04
> -#define OMAP_MCBSP_REG_DXR1	0x06
> -#define OMAP_MCBSP_REG_DRR	0x02
> -#define OMAP_MCBSP_REG_DXR	0x06
> -#define OMAP_MCBSP_REG_SPCR2	0x08
> -#define OMAP_MCBSP_REG_SPCR1	0x0a
> -#define OMAP_MCBSP_REG_RCR2	0x0c
> -#define OMAP_MCBSP_REG_RCR1	0x0e
> -#define OMAP_MCBSP_REG_XCR2	0x10
> -#define OMAP_MCBSP_REG_XCR1	0x12
> -#define OMAP_MCBSP_REG_SRGR2	0x14
> -#define OMAP_MCBSP_REG_SRGR1	0x16
> -#define OMAP_MCBSP_REG_MCR2	0x18
> -#define OMAP_MCBSP_REG_MCR1	0x1a
> -#define OMAP_MCBSP_REG_RCERA	0x1c
> -#define OMAP_MCBSP_REG_RCERB	0x1e
> -#define OMAP_MCBSP_REG_XCERA	0x20
> -#define OMAP_MCBSP_REG_XCERB	0x22
> -#define OMAP_MCBSP_REG_PCR0	0x24
> -#define OMAP_MCBSP_REG_RCERC	0x26
> -#define OMAP_MCBSP_REG_RCERD	0x28
> -#define OMAP_MCBSP_REG_XCERC	0x2A
> -#define OMAP_MCBSP_REG_XCERD	0x2C
> -#define OMAP_MCBSP_REG_RCERE	0x2E
> -#define OMAP_MCBSP_REG_RCERF	0x30
> -#define OMAP_MCBSP_REG_XCERE	0x32
> -#define OMAP_MCBSP_REG_XCERF	0x34
> -#define OMAP_MCBSP_REG_RCERG	0x36
> -#define OMAP_MCBSP_REG_RCERH	0x38
> -#define OMAP_MCBSP_REG_XCERG	0x3A
> -#define OMAP_MCBSP_REG_XCERH	0x3C
> -
> -/* Dummy defines, these are not available on omap1 */
> -#define OMAP_MCBSP_REG_XCCR	0x00
> -#define OMAP_MCBSP_REG_RCCR	0x00
> -
> -#else
> -
> +/* McBSP register addresses. OMAP1 address = address definition / 2
> */ +/* OMAP1-OMAP2420 registers */
>  #define OMAP_MCBSP_REG_DRR2	0x00
>  #define OMAP_MCBSP_REG_DRR1	0x04
>  #define OMAP_MCBSP_REG_DXR2	0x08
>  #define OMAP_MCBSP_REG_DXR1	0x0C
> +/* OMAP2430 and onwards */
>  #define OMAP_MCBSP_REG_DRR	0x00
>  #define OMAP_MCBSP_REG_DXR	0x08
> +/* Common registers */
>  #define OMAP_MCBSP_REG_SPCR2	0x10
>  #define OMAP_MCBSP_REG_SPCR1	0x14
>  #define OMAP_MCBSP_REG_RCR2	0x18
> @@ -126,6 +88,7 @@ static struct platform_device omap_mcbsp##port_nr
> = {	\ #define OMAP_MCBSP_REG_RCERH	0x70
>  #define OMAP_MCBSP_REG_XCERG	0x74
>  #define OMAP_MCBSP_REG_XCERH	0x78
> +/* OMAP2430 and onwards */
>  #define OMAP_MCBSP_REG_SYSCON	0x8C
>  #define OMAP_MCBSP_REG_THRSH2	0x90
>  #define OMAP_MCBSP_REG_THRSH1	0x94
> @@ -138,6 +101,7 @@ static struct platform_device omap_mcbsp##port_nr
> = {	\ #define OMAP_MCBSP_REG_RBUFFSTAT	0xB8
>  #define OMAP_MCBSP_REG_SSELCR	0xBC
> 
> +/* OMAP3 sidetone control registers */
>  #define OMAP_ST_REG_REV		0x00
>  #define OMAP_ST_REG_SYSCONFIG	0x10
>  #define OMAP_ST_REG_IRQSTATUS	0x18
> @@ -146,8 +110,6 @@ static struct platform_device omap_mcbsp##port_nr
> = {	\ #define OMAP_ST_REG_SFIRCR	0x28
>  #define OMAP_ST_REG_SSELCR	0x2C
> 
> -#endif
> -
>  /************************** McBSP SPCR1 bit definitions
> ***********************/ #define RRST			0x0001
>  #define RRDY			0x0002
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index 6c62af1..a7ced1b 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -37,6 +37,7 @@ int omap_mcbsp_count, omap_mcbsp_cache_size;
>  static void omap_mcbsp_write(struct omap_mcbsp *mcbsp, u16 reg, u32
> val) {
>  	if (cpu_class_is_omap1()) {
> +		reg /= 2;
>  		((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)] = (u16)val;
>  		__raw_writew((u16)val, mcbsp->io_base + reg);
>  	} else if (cpu_is_omap2420()) {
> @@ -51,6 +52,7 @@ static void omap_mcbsp_write(struct omap_mcbsp
> *mcbsp, u16 reg, u32 val) static int omap_mcbsp_read(struct
> omap_mcbsp *mcbsp, u16 reg, bool from_cache) {
>  	if (cpu_class_is_omap1()) {
> +		reg /= 2;
>  		return !from_cache ? __raw_readw(mcbsp->io_base + reg) :
>  				((u16 *)mcbsp->reg_cache)[reg / sizeof(u16)];
>  	} else if (cpu_is_omap2420()) {

[RFC 02/12] omap: mcbsp: Remove rx_/tx_word_length variables

[Tony Lindgren]

* Jarkko Nikula <jhnikula@gmail.com> [110701 01:48]:
> These variables got unused after ("omap: mcbsp: Drop in-driver transfer
> support") but was noticed only afterwards.

I'll queue this and the one after this into cleanup.

Regards,

Tony