From:   Mattijs Korpershoek <mkorpershoek@baylibre.com>
To:     Jiri Valek - 2N <jiriv@axis.com>, linux-input@vger.kernel.org
Cc:     devicetree@vger.kernel.org, dmitry.torokhov@gmail.com,
        krzysztof.kozlowski+dt@linaro.org, linux-kernel@vger.kernel.org,
        robh+dt@kernel.org, u.kleine-koenig@pengutronix.de, jiriv@axis.com
Subject: Re: [PATCH v4 2/2] Input: cap11xx - add support for cap1203,
 cap1293 and cap1298
In-Reply-To: <20230111131111.475270-3-jiriv@axis.com>
References: <20230111131111.475270-1-jiriv@axis.com>
 <20230111131111.475270-3-jiriv@axis.com>
Date:   Fri, 03 Feb 2023 16:13:23 +0100
Message-ID: <87mt5ulr3g.fsf@baylibre.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Wed, Jan 11, 2023 at 14:11, Jiri Valek - 2N <jiriv@axis.com> wrote:

> Add basic support for more CAP1xxx sensors.
> All models from CAP1xxx family are register-compatible.
> Some advanced features are not used and disabled by default.
>
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Small nitpick below

> ---
> Changes in v2:
>   - Fixed if statement.
>   
> Changes in v3:
>   - Model names sorted alphabetically.
>
> Changes in v4
>   - No changes.
>
>  drivers/input/keyboard/cap11xx.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 79afd0386e3f..ce27168302a8 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c
> @@ -98,14 +98,20 @@ enum {
>  	CAP1106,
>  	CAP1126,
>  	CAP1188,
> +	CAP1203,
>  	CAP1206,
> +	CAP1293,
> +	CAP1298
>  };
>  
>  static const struct cap11xx_hw_model cap11xx_devices[] = {
>  	[CAP1106] = { .product_id = 0x55, .num_channels = 6, .num_leds = 0, .no_gain = false },
>  	[CAP1126] = { .product_id = 0x53, .num_channels = 6, .num_leds = 2, .no_gain = false },
>  	[CAP1188] = { .product_id = 0x50, .num_channels = 8, .num_leds = 8, .no_gain = false },
> +	[CAP1203] = { .product_id = 0x6d, .num_channels = 3, .num_leds = 0, .no_gain = true },
>  	[CAP1206] = { .product_id = 0x67, .num_channels = 6, .num_leds = 0, .no_gain = true },
> +	[CAP1293] = { .product_id = 0x6f, .num_channels = 3, .num_leds = 0, .no_gain = false },
> +	[CAP1298] = { .product_id = 0x71, .num_channels = 8, .num_leds = 0, .no_gain = false },
>  };
>  
>  static const struct reg_default cap11xx_reg_defaults[] = {
> @@ -377,7 +383,8 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
>  	if (error < 0)
>  		return error;
>  
> -	dev_info(dev, "CAP11XX detected, revision 0x%02x\n", rev);
> +	dev_info(dev, "CAP11XX detected, model %s, revision 0x%02x\n",
> +			id->name, rev);
>  	node = dev->of_node;
>  
>  	if (!of_property_read_u32(node, "microchip,sensor-gain", &gain32)) {
> @@ -390,7 +397,9 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
>  			dev_err(dev, "Invalid sensor-gain value %d\n", gain32);
>  	}
>  
> -	if (id->driver_data != CAP1206) {
> +	if ((id->driver_data == CAP1106) ||
> +		(id->driver_data == CAP1126) ||
> +		(id->driver_data == CAP1188)) {

checkpatch --strict is not happy about this indendation:
CHECK: Alignment should match open parenthesis

>  		if (of_property_read_bool(node, "microchip,irq-active-high")) {
>  			error = regmap_update_bits(priv->regmap,
>  						   CAP11XX_REG_CONFIG2,
> @@ -483,7 +492,10 @@ static const struct of_device_id cap11xx_dt_ids[] = {
>  	{ .compatible = "microchip,cap1106", },
>  	{ .compatible = "microchip,cap1126", },
>  	{ .compatible = "microchip,cap1188", },
> +	{ .compatible = "microchip,cap1203", },
>  	{ .compatible = "microchip,cap1206", },
> +	{ .compatible = "microchip,cap1293", },
> +	{ .compatible = "microchip,cap1298", },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, cap11xx_dt_ids);
> @@ -492,7 +504,10 @@ static const struct i2c_device_id cap11xx_i2c_ids[] = {
>  	{ "cap1106", CAP1106 },
>  	{ "cap1126", CAP1126 },
>  	{ "cap1188", CAP1188 },
> +	{ "cap1203", CAP1203 },
>  	{ "cap1206", CAP1206 },
> +	{ "cap1293", CAP1293 },
> +	{ "cap1298", CAP1298 },
>  	{}
>  };
>  MODULE_DEVICE_TABLE(i2c, cap11xx_i2c_ids);
> -- 
> 2.25.1