Hi!
> We do not have a proper datasheet for the RT8515 but
> it turns out that RT9387A has a public datasheet and
> is essentially the same chip. We designed the driver
> in accordance with this datasheet. The day someone
> needs to drive a RT9387A this driver can probably
> easily be augmented to handle that chip too.
Please move this to the comment in the sources... perhaps with url for
the documenation.
> +/* This is setting the torch light level */
> +static int rt8515_led_brightness_set(struct led_classdev *led,
> + enum led_brightness brightness)
> +{
> + struct led_classdev_flash *fled = lcdev_to_flcdev(led);
> + struct rt8515 *rt = to_rt8515(fled);
> +
> + mutex_lock(&rt->lock);
> +
> + if (brightness == LED_OFF) {
> + /* Off */
> + gpiod_set_value(rt->enable_flash, 0);
> + gpiod_set_value(rt->enable_torch, 0);
> + } else if (brightness < RT8515_TORCH_MAX) {
> + /* Step it up to movie mode brightness using the flash pin */
> + rt8515_gpio_brightness_commit(rt->enable_torch, brightness);
> + } else {
> + /* Max torch brightness requested */
> + gpiod_set_value(rt->enable_torch, 1);
> + }
> +
> + mutex_unlock(&rt->lock);
Do you need to somehow reset the LED to lowest brightness before
rt8515_gpio_brightness_commit()?
> + ret1 = fwnode_property_read_u32(rt->dev->fwnode, resistance, &res);
> + ret2 = fwnode_property_read_u32(led, max_ua_prop, &ua);
> +
> + /* No info in DT, OK go with hardware maxima */
> + if (ret1 && ret2) {
> + max_ma = RT8515_MAX_IOUT_MA;
> + max_intensity = hw_max;
> + goto out_assign_max;
> + }
> +
> + if (ret1 || ret2) {
> + dev_err(rt->dev,
> + "either %s or %s missing from DT, using HW max\n",
> + resistance, max_ua_prop);
> + max_ma = RT8515_MAX_IOUT_MA;
> + max_intensity = hw_max;
> + goto out_assign_max;
> + }
I'd go with some minimum values if we don't have complete information
from devicetree.
> + /* Create a V4L2 Flash device if V4L2 flash is enabled */
> + rt->v4l2_flash = v4l2_flash_init(dev, child, fled, NULL, &v4l2_sd_cfg);
> + if (IS_ERR(rt->v4l2_flash)) {
> + ret = PTR_ERR(rt->v4l2_flash);
> + dev_err(dev, "failed to register V4L2 flash device (%d)\n",
> + ret);
> + /*
> + * Continue without the V4L2 flash
> + * (we still have the classdev)
> + */
> + }
> +
> + return 0;
> +}
> +
> +static int rt8515_remove(struct platform_device *pdev)
> +{
> + struct rt8515 *rt = platform_get_drvdata(pdev);
> +
> + v4l2_flash_release(rt->v4l2_flash);
Is it cool to call v4l2_flash_release() with error pointer?
> +MODULE_LICENSE("GPL v2");
v2+, iirc?
Driver looks good, thanks!
Pavel
--
http://www.livejournal.com/~pavelmachek
Attachment:
signature.asc
Description: PGP signature
