From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755358AbeASCvI (ORCPT ); Thu, 18 Jan 2018 21:51:08 -0500 Received: from szxga06-in.huawei.com ([45.249.212.32]:33620 "EHLO huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1752855AbeASCvA (ORCPT ); Thu, 18 Jan 2018 21:51:00 -0500 Subject: Re: [PATCH net-next 3/5] net: hns3: add ethtool -p support for phy device To: Andrew Lunn CC: , , , , References: <1516259632-85088-1-git-send-email-lipeng321@huawei.com> <1516259632-85088-4-git-send-email-lipeng321@huawei.com> <20180118142515.GI5894@lunn.ch> From: "lipeng (Y)" Message-ID: Date: Fri, 19 Jan 2018 10:50:09 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180118142515.GI5894@lunn.ch> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Originating-IP: [10.63.173.108] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2018/1/18 22:25, Andrew Lunn wrote: >> +static int hclge_set_led_status_phy(struct phy_device *phydev, int value) >> +{ >> + int ret, cur_page; >> + >> + mutex_lock(&phydev->lock); >> + >> + ret = phy_read(phydev, HCLGE_PHY_PAGE_REG); >> + if (ret < 0) >> + goto out; >> + else >> + cur_page = ret; >> + >> + ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, HCLGE_PHY_PAGE_LED); >> + if (ret) >> + goto out; >> + >> + ret = phy_write(phydev, HCLGE_LED_FC_REG, value); >> + if (ret) >> + goto out; >> + >> + ret = phy_write(phydev, HCLGE_PHY_PAGE_REG, cur_page); >> + >> +out: >> + mutex_unlock(&phydev->lock); >> + return ret; >> +} > Sorry, but NACK. > > Please add an interface to phylib and the phy driver you are using to > do this. > >> #define HCLGE_PHY_PAGE_MDIX 0 >> #define HCLGE_PHY_PAGE_COPPER 0 >> +#define HCLGE_PHY_PAGE_LED 3 >> >> /* Page Selection Reg. */ >> #define HCLGE_PHY_PAGE_REG 22 >> @@ -73,6 +74,15 @@ >> /* Copper Specific Status Register */ >> #define HCLGE_PHY_CSS_REG 17 >> >> +/* LED Function Control Register */ >> +#define HCLGE_LED_FC_REG 16 >> + >> +/* LED Polarity Control Register */ >> +#define HCLGE_LED_PC_REG 17 >> + >> +#define HCLGE_LED_FORCE_ON 9 >> +#define HCLGE_LED_FORCE_OFF 8 >> + > By the looks of these defines, you assume you have a Marvell PHY. > Please make this generic so anybody with a Marvell PHY can use it. > > Andrew Hi  Andrw, As your suggestion, we need add  interface to  phylib and the phy driver. We will consider your suggestion and push this patch after we fix your comments. so we will remove this patch  in V2 patch-set. Thanks Peng Li > . >