| Message ID | 1344636096-11669-3-git-send-email-timur@freescale.com |
|---|---|
| State | Superseded |
| Headers | show |
please don't post patches upstream with '[u-boot-release]' in the subject. On Fri, 10 Aug 2012 17:01:33 -0500 Timur Tabi <timur@freescale.com> wrote: > +enum srds_prtcl serdes_device_from_fm_port(enum fm_port port) > +{ > + switch (port) { > + case FM1_DTSEC1: > + return SGMII_FM1_DTSEC1; > + case FM1_DTSEC2: > + return SGMII_FM1_DTSEC2; > + case FM1_DTSEC3: > + return SGMII_FM1_DTSEC3; > + case FM1_DTSEC4: > + return SGMII_FM1_DTSEC4; > + case FM1_DTSEC5: > + return SGMII_FM1_DTSEC5; > + case FM1_10GEC1: > + return XAUI_FM1; > + case FM2_DTSEC1: > + return SGMII_FM2_DTSEC1; > + case FM2_DTSEC2: > + return SGMII_FM2_DTSEC2; > + case FM2_DTSEC3: > + return SGMII_FM2_DTSEC3; > + case FM2_DTSEC4: > + return SGMII_FM2_DTSEC4; > + case FM2_DTSEC5: > + return SGMII_FM2_DTSEC5; > + case FM2_10GEC1: > + return XAUI_FM2; > + default: > + return NONE; > + } > +} shouldn't this be a static const array lookup? Kim
Kim Phillips wrote: > please don't post patches upstream with '[u-boot-release]' in the > subject. I didn't. http://lists.denx.de/pipermail/u-boot/2012-August/130618.html Your mailer is confused. I bcc the u-boot-release mailing list, and I presume your mailer (or our mail server) sent you that copy of the mail instead of the "real" one. > shouldn't this be a static const array lookup? The compiler should convert it into an array lookup automatically, but I can change it if you insist. Since I don't like writing code that depends on the values of an enum, the array will look like this: static const enum srds_prtcl srds_table[] = { [FM1_DTSEC1] = SGMII_FM1_DTSEC1, [FM1_DTSEC2] = SGMII_FM1_DTSEC2, [FM1_DTSEC3] = SGMII_FM1_DTSEC3, [FM1_DTSEC4] = SGMII_FM1_DTSEC4, [FM1_DTSEC5] = SGMII_FM1_DTSEC5, [FM1_10GEC1] = XAUI_FM1, [FM2_DTSEC1] = SGMII_FM2_DTSEC1, [FM2_DTSEC2] = SGMII_FM2_DTSEC2, [FM2_DTSEC3] = SGMII_FM2_DTSEC3, [FM2_DTSEC4] = SGMII_FM2_DTSEC4, [FM2_DTSEC5] = SGMII_FM2_DTSEC5, [FM2_10GEC1] = XAUI_FM2, }; if ((port < FM1_DTSEC1) || (port > FM2_10GEC1)) return NONE; else return srds_table[port]; I'm not sure that's an improvement.
On Mon, 13 Aug 2012 16:22:01 -0500 Timur Tabi <timur@freescale.com> wrote: > Kim Phillips wrote: > > shouldn't this be a static const array lookup? > > The compiler should convert it into an array lookup automatically, but I > can change it if you insist. Since I don't like writing code that depends > on the values of an enum, the array will look like this: > > static const enum srds_prtcl srds_table[] = { > [FM1_DTSEC1] = SGMII_FM1_DTSEC1, > [FM1_DTSEC2] = SGMII_FM1_DTSEC2, > [FM1_DTSEC3] = SGMII_FM1_DTSEC3, > [FM1_DTSEC4] = SGMII_FM1_DTSEC4, > [FM1_DTSEC5] = SGMII_FM1_DTSEC5, > [FM1_10GEC1] = XAUI_FM1, > [FM2_DTSEC1] = SGMII_FM2_DTSEC1, > [FM2_DTSEC2] = SGMII_FM2_DTSEC2, > [FM2_DTSEC3] = SGMII_FM2_DTSEC3, > [FM2_DTSEC4] = SGMII_FM2_DTSEC4, > [FM2_DTSEC5] = SGMII_FM2_DTSEC5, > [FM2_10GEC1] = XAUI_FM2, > }; > > if ((port < FM1_DTSEC1) || (port > FM2_10GEC1)) > return NONE; > else > return srds_table[port]; > > I'm not sure that's an improvement. it's ~30% less lines of code, and the maps lie on the same line, which helps when grepping. Kim
Kim Phillips wrote: > it's ~30% less lines of code, and the maps lie on the same line, > which helps when grepping. OK.
diff --git a/board/freescale/common/fman.c b/board/freescale/common/fman.c index 6ddf816..58b7c44 100644 --- a/board/freescale/common/fman.c +++ b/board/freescale/common/fman.c @@ -25,6 +25,9 @@ #include <libfdt_env.h> #include <fdt_support.h> +#include <fm_eth.h> +#include <asm/fsl_serdes.h> + /* * Given the following ... * @@ -67,3 +70,40 @@ int fdt_set_phy_handle(void *fdt, char *compat, phys_addr_t addr, return fdt_setprop(fdt, offset, "phy-handle", &ph, sizeof(ph)); } + +/* + * Return the SerDes device enum for a given Fman port + * + * This function just maps the fm_port namespace to the srds_prtcl namespace. + */ +enum srds_prtcl serdes_device_from_fm_port(enum fm_port port) +{ + switch (port) { + case FM1_DTSEC1: + return SGMII_FM1_DTSEC1; + case FM1_DTSEC2: + return SGMII_FM1_DTSEC2; + case FM1_DTSEC3: + return SGMII_FM1_DTSEC3; + case FM1_DTSEC4: + return SGMII_FM1_DTSEC4; + case FM1_DTSEC5: + return SGMII_FM1_DTSEC5; + case FM1_10GEC1: + return XAUI_FM1; + case FM2_DTSEC1: + return SGMII_FM2_DTSEC1; + case FM2_DTSEC2: + return SGMII_FM2_DTSEC2; + case FM2_DTSEC3: + return SGMII_FM2_DTSEC3; + case FM2_DTSEC4: + return SGMII_FM2_DTSEC4; + case FM2_DTSEC5: + return SGMII_FM2_DTSEC5; + case FM2_10GEC1: + return XAUI_FM2; + default: + return NONE; + } +} diff --git a/board/freescale/common/fman.h b/board/freescale/common/fman.h index d39ef08..734b1da 100644 --- a/board/freescale/common/fman.h +++ b/board/freescale/common/fman.h @@ -23,4 +23,6 @@ int fdt_set_phy_handle(void *fdt, char *compat, phys_addr_t addr, const char *alias); +enum srds_prtcl serdes_device_from_fm_port(enum fm_port port); + #endif
In order to figure out which SerDes lane a given Fman port is connected to, we need a function that maps the fm_port namespace to the srds_prtcl namespace. Signed-off-by: Timur Tabi <timur@freescale.com> --- board/freescale/common/fman.c | 40 ++++++++++++++++++++++++++++++++++++++++ board/freescale/common/fman.h | 2 ++ 2 files changed, 42 insertions(+), 0 deletions(-)