Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
Hi David,
David Miller wrote:
From: Anatolij Gustschin ag...@denx.de
Date: Tue, 9 Feb 2010 15:23:17 +0100
In my understanding, in the ESP scsi driver the set of defines for
the register offsets is common for all chip drivers. The chip driver
methods for register access translate the offsets because the
registers on some chips are at different intervals (4-byte, 1-byte,
16-byte for mac_esp.c). But the register order is the same for
different chips.
In our case non only the register order is not the same for 8xx
FEC and 5121 FEC, but there are also other differences, different
reserved areas between several registers, some registers are
available only on 8xx and some only on 5121.
That only means you would need to use a table based register address
translation scheme, rather than a simple calculation. Something
like:
static unsigned int chip_xxx_table[] =
{
[GENERIC_REG_FOO]= CHIP_XXX_FOO,
...
};
static u32 chip_xxx_read_reg(struct chip *p, unsigned int reg)
{
unsigned int reg_off = chip_xxx_table[reg];
return readl(p-regs + reg_off);
}
And this table can have special tokens in entries for
registers which do not exist on a chip, so you can trap
attempted access to them in these read/write handlers.
Yes, that could be done, but to honest, I do not see any improvement in
respect to the previous patch where the register offset were defined via
pointers within a structure.
Please stop looking for excuses to fork this driver, a
unified driver I think can be done cleanly.
Other people suggested to fork the driver because it's getting too ugly.
Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
Wolfgang Grandegger wrote:
Hi David,
David Miller wrote:
From: Anatolij Gustschin ag...@denx.de
Date: Tue, 9 Feb 2010 15:23:17 +0100
In my understanding, in the ESP scsi driver the set of defines for
the register offsets is common for all chip drivers. The chip driver
methods for register access translate the offsets because the
registers on some chips are at different intervals (4-byte, 1-byte,
16-byte for mac_esp.c). But the register order is the same for
different chips.
In our case non only the register order is not the same for 8xx
FEC and 5121 FEC, but there are also other differences, different
reserved areas between several registers, some registers are
available only on 8xx and some only on 5121.
That only means you would need to use a table based register address
translation scheme, rather than a simple calculation. Something
like:
static unsigned int chip_xxx_table[] =
{
[GENERIC_REG_FOO]= CHIP_XXX_FOO,
...
};
static u32 chip_xxx_read_reg(struct chip *p, unsigned int reg)
{
unsigned int reg_off = chip_xxx_table[reg];
return readl(p-regs + reg_off);
}
And this table can have special tokens in entries for
registers which do not exist on a chip, so you can trap
attempted access to them in these read/write handlers.
Yes, that could be done, but to honest, I do not see any improvement in
respect to the previous patch where the register offset were defined via
pointers within a structure.
Please stop looking for excuses to fork this driver, a
unified driver I think can be done cleanly.
Other people suggested to fork the driver because it's getting too ugly.
That said, I think there is consensus that it does not make sense, and
it's even not possible, to provide a kernel image which runs on both,
the 8xx and the mpc512x. Therefore, there is also no need for sharing
this driver at run time. Compile time selection would allow a more
elegant and transparent implementation. Would that be an acceptable
solution?
Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
On Thu, 21 Jan 2010 18:03:11 -0800 (PST) David Miller da...@davemloft.net wrote: From: Wolfgang Grandegger w...@grandegger.com Date: Thu, 21 Jan 2010 16:25:38 +0100 Do you see a more clever solution to this problem? See how we handle this in the ESP scsi driver. We have a set of defines for the register offsets, and a set of methods a chip driver implements for register accesses. If the offsets differ, the register access method can translate the generic register offsets into whatever layout their implementation actually uses. First of all thanks for your suggestion. I have seen how you handle register access in the ESP scsi driver. The reason I didn't try to implement register access using similar approach is that we have different sort of problem. In my understanding, in the ESP scsi driver the set of defines for the register offsets is common for all chip drivers. The chip driver methods for register access translate the offsets because the registers on some chips are at different intervals (4-byte, 1-byte, 16-byte for mac_esp.c). But the register order is the same for different chips. In our case non only the register order is not the same for 8xx FEC and 5121 FEC, but there are also other differences, different reserved areas between several registers, some registers are available only on 8xx and some only on 5121. Now at least tree people suggested to fork the driver. My question is if you would accept a forked 5121 FEC specific driver realised similar to drivers/net/fs_enet/mac-fec.c and drivers/net/fs_enet/mii-fec.c drivers? Thanks, Anatolij ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
From: Anatolij Gustschin ag...@denx.de
Date: Tue, 9 Feb 2010 15:23:17 +0100
In my understanding, in the ESP scsi driver the set of defines for
the register offsets is common for all chip drivers. The chip driver
methods for register access translate the offsets because the
registers on some chips are at different intervals (4-byte, 1-byte,
16-byte for mac_esp.c). But the register order is the same for
different chips.
In our case non only the register order is not the same for 8xx
FEC and 5121 FEC, but there are also other differences, different
reserved areas between several registers, some registers are
available only on 8xx and some only on 5121.
That only means you would need to use a table based register address
translation scheme, rather than a simple calculation. Something
like:
static unsigned int chip_xxx_table[] =
{
[GENERIC_REG_FOO]= CHIP_XXX_FOO,
...
};
static u32 chip_xxx_read_reg(struct chip *p, unsigned int reg)
{
unsigned int reg_off = chip_xxx_table[reg];
return readl(p-regs + reg_off);
}
And this table can have special tokens in entries for
registers which do not exist on a chip, so you can trap
attempted access to them in these read/write handlers.
Please stop looking for excuses to fork this driver, a
unified driver I think can be done cleanly.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
Arnd Bergmann wrote: On Sunday 24 January 2010, Wolfgang Denk wrote: In message 4b5c5bdf.6020...@grandegger.com you wrote: You are probably right and your proposal would likely result in more transparent (less ugly) code. There has been some discussion about unifying FEC drivers when the patches (with the same subject) have been submitted for the first time in May last year, but it was not about 512x and 8xx, IIRC. You can re-read this discussion here: http://patchwork.ozlabs.org/patch/26927/ ee especiall Grant's note of 2009-05-21 15:36:11: If it looks too ugly, then just fork the driver. Ok. I fully agree with what Grant said in that thread, especially the way the files could be split. Forking the entire driver would work as an easy way to get it running at first, and we still have the option of reorganizing the duplicate parts later in a saner way if that's seen as helpful. I'd assume that at least some parts of it could become a lib_fs_enet module that can be shared by all of them. Yes, I also vote for forking the driver allowing a clean implementation. I don't think it makes sense to share a driver with the 8xx for the reasons you already mentioned. And the 8xx is a dying out arch anyway. Wolfgang. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
On Sunday 24 January 2010, Wolfgang Denk wrote: In message 4b5c5bdf.6020...@grandegger.com you wrote: You are probably right and your proposal would likely result in more transparent (less ugly) code. There has been some discussion about unifying FEC drivers when the patches (with the same subject) have been submitted for the first time in May last year, but it was not about 512x and 8xx, IIRC. You can re-read this discussion here: http://patchwork.ozlabs.org/patch/26927/ ee especiall Grant's note of 2009-05-21 15:36:11: If it looks too ugly, then just fork the driver. Ok. I fully agree with what Grant said in that thread, especially the way the files could be split. Forking the entire driver would work as an easy way to get it running at first, and we still have the option of reorganizing the duplicate parts later in a saner way if that's seen as helpful. I'd assume that at least some parts of it could become a lib_fs_enet module that can be shared by all of them. Arnd ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
Arnd Bergmann wrote:
On Thursday 21 January 2010, Wolfgang Grandegger wrote:
The major problem that Anatolij tries to solve are the different
register layouts of the supported SOCs, MPC52xx and MPC8xx. They use the
same registers but at different offsets. Therefore we cannot handle
this with a single fec_t struct to allow building a single kernel
image. Instead it's handled by filling a table with register addresses:
if (of_device_is_compatible(ofdev-node, fsl,mpc5121-fec)) {
fep-fec.fec_id = FS_ENET_MPC5121_FEC;
fec_reg_mpc5121(ievent);
fec_reg_mpc5121(imask);
...
} else {
fec_reg_mpc8xx(ievent);
fec_reg_mpc8xx(imask);
...
}
Do you see a more clever solution to this problem? Nevertheless, the
code could be improved by using offsetof, I think.
Is there any chance of building a kernel that runs on both mpc8xx and
mpc5121? AFAIK, the 5121 is built on a 6xx core which is fundamentally
incompatible with 8xx due to different memory management etc.
Since this makes it all a compile-time decision, it should be solvable
with a very small number of carefully placed #ifdef in the header files
an no runtime detection at all.
Obviously this approach would not work for drivers that want to be portable
across different register layouts on otherwise compatible platforms.
You are probably right and your proposal would likely result in more
transparent (less ugly) code. There has been some discussion about
unifying FEC drivers when the patches (with the same subject) have been
submitted for the first time in May last year, but it was not about 512x
and 8xx, IIRC.
Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
Dear Wolfgang Arnd, In message 4b5c5bdf.6020...@grandegger.com you wrote: Arnd Bergmann wrote: ... Is there any chance of building a kernel that runs on both mpc8xx and mpc5121? AFAIK, the 5121 is built on a 6xx core which is fundamentally incompatible with 8xx due to different memory management etc. It is my understanding as well that you cannot have a single image that boots both on 8xx and on 6xx cores. The focus was more on things like supporting MPC5200 and MPC512x with the same image. Since this makes it all a compile-time decision, it should be solvable with a very small number of carefully placed #ifdef in the header files an no runtime detection at all. Obviously this approach would not work for drivers that want to be portable across different register layouts on otherwise compatible platforms. You are probably right and your proposal would likely result in more transparent (less ugly) code. There has been some discussion about unifying FEC drivers when the patches (with the same subject) have been submitted for the first time in May last year, but it was not about 512x and 8xx, IIRC. You can re-read this discussion here: http://patchwork.ozlabs.org/patch/26927/ ee especiall Grant's note of 2009-05-21 15:36:11: If it looks too ugly, then just fork the driver. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Immortality consists largely of boredom. -- Zefrem Cochrane, Metamorphosis, stardate 3219.8 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
On Thursday 21 January 2010, Wolfgang Grandegger wrote:
The major problem that Anatolij tries to solve are the different
register layouts of the supported SOCs, MPC52xx and MPC8xx. They use the
same registers but at different offsets. Therefore we cannot handle
this with a single fec_t struct to allow building a single kernel
image. Instead it's handled by filling a table with register addresses:
if (of_device_is_compatible(ofdev-node, fsl,mpc5121-fec)) {
fep-fec.fec_id = FS_ENET_MPC5121_FEC;
fec_reg_mpc5121(ievent);
fec_reg_mpc5121(imask);
...
} else {
fec_reg_mpc8xx(ievent);
fec_reg_mpc8xx(imask);
...
}
Do you see a more clever solution to this problem? Nevertheless, the
code could be improved by using offsetof, I think.
Is there any chance of building a kernel that runs on both mpc8xx and
mpc5121? AFAIK, the 5121 is built on a 6xx core which is fundamentally
incompatible with 8xx due to different memory management etc.
Since this makes it all a compile-time decision, it should be solvable
with a very small number of carefully placed #ifdef in the header files
an no runtime detection at all.
Obviously this approach would not work for drivers that want to be portable
across different register layouts on otherwise compatible platforms.
Arnd
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
David Miller wrote: From: Wolfgang Grandegger w...@grandegger.com Date: Thu, 21 Jan 2010 16:25:38 +0100 Do you see a more clever solution to this problem? See how we handle this in the ESP scsi driver. We have a set of defines for the register offsets, and a set of methods a chip driver implements for register accesses. If the offsets differ, the register access method can translate the generic register offsets into whatever layout their implementation actually uses. I think you speak about: void (*esp_write8)(struct esp *esp, u8 val, unsigned long reg); u8 (*esp_read8)(struct esp *esp, unsigned long reg); But still we need to translate the *generic* offset (reg) into the real offset, which requires a lookup/table to get it. For me this seems not really more efficient and less transparent as it bends the offsets. Wolfgang. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
From: Anatolij Gustschin ag...@denx.de
Date: Thu, 21 Jan 2010 03:13:18 +0100
struct fec_info {
- fec_t __iomem *fecp;
+ void __iomem *fecp;
...
/* write */
-#define FW(_fecp, _reg, _v) __fs_out32((_fecp)-fec_ ## _reg, (_v))
+#define FW(_regp, _reg, _v) __fs_out32((_regp)-fec_ ## _reg, (_v))
...
+/* register address macros */
+#define fec_reg_addr(_type, _reg) \
+ (fep-fec.rtbl-fec_##_reg = (u32 __iomem *)((u32)fep-fec.fecp + \
+ (u32)((__typeof__(_type) *)NULL)-fec_##_reg))
+
+#define fec_reg_mpc8xx(_reg) \
+ fec_reg_addr(struct mpc8xx_fec, _reg)
+
+#define fec_reg_mpc5121(_reg) \
+ fec_reg_addr(struct mpc5121_fec, _reg)
This is a step backwards in my view.
If you use the fec_t __iomem * type for the register
pointer, you simply use p-fecp-XXX to get the I/O
address of register XXX and that's what you pass to
the appropriate I/O accessor routines.
Now you've made it typeless, and then you have to walk
through all of these contortions to get the offset.
I don't want to apply this, sorry...
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
On Thu, 21 Jan 2010 01:22:35 -0800 (PST)
David Miller da...@davemloft.net wrote:
From: Anatolij Gustschin ag...@denx.de
Date: Thu, 21 Jan 2010 03:13:18 +0100
struct fec_info {
- fec_t __iomem *fecp;
+ void __iomem *fecp;
...
/* write */
-#define FW(_fecp, _reg, _v) __fs_out32((_fecp)-fec_ ## _reg, (_v))
+#define FW(_regp, _reg, _v) __fs_out32((_regp)-fec_ ## _reg, (_v))
...
+/* register address macros */
+#define fec_reg_addr(_type, _reg) \
+ (fep-fec.rtbl-fec_##_reg = (u32 __iomem *)((u32)fep-fec.fecp + \
+ (u32)((__typeof__(_type) *)NULL)-fec_##_reg))
+
+#define fec_reg_mpc8xx(_reg) \
+ fec_reg_addr(struct mpc8xx_fec, _reg)
+
+#define fec_reg_mpc5121(_reg) \
+ fec_reg_addr(struct mpc5121_fec, _reg)
This is a step backwards in my view.
If you use the fec_t __iomem * type for the register
pointer, you simply use p-fecp-XXX to get the I/O
address of register XXX and that's what you pass to
the appropriate I/O accessor routines.
Now you've made it typeless, and then you have to walk
through all of these contortions to get the offset.
OK, i give up
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
David Miller wrote:
From: Anatolij Gustschin ag...@denx.de
Date: Thu, 21 Jan 2010 03:13:18 +0100
struct fec_info {
-fec_t __iomem *fecp;
+void __iomem *fecp;
To avoid confusion, the name base_addr seems more appropriate as it's
just used to calculate register offsets and for iomap/unmap.
...
/* write */
-#define FW(_fecp, _reg, _v) __fs_out32((_fecp)-fec_ ## _reg, (_v))
+#define FW(_regp, _reg, _v) __fs_out32((_regp)-fec_ ## _reg, (_v))
...
+/* register address macros */
+#define fec_reg_addr(_type, _reg) \
+(fep-fec.rtbl-fec_##_reg = (u32 __iomem *)((u32)fep-fec.fecp + \
+(u32)((__typeof__(_type) *)NULL)-fec_##_reg))
+
+#define fec_reg_mpc8xx(_reg) \
+fec_reg_addr(struct mpc8xx_fec, _reg)
+
+#define fec_reg_mpc5121(_reg) \
+fec_reg_addr(struct mpc5121_fec, _reg)
This is a step backwards in my view.
If you use the fec_t __iomem * type for the register
pointer, you simply use p-fecp-XXX to get the I/O
address of register XXX and that's what you pass to
the appropriate I/O accessor routines.
Now you've made it typeless, and then you have to walk
through all of these contortions to get the offset.
I don't want to apply this, sorry...
The major problem that Anatolij tries to solve are the different
register layouts of the supported SOCs, MPC52xx and MPC8xx. They use the
same registers but at *different* offsets. Therefore we cannot handle
this with a single fec_t struct to allow building a single kernel
image. Instead it's handled by filling a table with register addresses:
if (of_device_is_compatible(ofdev-node, fsl,mpc5121-fec)) {
fep-fec.fec_id = FS_ENET_MPC5121_FEC;
fec_reg_mpc5121(ievent);
fec_reg_mpc5121(imask);
...
} else {
fec_reg_mpc8xx(ievent);
fec_reg_mpc8xx(imask);
...
}
Do you see a more clever solution to this problem? Nevertheless, the
code could be improved by using offsetof, I think.
Wolfgang.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
Hi Anatolij,
I had a close look...
Anatolij Gustschin wrote:
drivers/net/fs_enet/*
Enable fs_enet driver to work 5121 FEC
Enable it with CONFIG_FS_ENET_MPC5121_FEC
Signed-off-by: John Rigby jcri...@gmail.com
Signed-off-by: Piotr Ziecik ko...@semihalf.com
Signed-off-by: Wolfgang Denk w...@denx.de
Signed-off-by: Anatolij Gustschin ag...@denx.de
Cc: linuxppc-...@ozlabs.org
Cc: Grant Likely grant.lik...@secretlab.ca
---
Changes since previous submited version:
- explicit type usage in register tables.
- don't use same variable name fecp for variables of
different types.
- avoid re-checking the compatible by passing data pointer
in the match struct.
drivers/net/fs_enet/Kconfig| 10 +-
drivers/net/fs_enet/fs_enet-main.c |4 +
drivers/net/fs_enet/fs_enet.h | 40 +++-
drivers/net/fs_enet/mac-fec.c | 212
+---
drivers/net/fs_enet/mii-fec.c | 76 ++---
drivers/net/fs_enet/mpc5121_fec.h | 64 +++
drivers/net/fs_enet/mpc8xx_fec.h | 37 ++
7 files changed, 356 insertions(+), 87 deletions(-)
create mode 100644 drivers/net/fs_enet/mpc5121_fec.h
create mode 100644 drivers/net/fs_enet/mpc8xx_fec.h
diff --git a/drivers/net/fs_enet/Kconfig b/drivers/net/fs_enet/Kconfig
index 562ea68..fc073b5 100644
--- a/drivers/net/fs_enet/Kconfig
+++ b/drivers/net/fs_enet/Kconfig
@@ -1,9 +1,13 @@
config FS_ENET
tristate Freescale Ethernet Driver
- depends on CPM1 || CPM2
+ depends on CPM1 || CPM2 || PPC_MPC512x
select MII
select PHYLIB
+config FS_ENET_MPC5121_FEC
+ def_bool y if (FS_ENET PPC_MPC512x)
+ select FS_ENET_HAS_FEC
+
config FS_ENET_HAS_SCC
bool Chip has an SCC usable for ethernet
depends on FS_ENET (CPM1 || CPM2)
@@ -16,13 +20,13 @@ config FS_ENET_HAS_FCC
config FS_ENET_HAS_FEC
bool Chip has an FEC usable for ethernet
- depends on FS_ENET CPM1
+ depends on FS_ENET (CPM1 || FS_ENET_MPC5121_FEC)
select FS_ENET_MDIO_FEC
default y
config FS_ENET_MDIO_FEC
tristate MDIO driver for FEC
- depends on FS_ENET CPM1
+ depends on FS_ENET (CPM1 || FS_ENET_MPC5121_FEC)
config FS_ENET_MDIO_FCC
tristate MDIO driver for FCC
diff --git a/drivers/net/fs_enet/fs_enet-main.c
b/drivers/net/fs_enet/fs_enet-main.c
index c34a7e0..6bce5c8 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -1095,6 +1095,10 @@ static struct of_device_id fs_enet_match[] = {
#endif
#ifdef CONFIG_FS_ENET_HAS_FEC
{
+ .compatible = fsl,mpc5121-fec,
+ .data = (void *)fs_fec_ops,
+ },
+ {
.compatible = fsl,pq1-fec-enet,
.data = (void *)fs_fec_ops,
},
diff --git a/drivers/net/fs_enet/fs_enet.h b/drivers/net/fs_enet/fs_enet.h
index ef01e09..df935e8 100644
--- a/drivers/net/fs_enet/fs_enet.h
+++ b/drivers/net/fs_enet/fs_enet.h
@@ -13,11 +13,47 @@
#ifdef CONFIG_CPM1
#include asm/cpm1.h
+#endif
+
+#if defined(CONFIG_FS_ENET_HAS_FEC)
+#include asm/cpm.h
+#include mpc8xx_fec.h
+#include mpc5121_fec.h
Do we really need the new header files? Why not adding the struct
definitions here or use struct fec from 8xx_immap.h. See below.
struct fec_info {
- fec_t __iomem *fecp;
+ void __iomem *fecp;
A name like fec_base or base_addr would help to avoid confusion with a
pointer to the old fec struct.
+ u32 __iomem *fec_r_cntrl;
+ u32 __iomem *fec_ecntrl;
+ u32 __iomem *fec_ievent;
+ u32 __iomem *fec_mii_data;
+ u32 __iomem *fec_mii_speed;
u32 mii_speed;
};
+
+struct reg_tbl {
A more specific name would be nice, e.g. fec_reg_tbl or fec_regs.
+ u32 __iomem *fec_ievent;
+ u32 __iomem *fec_imask;
+ u32 __iomem *fec_r_des_active;
+ u32 __iomem *fec_x_des_active;
+ u32 __iomem *fec_r_des_start;
+ u32 __iomem *fec_x_des_start;
+ u32 __iomem *fec_r_cntrl;
+ u32 __iomem *fec_ecntrl;
+ u32 __iomem *fec_ivec;
+ u32 __iomem *fec_mii_speed;
+ u32 __iomem *fec_addr_low;
+ u32 __iomem *fec_addr_high;
+ u32 __iomem *fec_hash_table_high;
+ u32 __iomem *fec_hash_table_low;
+ u32 __iomem *fec_r_buff_size;
+ u32 __iomem *fec_r_bound;
+ u32 __iomem *fec_r_fstart;
+ u32 __iomem *fec_x_fstart;
+ u32 __iomem *fec_fun_code;
+ u32 __iomem *fec_r_hash;
+ u32 __iomem *fec_x_cntrl;
+ u32 __iomem *fec_dma_control;
+};
#endif
#ifdef CONFIG_CPM2
@@ -113,7 +149,9 @@ struct fs_enet_private {
struct {
int idx;/* FEC1 = 0, FEC2 = 1 */
void __iomem *fecp; /* hw registers*/
See above.
+ struct reg_tbl *rtbl; /* used registers table */
u32 hthi, htlo; /* state for
Re: [net-next-2.6 PATCH 2/3] fs_enet: Add support for MPC512x to fs_enet driver
From: Wolfgang Grandegger w...@grandegger.com Date: Thu, 21 Jan 2010 16:25:38 +0100 Do you see a more clever solution to this problem? See how we handle this in the ESP scsi driver. We have a set of defines for the register offsets, and a set of methods a chip driver implements for register accesses. If the offsets differ, the register access method can translate the generic register offsets into whatever layout their implementation actually uses. Nevertheless, the code could be improved by using offsetof, I think. At a minimum :-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
