From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 21:01:44 +0530
Subject: [PATCH] Minimal Display driver for OMAP3Supports dynamic configuration of Panel and Video Encoder
Supports Background color on DVID
Supports Color bar on S-Video
We are getting there.. thanks a bunch. if you can split this series
into two sets:
a) introducing DSS layer
b) introduce beagle support for the same
it will be better.
but NAK to this patch.
Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
---
board/ti/beagle/beagle.c | 13 +++
board/ti/beagle/beagle.h | 73 ++++++++++++++
drivers/video/Makefile | 1 +
drivers/video/omap3_dss.c | 128 +++++++++++++++++++++++++
include/asm-arm/arch-omap3/dss.h | 193 ++++++++++++++++++++++++++++++++++++++
include/configs/omap3_beagle.h | 1 +
6 files changed, 409 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/omap3_dss.c
create mode 100644 include/asm-arm/arch-omap3/dss.h
[...]
diff --git a/include/asm-arm/arch-omap3/dss.h b/include/asm-arm/arch-omap3/dss.h
new file mode 100644
index 0000000..08c7d8d
--- /dev/null
+++ b/include/asm-arm/arch-omap3/dss.h
@@ -0,0 +1,193 @@
+/*
+ * (C) Copyright 2010
+ * Texas Instruments, <www.ti.com>
+ * Syed Mohammed Khasim <khasim@ti.com>
+ *
+ * Referred to Linux DSS driver files for OMAP3
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation's version 2 of
+ * the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef DSS_H
+#define DSS_H
+
+/* VENC Register address */
+#define VENC_REV_ID 0x48050C00
NAK. why do you need this if you have a struct?
here is what I think:
venc_config {
}
if it is organized as the register definitions,
configure_venc(struct venc_config *values)
struct venc_config * d = BASE_ADDRESS_OF_OMAP3_VENC;
writel(values->regx, &d->regx);
refer: drivers/mtd/nand/omap_gpmc.c
+#define VENC_STATUS 0x48050C04
+#define VENC_F_CONTROL 0x48050C08
[...]
Regards,
Nishanth Menon
From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 21:01:44 +0530
Subject: [PATCH] Minimal Display driver for OMAP3Supports dynamic configuration of Panel and Video Encoder
Supports Background color on DVID
Supports Color bar on S-VideoWe are getting there.. thanks a bunch. if you can split this series
into two sets:
a) introducing DSS layer
b) introduce beagle support for the same
it will be better.
Can you complete your review review comments here, I will take up this
when in-corporate all review comments together.
Kindly remember these patches will be tested on B4, C2, C3, C4, EVM
before releasing.
but NAK to this patch.
Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
---
board/ti/beagle/beagle.c | 13 +++
board/ti/beagle/beagle.h | 73 ++++++++++++++
drivers/video/Makefile | 1 +
drivers/video/omap3_dss.c | 128 +++++++++++++++++++++++++
include/asm-arm/arch-omap3/dss.h | 193 ++++++++++++++++++++++++++++++++++++++
include/configs/omap3_beagle.h | 1 +
6 files changed, 409 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/omap3_dss.c
create mode 100644 include/asm-arm/arch-omap3/dss.h[...]
diff --git a/include/asm-arm/arch-omap3/dss.h b/include/asm-arm/arch-omap3/dss.h
new file mode 100644
index 0000000..08c7d8d
--- /dev/null
+++ b/include/asm-arm/arch-omap3/dss.h
@@ -0,0 +1,193 @@
+/*
+ * (C) Copyright 2010
+ * Texas Instruments, <www.ti.com>
+ * Syed Mohammed Khasim <khasim@ti.com>
+ *
+ * Referred to Linux DSS driver files for OMAP3
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation's version 2 of
+ * the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef DSS_H
+#define DSS_H
+
+/* VENC Register address */
+#define VENC_REV_ID 0x48050C00NAK. why do you need this if you have a struct?
You need to read patch more carefully before giving NAK.
Structure is used to give multiple instance of Panels or different
VENC settings like NTSC or PAL
You can add a new panel or a new tv standard with these structures
easily. Structures are not used for register accesses.
here is what I think:
venc_config {
}if it is organized as the register definitions,
configure_venc(struct venc_config *values)
struct venc_config * d = BASE_ADDRESS_OF_OMAP3_VENC;
writel(values->regx, &d->regx);refer: drivers/mtd/nand/omap_gpmc.c
GPIO, GPMC and other controllers have multiple instances in OMAP, it
makes sense to organize such register set in structure mode. I did
start with that but found no use for DSS as it is just one instance.
Structures don't give any value here.
More over I am introducing minimal DSS driver with minimal register
set. It doesn't help any to give structure based register access for
single instance drivers.
Give more reasons for NAK.
Regards,
Khasim
Khasim Syed Mohammed said the following on 01/08/2010 09:00 PM:
From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 21:01:44 +0530
Subject: [PATCH] Minimal Display driver for OMAP3Supports dynamic configuration of Panel and Video Encoder
Supports Background color on DVID
Supports Color bar on S-Video
We are getting there.. thanks a bunch. if you can split this series
into two sets:
a) introducing DSS layer
b) introduce beagle support for the same
it will be better.
Can you complete your review review comments here, I will take up this
when in-corporate all review comments together.
if you can give it a week before posting again, you could probably collate comments from various quarters. I just am a part time reviewer ;)..
Kindly remember these patches will be tested on B4, C2, C3, C4, EVM
before releasing.
great. good to know.
but NAK to this patch.
Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
---
board/ti/beagle/beagle.c | 13 +++
board/ti/beagle/beagle.h | 73 ++++++++++++++
drivers/video/Makefile | 1 +
drivers/video/omap3_dss.c | 128 +++++++++++++++++++++++++
include/asm-arm/arch-omap3/dss.h | 193 ++++++++++++++++++++++++++++++++++++++
include/configs/omap3_beagle.h | 1 +
6 files changed, 409 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/omap3_dss.c
create mode 100644 include/asm-arm/arch-omap3/dss.h[...]
diff --git a/include/asm-arm/arch-omap3/dss.h b/include/asm-arm/arch-omap3/dss.h
new file mode 100644
index 0000000..08c7d8d
--- /dev/null
+++ b/include/asm-arm/arch-omap3/dss.h
@@ -0,0 +1,193 @@
+/*
+ * (C) Copyright 2010
+ * Texas Instruments, <www.ti.com>
+ * Syed Mohammed Khasim <khasim@ti.com>
+ *
+ * Referred to Linux DSS driver files for OMAP3
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation's version 2 of
+ * the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef DSS_H
+#define DSS_H
+
+/* VENC Register address */
+#define VENC_REV_ID 0x48050C00
NAK. why do you need this if you have a struct?
You need to read patch more carefully before giving NAK.Structure is used to give multiple instance of Panels or different
VENC settings like NTSC or PAL
Thanks for clarifying
That is the crux of the problem. if you use struct to describe the venc -> then you'd be staying with how the rest of u-boot accesses are:
using struct dereference. if you look at nand.c as I pointed out, Dirk had done a great job of converting register offsets
to struct and referencing off it. This was required to get the driver mainlined.
The recomendation here is to move from #defines to struct based register usage. I am ok with the rest(except for need to split).
You can add a new panel or a new tv standard with these structures
easily. Structures are not used for register accesses.here is what I think:
venc_config {
}if it is organized as the register definitions,
configure_venc(struct venc_config *values)
struct venc_config * d = BASE_ADDRESS_OF_OMAP3_VENC;
writel(values->regx, &d->regx);refer: drivers/mtd/nand/omap_gpmc.c
GPIO, GPMC and other controllers have multiple instances in OMAP, it
makes sense to organize such register set in structure mode. I did
start with that but found no use for DSS as it is just one instance.
Structures don't give any value here.
there were other reasons mentioned when nand got split -> one of them had to do with the compiler or something. Dirk might remember - unfortunately, this was more than a year back.. if I recollect right..
More over I am introducing minimal DSS driver with minimal register
set. It doesn't help any to give structure based register access for
single instance drivers.
moving to struct based is easy and done once and improves your chance of your driver getting upstreamed :).
Give more reasons for NAK.
This is the only reason I have other than the need to split this patch up into logical chunks. my NAK still stands unfortunately for the following generic reasons:
a) split the patch as mentioned above.
b) move away from #defines to struct based reg access.
Regards,
Khasim
Regards,
Nishanth Menon
Khasim Syed Mohammed said the following on 01/08/2010 09:00 PM:
From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 21:01:44 +0530
Subject: [PATCH] Minimal Display driver for OMAP3Supports dynamic configuration of Panel and Video Encoder
Supports Background color on DVID
Supports Color bar on S-VideoWe are getting there.. thanks a bunch. if you can split this series
into two sets:
a) introducing DSS layer
b) introduce beagle support for the same
it will be better.Can you complete your review review comments here, I will take up this
when in-corporate all review comments together.if you can give it a week before posting again, you could probably collate
comments from various quarters. I just am a part time reviewer ;)..Kindly remember these patches will be tested on B4, C2, C3, C4, EVM
before releasing.great. good to know.
but NAK to this patch.
Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
---
board/ti/beagle/beagle.c | 13 +++
board/ti/beagle/beagle.h | 73 ++++++++++++++
drivers/video/Makefile | 1 +
drivers/video/omap3_dss.c | 128 +++++++++++++++++++++++++
include/asm-arm/arch-omap3/dss.h | 193
++++++++++++++++++++++++++++++++++++++
include/configs/omap3_beagle.h | 1 +
6 files changed, 409 insertions(+), 0 deletions(-)
create mode 100644 drivers/video/omap3_dss.c
create mode 100644 include/asm-arm/arch-omap3/dss.h[...]
diff --git a/include/asm-arm/arch-omap3/dss.h
b/include/asm-arm/arch-omap3/dss.h
new file mode 100644
index 0000000..08c7d8d
--- /dev/null
+++ b/include/asm-arm/arch-omap3/dss.h
@@ -0,0 +1,193 @@
+/*
+ * (C) Copyright 2010
+ * Texas Instruments, <www.ti.com>
+ * Syed Mohammed Khasim <khasim@ti.com>
+ *
+ * Referred to Linux DSS driver files for OMAP3
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation's version 2 of
+ * the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef DSS_H
+#define DSS_H
+
+/* VENC Register address */
+#define VENC_REV_ID 0x48050C00NAK. why do you need this if you have a struct?
You need to read patch more carefully before giving NAK.
Structure is used to give multiple instance of Panels or different
VENC settings like NTSC or PALThanks for clarifying
That is the crux of the problem. if you use struct to describe the venc ->
then you'd be staying with how the rest of u-boot accesses are:
using struct dereference. if you look at nand.c as I pointed out, Dirk had
done a great job of converting register offsets
to struct and referencing off it. This was required to get the driver
mainlined.
As said before, I agree. NAND and GPIO needs struct based development
as it has multiple instances. It will look odd to implement in
#defines.
The recomendation here is to move from #defines to struct based register
usage. I am ok with the rest(except for need to split).
Split is done, posted yesterday.
Struct based register needs more comments, not that I am lazy to
implement that. I need to know the reason for doing the same when no
multiple instances are used.
You can add a new panel or a new tv standard with these structures
easily. Structures are not used for register accesses.here is what I think:
venc_config {
}if it is organized as the register definitions,
configure_venc(struct venc_config *values)
struct venc_config * d = BASE_ADDRESS_OF_OMAP3_VENC;
writel(values->regx, &d->regx);refer: drivers/mtd/nand/omap_gpmc.c
GPIO, GPMC and other controllers have multiple instances in OMAP, it
makes sense to organize such register set in structure mode. I did
start with that but found no use for DSS as it is just one instance.
Structures don't give any value here.there were other reasons mentioned when nand got split -> one of them had to
do with the compiler or something. Dirk might remember - unfortunately, this
was more than a year back.. if I recollect right..
Will try doing a google. May be some one can point me to that
decision. It would help developing drivers which have single instance
of controller being used.
More over I am introducing minimal DSS driver with minimal register
set. It doesn't help any to give structure based register access for
single instance drivers.moving to struct based is easy and done once and improves your chance of
your driver getting upstreamed :).
DSS in OMAP3 has following register domains.
DSI Protocol Engine 0x4804 FC00 512 bytes
DSI_PHY 0x4804 FE00 64 bytes
DSI PLL Controller 0x4804 FF00 32 bytes
Display Subsystem 0x4805 0000 512 bytes
Display Controller 0x4805 0400 1K byte
Display Controller VID1 0x4805 0400 1K byte
Display Controller VID2 0x4805 0400 1K byte
RFBI 0x4805 0800 256 bytes
Video Encode 0x4805 0C00 256 bytes
I am not sure why one would ask me to give struct definitions for
these 500 (approx) registers when only 50 of these are required to
implement background and color bar. As I am saying all the way, DSS is
not multiple instance module like GPMC (NAND) and GPIO it is just one
module.
Will wait for others in list to comment on these.
Give more reasons for NAK.
This is the only reason I have other than the need to split this patch up
into logical chunks. my NAK still stands unfortunately for the following
generic reasons:
a) split the patch as mentioned above.
Done.
b) move away from #defines to struct based reg access.
Reasons given above. Will wait for others in community to comment on
the above implementation.
Many thanks for the review.
Regards,
Khasim
Khasim Syed Mohammed said the following on 01/09/2010 09:16 PM:
Khasim Syed Mohammed said the following on 01/08/2010 09:00 PM:
From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
[...]
The recomendation here is to move from #defines to struct based register
usage. I am ok with the rest(except for need to split).Split is done, posted yesterday.
Struct based register needs more comments, not that I am lazy to
implement that. I need to know the reason for doing the same when no
multiple instances are used.You can add a new panel or a new tv standard with these structures
easily. Structures are not used for register accesses.here is what I think:
venc_config {
}if it is organized as the register definitions,
configure_venc(struct venc_config *values)
struct venc_config * d = BASE_ADDRESS_OF_OMAP3_VENC;
writel(values->regx, &d->regx);refer: drivers/mtd/nand/omap_gpmc.c
GPIO, GPMC and other controllers have multiple instances in OMAP, it
makes sense to organize such register set in structure mode. I did
start with that but found no use for DSS as it is just one instance.
Structures don't give any value here.there were other reasons mentioned when nand got split -> one of them had to
do with the compiler or something. Dirk might remember - unfortunately, this
was more than a year back.. if I recollect right..Will try doing a google. May be some one can point me to that
decision. It would help developing drivers which have single instance
of controller being used.
the reference I got:
http://old.nabble.com/-U-Boot---PATCH-08-13-v4--ARM%3A-OMAP3%3A-Add-NAND-support-tp20039673p20039673.html
similar changes happend for GPMC etc..
Quote:
> >Is GPMC_BASE an integer or a pointer?
>
> Nothing. A macro:
>
> #define OMAP34XX_GPMC_BASE (0x6E000000)
> #define GPMC_BASE (OMAP34XX_GPMC_BASE)
So it's an integer.
> It's then casted to volatile pointer by ARM's readx/writex.
The cast should be done by the driver, or you'll get warnings if
readx/writex ever become inline functions (as they are on other arches).
might help explain..
More over I am introducing minimal DSS driver with minimal register
set. It doesn't help any to give structure based register access for
single instance drivers.moving to struct based is easy and done once and improves your chance of
your driver getting upstreamed :).DSS in OMAP3 has following register domains.
DSI Protocol Engine 0x4804 FC00 512 bytes
DSI_PHY 0x4804 FE00 64 bytes
DSI PLL Controller 0x4804 FF00 32 bytes
Display Subsystem 0x4805 0000 512 bytes
Display Controller 0x4805 0400 1K byte
Display Controller VID1 0x4805 0400 1K byte
Display Controller VID2 0x4805 0400 1K byte
RFBI 0x4805 0800 256 bytes
Video Encode 0x4805 0C00 256 bytesI am not sure why one would ask me to give struct definitions for
these 500 (approx) registers when only 50 of these are required to
implement background and color bar. As I am saying all the way, DSS is
not multiple instance module like GPMC (NAND) and GPIO it is just one
module.
Aren't you extrapolating this a bit out of scope? DSI,RFBI etc.. is not relevant to your patch.
you may need DSS, controller and VID1(and VID2 is the same). I think your complaint is about having
to define the reg structs when multiple instances dont exist - how about OMAP4? wont these structs
get reused there(once we get around to it)?
Regards,
Nishanth Menon
This is a valid comment, many thanks for digging this out. Considering
this comment, my dss_read_reg and dss_write_reg should become as shown
below
+static inline void dss_write_reg(int reg, u32 val)
+{
+ __raw_writel(val, (uint32_t *) reg);
+}
Khasim Syed Mohammed said the following on 01/09/2010 09:16 PM:
Khasim Syed Mohammed said the following on 01/08/2010 09:00 PM:
From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>[...]
The recomendation here is to move from #defines to struct based register
usage. I am ok with the rest(except for need to split).Split is done, posted yesterday.
Struct based register needs more comments, not that I am lazy to
implement that. I need to know the reason for doing the same when no
multiple instances are used.You can add a new panel or a new tv standard with these structures
easily. Structures are not used for register accesses.here is what I think:
venc_config {
}if it is organized as the register definitions,
configure_venc(struct venc_config *values)
struct venc_config * d = BASE_ADDRESS_OF_OMAP3_VENC;
writel(values->regx, &d->regx);refer: drivers/mtd/nand/omap_gpmc.c
GPIO, GPMC and other controllers have multiple instances in OMAP, it
makes sense to organize such register set in structure mode. I did
start with that but found no use for DSS as it is just one instance.
Structures don't give any value here.there were other reasons mentioned when nand got split -> one of them had
to
do with the compiler or something. Dirk might remember - unfortunately,
this
was more than a year back.. if I recollect right..Will try doing a google. May be some one can point me to that
decision. It would help developing drivers which have single instance
of controller being used.the reference I got:
http://old.nabble.com/-U-Boot---PATCH-08-13-v4--ARM%3A-OMAP3%3A-Add-NAND-support-tp20039673p20039673.htmlsimilar changes happend for GPMC etc..
Quote:
>Is GPMC_BASE an integer or a pointer?
Nothing. A macro:
#define OMAP34XX_GPMC_BASE (0x6E000000)
#define GPMC_BASE (OMAP34XX_GPMC_BASE)So it's an integer.
It's then casted to volatile pointer by ARM's readx/writex.
The cast should be done by the driver, or you'll get warnings if
readx/writex ever become inline functions (as they are on other arches).might help explain..
This is a valid comment, many thanks for digging this out. Considering
this comment, my dss_read_reg and dss_write_reg should become as shown
below+static inline void dss_write_reg(int reg, u32 val)
+{
+ __raw_writel(val, (uint32_t *) reg);
+}
+
+static inline u32 dss_read_reg(int reg)
+{
+ u32 l = __raw_readl((uint32_t *) reg);
+ return l;
+}I will do the above changes and re-submit the patch.
But Kindly NOTE: This still doesn't give me a reason to implement the
register definition as structures when we have single instance of
register set. I am still not considering the structure based
read/write currently.
IIRC the main reason was that Wolfgang would refuse to merge initial
OMAP3 support unless _all_ register accesses were structure based,
single instance or not. He gave his reasons but they didn't look
convincing to me (personal humble opinion only), CCed him for a
possible comments or reminder 
Khasim Syed Mohammed said the following on 01/09/2010 09:16 PM:
Khasim Syed Mohammed said the following on 01/08/2010 09:00 PM:
From 239c47a4180fb4d5b5217f892955524d476916cf Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>[...]
The recomendation here is to move from #defines to struct based register
usage. I am ok with the rest(except for need to split).Split is done, posted yesterday.
Struct based register needs more comments, not that I am lazy to
implement that. I need to know the reason for doing the same when no
multiple instances are used.You can add a new panel or a new tv standard with these structures
easily. Structures are not used for register accesses.here is what I think:
venc_config {
}if it is organized as the register definitions,
configure_venc(struct venc_config *values)
struct venc_config * d = BASE_ADDRESS_OF_OMAP3_VENC;
writel(values->regx, &d->regx);refer: drivers/mtd/nand/omap_gpmc.c
GPIO, GPMC and other controllers have multiple instances in OMAP, it
makes sense to organize such register set in structure mode. I did
start with that but found no use for DSS as it is just one instance.
Structures don't give any value here.there were other reasons mentioned when nand got split -> one of them had
to
do with the compiler or something. Dirk might remember - unfortunately,
this
was more than a year back.. if I recollect right..Will try doing a google. May be some one can point me to that
decision. It would help developing drivers which have single instance
of controller being used.the reference I got:
http://old.nabble.com/-U-Boot---PATCH-08-13-v4--ARM%3A-OMAP3%3A-Add-NAND-support-tp20039673p20039673.htmlsimilar changes happend for GPMC etc..
Quote:
>Is GPMC_BASE an integer or a pointer?
Nothing. A macro:
#define OMAP34XX_GPMC_BASE (0x6E000000)
#define GPMC_BASE (OMAP34XX_GPMC_BASE)So it's an integer.
It's then casted to volatile pointer by ARM's readx/writex.
The cast should be done by the driver, or you'll get warnings if
readx/writex ever become inline functions (as they are on other arches).might help explain..
This is a valid comment, many thanks for digging this out. Considering
this comment, my dss_read_reg and dss_write_reg should become as shown
below+static inline void dss_write_reg(int reg, u32 val)
+{
+ __raw_writel(val, (uint32_t *) reg);
+}
+
+static inline u32 dss_read_reg(int reg)
+{
+ u32 l = __raw_readl((uint32_t *) reg);
+ return l;
+}I will do the above changes and re-submit the patch.
But Kindly NOTE: This still doesn't give me a reason to implement the
register definition as structures when we have single instance of
register set. I am still not considering the structure based
read/write currently.IIRC the main reason was that Wolfgang would refuse to merge initial
OMAP3 support unless _all_ register accesses were structure based,
single instance or not. He gave his reasons but they didn't look
convincing to me (personal humble opinion only), CCed him for a
possible comments or reminder
oh ok, then I don't want to waste time waiting. I will change them to
structures and repost.
Regards,
Khasim