Re: [PATCH v3 4/7] drivers: leds: Add the iEi WT61P803 PUZZLE LED driver

On Wed, Sep 30, 2020 at 9:48 PM Pavel Machek [off-list ref] wrote:

Hi!

quoted

+static int iei_wt61p803_puzzle_led_brightness_set_blocking(struct led_classdev *cdev,
+             enum led_brightness brightness)
+{
+     struct iei_wt61p803_puzzle_led *priv = cdev_to_iei_wt61p803_puzzle_led(cdev);
+     unsigned char *resp_buf = priv->response_buffer;
+     unsigned char led_power_cmd[5] = {
+             IEI_WT61P803_PUZZLE_CMD_HEADER_START,
+             IEI_WT61P803_PUZZLE_CMD_LED,
+             IEI_WT61P803_PUZZLE_CMD_LED_POWER,
+             (char)IEI_LED_OFF
+     };
+     size_t reply_size;
+
+     mutex_lock(&priv->lock);
+     if (brightness == LED_OFF) {
+             led_power_cmd[3] = (char)IEI_LED_OFF;
+             priv->led_power_state = LED_OFF;
+     } else {
+             led_power_cmd[3] = (char)IEI_LED_ON;
+             priv->led_power_state = LED_ON;
+     }
+     mutex_unlock(&priv->lock);
+
+     return iei_wt61p803_puzzle_write_command(priv->mcu, led_power_cmd,
+                     sizeof(led_power_cmd), resp_buf, &reply_size);
+}

Is the mutex needed? If so, should it include the
iei_wt61p803_puzzle_write_command()? Does
iei_wt61p803_puzzle_write_command() have internal locking to prevent
two messages from being mingled?

Best regards,
                                                                        Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Hello Pavel,

The mutex isn't needed and can be removed.
The function iei_wt61p803_puzzle_write_command() already handles its own
mutex locking, so a separate mutex isn't required.

Does brightness_set_blocking only block a single caller (each caller separately)
or does it block all callers until the previous caller is finished?

Kind regards,
Luka

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel