On Tue, Jan 29, 2013 at 09:17:04AM +0100, Peter Ujfalusi wrote: > On 01/28/2013 10:01 PM, Thierry Reding wrote: > > On Tue, Jan 22, 2013 at 02:39:53PM +0100, Peter Ujfalusi wrote: > >> It is expected that board files would have: > >> static unsigned int bl_levels[] = { 0, 50, 100, 150, 200, 250, }; > >> > >> static struct platform_pwm_backlight_data bl_data = { > >> .levels = bl_levels, > >> .max_brightness = ARRAY_SIZE(bl_levels), > >> .dft_brightness = 4, > >> .pwm_period_ns = 7812500, > >> }; > >> > >> In this case the max_brightness would be out of range in the levels array. > >> Decrement the received max_brightness in every case (DT or non DT) when the > >> levels has been provided. > > > > What's wrong with specifying .max_brightness = ARRAY_SIZE(bl_levels) - 1 > > instead? > > There is nothing wrong with that either but IMHO it is more natural for board > files to use just ARRAY_SIZE(bl_levels). In this way the handling of > data->max_brightness among non DT and DT booted kernel is more uniform in the > driver itself. The .max_brightness field is defined to be the maximum value that you can set the brightness to. If you use .levels and set .max_brightness to ARRAY_SIZE() then that's no longer true because the maximum value for the brightness will actually be ARRAY_SIZE() - 1. The reason why in the DT case we decrement .max_brightness is that it is used as a temporary variable to keep the number of levels, which corresponds to ARRAY_SIZE() in your example, and adjust it later on to match the definition. That's an implementation detail. Platform data content should be encoded properly without knowledge of the internal implementation. > Right now all board files are using only the .max_brightness to specify the > maximum value, I could not find any users of .levels in the kernel. Yes. But this is the legacy mode which should be considered deprecated. The reason why the concept of brightness-levels was introduced back when the DT bindings were created was that pretty much no hardware in existence actually works that way. This was a topic of discussion at the time when I first proposed these changes, see for example: http://www.mail-archive.com/devicetree-discuss@xxxxxxxxxxxxxxxx/msg09472.html Thierry
Attachment:
pgpoRP5vbFTBE.pgp
Description: PGP signature