[PATCH v1 0/5] pinctrl: at91: Cleanups

[PATCH v1 0/5] pinctrl: at91: Cleanups

[Andy Shevchenko]

The kasprintf() patch makes me look into the driver code and besides
missed fix, there is a room to improve. Hence this series.

(for now not even compile tested)

Andy Shevchenko (5):
  pinctrl: at91: use devm_kasprintf() to avoid potential leaks (part 2)
  pinctrl: at91: Don't mix non-devm calls with devm ones
  pinctrl: at91: Use of_device_get_match_data()
  pinctrl: at91: Use dev_err_probe() instead of custom messaging
  pinctrl: at91: Utilise temporary variable for struct device

 drivers/pinctrl/pinctrl-at91.c | 164 ++++++++++++++-------------------
 1 file changed, 67 insertions(+), 97 deletions(-)

-- 
2.39.1

[PATCH v1 1/5] pinctrl: at91: use devm_kasprintf() to avoid potential leaks (part 2)

[Andy Shevchenko]

Use devm_kasprintf() instead of kasprintf() to avoid any potential
leaks. At the moment drivers have no remove functionality hence
there is no need for fixes tag.

While at it, switch to use devm_kasprintf_strarray().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-at91.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 735c501e7a06..bb93d8bcfd08 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -18,6 +18,7 @@
 #include <linux/pm.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
+#include <linux/string_helpers.h>
 
 /* Since we request GPIOs from ourself */
 #include <linux/pinctrl/consumer.h>
@@ -1371,6 +1372,7 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 
 static int at91_pinctrl_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct at91_pinctrl *info;
 	struct pinctrl_pin_desc *pdesc;
 	int ret, i, j, k;
@@ -1394,9 +1396,19 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	for (i = 0, k = 0; i < gpio_banks; i++) {
+		char **pin_names;
+
+		pin_names = devm_kasprintf_strarray(dev, "pio", MAX_NB_GPIO_PER_BANK);
+		if (!names)
+			return -ENOMEM;
+
 		for (j = 0; j < MAX_NB_GPIO_PER_BANK; j++, k++) {
+			char *pin_name = pin_names[j];
+
+			strreplace(pin_name, '-', i + 'A');
+
 			pdesc->number = k;
-			pdesc->name = kasprintf(GFP_KERNEL, "pio%c%d", i + 'A', j);
+			pdesc->name = pin_name;
 			pdesc++;
 		}
 	}
