LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jacek Anaszewski <j.anaszewski@samsung.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: Greg KH <greg@kroah.com>,
	kernel list <linux-kernel@vger.kernel.org>,
	linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
	kyungmin.park@samsung.com, b.zolnierkie@samsung.com,
	cooloney@gmail.com, rpurdie@rpsys.net, sakari.ailus@iki.fi,
	s.nawrocki@samsung.com
Subject: Re: Reading /sys with side effects (was Re: [PATCH 1/2] Documentation: leds: Add description of LED Flash class extension)
Date: Wed, 28 Jan 2015 09:43:28 +0100	[thread overview]
Message-ID: <54C8A130.8000807@samsung.com> (raw)
In-Reply-To: <20150127221958.GA18993@amd>

Hi Pavel,

On 01/27/2015 11:37 PM, Pavel Machek wrote:
> Hi!
>
>> +	- flash_fault - list of flash faults that may have occurred:
>> +		* led-over-voltage - flash controller voltage to the flash LED
>> +			has exceededthe limit specific to the flash controller
>> +		* flash-timeout-exceeded - the flash strobe was still on when
>> +			the timeout set by the user has expired; not all flash
>> +			controllers may set this in all such conditions
>> +		* controller-over-temperature - the flash controller has
>> +			overheated
>> +		* controller-short-circuit - the short circuit protection
>> +			of the flash controller has been triggered
>> +		* led-power-supply-over-current - current in the LED power
>> +			supply has exceeded the limit specific to the flash
>> +			controller
>> +		* indicator-led-fault - the flash controller has detected
>> +			a short or open circuit condition on the indicator LED
>> +		* led-under-voltage - flash controller voltage to the flash
>> +			LED has been below the minimum limit specific to
>> +			the flash
>> +		* controller-under-voltage - the input voltage of the flash
>> +			controller is below the limit under which strobing the
>> +			flash at full current will not be possible. The condition
>> +			persists until this flag is no longer set
>> +		* led-over-temperature - the temperature of the LED has exceeded
>> +			its allowed upper limit
>> +
>> +		Flash faults are cleared, if possible, by reading the attribute.
>
> That's bad. Now you can no longer present flash_fault file as readable
> to non-root users, and grep -ri foo /sys will interfere with your
> camera application.
>
> Bad interface, just fix it.

In my opinion it isn't crucial for the user to be aware of the
fact that some non-persistent fault happened right after strobing the
flash (e.g. over temperature).

I cannot see anything harmful in the situation when someone does grep
on /sys and clears non-persistent fault on a flash LED device.

Also, not all devices may be able to report the faults that happened
earlier but are not valid at the time of I2C readout. In that case the
user will never now that the fault has ever occurred, unless they read
the flash_fault attribute at the proper moment.

In this case we cannot enforce consistent policy for all devices.

Please describe the use case when clearing the fault on read can be
harmful, if you have any.

Moreover, I don't see your reply to Sakari's message [1], where he
considers the problem from several perspectives.

[1] http://www.spinics.net/lists/linux-leds/msg02653.html

-- 
Best Regards,
Jacek Anaszewski

  reply	other threads:[~2015-01-28 20:53 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1422346028-16739-1-git-send-email-j.anaszewski@samsung.com>
2015-01-27 22:37 ` Pavel Machek
2015-01-28  8:43   ` Jacek Anaszewski [this message]
2015-01-29 21:14     ` Pavel Machek
2015-01-30  8:55       ` Jacek Anaszewski
2015-01-30 16:40         ` Greg KH
2015-02-02  9:07           ` Jacek Anaszewski
2015-02-02  9:44             ` Pavel Machek
2015-02-02 11:55               ` Jacek Anaszewski
2015-02-02 13:51                 ` Pavel Machek
2015-02-02 14:51                   ` Jacek Anaszewski

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=54C8A130.8000807@samsung.com \
    --to=j.anaszewski@samsung.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=cooloney@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=greg@kroah.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=pavel@ucw.cz \
    --cc=rpurdie@rpsys.net \
    --cc=s.nawrocki@samsung.com \
    --cc=sakari.ailus@iki.fi \
    --subject='Re: Reading /sys with side effects (was Re: [PATCH 1/2] Documentation: leds: Add description of LED Flash class extension)' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).