On Sun, Aug 08, 2021 at 06:23:25PM +0100, Jonathan Cameron wrote: > On Tue, 3 Aug 2021 21:06:16 +0900 > William Breathitt Gray wrote: > > > This is a reimplementation of the Generic Counter driver interface. > > There are no modifications to the Counter subsystem userspace interface, > > so existing userspace applications should continue to run seamlessly. > > > > The purpose of this patch is to internalize the sysfs interface code > > among the various counter drivers into a shared module. Counter drivers > > pass and take data natively (i.e. u8, u64, etc.) and the shared counter > > module handles the translation between the sysfs interface and the > > device drivers. This guarantees a standard userspace interface for all > > counter drivers, and helps generalize the Generic Counter driver ABI in > > order to support the Generic Counter chrdev interface (introduced in a > > subsequent patch) without significant changes to the existing counter > > drivers. > > > > Note, Counter device registration is the same as before: drivers > > populate a struct counter_device with components and callbacks, then > > pass the structure to the devm_counter_register function. However, > > what's different now is how the Counter subsystem code handles this > > registration internally. > > > > Whereas before callbacks would interact directly with sysfs data, this > > interaction is now abstracted and instead callbacks interact with native > > C data types. The counter_comp structure forms the basis for Counter > > extensions. > > > > The counter-sysfs.c file contains the code to parse through the > > counter_device structure and register the requested components and > > extensions. Attributes are created and populated based on type, with > > respective translation functions to handle the mapping between sysfs and > > the counter driver callbacks. > > > > The translation performed for each attribute is straightforward: the > > attribute type and data is parsed from the counter_attribute structure, > > the respective counter driver read/write callback is called, and sysfs > > I/O is handled before or after the driver read/write function is called. > > Hi William, > > There are some issues with this even after I've fixed the merge conflict > because of the earlier change.. > > DESCEND objtool > CALL scripts/atomic/check-atomics.sh > CALL scripts/checksyscalls.sh > CHK include/generated/compile.h > CC [M] drivers/counter/counter-core.o > CC [M] drivers/counter/counter-sysfs.o > CC [M] drivers/counter/104-quad-8.o > CC [M] drivers/counter/interrupt-cnt.o > CC [M] drivers/counter/stm32-timer-cnt.o > CC [M] drivers/counter/stm32-lptimer-cnt.o > drivers/counter/counter-core.c: In function ‘counter_device_release’: > drivers/counter/counter-core.c:23:9: error: implicit declaration of function ‘counter_chrdev_remove’ [-Werror=implicit-function-declaration] > 23 | counter_chrdev_remove(counter); > | ^~~~~~~~~~~~~~~~~~~~~ > cc1: some warnings being treated as errors > make[2]: *** [scripts/Makefile.build:271: drivers/counter/counter-core.o] Error 1 > make[2]: *** Waiting for unfinished jobs.... > CHECK drivers/counter/interrupt-cnt.c > CHECK drivers/counter/104-quad-8.c > CHECK drivers/counter/counter-sysfs.c > drivers/counter/stm32-timer-cnt.c:55:9: warning: implicit conversion from ‘enum stm32_count_function’ to ‘enum counter_function’ [-Wenum-conversion] > 55 | STM32_COUNT_SLAVE_MODE_DISABLED, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/counter/stm32-timer-cnt.c:56:9: warning: implicit conversion from ‘enum stm32_count_function’ to ‘enum counter_function’ [-Wenum-conversion] > 56 | STM32_COUNT_ENCODER_MODE_1, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/counter/stm32-timer-cnt.c:57:9: warning: implicit conversion from ‘enum stm32_count_function’ to ‘enum counter_function’ [-Wenum-conversion] > 57 | STM32_COUNT_ENCODER_MODE_2, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/counter/stm32-timer-cnt.c:58:9: warning: implicit conversion from ‘enum stm32_count_function’ to ‘enum counter_function’ [-Wenum-conversion] > 58 | STM32_COUNT_ENCODER_MODE_3, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/counter/stm32-timer-cnt.c: In function ‘stm32_count_function_read’: > drivers/counter/stm32-timer-cnt.c:97:27: warning: implicit conversion from ‘enum stm32_count_function’ to ‘enum counter_function’ [-Wenum-conversion] > 97 | *function = STM32_COUNT_SLAVE_MODE_DISABLED; > | ^ > drivers/counter/stm32-timer-cnt.c:100:27: warning: implicit conversion from ‘enum stm32_count_function’ to ‘enum counter_function’ [-Wenum-conversion] > 100 | *function = STM32_COUNT_ENCODER_MODE_1; > | ^ > drivers/counter/stm32-timer-cnt.c:103:27: warning: implicit conversion from ‘enum stm32_count_function’ to ‘enum counter_function’ [-Wenum-conversion] > 103 | *function = STM32_COUNT_ENCODER_MODE_2; > | ^ > drivers/counter/stm32-timer-cnt.c:106:27: warning: implicit conversion from ‘enum stm32_count_function’ to ‘enum counter_function’ [-Wenum-conversion] > 106 | *function = STM32_COUNT_ENCODER_MODE_3; > | ^ > drivers/counter/stm32-lptimer-cnt.c:139:9: warning: implicit conversion from ‘enum stm32_lptim_cnt_function’ to ‘enum counter_function’ [-Wenum-conversion] > 139 | STM32_LPTIM_COUNTER_INCREASE, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/counter/stm32-lptimer-cnt.c:140:9: warning: implicit conversion from ‘enum stm32_lptim_cnt_function’ to ‘enum counter_function’ [-Wenum-conversion] > 140 | STM32_LPTIM_ENCODER_BOTH_EDGE, > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/counter/stm32-lptimer-cnt.c: In function ‘stm32_lptim_cnt_function_read’: > drivers/counter/stm32-lptimer-cnt.c:195:27: warning: implicit conversion from ‘enum stm32_lptim_cnt_function’ to ‘enum counter_function’ [-Wenum-conversion] > 195 | *function = STM32_LPTIM_COUNTER_INCREASE; > | ^ > drivers/counter/stm32-lptimer-cnt.c:200:27: warning: implicit conversion from ‘enum stm32_lptim_cnt_function’ to ‘enum counter_function’ [-Wenum-conversion] > 200 | *function = STM32_LPTIM_ENCODER_BOTH_EDGE; > | ^ > drivers/counter/104-quad-8.c:58: warning: Function parameter or member 'lock' not described in 'quad8' > CHECK drivers/counter/stm32-timer-cnt.c > CHECK drivers/counter/stm32-lptimer-cnt.c > make[1]: *** [scripts/Makefile.build:514: drivers/counter] Error 2 > make: *** [Makefile:1841: drivers] Error 2 > > I could wait through these but it feels a little to likely I'll mess something up fixing things > so please fix the errors and warnings this patch introduces > with a W=1 C=1 build. > > Also, do a patch by patch build of the rest of the set in case there are any other cases > like this hiding. It's very easy for these to build up when you have a lot of > versions of a patch series. I've been caught out a few times myself! > > In the meantime, I'll keep the first 5 patches on my tree unless you shout otherwise. > > Thanks, > > Jonathan Thanks, I'll fix this up and submit a v15 to make sure these patches merge in smoothly. William Breathitt Gray