Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 28 Jun 2022 16:46:33 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Frank <Frank.Sae@motor-comm.com>
Cc:     Peter Geis <pgwipeout@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, yinghong.zhang@motor-comm.com,
        fei.zhang@motor-comm.com, hua.sun@motor-comm.com,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] net: phy: Add driver for Motorcomm yt8521 gigabit
 ethernet phy
Message-ID: <YrsUSSImpUXrZFUl@lunn.ch>
References: <20220628104245.35-1-Frank.Sae@motor-comm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220628104245.35-1-Frank.Sae@motor-comm.com>
Precedence: bulk

> --- 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