| Message ID | 20230613123826.558-1-machel@vivo.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | drivers/thunder:improve-warning-message-in-device_for_each_child_node() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On 2023/6/13 20:38, Wang Ming wrote: > In device_for_each_child_node(), it should have fwnode_handle_put() > before break to prevent stale device node references from being > left behind. > > Signed-off-by: Wang Ming <machel@vivo.com> A Fixes tag seems necessary according to the commit log, and should target the net branch using: [PATCH net] drivers/thunder: improve-warning-message-in-device_for_each_child_node() Also it seems confusing the 'improve' in the title suggest that it is not a fix. > --- > .../net/ethernet/cavium/thunder/thunder_bgx.c | 37 ++++++++++--------- > 1 file changed, 20 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > index a317feb8d..d37ee2872 100644 > --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > @@ -90,7 +90,7 @@ static const struct pci_device_id bgx_id_table[] = { > > MODULE_AUTHOR("Cavium Inc"); > MODULE_DESCRIPTION("Cavium Thunder BGX/MAC Driver"); > -MODULE_LICENSE("GPL v2"); > +MODULE_LICENSE("GPL"); Is there any reason you changing the license here? > MODULE_VERSION(DRV_VERSION); > MODULE_DEVICE_TABLE(pci, bgx_id_table); > > @@ -174,10 +174,10 @@ static struct bgx *get_bgx(int node, int bgx_idx) > } > > /* Return number of BGX present in HW */ > -unsigned bgx_get_map(int node) > +unsigned int bgx_get_map(int node) It seems to be unrelated change here, is the changing related to the problem you are fixing? > { > int i; > - unsigned map = 0; > + unsigned int map = 0; Same here. > > for (i = 0; i < max_bgx_per_node; i++) { > if (bgx_vnic[(node * max_bgx_per_node) + i]) > @@ -600,9 +600,9 @@ static void bgx_lmac_handler(struct net_device *netdev) > link_changed = -1; > > if (phydev->link && > - (lmac->last_duplex != phydev->duplex || > - lmac->last_link != phydev->link || > - lmac->last_speed != phydev->speed)) { > + (lmac->last_duplex != phydev->duplex || > + lmac->last_link != phydev->link || > + lmac->last_speed != phydev->speed)) { Same here. > link_changed = 1; > } > > @@ -783,7 +783,7 @@ static int bgx_lmac_xaui_init(struct bgx *bgx, struct lmac *lmac) > bgx_reg_write(bgx, lmacid, BGX_SPUX_BR_PMD_LD_REP, 0x00); > /* training enable */ > bgx_reg_modify(bgx, lmacid, > - BGX_SPUX_BR_PMD_CRTL, SPU_PMD_CRTL_TRAIN_EN); > + BGX_SPUX_BR_PMD_CRTL, SPU_PMD_CRTL_TRAIN_EN); Same here. Please make sure it only contain change related to fixing the problem.
Thank you, some changes in this patch are format issues in checkpatch, the fix is to add fwnode_handle_put() before break, I will resubmit the new version patch. :) -----邮件原件----- 发件人: Yunsheng Lin <linyunsheng@huawei.com> 发送时间: 2023年6月13日 20:53 收件人: 王明-软件底层技术部 <machel@vivo.com>; Sunil Goutham <sgoutham@marvell.com>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org 抄送: opensource.kernel <opensource.kernel@vivo.com> 主题: Re: [PATCH] drivers/thunder:improve-warning-message-in-device_for_each_child_node() [Some people who received this message don't often get email from linyunsheng@huawei.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] On 2023/6/13 20:38, Wang Ming wrote: > In device_for_each_child_node(), it should have fwnode_handle_put() > before break to prevent stale device node references from being left > behind. > > Signed-off-by: Wang Ming <machel@vivo.com> A Fixes tag seems necessary according to the commit log, and should target the net branch using: [PATCH net] drivers/thunder: improve-warning-message-in-device_for_each_child_node() Also it seems confusing the 'improve' in the title suggest that it is not a fix. > --- > .../net/ethernet/cavium/thunder/thunder_bgx.c | 37 > ++++++++++--------- > 1 file changed, 20 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > index a317feb8d..d37ee2872 100644 > --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c > @@ -90,7 +90,7 @@ static const struct pci_device_id bgx_id_table[] = { > > MODULE_AUTHOR("Cavium Inc"); > MODULE_DESCRIPTION("Cavium Thunder BGX/MAC Driver"); > -MODULE_LICENSE("GPL v2"); > +MODULE_LICENSE("GPL"); Is there any reason you changing the license here? > MODULE_VERSION(DRV_VERSION); > MODULE_DEVICE_TABLE(pci, bgx_id_table); > > @@ -174,10 +174,10 @@ static struct bgx *get_bgx(int node, int > bgx_idx) } > > /* Return number of BGX present in HW */ -unsigned bgx_get_map(int > node) > +unsigned int bgx_get_map(int node) It seems to be unrelated change here, is the changing related to the problem you are fixing? > { > int i; > - unsigned map = 0; > + unsigned int map = 0; Same here. > > for (i = 0; i < max_bgx_per_node; i++) { > if (bgx_vnic[(node * max_bgx_per_node) + i]) @@ -600,9 > +600,9 @@ static void bgx_lmac_handler(struct net_device *netdev) > link_changed = -1; > > if (phydev->link && > - (lmac->last_duplex != phydev->duplex || > - lmac->last_link != phydev->link || > - lmac->last_speed != phydev->speed)) { > + (lmac->last_duplex != phydev->duplex || > + lmac->last_link != phydev->link || > + lmac->last_speed != phydev->speed)) { Same here. > link_changed = 1; > } > > @@ -783,7 +783,7 @@ static int bgx_lmac_xaui_init(struct bgx *bgx, struct lmac *lmac) > bgx_reg_write(bgx, lmacid, BGX_SPUX_BR_PMD_LD_REP, 0x00); > /* training enable */ > bgx_reg_modify(bgx, lmacid, > - BGX_SPUX_BR_PMD_CRTL, SPU_PMD_CRTL_TRAIN_EN); > + BGX_SPUX_BR_PMD_CRTL, > + SPU_PMD_CRTL_TRAIN_EN); Same here. Please make sure it only contain change related to fixing the problem.
diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c index a317feb8d..d37ee2872 100644 --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c @@ -90,7 +90,7 @@ static const struct pci_device_id bgx_id_table[] = { MODULE_AUTHOR("Cavium Inc"); MODULE_DESCRIPTION("Cavium Thunder BGX/MAC Driver"); -MODULE_LICENSE("GPL v2"); +MODULE_LICENSE("GPL"); MODULE_VERSION(DRV_VERSION); MODULE_DEVICE_TABLE(pci, bgx_id_table); @@ -174,10 +174,10 @@ static struct bgx *get_bgx(int node, int bgx_idx) } /* Return number of BGX present in HW */ -unsigned bgx_get_map(int node) +unsigned int bgx_get_map(int node) { int i; - unsigned map = 0; + unsigned int map = 0; for (i = 0; i < max_bgx_per_node; i++) { if (bgx_vnic[(node * max_bgx_per_node) + i]) @@ -600,9 +600,9 @@ static void bgx_lmac_handler(struct net_device *netdev) link_changed = -1; if (phydev->link && - (lmac->last_duplex != phydev->duplex || - lmac->last_link != phydev->link || - lmac->last_speed != phydev->speed)) { + (lmac->last_duplex != phydev->duplex || + lmac->last_link != phydev->link || + lmac->last_speed != phydev->speed)) { link_changed = 1; } @@ -783,7 +783,7 @@ static int bgx_lmac_xaui_init(struct bgx *bgx, struct lmac *lmac) bgx_reg_write(bgx, lmacid, BGX_SPUX_BR_PMD_LD_REP, 0x00); /* training enable */ bgx_reg_modify(bgx, lmacid, - BGX_SPUX_BR_PMD_CRTL, SPU_PMD_CRTL_TRAIN_EN); + BGX_SPUX_BR_PMD_CRTL, SPU_PMD_CRTL_TRAIN_EN); } /* Append FCS to each packet */ @@ -1059,8 +1059,8 @@ static int bgx_lmac_enable(struct bgx *bgx, u8 lmacid) lmac->bgx = bgx; if ((lmac->lmac_type == BGX_MODE_SGMII) || - (lmac->lmac_type == BGX_MODE_QSGMII) || - (lmac->lmac_type == BGX_MODE_RGMII)) { + (lmac->lmac_type == BGX_MODE_QSGMII) || + (lmac->lmac_type == BGX_MODE_RGMII)) { lmac->is_sgmii = true; if (bgx_lmac_sgmii_init(bgx, lmac)) return -1; @@ -1096,9 +1096,9 @@ static int bgx_lmac_enable(struct bgx *bgx, u8 lmacid) bgx_reg_write(bgx, lmacid, BGX_CMRX_RX_DMAC_CTL, 0x03); if ((lmac->lmac_type != BGX_MODE_XFI) && - (lmac->lmac_type != BGX_MODE_XLAUI) && - (lmac->lmac_type != BGX_MODE_40G_KR) && - (lmac->lmac_type != BGX_MODE_10G_KR)) { + (lmac->lmac_type != BGX_MODE_XLAUI) && + (lmac->lmac_type != BGX_MODE_40G_KR) && + (lmac->lmac_type != BGX_MODE_10G_KR)) { if (!lmac->phydev) { if (lmac->autoneg) { bgx_reg_write(bgx, lmacid, @@ -1178,9 +1178,9 @@ static void bgx_lmac_disable(struct bgx *bgx, u8 lmacid) kfree(lmac->dmacs); if ((lmac->lmac_type != BGX_MODE_XFI) && - (lmac->lmac_type != BGX_MODE_XLAUI) && - (lmac->lmac_type != BGX_MODE_40G_KR) && - (lmac->lmac_type != BGX_MODE_10G_KR) && lmac->phydev) + (lmac->lmac_type != BGX_MODE_XLAUI) && + (lmac->lmac_type != BGX_MODE_40G_KR) && + (lmac->lmac_type != BGX_MODE_10G_KR) && lmac->phydev) phy_disconnect(lmac->phydev); lmac->phydev = NULL; @@ -1199,7 +1199,7 @@ static void bgx_init_hw(struct bgx *bgx) for (i = 0; i < bgx->lmac_count; i++) { lmac = &bgx->lmac[i]; bgx_reg_write(bgx, i, BGX_CMRX_CFG, - (lmac->lmac_type << 8) | lmac->lane_to_sds); + (lmac->lmac_type << 8) | lmac->lane_to_sds); bgx->lmac[i].lmacid_bd = lmac_count; lmac_count++; } @@ -1478,8 +1478,10 @@ static int bgx_init_of_phy(struct bgx *bgx) * cannot handle it, so exit the loop. */ node = to_of_node(fwn); - if (!node) + if (!node) { + fwnode_handle_put(fwn); break; + } of_get_mac_address(node, bgx->lmac[lmac].mac); @@ -1503,6 +1505,7 @@ static int bgx_init_of_phy(struct bgx *bgx) lmac++; if (lmac == bgx->max_lmac) { of_node_put(node); + fwnode_handle_put(fwn); break; } }
In device_for_each_child_node(), it should have fwnode_handle_put() before break to prevent stale device node references from being left behind. Signed-off-by: Wang Ming <machel@vivo.com> --- .../net/ethernet/cavium/thunder/thunder_bgx.c | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) -- 2.25.1