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=-5.3 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_2 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 49E1FC4338F for ; Mon, 9 Aug 2021 18:57:10 +0000 (UTC) Received: from shelob.surriel.com (shelob.surriel.com [96.67.55.147]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 96A5860FE3 for ; Mon, 9 Aug 2021 18:57:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 96A5860FE3 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kernelnewbies.org Received: from localhost ([::1] helo=shelob.surriel.com) by shelob.surriel.com with esmtp (Exim 4.94.2) (envelope-from ) id 1mDASB-0000eK-U3; Mon, 09 Aug 2021 14:56:51 -0400 Received: from mail.kernel.org ([198.145.29.99]) by shelob.surriel.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mDASA-0000dz-2s for kernelnewbies@kernelnewbies.org; Mon, 09 Aug 2021 14:56:50 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 862FB60F11; Mon, 9 Aug 2021 18:56:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1628535408; bh=Club1rpvylBf8gX0nH4hqgxX2CQBVQEJTP5FNvLKgAk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=ANWNU9Tm1mKTkj9B/l+QFWhzd7r/JBDVhmaDmwjWAHIfKqdJAN/jzm1orZCfZP5Wl t0OdJAU3y+ZCBGkiYl5lnLnC160AZGO+lWsZEhHjg2B0M+tZNQG9YPH1oKLA+dfqq7 y9Fu45aXst02HJVczZl2JshcolbV1AIGDoEtKLlcRD4YVj7Y8P/GVO9gKhxJXWJbTM NAKlzpgG3xxZsv6Qxk+bpusM+j8o1L2+y0b/UvNmGsr57+dnKodlo1/ztipg7iZ6EL peGna2Dl7YW2cdd+fSqDh+d5Xn9pc8kPYRrGtAikq4nbS2m78IihhCF770eCs72jgI SKq3tFfNBmhdA== Date: Mon, 9 Aug 2021 20:56:33 +0200 From: Marek =?UTF-8?B?QmVow7pu?= To: Ian Pilcher , pali@kernel.org Subject: Re: [RFC PATCH v2 00/10] Add configurable block device LED triggers Message-ID: <20210809205633.4300bbea@thinkpad> In-Reply-To: <20210809033217.1113444-1-arequipeno@gmail.com> References: <20210809033217.1113444-1-arequipeno@gmail.com> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Cc: axboe@kernel.dk, kernelnewbies@kernelnewbies.org, linux-kernel@vger.kernel.org, linux-block@vger.kernel.org, pavel@ucw.cz, linux-leds@vger.kernel.org X-BeenThere: kernelnewbies@kernelnewbies.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Learn about the Linux kernel List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kernelnewbies-bounces@kernelnewbies.org Hello Ian, thank you for your proposal. Some comments below: On Sun, 8 Aug 2021 22:32:07 -0500 Ian Pilcher wrote: > One thing that has not changed is that associations between block > devices and LEDs are still set via an attribute on the device, rather > than the LED. This is much simpler, as the device attribute only has > to handle a single value (the name of the associated LED), rather than > potentially handling multiple device names. It may be simpler, but it is in contrast to how the netdev trigger works, which already is in upstream for many years. I really think we should try to have similar sysfs ABIs here. (I understand that the netdev trigger is currently unable to handle multiple network interfaces - but it is possible to extend it so.) > I have modeled the interface for the /sys/block//led > attribute on the sysfs interface used for selecting a trigger. All > available LEDs (all LEDs associated with the blkdev trigger) are > shown when the attribute is read, with the currently selected LED > enclosed in square brackets ([]). I think it is reasonable to be able to set something like this: led0 : blink on activity on any of [sda, sdb, sdc] led1 : blink on activity on sda led2 : blink on activity on sdb led3 : blink on activity on sdc If I am reading your code correctly, it looks that only one LED can be configured for a block device. Is this true? If so, then the above configuration cannot be set. Also you are blinking the LED on any request to the block device. I would rather expect to be able to set the LED to blink on read and on write. (And possibly on other functions, like discard, or critical temperature, or error, ...) I would like to know what other people think about this. Marek _______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies