Re: [PATCH] backlight: Add TPS65217 WLED driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi AnilKumar,

El Wed, Aug 08, 2012 at 09:25:29AM +0000 AnilKumar, Chimata ha dit:

> Cross check with mfd/master also.

ok

> > > > +			if (!of_property_read_u32(np, "fdim", &val)) {
> > > > +				if (val == 100) {
> > > > +					pdata->bl_pdata->fdim = TPS65217_BL_FDIM_100HZ;
> > > > +				} else if (val == 200) {
> > > > +					pdata->bl_pdata->fdim = TPS65217_BL_FDIM_200HZ;
> > > > +				} else if (val == 500) {
> > > > +					pdata->bl_pdata->fdim = TPS65217_BL_FDIM_500HZ;
> > > > +				} else if (val == 1000) {
> > > > +					pdata->bl_pdata->fdim = TPS65217_BL_FDIM_1000HZ;
> > > > +				} else {
> > > > +					dev_err(&client->dev, "invalid value for backlight dimming frequency in the device tree\n");
> > > > +					return NULL;
> > > > +				}
> > > > +			} else {
> > > > +				pdata->bl_pdata->fdim = TPS65217_BL_FDIM_200HZ;
> > > > +			}
> > > > +		}
> > > > +	}
> > > 
> > > Same here.
> > 
> > not exactly, the value specified in the device tree for the dimming
> > frequency will be a frequency, not a value corresponding to the enum,
> > so a range check + assignment isn't enough. if you'd like to see a
> > similar handling an option would be to set TPS65217_BL_FDIM_100HZ to
> > 100, TPS..._200HZ to 200, ..., and do:
> > 
> > switch (val) {
> >   case TPS65217_BL_FDIM_100HZ:
> >   case TPS65217_BL_FDIM_200HZ:
> >   ...
> >     pdata->bl_pdata->fdim	= val;
> >     break;
> > 
> >   default:
> >     /* error handling */
> > }
> > 
> 
> This looks better.

taking a closer look i noticed that unfortunately it won't work that
way, as the constants TPS65217_BL_FDIM_*HZ are the values which are
written to the WLEDCTRL1 registers

so the outcome will be:

switch (val) {
  case 100:
    pdata->bl_pdata->fdim	= TPS65217_BL_FDIM_100HZ;
    break;

	case 200:

  ...

	default:
    /* error handling */
}

or the initial solution, which is slightly shorter, but i
think you prefer the switch construct

regards

-- 
Matthias Kaehlcke
Embedded Linux Developer
Amsterdam

   Programming is not just an act of telling a computer what to do:
  it is also an act of telling other programmers what you wished the
   computer to do. Both are important, and the latter deserves care
                          (Andrew Morton)
                                                                 .''`.
    using free software / Debian GNU/Linux | http://debian.org  : :'  :
                                                                `. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4                  `-
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux