Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1381392 > unrolled thread

[PATCH 1/3] gpio: tegra: Don't open code of_device_get_match_data()

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] gpio: tegra: Don't open code of_device_get_match_data() Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-18 11:00 +0200
    [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-18 11:00 +0200
      Re: [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle  for gpio driver Stephen Warren <swarren@wwwdotorg.org> - 2016-04-18 18:40 +0200
        Re: [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle  for gpio driver Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-18 19:20 +0200
          Re: [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle  for gpio driver Stephen Warren <swarren@wwwdotorg.org> - 2016-04-19 18:00 +0200
      Re: [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle  for gpio driver Alexandre Courbot <gnurou@gmail.com> - 2016-04-20 03:20 +0200
    Re: [PATCH 1/3] gpio: tegra: Don't open code  of_device_get_match_data() Stephen Warren <swarren@wwwdotorg.org> - 2016-04-18 18:30 +0200
    Re: [PATCH 1/3] gpio: tegra: Don't open code of_device_get_match_data() Alexandre Courbot <gnurou@gmail.com> - 2016-04-20 03:00 +0200

#1381392 — [PATCH 1/3] gpio: tegra: Don't open code of_device_get_match_data()

Use of_device_get_match_data() for getting matched data
instead of implementing this locally.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/gpio/gpio-tegra.c | 50 +++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 790bb11..1b0c497 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -75,6 +75,11 @@ struct tegra_gpio_bank {
 #endif
 };
 
+struct tegra_gpio_soc_config {
+	u32 bank_stride;
+	u32 upper_offset;
+};
+
 static struct device *dev;
 static struct irq_domain *irq_domain;
 static void __iomem *regs;
@@ -445,27 +450,6 @@ static const struct dev_pm_ops tegra_gpio_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(tegra_gpio_suspend, tegra_gpio_resume)
 };
 
-struct tegra_gpio_soc_config {
-	u32 bank_stride;
-	u32 upper_offset;
-};
-
-static struct tegra_gpio_soc_config tegra20_gpio_config = {
-	.bank_stride = 0x80,
-	.upper_offset = 0x800,
-};
-
-static struct tegra_gpio_soc_config tegra30_gpio_config = {
-	.bank_stride = 0x100,
-	.upper_offset = 0x80,
-};
-
-static const struct of_device_id tegra_gpio_of_match[] = {
-	{ .compatible = "nvidia,tegra30-gpio", .data = &tegra30_gpio_config },
-	{ .compatible = "nvidia,tegra20-gpio", .data = &tegra20_gpio_config },
-	{ },
-};
-
 /* This lock class tells lockdep that GPIO irqs are in a different
  * category than their parents, so it won't report false recursion.
  */
@@ -473,8 +457,7 @@ static struct lock_class_key gpio_lock_class;
 
 static int tegra_gpio_probe(struct platform_device *pdev)
 {
-	const struct of_device_id *match;
-	struct tegra_gpio_soc_config *config;
+	const struct tegra_gpio_soc_config *config;
 	struct resource *res;
 	struct tegra_gpio_bank *bank;
 	int ret;
@@ -484,12 +467,11 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 
 	dev = &pdev->dev;
 
-	match = of_match_device(tegra_gpio_of_match, &pdev->dev);
-	if (!match) {
+	config = of_device_get_match_data(&pdev->dev);
+	if (!config) {
 		dev_err(&pdev->dev, "Error: No device match found\n");
 		return -ENODEV;
 	}
-	config = (struct tegra_gpio_soc_config *)match->data;
 
 	tegra_gpio_bank_stride = config->bank_stride;
 	tegra_gpio_upper_offset = config->upper_offset;
@@ -578,6 +560,22 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static struct tegra_gpio_soc_config tegra20_gpio_config = {
+	.bank_stride = 0x80,
+	.upper_offset = 0x800,
+};
+
+static struct tegra_gpio_soc_config tegra30_gpio_config = {
+	.bank_stride = 0x100,
+	.upper_offset = 0x80,
+};
+
+static const struct of_device_id tegra_gpio_of_match[] = {
+	{ .compatible = "nvidia,tegra30-gpio", .data = &tegra30_gpio_config },
+	{ .compatible = "nvidia,tegra20-gpio", .data = &tegra20_gpio_config },
+	{ },
+};
+
 static struct platform_driver tegra_gpio_driver = {
 	.driver		= {
 		.name	= "tegra-gpio",
-- 
2.1.4

[toc] | [next] | [standalone]

#1381394 — [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver

Remove the file static device handle variable as this is just
required for prints. The required handle can be stored in
tegra_gpio_chip and hence it become redundancy.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/gpio/gpio-tegra.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index 1b0c497..de022a9 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -80,7 +80,6 @@ struct tegra_gpio_soc_config {
 	u32 upper_offset;
 };
 
-static struct device *dev;
 static struct irq_domain *irq_domain;
 static void __iomem *regs;
 static u32 tegra_gpio_bank_count;
@@ -240,7 +239,8 @@ static int tegra_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 
 	ret = gpiochip_lock_as_irq(&tegra_gpio_chip, gpio);
 	if (ret) {
-		dev_err(dev, "unable to lock Tegra GPIO %d as IRQ\n", gpio);
+		dev_err(tegra_gpio_chip.parent,
+			"unable to lock Tegra GPIO %d as IRQ\n", gpio);
 		return ret;
 	}
 
@@ -465,8 +465,6 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 	int i;
 	int j;
 
-	dev = &pdev->dev;
-
 	config = of_device_get_match_data(&pdev->dev);
 	if (!config) {
 		dev_err(&pdev->dev, "Error: No device match found\n");
@@ -488,6 +486,8 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 	}
 
 	tegra_gpio_chip.ngpio = tegra_gpio_bank_count * 32;
+	tegra_gpio_chip.parent = &pdev->dev;
+
 
 	tegra_gpio_banks = devm_kzalloc(&pdev->dev,
 			tegra_gpio_bank_count * sizeof(*tegra_gpio_banks),
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1381879 — Re: [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver

On 04/18/2016 02:46 AM, Laxman Dewangan wrote:
> Remove the file static device handle variable as this is just
> required for prints. The required handle can be stored in
> tegra_gpio_chip and hence it become redundancy.

This seems fine as far as it goes, but if it's worth doing this, please 
move all the globals into the GPIO chip rather than just one of the 7 
globals.

[toc] | [prev] | [next] | [standalone]

#1381927 — Re: [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver

On Monday 18 April 2016 09:59 PM, Stephen Warren wrote:
> On 04/18/2016 02:46 AM, Laxman Dewangan wrote:
>> Remove the file static device handle variable as this is just
>> required for prints. The required handle can be stored in
>> tegra_gpio_chip and hence it become redundancy.
>
> This seems fine as far as it goes, but if it's worth doing this, 
> please move all the globals into the GPIO chip rather than just one of 
> the 7 globals.

the device pointer is part of the gpiochip and so it is better to use 
gpiochip parent member instead of locally duplicating.

However, moving to other global variables needs some major changes and I 
think it should be treated as independent of this patch.
This patch just utilizes the gpiochip.parent here.

[toc] | [prev] | [next] | [standalone]

#1382647 — Re: [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver

On 04/18/2016 11:00 AM, Laxman Dewangan wrote:
>
> On Monday 18 April 2016 09:59 PM, Stephen Warren wrote:
>> On 04/18/2016 02:46 AM, Laxman Dewangan wrote:
>>> Remove the file static device handle variable as this is just
>>> required for prints. The required handle can be stored in
>>> tegra_gpio_chip and hence it become redundancy.
>>
>> This seems fine as far as it goes, but if it's worth doing this,
>> please move all the globals into the GPIO chip rather than just one of
>> the 7 globals.
>
> the device pointer is part of the gpiochip and so it is better to use
> gpiochip parent member instead of locally duplicating.
>
> However, moving to other global variables needs some major changes and I
> think it should be treated as independent of this patch.
> This patch just utilizes the gpiochip.parent here.

Looking at the patch this just trades using one global (dev) for another 
(tegra_gpio_chip), so when the other globals are removed, you'll need to 
go back and change tegra_gpio_irq_set_type() again to remove use of the 
global tegra_gpio_chip.

Still, this /does/ remove one global so I guess it's OK. I don't feel 
terribly strongly, especially if you're going to send more patches soon 
to remove the other globals. I'll leave the call to Linus.

[toc] | [prev] | [next] | [standalone]

#1382953 — Re: [PATCH 2/3] gpio: tegra: Remove the need of keeping device handle for gpio driver

On Mon, Apr 18, 2016 at 5:46 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> Remove the file static device handle variable as this is just
> required for prints. The required handle can be stored in
> tegra_gpio_chip and hence it become redundancy.

Small but still worthy change. "dev" in the file's global namespace is
scary and prone to conflict with local variables declarations.

Acked-by: Alexandre Courbot <acourbot@nvidia.com>

Now I hope you will take care of "regs" and the other static variables
as Stephen rightfully suggested. :)

[toc] | [prev] | [next] | [standalone]

#1381877 — Re: [PATCH 1/3] gpio: tegra: Don't open code of_device_get_match_data()

On 04/18/2016 02:46 AM, Laxman Dewangan wrote:
> Use of_device_get_match_data() for getting matched data
> instead of implementing this locally.

This patch,
Reviewed-by: Stephen Warren <swarren@nvidia.com>

[toc] | [prev] | [next] | [standalone]

#1382936

On Mon, Apr 18, 2016 at 5:46 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> Use of_device_get_match_data() for getting matched data
> instead of implementing this locally.

Maybe this is moving things around more than needed, but the end
result is arguably nicer.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web