powerpc/85xx: fix null pointer dereference when initializing the SGMII TBI PHY

PATCH New Timur Tabi 3 participants 4 messages Oct 4, 2011 – Oct 7, 2011

Subsystems (2)

Network drivers PowerPC

lore

Share & tools

lore

Timur Tabi

author

2011-10-04 21:44 UTC #0 lore raw

PATCH 1/1 powerpc/85xx: fix null pointer dereference when initializing the SGMII TBI PHY

Function dtsec_configure_serdes() needs to know where the TBI PHY registers are in order to configure SGMII for proper SerDes operation.

During SGMII initialzation, fm_eth_init_mac() passing NULL for 'phyregs' when it called init_dtsec(), because it was believed that phyregs was not used. In fact, it is used by dtsec_configure_serdes() to configure the TBI PHY registers.

We also need to define the PHY registers in struct fm_mdio.

Signed-off-by Timur Tabi <[email protected]>

---

Note: I'm not sure of my change to struct fm_mdio. It works, but it doesn't smell right.

 arch/powerpc/include/asm/fsl_fman.h |    9 ++++++++-
 drivers/net/fm/dtsec.c              |    2 +-
 drivers/net/fm/eth.c                |    3 ++-
 3 files changed, 11 insertions(+), 3 deletions(-)

arch/powerpc/include/asm/fsl_fman.h +8 -1

index fddc0cc..2c0c9bc 100644
@@ -405,7 +405,14 @@ typedef struct fm_dtesc {
 } fm_dtsec_t;
 
 typedef struct fm_mdio {
-	u8	res[4*1024];
+	u8	res0[0x120];
+	u32	miimcfg;	/* MII management configuration reg */
+	u32	miimcom;	/* MII management command reg */
+	u32	miimadd;	/* MII management address reg */
+	u32	miimcon;	/* MII management control reg */
+	u32	miimstat;	/* MII management status reg  */
+	u32	miimind;	/* MII management indication reg */
+	u8	res1[0x1000 - 0x138];
 } fm_mdio_t;
 
 typedef struct fm_10gec {

drivers/net/fm/dtsec.c +1 -1

index a77ee20..7dd78f2 100644
@@ -171,7 +171,7 @@ void init_dtsec(struct fsl_enet_mac *mac, void *base,
 		void *phyregs, int max_rx_len)
 {
 	mac->base = base;
-	mac->phyregs = NULL;
+	mac->phyregs = phyregs;
 	mac->max_rx_len = max_rx_len;
 	mac->init_mac = dtsec_init_mac;
 	mac->enable_mac = dtsec_enable_mac;

drivers/net/fm/eth.c +2 -1

index 308d610..f7ed850 100644
@@ -537,6 +537,7 @@ static int fm_eth_init_mac(struct fm_eth *fm_eth, struct ccsr_fman *reg)
 	/* Get the mac registers base address */
 	if (fm_eth->type == FM_ETH_1G_E) {
 		base = &reg->mac_1g[num].fm_dtesc;
+		phyregs = &reg->mac_1g[num].fm_mdio.miimcfg;
 	} else {
 		base = &reg->mac_10g[num].fm_10gec;
 		phyregs = &reg->mac_10g[num].fm_10gec_mdio;

@@ -552,7 +553,7 @@ static int fm_eth_init_mac(struct fm_eth *fm_eth, struct ccsr_fman *reg)
 	fm_eth->mac = mac;
 
 	if (fm_eth->type == FM_ETH_1G_E)
-		init_dtsec(mac, base, NULL, MAX_RXBUF_LEN);
+		init_dtsec(mac, base, phyregs, MAX_RXBUF_LEN);
 	else
 		init_tgec(mac, base, phyregs, MAX_RXBUF_LEN);

Signature · 1 line

1.7.3.4

Kumar Gala

2011-10-04 21:50 UTC #1 lore raw

PATCH 1/1 powerpc/85xx: fix null pointer dereference when initializing the SGMII TBI PHY

Quoted 8 lines · Timur Tabi

Function dtsec_configure_serdes() needs to know where the TBI PHY registers are in order to configure SGMII for proper SerDes operation.
During SGMII initialzation, fm_eth_init_mac() passing NULL for 'phyregs' when it called init_dtsec(), because it was believed that phyregs was not used. In fact, it is used by dtsec_configure_serdes() to configure the TBI PHY registers.

Quoted 7 lines

We also need to define the PHY registers in struct fm_mdio.
Signed-off-by Timur Tabi <[email protected]>
---
Note: I'm not sure of my change to struct fm_mdio. It works, but it doesn't smell right.

What exactly is the concern?

- k

Tabi Timur-B04825

2011-10-04 21:52 UTC #2 lore raw

PATCH 1/1 powerpc/85xx: fix null pointer dereference when initializing the SGMII TBI PHY

Quoted 4 lines · Kumar Gala

Quoted 2 lines
Quoted 2 lines
Note: I'm not sure of my change to struct fm_mdio. It works, but it doesn't smell right.
What exactly is the concern?

Nothing specific, just a gut feeling.

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Kumar Gala

Applied

2011-10-07 15:06 UTC #3 lore raw

PATCH 1/1 powerpc/85xx: fix null pointer dereference when initializing the SGMII TBI PHY

In reply to Timur Tabi

Quoted 13 lines · Timur Tabi

Function dtsec_configure_serdes() needs to know where the TBI PHY registers are in order to configure SGMII for proper SerDes operation.
During SGMII initialzation, fm_eth_init_mac() passing NULL for 'phyregs' when it called init_dtsec(), because it was believed that phyregs was not used. In fact, it is used by dtsec_configure_serdes() to configure the TBI PHY registers.
We also need to define the PHY registers in struct fm_mdio.
Signed-off-by Timur Tabi <[email protected]>
---

Quoted 7 lines

Note: I'm not sure of my change to struct fm_mdio. It works, but it doesn't smell right.
arch/powerpc/include/asm/fsl_fman.h |    9 ++++++++-
drivers/net/fm/dtsec.c              |    2 +-
drivers/net/fm/eth.c                |    3 ++-
3 files changed, 11 insertions(+), 3 deletions(-)

applied to 85xx

- k