All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pavel Machek <pavel@ucw.cz>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: gregkh@linuxfoundation.org, linuxarm@huawei.com,
	mauro.chehab@huawei.com,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	linux-leds@vger.kernel.org, linux-staging@lists.linux.dev
Subject: Re: [PATCH 17/17] staging: nuc-led: update the TODOs
Date: Mon, 17 May 2021 10:05:27 +0200	[thread overview]
Message-ID: <20210517080527.GA18642@amd> (raw)
In-Reply-To: <20210517083001.7688acd7@coco.lan>

[-- Attachment #1: Type: text/plain, Size: 2505 bytes --]

Hi!

> > > -  Need to validate the uAPI and document it before moving
> > >    this driver out of staging.  
> > 
> > >  - Stabilize and document its sysfs interface.  
> >    
> > Would you mind starting with this one?
> 
> Do you mean writing the ABI document for it? Surely I can do that,
> but I'm not sure where to put such document while it is on staging.

No need for formal ABI just yet, but it needs to be clear what the interface
is.

> > We should have existing APIs
> > for most of functionality described...
> 
> I tried to stay as close as possible to the existing API, but
> there are some things that required a different one.

I believe it should be possible to move _way_ closer to existing APIs.

> For instance, with WMI rev 0.64 and 1.0, any LED of the device
> can be programmed to be a power indicator.
> 
> When a LED is programmed this way, there are up to 3 (on rev 1.0) or up
> to 4 (on rev 0.64) different brightness level of the LED, and those
> are associated with a power status (like S0, S3, S5, "ready mode").

I'll need a description how this works.

> 	/sys/class/leds/nuc::front1/
> 	├── blink_behavior
> 	├── blink_frequency

We have timer trigger for these.

> 	├── ethernet_type
> 	├── hdd_default
> 	├── indicator
> 	├── ready_mode_blink_behavior
> 	├── ready_mode_blink_frequency
> 	├── ready_mode_brightness
> 	├── s0_blink_behavior
> 	├── s0_blink_frequency
> 	├── s0_brightness
> 	├── s3_blink_behavior
> 	├── s3_blink_frequency
> 	├── s3_brightness
> 	├── s5_blink_behavior
> 	├── s5_blink_frequency
> 	├── s5_brightness

No. Take a look at triggers; for example hdd monitor should look very
much like existing disk trigger.

> On other words, the "indicator" tells what type of hardware event
> the LED is currently measuring:
> 
> 	$ cat /sys/class/leds/nuc\:\:front1/indicator 
> 	Power State  [HDD Activity]  Ethernet  WiFi  Software  Power Limit  Disable  

So this will use existing "trigger" infrastructure.

> That should likely be easier to discuss if any changes at the
> ABI would be needed. Before moving it out of staging, I would
> add another one under Documentation/ABI describing the meaning
> of each sysfs node.

Lets get reasonable ABI before moving it _into_ tree, staging or
otherwise.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Pavel Machek <pavel@ucw.cz>
To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
	linux-staging@lists.linux.dev, linuxarm@huawei.com,
	linux-kernel@vger.kernel.org, mauro.chehab@huawei.com,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-leds@vger.kernel.org
Subject: Re: [PATCH 17/17] staging: nuc-led: update the TODOs
Date: Mon, 17 May 2021 10:05:27 +0200	[thread overview]
Message-ID: <20210517080527.GA18642@amd> (raw)
In-Reply-To: <20210517083001.7688acd7@coco.lan>


[-- Attachment #1.1: Type: text/plain, Size: 2505 bytes --]

Hi!

> > > -  Need to validate the uAPI and document it before moving
> > >    this driver out of staging.  
> > 
> > >  - Stabilize and document its sysfs interface.  
> >    
> > Would you mind starting with this one?
> 
> Do you mean writing the ABI document for it? Surely I can do that,
> but I'm not sure where to put such document while it is on staging.

No need for formal ABI just yet, but it needs to be clear what the interface
is.

> > We should have existing APIs
> > for most of functionality described...
> 
> I tried to stay as close as possible to the existing API, but
> there are some things that required a different one.

I believe it should be possible to move _way_ closer to existing APIs.

> For instance, with WMI rev 0.64 and 1.0, any LED of the device
> can be programmed to be a power indicator.
> 
> When a LED is programmed this way, there are up to 3 (on rev 1.0) or up
> to 4 (on rev 0.64) different brightness level of the LED, and those
> are associated with a power status (like S0, S3, S5, "ready mode").

I'll need a description how this works.

> 	/sys/class/leds/nuc::front1/
> 	├── blink_behavior
> 	├── blink_frequency

We have timer trigger for these.

> 	├── ethernet_type
> 	├── hdd_default
> 	├── indicator
> 	├── ready_mode_blink_behavior
> 	├── ready_mode_blink_frequency
> 	├── ready_mode_brightness
> 	├── s0_blink_behavior
> 	├── s0_blink_frequency
> 	├── s0_brightness
> 	├── s3_blink_behavior
> 	├── s3_blink_frequency
> 	├── s3_brightness
> 	├── s5_blink_behavior
> 	├── s5_blink_frequency
> 	├── s5_brightness

No. Take a look at triggers; for example hdd monitor should look very
much like existing disk trigger.

> On other words, the "indicator" tells what type of hardware event
> the LED is currently measuring:
> 
> 	$ cat /sys/class/leds/nuc\:\:front1/indicator 
> 	Power State  [HDD Activity]  Ethernet  WiFi  Software  Power Limit  Disable  

So this will use existing "trigger" infrastructure.

> That should likely be easier to discuss if any changes at the
> ABI would be needed. Before moving it out of staging, I would
> add another one under Documentation/ABI describing the meaning
> of each sysfs node.

Lets get reasonable ABI before moving it _into_ tree, staging or
otherwise.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

  reply	other threads:[~2021-05-17  8:05 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-16 10:53 [PATCH 00/17] Add an experimental driver for Intel NUC leds Mauro Carvalho Chehab
2021-05-16 10:53 ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 01/17] staging: add support for NUC WMI LEDs Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 16:12   ` Randy Dunlap
2021-05-16 16:12     ` Randy Dunlap
2021-05-17  8:20   ` Greg KH
2021-05-17  8:20     ` Greg KH
2021-05-16 10:53 ` [PATCH 02/17] staging: nuc-wmi: detect WMI API detection Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-17  9:35   ` Dan Carpenter
2021-05-17  9:35     ` Dan Carpenter
2021-05-16 10:53 ` [PATCH 03/17] staging: nuc-wmi: add support for changing S0 brightness Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 04/17] staging: nuc-wmi: add all types of brightness Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 05/17] staging: nuc-wmi: allow changing the LED colors Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 06/17] staging: nuc-wmi: add support for WMI API version 1.0 Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 07/17] staging: nuc-wmi: add basic support for NUC6 WMI Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-17  9:44   ` Dan Carpenter
2021-05-17  9:44     ` Dan Carpenter
2021-05-16 10:53 ` [PATCH 08/17] staging: muc-wmi: add brightness and color for NUC6 API Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 09/17] staging: nuc-wmi: Add support to blink behavior for NUC8/10 Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 10/17] staging: nuc-wmi: get rid of an unused variable Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 11/17] staging: nuc-wmi: implement blink control for NUC6 Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 12/17] staging: nuc-wmi: better detect NUC6/NUC7 devices Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 13/17] staging: nuc-led: add support for HDD activity default Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 14/17] staging: nuc-wmi: fix software blink behavior logic Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 15/17] staging: nuc-wmi: add support for changing the ethernet type indicator Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 16/17] staging: nuc-wmi: add support for changing the power limit scheme Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 10:53 ` [PATCH 17/17] staging: nuc-led: update the TODOs Mauro Carvalho Chehab
2021-05-16 10:53   ` Mauro Carvalho Chehab
2021-05-16 18:21   ` Pavel Machek
2021-05-16 18:21     ` Pavel Machek
2021-05-17  6:30     ` Mauro Carvalho Chehab
2021-05-17  6:30       ` Mauro Carvalho Chehab
2021-05-17  8:05       ` Pavel Machek [this message]
2021-05-17  8:05         ` Pavel Machek
2021-05-17  8:57         ` Mauro Carvalho Chehab
2021-05-17  8:57           ` Mauro Carvalho Chehab
2021-05-17  9:12           ` Mauro Carvalho Chehab
2021-05-17  9:12             ` Mauro Carvalho Chehab
2021-05-17 12:19         ` Mauro Carvalho Chehab
2021-05-17 12:19           ` Mauro Carvalho Chehab
2021-05-17  8:18 ` [PATCH 00/17] Add an experimental driver for Intel NUC leds Greg KH
2021-05-17  8:18   ` Greg KH
2021-05-17  9:02   ` Mauro Carvalho Chehab
2021-05-17  9:02     ` Mauro Carvalho Chehab
2021-05-17  9:08     ` Greg KH
2021-05-17  9:08       ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210517080527.GA18642@amd \
    --to=pavel@ucw.cz \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linuxarm@huawei.com \
    --cc=mauro.chehab@huawei.com \
    --cc=mchehab+huawei@kernel.org \
    --cc=mchehab@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.