Re: [PATCH/RFC v10 15/19] media: Add registration helpers for V4L2 flash sub-devices
From: Pavel Machek <hidden>
Date: 2015-01-12 13:27:37
Also in:
linux-leds, linux-media, lkml
Hi!
quoted
quoted
+ * the state of V4L2_CID_FLASH_INDICATOR_INTENSITY control only. + * Therefore it must be possible to set it to 0 level which in + * the LED subsystem reflects LED_OFF state. + */ + if (cdata_id != INDICATOR_INTENSITY) + ++__intensity;And normally we'd do i++ instead of ++i, and avoid __ for local variables...?Pre-incrementation operator is favourable over the post-incrementation one if we don't want to have an access to the value of a variable before incrementation, which is the case here.
That may be some old C++ convention, but I'm pretty sure gcc does not care.
Maybe gcc detects the cases when the value of a variable is not assigned and doesn't copy it before incrementing, however I haven't found any reference. I see that often in the for loops the i++ version is used, but I am not sure if this is done because developers are aware that gcc will optimize it anyway or it is just an omission.
The code is equivalent, and normally the n++ version is used. gcc will get it right.
quoted
quoted
+struct v4l2_flash_ctrl_config { + struct v4l2_ctrl_config intensity; + struct v4l2_ctrl_config flash_intensity; + struct v4l2_ctrl_config flash_timeout; + u32 flash_faults; + bool has_external_strobe:1; + bool indicator_led:1; +};I don't think you are supposed to do boolean bit arrays.These bit fields allow to reduce memory usage. If they were not bit fields, the address of the next variable would be aligned to the multiply of the CPU word size. Please look e.g. at struct dev_pm_info in the file include/linux/pm.h. It also contains boolean bit fields.
Looks like you are right. I guess I confused bool foo:1 with int foo:1. Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html