@@ -1797,7 +1809,8 @@ static const struct of_device_id at91_gpio_of_match[] = {
 
 static int at91_gpio_probe(struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct at91_gpio_chip *at91_chip = NULL;
 	struct gpio_chip *chip;
 	struct pinctrl_gpio_range *range;
@@ -1866,16 +1879,14 @@ static int at91_gpio_probe(struct platform_device *pdev)
 			chip->ngpio = ngpio;
 	}
 
-	names = devm_kcalloc(&pdev->dev, chip->ngpio, sizeof(char *),
-			     GFP_KERNEL);
-
+	names = devm_kasprintf_strarray(dev, "pio", chip->ngpio);
 	if (!names) {
 		ret = -ENOMEM;
 		goto clk_enable_err;
 	}
 
 	for (i = 0; i < chip->ngpio; i++)
-		names[i] = devm_kasprintf(&pdev->dev, GFP_KERNEL, "pio%c%d", alias_idx + 'A', i);
+		strreplace('-', alias_idx + 'A');
 
 	chip->names = (const char *const *)names;
 
-- 
2.39.1

Re: [PATCH v1 1/5] pinctrl: at91: use devm_kasprintf() to avoid potential leaks (part 2)

[kernel test robot]

Hi Andy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linusw-pinctrl/devel]
[also build test WARNING on linusw-pinctrl/for-next next-20230210]
[cannot apply to clk/clk-next soc/for-next linus/master v6.2-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/pinctrl-at91-use-devm_kasprintf-to-avoid-potential-leaks-part-2/20230210-225817
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link:    https://lore.kernel.org/r/20230210145656.71838-2-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 1/5] pinctrl: at91: use devm_kasprintf() to avoid potential leaks (part 2)
config: arm-randconfig-r046-20230210 (https://download.01.org/0day-ci/archive/20230211/202302110336.1P7vM7AU-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/0562771ccfa099db4361c2e5958ca1685f498cdf
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Andy-Shevchenko/pinctrl-at91-use-devm_kasprintf-to-avoid-potential-leaks-part-2/20230210-225817
        git checkout 0562771ccfa099db4361c2e5958ca1685f498cdf
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/pinctrl/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302110336.1P7vM7AU-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/pinctrl/pinctrl-at91.c: In function 'at91_pinctrl_probe':
   drivers/pinctrl/pinctrl-at91.c:1402:22: error: 'names' undeclared (first use in this function)
    1402 |                 if (!names)
         |                      ^~~~~
   drivers/pinctrl/pinctrl-at91.c:1402:22: note: each undeclared identifier is reported only once for each function it appears in
   drivers/pinctrl/pinctrl-at91.c: In function 'at91_gpio_probe':
>> drivers/pinctrl/pinctrl-at91.c:1889:28: warning: passing argument 1 of 'strreplace' makes pointer from integer without a cast [-Wint-conversion]
    1889 |                 strreplace('-', alias_idx + 'A');
         |                            ^~~
         |                            |
         |                            int
   In file included from include/linux/bitmap.h:11,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/mutex.h:17,
                    from include/linux/notifier.h:14,
                    from include/linux/clk.h:14,
                    from drivers/pinctrl/pinctrl-at91.c:8:
   include/linux/string.h:172:24: note: expected 'char *' but argument is of type 'int'
     172 | char *strreplace(char *s, char old, char new);
         |                  ~~~~~~^
   drivers/pinctrl/pinctrl-at91.c:1889:17: error: too few arguments to function 'strreplace'
    1889 |                 strreplace('-', alias_idx + 'A');
         |                 ^~~~~~~~~~
   include/linux/string.h:172:7: note: declared here
     172 | char *strreplace(char *s, char old, char new);
         |       ^~~~~~~~~~


vim +/strreplace +1889 drivers/pinctrl/pinctrl-at91.c

  1809	
  1810	static int at91_gpio_probe(struct platform_device *pdev)
  1811	{
  1812		struct device *dev = &pdev->dev;
  1813		struct device_node *np = dev->of_node;
  1814		struct at91_gpio_chip *at91_chip = NULL;
  1815		struct gpio_chip *chip;
  1816		struct pinctrl_gpio_range *range;
  1817		int ret = 0;
  1818		int irq, i;
  1819		int alias_idx = of_alias_get_id(np, "gpio");
  1820		uint32_t ngpio;
  1821		char **names;
  1822	
  1823		BUG_ON(alias_idx >= ARRAY_SIZE(gpio_chips));
  1824		if (gpio_chips[alias_idx]) {
  1825			ret = -EBUSY;
  1826			goto err;
  1827		}
  1828	
  1829		irq = platform_get_irq(pdev, 0);
  1830		if (irq < 0) {
  1831			ret = irq;
  1832			goto err;
  1833		}
  1834	
  1835		at91_chip = devm_kzalloc(&pdev->dev, sizeof(*at91_chip), GFP_KERNEL);
  1836		if (!at91_chip) {
  1837			ret = -ENOMEM;
  1838			goto err;
  1839		}
  1840	
  1841		at91_chip->regbase = devm_platform_ioremap_resource(pdev, 0);
  1842		if (IS_ERR(at91_chip->regbase)) {
  1843			ret = PTR_ERR(at91_chip->regbase);
  1844			goto err;
  1845		}
  1846	
  1847		at91_chip->ops = (const struct at91_pinctrl_mux_ops *)
  1848			of_match_device(at91_gpio_of_match, &pdev->dev)->data;
  1849		at91_chip->pioc_virq = irq;
  1850		at91_chip->pioc_idx = alias_idx;
  1851	
  1852		at91_chip->clock = devm_clk_get(&pdev->dev, NULL);
  1853		if (IS_ERR(at91_chip->clock)) {
  1854			dev_err(&pdev->dev, "failed to get clock, ignoring.\n");
  1855			ret = PTR_ERR(at91_chip->clock);
  1856			goto err;
  1857		}
  1858	
  1859		ret = clk_prepare_enable(at91_chip->clock);
  1860		if (ret) {
  1861			dev_err(&pdev->dev, "failed to prepare and enable clock, ignoring.\n");
  1862			goto clk_enable_err;
  1863		}
  1864	
  1865		at91_chip->chip = at91_gpio_template;
  1866		at91_chip->id = alias_idx;
  1867	
  1868		chip = &at91_chip->chip;
  1869		chip->label = dev_name(&pdev->dev);
  1870		chip->parent = &pdev->dev;
  1871		chip->owner = THIS_MODULE;
  1872		chip->base = alias_idx * MAX_NB_GPIO_PER_BANK;
  1873	
  1874		if (!of_property_read_u32(np, "#gpio-lines", &ngpio)) {
  1875			if (ngpio >= MAX_NB_GPIO_PER_BANK)
  1876				pr_err("at91_gpio.%d, gpio-nb >= %d failback to %d\n",
  1877				       alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
  1878			else
  1879				chip->ngpio = ngpio;
  1880		}
  1881	
  1882		names = devm_kasprintf_strarray(dev, "pio", chip->ngpio);
  1883		if (!names) {
  1884			ret = -ENOMEM;
  1885			goto clk_enable_err;
  1886		}
  1887	
  1888		for (i = 0; i < chip->ngpio; i++)
> 1889			strreplace('-', alias_idx + 'A');
  1890	
  1891		chip->names = (const char *const *)names;
  1892	
  1893		range = &at91_chip->range;
  1894		range->name = chip->label;
  1895		range->id = alias_idx;
  1896		range->pin_base = range->base = range->id * MAX_NB_GPIO_PER_BANK;
  1897	
  1898		range->npins = chip->ngpio;
  1899		range->gc = chip;
  1900	
  1901		ret = at91_gpio_of_irq_setup(pdev, at91_chip);
  1902		if (ret)
  1903			goto gpiochip_add_err;
  1904	
  1905		ret = gpiochip_add_data(chip, at91_chip);
  1906		if (ret)
  1907			goto gpiochip_add_err;
  1908	
  1909		gpio_chips[alias_idx] = at91_chip;
  1910		platform_set_drvdata(pdev, at91_chip);
  1911		gpio_banks = max(gpio_banks, alias_idx + 1);
  1912	
  1913		dev_info(&pdev->dev, "at address %p\n", at91_chip->regbase);
  1914	
  1915		return 0;
  1916	
  1917	gpiochip_add_err:
  1918	clk_enable_err:
  1919		clk_disable_unprepare(at91_chip->clock);
  1920	err:
  1921		dev_err(&pdev->dev, "Failure %i for GPIO %i\n", ret, alias_idx);
  1922	
  1923		return ret;
  1924	}
  1925	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v1 1/5] pinctrl: at91: use devm_kasprintf() to avoid potential leaks (part 2)

[kernel test robot]

Hi Andy,

I love your patch! Yet something to improve:

[auto build test ERROR on linusw-pinctrl/devel]
[also build test ERROR on linusw-pinctrl/for-next next-20230210]
[cannot apply to clk/clk-next soc/for-next linus/master v6.2-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/pinctrl-at91-use-devm_kasprintf-to-avoid-potential-leaks-part-2/20230210-225817
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link:    https://lore.kernel.org/r/20230210145656.71838-2-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 1/5] pinctrl: at91: use devm_kasprintf() to avoid potential leaks (part 2)
config: arm-randconfig-r046-20230210 (https://download.01.org/0day-ci/archive/20230211/202302110407.TpDeAlpQ-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/0562771ccfa099db4361c2e5958ca1685f498cdf
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Andy-Shevchenko/pinctrl-at91-use-devm_kasprintf-to-avoid-potential-leaks-part-2/20230210-225817
        git checkout 0562771ccfa099db4361c2e5958ca1685f498cdf
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302110407.TpDeAlpQ-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pinctrl/pinctrl-at91.c: In function 'at91_pinctrl_probe':
>> drivers/pinctrl/pinctrl-at91.c:1402:22: error: 'names' undeclared (first use in this function)
    1402 |                 if (!names)
         |                      ^~~~~
   drivers/pinctrl/pinctrl-at91.c:1402:22: note: each undeclared identifier is reported only once for each function it appears in
   drivers/pinctrl/pinctrl-at91.c: In function 'at91_gpio_probe':
   drivers/pinctrl/pinctrl-at91.c:1889:28: warning: passing argument 1 of 'strreplace' makes pointer from integer without a cast [-Wint-conversion]
    1889 |                 strreplace('-', alias_idx + 'A');
         |                            ^~~
         |                            |
         |                            int
   In file included from include/linux/bitmap.h:11,
                    from include/linux/cpumask.h:12,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/mutex.h:17,
                    from include/linux/notifier.h:14,
                    from include/linux/clk.h:14,
                    from drivers/pinctrl/pinctrl-at91.c:8:
   include/linux/string.h:172:24: note: expected 'char *' but argument is of type 'int'
     172 | char *strreplace(char *s, char old, char new);
         |                  ~~~~~~^
>> drivers/pinctrl/pinctrl-at91.c:1889:17: error: too few arguments to function 'strreplace'
    1889 |                 strreplace('-', alias_idx + 'A');
         |                 ^~~~~~~~~~
   include/linux/string.h:172:7: note: declared here
     172 | char *strreplace(char *s, char old, char new);
         |       ^~~~~~~~~~


vim +/names +1402 drivers/pinctrl/pinctrl-at91.c

  1372	
  1373	static int at91_pinctrl_probe(struct platform_device *pdev)
  1374	{
  1375		struct device *dev = &pdev->dev;
  1376		struct at91_pinctrl *info;
  1377		struct pinctrl_pin_desc *pdesc;
  1378		int ret, i, j, k;
  1379	
  1380		info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
  1381		if (!info)
  1382			return -ENOMEM;
  1383	
  1384		ret = at91_pinctrl_probe_dt(pdev, info);
  1385		if (ret)
  1386			return ret;
  1387	
  1388		at91_pinctrl_desc.name = dev_name(&pdev->dev);
  1389		at91_pinctrl_desc.npins = gpio_banks * MAX_NB_GPIO_PER_BANK;
  1390		at91_pinctrl_desc.pins = pdesc =
  1391			devm_kcalloc(&pdev->dev,
  1392				     at91_pinctrl_desc.npins, sizeof(*pdesc),
  1393				     GFP_KERNEL);
  1394	
  1395		if (!at91_pinctrl_desc.pins)
  1396			return -ENOMEM;
  1397	
  1398		for (i = 0, k = 0; i < gpio_banks; i++) {
  1399			char **pin_names;
  1400	
  1401			pin_names = devm_kasprintf_strarray(dev, "pio", MAX_NB_GPIO_PER_BANK);
> 1402			if (!names)
  1403				return -ENOMEM;
  1404	
  1405			for (j = 0; j < MAX_NB_GPIO_PER_BANK; j++, k++) {
  1406				char *pin_name = pin_names[j];
  1407	
  1408				strreplace(pin_name, '-', i + 'A');
  1409	
  1410				pdesc->number = k;
  1411				pdesc->name = pin_name;
  1412				pdesc++;
  1413			}
  1414		}
  1415	
  1416		platform_set_drvdata(pdev, info);
  1417		info->pctl = devm_pinctrl_register(&pdev->dev, &at91_pinctrl_desc,
  1418						   info);
  1419	
  1420		if (IS_ERR(info->pctl)) {
  1421			dev_err(&pdev->dev, "could not register AT91 pinctrl driver\n");
  1422			return PTR_ERR(info->pctl);
  1423		}
  1424	
  1425		/* We will handle a range of GPIO pins */
  1426		for (i = 0; i < gpio_banks; i++)
  1427			if (gpio_chips[i])
  1428				pinctrl_add_gpio_range(info->pctl, &gpio_chips[i]->range);
  1429	
  1430		dev_info(&pdev->dev, "initialized AT91 pinctrl driver\n");
  1431	
  1432		return 0;
  1433	}
  1434	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

[PATCH v1 2/5] pinctrl: at91: Don't mix non-devm calls with devm ones

[Andy Shevchenko]

Replace devm_clk_get() by devm_clk_get_enabled() and drop
unneeded code pieces. This will make sure we keep the ordering
of the resource allocation correct.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-at91.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index bb93d8bcfd08..6d5ebdae0bf9 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1849,19 +1849,13 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	at91_chip->pioc_virq = irq;
 	at91_chip->pioc_idx = alias_idx;
 
-	at91_chip->clock = devm_clk_get(&pdev->dev, NULL);
+	at91_chip->clock = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(at91_chip->clock)) {
 		dev_err(&pdev->dev, "failed to get clock, ignoring.\n");
 		ret = PTR_ERR(at91_chip->clock);
 		goto err;
 	}
 
-	ret = clk_prepare_enable(at91_chip->clock);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to prepare and enable clock, ignoring.\n");
-		goto clk_enable_err;
-	}
-
 	at91_chip->chip = at91_gpio_template;
 	at91_chip->id = alias_idx;
 
@@ -1882,7 +1876,7 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	names = devm_kasprintf_strarray(dev, "pio", chip->ngpio);
 	if (!names) {
 		ret = -ENOMEM;
-		goto clk_enable_err;
+		goto err;
 	}
 
 	for (i = 0; i < chip->ngpio; i++)
@@ -1915,8 +1909,6 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	return 0;
 
 gpiochip_add_err:
-clk_enable_err:
-	clk_disable_unprepare(at91_chip->clock);
 err:
 	dev_err(&pdev->dev, "Failure %i for GPIO %i\n", ret, alias_idx);
 
-- 
2.39.1

[PATCH v1 3/5] pinctrl: at91: Use of_device_get_match_data()

[Andy Shevchenko]

Use of_device_get_match_data() to simplify the code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-at91.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 6d5ebdae0bf9..203a29ad9344 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1304,8 +1304,7 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 		return -ENODEV;
 
 	info->dev = &pdev->dev;
-	info->ops = (const struct at91_pinctrl_mux_ops *)
-		of_match_device(at91_pinctrl_of_match, &pdev->dev)->data;
+	info->ops = of_device_get_match_data(dev);
 	at91_pinctrl_child_count(info, np);
 
 	/*
@@ -1844,8 +1843,7 @@ static int at91_gpio_probe(struct platform_device *pdev)
 		goto err;
 	}
 
-	at91_chip->ops = (const struct at91_pinctrl_mux_ops *)
-		of_match_device(at91_gpio_of_match, &pdev->dev)->data;
+	at91_chip->ops = of_device_get_match_data(dev);
 	at91_chip->pioc_virq = irq;
 	at91_chip->pioc_idx = alias_idx;
 
-- 
2.39.1

[PATCH v1 4/5] pinctrl: at91: Use dev_err_probe() instead of custom messaging

[Andy Shevchenko]

The custom message has no value except printing the error code,
the same does dev_err_probe(). Let's use the latter for the sake
of unification.

Note that some APIs already have messaging in them and some simply
do not require the current noise.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-at91.c | 64 +++++++++++-----------------------
 1 file changed, 21 insertions(+), 43 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 203a29ad9344..797382a237e8 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1294,10 +1294,11 @@ static const struct of_device_id at91_pinctrl_of_match[] = {
 static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 				 struct at91_pinctrl *info)
 {
+	struct device *dev = &pdev->dev;
 	int ret = 0;
 	int i, j, ngpio_chips_enabled = 0;
 	uint32_t *tmp;
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = dev->of_node;
 	struct device_node *child;
 
 	if (!np)
@@ -1360,9 +1361,8 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 			continue;
 		ret = at91_pinctrl_parse_functions(child, info, i++);
 		if (ret) {
-			dev_err(&pdev->dev, "failed to parse function\n");
 			of_node_put(child);
-			return ret;
+			return dev_err_probe(dev, ret, "failed to parse function\n");
 		}
 	}
 
@@ -1415,11 +1415,8 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, info);
 	info->pctl = devm_pinctrl_register(&pdev->dev, &at91_pinctrl_desc,
 					   info);
-
-	if (IS_ERR(info->pctl)) {
-		dev_err(&pdev->dev, "could not register AT91 pinctrl driver\n");
-		return PTR_ERR(info->pctl);
-	}
+	if (IS_ERR(info->pctl))
+		return dev_err_probe(dev, PTR_ERR(info->pctl), "could not register AT91 pinctrl driver\n");
 
 	/* We will handle a range of GPIO pins */
 	for (i = 0; i < gpio_banks; i++)
@@ -1820,39 +1817,28 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	char **names;
 
 	BUG_ON(alias_idx >= ARRAY_SIZE(gpio_chips));
-	if (gpio_chips[alias_idx]) {
-		ret = -EBUSY;
-		goto err;
-	}
+	if (gpio_chips[alias_idx])
+		return dev_err_probe(dev, -EBUSY, "%d slot is occupied.\n", alias_idx);
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		ret = irq;
-		goto err;
-	}
+	if (irq < 0)
+		return irq;
 
 	at91_chip = devm_kzalloc(&pdev->dev, sizeof(*at91_chip), GFP_KERNEL);
-	if (!at91_chip) {
-		ret = -ENOMEM;
-		goto err;
-	}
+	if (!at91_chip)
+		return -ENOMEM;
 
 	at91_chip->regbase = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(at91_chip->regbase)) {
-		ret = PTR_ERR(at91_chip->regbase);
-		goto err;
-	}
+	if (IS_ERR(at91_chip->regbase))
+		return PTR_ERR(at91_chip->regbase);
 
 	at91_chip->ops = of_device_get_match_data(dev);
 	at91_chip->pioc_virq = irq;
 	at91_chip->pioc_idx = alias_idx;
 
 	at91_chip->clock = devm_clk_get_enabled(&pdev->dev, NULL);
