* [PATCH 0/2] leds: change led_brightness definition from enum to typedef
@ 2021-07-15 21:18 Amy Parker
2021-07-15 21:18 ` [PATCH 2/2] drivers/leds/TODO: update TODO to reflect changes Amy Parker
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
0 siblings, 2 replies; 14+ messages in thread
From: Amy Parker @ 2021-07-15 21:18 UTC (permalink / raw)
To: pavel; +Cc: Amy Parker, linux-leds, linux-kernel
This patch series solves the TODO listed in drivers/leds, and changes
the implementation of led_brightness from an enum to a combined typedef
of a u8 and set of macro definitions as suggested in the TODO.
Signed-off-by: Amy Parker <apark0006@student.cerritos.edu>
Amy Parker (2):
swap led_brightness from enum to typedef
drivers/leds/TODO: update TODO to reflect changes
Documentation/leds/leds-class-flash.rst | 2 +-
arch/arm/mach-davinci/board-dm365-evm.c | 4 +--
arch/arm/mach-footbridge/ebsa285.c | 4 +--
arch/arm/mach-footbridge/netwinder-hw.c | 4 +--
arch/arm/mach-pxa/idp.c | 4 +--
arch/arm/mach-pxa/lubbock.c | 4 +--
arch/arm/mach-pxa/mainstone.c | 4 +--
arch/arm/plat-omap/debug-leds.c | 4 +--
arch/mips/alchemy/devboards/db1000.c | 4 +--
arch/mips/alchemy/devboards/db1200.c | 4 +--
arch/mips/alchemy/devboards/db1300.c | 4 +--
arch/mips/txx9/rbtx4939/setup.c | 2 +-
arch/x86/platform/ts5500/ts5500.c | 4 +--
drivers/gpu/drm/nouveau/nouveau_led.c | 4 +--
drivers/hid/hid-asus.c | 4 +--
drivers/hid/hid-bigbenff.c | 4 +--
drivers/hid/hid-corsair.c | 6 ++--
drivers/hid/hid-elan.c | 2 +-
drivers/hid/hid-google-hammer.c | 2 +-
drivers/hid/hid-gt683r.c | 4 +--
drivers/hid/hid-led.c | 20 ++++++------
drivers/hid/hid-lenovo.c | 4 +--
drivers/hid/hid-lg-g15.c | 24 +++++++-------
drivers/hid/hid-lg4ff.c | 4 +--
drivers/hid/hid-picolcd_leds.c | 4 +--
drivers/hid/hid-sony.c | 4 +--
drivers/hid/hid-steelseries.c | 8 ++---
drivers/hid/hid-u2fzero.c | 2 +-
drivers/hid/hid-wiimote-modules.c | 4 +--
drivers/hid/wacom.h | 2 +-
drivers/hid/wacom_sys.c | 8 ++---
drivers/hwmon/applesmc.c | 2 +-
drivers/hwmon/pmbus/ibm-cffps.c | 2 +-
drivers/input/input-leds.c | 4 +--
drivers/input/joystick/xpad.c | 2 +-
drivers/input/keyboard/applespi.c | 2 +-
drivers/input/keyboard/cap11xx.c | 2 +-
drivers/input/keyboard/lm8323.c | 2 +-
drivers/input/keyboard/qt2160.c | 4 +--
drivers/input/keyboard/tm2-touchkey.c | 2 +-
drivers/input/misc/apanel.c | 2 +-
drivers/input/misc/ims-pcu.c | 4 +--
drivers/input/misc/wistron_btns.c | 4 +--
drivers/input/touchscreen/stmfts.c | 4 +--
drivers/leds/TODO | 4 ---
drivers/leds/blink/leds-lgm-sso.c | 6 ++--
drivers/leds/flash/leds-rt4505.c | 4 +--
drivers/leds/flash/leds-rt8515.c | 2 +-
drivers/leds/led-class-multicolor.c | 2 +-
drivers/leds/led-triggers.c | 2 +-
drivers/leds/leds-88pm860x.c | 2 +-
drivers/leds/leds-aat1290.c | 8 ++---
drivers/leds/leds-acer-a500.c | 2 +-
drivers/leds/leds-adp5520.c | 2 +-
drivers/leds/leds-an30259a.c | 2 +-
drivers/leds/leds-apu.c | 4 +--
drivers/leds/leds-ariel.c | 4 +--
drivers/leds/leds-as3645a.c | 4 +--
drivers/leds/leds-asic3.c | 2 +-
drivers/leds/leds-aw2013.c | 2 +-
drivers/leds/leds-bcm6328.c | 2 +-
drivers/leds/leds-bcm6358.c | 2 +-
drivers/leds/leds-bd2802.c | 2 +-
drivers/leds/leds-blinkm.c | 8 ++---
drivers/leds/leds-clevo-mail.c | 2 +-
drivers/leds/leds-cobalt-qube.c | 2 +-
drivers/leds/leds-cobalt-raq.c | 4 +--
drivers/leds/leds-cpcap.c | 2 +-
drivers/leds/leds-cr0014114.c | 2 +-
drivers/leds/leds-da903x.c | 2 +-
drivers/leds/leds-da9052.c | 4 +--
drivers/leds/leds-dac124s085.c | 2 +-
drivers/leds/leds-el15203000.c | 2 +-
drivers/leds/leds-fsg.c | 12 +++----
drivers/leds/leds-gpio.c | 4 +--
drivers/leds/leds-hp6xx.c | 4 +--
drivers/leds/leds-ip30.c | 2 +-
drivers/leds/leds-ipaq-micro.c | 2 +-
drivers/leds/leds-is31fl319x.c | 2 +-
drivers/leds/leds-is31fl32xx.c | 2 +-
drivers/leds/leds-ktd2692.c | 6 ++--
drivers/leds/leds-lm3530.c | 4 +--
drivers/leds/leds-lm3532.c | 2 +-
drivers/leds/leds-lm3533.c | 4 +--
drivers/leds/leds-lm355x.c | 6 ++--
drivers/leds/leds-lm3601x.c | 2 +-
drivers/leds/leds-lm36274.c | 2 +-
drivers/leds/leds-lm3642.c | 6 ++--
drivers/leds/leds-lm3692x.c | 4 +--
drivers/leds/leds-lm3697.c | 2 +-
drivers/leds/leds-locomo.c | 6 ++--
drivers/leds/leds-lp3944.c | 4 +--
drivers/leds/leds-lp3952.c | 2 +-
drivers/leds/leds-lp50xx.c | 2 +-
drivers/leds/leds-lp55xx-common.c | 4 +--
drivers/leds/leds-lp8788.c | 2 +-
drivers/leds/leds-lp8860.c | 2 +-
drivers/leds/leds-lt3593.c | 2 +-
drivers/leds/leds-max77650.c | 2 +-
drivers/leds/leds-max77693.c | 2 +-
drivers/leds/leds-max8997.c | 4 +--
drivers/leds/leds-mc13783.c | 2 +-
drivers/leds/leds-menf21bmc.c | 2 +-
drivers/leds/leds-mlxcpld.c | 4 +--
drivers/leds/leds-mlxreg.c | 8 ++---
drivers/leds/leds-mt6323.c | 10 +++---
drivers/leds/leds-net48xx.c | 2 +-
drivers/leds/leds-netxbig.c | 2 +-
drivers/leds/leds-nic78bx.c | 4 +--
drivers/leds/leds-ns2.c | 4 +--
drivers/leds/leds-ot200.c | 2 +-
drivers/leds/leds-pca9532.c | 4 +--
drivers/leds/leds-pca955x.c | 2 +-
drivers/leds/leds-pca963x.c | 4 +--
drivers/leds/leds-pm8058.c | 6 ++--
drivers/leds/leds-powernv.c | 8 ++---
drivers/leds/leds-pwm.c | 2 +-
drivers/leds/leds-rb532.c | 4 +--
drivers/leds/leds-regulator.c | 4 +--
drivers/leds/leds-s3c24xx.c | 2 +-
drivers/leds/leds-sc27xx-bltc.c | 4 +--
drivers/leds/leds-sgm3140.c | 2 +-
drivers/leds/leds-spi-byte.c | 2 +-
drivers/leds/leds-ss4200.c | 2 +-
drivers/leds/leds-sunfire.c | 18 +++++------
drivers/leds/leds-syscon.c | 2 +-
drivers/leds/leds-tca6507.c | 2 +-
drivers/leds/leds-tlc591xx.c | 2 +-
drivers/leds/leds-tps6105x.c | 2 +-
drivers/leds/leds-turris-omnia.c | 2 +-
drivers/leds/leds-wm831x-status.c | 4 +--
drivers/leds/leds-wm8350.c | 2 +-
drivers/leds/leds-wrap.c | 6 ++--
drivers/leds/trigger/ledtrig-audio.c | 6 ++--
drivers/leds/trigger/ledtrig-camera.c | 4 +--
drivers/leds/uleds.c | 2 +-
drivers/macintosh/via-pmu-led.c | 2 +-
drivers/media/radio/radio-shark.c | 6 ++--
drivers/media/radio/radio-shark2.c | 4 +--
drivers/media/rc/redrat3.c | 2 +-
drivers/media/rc/ttusbir.c | 4 +--
drivers/media/rc/winbond-cir.c | 4 +--
.../media/v4l2-core/v4l2-flash-led-class.c | 6 ++--
drivers/mmc/host/rtsx_usb_sdmmc.c | 2 +-
drivers/mmc/host/sdhci.c | 2 +-
drivers/net/arcnet/com20020-pci.c | 4 +--
drivers/net/dsa/hirschmann/hellcreek_ptp.c | 12 +++----
drivers/net/wireless/ath/ath5k/led.c | 2 +-
drivers/net/wireless/ath/ath9k/gpio.c | 2 +-
drivers/net/wireless/ath/ath9k/htc.h | 2 +-
drivers/net/wireless/ath/ath9k/htc_drv_gpio.c | 2 +-
drivers/net/wireless/ath/carl9170/led.c | 2 +-
drivers/net/wireless/broadcom/b43/leds.c | 2 +-
.../net/wireless/broadcom/b43legacy/leds.c | 2 +-
.../broadcom/brcm80211/brcmsmac/led.c | 2 +-
drivers/net/wireless/intel/iwlegacy/common.c | 2 +-
drivers/net/wireless/intel/iwlwifi/dvm/led.c | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/led.c | 2 +-
drivers/net/wireless/intersil/p54/led.c | 2 +-
.../net/wireless/mediatek/mt76/mt7603/init.c | 2 +-
.../wireless/mediatek/mt76/mt7615/pci_init.c | 2 +-
.../net/wireless/mediatek/mt76/mt76x02_util.c | 2 +-
.../net/wireless/ralink/rt2x00/rt2400pci.c | 2 +-
.../net/wireless/ralink/rt2x00/rt2500pci.c | 2 +-
.../net/wireless/ralink/rt2x00/rt2500usb.c | 2 +-
.../net/wireless/ralink/rt2x00/rt2800lib.c | 2 +-
drivers/net/wireless/ralink/rt2x00/rt61pci.c | 2 +-
drivers/net/wireless/ralink/rt2x00/rt73usb.c | 2 +-
.../wireless/realtek/rtl818x/rtl8187/leds.c | 2 +-
.../platform/chrome/cros_kbd_led_backlight.c | 4 +--
.../platform/chrome/wilco_ec/keyboard_leds.c | 4 +--
drivers/platform/x86/acer-wmi.c | 2 +-
drivers/platform/x86/asus-laptop.c | 8 ++---
drivers/platform/x86/asus-wireless.c | 4 +--
drivers/platform/x86/asus-wmi.c | 18 +++++------
drivers/platform/x86/dell/alienware-wmi.c | 4 +--
drivers/platform/x86/dell/dell-laptop.c | 16 +++++-----
drivers/platform/x86/dell/dell-wmi-led.c | 2 +-
drivers/platform/x86/dell/dell-wmi-privacy.c | 2 +-
drivers/platform/x86/eeepc-laptop.c | 4 +--
drivers/platform/x86/fujitsu-laptop.c | 22 ++++++-------
drivers/platform/x86/hp_accel.c | 8 ++---
drivers/platform/x86/huawei-wmi.c | 2 +-
drivers/platform/x86/ideapad-laptop.c | 4 +--
drivers/platform/x86/lg-laptop.c | 8 ++---
drivers/platform/x86/samsung-laptop.c | 4 +--
drivers/platform/x86/system76_acpi.c | 16 +++++-----
drivers/platform/x86/thinkpad_acpi.c | 20 ++++++------
drivers/platform/x86/topstar-laptop.c | 4 +--
drivers/platform/x86/toshiba_acpi.c | 12 +++----
drivers/staging/greybus/light.c | 4 +--
drivers/staging/nvec/nvec_paz00.c | 2 +-
drivers/video/backlight/adp8860_bl.c | 4 +--
drivers/video/backlight/adp8870_bl.c | 4 +--
drivers/video/backlight/lm3639_bl.c | 4 +--
include/linux/led-class-multicolor.h | 4 +--
include/linux/leds-regulator.h | 2 +-
include/linux/leds.h | 31 +++++++++----------
include/linux/mfd/wm8350/pmic.h | 2 +-
include/media/v4l2-flash-led-class.h | 6 ++--
net/rfkill/core.c | 2 +-
sound/pci/hda/hda_generic.c | 6 ++--
sound/pci/hda/hda_generic.h | 4 +--
sound/pci/hda/patch_conexant.c | 6 ++--
sound/pci/hda/patch_realtek.c | 12 +++----
sound/pci/hda/patch_sigmatel.c | 4 +--
sound/usb/line6/toneport.c | 2 +-
207 files changed, 436 insertions(+), 441 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] drivers/leds/TODO: update TODO to reflect changes
2021-07-15 21:18 [PATCH 0/2] leds: change led_brightness definition from enum to typedef Amy Parker
@ 2021-07-15 21:18 ` Amy Parker
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
1 sibling, 0 replies; 14+ messages in thread
From: Amy Parker @ 2021-07-15 21:18 UTC (permalink / raw)
To: pavel; +Cc: Amy Parker, linux-leds, linux-kernel
The previous commit in this chain (swap led_brightness from enum to
typedef) fixed the removed issue in this TODO. This patch updates
the TODO to indicate this.
Signed-off-by: Amy Parker <apark0006@student.cerritos.edu>
---
drivers/leds/TODO | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/leds/TODO b/drivers/leds/TODO
index e1d771513b98..7b97bb733392 100644
--- a/drivers/leds/TODO
+++ b/drivers/leds/TODO
@@ -1,10 +1,6 @@
-*- org -*-
* On/off LEDs should have max_brightness of 1
-* Get rid of led_brightness
-
-It is really an integer, as maximum is configurable. Get rid of it, or
-make it into typedef or something.
* Review atomicity requirements in LED subsystem
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
@ 2021-07-16 0:41 ` kernel test robot
2021-07-16 2:14 ` kernel test robot
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-07-16 0:41 UTC (permalink / raw)
To: Amy Parker, pavel
Cc: clang-built-linux, kbuild-all, Amy Parker, linux-leds,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]
Hi Amy,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.14-rc1 next-20210715]
[cannot apply to pavel-linux-leds/for-next wireless-drivers-next/master wireless-drivers/master]
[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/0day-ci/linux/commits/Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd9c7df94c1b23feacd54112f33ad95d93f64533
config: arm-randconfig-r023-20210715 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 0e49c54a8cbd3e779e5526a5888c683c01cc3c50)
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/0day-ci/linux/commit/b14a971f1045205d49d9d001f33d33afdd8208f9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
git checkout b14a971f1045205d49d9d001f33d33afdd8208f9
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/media/v4l2-core/v4l2-flash-led-class.c:15:
>> include/media/v4l2-flash-led-class.h:18:1: warning: declaration does not declare anything [-Wmissing-declarations]
led_brightness;
^~~~~~~~~~~~~~
1 warning generated.
vim +18 include/media/v4l2-flash-led-class.h
14
15 struct led_classdev_flash;
16 struct led_classdev;
17 struct v4l2_flash;
> 18 led_brightness;
19
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26518 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
2021-07-16 0:41 ` [PATCH 1/2] swap led_brightness from enum to typedef kernel test robot
@ 2021-07-16 2:14 ` kernel test robot
2021-07-16 3:11 ` Amy Parker
2021-07-19 7:16 ` Geert Uytterhoeven
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: kernel test robot @ 2021-07-16 2:14 UTC (permalink / raw)
To: Amy Parker, pavel; +Cc: kbuild-all, Amy Parker, linux-leds, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]
Hi Amy,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.14-rc1 next-20210715]
[cannot apply to pavel-linux-leds/for-next wireless-drivers-next/master wireless-drivers/master]
[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/0day-ci/linux/commits/Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd9c7df94c1b23feacd54112f33ad95d93f64533
config: m68k-buildonly-randconfig-r006-20210715 (attached as .config)
compiler: m68k-linux-gcc (GCC) 10.3.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/0day-ci/linux/commit/b14a971f1045205d49d9d001f33d33afdd8208f9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
git checkout b14a971f1045205d49d9d001f33d33afdd8208f9
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash drivers/md/bcache/ drivers/media/v4l2-core/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from drivers/media/v4l2-core/v4l2-flash-led-class.c:15:
>> include/media/v4l2-flash-led-class.h:18:1: warning: useless type name in empty declaration
18 | led_brightness;
| ^~~~~~~~~~~~~~
vim +18 include/media/v4l2-flash-led-class.h
14
15 struct led_classdev_flash;
16 struct led_classdev;
17 struct v4l2_flash;
> 18 led_brightness;
19
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28694 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
2021-07-16 2:14 ` kernel test robot
@ 2021-07-16 3:11 ` Amy Parker
2021-07-16 21:07 ` Amy Parker
0 siblings, 1 reply; 14+ messages in thread
From: Amy Parker @ 2021-07-16 3:11 UTC (permalink / raw)
To: kernel test robot; +Cc: pavel, kbuild-all, linux-leds, linux-kernel
Ah - I see there was an issue with header files not being properly updated.
Check back for another patch resolving this.
On Thu, Jul 15, 2021 at 7:15 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Amy,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v5.14-rc1 next-20210715]
> [cannot apply to pavel-linux-leds/for-next wireless-drivers-next/master wireless-drivers/master]
> [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/0day-ci/linux/commits/Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd9c7df94c1b23feacd54112f33ad95d93f64533
> config: m68k-buildonly-randconfig-r006-20210715 (attached as .config)
> compiler: m68k-linux-gcc (GCC) 10.3.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/0day-ci/linux/commit/b14a971f1045205d49d9d001f33d33afdd8208f9
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
> git checkout b14a971f1045205d49d9d001f33d33afdd8208f9
> # save the attached .config to linux build tree
> mkdir build_dir
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash drivers/md/bcache/ drivers/media/v4l2-core/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> In file included from drivers/media/v4l2-core/v4l2-flash-led-class.c:15:
> >> include/media/v4l2-flash-led-class.h:18:1: warning: useless type name in empty declaration
> 18 | led_brightness;
> | ^~~~~~~~~~~~~~
>
>
> vim +18 include/media/v4l2-flash-led-class.h
>
> 14
> 15 struct led_classdev_flash;
> 16 struct led_classdev;
> 17 struct v4l2_flash;
> > 18 led_brightness;
> 19
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
--
Amy Iris Parker
Please refer to me using she/her pronouns.
CIS Major
Dual Enrollment with WHS co2025
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
2021-07-16 3:11 ` Amy Parker
@ 2021-07-16 21:07 ` Amy Parker
2021-07-16 21:43 ` Randy Dunlap
0 siblings, 1 reply; 14+ messages in thread
From: Amy Parker @ 2021-07-16 21:07 UTC (permalink / raw)
To: kernel test robot; +Cc: pavel, kbuild-all, linux-leds, linux-kernel
On Thu, Jul 15, 2021 at 8:11 PM Amy Parker
<apark0006@student.cerritos.edu> wrote:
>
> Ah - I see there was an issue with header files not being properly updated.
>
> Check back for another patch resolving this.
>
>
> On Thu, Jul 15, 2021 at 7:15 PM kernel test robot <lkp@intel.com> wrote:
> >
> > Hi Amy,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on v5.14-rc1 next-20210715]
> > [cannot apply to pavel-linux-leds/for-next wireless-drivers-next/master wireless-drivers/master]
> > [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/0day-ci/linux/commits/Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd9c7df94c1b23feacd54112f33ad95d93f64533
> > config: m68k-buildonly-randconfig-r006-20210715 (attached as .config)
> > compiler: m68k-linux-gcc (GCC) 10.3.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/0day-ci/linux/commit/b14a971f1045205d49d9d001f33d33afdd8208f9
> > git remote add linux-review https://github.com/0day-ci/linux
> > git fetch --no-tags linux-review Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
> > git checkout b14a971f1045205d49d9d001f33d33afdd8208f9
> > # save the attached .config to linux build tree
> > mkdir build_dir
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash drivers/md/bcache/ drivers/media/v4l2-core/
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>):
> >
> > In file included from drivers/media/v4l2-core/v4l2-flash-led-class.c:15:
> > >> include/media/v4l2-flash-led-class.h:18:1: warning: useless type name in empty declaration
> > 18 | led_brightness;
> > | ^~~~~~~~~~~~~~
> >
> >
> > vim +18 include/media/v4l2-flash-led-class.h
> >
> > 14
> > 15 struct led_classdev_flash;
> > 16 struct led_classdev;
> > 17 struct v4l2_flash;
> > > 18 led_brightness;
> > 19
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
>
>
> --
> Amy Iris Parker
> Please refer to me using she/her pronouns.
> CIS Major
> Dual Enrollment with WHS co2025
Another patch was sent into the list to correct this error.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
2021-07-16 21:07 ` Amy Parker
@ 2021-07-16 21:43 ` Randy Dunlap
2021-07-19 12:05 ` Pavel Machek
0 siblings, 1 reply; 14+ messages in thread
From: Randy Dunlap @ 2021-07-16 21:43 UTC (permalink / raw)
To: Amy Parker, kernel test robot; +Cc: pavel, kbuild-all, linux-leds, linux-kernel
Amy,
Please see comments below.
On 7/16/21 2:07 PM, Amy Parker wrote:
> On Thu, Jul 15, 2021 at 8:11 PM Amy Parker
> <apark0006@student.cerritos.edu> wrote:
>>
>> Ah - I see there was an issue with header files not being properly updated.
>>
>> Check back for another patch resolving this.
>>
>>
>> On Thu, Jul 15, 2021 at 7:15 PM kernel test robot <lkp@intel.com> wrote:
>>>
>>> Hi Amy,
>>>
>>> Thank you for the patch! Perhaps something to improve:
>>>
>>> [auto build test WARNING on linus/master]
>>> [also build test WARNING on v5.14-rc1 next-20210715]
>>> [cannot apply to pavel-linux-leds/for-next wireless-drivers-next/master wireless-drivers/master]
>>> [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/0day-ci/linux/commits/Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
>>> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd9c7df94c1b23feacd54112f33ad95d93f64533
>>> config: m68k-buildonly-randconfig-r006-20210715 (attached as .config)
>>> compiler: m68k-linux-gcc (GCC) 10.3.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/0day-ci/linux/commit/b14a971f1045205d49d9d001f33d33afdd8208f9
>>> git remote add linux-review https://github.com/0day-ci/linux
>>> git fetch --no-tags linux-review Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
>>> git checkout b14a971f1045205d49d9d001f33d33afdd8208f9
>>> # save the attached .config to linux build tree
>>> mkdir build_dir
>>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-10.3.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash drivers/md/bcache/ drivers/media/v4l2-core/
>>>
>>> If you fix the issue, kindly add following tag as appropriate
>>> Reported-by: kernel test robot <lkp@intel.com>
>>>
>>> All warnings (new ones prefixed by >>):
>>>
>>> In file included from drivers/media/v4l2-core/v4l2-flash-led-class.c:15:
>>>>> include/media/v4l2-flash-led-class.h:18:1: warning: useless type name in empty declaration
>>> 18 | led_brightness;
>>> | ^~~~~~~~~~~~~~
>>>
>>>
>>> vim +18 include/media/v4l2-flash-led-class.h
>>>
>>> 14
>>> 15 struct led_classdev_flash;
>>> 16 struct led_classdev;
>>> 17 struct v4l2_flash;
>>> > 18 led_brightness;
>>> 19
>>>
>>> ---
>
> Another patch was sent into the list to correct this error.
Hopefully Pavel (LED subsystem maintainer) will comment soon-ish.
My comments:
a. This patch would be the right thing to do if your large patch had already been
applied (merged) somewhere, but AFAIK it hasn't been. So:
b. IMO you should resend your entire patch set with this fix included.
Send it as "v2" (version 2) and explain the changes in it since your
original patch (which was v1). This v2 explanation should be below the
"---" line in the patch. (See Documentation/process/submitting-patches.rst
for more info -- or ask for more info/help.)
c. For your follow-up patch to include/media/v4l2-flash-led-class.h, which was:
-led_brightness;
+typedef u8 led_brightness;
I would just add this to include/media/v4l2-flash-led-class.h:
#include <linux/leds.h>
That way, in a few years, when the type of led_brightness changes again,
someone won't have to remember to search for other typedefs of it and
update them also. Or maybe they will do that after a bug happens and
someone notices it.
(Note that I am just trying to help. Pavel has more of a final
say-so about this.)
HTH.
--
~Randy
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
2021-07-16 0:41 ` [PATCH 1/2] swap led_brightness from enum to typedef kernel test robot
2021-07-16 2:14 ` kernel test robot
@ 2021-07-19 7:16 ` Geert Uytterhoeven
2021-07-19 8:40 ` Geert Uytterhoeven
2021-07-19 8:05 ` Dan Carpenter
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Geert Uytterhoeven @ 2021-07-19 7:16 UTC (permalink / raw)
To: Amy Parker; +Cc: pavel, linux-leds, linux-kernel
Hi Amy,
On Thu, 15 Jul 2021, Amy Parker wrote:
> This commit changes how led_brightness, declared in header file
> include/linux/leds.h, works throughout the kernel, and updates other
> files in accordance.
>
> The TODO located at drivers/leds/TODO requests:
>
> * Get rid of led_brightness
>
> It is really an integer, as maximum is configurable. Get rid of it, or
> make it into typedef or something.
>
> This patch changes the declaration of led_brightness from an enum to a
> typedef. In order to hold the currently existing enum values, macro
> definitions are provided. Files which use led_brightness are updated to
> conform to the new types.
>
> Signed-off-by: Amy Parker <apark0006@student.cerritos.edu>
Thanks for your patch!
> 207 files changed, 437 insertions(+), 438 deletions(-)
This touches a lot of files, so we better get it right.
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -26,12 +26,11 @@ struct device_node;
> */
>
> /* This is obsolete/useless. We now support variable maximum brightness. */
> -enum led_brightness {
> - LED_OFF = 0,
> - LED_ON = 1,
> - LED_HALF = 127,
> - LED_FULL = 255,
> -};
> +typedef u8 led_brightness;
In general, typedefs are frowned upon in the kernel, but there can be a
good reason to use one.
What if the maximum brightness is larger than 255?
Using "unsigned int" sounds better to me, but let's wait for Pavel...
> +#define LED_OFF 0
> +#define LED_ON 1
> +#define LED_HALF 127
> +#define LED_FULL 255
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
` (2 preceding siblings ...)
2021-07-19 7:16 ` Geert Uytterhoeven
@ 2021-07-19 8:05 ` Dan Carpenter
2021-07-19 8:21 ` Dan Carpenter
2021-07-19 12:08 ` Pavel Machek
5 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2021-07-19 8:05 UTC (permalink / raw)
To: kbuild, Amy Parker, pavel
Cc: lkp, kbuild-all, Amy Parker, linux-leds, linux-kernel
Hi Amy,
url: https://github.com/0day-ci/linux/commits/Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd9c7df94c1b23feacd54112f33ad95d93f64533
config: i386-randconfig-m021-20210715 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/hid/hid-corsair.c:157 k90_backlight_get() warn: signedness bug returning '(-12)'
vim +157 drivers/hid/hid-corsair.c
b14a971f104520 Amy Parker 2021-07-15 145 static led_brightness k90_backlight_get(struct led_classdev *led_cdev)
^^^^^^^^^^^^^^
Just declare this as int instead of typedef.
6f78193ee9ea55 Clément Vuchener 2015-09-30 146 {
6f78193ee9ea55 Clément Vuchener 2015-09-30 147 int ret;
6f78193ee9ea55 Clément Vuchener 2015-09-30 148 struct k90_led *led = container_of(led_cdev, struct k90_led, cdev);
6f78193ee9ea55 Clément Vuchener 2015-09-30 149 struct device *dev = led->cdev.dev->parent;
6f78193ee9ea55 Clément Vuchener 2015-09-30 150 struct usb_interface *usbif = to_usb_interface(dev->parent);
6f78193ee9ea55 Clément Vuchener 2015-09-30 151 struct usb_device *usbdev = interface_to_usbdev(usbif);
6f78193ee9ea55 Clément Vuchener 2015-09-30 152 int brightness;
6d104af38b570d Johan Hovold 2017-01-12 153 char *data;
6d104af38b570d Johan Hovold 2017-01-12 154
6d104af38b570d Johan Hovold 2017-01-12 155 data = kmalloc(8, GFP_KERNEL);
6d104af38b570d Johan Hovold 2017-01-12 156 if (!data)
6d104af38b570d Johan Hovold 2017-01-12 @157 return -ENOMEM;
Negative error codes
6f78193ee9ea55 Clément Vuchener 2015-09-30 158
6f78193ee9ea55 Clément Vuchener 2015-09-30 159 ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
6f78193ee9ea55 Clément Vuchener 2015-09-30 160 K90_REQUEST_STATUS,
6f78193ee9ea55 Clément Vuchener 2015-09-30 161 USB_DIR_IN | USB_TYPE_VENDOR |
6f78193ee9ea55 Clément Vuchener 2015-09-30 162 USB_RECIP_DEVICE, 0, 0, data, 8,
6f78193ee9ea55 Clément Vuchener 2015-09-30 163 USB_CTRL_SET_TIMEOUT);
7a546af50eb78a Johan Hovold 2017-01-12 164 if (ret < 5) {
6f78193ee9ea55 Clément Vuchener 2015-09-30 165 dev_warn(dev, "Failed to get K90 initial state (error %d).\n",
6f78193ee9ea55 Clément Vuchener 2015-09-30 166 ret);
6d104af38b570d Johan Hovold 2017-01-12 167 ret = -EIO;
6d104af38b570d Johan Hovold 2017-01-12 168 goto out;
6f78193ee9ea55 Clément Vuchener 2015-09-30 169 }
6f78193ee9ea55 Clément Vuchener 2015-09-30 170 brightness = data[4];
6f78193ee9ea55 Clément Vuchener 2015-09-30 171 if (brightness < 0 || brightness > 3) {
6f78193ee9ea55 Clément Vuchener 2015-09-30 172 dev_warn(dev,
6f78193ee9ea55 Clément Vuchener 2015-09-30 173 "Read invalid backlight brightness: %02hhx.\n",
6f78193ee9ea55 Clément Vuchener 2015-09-30 174 data[4]);
6d104af38b570d Johan Hovold 2017-01-12 175 ret = -EIO;
6d104af38b570d Johan Hovold 2017-01-12 176 goto out;
6f78193ee9ea55 Clément Vuchener 2015-09-30 177 }
6d104af38b570d Johan Hovold 2017-01-12 178 ret = brightness;
6d104af38b570d Johan Hovold 2017-01-12 179 out:
6d104af38b570d Johan Hovold 2017-01-12 180 kfree(data);
6d104af38b570d Johan Hovold 2017-01-12 181
6d104af38b570d Johan Hovold 2017-01-12 182 return ret;
6f78193ee9ea55 Clément Vuchener 2015-09-30 183 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
` (3 preceding siblings ...)
2021-07-19 8:05 ` Dan Carpenter
@ 2021-07-19 8:21 ` Dan Carpenter
2021-07-19 12:08 ` Pavel Machek
5 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2021-07-19 8:21 UTC (permalink / raw)
To: kbuild, Amy Parker, pavel
Cc: lkp, kbuild-all, Amy Parker, linux-leds, linux-kernel
Hi Amy,
url: https://github.com/0day-ci/linux/commits/Amy-Parker/leds-change-led_brightness-definition-from-enum-to-typedef/20210716-052140
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd9c7df94c1b23feacd54112f33ad95d93f64533
config: x86_64-randconfig-m001-20210718 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/platform/chrome/cros_kbd_led_backlight.c:53 keyboard_led_get_brightness() warn: signedness bug returning '(-5)'
drivers/platform/x86/samsung-laptop.c:1124 kbd_led_set() warn: impossible condition '(value < 0) => (0-255 < 0)'
vim +53 drivers/platform/chrome/cros_kbd_led_backlight.c
b14a971f104520 Amy Parker 2021-07-15 42 static led_brightness
^^^^^^^^^^^^^^
492ef7829d2d09 Simon Que 2016-03-08 43 keyboard_led_get_brightness(struct led_classdev *cdev)
492ef7829d2d09 Simon Que 2016-03-08 44 {
492ef7829d2d09 Simon Que 2016-03-08 45 unsigned long long brightness;
492ef7829d2d09 Simon Que 2016-03-08 46 acpi_status status;
492ef7829d2d09 Simon Que 2016-03-08 47
492ef7829d2d09 Simon Que 2016-03-08 48 status = acpi_evaluate_integer(NULL, ACPI_KEYBOARD_BACKLIGHT_READ,
492ef7829d2d09 Simon Que 2016-03-08 49 NULL, &brightness);
492ef7829d2d09 Simon Que 2016-03-08 50 if (ACPI_FAILURE(status)) {
492ef7829d2d09 Simon Que 2016-03-08 51 dev_err(cdev->dev, "Error getting keyboard LED value: %d\n",
492ef7829d2d09 Simon Que 2016-03-08 52 status);
492ef7829d2d09 Simon Que 2016-03-08 @53 return -EIO;
^^^^^^^^^^^^
To be honest, I'm confused why we are changing this from an enum to a
typedef. In the kernel we generally avoid typedefs where possible.
What's the problem that using a typedef fixes?
Also probably the name should end in a _t.
492ef7829d2d09 Simon Que 2016-03-08 54 }
492ef7829d2d09 Simon Que 2016-03-08 55
492ef7829d2d09 Simon Que 2016-03-08 56 return brightness;
492ef7829d2d09 Simon Que 2016-03-08 57 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
2021-07-19 7:16 ` Geert Uytterhoeven
@ 2021-07-19 8:40 ` Geert Uytterhoeven
2021-08-03 21:38 ` Pavel Machek
0 siblings, 1 reply; 14+ messages in thread
From: Geert Uytterhoeven @ 2021-07-19 8:40 UTC (permalink / raw)
To: Amy Parker
Cc: Pavel Machek, linux-leds, Linux Kernel Mailing List,
Dan Carpenter
On Mon, Jul 19, 2021 at 9:18 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, 15 Jul 2021, Amy Parker wrote:
> > This commit changes how led_brightness, declared in header file
> > include/linux/leds.h, works throughout the kernel, and updates other
> > files in accordance.
> >
> > The TODO located at drivers/leds/TODO requests:
> >
> > * Get rid of led_brightness
> >
> > It is really an integer, as maximum is configurable. Get rid of it, or
> > make it into typedef or something.
> >
> > This patch changes the declaration of led_brightness from an enum to a
> > typedef. In order to hold the currently existing enum values, macro
> > definitions are provided. Files which use led_brightness are updated to
> > conform to the new types.
> >
> > Signed-off-by: Amy Parker <apark0006@student.cerritos.edu>
>
> Thanks for your patch!
>
> > 207 files changed, 437 insertions(+), 438 deletions(-)
>
> This touches a lot of files, so we better get it right.
>
> > --- a/include/linux/leds.h
> > +++ b/include/linux/leds.h
> > @@ -26,12 +26,11 @@ struct device_node;
> > */
> >
> > /* This is obsolete/useless. We now support variable maximum brightness. */
> > -enum led_brightness {
> > - LED_OFF = 0,
> > - LED_ON = 1,
> > - LED_HALF = 127,
> > - LED_FULL = 255,
> > -};
> > +typedef u8 led_brightness;
>
> In general, typedefs are frowned upon in the kernel, but there can be a
> good reason to use one.
> What if the maximum brightness is larger than 255?
> Using "unsigned int" sounds better to me, but let's wait for Pavel...
And as Dan just pointed out, "signed int" would be even better, as it
would allow a function to return an error code.
> > +#define LED_OFF 0
> > +#define LED_ON 1
> > +#define LED_HALF 127
> > +#define LED_FULL 255
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
2021-07-16 21:43 ` Randy Dunlap
@ 2021-07-19 12:05 ` Pavel Machek
0 siblings, 0 replies; 14+ messages in thread
From: Pavel Machek @ 2021-07-19 12:05 UTC (permalink / raw)
To: Randy Dunlap
Cc: Amy Parker, kernel test robot, kbuild-all, linux-leds,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1694 bytes --]
Hi!
> >>> vim +18 include/media/v4l2-flash-led-class.h
> >>>
> >>> 14
> >>> 15 struct led_classdev_flash;
> >>> 16 struct led_classdev;
> >>> 17 struct v4l2_flash;
> >>> > 18 led_brightness;
> >>> 19
> >>>
> >>> ---
>
> >
> > Another patch was sent into the list to correct this error.
>
> Hopefully Pavel (LED subsystem maintainer) will comment soon-ish.
>
> My comments:
>
> a. This patch would be the right thing to do if your large patch had already been
> applied (merged) somewhere, but AFAIK it hasn't been. So:
>
> b. IMO you should resend your entire patch set with this fix included.
> Send it as "v2" (version 2) and explain the changes in it since your
> original patch (which was v1). This v2 explanation should be below the
> "---" line in the patch. (See Documentation/process/submitting-patches.rst
> for more info -- or ask for more info/help.)
I still remember the old patch, so b. is not strictly neccessary here.
> c. For your follow-up patch to include/media/v4l2-flash-led-class.h, which was:
>
> -led_brightness;
> +typedef u8 led_brightness;
>
> I would just add this to include/media/v4l2-flash-led-class.h:
>
> #include <linux/leds.h>
>
> That way, in a few years, when the type of led_brightness changes again,
> someone won't have to remember to search for other typedefs of it and
> update them also. Or maybe they will do that after a bug happens and
> someone notices it.
>
> (Note that I am just trying to help. Pavel has more of a final
> say-so about this.)
And your comments are reasonable.
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
` (4 preceding siblings ...)
2021-07-19 8:21 ` Dan Carpenter
@ 2021-07-19 12:08 ` Pavel Machek
5 siblings, 0 replies; 14+ messages in thread
From: Pavel Machek @ 2021-07-19 12:08 UTC (permalink / raw)
To: Amy Parker; +Cc: linux-leds, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]
Hi!
> This commit changes how led_brightness, declared in header file
> include/linux/leds.h, works throughout the kernel, and updates other
> files in accordance.
>
> The TODO located at drivers/leds/TODO requests:
>
> * Get rid of led_brightness
>
> It is really an integer, as maximum is configurable. Get rid of it, or
> make it into typedef or something.
>
> This patch changes the declaration of led_brightness from an enum to a
> typedef. In order to hold the currently existing enum values, macro
> definitions are provided. Files which use led_brightness are updated to
> conform to the new types.
Hmm... thanks.
But I can't really pull this through the LED tree as it touches other
people's code.
> index bfa60fa1d812..e1d771513b98 100644
> --- a/drivers/leds/TODO
> +++ b/drivers/leds/TODO
> @@ -1,7 +1,7 @@
> -*- org -*-
>
> * On/off LEDs should have max_brightness of 1
> -* Get rid of enum led_brightness
> +* Get rid of led_brightness
>
> It is really an integer, as maximum is configurable. Get rid of it, or
> make it into typedef or something.
You can delete this.
Probably new type should be called led_brightness_t. And probably it
should be typedef for the enum _for now_, so that we can switch users
gradually.
It should also be more than u8, I believe someone has more than 255
levels at this point.
> +++ b/include/linux/leds.h
> @@ -26,12 +26,11 @@ struct device_node;
> */
>
> /* This is obsolete/useless. We now support variable maximum brightness. */
> -enum led_brightness {
> - LED_OFF = 0,
> - LED_ON = 1,
> - LED_HALF = 127,
> - LED_FULL = 255,
> -};
> +typedef u8 led_brightness;
> +#define LED_OFF 0
> +#define LED_ON 1
> +#define LED_HALF 127
> +#define LED_FULL 255
>
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] swap led_brightness from enum to typedef
2021-07-19 8:40 ` Geert Uytterhoeven
@ 2021-08-03 21:38 ` Pavel Machek
0 siblings, 0 replies; 14+ messages in thread
From: Pavel Machek @ 2021-08-03 21:38 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Amy Parker, linux-leds, Linux Kernel Mailing List, Dan Carpenter
[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]
Hi!
> > > The TODO located at drivers/leds/TODO requests:
> > >
> > > * Get rid of led_brightness
> > >
> > > It is really an integer, as maximum is configurable. Get rid of it, or
> > > make it into typedef or something.
> > >
> > > This patch changes the declaration of led_brightness from an enum to a
> > > typedef. In order to hold the currently existing enum values, macro
> > > definitions are provided. Files which use led_brightness are updated to
> > > conform to the new types.
> > >
> > > Signed-off-by: Amy Parker <apark0006@student.cerritos.edu>
> >
> > Thanks for your patch!
> >
> > > 207 files changed, 437 insertions(+), 438 deletions(-)
> >
> > This touches a lot of files, so we better get it right.
> >
> > > --- a/include/linux/leds.h
> > > +++ b/include/linux/leds.h
> > > @@ -26,12 +26,11 @@ struct device_node;
> > > */
> > >
> > > /* This is obsolete/useless. We now support variable maximum brightness. */
> > > -enum led_brightness {
> > > - LED_OFF = 0,
> > > - LED_ON = 1,
> > > - LED_HALF = 127,
> > > - LED_FULL = 255,
> > > -};
> > > +typedef u8 led_brightness;
> >
> > In general, typedefs are frowned upon in the kernel, but there can be a
> > good reason to use one.
> > What if the maximum brightness is larger than 255?
> > Using "unsigned int" sounds better to me, but let's wait for Pavel...
>
> And as Dan just pointed out, "signed int" would be even better, as it
> would allow a function to return an error code.
We can not apply huge patch all at once, and changing from enum to int
directly will result in warnings in some configurations, no?
Agreed that int would be good.
Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-08-03 21:38 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-15 21:18 [PATCH 0/2] leds: change led_brightness definition from enum to typedef Amy Parker
2021-07-15 21:18 ` [PATCH 2/2] drivers/leds/TODO: update TODO to reflect changes Amy Parker
[not found] ` <9b5902665dcc4c0fca7546987303e348d8657f59.1626383424.git.apark0006@student.cerritos.edu>
2021-07-16 0:41 ` [PATCH 1/2] swap led_brightness from enum to typedef kernel test robot
2021-07-16 2:14 ` kernel test robot
2021-07-16 3:11 ` Amy Parker
2021-07-16 21:07 ` Amy Parker
2021-07-16 21:43 ` Randy Dunlap
2021-07-19 12:05 ` Pavel Machek
2021-07-19 7:16 ` Geert Uytterhoeven
2021-07-19 8:40 ` Geert Uytterhoeven
2021-08-03 21:38 ` Pavel Machek
2021-07-19 8:05 ` Dan Carpenter
2021-07-19 8:21 ` Dan Carpenter
2021-07-19 12:08 ` Pavel Machek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).