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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4EF5AC4332F for ; Wed, 21 Dec 2022 00:12:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234083AbiLUAMc (ORCPT ); Tue, 20 Dec 2022 19:12:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50344 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229451AbiLUAM3 (ORCPT ); Tue, 20 Dec 2022 19:12:29 -0500 Received: from vps0.lunn.ch (vps0.lunn.ch [156.67.10.101]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E5E031EAFB; Tue, 20 Dec 2022 16:12:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=GH9M5bV1VooJkjUWIga/hv2jQxEHFDZMjQbWm1O6tc8=; b=YulGIBLlt+uAz1B3XpSn3PEV2l JlPFmq6JYNmPnNsrPUTUWCQv1rTeNKwyMxxG8RqU1knVI4VYHeVZ0IboHTv33PVi7B2CsmxPiyH00 t5APlVAPVzm1NTlEkuCzGpEYQKYCtPDAOUKwwtDLsojqRWFXbCL3dSTlJHcPhzRisrzI=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1p7miW-0008LK-86; Wed, 21 Dec 2022 01:12:16 +0100 Date: Wed, 21 Dec 2022 01:12:16 +0100 From: Andrew Lunn To: "Russell King (Oracle)" Cc: Christian Marangi , Florian Fainelli , Vladimir Oltean , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Jonathan Corbet , Pavel Machek , John Crispin , netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-leds@vger.kernel.org, Tim Harvey , Alexander Stein , Rasmus Villemoes Subject: Re: [PATCH v7 09/11] leds: trigger: netdev: add additional hardware only triggers Message-ID: References: <20221214235438.30271-1-ansuelsmth@gmail.com> <20221214235438.30271-10-ansuelsmth@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 15, 2022 at 05:35:47PM +0000, Russell King (Oracle) wrote: > On Thu, Dec 15, 2022 at 12:54:36AM +0100, Christian Marangi wrote: > > Add additional hardware only triggers commonly supported by switch LEDs. > > > > Additional modes: > > link_10: LED on with link up AND speed 10mbps > > link_100: LED on with link up AND speed 100mbps > > link_1000: LED on with link up AND speed 1000mbps > > half_duplex: LED on with link up AND half_duplex mode > > full_duplex: LED on with link up AND full duplex mode > > Looking at Marvell 88e151x, I don't think this is usable there. > We have the option of supporting link_1000 on one of the LEDs, > link_100 on another, and link_10 on the other. It's rather rare > for all three leds to be wired though. The 88e151x will need to enumerate what it actually supports from the above list, per LED. I also think we can carefully expand the list above, adding a few more modes. We just need to ensure what is added is reasonably generic, modes we expect multiple PHY to support. What we need to avoid is adding every single mode a PHY supports, but no other PHY has. > This is also a PHY where "activity" mode is supported (illuminated > or blinking if any traffic is transmitted or received) but may not > support individual directional traffic in hardware. However, it > does support forcing the LED on or off, so software mode can handle > those until the user selects a combination of modes that are > supported in the hardware. > > > Additional blink interval modes: > > blink_2hz: LED blink on any even at 2Hz (250ms) > > blink_4hz: LED blink on any even at 4Hz (125ms) > > blink_8hz: LED blink on any even at 8Hz (62ms) > > This seems too restrictive. For example, Marvell 88e151x supports > none of these, but does support 42, 84, 170, 340, 670ms. I would actually drop this whole idea of being able to configure the blink period. It seems like it is going to cause problems. I expect most PHYs actual share the period across multiple LEDs, which you cannot easily model here. So i would have the driver hard coded to pick a frequency at thats' it. Andrew