-	if (IS_ERR(at91_chip->clock)) {
-		dev_err(&pdev->dev, "failed to get clock, ignoring.\n");
-		ret = PTR_ERR(at91_chip->clock);
-		goto err;
-	}
+	if (IS_ERR(at91_chip->clock))
+		return dev_err_probe(dev, PTR_ERR(at91_chip->clock), "failed to get clock, ignoring.\n");
 
 	at91_chip->chip = at91_gpio_template;
 	at91_chip->id = alias_idx;
@@ -1865,17 +1851,15 @@ static int at91_gpio_probe(struct platform_device *pdev)
 
 	if (!of_property_read_u32(np, "#gpio-lines", &ngpio)) {
 		if (ngpio >= MAX_NB_GPIO_PER_BANK)
-			pr_err("at91_gpio.%d, gpio-nb >= %d failback to %d\n",
-			       alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
+			dev_err(dev, "at91_gpio.%d, gpio-nb >= %d failback to %d\n",
+				alias_idx, MAX_NB_GPIO_PER_BANK, MAX_NB_GPIO_PER_BANK);
 		else
 			chip->ngpio = ngpio;
 	}
 
 	names = devm_kasprintf_strarray(dev, "pio", chip->ngpio);
-	if (!names) {
-		ret = -ENOMEM;
-		goto err;
-	}
+	if (!names)
+		return -ENOMEM;
 
 	for (i = 0; i < chip->ngpio; i++)
 		strreplace('-', alias_idx + 'A');
@@ -1892,11 +1876,11 @@ static int at91_gpio_probe(struct platform_device *pdev)
 
 	ret = at91_gpio_of_irq_setup(pdev, at91_chip);
 	if (ret)
-		goto gpiochip_add_err;
+		return ret;
 
 	ret = gpiochip_add_data(chip, at91_chip);
 	if (ret)
-		goto gpiochip_add_err;
+		return ret;
 
 	gpio_chips[alias_idx] = at91_chip;
 	platform_set_drvdata(pdev, at91_chip);
@@ -1905,12 +1889,6 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "at address %p\n", at91_chip->regbase);
 
 	return 0;
