Trilok,

See the comments below.

Trilok Soni wrote:
Hi Miguel,

On Sat, Oct 10, 2009 at 12:30 AM, Miguel Aguilar
<[email protected]> wrote:
Hi Trilok
Trilok Soni wrote:
Hi Miguel,

To me it seems that scripts/checkpatch.pl will show few errors. Please
run it through checkpatch.pl.
This patch was made with git format-patch.

As I said below it will definitely show few errors.

+
+#ifndef DAVINCI_KEYSCAN_H
+#define DAVINCI_KEYSCAN_H
+
+#include <linux/io.h>
+
+/* Base of key scan register bank */
+#define DM365_KEYSCAN_BASE             (0x01C69400)
Why? You don't need this if you do ioremap and define this base
address through resources[].
Actually this macro is used to define the base address through resources in
the dm365.c file, so it does exactly what you mean. DaVinci platforms handle
the base address in this way.

I think there is no need of putting this #define in this file at all.
Just use this base address directly in resource in devices.c or put
this #define at top of the resources itself. It serves no purpose in
this file.

Use the base address in the resource structure directly I'm sure is not well accepted by the community because it is not as readable as using macros, as I told you this way of handling the base address is the same used in the Davinci platforms for many devices, for example:

The file arch/arm/mach-davinci/include/mach/asp.h defines many base addresses for Davinci ASP devices:

------------------------------------------------------------------------------
/*
 * <mach/asp.h> - DaVinci Audio Serial Port support
 */
#ifndef __ASM_ARCH_DAVINCI_ASP_H
#define __ASM_ARCH_DAVINCI_ASP_H

#include <mach/irqs.h>
#include <mach/edma.h>

/* Bases of dm644x and dm355 register banks */
#define DAVINCI_ASP0_BASE       0x01E02000
#define DAVINCI_ASP1_BASE       0x01E04000

/* Bases of dm365 register banks */
#define DAVINCI_DM365_ASP0_BASE 0x01D02000

/* Bases of dm646x register banks */
#define DAVINCI_DM646X_MCASP0_REG_BASE          0x01D01000
#define DAVINCI_DM646X_MCASP1_REG_BASE          0x01D01800

/* Bases of da850/da830 McASP0  register banks */
#define DAVINCI_DA8XX_MCASP0_REG_BASE   0x01D00000

/* Bases of da830 McASP1 register banks */
#define DAVINCI_DA830_MCASP1_REG_BASE   0x01D04000
.....
-------------------------------------------------------------------------------

Then for example the base address of DM365 ASP, it is defined at asp.h and it is used in the dm365 by resources in the file arch/arm/mach-davinci/dm365.c

-------------------------------------------------------------------------------
static struct resource dm365_asp_resources[] = {
        {
                .start  = DAVINCI_DM365_ASP0_BASE,
                .end    = DAVINCI_DM365_ASP0_BASE + SZ_8K - 1,
                .flags  = IORESOURCE_MEM,
        },
.....
--------------------------------------------------------------------------------

So the keyscan is handle in the same way using the keyscan.h and the dm365.c.

Thanks,

Miguel Aguilar

_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to