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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 728F2C433FE for ; Fri, 1 Oct 2021 10:53:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 50DFA61A6E for ; Fri, 1 Oct 2021 10:53:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1353615AbhJAKyy (ORCPT ); Fri, 1 Oct 2021 06:54:54 -0400 Received: from mail.fris.de ([116.203.77.234]:43232 "EHLO mail.fris.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1352922AbhJAKye (ORCPT ); Fri, 1 Oct 2021 06:54:34 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id 363A0BFC32; Fri, 1 Oct 2021 12:52:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fris.de; s=dkim; t=1633085567; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=6pYBgI+8NKm6cyLCUDJZUxM0MC4KtCLVKARxH3IY6YI=; b=M34EetMd2A5fYceHKi9nBMFj8eGcgra/MGvCQfMERj4Z5QnBX0dWDT389Qm7xjJ+Vmp9tM znCbQlnNxfBuGI0x1pQlGe/3fhFEIOx9XuiOAwkepLrrQG6Sg/iaLfi8AFLExiFqiNJzwM 9Q7DoZcGcKo8cd2vjAsSlvdPcUO6EJz3KlxeenB7DBeSFdJz+0qDCyH/vY9UU/+x77Rvni zgCNzrFe2zYnGB7PEysknu/M7lmdCJIx6lk63DgSwiq/P2NX2IpC+0Vms9DAMHKdz85j00 FA35Gdq532c7H4X7nJTjoyElqcSbDYNTHjqvO0Y+XIB5p4POk+dY/5VmVWabhQ== Message-ID: <25f24602-e3b4-197f-338b-167b67308f2c@fris.de> Date: Fri, 1 Oct 2021 12:52:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.1 Subject: Re: [PATCH 1/3] net: phy: mscc: Add possibilty to disable combined LED mode Content-Language: en-GB To: =?UTF-8?Q?Marek_Beh=c3=ban?= , Frieder Schrempf , linux-leds@vger.kernel.org Cc: Andrew Lunn , "David S. Miller" , Heiner Kallweit , Jakub Kicinski , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, Bjarni Jonasson , Ioana Ciornei , Russell King , Steen Hegelund References: <20210930125747.2511954-1-frieder@fris.de> <18de5e10-f41f-0790-89c8-3a70d48539be@kontron.de> <20211001120952.6be6bb36@thinkpad> From: Frieder Schrempf In-Reply-To: <20211001120952.6be6bb36@thinkpad> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Marek, On 01.10.21 12:09, Marek Behún wrote: > On Fri, 1 Oct 2021 11:20:36 +0200 > Frieder Schrempf wrote: > >> On 01.10.21 02:05, Andrew Lunn wrote: >>> On Thu, Sep 30, 2021 at 02:57:43PM +0200, Frieder Schrempf wrote: >>>> From: Frieder Schrempf >>>> >>>> By default the LED modes offer to combine two indicators like speed/link >>>> and activity in one LED. In order to use a LED only for the first of the >>>> two modes, the combined feature needs to be disabled. >>>> >>>> In order to do this we introduce a boolean devicetree property >>>> 'vsc8531,led-[N]-combine-disable' and wire it up to the matching >>>> bits in the LED behavior register. >>> >>> Sorry, but no DT property. Each PHY has its own magic combination of >>> DT properties, nothing shared, nothing common. This does not scale. >>> >>> Please look at the work being done to control PHY LEDs using the Linux >>> LED infrastructure. That should give us one uniform interface for all >>> PHY LEDs. >> >> +Cc: Marek >> >> I guess you are referring to this: [1]? >> >> If so, the last version I could find is a year old now. Is anyone still >> working on this? > > Yes, I am still working on this. > > Anyway the last version is not one year old, the last version to add > this support is 4 months old: > https://lore.kernel.org/netdev/20210602144439.4d20b295@dellmb/T/ Thanks for pointing out the latest patches. Good to know that you are still working on this. > > This version does not add the code for ethernet PHYs, instead it just > tries to touch only the LED subsystem by adding the API for offloading > LED triggers and an example implementation for Turris Omnia LED > controller. > > I will probably send another version this weekend. Sorry this takes > this long. No worries, and thanks for the work! > > >> I understand, that the generic approach is the one we want to have, but >> does this really mean adding PHY led configuration via DT to existing >> drivers (that already use DT properties for LED modes) is not accepted >> anymore, even if the new API is not yet in place? > > I don't know about Rob, but I would be against it. > > But if you need to have your PHY LED configured with via devicetree > ASAP, instead of proposing the vendor specific property, you can > propose LED subnodes and properties that will be generic and compatible > with the LED subsystem API, i.e. something like: > > ethernet-phy@1 { > .... eth phy properties; > > leds { > led@0 { > reg = <0>; > color = ; > /* this LED should indicate link/speed */ > function = LED_FUNCTION_LINK; > }; > }; > } > > Then make your PHY driver parse this, and make it so that if > function is LED_FUNCTION_LINK or LED_FUNCTION_ACTIVITY, the driver will > disable combined mode. > > Afterwards, when LED subsystem has support for trigger offloading, you > can update mscc driver so that instead of just disabling combined mode, > it will register the LEDs via LED subsystem... Good idea, but I'm not really in a hurry. Now knowing that work on the trigger offloading is still active, I guess I will just wait a bit until the dust has settled and maybe the bindings have been defined. Then I can try to implement this in the PHY driver. Thanks Frieder