From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751678AbdAMNFC (ORCPT <rfc822;w@1wt.eu>);
        Fri, 13 Jan 2017 08:05:02 -0500
Received: from mail-lf0-f67.google.com ([209.85.215.67]:34226 "EHLO
        mail-lf0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751444AbdAMNEy (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 13 Jan 2017 08:04:54 -0500
Date: Fri, 13 Jan 2017 15:04:21 +0200
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: linux-kernel@vger.kernel.org, Laxman Dewangan <ldewangan@nvidia.com>,
        Krzysztof Kozlowski <krzk@kernel.org>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH 2/4] mfd: max77686: Use of_device_get_match_data() helper
Message-ID: <20170113130421.xm76aik6fq6cc4vb@kozik-lap>
References: <1484228857-20182-1-git-send-email-javier@osg.samsung.com>
 <1484228857-20182-3-git-send-email-javier@osg.samsung.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1484228857-20182-3-git-send-email-javier@osg.samsung.com>
User-Agent: Mutt/1.6.2-neo (2016-08-21)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Jan 12, 2017 at 10:47:35AM -0300, Javier Martinez Canillas wrote:
> Use the generic helper to get the matched of_device_id .data,
> instead of open coding it.
> 
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> ---
> 
>  drivers/mfd/max77686.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
> index ddae3bf3e46c..33dd09493605 100644
> --- a/drivers/mfd/max77686.c
> +++ b/drivers/mfd/max77686.c
> @@ -34,6 +34,7 @@
>  #include <linux/mfd/max77686-private.h>
>  #include <linux/err.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  
>  static const struct mfd_cell max77686_devs[] = {
>  	{ .name = "max77686-pmic", },
> @@ -175,7 +176,6 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  			      const struct i2c_device_id *id)
>  {
>  	struct max77686_dev *max77686 = NULL;
> -	const struct of_device_id *match;
>  	unsigned int data;
>  	int ret = 0;
>  	const struct regmap_config *config;
> @@ -188,13 +188,8 @@ static int max77686_i2c_probe(struct i2c_client *i2c,
>  	if (!max77686)
>  		return -ENOMEM;
>  
> -	match = of_match_node(max77686_pmic_dt_match, i2c->dev.of_node);
> -	if (!match)
> -		return -EINVAL;

The commit message would suggest that the code is equivalent (except usage
of helper) but it is not the same entirely. You are not checking for
matched data. Returned NULL will be cast back to type to valid
TYPE_MAX77686. This should not happen but either mention the removal of
check in commit msg or make it:
enum max77686_types {
	TYPE_MAX77686_UNKNOWN
	...
}
if (max77686->type == TYPE_MAX77686_UNKNOWN)
	return -EINVAL;

Best regards,
Krzysztof

> -
> -	max77686->type = (unsigned long)match->data;
> -
>  	i2c_set_clientdata(i2c, max77686);
> +	max77686->type = (unsigned long)of_device_get_match_data(&i2c->dev);
>  	max77686->dev = &i2c->dev;
>  	max77686->i2c = i2c;
>  
> -- 
> 2.7.4
>