Messages in this thread | | | Subject | Re: [PATCH v2] leds: lm3532: Fix optional led-max-microamp prop error handling | From | Dan Murphy <> | Date | Thu, 12 Sep 2019 14:47:32 -0500 |
| |
Jacek
On 9/12/19 2:12 PM, Jacek Anaszewski wrote: > On 9/12/19 8:32 PM, Jacek Anaszewski wrote:> Hi Dan, >> Thank you for the update. >> >> On 9/11/19 8:27 PM, Dan Murphy wrote: >>> Fix the error handling for the led-max-microamp property. >>> Need to check if the property is present and then if it is >>> retrieve the setting and its max boundary >>> >>> Reported-by: Pavel Machek <pavel@ucw.cz> >>> Signed-off-by: Dan Murphy <dmurphy@ti.com> >>> --- >>> >>> v2 - Changed full scale current check to use min function >>> >>> drivers/leds/leds-lm3532.c | 14 +++++++++----- >>> 1 file changed, 9 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c >>> index 62ace6698d25..a1742dc1f6fa 100644 >>> --- a/drivers/leds/leds-lm3532.c >>> +++ b/drivers/leds/leds-lm3532.c >>> @@ -601,11 +601,15 @@ static int lm3532_parse_node(struct lm3532_data *priv) >>> goto child_out; >>> } >>> >>> - ret = fwnode_property_read_u32(child, "led-max-microamp", >>> - &led->full_scale_current); >>> - >>> - if (led->full_scale_current > LM3532_FS_CURR_MAX) >>> - led->full_scale_current = LM3532_FS_CURR_MAX; >>> + if (fwnode_property_present(child, "led-max-microamp")) { >>> + if (fwnode_property_read_u32(child, "led-max-microamp", >>> + &led->full_scale_current)) >>> + dev_err(&priv->client->dev, >>> + "Failed getting led-max-microamp\n"); >>> + else >>> + min(led->full_scale_current, >>> + LM3532_FS_CURR_MAX); > I didn't previously notice lack of assignment of min() return value. > > I've amended that and while at it improved a bit this construction to > avoid some indentations and line breaks: > > diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c > index 62ace6698d25..0507c6575c08 100644 > --- a/drivers/leds/leds-lm3532.c > +++ b/drivers/leds/leds-lm3532.c > @@ -601,11 +601,14 @@ static int lm3532_parse_node(struct lm3532_data *priv) > goto child_out; > } > > - ret = fwnode_property_read_u32(child, "led-max-microamp", > - &led->full_scale_current); > - > - if (led->full_scale_current > LM3532_FS_CURR_MAX) > - led->full_scale_current = LM3532_FS_CURR_MAX; > + if (fwnode_property_present(child, "led-max-microamp") && > + fwnode_property_read_u32(child, "led-max-microamp", > + &led->full_scale_current)) > + dev_err(&priv->client->dev, > + "Failed getting led-max-microamp\n"); > + else > + led->full_scale_current = min(led->full_scale_current, > + LM3532_FS_CURR_MAX); > > if (led->mode == LM3532_BL_MODE_ALS) { > led->mode = LM3532_ALS_CTRL; > > > Please let me know in case of any doubts.
This looks fine. It always passed for me because I never set the FSC above max
Dan
| |