All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral
@ 2022-05-06  5:44 kernel test robot
  0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2022-05-06  5:44 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 19981 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
BCC: lkp(a)intel.com
In-Reply-To: <20220505152521.71019-2-markuss.broks@gmail.com>
References: <20220505152521.71019-2-markuss.broks@gmail.com>
TO: Markuss Broks <markuss.broks@gmail.com>
TO: pavel(a)ucw.cz
CC: Markuss Broks <markuss.broks@gmail.com>
CC: Rob Herring <robh+dt@kernel.org>
CC: Krzysztof Kozlowski <krzk@kernel.org>
CC: Linus Walleij <linus.walleij@linaro.org>
CC: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
CC: Andy Shevchenko <andy.shevchenko@gmail.com>
CC: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
CC: linux-leds(a)vger.kernel.org
CC: devicetree(a)vger.kernel.org
CC: linux-kernel(a)vger.kernel.org

Hi Markuss,

I love your patch! Perhaps something to improve:

[auto build test WARNING on pavel-leds/for-next]
[also build test WARNING on robh/for-next v5.18-rc5]
[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]

url:    https://github.com/intel-lab-lkp/linux/commits/Markuss-Broks/leds-ktd2692-Avoid-duplicate-error-messages-on-probe-deferral/20220506-002732
base:   git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git for-next
:::::: branch date: 13 hours ago
:::::: commit date: 13 hours ago
config: arm-randconfig-c002-20220505 (https://download.01.org/0day-ci/archive/20220506/202205061352.CPtBdwTH-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5e004fb787698440a387750db7f8028e7cb14cfc)
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
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/844df84cf0fa041800781e7597b91580fa6e111b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Markuss-Broks/leds-ktd2692-Avoid-duplicate-error-messages-on-probe-deferral/20220506-002732
        git checkout 844df84cf0fa041800781e7597b91580fa6e111b
        # save the config file
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm clang-analyzer 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


clang-analyzer warnings: (new ones prefixed by >>)
   include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^~~~~~~~~~~~~~~~~
   note: expanded from here
   include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^~~~~~~~~~~~~~~~
   fs/hpfs/buffer.c:224:3: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
                   memcpy(qbh->bh[1]->b_data, qbh->data + 1 * 512, 512);
                   ^
   include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^~~~~~~~~~~~~~~~~
   note: expanded from here
   include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^~~~~~~~~~~~~~~~
   fs/hpfs/buffer.c:224:3: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
                   memcpy(qbh->bh[1]->b_data, qbh->data + 1 * 512, 512);
                   ^
   include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^~~~~~~~~~~~~~~~~
   note: expanded from here
   include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^~~~~~~~~~~~~~~~
   fs/hpfs/buffer.c:225:3: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
                   memcpy(qbh->bh[2]->b_data, qbh->data + 2 * 512, 512);
                   ^
   include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^~~~~~~~~~~~~~~~~
   note: expanded from here
   include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^~~~~~~~~~~~~~~~
   fs/hpfs/buffer.c:225:3: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
                   memcpy(qbh->bh[2]->b_data, qbh->data + 2 * 512, 512);
                   ^
   include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^~~~~~~~~~~~~~~~~
   note: expanded from here
   include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^~~~~~~~~~~~~~~~
   fs/hpfs/buffer.c:226:3: warning: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
                   memcpy(qbh->bh[3]->b_data, qbh->data + 3 * 512, 512);
                   ^
   include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^~~~~~~~~~~~~~~~~
   note: expanded from here
   include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^~~~~~~~~~~~~~~~
   fs/hpfs/buffer.c:226:3: note: Call to function 'memcpy' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memcpy_s' in case of C11
                   memcpy(qbh->bh[3]->b_data, qbh->data + 3 * 512, 512);
                   ^
   include/linux/fortify-string.h:369:26: note: expanded from macro 'memcpy'
   #define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                  \
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:362:2: note: expanded from macro '__fortify_memcpy_chk'
           __underlying_##op(p, q, __fortify_size);                        \
           ^~~~~~~~~~~~~~~~~
   note: expanded from here
   include/linux/fortify-string.h:45:29: note: expanded from macro '__underlying_memcpy'
   #define __underlying_memcpy     __builtin_memcpy
                                   ^~~~~~~~~~~~~~~~
   Suppressed 50 warnings (50 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   37 warnings generated.
   drivers/leds/uleds.c:150:4: warning: Value stored to 'retval' is never read [clang-analyzer-deadcode.DeadStores]
                           retval = copy_to_user(buffer, &udev->brightness,
                           ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/leds/uleds.c:150:4: note: Value stored to 'retval' is never read
                           retval = copy_to_user(buffer, &udev->brightness,
                           ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   Suppressed 36 warnings (36 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   51 warnings generated.
   Suppressed 51 warnings (50 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   50 warnings generated.
>> drivers/leds/flash/leds-ktd2692.c:241:15: warning: Assigned value is garbage or undefined [clang-analyzer-core.uninitialized.Assign]
           setting->max = cfg->flash_max_timeout;
                        ^
   drivers/leds/flash/leds-ktd2692.c:355:6: note: Assuming 'led' is non-null
           if (!led)
               ^~~~
   drivers/leds/flash/leds-ktd2692.c:355:2: note: Taking false branch
           if (!led)
           ^
   drivers/leds/flash/leds-ktd2692.c:361:8: note: Calling 'ktd2692_parse_dt'
           ret = ktd2692_parse_dt(led, &pdev->dev, &led_cfg);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/leds/flash/leds-ktd2692.c:277:6: note: Assuming 'np' is non-null
           if (!np)
               ^~~
   drivers/leds/flash/leds-ktd2692.c:277:2: note: Taking false branch
           if (!np)
           ^
   drivers/leds/flash/leds-ktd2692.c:282:6: note: 'ret' is not equal to 0
           if (ret)
               ^~~
   drivers/leds/flash/leds-ktd2692.c:282:2: note: Taking true branch
           if (ret)
           ^
   drivers/leds/flash/leds-ktd2692.c:283:3: note: Returning without writing to 'cfg->flash_max_timeout'
                   return dev_err_probe(dev, ret, "cannot get ctrl-gpios\n");
                   ^
   drivers/leds/flash/leds-ktd2692.c:361:8: note: Returning from 'ktd2692_parse_dt'
           ret = ktd2692_parse_dt(led, &pdev->dev, &led_cfg);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/leds/flash/leds-ktd2692.c:362:6: note: Assuming 'ret' is 0
           if (ret)
               ^~~
   drivers/leds/flash/leds-ktd2692.c:362:2: note: Taking false branch
           if (ret)
           ^
   drivers/leds/flash/leds-ktd2692.c:365:2: note: Calling 'ktd2692_init_flash_timeout'
           ktd2692_init_flash_timeout(fled_cdev, &led_cfg);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/leds/flash/leds-ktd2692.c:241:15: note: Assigned value is garbage or undefined
           setting->max = cfg->flash_max_timeout;
                        ^ ~~~~~~~~~~~~~~~~~~~~~~
   Suppressed 49 warnings (49 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   50 warnings generated.
   drivers/mmc/core/sdio_cis.c:63:3: warning: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119 [clang-analyzer-security.insecureAPI.strcpy]
                   strcpy(string, buf);
                   ^~~~~~
   drivers/mmc/core/sdio_cis.c:63:3: note: Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119
                   strcpy(string, buf);
                   ^~~~~~
   Suppressed 49 warnings (49 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   49 warnings generated.
   Suppressed 49 warnings (49 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   49 warnings generated.
   Suppressed 49 warnings (49 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   51 warnings generated.
   drivers/mfd/gateworks-gsc.c:113:8: warning: Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
                   rz = sprintf(buf, "%d\n", gsc->fwver);
                        ^~~~~~~
   drivers/mfd/gateworks-gsc.c:113:8: note: Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11
                   rz = sprintf(buf, "%d\n", gsc->fwver);
                        ^~~~~~~
   drivers/mfd/gateworks-gsc.c:115:8: warning: Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
                   rz = sprintf(buf, "0x%04x\n", gsc->fwcrc);
                        ^~~~~~~
   drivers/mfd/gateworks-gsc.c:115:8: note: Call to function 'sprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'sprintf_s' in case of C11
                   rz = sprintf(buf, "0x%04x\n", gsc->fwcrc);
                        ^~~~~~~
   Suppressed 49 warnings (49 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   36 warnings generated.
   Suppressed 36 warnings (36 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   49 warnings generated.
   Suppressed 49 warnings (49 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   49 warnings generated.
   Suppressed 49 warnings (49 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   62 warnings generated.
   drivers/mmc/host/mmc_spi.c:248:2: warning: Call to function 'snprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'snprintf_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           snprintf(tag, sizeof(tag), "  ... CMD%d response SPI_%s",
           ^~~~~~~~
   drivers/mmc/host/mmc_spi.c:248:2: note: Call to function 'snprintf' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'snprintf_s' in case of C11
           snprintf(tag, sizeof(tag), "  ... CMD%d response SPI_%s",
           ^~~~~~~~
   drivers/mmc/host/mmc_spi.c:354:4: warning: Value stored to 'end' is never read [clang-analyzer-deadcode.DeadStores]
                           end = cp+1;
                           ^     ~~~~
   drivers/mmc/host/mmc_spi.c:354:4: note: Value stored to 'end' is never read
                           end = cp+1;
                           ^     ~~~~
   drivers/mmc/host/mmc_spi.c:444:2: warning: Call to function 'memset' is insecure as it does not provide security checks introduced in the C11 standard. Replace with analogous functions that support length arguments or provides boundary checks such as 'memset_s' in case of C11 [clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling]
           memset(cp, 0xff, sizeof(data->status));
           ^
   include/linux/fortify-string.h:272:25: note: expanded from macro 'memset'
   #define memset(p, c, s) __fortify_memset_chk(p, c, s,                   \

vim +241 drivers/leds/flash/leds-ktd2692.c

b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  233  
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  234  static void ktd2692_init_flash_timeout(struct led_classdev_flash *fled_cdev,
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  235  				       struct ktd2692_led_config_data *cfg)
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  236  {
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  237  	struct led_flash_setting *setting;
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  238  
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  239  	setting = &fled_cdev->timeout;
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  240  	setting->min = KTD2692_FLASH_MODE_TIMEOUT_DISABLE;
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12 @241  	setting->max = cfg->flash_max_timeout;
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  242  	setting->step = cfg->flash_max_timeout
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  243  			/ (KTD2692_FLASH_MODE_TIMEOUT_LEVELS - 1);
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  244  	setting->val = cfg->flash_max_timeout;
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  245  }
b7da8c5c725cc2 drivers/leds/leds-ktd2692.c Ingi Kim 2015-05-12  246  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral
  2022-05-07 21:14     ` Pavel Machek
@ 2022-05-08  9:55       ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-05-08  9:55 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Markuss Broks, Rob Herring, Krzysztof Kozlowski, Linus Walleij,
	Lad Prabhakar, Christophe JAILLET, linux-leds, devicetree,
	linux-kernel

On Sat, May 7, 2022 at 11:14 PM Pavel Machek <pavel@ucw.cz> wrote:
>
> Hi!
>
> > > Use dev_err_probe instead of dev_err to avoid duplicate error
> > > messages if the GPIO allocation makes the probe defer.
> >
> > Thanks!
> >
> > There are two improvements we can make here.
> > 1) adding a Fixes tag, so it can be backported to stable kernels;
> > 2) see below.
>
> I don't believe this is severe-enough error to be worth stable.

Fixes tag is good to have in either case, but I agree that is not so critical.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral
  2022-05-05 17:29   ` Andy Shevchenko
  2022-05-05 17:34     ` Andy Shevchenko
@ 2022-05-07 21:14     ` Pavel Machek
  2022-05-08  9:55       ` Andy Shevchenko
  1 sibling, 1 reply; 6+ messages in thread
From: Pavel Machek @ 2022-05-07 21:14 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Markuss Broks, Rob Herring, Krzysztof Kozlowski, Linus Walleij,
	Lad Prabhakar, Christophe JAILLET, linux-leds, devicetree,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

Hi!

> > Use dev_err_probe instead of dev_err to avoid duplicate error
> > messages if the GPIO allocation makes the probe defer.
> 
> Thanks!
> 
> There are two improvements we can make here.
> 1) adding a Fixes tag, so it can be backported to stable kernels;
> 2) see below.

I don't believe this is severe-enough error to be worth stable.

Best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral
  2022-05-05 17:29   ` Andy Shevchenko
@ 2022-05-05 17:34     ` Andy Shevchenko
  2022-05-07 21:14     ` Pavel Machek
  1 sibling, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-05-05 17:34 UTC (permalink / raw)
  To: Markuss Broks
  Cc: pavel, Rob Herring, Krzysztof Kozlowski, Linus Walleij,
	Lad Prabhakar, Christophe JAILLET, linux-leds, devicetree,
	linux-kernel

On Thu, May 5, 2022 at 7:29 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, May 5, 2022 at 5:25 PM Markuss Broks <markuss.broks@gmail.com> wrote:

...

> 1) adding a Fixes tag, so it can be backported to stable kernels;

To avoid additional versions in case you don't know how Fixes tag
should look like:

Fixes: b7da8c5c725c ("leds: Add ktd2692 flash LED driver")

Just add this before other tags, like SoB or Reviewed-by (no blank
lines in the tag block).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral
  2022-05-05 15:25 ` [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral Markuss Broks
@ 2022-05-05 17:29   ` Andy Shevchenko
  2022-05-05 17:34     ` Andy Shevchenko
  2022-05-07 21:14     ` Pavel Machek
  0 siblings, 2 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-05-05 17:29 UTC (permalink / raw)
  To: Markuss Broks
  Cc: pavel, Rob Herring, Krzysztof Kozlowski, Linus Walleij,
	Lad Prabhakar, Christophe JAILLET, linux-leds, devicetree,
	linux-kernel

On Thu, May 5, 2022 at 5:25 PM Markuss Broks <markuss.broks@gmail.com> wrote:
>
> Use dev_err_probe instead of dev_err to avoid duplicate error
> messages if the GPIO allocation makes the probe defer.

Thanks!

There are two improvements we can make here.
1) adding a Fixes tag, so it can be backported to stable kernels;
2) see below.

...

>         led->ctrl_gpio = devm_gpiod_get(dev, "ctrl", GPIOD_ASIS);
>         ret = PTR_ERR_OR_ZERO(led->ctrl_gpio);
> -       if (ret) {
> -               dev_err(dev, "cannot get ctrl-gpios %d\n", ret);
> -               return ret;
> -       }
> +       if (ret)
> +               return dev_err_probe(dev, ret, "cannot get ctrl-gpios\n");

You may improve this and simultaneously prepare for the next change to
be smaller

       led->ctrl_gpio = devm_gpiod_get(dev, "ctrl", GPIOD_ASIS);
       if (IS_ERR(led->ctrl_gpio))
               return dev_err_probe(dev, PTR_ERR(led->ctrl_gpio),
"cannot get ctrl-gpios\n");

All the same for the other case.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral
  2022-05-05 15:25 [PATCH v8 0/2] Make AUX gpio pin optional for ktd2692 Markuss Broks
@ 2022-05-05 15:25 ` Markuss Broks
  2022-05-05 17:29   ` Andy Shevchenko
  0 siblings, 1 reply; 6+ messages in thread
From: Markuss Broks @ 2022-05-05 15:25 UTC (permalink / raw)
  To: pavel
  Cc: Markuss Broks, Rob Herring, Krzysztof Kozlowski, Linus Walleij,
	Lad Prabhakar, Andy Shevchenko, Christophe JAILLET, linux-leds,
	devicetree, linux-kernel

Use dev_err_probe instead of dev_err to avoid duplicate error
messages if the GPIO allocation makes the probe defer.

Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 drivers/leds/flash/leds-ktd2692.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/flash/leds-ktd2692.c b/drivers/leds/flash/leds-ktd2692.c
index ed1f20a58bf6..8b99742cb68a 100644
--- a/drivers/leds/flash/leds-ktd2692.c
+++ b/drivers/leds/flash/leds-ktd2692.c
@@ -279,17 +279,13 @@ static int ktd2692_parse_dt(struct ktd2692_context *led, struct device *dev,
 
 	led->ctrl_gpio = devm_gpiod_get(dev, "ctrl", GPIOD_ASIS);
 	ret = PTR_ERR_OR_ZERO(led->ctrl_gpio);
-	if (ret) {
-		dev_err(dev, "cannot get ctrl-gpios %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "cannot get ctrl-gpios\n");
 
 	led->aux_gpio = devm_gpiod_get(dev, "aux", GPIOD_ASIS);
 	ret = PTR_ERR_OR_ZERO(led->aux_gpio);
-	if (ret) {
-		dev_err(dev, "cannot get aux-gpios %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, PTR_ERR(led->aux_gpio), "cannot get aux-gpios\n");
 
 	led->regulator = devm_regulator_get(dev, "vin");
 	if (IS_ERR(led->regulator))
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-05-08  9:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-06  5:44 [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2022-05-05 15:25 [PATCH v8 0/2] Make AUX gpio pin optional for ktd2692 Markuss Broks
2022-05-05 15:25 ` [PATCH v8 1/3] leds: ktd2692: Avoid duplicate error messages on probe deferral Markuss Broks
2022-05-05 17:29   ` Andy Shevchenko
2022-05-05 17:34     ` Andy Shevchenko
2022-05-07 21:14     ` Pavel Machek
2022-05-08  9:55       ` Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.