-
-gpiochip_add_err:
-err:
-	dev_err(&pdev->dev, "Failure %i for GPIO %i\n", ret, alias_idx);
-
-	return ret;
 }
 
 static const struct dev_pm_ops at91_gpio_pm_ops = {
-- 
2.39.1

[PATCH v1 5/5] pinctrl: at91: Utilise temporary variable for struct device

[Andy Shevchenko]

We have a temporary variable to keep pointer to struct device.
Utilise it inside the ->probe() implementation.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pinctrl/pinctrl-at91.c | 63 +++++++++++++++-------------------
 1 file changed, 27 insertions(+), 36 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 797382a237e8..f57781c2786a 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1304,7 +1304,7 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 	if (!np)
 		return -ENODEV;
 
-	info->dev = &pdev->dev;
+	info->dev = dev;
 	info->ops = of_device_get_match_data(dev);
 	at91_pinctrl_child_count(info, np);
 
@@ -1324,35 +1324,30 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
 	if (ret)
 		return ret;
 
-	dev_dbg(&pdev->dev, "nmux = %d\n", info->nmux);
+	dev_dbg(dev, "nmux = %d\n", info->nmux);
 
-	dev_dbg(&pdev->dev, "mux-mask\n");
+	dev_dbg(dev, "mux-mask\n");
 	tmp = info->mux_mask;
 	for (i = 0; i < gpio_banks; i++) {
-		for (j = 0; j < info->nmux; j++, tmp++) {
-			dev_dbg(&pdev->dev, "%d:%d\t0x%x\n", i, j, tmp[0]);
-		}
+		for (j = 0; j < info->nmux; j++, tmp++)
+			dev_dbg(dev, "%d:%d\t0x%x\n", i, j, tmp[0]);
 	}
 
-	dev_dbg(&pdev->dev, "nfunctions = %d\n", info->nfunctions);
-	dev_dbg(&pdev->dev, "ngroups = %d\n", info->ngroups);
-	info->functions = devm_kcalloc(&pdev->dev,
-					info->nfunctions,
-					sizeof(struct at91_pmx_func),
-					GFP_KERNEL);
+	dev_dbg(dev, "nfunctions = %d\n", info->nfunctions);
+	dev_dbg(dev, "ngroups = %d\n", info->ngroups);
+	info->functions = devm_kcalloc(dev, info->nfunctions, sizeof(*info->functions),
+				       GFP_KERNEL);
 	if (!info->functions)
 		return -ENOMEM;
 
-	info->groups = devm_kcalloc(&pdev->dev,
-					info->ngroups,
-					sizeof(struct at91_pin_group),
-					GFP_KERNEL);
+	info->groups = devm_kcalloc(dev, info->ngroups, sizeof(*info->groups),
+				    GFP_KERNEL);
 	if (!info->groups)
 		return -ENOMEM;
 
-	dev_dbg(&pdev->dev, "nbanks = %d\n", gpio_banks);
-	dev_dbg(&pdev->dev, "nfunctions = %d\n", info->nfunctions);
-	dev_dbg(&pdev->dev, "ngroups = %d\n", info->ngroups);
+	dev_dbg(dev, "nbanks = %d\n", gpio_banks);
+	dev_dbg(dev, "nfunctions = %d\n", info->nfunctions);
+	dev_dbg(dev, "ngroups = %d\n", info->ngroups);
 
 	i = 0;
 
@@ -1376,7 +1371,7 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 	struct pinctrl_pin_desc *pdesc;
 	int ret, i, j, k;
 
-	info = devm_kzalloc(&pdev->dev, sizeof(*info), GFP_KERNEL);
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
 	if (!info)
 		return -ENOMEM;
 
@@ -1384,13 +1379,10 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	at91_pinctrl_desc.name = dev_name(&pdev->dev);
+	at91_pinctrl_desc.name = dev_name(dev);
 	at91_pinctrl_desc.npins = gpio_banks * MAX_NB_GPIO_PER_BANK;
 	at91_pinctrl_desc.pins = pdesc =
-		devm_kcalloc(&pdev->dev,
-			     at91_pinctrl_desc.npins, sizeof(*pdesc),
-			     GFP_KERNEL);
-
+		devm_kcalloc(dev, at91_pinctrl_desc.npins, sizeof(*pdesc), GFP_KERNEL);
 	if (!at91_pinctrl_desc.pins)
 		return -ENOMEM;
 
@@ -1413,8 +1405,7 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, info);
-	info->pctl = devm_pinctrl_register(&pdev->dev, &at91_pinctrl_desc,
-					   info);
+	info->pctl = devm_pinctrl_register(dev, &at91_pinctrl_desc, info);
 	if (IS_ERR(info->pctl))
 		return dev_err_probe(dev, PTR_ERR(info->pctl), "could not register AT91 pinctrl driver\n");
 
