Le 10/06/2025 à 19:43, Nam Tran a écrit :
The LP5812 is a 4×3 matrix RGB LED driver with an autonomous animation
engine and time-cross-multiplexing (TCM) support for up to 12 LEDs or
4 RGB LEDs. Each LED can be configured through the related registers
to realize vivid and fancy lighting effects.
Hi,
...
+static struct lp5812_data *lp5812_of_populate_pdata(struct device *dev,
+ struct device_node *np,
+ struct lp5812_chip *chip)
+{
+ struct device_node *child;
+ struct lp5812_data *pdata;
+ struct lp5812_led_config *cfg;
+ int num_channels, i = 0, ret;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ num_channels = of_get_available_child_count(np);
+ if (num_channels == 0) {
+ dev_err(dev, "no LED channels\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ cfg = devm_kcalloc(dev, num_channels, sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return ERR_PTR(-ENOMEM);
+
+ pdata->led_config = &cfg[0];
+ pdata->num_channels = num_channels;
+
+ for_each_available_child_of_node(np, child) {
Maybe for_each_available_child_of_node_scoped() to slihtly simplify the
code?
+ ret = lp5812_parse_logical_led(child, cfg, i);
+ if (ret) {
+ of_node_put(child);
+ return ERR_PTR(-EINVAL);
+ }
+ i++;
+ }
+
+ of_property_read_string(np, "label", &pdata->label);
+
+ return pdata;
+}
...
+static ssize_t lp5812_aeu_slope_time(struct device *dev,
+ struct device_attribute *attr,
+ enum slope_time_num slope_chan,
+ const char *buf, size_t len)
+{
+ struct lp5812_led *led;
+ struct lp5812_chip *chip;
+ struct lp5812_led_config *led_cfg;
+ const char *name = dev->platform_data;
+ int val[LED_COLOR_ID_MAX];
+ u8 chan_nr = 0;
+ char *sub_str, *str = (char *)buf;
+ int i, ret, aeu;
+ union slope_time slope_time_val;
+ u16 reg;
+
+ if (strcmp(name, LP5812_SC_LED) == 0)
+ led = dev_to_lp5812_led(dev);
+ else
+ led = dev_to_lp5812_led_mc(dev);
+
+ chan_nr = led->chan_nr;
+ chip = led->chip;
+ led_cfg = &chip->pdata->led_config[chan_nr];
+
+ sub_str = strsep(&str, ":");
+ if (!sub_str)
+ return -EINVAL;
+ if (kstrtoint(&sub_str[3], 0, &aeu))
+ return -EINVAL;
+
+ pr_info("AEU = %d", aeu);
+
+ guard(mutex)(&chip->lock);
+ for (i = 0; i < led_cfg->num_colors; i++) {
+ sub_str = strsep(&str, " ");
+ if (!sub_str)
+ return -EINVAL;
+ if (kstrtoint(sub_str, 0, &val[i]))
+ return -EINVAL;
+ if (val[i] < 0 || val[i] > 15)
+ return -EINVAL;
+
+ reg = LP5812_AEU_SLOPE_TIME_ADDR(led_cfg->led_id[i], aeu, slope_chan);
+
+ /* get original value of slope time */
+ ret = lp5812_read(chip, reg, &slope_time_val.time_val);
+ if (ret)
+ return ret;
+
+ /* Update new value for slope time*/
+ if (slope_chan == LP5812_SLOPE_TIME_T1 || slope_chan == LP5812_SLOPE_TIME_T3)
+ slope_time_val.s_time.first = val[i];
+ if (slope_chan == LP5812_SLOPE_TIME_T2 || slope_chan == LP5812_SLOPE_TIME_T4)
+ slope_time_val.s_time.second = val[i];
+
+ /* Save updated value to hardware */
+ ret = lp5812_write(chip, reg, slope_time_val.time_val);
Should we do something if ret != 0?
+ }
+
+ return len;
+}
...
+static struct attribute *lp5812_led_attrs[] = {
+ &dev_attr_led_current.attr,
+ &dev_attr_max_current.attr,
+ &dev_attr_mode.attr,
+ &dev_attr_activate.attr,
+ &dev_attr_pwm_dimming_scale.attr,
+ &dev_attr_pwm_phase_align.attr,
+ &dev_attr_auto_time_pause_at_start.attr,
+ &dev_attr_auto_time_pause_at_stop.attr,
+ &dev_attr_auto_playback_eau_number.attr,
+ &dev_attr_auto_playback_time.attr,
+ &dev_attr_aeu_playback_time.attr,
+ &dev_attr_aeu_pwm1.attr,
+ &dev_attr_aeu_pwm2.attr,
+ &dev_attr_aeu_pwm3.attr,
+ &dev_attr_aeu_pwm4.attr,
+ &dev_attr_aeu_pwm5.attr,
+ &dev_attr_aeu_slop_time_t1.attr,
+ &dev_attr_aeu_slop_time_t2.attr,
+ &dev_attr_aeu_slop_time_t3.attr,
+ &dev_attr_aeu_slop_time_t4.attr,
+ &dev_attr_lod_lsd.attr,
+ NULL,
Unneeded trailing comma after a terminator.
+};
+ATTRIBUTE_GROUPS(lp5812_led);
...
+static int lp5812_init_led(struct lp5812_led *led, struct lp5812_chip *chip, int chan)
+{
+ struct lp5812_data *pdata = chip->pdata;
+ struct device *dev = &chip->i2c_cl->dev;
+ struct mc_subled *mc_led_info;
+ struct led_classdev *led_cdev;
+ char name[32];
+ int i, ret = 0;
+
+ if (pdata->led_config[chan].name) {
+ led->cdev.name = pdata->led_config[chan].name;
+ } else {
+ snprintf(name, sizeof(name), "%s:channel%d",
+ pdata->label ? : chip->i2c_cl->name, chan);
+ led->cdev.name = name;
Is it fine below when 'name' is defined on the stack and is used...
+ }
+
+ if (pdata->led_config[chan].is_sc_led == 0) {
+ mc_led_info = devm_kcalloc(dev,
+ pdata->led_config[chan].num_colors,
+ sizeof(*mc_led_info), GFP_KERNEL);
+ if (!mc_led_info)
+ return -ENOMEM;
+
+ led_cdev = &led->mc_cdev.led_cdev;
+ led_cdev->name = led->cdev.name;
...here?
+ led_cdev->brightness_set_blocking = lp5812_set_mc_brightness;
+ led->mc_cdev.num_colors = pdata->led_config[chan].num_colors;
+ for (i = 0; i < led->mc_cdev.num_colors; i++) {
+ mc_led_info[i].color_index =
+ pdata->led_config[chan].color_id[i];
+ mc_led_info[i].channel =
+ pdata->led_config[chan].led_id[i];
+ }
+
+ led->mc_cdev.subled_info = mc_led_info;
+ } else {
+ led->cdev.brightness_set_blocking = lp5812_set_brightness;
+ }
+
+ led->cdev.groups = lp5812_led_groups;
+ led->chan_nr = chan;
+
+ if (pdata->led_config[chan].is_sc_led) {
+ ret = devm_led_classdev_register(dev, &led->cdev);
+ if (ret == 0) {
+ led->cdev.dev->platform_data = devm_kstrdup(dev, LP5812_SC_LED, GFP_KERNEL);
+ if (!led->cdev.dev->platform_data)
+ return -ENOMEM;
+ }
+ } else {
+ ret = devm_led_classdev_multicolor_register(dev, &led->mc_cdev);
+ if (ret == 0) {
+ led->mc_cdev.led_cdev.dev->platform_data =
+ devm_kstrdup(dev, LP5812_MC_LED, GFP_KERNEL);
+ if (!led->mc_cdev.led_cdev.dev->platform_data)
+ return -ENOMEM;
+
+ ret = sysfs_create_groups(&led->mc_cdev.led_cdev.dev->kobj,
+ lp5812_led_groups);
+ if (ret)
+ dev_err(dev, "sysfs_create_groups failed\n");
+ }
+ }
+
+ if (ret) {
+ dev_err(dev, "led register err: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
...
CJ
