On Tue, May 31, 2022 at 1:32 PM ChiaEn Wu <peterwu.pub@xxxxxxxxx> wrote: > > From: Alice Chen <alice_chen@xxxxxxxxxxx> > > Add Mediatek MT6370 flashlight support Same comments about the commit message. ... > +#include <linux/platform_device.h> > +#include <linux/property.h> > +#include <linux/regmap.h> + blank line? > +#include <media/v4l2-flash-led-class.h> + blank line > +enum { > + MT6370_LED_FLASH1, > + MT6370_LED_FLASH2, > + MT6370_MAX_LEDS > +}; ... > + struct mt6370_led *led = container_of(fl_cdev, struct mt6370_led, > + flash); > + struct mt6370_led *led = container_of(fl_cdev, struct mt6370_led, > + flash); Make a helper out of this #define to_mt637_led() container_of() and reuse. ... > + /* > + * For the flash turn on/off, HW rampping up/down time is 5ms/500us, ramping > + * respectively Period! > + */ ... > + const char * const states[] = { "off", "keep", "on" }; > + const char *str; > + int ret; > + > + if (!fwnode_property_read_string(init_data->fwnode, > + "default-state", &str)) { > + ret = match_string(states, ARRAY_SIZE(states), str); > + if (ret < 0) > + ret = STATE_OFF; > + > + led->default_state = ret; > + } fwnode_property_match_string()? ... > + if (!count || count > MT6370_MAX_LEDS) { > + dev_err(&pdev->dev, > + "No child node or node count over max led number %lu\n", count); > + return -EINVAL; return dev_err_probe(...); > + } -- With Best Regards, Andy Shevchenko