Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] sysfs show functions `show_als_en` and `show_linear` return prematurely when the tested bits are set, breaking the sysfs read and returning an empty/uninitialized buffer. --
commit 549a7f316072e9f93bfcd3d182e73739a35c0c7a Author: Svyatoslav Ryhel <[email protected]> video: backlight: lm3533_bl: Improve logic of sysfs functions Simplify the sysfs logic of properties by switching to macros and proper regmap helpers. > diff --git a/drivers/video/backlight/lm3533_bl.c > b/drivers/video/backlight/lm3533_bl.c > --- a/drivers/video/backlight/lm3533_bl.c > +++ b/drivers/video/backlight/lm3533_bl.c [ ... ] > @@ -85,88 +87,68 @@ static ssize_t show_als_channel(struct device *dev, > return scnprintf(buf, PAGE_SIZE, "%u\n", channel); > } > > -static ssize_t show_als_en(struct device *dev, > - struct device_attribute *attr, char *buf) > +static ssize_t show_als_en(struct device *dev, struct device_attribute *attr, > + char *buf) > { > struct lm3533_bl *bl = dev_get_drvdata(dev); > int ctrlbank = lm3533_bl_get_ctrlbank_id(bl); > - u32 val; > - u8 mask; > - bool enable; > int ret; > > - ret = regmap_read(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val); > + ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF, > + CTRLBANK_AB_BCONF_ALS(ctrlbank)); > if (ret) > return ret; [Severity: High] Does this error check prematurely exit the function when the bit is set? regmap_test_bits() returns 1 if the specified bit is set. If the bit is set, the if (ret) condition evaluates to true, causing the function to return 1 immediately instead of formatting the buffer. This could break the sysfs API contract and return a single null byte to userspace instead of the expected string. Should this check be if (ret < 0) instead? > > - mask = 1 << (2 * ctrlbank); > - enable = val & mask; > - > - return scnprintf(buf, PAGE_SIZE, "%d\n", enable); > + return scnprintf(buf, PAGE_SIZE, "%d\n", ret); > } [ ... ] > -static ssize_t show_linear(struct device *dev, > - struct device_attribute *attr, char *buf) > +static ssize_t show_linear(struct device *dev, struct device_attribute *attr, > + char *buf) > { > struct lm3533_bl *bl = dev_get_drvdata(dev); > - u32 val; > - u8 mask; > - int linear; > + int ctrlbank = lm3533_bl_get_ctrlbank_id(bl); > int ret; > > - ret = regmap_read(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF, &val); > + ret = regmap_test_bits(bl->regmap, LM3533_REG_CTRLBANK_AB_BCONF, > + CTRLBANK_AB_BCONF_MODE(ctrlbank)); > if (ret) > return ret; [Severity: High] Could this have the same issue as show_als_en() above? When the linear mode bit is set, regmap_test_bits() returns 1, triggering this error path and returning 1 directly instead of calling scnprintf(). > > - mask = 1 << (2 * lm3533_bl_get_ctrlbank_id(bl) + 1); > - > - if (val & mask) > - linear = 1; > - else > - linear = 0; > - > - return scnprintf(buf, PAGE_SIZE, "%x\n", linear); > + return scnprintf(buf, PAGE_SIZE, "%x\n", ret); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
