RE: [PATCH 6/9] net/ngbe: add support to custom PHY interfaces

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On February 9, 2022 1:41 AM, Ferruh Yigit wrote:
> On 2/8/2022 10:11 AM, Jiawen Wu wrote:
> > Support sub_device ID 61/62/64 for YT8521S SFP, and 51/52 for M88E1512
> > PHY.
> >
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> 
> <...>
> 
> > @@ -69,6 +69,10 @@ New Features
> >     * Added AES-XCBC support in lookaside protocol (IPsec) for CN9K &
> CN10K.
> >     * Added AES-CMAC support in CN9K & CN10K.
> >
> > +* **Updated Wangxun ngbe driver.**
> > +
> > +  * Added support for devices of custom PHY interfaces.
> > +
> 
> "Custom PHY" is vague, should we provide more details on added PHY
> interfaces, like code names etc....
> 
> <...>
> 
> > @@ -1815,11 +1816,23 @@ s32 ngbe_set_mac_type(struct ngbe_hw *hw)
> >   	case NGBE_SUB_DEV_ID_EM_MVL_RGMII:
> >   		hw->phy.media_type = ngbe_media_type_copper;
> >   		hw->mac.type = ngbe_mac_em;
> > +		hw->mac.link_type = ngbe_link_copper;
> > +		break;
> > +	case NGBE_SUB_DEV_ID_EM_RTL_YT8521S_SFP:
> > +		hw->phy.media_type = ngbe_media_type_copper;
> > +		hw->mac.type = ngbe_mac_em;
> > +		hw->mac.link_type = ngbe_link_fiber;
> >   		break;
> >   	case NGBE_SUB_DEV_ID_EM_MVL_SFP:
> >   	case NGBE_SUB_DEV_ID_EM_YT8521S_SFP:
> >   		hw->phy.media_type = ngbe_media_type_fiber;
> >   		hw->mac.type = ngbe_mac_em;
> > +		hw->mac.link_type = ngbe_link_fiber;
> > +		break;
> > +	case NGBE_SUB_DEV_ID_EM_MVL_MIX:
> > +		hw->phy.media_type = ngbe_media_type_unknown;
> > +		hw->mac.type = ngbe_mac_em;
> > +		hw->mac.link_type = ngbe_media_type_unknown;
> 
> This gives build error [1] because assigned enum type is wrong, is this
> copy/paste error?
> 
>   ../drivers/net/ngbe/base/ngbe_hw.c: In function ‘ngbe_set_mac_type’:
>   ../drivers/net/ngbe/base/ngbe_hw.c:1835:35: error: implicit conversion
> from ‘enum ngbe_media_type’ to ‘enum ngbe_link_type’
> [-Werror=enum-conversion]
>    1835 |                 hw->mac.link_type =
> ngbe_media_type_unknown;
>         |
> 
> 
> [1]
> http://mails.dpdk.org/archives/test-report/2022-February/257750.html
> 
> 
> <...>
> 
> > +s32 ngbe_check_phy_mode_mvl(struct ngbe_hw *hw) {
> > +	u16 value = 0;
> > +
> > +	/* select page 18 reg 20 */
> > +	ngbe_write_phy_reg_mdi(hw, MVL_PAGE_SEL, 0, 18);
> > +	ngbe_read_phy_reg_mdi(hw, MVL_GEN_CTL, 0, &value);
> > +	if (MVL_GEN_CTL_MODE(value) == MVL_GEN_CTL_MODE_COPPER) {
> > +		/* mode select to RGMII-to-copper */
> > +		hw->phy.type = ngbe_phy_mvl;
> > +		hw->phy.media_type = ngbe_media_type_copper;
> > +		hw->mac.link_type = ngbe_link_copper;
> > +	} else if (MVL_GEN_CTL_MODE(value) == MVL_GEN_CTL_MODE_FIBER) {
> > +		/* mode select to RGMII-to-sfi */
> > +		hw->phy.type = ngbe_phy_mvl_sfi;
> > +		hw->phy.media_type = ngbe_media_type_fiber;
> > +		hw->mac.link_type = ngbe_link_fiber;
> > +	} else {
> > +		DEBUGOUT("marvell 88E1512 mode %x is not supported.\n", value);
> 
> 'DEBUGOUT' already adds '\n' as far as I can see, it log add double line.
> 
> There are multiple usages in this patch.

I would fix it in a separate patch.