LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: William Breathitt Gray <vilhelm.gray@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: linux-stm32@st-md-mailman.stormreply.com, kernel@pengutronix.de,
	a.fatoum@pengutronix.de, kamel.bouhara@bootlin.com,
	gwendal@chromium.org, alexandre.belloni@bootlin.com,
	david@lechnology.com, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, syednwaris@gmail.com,
	patrick.havelange@essensium.com, fabrice.gasnier@st.com,
	mcoquelin.stm32@gmail.com, alexandre.torgue@st.com,
	o.rempel@pengutronix.de, jarkko.nikula@linux.intel.com,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH v14 06/17] counter: Internalize sysfs interface code
Date: Mon, 9 Aug 2021 12:59:51 +0900	[thread overview]
Message-ID: <YRCoN5YZhytejzFh@shinobu> (raw)
In-Reply-To: <20210808182325.2620de00@jic23-huawei>

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

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 <vilhelm.gray@gmail.com> 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

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

  reply	other threads:[~2021-08-09  4:00 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-03 12:06 [PATCH v14 00/17] Introduce the Counter character device interface William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 01/17] counter: 104-quad-8: Return error when invalid mode during ceiling_write William Breathitt Gray
2021-08-08 17:06   ` Jonathan Cameron
2021-08-03 12:06 ` [PATCH v14 02/17] counter: Return error code on invalid modes William Breathitt Gray
2021-08-08 17:07   ` Jonathan Cameron
2021-08-03 12:06 ` [PATCH v14 03/17] counter: Standardize to ERANGE for limit exceeded errors William Breathitt Gray
2021-08-08 17:08   ` Jonathan Cameron
2021-08-03 12:06 ` [PATCH v14 04/17] counter: Rename counter_signal_value to counter_signal_level William Breathitt Gray
2021-08-08 17:10   ` Jonathan Cameron
2021-08-03 12:06 ` [PATCH v14 05/17] counter: Rename counter_count_function to counter_function William Breathitt Gray
2021-08-08 17:15   ` Jonathan Cameron
2021-08-03 12:06 ` [PATCH v14 06/17] counter: Internalize sysfs interface code William Breathitt Gray
2021-08-08 17:23   ` Jonathan Cameron
2021-08-09  3:59     ` William Breathitt Gray [this message]
2021-08-03 12:06 ` [PATCH v14 07/17] counter: Update counter.h comments to reflect sysfs internalization William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 08/17] docs: counter: Update " William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 09/17] counter: Move counter enums to uapi header William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 10/17] counter: Add character device interface William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 11/17] docs: counter: Document " William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 12/17] tools/counter: Create Counter tools William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 13/17] counter: Implement signalZ_action_component_id sysfs attribute William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 14/17] counter: Implement *_component_id sysfs attributes William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 15/17] counter: Implement events_queue_size sysfs attribute William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 16/17] counter: 104-quad-8: Replace mutex with spinlock William Breathitt Gray
2021-08-03 12:06 ` [PATCH v14 17/17] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8 William Breathitt Gray

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YRCoN5YZhytejzFh@shinobu \
    --to=vilhelm.gray@gmail.com \
    --cc=a.fatoum@pengutronix.de \
    --cc=alexandre.belloni@bootlin.com \
    --cc=alexandre.torgue@st.com \
    --cc=dan.carpenter@oracle.com \
    --cc=david@lechnology.com \
    --cc=fabrice.gasnier@st.com \
    --cc=gwendal@chromium.org \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=jic23@kernel.org \
    --cc=kamel.bouhara@bootlin.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=o.rempel@pengutronix.de \
    --cc=patrick.havelange@essensium.com \
    --cc=syednwaris@gmail.com \
    --subject='Re: [PATCH v14 06/17] counter: Internalize sysfs interface code' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).