Re: [PATCH 3/3] net: j2: Introduce J-Core EMAC

[Artur Rojek <contact@artur-rojek.eu>]

On 2025-08-15 22:16, Andrew Lunn wrote:

Hi Andrew,
thanks for the review!

>> +static irqreturn_t jcore_emac_irq(int irq, void *data)
>> +{
>> +	struct jcore_emac *priv = data;
>> +	struct net_device *ndev = priv->ndev;
>> +	struct sk_buff *skb;
>> +	struct {
>> +		int packets;
>> +		int bytes;
>> +		int dropped;
>> +		int crc_errors;
>> +	} stats = {};
>> +	unsigned int status, pkt_len, i;
> 
> netdev uses 'reverse christmas tree' for local variables. They should
> be sorted longest to shortest. This sometimes means you need to move
> assignments into the body of the function, in this case, ndev.
> 
>> +	jcore_emac_read_hw_addr(priv, mac);
>> +	if (is_zero_ether_addr(mac)) {
> 
> It would be more normal to use !is_valid_ether_addr()
> 
> What support is there for MDIO? Normally the MAC driver would not be
> setting the carrier status, phylink or phylib would do that.

 From what I can tell, none. This is a very simple FPGA RTL
implementation of a MAC, and looking at the VHDL, I don't see any MDIO
registers. Moreover, the MDIO pin on the PHY IC on my dev board also
appears unconnected. Perhaps Rob L. or Jeff can shine more light on this
design wise.

Cheers,
Artur

> 
>     Andrew
> 
> ---
> pw-bot: cr