Messages in this thread | | | Date | Thu, 3 May 2012 13:28:03 +0200 | From | Johan Hovold <> | Subject | Re: [PATCH v2 1/4] mfd: add LM3533 lighting-power core driver |
| |
On Thu, May 03, 2012 at 11:38:48AM +0100, Mark Brown wrote: > On Thu, May 03, 2012 at 12:26:36PM +0200, Johan Hovold wrote: > > Add support for National Semiconductor / TI LM3533 lighting power chips. > > > > This is the core driver which provides register access over I2C and > > registers the ambient-light-sensor, LED and backlight sub-drivers. > > Reviwed-by: Mark Brown <broonie@opensource.wolfsonmicro.com> > > though > > > + dev_dbg(lm3533->dev, "read [%02x]: %02x\n", reg, *val); > > I'd expect you can drop these log messages, if there's stuff like this > missing we should add it to regmap. At the minute the regmap logging is > via trace points rather than debug logs as you can leave them enabled > all the time.
If such debugging is added to regmap we still need a way to enable them per driver (or rather regmap) to not clutter the logs.
These three dev_dbg statements are extremely useful during debugging / development especially in combination with the other dynamic printks in these drivers.
I'd actually prefer just keeping them for now.
> Might also be worth moving some of the sysfs stuff to live with the > relevant drivers.
Which attributes do you have in mind?
The boost_freq and boost_ovp affect both backlight devices (i.e. cannot be set separately) and as such belong in the parent driver IMO.
Same with the output_hvled{1..2}, output_lvled{1..5} attributes. The chip has four logical LEDs ("control banks") but five low-voltage output sinks. The five output_lvled attributes determine the mapping and as such belong in the parent driver. The two logical backlight devices can likewise be used to control either or both high-voltage outputs and belong in the parent driver for the same reasons.
Thanks, Johan
| |