Janusz Krzysztofik napisał(a):
Thursday 03 December 2009 21:03:02 Tony Lindgren napisał(a):
Hi,
* Janusz Krzysztofik <jkrzyszt@xxxxxxxxxxxx> [091203 04:29]:
Reserve space inside omap_mcbsp structure for storing cached copies of
McBSP register values.
Modify the MCBSP_WRITE() macro to update the cache with every register
write operation.
Introduce a new macro that reads from the cache instead of hardware.
Applies on top of patch 2 from this series:
[PATCH v3 2/4] OMAP: McBSP: Prepare register read/write macros API for
caching
Tested on OMAP1510 based Amstrad Delta using linux-omap for-next,
commit 4421752e331cfb1d942b47ffdb26e451a8da58a0.
Compile-tested with omap_3430sdp_defconfig.
Signed-off-by: Janusz Krzysztofik <jkrzyszt@xxxxxxxxxxxx>
---
Thursday 03 December 2009 08:49:21 Jarkko Nikula wrote:
On Tue, 1 Dec 2009 04:15:50 +0100
Janusz Krzysztofik <jkrzyszt@xxxxxxxxxxxx> wrote:
#define MCBSP_WRITE(mcbsp, reg, val) \
- omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
+ omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
+ mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1] \
+ = val)
+#define MCBSP_READ_CACHE(mcbsp, reg) \
+ (mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / OMAP_MCBSP_REG_DRR1])
These divisions by DDR1 are confusing. Use rather sizeof:
reg_cache[OMAP_MCBSP_REG_##reg / sizeof(*mcbsp->reg_cache)]
Done.
Jarkko, many thanks for your cooperation :).
Janusz
arch/arm/plat-omap/include/plat/mcbsp.h | 5 +++++
arch/arm/plat-omap/mcbsp.c | 7 ++++++-
2 files changed, 11 insertions(+), 1 deletion(-)
--- git/arch/arm/plat-omap/include/plat/mcbsp.h.orig 2009-11-27
11:53:45.000000000 +0100 +++
git/arch/arm/plat-omap/include/plat/mcbsp.h 2009-12-01 03:37:21.000000000
+0100 @@ -415,6 +415,11 @@ struct omap_mcbsp {
u16 max_tx_thres;
u16 max_rx_thres;
#endif
+#ifdef CONFIG_ARCH_OMAP1
+ u16 reg_cache[OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
+#else
+ u32 reg_cache[OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
+#endif
};
extern struct omap_mcbsp **mcbsp_ptr;
extern int omap_mcbsp_count;
How about rather set:
static u16 omap1_reg_cache[];
and
static u32 omap2_reg_cache[];
Hi Tony,
Let me understand more precisely what you mean before I submit next version.
Did you mean something like:
static u16 omap1_reg_cache[3][OMAP_MCBSP_REG_XCERH / sizeof(u16) + 1];
static u32 omap2_reg_cache[5][OMAP_MCBSP_REG_RCCR / sizeof(u32) + 1];
both put in arch/arm/plat-omap/mcbsp.c without any ifdefs?
Tony,
I think I've already found one part of the answer myself: you probably
mean putting each table definition in a corresponding
arch/arm/mach-omap[12]/mcbsp.c source file, don't you?
What remains unclear for me is if you really intend to reserve static
cache space for all McBSP ports supported by a machine, even if not
supported by a particular cpu or not used on a particular board.
--- git/arch/arm/plat-omap/mcbsp.c.orig 2009-12-01 03:19:56.000000000
+0100 +++ git/arch/arm/plat-omap/mcbsp.c 2009-12-03 11:37:21.000000000
+0100 @@ -49,7 +49,12 @@ int omap_mcbsp_read(void __iomem *io_bas
#define MCBSP_READ(mcbsp, reg) \
omap_mcbsp_read(mcbsp->io_base, OMAP_MCBSP_REG_##reg)
#define MCBSP_WRITE(mcbsp, reg, val) \
- omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, val)
+ omap_mcbsp_write(mcbsp->io_base, OMAP_MCBSP_REG_##reg, \
+ mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
+ sizeof(*mcbsp->reg_cache)] = val)
+#define MCBSP_READ_CACHE(mcbsp, reg) \
+ (mcbsp->reg_cache[OMAP_MCBSP_REG_##reg / \
+ sizeof(*mcbsp->reg_cache)])
#define omap_mcbsp_check_valid_id(id) (id < omap_mcbsp_count)
#define id_to_mcbsp_ptr(id) mcbsp_ptr[id];
Then rather than doing this cache manipulation in the macro, just
do it in the omap_mcbsp_read/write functions based on the
cpu_class_is_omap1() accessing omap1_reg_cache or omap2_reg_cache.
omap_mcbsp_wrtie() case seems clear to me, but:
Did you mean defining a new omap_mcbsp_read_cache() function, similiar to
omap_mcbsp_read(), but reading from cache instead of hardware?
Or do you prefere if I modify omap_mcbsp_read() to be able to read from both
hardware and cache, dependig on an additional argument, for example?
Thanks,
Janusz
This way you can get rid of the ifdef else.
But shouldn't declarations for both tables be placed in
arch/arm/plat-omap/include/plat/mcbsp.h withing an ifdef else block? If
yes, one ifdef else block would be replaced with another. Or did you
mean placing those declarations inside existing ifdef block with defines
for register offsets?
Thanks,
Janusz
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
