From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753195AbbJPFhL (ORCPT ); Fri, 16 Oct 2015 01:37:11 -0400 Received: from mail-out.m-online.net ([212.18.0.9]:38177 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751936AbbJPFhJ (ORCPT ); Fri, 16 Oct 2015 01:37:09 -0400 X-Auth-Info: +5s79lJbVbblabQBu7haBtLeigZe7KkNbgSSLVB3eVo= Subject: Re: [PATCH] net: phy: smsc: disable energy detect mode To: Florian Fainelli References: <1444713189-23063-1-git-send-email-hs@denx.de> <561D5AC8.7040801@gmail.com> <561DD749.9000802@denx.de> Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, netdev@vger.kernel.org, Georg.Soffel@bosch-si.com From: Heiko Schocher Reply-To: hs@denx.de Message-ID: <56208D00.80707@denx.de> Date: Fri, 16 Oct 2015 07:37:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <561DD749.9000802@denx.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Florian, Am 14.10.2015 um 06:17 schrieb Heiko Schocher: > Hello Florian, > > Am 13.10.2015 um 21:26 schrieb Florian Fainelli: >> On 12/10/15 22:13, Heiko Schocher wrote: >>> On some boards the energy enable detect mode leads in >>> trouble with some switches, so make the enabling of >>> this mode configurable through DT. >>> >>> Signed-off-by: Heiko Schocher >>> --- >>> >>> .../devicetree/bindings/net/smsc-lan87xx.txt | 19 +++++++++++++++++ >>> drivers/net/phy/smsc.c | 24 +++++++++++++++++----- >>> 2 files changed, 38 insertions(+), 5 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> >>> diff --git a/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> new file mode 100644 >>> index 0000000..39aa1dc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/smsc-lan87xx.txt >>> @@ -0,0 +1,19 @@ >>> +SMSC LAN87xx Ethernet PHY >>> + >>> +Some boards require special tuning values. Configure them >>> +through an Ethernet OF device node. >>> + >>> +Optional properties: >>> + >>> +- disable-energy-detect: >>> + If set, do not enable energy detect mode for the SMSC phy. >>> + default: enable energy detect mode >> >> Although energy detection is something that is implemented by many PHYs, >> I am not sure a generic property is suitable here, I would prefix that >> with the SMSC vendor prefix here to make it clear this only applies to >> this PHY. > > Hmm... but all PHYs should be able to enable, disable it in some way, or? ping? >> Would not you want to make it a reverse property here though, something >> like this: >> >> smsc,energy-detect: boolean, when present indicates the PHY reliably >> supports energy detection > > Yes, that was also my first thought, but currently, on this PHYs > energy detect mode is on ... and if I introduce such a property, > it will disable it for all existing boards, because property is > missing ... so, maybe I break boards ... I have a v2 prepared, but what to do here? >>> + >>> +Examples: >>> + >>> + /* Attach to an Ethernet device with autodetected PHY */ >>> + &cpsw_emac0 { >>> + phy_id = <&davinci_mdio>, <0>; >>> + phy-mode = "mii"; >>> + disable-energy-detect; >>> + }; >>> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c >>> index 70b0895..f90fbf3 100644 >>> --- a/drivers/net/phy/smsc.c >>> +++ b/drivers/net/phy/smsc.c >>> @@ -43,16 +43,30 @@ static int smsc_phy_ack_interrupt(struct phy_device *phydev) >>> >>> static int smsc_phy_config_init(struct phy_device *phydev) >>> { >>> +#ifdef CONFIG_OF >>> + int len; >>> + struct device *dev = &phydev->dev; >>> + struct device_node *of_node = dev->of_node; >> >> That does not need to be ifdefd out, at best annontate with __maybe_unused? > > Yes, I try it. removed. >>> +#endif >>> int rc = phy_read(phydev, MII_LAN83C185_CTRL_STATUS); >>> + int enable_energy = 1; >>> >>> if (rc < 0) >>> return rc; >>> >>> - /* Enable energy detect mode for this SMSC Transceivers */ >>> - rc = phy_write(phydev, MII_LAN83C185_CTRL_STATUS, >>> - rc | MII_LAN83C185_EDPWRDOWN); >>> - if (rc < 0) >>> - return rc; >>> +#ifdef CONFIG_OF >>> + if (!of_node && dev->parent->of_node) >>> + of_node = dev->parent->of_node; >> >> That looks strange, why would the property be placed at the parent level >> when this is a PHY device tree node property? > > Hmm.. I recheck this. Hmm.. the drivers/net/ethernet/ti/cpsw.c has no phy-handle support ... I added this, so my v2 is now a patchset of 2 patches. bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany