LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: William Breathitt Gray <vilhelm.gray@gmail.com>
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,
	Pavel Machek <pavel@ucw.cz>
Subject: Re: [PATCH v16 09/14] tools/counter: Create Counter tools
Date: Sun, 12 Sep 2021 17:26:35 +0100	[thread overview]
Message-ID: <20210912172635.14cc0923@jic23-huawei> (raw)
In-Reply-To: <7fb22281fde0874614a87b0a000b2bf27e17043e.1630031207.git.vilhelm.gray@gmail.com>

On Fri, 27 Aug 2021 12:47:53 +0900
William Breathitt Gray <vilhelm.gray@gmail.com> wrote:

> This creates an example Counter program under tools/counter/*
> to exemplify the Counter character device interface.
> 
> Cc: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>

It's great to have this example, but it very much an example rather than
a generic tool. You may want to revisit and provide a more generic
tool in the future.

A trivial comment about using a loop inline.

Jonathan

> ---
>  MAINTAINERS                     |  1 +
>  tools/Makefile                  | 13 ++---
>  tools/counter/Build             |  1 +
>  tools/counter/Makefile          | 53 +++++++++++++++++++
>  tools/counter/counter_example.c | 93 +++++++++++++++++++++++++++++++++
>  5 files changed, 155 insertions(+), 6 deletions(-)
>  create mode 100644 tools/counter/Build
>  create mode 100644 tools/counter/Makefile
>  create mode 100644 tools/counter/counter_example.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f2fdd2202605..57dc9b6ff82a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4782,6 +4782,7 @@ F:	Documentation/driver-api/generic-counter.rst
>  F:	drivers/counter/
>  F:	include/linux/counter.h
>  F:	include/uapi/linux/counter.h
> +F:	tools/counter/
>  
>  CP2615 I2C DRIVER
>  M:	Bence Csókás <bence98@sch.bme.hu>
> diff --git a/tools/Makefile b/tools/Makefile
> index 7e9d34ddd74c..5da1fde03a9a 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -12,6 +12,7 @@ help:
>  	@echo '  acpi                   - ACPI tools'
>  	@echo '  bpf                    - misc BPF tools'
>  	@echo '  cgroup                 - cgroup tools'
> +	@echo '  counter                - counter tools'
>  	@echo '  cpupower               - a tool for all things x86 CPU power'
>  	@echo '  debugging              - tools for debugging'
>  	@echo '  firewire               - the userspace part of nosy, an IEEE-1394 traffic sniffer'
> @@ -65,7 +66,7 @@ acpi: FORCE
>  cpupower: FORCE
>  	$(call descend,power/$@)
>  
> -cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging tracing: FORCE
> +cgroup counter firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging tracing: FORCE
>  	$(call descend,$@)
>  
>  bpf/%: FORCE
> @@ -100,7 +101,7 @@ freefall: FORCE
>  kvm_stat: FORCE
>  	$(call descend,kvm/$@)
>  
> -all: acpi cgroup cpupower gpio hv firewire liblockdep \
> +all: acpi cgroup counter cpupower gpio hv firewire liblockdep \
>  		perf selftests bootconfig spi turbostat usb \
>  		virtio vm bpf x86_energy_perf_policy \
>  		tmon freefall iio objtool kvm_stat wmi \
> @@ -112,7 +113,7 @@ acpi_install:
>  cpupower_install:
>  	$(call descend,power/$(@:_install=),install)
>  
> -cgroup_install firewire_install gpio_install hv_install iio_install perf_install bootconfig_install spi_install usb_install virtio_install vm_install bpf_install objtool_install wmi_install pci_install debugging_install tracing_install:
> +cgroup_install counter_install firewire_install gpio_install hv_install iio_install perf_install bootconfig_install spi_install usb_install virtio_install vm_install bpf_install objtool_install wmi_install pci_install debugging_install tracing_install:
>  	$(call descend,$(@:_install=),install)
>  
>  liblockdep_install:
> @@ -133,7 +134,7 @@ freefall_install:
>  kvm_stat_install:
>  	$(call descend,kvm/$(@:_install=),install)
>  
> -install: acpi_install cgroup_install cpupower_install gpio_install \
> +install: acpi_install cgroup_install counter_install cpupower_install gpio_install \
>  		hv_install firewire_install iio_install liblockdep_install \
>  		perf_install selftests_install turbostat_install usb_install \
>  		virtio_install vm_install bpf_install x86_energy_perf_policy_install \
> @@ -147,7 +148,7 @@ acpi_clean:
>  cpupower_clean:
>  	$(call descend,power/cpupower,clean)
>  
> -cgroup_clean hv_clean firewire_clean bootconfig_clean spi_clean usb_clean virtio_clean vm_clean wmi_clean bpf_clean iio_clean gpio_clean objtool_clean leds_clean pci_clean firmware_clean debugging_clean tracing_clean:
> +cgroup_clean counter_clean hv_clean firewire_clean bootconfig_clean spi_clean usb_clean virtio_clean vm_clean wmi_clean bpf_clean iio_clean gpio_clean objtool_clean leds_clean pci_clean firmware_clean debugging_clean tracing_clean:
>  	$(call descend,$(@:_clean=),clean)
>  
>  liblockdep_clean:
> @@ -181,7 +182,7 @@ freefall_clean:
>  build_clean:
>  	$(call descend,build,clean)
>  
> -clean: acpi_clean cgroup_clean cpupower_clean hv_clean firewire_clean \
> +clean: acpi_clean cgroup_clean counter_clean cpupower_clean hv_clean firewire_clean \

This is far from alphabetical, though I have no idea if there is any strong convention on where
to add new entries.

>  		perf_clean selftests_clean turbostat_clean bootconfig_clean spi_clean usb_clean virtio_clean \
>  		vm_clean bpf_clean iio_clean x86_energy_perf_policy_clean tmon_clean \
>  		freefall_clean build_clean libbpf_clean libsubcmd_clean liblockdep_clean \
> diff --git a/tools/counter/Build b/tools/counter/Build
> new file mode 100644
> index 000000000000..33f4a51d715e
> --- /dev/null
> +++ b/tools/counter/Build
> @@ -0,0 +1 @@
> +counter_example-y += counter_example.o
> diff --git a/tools/counter/Makefile b/tools/counter/Makefile
> new file mode 100644
> index 000000000000..5ebc195fd9c0
> --- /dev/null
> +++ b/tools/counter/Makefile
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: GPL-2.0
> +include ../scripts/Makefile.include
> +
> +bindir ?= /usr/bin
> +
> +ifeq ($(srctree),)
> +srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> +srctree := $(patsubst %/,%,$(dir $(srctree)))
> +endif
> +
> +# Do not use make's built-in rules
> +# (this improves performance and avoids hard-to-debug behaviour);
> +MAKEFLAGS += -r
> +
> +override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include
> +
> +ALL_TARGETS := counter_example
> +ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
> +
> +all: $(ALL_PROGRAMS)
> +
> +export srctree OUTPUT CC LD CFLAGS
> +include $(srctree)/tools/build/Makefile.include
> +
> +#
> +# We need the following to be outside of kernel tree
> +#
> +$(OUTPUT)include/linux/counter.h: ../../include/uapi/linux/counter.h
> +	mkdir -p $(OUTPUT)include/linux 2>&1 || true
> +	ln -sf $(CURDIR)/../../include/uapi/linux/counter.h $@
> +
> +prepare: $(OUTPUT)include/linux/counter.h
> +
> +COUNTER_EXAMPLE := $(OUTPUT)counter_example.o
> +$(COUNTER_EXAMPLE): prepare FORCE
> +	$(Q)$(MAKE) $(build)=counter_example
> +$(OUTPUT)counter_example: $(COUNTER_EXAMPLE)
> +	$(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
> +
> +clean:
> +	rm -f $(ALL_PROGRAMS)
> +	rm -rf $(OUTPUT)include/linux/counter.h
> +	find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.d' -delete
> +
> +install: $(ALL_PROGRAMS)
> +	install -d -m 755 $(DESTDIR)$(bindir);		\
> +	for program in $(ALL_PROGRAMS); do		\
> +		install $$program $(DESTDIR)$(bindir);	\
> +	done
> +
> +FORCE:
> +
> +.PHONY: all install clean FORCE prepare
> diff --git a/tools/counter/counter_example.c b/tools/counter/counter_example.c
> new file mode 100644
> index 000000000000..90d69fb9463b
> --- /dev/null
> +++ b/tools/counter/counter_example.c
> @@ -0,0 +1,93 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Counter - example userspace application
> + *
> + * The userspace application opens /dev/counter0, configures the
> + * COUNTER_EVENT_INDEX event channel 0 to gather Count 0 count and Count
> + * 1 count, and prints out the data as it becomes available on the
> + * character device node.
> + *
> + * Copyright (C) 2021 William Breathitt Gray
> + */
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <linux/counter.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <unistd.h>
> +
> +static struct counter_watch watches[2] = {
> +	{
> +		/* Component data: Count 0 count */
> +		.component.type = COUNTER_COMPONENT_COUNT,
> +		.component.scope = COUNTER_SCOPE_COUNT,
> +		.component.parent = 0,
> +		/* Event type: Index */
> +		.event = COUNTER_EVENT_INDEX,
> +		/* Device event channel 0 */
> +		.channel = 0,
> +	},
> +	{
> +		/* Component data: Count 1 count */
> +		.component.type = COUNTER_COMPONENT_COUNT,
> +		.component.scope = COUNTER_SCOPE_COUNT,
> +		.component.parent = 1,
> +		/* Event type: Index */
> +		.event = COUNTER_EVENT_INDEX,
> +		/* Device event channel 0 */
> +		.channel = 0,
> +	},
> +};
> +
> +int main(void)
> +{
> +	int fd;
> +	int ret;
> +	struct counter_event event_data[2];
> +
> +	fd = open("/dev/counter0", O_RDWR);

You may want to make this tool more flexible at somepoint. It is
in tools rather than Documentation after all.

> +	if (fd == -1) {
> +		perror("Unable to open /dev/counter0");
> +		return -errno;
> +	}
> +
> +	ret = ioctl(fd, COUNTER_ADD_WATCH_IOCTL, watches);

Loop?

> +	if (ret == -1) {
> +		perror("Error adding watches[0]");
> +		return -errno;
> +	}
> +	ret = ioctl(fd, COUNTER_ADD_WATCH_IOCTL, watches + 1);
> +	if (ret == -1) {
> +		perror("Error adding watches[1]");
> +		return -errno;
> +	}
> +	ret = ioctl(fd, COUNTER_ENABLE_EVENTS_IOCTL);
> +	if (ret == -1) {
> +		perror("Error enabling events");
> +		return -errno;
> +	}
> +
> +	for (;;) {
> +		ret = read(fd, event_data, sizeof(event_data));
> +		if (ret == -1) {
> +			perror("Failed to read event data");
> +			return 1;
> +		}
> +
> +		if (ret != sizeof(event_data)) {
> +			fprintf(stderr, "Failed to read event data\n");
> +			return -EIO;
> +		}
> +
> +		printf("Timestamp 0: %llu\tCount 0: %llu\n"
> +		       "Error Message 0: %s\n"
> +		       "Timestamp 1: %llu\tCount 1: %llu\n"
> +		       "Error Message 1: %s\n",
> +		       event_data[0].timestamp, event_data[0].value,
> +		       strerror(event_data[0].status),
> +		       event_data[1].timestamp, event_data[1].value,
> +		       strerror(event_data[1].status));
> +	}
> +
> +	return 0;
> +}


  reply	other threads:[~2021-09-12 16:23 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-27  3:47 [PATCH v16 00/14] Introduce the Counter character device interface William Breathitt Gray
2021-08-27  3:47 ` [PATCH v16 01/14] counter: stm32-lptimer-cnt: Provide defines for clock polarities William Breathitt Gray
2021-08-31 13:38   ` Fabrice Gasnier
2021-09-08 17:31     ` Jonathan Cameron
2021-08-27  3:47 ` [PATCH v16 02/14] counter: stm32-timer-cnt: Provide defines for slave mode selection William Breathitt Gray
2021-08-31 13:40   ` Fabrice Gasnier
2021-09-08 17:31     ` Jonathan Cameron
2021-08-27  3:47 ` [PATCH v16 03/14] counter: Internalize sysfs interface code William Breathitt Gray
2021-08-31 13:44   ` [Linux-stm32] " Fabrice Gasnier
2021-08-31 14:16     ` William Breathitt Gray
2021-08-31 14:55       ` William Breathitt Gray
2021-09-08 17:44     ` Jonathan Cameron
2021-08-27  3:47 ` [PATCH v16 04/14] counter: Update counter.h comments to reflect sysfs internalization William Breathitt Gray
2021-09-08 17:46   ` Jonathan Cameron
2021-08-27  3:47 ` [PATCH v16 05/14] docs: counter: Update " William Breathitt Gray
2021-09-08 17:48   ` Jonathan Cameron
2021-08-27  3:47 ` [PATCH v16 06/14] counter: Move counter enums to uapi header William Breathitt Gray
2021-08-27  3:47 ` [PATCH v16 07/14] counter: Add character device interface William Breathitt Gray
2021-09-12 16:18   ` Jonathan Cameron
2021-09-20 10:09     ` William Breathitt Gray
2021-09-26 15:15       ` Jonathan Cameron
2021-09-27 10:21         ` William Breathitt Gray
2021-09-27 11:20           ` Jonathan Cameron
2021-09-27 11:33             ` William Breathitt Gray
2021-08-27  3:47 ` [PATCH v16 08/14] docs: counter: Document " William Breathitt Gray
2021-09-12 16:18   ` Jonathan Cameron
2021-08-27  3:47 ` [PATCH v16 09/14] tools/counter: Create Counter tools William Breathitt Gray
2021-09-12 16:26   ` Jonathan Cameron [this message]
2021-08-27  3:47 ` [PATCH v16 10/14] counter: Implement signalZ_action_component_id sysfs attribute William Breathitt Gray
2021-08-27  3:47 ` [PATCH v16 11/14] counter: Implement *_component_id sysfs attributes William Breathitt Gray
2021-08-27  3:47 ` [PATCH v16 12/14] counter: Implement events_queue_size sysfs attribute William Breathitt Gray
2021-08-27  3:47 ` [PATCH v16 13/14] counter: 104-quad-8: Replace mutex with spinlock William Breathitt Gray
2021-08-27  3:47 ` [PATCH v16 14/14] counter: 104-quad-8: Add IRQ support for the ACCES 104-QUAD-8 William Breathitt Gray
2021-08-30 17:17 ` [PATCH v16 00/14] Introduce the Counter character device interface Jonathan Cameron
2021-09-12 16:36   ` Jonathan Cameron

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=20210912172635.14cc0923@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=a.fatoum@pengutronix.de \
    --cc=alexandre.belloni@bootlin.com \
    --cc=alexandre.torgue@st.com \
    --cc=david@lechnology.com \
    --cc=fabrice.gasnier@st.com \
    --cc=gwendal@chromium.org \
    --cc=jarkko.nikula@linux.intel.com \
    --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=pavel@ucw.cz \
    --cc=syednwaris@gmail.com \
    --cc=vilhelm.gray@gmail.com \
    --subject='Re: [PATCH v16 09/14] tools/counter: Create Counter tools' \
    /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).