RE: [PATCH 5/6] net: macb: add support for high speed interface

[Parshuram Raju Thombare <pthombar@cadence.com>]

>
>>  	switch (state->interface) {
>> +	case PHY_INTERFACE_MODE_NA:
>
>I would not list PHY_INTERFACE_MODE_NA here.
>
This was to experiment in band mode with sfp. 
phylink_sfp_module_insert call phylink_validate with interface set to PHY_INTERFACE_MODE_NA
, if it is not listed in validate method supported bitmask will be empty.  
But anyway since I am configuring fixed mode, removing this case.

>> +	case PHY_INTERFACE_MODE_USXGMII:
>> +	case PHY_INTERFACE_MODE_10GKR:
>> +		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE) {
>> +			phylink_set(mask, 10000baseCR_Full);
>> +			phylink_set(mask, 10000baseER_Full);
>> +			phylink_set(mask, 10000baseKR_Full);
>> +			phylink_set(mask, 10000baseLR_Full);
>> +			phylink_set(mask, 10000baseLRM_Full);
>> +			phylink_set(mask, 10000baseSR_Full);
>> +			phylink_set(mask, 10000baseT_Full);
>> +			phylink_set(mask, 5000baseT_Full);
>> +			phylink_set(mask, 2500baseX_Full);
>> +			phylink_set(mask, 1000baseX_Full);
>> +		}
>> +		/* Fall-through */
>>  	case PHY_INTERFACE_MODE_SGMII:
>>  		if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>>  			phylink_set(mask, 2500baseT_Full); @@ -594,17 +639,55
>@@ static
>> void gem_mac_config(struct phylink_config *pl_config, unsigned int mode,
>>  			reg |= MACB_BIT(FD);
>>  		macb_or_gem_writel(bp, NCFGR, reg);
>>
>> -		if (state->speed == SPEED_2500) {
>> -			gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> -				   gem_readl(bp, NCFGR));
>> -			gem_writel(bp, NCR, GEM_BIT(TWO_PT_FIVE_GIG) |
>> -				   gem_readl(bp, NCR));
>> -		} else if (state->speed == SPEED_1000) {
>> -			gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> -				   gem_readl(bp, NCFGR));
>> -		} else if (state->speed == SPEED_100) {
>> -			macb_writel(bp, NCFGR, MACB_BIT(SPD) |
>> -				    macb_readl(bp, NCFGR));
>> +		if (bp->phy_interface == PHY_INTERFACE_MODE_USXGMII) {
>> +			u32 speed;
>> +
>> +			switch (state->speed) {
>> +			case SPEED_10000:
>> +				if (bp->serdes_rate ==
>> +				    MACB_SERDES_RATE_10_PT_3125Gbps) {
>> +					speed = HS_MAC_SPEED_10000M;
>> +				} else {
>> +					netdev_warn(netdev,
>> +						    "10G not supported by HW");
>> +					netdev_warn(netdev, "Setting speed to
>1G");
>> +					speed = HS_MAC_SPEED_1000M;
>> +				}
>> +				break;
>> +			case SPEED_5000:
>> +				speed = HS_MAC_SPEED_5000M;
>> +				break;
>> +			case SPEED_2500:
>> +				speed = HS_MAC_SPEED_2500M;
>> +				break;
>> +			case SPEED_1000:
>> +				speed = HS_MAC_SPEED_1000M;
>> +				break;
>> +			default:
>> +			case SPEED_100:
>> +				speed = HS_MAC_SPEED_100M;
>> +				break;
>> +			}
>> +
>> +			gem_writel(bp, HS_MAC_CONFIG,
>> +				   GEM_BFINS(HS_MAC_SPEED, speed,
>> +					     gem_readl(bp, HS_MAC_CONFIG)));
>> +			gem_writel(bp, USX_CONTROL,
>> +				   GEM_BFINS(USX_CTRL_SPEED, speed,
>> +					     gem_readl(bp, USX_CONTROL)));
>> +		} else {
>> +			if (state->speed == SPEED_2500) {
>> +				gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> +					   gem_readl(bp, NCFGR));
>> +				gem_writel(bp, NCR,
>GEM_BIT(TWO_PT_FIVE_GIG) |
>> +					   gem_readl(bp, NCR));
>> +			} else if (state->speed == SPEED_1000) {
>> +				gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>> +					   gem_readl(bp, NCFGR));
>> +			} else if (state->speed == SPEED_100) {
>> +				macb_writel(bp, NCFGR, MACB_BIT(SPD) |
>> +					    macb_readl(bp, NCFGR));
>> +			}
>
>Maybe split this up into two helper functions?
Ok 
>
>      Andrew


Regards,
Parshuram Thombare