lkml.org 
[lkml]   [2010]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC v1 PATCH 3/6] led: pmic8058: Add PMIC8058 leds driver
    On 12/06/2010 02:44 PM, Trilok Soni wrote:
    > Hi Peter,
    >
    >>
    >>>> +
    >>>> +/**
    >>>> + * struct pmic8058_led - per led data
    >>>> + * @name - name of the led
    >>>> + * @default_trigger - default trigger which needs to e attached
    >>>> + * @max_brightness - maximum brightness level supported by the led
    >>>> + * @id - supported led id
    >>>> + */
    >>>> +struct pmic8058_led {
    >>>> + const char *name;
    >>>> + const char *default_trigger;
    >>>> + unsigned max_brightness;
    >>> Should max_brightness not rather be hardcoded in the driver? As far as I can tell it
    >>> depend on the hardware and is 4 bits wide for flash and bl leds and 5 bits for the
    >>> others.
    >>>> + int id;
    >>>
    >>> enum pmic8058_leds instead of int
    >>
    >> Ack.
    >>
    >>>> +struct pmic8058_leds_platform_data {
    >>>> + int num_leds;
    >>> size_t
    >>
    >> Ack.
    >>
    >>>> + struct pmic8058_led *leds;
    >>>> +};
    >>>
    >>>
    >>> If max_brightness is hardcoded in the driver you can reuse "struct led_info" and
    >>> "struct struct led_platform_data" instead of adding your own structs.
    >>
    >
    > I couldn't remove these pmic8058_led structure due to the "enum pmic8058_led id" member
    > info which I need from every led. This can be removed completely only if I abuse
    > the "flags" parameter in struct led_info to pass the led id. Let me know what you think.
    >
    > ---Trilok Soni
    >

    Hi

    I think that would be ok, other drivers seem to do the same.

    - Lars


    \
     
     \ /
      Last update: 2010-12-07 16:13    [W:3.950 / U:0.544 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site