From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jacek Anaszewski Subject: Re: [PATCH v2 2/2] leds/powernv: Add driver for PowerNV platform Date: Thu, 16 Apr 2015 13:34:17 +0200 Message-ID: <552F9E39.4050801@samsung.com> References: <20150320105921.14866.83209.stgit@localhost.localdomain> <20150320110328.14866.98225.stgit@localhost.localdomain> <552D303A.5080506@samsung.com> <552E049D.2030604@linux.vnet.ibm.com> <552E2480.9060102@samsung.com> <552E3A56.8030300@linux.vnet.ibm.com> <552E63D2.4070209@samsung.com> <552F5C1A.3060701@linux.vnet.ibm.com> <552F7807.9080605@samsung.com> <552F8E48.6070903@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-reply-to: <552F8E48.6070903@linux.vnet.ibm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" To: Vasant Hegde Cc: stewart@linux.vnet.ibm.com, cooloney@gmail.com, rpurdie@rpsys.net, linuxppc-dev@lists.ozlabs.org, linux-leds@vger.kernel.org, khandual@linux.vnet.ibm.com List-Id: linux-leds@vger.kernel.org T24gMDQvMTYvMjAxNSAxMjoyNiBQTSwgVmFzYW50IEhlZ2RlIHdyb3RlOgo+IE9uIDA0LzE2LzIw MTUgMDI6MjEgUE0sIEphY2VrIEFuYXN6ZXdza2kgd3JvdGU6Cj4+IEhpIFZhc2FudCwKPj4KPj4g T24gMDQvMTYvMjAxNSAwODo1MiBBTSwgVmFzYW50IEhlZ2RlIHdyb3RlOgo+Pj4gT24gMDQvMTUv MjAxNSAwNjo0MiBQTSwgSmFjZWsgQW5hc3pld3NraSB3cm90ZToKPj4+PiBPbiAwNC8xNS8yMDE1 IDEyOjE1IFBNLCBWYXNhbnQgSGVnZGUgd3JvdGU6Cj4+Pj4+IE9uIDA0LzE1LzIwMTUgMDI6MTIg UE0sIEphY2VrIEFuYXN6ZXdza2kgd3JvdGU6Cj4+Pj4+PiBIaSBWYXNhbnQsCj4+Pgo+Pj4gSGkg SmFjZWssCj4+Pgo+Pj4gLi4uLy4uLgo+Pj4KPj4+Pj4+Pj4KPj4+Pj4+Pgo+Pj4+Pj4+IEkgbWVh biwgd2UgaGF2ZSB0byByZXRhaW4gdGhlIHN0YXRlIG9mIExFRCBhY3Jvc3Mgc3lzdGVtIHJlYm9v dC4KPj4+Pj4+Cj4+Pj4+PiBTdGF0aWMgdmFyaWFibGVzIGFyZSByZWluaXRpYWxpemVkIG9uIHN5 c3RlbSByZWJvb3QsIGFyZW4ndCB0aGV5Pwo+Pj4+Pgo+Pj4+PiBTb3JyeS4gSSB0aGluayBjb21t ZW50IHdhcyBjb25mdXNpbmcuLgo+Pj4+Pgo+Pj4+PiBBcyBJIHVuZGVyc3Rvb2QsIGNsYXNzZGV2 X3VucmVnaXN0ZXIgY2FsbCByZXNldHMgYWxsIExFRHMgc3RhdGUuIEhvd2V2ZXIgaW4gb3VyCj4+ Pj4+IGNhc2UsIHdlIGRvbid0Cj4+Pj4+IHdhbnQgdG8gY2hhbmdlIHRoZSBMRUQgc3RhdGUgZHVy aW5nIHN5c3RlbSBzaHV0ZG93bi9yZWJvb3QuCj4+Pj4+Cj4+Pj4+IEhlbmNlIEkgaGF2ZSBpbnRy b2R1Y2VkIHN0YXRlIHZhcmlhYmxlIGhlcmUuIFNvIGR1cmluZyByZWdpc3RlciBjYWxsLCBJIGp1 c3QKPj4+Pj4gZGlzYWJsZSBMRURzIHNvIHRoYXQgYW55IGZ1cnRoZXIgY2FsbCB3aWxsIGp1c3Qg cmV0dXJuLiBUaGF0IHdheSB3ZSByZXRhaW4gTEVECj4+Pj4+IHN0YXRlIGV2ZW4gYWZ0ZXIgdW5s b2FkaW5nIG1vZHVsZS4KPj4+Pj4KPj4+Pj4gUGxlYXNlIGxldCBtZSBrbm93IGlmIHRoZXJlIGlz IGFueSBiZXR0ZXIgd2F5IHRvIGFjaGlldmUgdGhpcy4KPj4+Pgo+Pj4+IFNpbmNlIHRoaXMgaXMg bm90IGEgZmVhdHVyZSBvZiB0aGUgZGV2aWNlLCBidXQgcmF0aGVyIGEgdXNlIGNhc2UsIHRoZW4K Pj4+PiBpdCBjYW5ub3QgYmUgaGFyZCBjb2RlZCBpbiB0aGUgZHJpdmVyIHRoaXMgd2F5LiBUaGUg c29sdXRpb24gSSBzZWUgaXMKPj4+PiBpbnRyb2R1Y2luZyBhIHN5c2ZzIGF0dHJpYnV0ZSB0aGF0 IHdvdWxkIGRldGVybWluZSBpZiB3ZSB3YW50IHRoZQo+Pj4+IExFRCB0byBiZSB0dXJuZWQgb2Zm IG9uIHVucmVnaXN0cmF0aW9uIG9yIG5vdC4KPj4+Cj4+PiBIbW1tLiBJIHRob3VnaHQgaGF2aW5n IHNpbXBsZSB2YXJpb3VzIGlzIGVub3VnaCAuLgo+Pj4KPj4+IEkgZG9uJ3Qga25vdyB0aGUgdXNh Z2Ugb2Ygb3RoZXIgTEVEIGRyaXZlcnMgLi4gSnVzdCBhIHRob3VnaHQgLi4gSG93IGFib3V0Cj4+ PiBlbmFibGluZyB0aGlzIGZlYXR1cmUgaW4gTEVEIGNsYXNzIGl0c2VsZiAuLi5Tb21ldGhpbmcg bGlrZToKPj4+ICAgICAtIEFkZCBuZXcgYXR0cmlidXRlIHRvIGdlbmVyaWMgTEVEIGNsYXNzIChz YXkgcGVyc2lzdGVudCk/Cj4+PiAgICAgLSBJZiBkcml2ZXJzIHdhbnRzIHRvIHJldGFpbiBMRUQg c3RhdGUgYWNyb3NzIHJlYm9vdCwgdGhlbiBlbmFibGUgdGhpcyBvcHRpb24KPj4+ICAgICAtIER1 cmluZyB1bnJlZ2lzdGVyIGNhbGwgY2hlY2sgZm9yIHRoaXMgYXR0cmlidXRlCj4+Pgo+Pj4+Cj4+ Pj4gWW91IGNhbiByZWZlciB0byB0aGUgZm9sbG93aW5nIGRyaXZlciB0byBmaW5kIG91dCBob3cg dG8gYWRkIHRoZQo+Pj4+IHN5c2ZzIGF0dHJpYnV0ZToKPj4+Pgo+Pj4+IGRyaXZlcnMvbGVkcy9s ZWRzLWxtMzUzMy5jCj4+Pj4KPj4+PiBUaGUgYXR0cmlidXRlIHdpbGwgYWxzbyBoYXZlIHRvIGJl IGRvY3VtZW50ZWQsIHNpbWlsYXJseSB0byB0aGVzZToKPj4+Pgo+Pj4+IERvY3VtZW50YXRpb24v QUJJL3Rlc3Rpbmcvc3lzZnMtY2xhc3MtbGVkLWRyaXZlci1sbTM1MzMKPj4+Pgo+Pj4+IEN1cnJl bnRseSBJIGRvbid0IGhhdmUgYSBnb29kIGNhbmRpZGF0ZSBmb3IgYXR0cmlidXRlCj4+Pj4gbmFt ZSwgc28gZmVlbCBmcmVlIHRvIHByb3Bvc2Ugb25lLgo+Pj4KPj4+IElmIHlvdSBkb24ndCBsaWtl IGdlbmVyaWMgYXR0cmlidXRlLCB0aGVuIHNoYWxsIEkgaW50cm9kdWNlICJwZXJzaXN0ZW50Igo+ Pj4gYXR0cmlidXRlIGluc2lkZSBteSBkcml2ZXIuID8KPj4+ICAgICAtIEJ5IGRlZmF1bHQgdGhp cyBhdHRyaWJ1dGUgaXMgT04gYW5kIGlmIHVzZXIgd2FudHMgaGUgY2FuIGRpc2FibGUgdGhpcyAu Cj4+PiAgICAgLSBBbmQgSSB3aWxsIGhhdmUgYW5vdGhlciB2YXJpYWJsZSAoc2F5IG9wX3N1cHBv cnQpLi4gd2hpY2ggSSB3aWxsIGRpc2FibGUgaW4KPj4+IHVubG9hZCBwYXRoLi4KPj4+IC4uLi8u Li4KPj4+Cj4+Pj4+Pgo+Pj4+Pj4gVGhlIGxhYmVsIGNvdWxkIGJlIGNvbXBvc2VkIG9mIHNlZ21l bnRzIGFuZCBhbiBvcmRpbmFsIG51bWJlciBhcwo+Pj4+Pj4gbGFiZWxzIGhhdmUgdG8gYmUgdW5p cXVlLCBlLmcuIGF0dG5faWRlbnRfMCwgYXR0bl9pZGVudF8xLgo+Pj4+Pj4gVGhlIHNlZ21lbnRz IHdvdWxkIGhhdmUgdG8gYmUgcGFyc2VkIGJ5IHRoZSBkcml2ZXIgdG8gZGlzY292ZXIKPj4+Pj4+ IGFsbCB0aGUgTEVEJ3MgYXZhaWxhYmxlIG1vZGVzLgo+Pj4+Pj4KPj4+Pj4+IG5pdHBpY2tpbmc6 IGlkZW50aWZ5IGlzIGEgdmVyYiBhbmQgaXMgbm90IGEgcHJvcGVyIG5hbWUgZm9yIHRoZSBMRUQu Cj4+Pj4+PiBDb3VsZCB5b3UgZGVzY3JpYmUgdGhlIHB1cnBvc2Ugb2YgdGhpcyBtb2RlLCBzbyB0 aGF0IHdlIGNvdWxkIGNvbWUKPj4+Pj4+IHVwIHdpdGggYSBiZXR0ZXIgbmFtZT8KPj4+Pj4KPj4+ Pj4gRWFjaCBjb21wb25lbnQgKEZpZWxkIFJlcGxhY2VtZW50IFVuaXQpIHdpbGwgaGF2ZSBzZXJ2 aWNlIGluZGljYXRvciAoTEVEUykKPj4+Pj4gd2hpY2gKPj4+Pj4gY2FuIGhhdmUgYmVsb3cgc3Rh dGVzIDoKPj4+Pj4gICAgICAtIE9GRiAgICAgOiBubyBhY3Rpb24KPj4+Pj4gICAgICAtIElkZW50 aWZ5OiBibGlua2luZyBzdGF0ZSAodXNlciBjYW4gdXNlIHRoaXMgc3RhdGUgdG8gaWRlbnRpZnkg cGFydGljdWxhcgo+Pj4+PiBjb21wb25lbnQpLgo+Pj4+PiAgICAgICAgICAgSW4gUG93ZXIgU3lz dGVtcyB3b3JsZCB3ZSBjYWxsIGl0IGFzICJpZGVudGlmeSIgaW5kaWNhdG9yLi4gSGVuY2UgSQo+ Pj4+PiByZXRhaW5lZCBzYW1lIG5hbWUgaGVyZS4KPj4+Pj4gICAgICAgICAgIEhvdyBhYm91dCBq dXN0ICJpZGVudCIgPwo+Pj4+Cj4+Pj4gU291bmRzIGdvb2QuCj4+Pj4KPj4+Pj4gICAgICAtIGZh dWx0IDogc29saWQgc3RhdGUgKHdoZW4gY29tcG9uZW50IGdvZXMgYmFkLCBMRUQgZ29lcyB0byBz b2xpZCBzdGF0ZSkKPj4+Pj4gICAgICAgICBOb3RlIHRoYXQgb3VyIEZXIGlzIGNhcGFibGUgb2Yg aXNvbGF0aW5nIHNvbWUgb2YgdGhlIGlzc3VlcyBhbmQgaXQKPj4+Pj4gY2FuIHR1cm4KPj4+Pj4g b24gTEVEcyB3aXRob3V0IE9TCj4+Pj4+ICAgICAgICAgIGludGVyZmVyZW5jZS4KPj4+Pgo+Pj4+ IERvZXMgaXQgbWVhbiB0aGF0IHRoZSBMRUQgY2FuIGJlIGNvbnRyb2xsZWQgZnJvbSBoYXJkd2Fy ZT8KPj4+PiBJZiBzbywgd2hhdCB3b3VsZCBiZSBzb2Z0d2FyZSB1c2UgY2FzZXMgdGhlbj8gVGhl IHNhbWUgcXVlc3Rpb24gaXMKPj4+PiByZWxhdGVkIHRvIHRoZSBhdHRuIGFuZCBpbmRlbnQgc3Rh dGVzLgo+Pj4KPj4+IC0gSWRlbnRpZnkgTEVEczoKPj4+ICAgICBXZSBoYXZlIHNlcnZpY2UgcHJv Y2Vzc29yIGludGVyZmFjZSB0byBzZXQvcmVzZXQgdGhpcyBMRURzLi4gU2ltaWxhciBvcGVyYXRp b24KPj4+IGNhbiBiZSBkb25lIGZyb20gaW5iYW5kIGludGVyZmFjZSBhcyB3ZWxsICh2aWEgdXNl ciBzcGFjZSB0b29scyBpbiBMaW51eCkuLgo+Pj4gTGF0ZXIgbWFuYWdlbWVudCBsYXllciBjYW4g bWFrZSB1c2Ugb2YgdGhpcy4KPj4+Cj4+PiAtIEZhdWx0IC8gQXR0ZW50aW9uIDoKPj4+ICAgICBG VyBjYW4gU0VUIHRoZXNlIExFRHMgaWYgaXRzIGNhcGFibGUgb2YgaXNvbGF0aW5nIHByb2JsZW0u Cj4+PiAgICAgU2ltaWxhcmx5IGhvc3QvdXNlciBzcGFjZSBjYW4gU0VUIHRoZXNlIExFRHMgaWYg dGhlbiBjYW4gZG8gZmluZSBncmFpbmVkCj4+PiBwcm9ibGVtIGlzb2xhdGlvbiBldGMuCj4+PiAg ICAgSG9zdC91c2VyIHNwYWNlIGNhbiBSRVNFVCB0aGVzZSBMRURzLgo+Pj4KPj4+IEhlbmNlIHdl IGFyZSBhZGRpbmcgaG9zdCBzdXBwb3J0IHRvIGFsbCB0aGUgTEVEcyB3aGljaCBjYW4gYmUgbW9k aWZpZWQgYnkgaG9zdC4KPj4+Cj4+PiBIb3BlIHRoaXMgY2xhcmlmaWVzLgo+Pgo+PiBUaGFua3Mg Zm9yIHRoaXMgZXhwbGFuYXRpb24uCj4+Cj4+IEkgYW0gY2hhbmdpbmcgbXkgbWluZCBhYm91dCB0 aGVzZSBMRURzLiBTaW5jZSB0aGV5IGNhbiBiZSBjb250cm9sbGVkCj4+IGZyb20gaGFyZHdhcmUg d2l0aG91dCBhbnkgc3lzdGVtIGludGVyYWN0aW9uLCB0aGVuIHR1cm5pbmcgdGhlbSBvZmYKPj4g b24gZHJpdmVyIHJlbW92YWwgbWFrZXMgbm8gc2Vuc2UuIFRoZSBMRURzIGNvdWxkIGJlIHR1cm5l ZCBvbiBhZ2FpbiBldmVuCj4+IGlmIHRoZSBkcml2ZXIgaXMgdW5sb2FkZWQuCj4+IFNpbmNlIHRo ZSBkcml2ZXIgZG9lc24ndCBwZXJmb3JtIGFueSBpbml0aWFsaXphdGlvbiBvZiB0aGUgTEVEcywK Pj4gbmVpdGhlciBzaG91bGQgaXQgdHVybiB0aGVtIG9mZiBvbiByZW1vdmFsLgo+Pgo+PiBJZiBJ IHVuZGVyc3RhbmQgdGhpcyBjb3JyZWN0bHksIHRoZW4gdGhlIHNvbHV0aW9uIHdpdGggdmFyaWFi bGUgd291bGQKPj4gZG8gYW5kIG5vIHN5c2ZzIGF0dHJpYnV0ZSB3b3VsZCBiZSByZXF1aXJlZC4K Pj4KPgo+IEphY2VrLAo+Cj4gVGhhbmtzLiBUaGVuIEkgd2lsbCByZXRhaW4gY3VycmVudCBtZXRo b2QuCj4KPiBPbmUgcXVlc3Rpb24uLldoYXQgaXMgdGhlIHByZWZlcnJlZCB3YXkgdG8gbmFtZSBM RUQgbm9kZSBpbiB0aGlzIGNhc2UgPwo+Cj4gICAgIDxsb2NhdGlvbl9jb2RlPjo8QVRURU5USU9O fElERU5USUZZfEZBVUxUPgo+ICAgICBPUgo+ICAgICA8bG9jYXRpb25fY29kZT4KPiAgICAgICAg ICBpZGVudCAgPC0gYXR0cmlidXRlIHVuZGVyIGVhY2ggbm9kZQo+ICAgICAgICAgIGZhdWx0Cj4g ICAgICAgICAgYXR0ZW50aW9uCgoKSWYgcG9zc2libGUgbG9jYXRpb25zIGFyZSBlY2xvc3VyZS9k ZXNjZW5kZW50IGFzIGluIHRoZSBkb2N1bWVudGF0aW9uCnlvdSBnYXZlIGEgcmVmZXJlbmNlIHRv LCB0aGVuIHRoZSByZWxhdGVkIGlkZW50aWZpZXJzIGNvdWxkIGJlOgoKZW5jbG9zdXJlOiBlbmNs CmRlc2NlbmRlbnQ6IGRlc2Mgb3IgZnJ1IChob3cgZG9lcyBmcnUgZXhwYW5kIGJ0dz8pCgoKQ2hp bGQgbm9kZXMgY291bGQgYmUgZGVmaW5lZCBhcyBmb2xsb3dzOgoKCmxlZDAgewkKCWxhYmVsID0g InBvd2VybnYwOmVuY2w6YXR0bjppZGVudDpmYXVsdCIKfQoKbGVkMSB7CQoJbGFiZWwgPSAicG93 ZXJudjE6ZW5jbDo6aWRlbnQ6ZmF1bHQiCn0KCmxlZDIgewkKCWxhYmVsID0gInBvd2VybnYyOmRl c2M6YXR0bjo6ZmF1bHQ6Igp9CgpsZWQyIHsJCglsYWJlbCA9ICJwb3dlcm52MzpkZXNjOjo6ZmF1 bHQ6Igp9CgotLSAKQmVzdCBSZWdhcmRzLApKYWNlayBBbmFzemV3c2tpCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkxpbnV4cHBjLWRldiBtYWlsaW5nIGxp c3QKTGludXhwcGMtZGV2QGxpc3RzLm96bGFicy5vcmcKaHR0cHM6Ly9saXN0cy5vemxhYnMub3Jn L2xpc3RpbmZvL2xpbnV4cHBjLWRldg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id D9FB71A0021 for ; Thu, 16 Apr 2015 21:34:23 +1000 (AEST) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout1.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NMW00HRJDNZAO60@mailout1.w1.samsung.com> for linuxppc-dev@lists.ozlabs.org; Thu, 16 Apr 2015 12:38:23 +0100 (BST) Message-id: <552F9E39.4050801@samsung.com> Date: Thu, 16 Apr 2015 13:34:17 +0200 From: Jacek Anaszewski MIME-version: 1.0 To: Vasant Hegde Subject: Re: [PATCH v2 2/2] leds/powernv: Add driver for PowerNV platform References: <20150320105921.14866.83209.stgit@localhost.localdomain> <20150320110328.14866.98225.stgit@localhost.localdomain> <552D303A.5080506@samsung.com> <552E049D.2030604@linux.vnet.ibm.com> <552E2480.9060102@samsung.com> <552E3A56.8030300@linux.vnet.ibm.com> <552E63D2.4070209@samsung.com> <552F5C1A.3060701@linux.vnet.ibm.com> <552F7807.9080605@samsung.com> <552F8E48.6070903@linux.vnet.ibm.com> In-reply-to: <552F8E48.6070903@linux.vnet.ibm.com> Content-type: text/plain; charset=UTF-8; format=flowed Cc: stewart@linux.vnet.ibm.com, cooloney@gmail.com, rpurdie@rpsys.net, linuxppc-dev@lists.ozlabs.org, linux-leds@vger.kernel.org, khandual@linux.vnet.ibm.com List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 04/16/2015 12:26 PM, Vasant Hegde wrote: > On 04/16/2015 02:21 PM, Jacek Anaszewski wrote: >> Hi Vasant, >> >> On 04/16/2015 08:52 AM, Vasant Hegde wrote: >>> On 04/15/2015 06:42 PM, Jacek Anaszewski wrote: >>>> On 04/15/2015 12:15 PM, Vasant Hegde wrote: >>>>> On 04/15/2015 02:12 PM, Jacek Anaszewski wrote: >>>>>> Hi Vasant, >>> >>> Hi Jacek, >>> >>> .../... >>> >>>>>>>> >>>>>>> >>>>>>> I mean, we have to retain the state of LED across system reboot. >>>>>> >>>>>> Static variables are reinitialized on system reboot, aren't they? >>>>> >>>>> Sorry. I think comment was confusing.. >>>>> >>>>> As I understood, classdev_unregister call resets all LEDs state. However in our >>>>> case, we don't >>>>> want to change the LED state during system shutdown/reboot. >>>>> >>>>> Hence I have introduced state variable here. So during register call, I just >>>>> disable LEDs so that any further call will just return. That way we retain LED >>>>> state even after unloading module. >>>>> >>>>> Please let me know if there is any better way to achieve this. >>>> >>>> Since this is not a feature of the device, but rather a use case, then >>>> it cannot be hard coded in the driver this way. The solution I see is >>>> introducing a sysfs attribute that would determine if we want the >>>> LED to be turned off on unregistration or not. >>> >>> Hmmm. I thought having simple various is enough .. >>> >>> I don't know the usage of other LED drivers .. Just a thought .. How about >>> enabling this feature in LED class itself ...Something like: >>> - Add new attribute to generic LED class (say persistent)? >>> - If drivers wants to retain LED state across reboot, then enable this option >>> - During unregister call check for this attribute >>> >>>> >>>> You can refer to the following driver to find out how to add the >>>> sysfs attribute: >>>> >>>> drivers/leds/leds-lm3533.c >>>> >>>> The attribute will also have to be documented, similarly to these: >>>> >>>> Documentation/ABI/testing/sysfs-class-led-driver-lm3533 >>>> >>>> Currently I don't have a good candidate for attribute >>>> name, so feel free to propose one. >>> >>> If you don't like generic attribute, then shall I introduce "persistent" >>> attribute inside my driver. ? >>> - By default this attribute is ON and if user wants he can disable this . >>> - And I will have another variable (say op_support).. which I will disable in >>> unload path.. >>> .../... >>> >>>>>> >>>>>> The label could be composed of segments and an ordinal number as >>>>>> labels have to be unique, e.g. attn_ident_0, attn_ident_1. >>>>>> The segments would have to be parsed by the driver to discover >>>>>> all the LED's available modes. >>>>>> >>>>>> nitpicking: identify is a verb and is not a proper name for the LED. >>>>>> Could you describe the purpose of this mode, so that we could come >>>>>> up with a better name? >>>>> >>>>> Each component (Field Replacement Unit) will have service indicator (LEDS) >>>>> which >>>>> can have below states : >>>>> - OFF : no action >>>>> - Identify: blinking state (user can use this state to identify particular >>>>> component). >>>>> In Power Systems world we call it as "identify" indicator.. Hence I >>>>> retained same name here. >>>>> How about just "ident" ? >>>> >>>> Sounds good. >>>> >>>>> - fault : solid state (when component goes bad, LED goes to solid state) >>>>> Note that our FW is capable of isolating some of the issues and it >>>>> can turn >>>>> on LEDs without OS >>>>> interference. >>>> >>>> Does it mean that the LED can be controlled from hardware? >>>> If so, what would be software use cases then? The same question is >>>> related to the attn and indent states. >>> >>> - Identify LEDs: >>> We have service processor interface to set/reset this LEDs.. Similar operation >>> can be done from inband interface as well (via user space tools in Linux).. >>> Later management layer can make use of this. >>> >>> - Fault / Attention : >>> FW can SET these LEDs if its capable of isolating problem. >>> Similarly host/user space can SET these LEDs if then can do fine grained >>> problem isolation etc. >>> Host/user space can RESET these LEDs. >>> >>> Hence we are adding host support to all the LEDs which can be modified by host. >>> >>> Hope this clarifies. >> >> Thanks for this explanation. >> >> I am changing my mind about these LEDs. Since they can be controlled >> from hardware without any system interaction, then turning them off >> on driver removal makes no sense. The LEDs could be turned on again even >> if the driver is unloaded. >> Since the driver doesn't perform any initialization of the LEDs, >> neither should it turn them off on removal. >> >> If I understand this correctly, then the solution with variable would >> do and no sysfs attribute would be required. >> > > Jacek, > > Thanks. Then I will retain current method. > > One question..What is the preferred way to name LED node in this case ? > > : > OR > > ident <- attribute under each node > fault > attention If possible locations are eclosure/descendent as in the documentation you gave a reference to, then the related identifiers could be: enclosure: encl descendent: desc or fru (how does fru expand btw?) Child nodes could be defined as follows: led0 { label = "powernv0:encl:attn:ident:fault" } led1 { label = "powernv1:encl::ident:fault" } led2 { label = "powernv2:desc:attn::fault:" } led2 { label = "powernv3:desc:::fault:" } -- Best Regards, Jacek Anaszewski