Re: [PATCH 4/9] pinctrl: samsung: Use generic of_device_get_match_data helper

[Tomasz Figa <tomasz.figa@xxxxxxxxx>]

2016-12-25 21:56 GMT+09:00 Krzysztof Kozlowski <krzk@xxxxxxxxxx>:
> On Fri, Dec 23, 2016 at 01:24:44PM +0100, Marek Szyprowski wrote:
>> Replace custom code with generic helper.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>> ---
>>  drivers/pinctrl/samsung/pinctrl-samsung.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
>> index 4d9262051ff1..a6c2ea74e0f3 100644
>> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
>> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
>> @@ -27,6 +27,7 @@
>>  #include <linux/err.h>
>>  #include <linux/gpio.h>
>>  #include <linux/irqdomain.h>
>> +#include <linux/of_device.h>
>>  #include <linux/spinlock.h>
>>  #include <linux/syscore_ops.h>
>>
>> @@ -967,15 +968,13 @@ static int samsung_gpiolib_unregister(struct platform_device *pdev,
>>       return 0;
>>  }
>>
>> -static const struct of_device_id samsung_pinctrl_dt_match[];
>> -
>>  /* retrieve the soc specific data */
>>  static const struct samsung_pin_ctrl *
>>  samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
>>                            struct platform_device *pdev)
>>  {
>>       int id;
>> -     const struct of_device_id *match;
>> +     const struct samsung_pin_ctrl *match_data;
>>       struct device_node *node = pdev->dev.of_node;
>>       struct device_node *np;
>>       const struct samsung_pin_bank_data *bdata;
>> @@ -990,8 +989,8 @@ static int samsung_gpiolib_unregister(struct platform_device *pdev,
>>               dev_err(&pdev->dev, "failed to get alias id\n");
>>               return ERR_PTR(-ENOENT);
>>       }
>> -     match = of_match_node(samsung_pinctrl_dt_match, node);
>> -     ctrl = (struct samsung_pin_ctrl *)match->data + id;
>> +     match_data = of_device_get_match_data(&pdev->dev);
>> +     ctrl = match_data + id;
>
> Either you need a check for match_data != NULL or just remove match_data
> and:
>         ctrl = of_device_get_match_data();
>         ctrl += id;
>
> Actually match_data can be removed even with the check for non-NULL...

I don't think this function can ever return NULL if the match array
contains a non-NULL value for each compatible string and the driver
can be probed only by DT.
But you still need to cast the match data pointer to the correct type
and using a variable for it IMHO makes the code cleaner.

Acked-by: Tomasz Figa <tomasz.figa@xxxxxxxxx>

Best regards,
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html