Re: [PATCH v2] net: phy: Add driver for Motorcomm yt8521 gigabit ethernet phy

[Andrew Lunn <andrew@lunn.ch>]

> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c
> @@ -1,15 +1,115 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * Driver for Motorcomm PHYs
> + * motorcomm.c: Motorcomm 8511/8521 PHY driver.

Generally you don't include a files name in the file, because it can
get renamed and nobody updates it.

>   *
>   * Author: Peter Geis <pgwipeout@gmail.com>
> + * Author: Frank <Frank.Sae@motor-comm.com>
>   */
>  
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/phy.h>
> +#include <linux/etherdevice.h>

The includes are currently sorted, please keep it so.

>  
>  #define PHY_ID_YT8511		0x0000010a
> +#define PHY_ID_YT8521				0x0000011A
> +
> +/* YT8521 Register Overview
> + *	UTP Register space	|	FIBER Register space
> + *  ------------------------------------------------------------
> + * |	UTP MII			|	FIBER MII		|
> + * |	UTP MMD			|				|
> + * |	UTP Extended		|	FIBER Extended		|
> + *  ------------------------------------------------------------
> + * |			Common Extended				|
> + *  ------------------------------------------------------------
> + */
> +
> +/* 0x10 ~ 0x15 , 0x1E and 0x1F are common MII registers of yt phy */
> +
> +/* Specific Function Control Register */
> +#define YTPHY_SPECIFIC_FUNCTION_CONTROL_REG	0x10
> +
> +/* 2b00 Manual MDI configuration
> + * 2b01 Manual MDIX configuration
> + * 2b10 Reserved
> + * 2b11 Enable automatic crossover for all modes  *default*

> +
> +/* 2b00 10 Mbps
> + * 2b01 100 Mbps
> + * 2b10 1000 Mbps
> + * 2b11 Reserved
> +
> +/* 8 working modes:
> + * 3b000 UTP_TO_RGMII  *default*
> + * 3b001 FIBER_TO_RGMII
> + * 3b010 UTP_FIBER_TO_RGMII
> + * 3b011 UTP_TO_SGMII
> + * 3b100 SGPHY_TO_RGMAC
> + * 3b101 SGMAC_TO_RGPHY
> + * 3b110 UTP_TO_FIBER_AUTO
> + * 3b111 UTP_TO_FIBER_FORCE

I don't think this notation is used anywhere else in the kernel. It
also took me a little while to figure it out. It would probably be
better to just add comments next to the #define.

       Andrew