From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut To: linux-hwmon@vger.kernel.org Date: Fri, 04 Jan 2019 11:29:29 -0800 Subject: Re: [PATCH V4] net: phy: tja11xx: Add TJA11xx PHY driver In-Reply-To: <20190104021547.15527-1-marex@denx.de> Message-ID: <5c3b2b4e-2dd2-98ad-4eb2-77cde1373071@denx.de> MIME-Version: 1.0 Content-Type: text/plain List-ID: On 1/4/19 5:24 PM, Florian Fainelli wrote: > > > On January 3, 2019 6:15:47 PM PST, Marek Vasut wrote: >> Add driver for the NXP TJA1100 and TJA1101 PHYs. These PHYs are special >> BroadRReach 100BaseT1 PHYs used in automotive. >> >> Signed-off-by: Marek Vasut >> Cc: Andrew Lunn >> Cc: Florian Fainelli >> Cc: Guenter Roeck >> Cc: Heiner Kallweit >> Cc: Jean Delvare >> Cc: linux-hwmon@vger.kernel.org >> --- >> V2: - Use phy_modify(), phy_{set,clear}_bits() >> - Drop enable argument of tja11xx_enable_link_control() >> - Use PHY_BASIC_T1_FEATURES and dont modify supported/advertised >> features in config_init callback >> - Use genphy_soft_reset() instead of opencoding the reset sequence. >> - Drop the aneg parts, since the PHY datasheet claims it does not >> support aneg >> V3: - Replace clr with mask >> - Add hwmon support >> - Check commstat in tja11xx_read_status() only if link is up >> - Use PHY_ID_MATCH_MODEL() >> V4: - Use correct bit in tja11xx_hwmon_read() hwmon_temp_crit_alarm >> - Use ENOMEM if devm_kstrdup() fails >> - Check $type in tja11xx_hwmon_read() in addition to $attr >> --- > > [Snip] > >> + ret = tja11xx_enable_reg_write(phydev); >> + if (ret) >> + return ret; >> + >> + phydev->irq = PHY_POLL; > > The PHY driver should not be imposing that, leave it up to the platform > configuration to set that. OK >> + phydev->autoneg = AUTONEG_DISABLE; >> + phydev->speed = SPEED_100; >> + phydev->duplex = DUPLEX_FULL; >> + phydev->pause = 0; >> + phydev->asym_pause = 0; > > Are any of those necessary if you set basic T1 features? autoneg, speed and duplex need to be set, the rest can be left as defaults. Shall I drop those (pause, asym_pause) ? > Everything else looks good to me. Thanks! > -- Best regards, Marek Vasut From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1921DC43387 for ; Fri, 4 Jan 2019 19:29:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DF12D21872 for ; Fri, 4 Jan 2019 19:29:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725936AbfADT3U (ORCPT ); Fri, 4 Jan 2019 14:29:20 -0500 Received: from mail-out.m-online.net ([212.18.0.9]:47387 "EHLO mail-out.m-online.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725930AbfADT3U (ORCPT ); Fri, 4 Jan 2019 14:29:20 -0500 Received: from frontend01.mail.m-online.net (unknown [192.168.8.182]) by mail-out.m-online.net (Postfix) with ESMTP id 43WZbb3y7dz1qwC3; Fri, 4 Jan 2019 20:29:15 +0100 (CET) Received: from localhost (dynscan1.mnet-online.de [192.168.6.70]) by mail.m-online.net (Postfix) with ESMTP id 43WZbb3GXLz1qvXH; Fri, 4 Jan 2019 20:29:15 +0100 (CET) X-Virus-Scanned: amavisd-new at mnet-online.de Received: from mail.mnet-online.de ([192.168.8.182]) by localhost (dynscan1.mail.m-online.net [192.168.6.70]) (amavisd-new, port 10024) with ESMTP id I3rtG9FU0FZX; Fri, 4 Jan 2019 20:29:13 +0100 (CET) X-Auth-Info: 51b6w8eEWQ6jJMV3FQnPwkD44riKNDSK86wdn8NG8lc= Received: from [IPv6:::1] (unknown [195.140.253.167]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.mnet-online.de (Postfix) with ESMTPSA; Fri, 4 Jan 2019 20:29:13 +0100 (CET) Subject: Re: [PATCH V4] net: phy: tja11xx: Add TJA11xx PHY driver To: Florian Fainelli , netdev@vger.kernel.org Cc: Andrew Lunn , Guenter Roeck , Heiner Kallweit , Jean Delvare , linux-hwmon@vger.kernel.org References: <20190104021547.15527-1-marex@denx.de> From: Marek Vasut Openpgp: preference=signencrypt Autocrypt: addr=marex@denx.de; prefer-encrypt=mutual; keydata= mQINBFHmnxgBEACuQOC6Kaw/32MTeUJdFuDZ1FrbG76a0Ys/I02Kj9jXDmCCLvqq18Z4A1b0 xbuMKGDy5WR77fqGV8zADUo6i1ATgCZeg+SRmQROF8r9K6n6digTznBySSLANhN3kXUMNRE1 WEIBGCZJ5FF+Qq59AkAUTB8CiIzfEW98o7lUjeEume/78wR18+QW+2z6eYli2qNECceRINXT zS3oxRMr+ivqEUGKvMBC/WNLuvJoCGsfSQc2I+uGEU7MOdOCC6SsKdnPBGKYth5Ieb16bRS1 b9M5BoEKTEzDCOWn92OxeHX6M2gLEMQobfM0RdIowMfWaUHdci2cLUTyL0T/P/gIpHMR2LhL 8sdbNZufgv73s9PDgxTWMzypXimMJ7VZmVh9I2nQd2xm8+uE1rghqb90aEMFCTwUlrz4Qhjh vmczd2ScuuOMLzHEaaoOrMGbaWIEFcJvQgyHzJgMPgnG64eDq6uGyBEXRc3bBzv7B765Hcg8 SSNqoUstjuQQlGp3y3Yj16l+PyZ3Ucy2swFYLVPTc35xFBk/uGEIhGncoFpOX29rxt9M8r5G hm7395m0GmDy50H/HN61/S8EPvM3HUjqBvX1EqU+vJXfwozxkKpIwcjx7h3W+PPS9TUb7r5v vHCqnrWRd/m6KWbCJsv0rsIU66o2qKYX5cIHV6u6Y7Zm7BtHfwARAQABtBtNYXJlayBWYXN1 dCA8bWFyZXhAZGVueC5kZT6JAjgEEwECACIFAlHmnxgCGwMGCwkIBwMCBhUIAgkKCwQWAgMB Ah4BAheAAAoJEOtsLUEh5B0XLk0QAINOYFYB3v4KjXSFHYBQLlDblqhXvVtjyQHMiJsY1BMO mMrANUJQtpY3UkYquFspe2GBiFQbfW+mDlwFlSNpzaJ68qGEK+57I/MufsZKV6Ze9j7QeClu orYH+zfIBI7sn0HkY/MWN/Z270gRv2xSxDBP/8SPdB53EkImLZUFOo4/5eyuQ4t8HLgol02u 2ncwXrnT036QC3SiNJDCJhwkpjvamPHghxr8hbIwkdOLZlYWfl0yzYzQohl8zBEwtBxl5cS4 1TcrgBXsanQUMVNBpl0s8nQLKuHJNPOAhBnKstAe54yY3iWswYayHqqgqIQldcDqttHhdTJW mb9hTSf5p6fnZqcsfi3PUFwj5PJSN3aAbF8w42FwRvIOWbksFIWXpxYI3mq2TmX4GtlKdlF8 xT+Q+Cbk538IBV4OQ5BapuYHs1C1ff9gVC0rfrCEloyteHafHwOv3ZuEGPlH89Rl4EjRvJxX 8nE0sCiq6yUbpom8xRA5nFwA0bbTDwhH5RD/952bZraLpWcdJ6cWA2gefd2+2fy0268xyHmD m87B49BIaAsZ2kvEb/scCZ/CvPHjHLAjr+/GsdzOxwB68P41ZajujMDmbka00CyeAl88pgLX tTkPvAzuEDpRoJmg8zrQqrsmEKSdhFJhZ7d2MMKpCcVnInByXjM+1GEfSisTgWnluQINBFHm nxgBEAC8MpoO1s1AB0uRQGXlhYzkYvxkDGAe50/18ct2K6ORSv7HjCmZBjJX+2xTPSmML9ju 3P0KrlnRdT8qCh+ozijffLjm5X9Fk+6mGQ56UQzivuPNlgyC3epF3Z58VPVQcIfE2/pdAxtZ zKc4P5t2yo5qk635huo0NvNg5mRhvfZ7mZpZuBahkHguR0Heh/tnGCa2v5P6uFbGX8+6rAA8 EKxl5Tclf27PFZwbIWL1buS9RwgzsHj2TFnnEFIcWdMHyGy2GT8JMgY0VwxKebzGJg2RqfOL PaPjnvnXHAIYEknQp0TUtUiNxm0PBa4IQ30XhrB9D5QYdcw/DVvCzb9qyIlaQKEqHZm1fGU4 iCsH3jV+5D4Lrn5JfXc/+A1NsLUq/NFIYhphbX4fGjR2QdZJrDnGVcxSlwP7CeRuxGELrASz m4G4Q0mYz7HdAlzBJHi8Ej4yC9l7PPlnxdUcAwheLxGwzMCf5vxw1C6Zi8PvKu/sY7Bha9XJ plvuLBi7QrkD8mZEzt+xC9nWRt7hL47+UvyduFe4qDMTPrW20ROxCykC36gj53YhqqLblioX 2//vGLKj8x+LiLSTwjkLkrwOremhdTqr457511vOXyaZyOlWhFjN+4j9xwbbg1IWwMenRAb7 Qwuipck6fN2o+PK9i6t6pWXrUDNI/VCMbimnuqPwAQARAQABiQIfBBgBAgAJBQJR5p8YAhsM AAoJEOtsLUEh5B0XMqAP/1HbrClefDZ/Lvvo89mgC56vWzEstmFo8EihqxVZvpkiCjJoCH53 VCYeGl41p0y6K5gaLT28s9waVHBw+dhpwABba3neV/vyXv0wUtvkS3T0e4zruYFWw0lQoZi+ 8rtXTsuWN5t3u8avXsrdqD0CteTJdgZ7yBV8bBvK2ekqFMS/cLC+MoYlmUFn6Tcxmv0x8QZY ux6ts9YpUvx8QxMJt9vfwt1WIUEFKR3JQdrZmbPGqWJ3s+u/C+v9stC5qf2eYafRjzy05lEn B06W5D5Uc+FGEhuzq4G0eRLgivMoC0Eqz7HuwGcRAJYQILQ3Vzd4oHKPoUAtvlKqUwDmHodT HPmN73JMsvO3jLrSdl4k6o3CdlS/DI0Eto4fD0Wqh6d5q11u1TOM7+/LehWrOOoGVqRc6FFT ofck6h6rN/Urwkr1nWQ3kgO1cd/gevqy8Tevo/qkPYIf71BlypcXhKqn6IPjkq4QLiDPRjHM tgPc2T/X/ETe5eCuhxMytIYbt1fK2pDXPoIKbbDK4uEmg9USXZ+pYrac4PFo1d+6D6vmTjRZ GRRITOVpKgBndfPyqofxeKNKGdNf9FS/x89RlnDWXsQHm+0pXguSRG9XdB16ZFNgeo8SeZVr qc9uLfhyQp/zB6qEnuX1TToug7PuDgcNZdjN3vgTXyno2TFMxp/LKHqg Message-ID: <5c3b2b4e-2dd2-98ad-4eb2-77cde1373071@denx.de> Date: Fri, 4 Jan 2019 20:27:50 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-hwmon-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-hwmon@vger.kernel.org Message-ID: <20190104192750.4R5DrEA0TO_bP6SOt25EK-y8I-6NxaHWEJXdMVAgPmA@z> On 1/4/19 5:24 PM, Florian Fainelli wrote: > > > On January 3, 2019 6:15:47 PM PST, Marek Vasut wrote: >> Add driver for the NXP TJA1100 and TJA1101 PHYs. These PHYs are special >> BroadRReach 100BaseT1 PHYs used in automotive. >> >> Signed-off-by: Marek Vasut >> Cc: Andrew Lunn >> Cc: Florian Fainelli >> Cc: Guenter Roeck >> Cc: Heiner Kallweit >> Cc: Jean Delvare >> Cc: linux-hwmon@vger.kernel.org >> --- >> V2: - Use phy_modify(), phy_{set,clear}_bits() >> - Drop enable argument of tja11xx_enable_link_control() >> - Use PHY_BASIC_T1_FEATURES and dont modify supported/advertised >> features in config_init callback >> - Use genphy_soft_reset() instead of opencoding the reset sequence. >> - Drop the aneg parts, since the PHY datasheet claims it does not >> support aneg >> V3: - Replace clr with mask >> - Add hwmon support >> - Check commstat in tja11xx_read_status() only if link is up >> - Use PHY_ID_MATCH_MODEL() >> V4: - Use correct bit in tja11xx_hwmon_read() hwmon_temp_crit_alarm >> - Use ENOMEM if devm_kstrdup() fails >> - Check $type in tja11xx_hwmon_read() in addition to $attr >> --- > > [Snip] > >> + ret = tja11xx_enable_reg_write(phydev); >> + if (ret) >> + return ret; >> + >> + phydev->irq = PHY_POLL; > > The PHY driver should not be imposing that, leave it up to the platform configuration to set that. OK >> + phydev->autoneg = AUTONEG_DISABLE; >> + phydev->speed = SPEED_100; >> + phydev->duplex = DUPLEX_FULL; >> + phydev->pause = 0; >> + phydev->asym_pause = 0; > > Are any of those necessary if you set basic T1 features? autoneg, speed and duplex need to be set, the rest can be left as defaults. Shall I drop those (pause, asym_pause) ? > Everything else looks good to me. Thanks! > -- Best regards, Marek Vasut