Hi Joe,
-----Original Message----- From: Joe Hershberger [mailto:joe.hershberger@ni.com] Sent: Wednesday, October 10, 2018 9:29 AM To: Pankaj Bansal pankaj.bansal@nxp.com Cc: Joseph Hershberger joseph.hershberger@ni.com; u-boot <u- boot@lists.denx.de> Subject: Re: [U-Boot] [PATCH v3 2/6] driver: net: fsl-mc: remove unused strcture elements
On Tue, Oct 9, 2018 at 9:59 PM Pankaj Bansal pankaj.bansal@nxp.com wrote:
The phydev structure is present in both ldpaa_eth_priv and wriop_dpmac_info. the phydev in wriop_dpmac_info is not being used
As the phydev is created based on phy_addr and bus members of wriop_dpmac_info, it is appropriate to keep phydev in
wriop_dpmac_info.
Also phy_regs is not being used, therefore remove it
Signed-off-by: Pankaj Bansal pankaj.bansal@nxp.com Acked-by: Joe Hershberger joe.hershberger@ni.com
Notes: V3: - No change
Please be sure you are running scripts/checkpatch.pl on your patches. v2 had a number of issues I had to fix up. I'm pretty sure this was one of them.
I had run checkpatch script on all the versions of patches I sent. I use this command "./scripts/checkpatch.pl 0001-something.patch" This "no return at the end of function" issue was not reported by checkpatch script.
Can you please tell me which issues in V2 are you referring to? Because when I ran checkpatch.pl, it gave me no errors or warnings but 7 checks regarding alignment in board/freescale/ls2080aqds/eth.c. I did not do any changes for that because that code was not part of my patch and I think that was done so that line doesn't exceed 80 characters.
You would do yourself a favor to use tools/patman/patman.
This is a good advice. I will use patman from now onwards to prepare and send patches.
V2: - change (phydev && bus != NULL) to (phydev && bus) - after free phydev just pass NULL into wriop_set_phy_dev()drivers/net/ldpaa_eth/ldpaa_eth.c | 56 +++++++++++++++------------ drivers/net/ldpaa_eth/ldpaa_eth.h | 1 - drivers/net/ldpaa_eth/ldpaa_wriop.c | 2 + include/fsl-mc/ldpaa_wriop.h | 1 - 4 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.c b/drivers/net/ldpaa_eth/ldpaa_eth.c index 82a684bea2..ca3459cc33 100644 --- a/drivers/net/ldpaa_eth/ldpaa_eth.c +++ b/drivers/net/ldpaa_eth/ldpaa_eth.c @@ -35,7 +35,7 @@ static int init_phy(struct eth_device *dev) return -1; }
priv->phydev = phydev;
wriop_set_phy_dev(priv->dpmac_id, phydev); return phy_config(phydev);} @@ -388,6 +388,7 @@ static int ldpaa_eth_open(struct eth_device
*net_dev, bd_t *bd)
struct mii_dev *bus; phy_interface_t enet_if; struct dpni_queue d_queue;
struct phy_device *phydev = NULL; if (net_dev->state == ETH_STATE_ACTIVE) return 0;@@ -408,38 +409,41 @@ static int ldpaa_eth_open(struct eth_device
*net_dev, bd_t *bd)
goto err_dpmac_setup;#ifdef CONFIG_PHYLIB
if (priv->phydev) {err = phy_startup(priv->phydev);
phydev = wriop_get_phy_dev(priv->dpmac_id);if (phydev) {err = phy_startup(phydev); if (err) { printf("%s: Could not initialize\n",
priv->phydev->dev->name);
phydev->dev->name); goto err_dpmac_bind; } }#else
priv->phydev = (struct phy_device *)malloc(sizeof(structphy_device));
memset(priv->phydev, 0, sizeof(struct phy_device));
phydev = (struct phy_device *)malloc(sizeof(struct phy_device));memset(phydev, 0, sizeof(struct phy_device));wriop_set_phy_dev(priv->dpmac_id, phydev);
priv->phydev->speed = SPEED_1000;priv->phydev->link = 1;priv->phydev->duplex = DUPLEX_FULL;
phydev->speed = SPEED_1000;phydev->link = 1;phydev->duplex = DUPLEX_FULL;#endif
bus = wriop_get_mdio(priv->dpmac_id); enet_if = wriop_get_enet_if(priv->dpmac_id); if ((bus == NULL) && (enet_if == PHY_INTERFACE_MODE_XGMII)) {
priv->phydev = (struct phy_device *)
phydev = (struct phy_device *) malloc(sizeof(struct phy_device));
memset(priv->phydev, 0, sizeof(struct phy_device));
memset(phydev, 0, sizeof(struct phy_device));wriop_set_phy_dev(priv->dpmac_id, phydev);
priv->phydev->speed = SPEED_10000;priv->phydev->link = 1;priv->phydev->duplex = DUPLEX_FULL;
phydev->speed = SPEED_10000;phydev->link = 1;phydev->duplex = DUPLEX_FULL; }
if (!priv->phydev->link) {printf("%s: No link.\n", priv->phydev->dev->name);
if (!phydev->link) {printf("%s: No link.\n", phydev->dev->name); err = -1; goto err_dpmac_bind; }@@ -476,17 +480,17 @@ static int ldpaa_eth_open(struct eth_device
*net_dev, bd_t *bd)
return err; }
dpmac_link_state.rate = priv->phydev->speed;
dpmac_link_state.rate = phydev->speed;
if (priv->phydev->autoneg == AUTONEG_DISABLE)
if (phydev->autoneg == AUTONEG_DISABLE) dpmac_link_state.options &= ~DPMAC_LINK_OPT_AUTONEG; else dpmac_link_state.options |= DPMAC_LINK_OPT_AUTONEG;
if (priv->phydev->duplex == DUPLEX_HALF)
if (phydev->duplex == DUPLEX_HALF) dpmac_link_state.options |=DPMAC_LINK_OPT_HALF_DUPLEX;
dpmac_link_state.up = priv->phydev->link;
dpmac_link_state.up = phydev->link; err = dpmac_set_link_state(dflt_mc_io, MC_CMD_NO_FLAGS, priv->dpmac_handle,&dpmac_link_state); @@ -530,7 +534,7 @@ static int
ldpaa_eth_open(struct eth_device *net_dev, bd_t *bd)
goto err_qdid; }
return priv->phydev->link;
return phydev->link;err_qdid: err_get_queue: @@ -556,6 +560,7 @@ static void ldpaa_eth_stop(struct eth_device *net_dev) #ifdef CONFIG_PHYLIB struct mii_dev *bus = wriop_get_mdio(priv->dpmac_id); #endif
struct phy_device *phydev = NULL; if ((net_dev->state == ETH_STATE_PASSIVE) || (net_dev->state == ETH_STATE_INIT)) @@ -588,11 +593,12 @@static void ldpaa_eth_stop(struct eth_device *net_dev) printf("dpni_disable() failed\n");
#ifdef CONFIG_PHYLIB
if (priv->phydev && bus != NULL)phy_shutdown(priv->phydev);
phydev = wriop_get_phy_dev(priv->dpmac_id);if (phydev && bus)phy_shutdown(phydev); else {
free(priv->phydev);priv->phydev = NULL;
free(phydev);wriop_set_phy_dev(priv->dpmac_id, NULL); }#endif
diff --git a/drivers/net/ldpaa_eth/ldpaa_eth.h b/drivers/net/ldpaa_eth/ldpaa_eth.h index ee784a55ee..3f9154b5bb 100644 --- a/drivers/net/ldpaa_eth/ldpaa_eth.h +++ b/drivers/net/ldpaa_eth/ldpaa_eth.h @@ -127,7 +127,6 @@ struct ldpaa_eth_priv { uint16_t tx_flow_id;
enum ldpaa_eth_type type; /* 1G or 10G ethernet */
struct phy_device *phydev;};
struct dprc_endpoint dpmac_endpoint; diff --git a/drivers/net/ldpaa_eth/ldpaa_wriop.c b/drivers/net/ldpaa_eth/ldpaa_wriop.c index 0731a795c8..afbb1ca91e 100644 --- a/drivers/net/ldpaa_eth/ldpaa_wriop.c +++ b/drivers/net/ldpaa_eth/ldpaa_wriop.c @@ -26,6 +26,7 @@ void wriop_init_dpmac(int sd, int dpmac_id, int
lane_prtcl)
dpmac_info[dpmac_id].enabled = 0; dpmac_info[dpmac_id].id = 0; dpmac_info[dpmac_id].phy_addr = -1;
dpmac_info[dpmac_id].phydev = NULL; dpmac_info[dpmac_id].enet_if = PHY_INTERFACE_MODE_NONE; enet_if = wriop_dpmac_enet_if(dpmac_id, lane_prtcl); @@ -42,6+43,7 @@ void wriop_init_dpmac_enet_if(int dpmac_id, phy_interface_t
enet_if)
dpmac_info[dpmac_id].id = dpmac_id; dpmac_info[dpmac_id].phy_addr = -1; dpmac_info[dpmac_id].enet_if = enet_if;
dpmac_info[dpmac_id].phydev = NULL;}
diff --git a/include/fsl-mc/ldpaa_wriop.h b/include/fsl-mc/ldpaa_wriop.h index 07e5130264..8971c6c55b 100644 --- a/include/fsl-mc/ldpaa_wriop.h +++ b/include/fsl-mc/ldpaa_wriop.h @@ -41,7 +41,6 @@ struct wriop_dpmac_info { u8 id; u8 board_mux; int phy_addr;
void *phy_regs; phy_interface_t enet_if; struct phy_device *phydev; struct mii_dev *bus;-- 2.17.1
U-Boot mailing list U-Boot@lists.denx.de
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
ts.denx.de%2Flistinfo%2Fu-
boot&data=02%7C01%7Cpankaj.bansal%40nxp.
com%7Cbced1ce32f8e455148d408d62e64c7f5%7C686ea1d3bc2b4c6fa92c d99c5c301
635%7C0%7C0%7C636747407731429023&sdata=4cwaAt8C4V91TNo iNFGcoznoGEn
POsCGDmh9maFv%2FOA%3D&reserved=0