lkml.org 
[lkml]   [2022]   [Jul]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 6/6] leds: is31fl319x: Add support for is31fl319{0,1,3} chips
    On Tue, Jul 5, 2022 at 6:32 PM Vincent Knecht <vincent.knecht@mailoo.org> wrote:
    >
    > Set specific chipset structs values for is31fl319{0,1,3}
    > so that those chips can actually work.
    > Datasheets:
    > https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf
    > https://lumissil.com/assets/pdf/core/IS31FL3191_DS.pdf
    > https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf
    > https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf
    > https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf

    ...

    Two comments as per previous patch reviews.

    ...

    > + struct is31fl319x_led *led = container_of(cdev, struct is31fl319x_led,
    > + cdev);

    One line?

    ...

    > + ret = regmap_read(is31->regmap, IS31FL3190_PWM(i), &pwm_value);

    > + dev_dbg(&is31->client->dev, "%s read %d: ret=%d: %d\n",
    > + __func__, i, ret, pwm_value);

    regmap has tracepoints Do you need to duplicate it with slow printk()?

    ...

    > +static inline int is31fl3190_microamp_to_cs(struct device *dev, u32 microamp)
    > +{
    > + switch (microamp) {
    > + case 5000:
    > + return IS31FL3190_CURRENT_5_mA;
    > + case 10000:
    > + return IS31FL3190_CURRENT_10_mA;
    > + case 17500:
    > + return IS31FL3190_CURRENT_17dot5_mA;
    > + case 30000:
    > + return IS31FL3190_CURRENT_30_mA;
    > + case 42000:
    > + return IS31FL3190_CURRENT_42_mA;
    > + default:
    > + dev_warn(dev, "Unsupported current value: %d, using 5000 µA!\n", microamp);
    > + }
    > +
    > + return IS31FL3190_CURRENT_5_mA;

    It's more consistent to return directly from the default case.

    > +}

    --
    With Best Regards,
    Andy Shevchenko

    \
     
     \ /
      Last update: 2022-07-05 20:58    [W:3.312 / U:0.676 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site