lkml.org 
[lkml]   [2012]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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


    \
     
     \ /
      Last update: 2012-05-03 14:01    [W:0.046 / U:2.996 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site