[PATCH 112/182] pinctrl: samsung: use gpiochip data pointer

[PATCH 112/182] pinctrl: samsung: use gpiochip data pointer

[Linus Walleij]

This makes the driver use the data pointer added to the gpio_chip
to store a pointer to the state container instead of relying on
container_of().

Cc: Tomasz Figa <tomasz.figa@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/pinctrl/samsung/pinctrl-samsung.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
index bb4db2050f19..a4fb8379f521 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.c
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
@@ -50,11 +50,6 @@ static LIST_HEAD(drvdata_list);
 
 static unsigned int pin_base;
 
-static inline struct samsung_pin_bank *gc_to_pin_bank(struct gpio_chip *gc)
-{
-	return container_of(gc, struct samsung_pin_bank, gpio_chip);
-}
-
 static int samsung_get_group_count(struct pinctrl_dev *pctldev)
 {
 	struct samsung_pinctrl_drv_data *pmx = pinctrl_dev_get_drvdata(pctldev);
@@ -522,7 +517,7 @@ static const struct pinconf_ops samsung_pinconf_ops = {
 /* gpiolib gpio_set callback function */
 static void samsung_gpio_set(struct gpio_chip *gc, unsigned offset, int value)
 {
-	struct samsung_pin_bank *bank = gc_to_pin_bank(gc);
+	struct samsung_pin_bank *bank = gpiochip_get_data(gc);
 	const struct samsung_pin_bank_type *type = bank->type;
 	unsigned long flags;
 	void __iomem *reg;
@@ -546,7 +541,7 @@ static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset)
 {
 	void __iomem *reg;
 	u32 data;
-	struct samsung_pin_bank *bank = gc_to_pin_bank(gc);
+	struct samsung_pin_bank *bank = gpiochip_get_data(gc);
 	const struct samsung_pin_bank_type *type = bank->type;
 
 	reg = bank->drvdata->virt_base + bank->pctl_offset;
@@ -571,7 +566,7 @@ static int samsung_gpio_set_direction(struct gpio_chip *gc,
 	u32 data, mask, shift;
 	unsigned long flags;
 
-	bank = gc_to_pin_bank(gc);
+	bank = gpiochip_get_data(gc);
 	type = bank->type;
 	drvdata = bank->drvdata;
 
@@ -619,7 +614,7 @@ static int samsung_gpio_direction_output(struct gpio_chip *gc, unsigned offset,
  */
 static int samsung_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
 {
-	struct samsung_pin_bank *bank = gc_to_pin_bank(gc);
+	struct samsung_pin_bank *bank = gpiochip_get_data(gc);
 	unsigned int virq;
 
 	if (!bank->irq_domain)
@@ -918,7 +913,7 @@ static int samsung_gpiolib_register(struct platform_device *pdev,
 		gc->of_node = bank->of_node;
 		gc->label = bank->name;
 
-		ret = gpiochip_add(gc);
+		ret = gpiochip_add_data(gc, bank);
 		if (ret) {
 			dev_err(&pdev->dev, "failed to register gpio_chip %s, error code: %d\n",
 							gc->label, ret);
-- 
2.4.3

Re: [PATCH 112/182] pinctrl: samsung: use gpiochip data pointer

[Tomasz Figa]

Hi Linus,

2015-12-09 22:33 GMT+09:00 Linus Walleij <linus.walleij@linaro.org>:
> This makes the driver use the data pointer added to the gpio_chip
> to store a pointer to the state container instead of relying on
> container_of().

I always thought container_of() was preferred because of higher type
safety than void *data pointers. Is there any reason for mandating the
(IMHO worse) new method of accessing data, even though the one
currently used seems to be still valid?

Best regards,
Tomasz

Re: [PATCH 112/182] pinctrl: samsung: use gpiochip data pointer

[Linus Walleij]

On Fri, Dec 11, 2015 at 7:14 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Linus,
>
> 2015-12-09 22:33 GMT+09:00 Linus Walleij <linus.walleij@linaro.org>:
>> This makes the driver use the data pointer added to the gpio_chip
>> to store a pointer to the state container instead of relying on
>> container_of().
>
> I always thought container_of() was preferred because of higher type
> safety than void *data pointers. Is there any reason for mandating the
> (IMHO worse) new method of accessing data, even though the one
> currently used seems to be still valid?

This is explained in patch 000/182:
http://marc.info/?l=linux-gpio&m=144966662402660&w=2

In the generic GPIO case it gets really really messy with
container_of(container_of(x)) in every driver, making the code
so hard to read that it becomes a pain to maintain and do the
right thing, trumping any type safety.

Unless someone has a real performance issue I think this is
the way to go.

Yours,
Linus Walleij

Re: [PATCH 112/182] pinctrl: samsung: use gpiochip data pointer

[Tomasz Figa]

2015-12-14 22:55 GMT+09:00 Linus Walleij <linus.walleij@linaro.org>:
> On Fri, Dec 11, 2015 at 7:14 AM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>> Hi Linus,
>>
>> 2015-12-09 22:33 GMT+09:00 Linus Walleij <linus.walleij@linaro.org>:
>>> This makes the driver use the data pointer added to the gpio_chip
>>> to store a pointer to the state container instead of relying on
>>> container_of().
>>
>> I always thought container_of() was preferred because of higher type
>> safety than void *data pointers. Is there any reason for mandating the
>> (IMHO worse) new method of accessing data, even though the one
>> currently used seems to be still valid?
>
> This is explained in patch 000/182:
> http://marc.info/?l=linux-gpio&m=144966662402660&w=2

Okay, fair enough.

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

Best regards,
Tomasz