Re: [PATCH v1 leds-next 2/2] leds: mlxreg: Add support for capability register

[Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>]

Hi Vadim,

Thank you for the patch.

On 2/19/19 12:33 PM, Vadim Pasternak wrote:
> Add support for capability register in order to distinct between the
> systems with minor LED configuration differences. It reduces the amount
> of code describing systems' LED configuration.
> For example one system can be equipped with six LED, while the other
> with only four. Reading this information from the capability registers
> allows to use the same LED structure for such systems and set the
> relevant configuration dynamically based on capability register
> content.
>
> Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> ---
>   drivers/leds/leds-mlxreg.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
>
> diff --git a/drivers/leds/leds-mlxreg.c b/drivers/leds/leds-mlxreg.c
> index 1ee48cb21df9..cabe379071a7 100644
> --- a/drivers/leds/leds-mlxreg.c
> +++ b/drivers/leds/leds-mlxreg.c
> @@ -22,6 +22,7 @@
>   #define MLXREG_LED_AMBER_SOLID		0x09 /* Solid amber */
>   #define MLXREG_LED_BLINK_3HZ		167 /* ~167 msec off/on - HW support */
>   #define MLXREG_LED_BLINK_6HZ		83 /* ~83 msec off/on - HW support */
> +#define MLXREG_LED_CAPABILITY_CLEAR	GENMASK(31, 8) /* Clear mask */
>   
>   /**
>    * struct mlxreg_led_data - led control data:
> @@ -187,6 +188,7 @@ static int mlxreg_led_config(struct mlxreg_led_priv_data *priv)
>   	struct mlxreg_led_data *led_data;
>   	struct led_classdev *led_cdev;
>   	enum led_brightness brightness;
> +	u32 regval;
>   	int i;
>   	int err;
>   
> @@ -196,6 +198,23 @@ static int mlxreg_led_config(struct mlxreg_led_priv_data *priv)
>   		if (!led_data)
>   			return -ENOMEM;
>   
> +		if (data->capability) {
> +			err = regmap_read(led_pdata->regmap, data->capability,
> +					  &regval);
> +			if (err) {
> +				dev_err(&priv->pdev->dev, "Failed to query capability register\n");
> +				return err;
> +			}
> +			if (!(regval & data->bit))
> +				continue;
> +			/*
> +			 * Field "bit" can contain one capability bit in 0 byte
> +			 * and offset bit in 1-3 bytes. Clear capability bit and
> +			 * keep only offset bit.
> +			 */
> +			data->bit &= MLXREG_LED_CAPABILITY_CLEAR;
> +		}
> +
>   		led_cdev = &led_data->led_cdev;
>   		led_data->data_parent = priv;
>   		if (strstr(data->label, "red") ||

For this going through linux-platform-drivers-x86.git:

Acked-by: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>

Darren, will you take it together with the patch introducing
the "capability" field?

--
Best regards,
Jacek Anaszewski