linux-kernel - Re: [PATCH 17/17] staging: nuc-led: update the TODOs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210517080527.GA18642@amd>
Date:   Mon, 17 May 2021 10:05:27 +0200
From:   Pavel Machek <pavel@....cz>
To:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc:     gregkh@...uxfoundation.org, linuxarm@...wei.com,
        mauro.chehab@...wei.com,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
        linux-leds@...r.kernel.org, linux-staging@...ts.linux.dev
Subject: Re: [PATCH 17/17] staging: nuc-led: update the TODOs

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

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)