Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Christian Herber <christian.herber@nxp.com>
To:     Andrew Lunn <andrew@lunn.ch>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [EXT] Re: [PATCH net-next 1/1] Added BASE-T1 PHY support to PHY
 Subsystem
Thread-Topic: [EXT] Re: [PATCH net-next 1/1] Added BASE-T1 PHY support to PHY
 Subsystem
Thread-Index: AQHVU36mdpwX4yyX/0O62UH8/z+9iA==
Date:   Fri, 16 Aug 2019 11:56:38 +0000
Message-ID: <AM6PR0402MB3798CD854B37F430980E204F86AF0@AM6PR0402MB3798.eurprd04.prod.outlook.com>
References: <20190815153209.21529-1-christian.herber@nxp.com>
 <20190815153209.21529-2-christian.herber@nxp.com>
 <20190815155613.GE15291@lunn.ch>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 11062640-a5eb-4d9b-d2df-08d72240cb65
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Aug 2019 11:56:38.0984
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 22kuiPBddI4e5UPyHVuhsndWKpedZNzcP0HTGiNUWwqQCbR2+Q3YLkStiwGZdtJnCAkjmnNNc+V3E56XLasoZiKGK2YTiWpjTJmV6668yfc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR0402MB3848
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 15.08.2019 17:56, Andrew Lunn wrote:=0A=
> Caution: EXT Email=0A=
> =0A=
> On Thu, Aug 15, 2019 at 03:32:29PM +0000, Christian Herber wrote:=0A=
>> BASE-T1 is a category of Ethernet PHYs.=0A=
>> They use a single copper pair for transmission.=0A=
>> This patch add basic support for this category of PHYs.=0A=
>> It coveres the discovery of abilities and basic configuration.=0A=
>> It includes setting fixed speed and enabling auto-negotiation.=0A=
>> BASE-T1 devices should always Clause-45 managed.=0A=
>> Therefore, this patch extends phy-c45.c.=0A=
>> While for some functions like auto-neogtiation different registers are=
=0A=
>> used, the layout of these registers is the same for the used fields.=0A=
>> Thus, much of the logic of basic Clause-45 devices can be reused.=0A=
>>=0A=
>> Signed-off-by: Christian Herber <christian.herber@nxp.com>=0A=
>> ---=0A=
>>   drivers/net/phy/phy-c45.c    | 113 +++++++++++++++++++++++++++++++----=
=0A=
>>   drivers/net/phy/phy-core.c   |   4 +-=0A=
>>   include/uapi/linux/ethtool.h |   2 +=0A=
>>   include/uapi/linux/mdio.h    |  21 +++++++=0A=
>>   4 files changed, 129 insertions(+), 11 deletions(-)=0A=
>>=0A=
>> diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c=0A=
>> index b9d4145781ca..9ff0b8c785de 100644=0A=
>> --- a/drivers/net/phy/phy-c45.c=0A=
>> +++ b/drivers/net/phy/phy-c45.c=0A=
>> @@ -8,13 +8,23 @@=0A=
>>   #include <linux/mii.h>=0A=
>>   #include <linux/phy.h>=0A=
>>=0A=
>> +#define IS_100BASET1(phy) (linkmode_test_bit( \=0A=
>> +                        ETHTOOL_LINK_MODE_100baseT1_Full_BIT, \=0A=
>> +                        (phy)->supported))=0A=
>> +#define IS_1000BASET1(phy) (linkmode_test_bit( \=0A=
>> +                         ETHTOOL_LINK_MODE_1000baseT1_Full_BIT, \=0A=
>> +                         (phy)->supported))=0A=
> =0A=
> Hi Christian=0A=
> =0A=
> We already have the flag phydev->is_gigabit_capable. Maybe add a flag=0A=
> phydev->is_t1_capable=0A=
> =0A=
>> +=0A=
>> +static u32 get_aneg_ctrl(struct phy_device *phydev);=0A=
>> +static u32 get_aneg_stat(struct phy_device *phydev);=0A=
> =0A=
> No forward declarations please. Put the code in the right order so=0A=
> they are not needed.=0A=
> =0A=
> Thanks=0A=
> =0A=
>       Andrew=0A=
> =0A=
=0A=
Hi Andrew,=0A=
=0A=
thanks for feedback. The use of an additional flag is a good proposal.=0A=
I was hesitant to touch the phydev structure.=0A=
I will add this along with removing the forward declaration in v2.=0A=
=0A=
Regards,=0A=
=0A=
Christian=0A=