@@ -1423,7 +1414,7 @@ static int at91_pinctrl_probe(struct platform_device *pdev)
 		if (gpio_chips[i])
 			pinctrl_add_gpio_range(info->pctl, &gpio_chips[i]->range);
 
-	dev_info(&pdev->dev, "initialized AT91 pinctrl driver\n");
+	dev_info(dev, "initialized AT91 pinctrl driver\n");
 
 	return 0;
 }
@@ -1714,6 +1705,7 @@ static void gpio_irq_handler(struct irq_desc *desc)
 static int at91_gpio_of_irq_setup(struct platform_device *pdev,
 				  struct at91_gpio_chip *at91_gpio)
 {
+	struct device		*dev = &pdev->dev;
 	struct gpio_chip	*gpiochip_prev = NULL;
 	struct at91_gpio_chip   *prev = NULL;
 	struct irq_data		*d = irq_get_irq_data(at91_gpio->pioc_virq);
@@ -1721,8 +1713,7 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,
 	struct gpio_irq_chip	*girq;
 	int i;
 
-	gpio_irqchip = devm_kzalloc(&pdev->dev, sizeof(*gpio_irqchip),
-				    GFP_KERNEL);
+	gpio_irqchip = devm_kzalloc(dev, sizeof(*gpio_irqchip), GFP_KERNEL);
 	if (!gpio_irqchip)
 		return -ENOMEM;
 
@@ -1758,7 +1749,7 @@ static int at91_gpio_of_irq_setup(struct platform_device *pdev,
 	if (!gpiochip_prev) {
 		girq->parent_handler = gpio_irq_handler;
 		girq->num_parents = 1;
-		girq->parents = devm_kcalloc(&pdev->dev, 1,
+		girq->parents = devm_kcalloc(dev, girq->num_parents,
 					     sizeof(*girq->parents),
 					     GFP_KERNEL);
 		if (!girq->parents)
@@ -1824,7 +1815,7 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	at91_chip = devm_kzalloc(&pdev->dev, sizeof(*at91_chip), GFP_KERNEL);
+	at91_chip = devm_kzalloc(dev, sizeof(*at91_chip), GFP_KERNEL);
 	if (!at91_chip)
 		return -ENOMEM;
 
@@ -1836,7 +1827,7 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	at91_chip->pioc_virq = irq;
 	at91_chip->pioc_idx = alias_idx;
 
-	at91_chip->clock = devm_clk_get_enabled(&pdev->dev, NULL);
+	at91_chip->clock = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(at91_chip->clock))
 		return dev_err_probe(dev, PTR_ERR(at91_chip->clock), "failed to get clock, ignoring.\n");
 
@@ -1844,8 +1835,8 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	at91_chip->id = alias_idx;
 
 	chip = &at91_chip->chip;
-	chip->label = dev_name(&pdev->dev);
-	chip->parent = &pdev->dev;
+	chip->label = dev_name(dev);
+	chip->parent = dev;
 	chip->owner = THIS_MODULE;
 	chip->base = alias_idx * MAX_NB_GPIO_PER_BANK;
 
@@ -1886,7 +1877,7 @@ static int at91_gpio_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, at91_chip);
 	gpio_banks = max(gpio_banks, alias_idx + 1);
 
-	dev_info(&pdev->dev, "at address %p\n", at91_chip->regbase);
+	dev_info(dev, "at address %p\n", at91_chip->regbase);
 
 	return 0;
 }
-- 
2.39.1