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=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no 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 BBC10C433E0 for ; Tue, 26 Jan 2021 13:19:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 86668206C0 for ; Tue, 26 Jan 2021 13:19:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2403902AbhAZNTK (ORCPT ); Tue, 26 Jan 2021 08:19:10 -0500 Received: from vps0.lunn.ch ([185.16.172.187]:60076 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2391830AbhAZNPc (ORCPT ); Tue, 26 Jan 2021 08:15:32 -0500 Received: from andrew by vps0.lunn.ch with local (Exim 4.94) (envelope-from ) id 1l4OB6-002hfZ-42; Tue, 26 Jan 2021 14:14:40 +0100 Date: Tue, 26 Jan 2021 14:14:40 +0100 From: Andrew Lunn To: Mike Looijmans Cc: netdev@vger.kernel.org, "David S. Miller" , Heiner Kallweit , Jakub Kicinski , Russell King , linux-kernel@vger.kernel.org Subject: Re: [PATCH] net: mdiobus: Prevent spike on MDIO bus reset signal Message-ID: References: <20210126073337.20393-1-mike.looijmans@topic.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210126073337.20393-1-mike.looijmans@topic.nl> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 26, 2021 at 08:33:37AM +0100, Mike Looijmans wrote: > The mdio_bus reset code first de-asserted the reset by allocating with > GPIOD_OUT_LOW, then asserted and de-asserted again. In other words, if > the reset signal defaulted to asserted, there'd be a short "spike" > before the reset. > > Instead, directly assert the reset signal using GPIOD_OUT_HIGH, this > removes the spike and also removes a line of code since the signal > is already high. Hi Mike This however appears to remove the reset pulse, if the reset line was already low to start with. Notice you left fsleep(bus->reset_delay_us); without any action before it? What are we now waiting for? Most data sheets talk of a reset pulse. Take the reset line high, wait for some time, take the reset low, wait for some time, and then start talking to the PHY. I think with this patch, we have lost the guarantee of a low to high transition. Is this spike, followed by a pulse actually causing you problems? If so, i would actually suggest adding another delay, to stretch the spike. We have no control over the initial state of the reset line, it is how the bootloader left it, we have to handle both states. Andrew