Re: [RFC PATCH 04/10] net: ti: prueth: Adds link detection, RX and TX support.

[Basharath Hussain Khaja <basharath@couthit.com>]

>> +/* update phy/port status information for firmware */
>> +static void icssm_emac_update_phystatus(struct prueth_emac *emac)
>> +{
>> +	struct prueth *prueth = emac->prueth;
>> +	u32 phy_speed, port_status = 0;
>> +	enum prueth_mem region;
>> +	u32 delay;
>> +
>> +	region = emac->dram;
>> +	phy_speed = emac->speed;
>> +	icssm_prueth_write_reg(prueth, region, PHY_SPEED_OFFSET, phy_speed);
>> +
>> +	if (phy_speed == SPEED_10)
>> +		delay = TX_CLK_DELAY_10M;
> 
> How can speed to 10? You removed those link modes?
> 

We will clean it up in the next version.

>> +/**
>> + * icssm_emac_ndo_start_xmit - EMAC Transmit function
>> + * @skb: SKB pointer
>> + * @ndev: EMAC network adapter
>> + *
>> + * Called by the system to transmit a packet  - we queue the packet in
>> + * EMAC hardware transmit queue
>> + *
>> + * Return: success(NETDEV_TX_OK) or error code (typically out of desc's)
>> + */
>> +static int icssm_emac_ndo_start_xmit(struct sk_buff *skb,
>> +				     struct net_device *ndev)
>> +{
>> +	struct prueth_emac *emac = netdev_priv(ndev);
>> +	int ret = 0;
>> +	u16 qid;
>> +
>> +	if (unlikely(!emac->link)) {
>> +		if (netif_msg_tx_err(emac) && net_ratelimit())
>> +			netdev_err(ndev, "No link to transmit");
>> +		goto fail_tx;
>> +	}
> 
> Do many other MAC drivers have this test?
> 

This was an experimental safety check to avoid pushing anymore packets 
into PRU buffers when link down occurred but upper layers are not notified
yet. We will remove this check in the next version.

>> --- a/drivers/net/ethernet/ti/icssm/icssm_prueth.h
>> +++ b/drivers/net/ethernet/ti/icssm/icssm_prueth.h
>> @@ -17,6 +17,11 @@
>>  
>>  /* PRUSS local memory map */
>>  #define ICSS_LOCAL_SHARED_RAM	0x00010000
>> +#define EMAC_MAX_PKTLEN		(ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
>> +/* Below macro is for 1528 Byte Frame support, to Allow even with
>> + * Redundancy tag
>> + */
>> +#define EMAC_MAX_FRM_SUPPORT (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN + 6)
> 
> This looks familiar....
> 

We will use the MACRO instead of hard-coded value.

Thanks & Best Regards,
Basharath