Re: [PATCH v4] Drivers: hv: vmbus: Expose monitor data only when monitor pages are used
On Fri, Mar 01, 2019 at 02:18:24PM -0500, Kimberly Brown wrote:
> +/*
> + * Channel-level attribute_group callback function. Returns the permission
> for
> + * each attribute, and returns 0 if an attribute is not visible.
> + */
> +static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int idx)
> +{
> + const struct vmbus_channel *channel =
> + container_of(kobj, struct vmbus_channel, kobj);
> +
> + /* Hide the monitor attributes if the monitor mechanism is not used. */
> + if (!channel->offermsg.monitor_allocated &&
> + (attr == &chan_attr_pending.attr ||
> + attr == &chan_attr_latency.attr ||
> + attr == &chan_attr_monitor_id.attr))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static struct attribute_group vmbus_chan_group = {
> + .attrs = vmbus_chan_attrs,
> + .is_visible = vmbus_chan_attr_is_visible
> +};
> +
> static struct kobj_type vmbus_chan_ktype = {
> .sysfs_ops = &vmbus_chan_sysfs_ops,
> .release = vmbus_chan_release,
> - .default_attrs = vmbus_chan_attrs,
Why did you remove this line?
> };
>
> /*
> @@ -1571,6 +1624,12 @@ int vmbus_add_channel_kobj(struct hv_device *dev,
> struct vmbus_channel *channel)
> if (ret)
> return ret;
>
> + ret = sysfs_create_group(kobj, &vmbus_chan_group);
Why are you adding these "by hand"? What was wrong with using the
default attribute group pointer? You also are not removing the
attributes :(
greg k-h
RE: [PATCH 11/12] percpu: convert chunk hints to be based on pcpu_block_md
> -Original Message-
> From: owner-linux...@kvack.org [mailto:owner-linux...@kvack.org] On
> Behalf Of Dennis Zhou
> Sent: 2019年2月28日 10:19
> To: Dennis Zhou ; Tejun Heo ; Christoph
> Lameter
> Cc: Vlad Buslov ; kernel-t...@fb.com;
> linux...@kvack.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 11/12] percpu: convert chunk hints to be based on
> pcpu_block_md
>
> As mentioned in the last patch, a chunk's hints are no different than a block
> just responsible for more bits. This converts chunk level hints to use a
> pcpu_block_md to maintain them. This lets us reuse the same hint helper
> functions as a block. The left_free and right_free are unused by the chunk's
> pcpu_block_md.
>
> Signed-off-by: Dennis Zhou
> ---
> mm/percpu-internal.h | 5 +-
> mm/percpu-stats.c| 5 +-
> mm/percpu.c | 120 +++
> 3 files changed, 57 insertions(+), 73 deletions(-)
>
> diff --git a/mm/percpu-internal.h b/mm/percpu-internal.h index
> 119bd1119aa7..0468ba500bd4 100644
> --- a/mm/percpu-internal.h
> +++ b/mm/percpu-internal.h
> @@ -39,9 +39,7 @@ struct pcpu_chunk {
>
> struct list_headlist; /* linked to pcpu_slot lists */
> int free_bytes; /* free bytes in the chunk */
> - int contig_bits;/* max contiguous size hint */
> - int contig_bits_start; /* contig_bits starting
> - offset */
> + struct pcpu_block_mdchunk_md;
> void*base_addr; /* base address of this chunk */
>
> unsigned long *alloc_map; /* allocation map */
> @@ -49,7 +47,6 @@ struct pcpu_chunk {
> struct pcpu_block_md*md_blocks; /* metadata blocks */
>
> void*data; /* chunk data */
> - int first_bit; /* no free below this */
> boolimmutable; /* no [de]population allowed */
> int start_offset; /* the overlap with the previous
> region to have a page aligned
> diff --git a/mm/percpu-stats.c b/mm/percpu-stats.c index
> b5fdd43b60c9..ef5034a0464e 100644
> --- a/mm/percpu-stats.c
> +++ b/mm/percpu-stats.c
> @@ -53,6 +53,7 @@ static int find_max_nr_alloc(void) static void
> chunk_map_stats(struct seq_file *m, struct pcpu_chunk *chunk,
> int *buffer)
> {
> + struct pcpu_block_md *chunk_md = &chunk->chunk_md;
> int i, last_alloc, as_len, start, end;
> int *alloc_sizes, *p;
> /* statistics */
> @@ -121,9 +122,9 @@ static void chunk_map_stats(struct seq_file *m,
> struct pcpu_chunk *chunk,
> P("nr_alloc", chunk->nr_alloc);
> P("max_alloc_size", chunk->max_alloc_size);
> P("empty_pop_pages", chunk->nr_empty_pop_pages);
> - P("first_bit", chunk->first_bit);
> + P("first_bit", chunk_md->first_free);
> P("free_bytes", chunk->free_bytes);
> - P("contig_bytes", chunk->contig_bits * PCPU_MIN_ALLOC_SIZE);
> + P("contig_bytes", chunk_md->contig_hint * PCPU_MIN_ALLOC_SIZE);
> P("sum_frag", sum_frag);
> P("max_frag", max_frag);
> P("cur_min_alloc", cur_min_alloc);
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 7cdf14c242de..197479f2c489 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -233,10 +233,13 @@ static int pcpu_size_to_slot(int size)
>
> static int pcpu_chunk_slot(const struct pcpu_chunk *chunk) {
> - if (chunk->free_bytes < PCPU_MIN_ALLOC_SIZE || chunk->contig_bits
> == 0)
> + const struct pcpu_block_md *chunk_md = &chunk->chunk_md;
> +
> + if (chunk->free_bytes < PCPU_MIN_ALLOC_SIZE ||
> + chunk_md->contig_hint == 0)
> return 0;
>
> - return pcpu_size_to_slot(chunk->contig_bits * PCPU_MIN_ALLOC_SIZE);
> + return pcpu_size_to_slot(chunk_md->contig_hint *
> PCPU_MIN_ALLOC_SIZE);
> }
>
> /* set the pointer to a chunk in a page struct */ @@ -592,54 +595,6 @@
> static inline bool pcpu_region_overlap(int a, int b, int x, int y)
> return false;
> }
>
> -/**
> - * pcpu_chunk_update - updates the chunk metadata given a free area
> - * @chunk: chunk of interest
> - * @bit_off: chunk offset
> - * @bits: size of free area
> - *
> - * This updates the chunk's contig hint and starting offset given a free
> area.
> - * Choose the best starting offset if the contig hint is equal.
> - */
> -static void pcpu_chunk_update(struct pcpu_chunk *chunk, int bit_off, int
> bits)
> -{
> - if (bits > chunk->contig_bits) {
> - chunk->contig_bits_start = bit_off;
> - chunk->contig_bits = bits;
> - } else if (bits == chunk->contig_bits && chunk->contig_bits_start &&
> -(!bit_off ||
> - __ffs(bit_off) > __ffs(chunk->contig_bits_start))) {
> - /* use the start with the best alignment */
> -
[PATCH] ext4: do prefetchw while the page pointer has been updated
When pages is not NULL, prefetchw(&page->flags) always works on
the last consumed page. This might do little improvment for handling
current page. It is better to do prefetchw while the page pointer
has just been updated.
Signed-off-by: Liu Xiang
---
fs/ext4/readpage.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index 6aa282e..0b68dbe 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -126,14 +126,17 @@ int ext4_mpage_readpages(struct address_space *mapping,
int fully_mapped = 1;
unsigned first_hole = blocks_per_page;
- prefetchw(&page->flags);
if (pages) {
page = lru_to_page(pages);
+
+ prefetchw(&page->flags);
list_del(&page->lru);
if (add_to_page_cache_lru(page, mapping, page->index,
readahead_gfp_mask(mapping)))
goto next_page;
- }
+ } else
+ prefetchw(&page->flags);
+
if (page_has_buffers(page))
goto confused;
--
1.9.1
RE: [PATCH 12/12] percpu: use chunk scan_hint to skip some scanning
> -Original Message-
> From: owner-linux...@kvack.org [mailto:owner-linux...@kvack.org] On
> Behalf Of Dennis Zhou
> Sent: 2019年2月28日 10:19
> To: Dennis Zhou ; Tejun Heo ; Christoph
> Lameter
> Cc: Vlad Buslov ; kernel-t...@fb.com;
> linux...@kvack.org; linux-kernel@vger.kernel.org
> Subject: [PATCH 12/12] percpu: use chunk scan_hint to skip some scanning
>
> Just like blocks, chunks now maintain a scan_hint. This can be used to skip
> some scanning by promoting the scan_hint to be the contig_hint.
> The chunk's scan_hint is primarily updated on the backside and relies on full
> scanning when a block becomes free or the free region spans across blocks.
>
> Signed-off-by: Dennis Zhou
> ---
> mm/percpu.c | 36 +++-
> 1 file changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/mm/percpu.c b/mm/percpu.c
> index 197479f2c489..40d49d7fb286 100644
> --- a/mm/percpu.c
> +++ b/mm/percpu.c
> @@ -711,20 +711,31 @@ static void pcpu_block_update_scan(struct
> pcpu_chunk *chunk, int bit_off,
> /**
> * pcpu_chunk_refresh_hint - updates metadata about a chunk
> * @chunk: chunk of interest
> + * @full_scan: if we should scan from the beginning
> *
> * Iterates over the metadata blocks to find the largest contig area.
> - * It also counts the populated pages and uses the delta to update the
> - * global count.
> + * A full scan can be avoided on the allocation path as this is
> + triggered
> + * if we broke the contig_hint. In doing so, the scan_hint will be
> + before
> + * the contig_hint or after if the scan_hint == contig_hint. This
> + cannot
> + * be prevented on freeing as we want to find the largest area possibly
> + * spanning blocks.
> */
> -static void pcpu_chunk_refresh_hint(struct pcpu_chunk *chunk)
> +static void pcpu_chunk_refresh_hint(struct pcpu_chunk *chunk, bool
> +full_scan)
> {
> struct pcpu_block_md *chunk_md = &chunk->chunk_md;
> int bit_off, bits;
>
> - /* clear metadata */
> - chunk_md->contig_hint = 0;
> + /* promote scan_hint to contig_hint */
> + if (!full_scan && chunk_md->scan_hint) {
> + bit_off = chunk_md->scan_hint_start + chunk_md->scan_hint;
> + chunk_md->contig_hint_start = chunk_md->scan_hint_start;
> + chunk_md->contig_hint = chunk_md->scan_hint;
> + chunk_md->scan_hint = 0;
> + } else {
> + bit_off = chunk_md->first_free;
> + chunk_md->contig_hint = 0;
> + }
>
> - bit_off = chunk_md->first_free;
> bits = 0;
> pcpu_for_each_md_free_region(chunk, bit_off, bits) {
> pcpu_block_update(chunk_md, bit_off, bit_off + bits); @@ -884,6
> +895,13 @@ static void pcpu_block_update_hint_alloc(struct pcpu_chunk
> *chunk, int bit_off,
> if (nr_empty_pages)
> pcpu_update_empty_pages(chunk, -1 * nr_empty_pages);
>
> + if (pcpu_region_overlap(chunk_md->scan_hint_start,
> + chunk_md->scan_hint_start +
> + chunk_md->scan_hint,
> + bit_off,
> + bit_off + bits))
> + chunk_md->scan_hint = 0;
> +
> /*
>* The only time a full chunk scan is required is if the chunk
>* contig hint is broken. Otherwise, it means a smaller space @@
> -894,7 +912,7 @@ static void pcpu_block_update_hint_alloc(struct
> pcpu_chunk *chunk, int bit_off,
> chunk_md->contig_hint,
> bit_off,
> bit_off + bits))
> - pcpu_chunk_refresh_hint(chunk);
> + pcpu_chunk_refresh_hint(chunk, false);
> }
>
> /**
> @@ -1005,7 +1023,7 @@ static void pcpu_block_update_hint_free(struct
> pcpu_chunk *chunk, int bit_off,
>* the else condition below.
>*/
> if (((end - start) >= PCPU_BITMAP_BLOCK_BITS) || s_index != e_index)
> - pcpu_chunk_refresh_hint(chunk);
> + pcpu_chunk_refresh_hint(chunk, true);
> else
> pcpu_block_update(&chunk->chunk_md,
> pcpu_block_off_to_off(s_index, start), @@
> -1078,7
> +1096,7 @@ static int pcpu_find_block_fit(struct pcpu_chunk *chunk, int
> alloc_bits,
> if (bit_off + alloc_bits > chunk_md->contig_hint)
> return -1;
>
> - bit_off = chunk_md->first_free;
> + bit_off = pcpu_next_hint(chunk_md, alloc_bits);
> bits = 0;
> pcpu_for_each_fit_region(chunk, alloc_bits, align, bit_off, bits) {
> if (!pop_only || pcpu_is_populated(chunk, bit_off, bits,
Reviewed-by: Peng Fan
> --
> 2.17.1
RE: [PATCH 02/12] percpu: do not search past bitmap when allocating an area
> -Original Message- > From: Dennis Zhou [mailto:den...@kernel.org] > Sent: 2019年3月3日 6:24 > To: Peng Fan > Cc: Tejun Heo ; Christoph Lameter ; Vlad > Buslov ; kernel-t...@fb.com; linux...@kvack.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH 02/12] percpu: do not search past bitmap when allocating > an area > > On Sat, Mar 02, 2019 at 01:32:04PM +, Peng Fan wrote: > > Hi Dennis, > > > > > -Original Message- > > > From: owner-linux...@kvack.org [mailto:owner-linux...@kvack.org] > On > > > Behalf Of Dennis Zhou > > > Sent: 2019年2月28日 10:18 > > > To: Dennis Zhou ; Tejun Heo ; > > > Christoph Lameter > > > Cc: Vlad Buslov ; kernel-t...@fb.com; > > > linux...@kvack.org; linux-kernel@vger.kernel.org > > > Subject: [PATCH 02/12] percpu: do not search past bitmap when > > > allocating an area > > > > > > pcpu_find_block_fit() guarantees that a fit is found within > > > PCPU_BITMAP_BLOCK_BITS. Iteration is used to determine the first fit > > > as it compares against the block's contig_hint. This can lead to > > > incorrectly scanning past the end of the bitmap. The behavior was > > > okay given the check after for bit_off >= end and the correctness of the > hints from pcpu_find_block_fit(). > > > > > > This patch fixes this by bounding the end offset by the number of > > > bits in a chunk. > > > > > > Signed-off-by: Dennis Zhou > > > --- > > > mm/percpu.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/mm/percpu.c b/mm/percpu.c index > > > 53bd79a617b1..69ca51d238b5 100644 > > > --- a/mm/percpu.c > > > +++ b/mm/percpu.c > > > @@ -988,7 +988,8 @@ static int pcpu_alloc_area(struct pcpu_chunk > > > *chunk, int alloc_bits, > > > /* > > >* Search to find a fit. > > >*/ > > > - end = start + alloc_bits + PCPU_BITMAP_BLOCK_BITS; > > > + end = min_t(int, start + alloc_bits + PCPU_BITMAP_BLOCK_BITS, > > > + pcpu_chunk_map_bits(chunk)); > > > bit_off = bitmap_find_next_zero_area(chunk->alloc_map, end, > start, > > >alloc_bits, align_mask); > > > if (bit_off >= end) > > > -- > > > > From pcpu_alloc_area itself, I think this is correct to avoid > > bitmap_find_next_zero_area scan past the boundaries of alloc_map, so > > > > Reviewed-by: Peng Fan > > > > There are a few points I did not understand well, Per understanding > > pcpu_find_block_fit is to find the first bit off in a chunk which > > could satisfy the bits allocation, so bits might be larger than > > PCPU_BITMAP_BLOCK_BITS. And if pcpu_find_block_fit returns a good off, > > it means there is a area in the chunk could satisfy the bits > > allocation, then the following pcpu_alloc_area will not scan past the > boundaries of alloc_map, right? > > > > pcpu_find_block_fit() finds the chunk offset corresponding to the block that > will be able to fit the chunk. Allocations are done by first fit, so scanning > begins > from the first_free of a block. Because the hints are always accurate, you > never fail to find a fit in pcpu_alloc_area() if > pcpu_find_block_fit() gives you an offset. This means you never scan past the > end anyway. Thanks for explanation. Thanks, Peng. > > Thanks, > Dennis
RE: [PATCH 04/12] percpu: manage chunks based on contig_bits instead of free_bytes
> -Original Message- > From: Dennis Zhou [mailto:den...@kernel.org] > Sent: 2019年3月3日 6:32 > To: Peng Fan > Cc: Tejun Heo ; Christoph Lameter ; Vlad > Buslov ; kernel-t...@fb.com; linux...@kvack.org; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH 04/12] percpu: manage chunks based on contig_bits > instead of free_bytes > > On Sat, Mar 02, 2019 at 01:48:20PM +, Peng Fan wrote: > > > > > > > -Original Message- > > > From: owner-linux...@kvack.org [mailto:owner-linux...@kvack.org] > On > > > Behalf Of Dennis Zhou > > > Sent: 2019年2月28日 10:19 > > > To: Dennis Zhou ; Tejun Heo ; > > > Christoph Lameter > > > Cc: Vlad Buslov ; kernel-t...@fb.com; > > > linux...@kvack.org; linux-kernel@vger.kernel.org > > > Subject: [PATCH 04/12] percpu: manage chunks based on contig_bits > > > instead of free_bytes > > > > > > When a chunk becomes fragmented, it can end up having a large number > > > of small allocation areas free. The free_bytes sorting of chunks > > > leads to unnecessary checking of chunks that cannot satisfy the > > > allocation. > > > Switch to contig_bits sorting to prevent scanning chunks that may > > > not be able to service the allocation request. > > > > > > Signed-off-by: Dennis Zhou > > > --- > > > mm/percpu.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/mm/percpu.c b/mm/percpu.c index > > > b40112b2fc59..c996bcffbb2a 100644 > > > --- a/mm/percpu.c > > > +++ b/mm/percpu.c > > > @@ -234,7 +234,7 @@ static int pcpu_chunk_slot(const struct > > > pcpu_chunk > > > *chunk) > > > if (chunk->free_bytes < PCPU_MIN_ALLOC_SIZE || > chunk->contig_bits > > > == 0) > > > return 0; > > > > > > - return pcpu_size_to_slot(chunk->free_bytes); > > > + return pcpu_size_to_slot(chunk->contig_bits * > > > +PCPU_MIN_ALLOC_SIZE); > > > } > > > > > > /* set the pointer to a chunk in a page struct */ > > > > Reviewed-by: Peng Fan > > > > Not relevant to this patch, another optimization to percpu might be > > good to use per chunk spin_lock, not gobal pcpu_lock. > > > > Percpu memory itself is expensive and for the most part shouldn't be part of > the critical path. Ideally, we don't have multiple chunks being allocated > simultaneously because once an allocation is given out, the chunk is pinned > until all allocations are freed. Thanks for clarification, since not critical patch, use global lock should be ok. Thanks, Peng. > > Thanks, > Dennis
RE: [PATCH 1/2] percpu: km: remove SMP check
> -Original Message- > From: Dennis Zhou [mailto:den...@kernel.org] > Sent: 2019年2月28日 0:41 > To: Peng Fan > Cc: Dennis Zhou ; Christopher Lameter ; > t...@kernel.org; linux...@kvack.org; linux-kernel@vger.kernel.org; > van.free...@gmail.com > Subject: Re: [PATCH 1/2] percpu: km: remove SMP check > > On Wed, Feb 27, 2019 at 01:02:16PM +, Peng Fan wrote: > > Hi Dennis > > > > > -Original Message- > > > From: Dennis Zhou [mailto:den...@kernel.org] > > > Sent: 2019年2月27日 1:04 > > > To: Christopher Lameter > > > Cc: Peng Fan ; t...@kernel.org; linux...@kvack.org; > > > linux-kernel@vger.kernel.org; van.free...@gmail.com > > > Subject: Re: [PATCH 1/2] percpu: km: remove SMP check > > > > > > On Tue, Feb 26, 2019 at 03:16:44PM +, Christopher Lameter wrote: > > > > On Mon, 25 Feb 2019, Dennis Zhou wrote: > > > > > > > > > > @@ -27,7 +27,7 @@ > > > > > > * chunk size is not aligned. percpu-km code will whine about > it. > > > > > > */ > > > > > > > > > > > > -#if defined(CONFIG_SMP) && > > > > > > defined(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) > > > > > > +#if defined(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) > > > > > > #error "contiguous percpu allocation is incompatible with > > > > > > paged first > > > chunk" > > > > > > #endif > > > > > > > > > > > > -- > > > > > > 2.16.4 > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > I think keeping CONFIG_SMP makes this easier to remember > > > > > dependencies rather than having to dig into the config. So this > > > > > is a NACK > > > from me. > > > > > > > > But it simplifies the code and makes it easier to read. > > > > > > > > > > > > > > I think the check isn't quite right after looking at it a little longer. > > > Looking at x86, I believe you can compile it with !SMP and > > > CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK will still be set. This > should > > > still work because x86 has an MMU. > > > > You are right, x86 could boots up with > NEED_PER_CPU_PAGE_FIRST_CHUNK > > =y and SMP=n. Tested with qemu, info as below: > > > > / # zcat /proc/config.gz | grep NEED_PER_CPU_KM > > CONFIG_NEED_PER_CPU_KM=y / # zcat /proc/config.gz | grep SMP > > CONFIG_BROKEN_ON_SMP=y # CONFIG_SMP is not set > > CONFIG_GENERIC_SMP_IDLE_THREAD=y / # zcat /proc/config.gz | grep > > NEED_PER_CPU_PAGE_FIRST_CHUNK > CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y > > / # cat /proc/cpuinfo > > processor : 0 > > vendor_id : AuthenticAMD > > cpu family : 6 > > model : 6 > > model name : QEMU Virtual CPU version 2.5+ > > stepping: 3 > > cpu MHz : 3192.613 > > cache size : 512 KB > > fpu : yes > > fpu_exception : yes > > cpuid level : 13 > > wp : yes > > flags : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca > cmov pat pse36 clflush mmx fxsr sse sse2 syscall nx lm nopl cpuid pni cx16 > hypervisor lahf_lm svm 3dnowprefetl > > bugs: fxsave_leak sysret_ss_attrs spectre_v1 spectre_v2 > spec_store_bypass > > bogomips: 6385.22 > > TLB size: 1024 4K pages > > clflush size: 64 > > cache_alignment : 64 > > address sizes : 42 bits physical, 48 bits virtual > > power management: > > > > > > But from the comments in this file: > > " > > * - CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK must not be defined. > It's > > * not compatible with PER_CPU_KM. EMBED_FIRST_CHUNK should > work > > * fine. > > " > > > > I did not read into details why it is not allowed, but x86 could still > > work with KM and NEED_PER_CPU_PAGE_FIRST_CHUNK. > > > > The first chunk requires special handling on SMP to bring the static variables > into the percpu address space. On UP, identity mapping makes static variables > indistinguishable by aligning the percpu address space and the virtual adress > space. The percpu-km allocator allocates full chunks at a time to deal with > NOMMU archs. So the difference is if the virtual address space is the same as > the physical. Thanks for clarification. > > > > > > > I think more correctly it would be something like below, but I don't > > > have the time to fully verify it right now. > > > > > > Thanks, > > > Dennis > > > > > > --- > > > diff --git a/mm/percpu-km.c b/mm/percpu-km.c index > > > 0f643dc2dc65..69ccad7d9807 100644 > > > --- a/mm/percpu-km.c > > > +++ b/mm/percpu-km.c > > > @@ -27,7 +27,7 @@ > > > * chunk size is not aligned. percpu-km code will whine about it. > > > */ > > > > > > -#if defined(CONFIG_SMP) && > > > defined(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) > > > +#if !defined(CONFIG_MMU) && > > > +defined(CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK) > > > #error "contiguous percpu allocation is incompatible with paged first > chunk" > > > #endif > > > > > > > Acked-by: Peng Fan > > > > Thanks, > > Peng > > While this change may seem right to me. Verification would be to double > check other architectures too. x86 just happened to be a counter example I > had in mind. Unless someone reports this as being an issue or
[PATCH 0/2] Thermal MMIO Driver
This series introduces the generic thermal MMIO driver that will use memory mapped reads to get the temperature. Any HW/System that allows temperature reading by a single memory-mapped reading, be it register or shared memory, is a potential candidate to work with this driver. This driver is most suitable for cases such as the following: - The entire thermal HW setup is done by another SW entity (e.g. bootloader) and all that is left is to read the current temperature from a register. - The thermal HW setup is done via an external CPU (e.g. micro-controller) and that CPU has is using shared memory that can be memory-mapped to this driver. - The thermal HW setup and reading is done via CPLD, which exports the current temperature to the system via a register. - The thermal HW is working out-of-the-box and only reports temperature via a single register access. Talel Shenhar (2): dt-bindings: thermal: thermal_mmio: Add binding documentation thermal: Introduce thermal MMIO .../devicetree/bindings/thermal/thermal_mmio.txt | 173 + drivers/thermal/Kconfig| 11 ++ drivers/thermal/Makefile | 3 + drivers/thermal/thermal_mmio.c | 214 + 4 files changed, 401 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/thermal_mmio.txt create mode 100644 drivers/thermal/thermal_mmio.c -- 2.7.4
[PATCH 2/2] thermal: Introduce thermal MMIO
This patch introduces a generic thermal driver, which enables easy
connectivity between "simple" thermal HW devices and the thermal
subsystem.
"simple" may be any system that allows temperature reading via a single
memory-mapped read, be it register or shared memory, and that doesn't
require any HW configuration to be done by the driver.
Such "simple" HW is a candidate to work with this driver.
Signed-off-by: Talel Shenhar
---
drivers/thermal/Kconfig| 11 +++
drivers/thermal/Makefile | 3 +
drivers/thermal/thermal_mmio.c | 214 +
3 files changed, 228 insertions(+)
create mode 100644 drivers/thermal/thermal_mmio.c
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 1775d44..0033ba2 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -199,6 +199,17 @@ config THERMAL_EMULATION
because userland can easily disable the thermal policy by simply
flooding this sysfs node with low temperature values.
+config THERMAL_MMIO
+ bool
+ depends on OF || COMPILE_TEST
+ prompt "mmio thermal driver"
+ help
+ This option enables the generic thermal MMIO driver that will use
+ memory-mapped reads to get the temperature. Any HW/System that
+ allows temperature reading by a single memory-mapped reading, be it
+ register or shared memory, is a potential candidate to work with this
+ driver.
+
config HISI_THERMAL
tristate "Hisilicon thermal driver"
depends on ARCH_HISI || COMPILE_TEST
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 82bb50d..beec616 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -27,6 +27,9 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL) += clock_cooling.o
# devfreq cooling
thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
+# generic memory mapped thermal driver
+thermal_sys-$(CONFIG_THERMAL_MMIO) += thermal_mmio.o
+
# platform thermal drivers
obj-y += broadcom/
obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM) += qcom-spmi-temp-alarm.o
diff --git a/drivers/thermal/thermal_mmio.c b/drivers/thermal/thermal_mmio.c
new file mode 100644
index 000..1119e71
--- /dev/null
+++ b/drivers/thermal/thermal_mmio.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2018 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ */
+
+#include
+#include
+#include
+#include
+
+#define THERMAL_MMIO_DEFAULT_SENSOR_WIDTH sizeof(u32)
+#define THERMAL_MMIO_DEFAULT_SENSOR_FACTOR 1
+#define THERMAL_MMIO_DEFAULT_SENSOR_BIAS 0
+#define THERMAL_MMIO_DEFAULT_SENSOR_DIVIDER 1
+
+struct thermal_mmio {
+ void __iomem *mmio_base;
+ u32 (*read_mmio)(void __iomem *mmio_base);
+ int width;
+ u32 mask;
+ int factor;
+ int bias;
+ int divider;
+};
+
+static u32 thermal_mmio_readb(void __iomem *mmio_base)
+{
+ return readb(mmio_base);
+}
+
+static u32 thermal_mmio_readw(void __iomem *mmio_base)
+{
+ return readw(mmio_base);
+}
+
+static u32 thermal_mmio_readl(void __iomem *mmio_base)
+{
+ return readl(mmio_base);
+}
+
+static int thermal_mmio_get_temperature(void *private, int *temp)
+{
+ int t;
+ struct thermal_mmio *sensor =
+ (struct thermal_mmio *)private;
+
+ t = sensor->read_mmio(sensor->mmio_base) & sensor->mask;
+ t *= sensor->factor;
+ t += sensor->bias;
+ t /= sensor->divider;
+
+ *temp = t;
+
+ return 0;
+}
+
+static struct thermal_zone_of_device_ops thermal_mmio_ops = {
+ .get_temp = thermal_mmio_get_temperature,
+};
+
+static int thermal_mmio_parse_dt(struct platform_device *pdev,
+struct thermal_mmio *sensor)
+{
+ struct resource *resource;
+ int ret;
+
+ resource = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (IS_ERR(resource)) {
+ dev_err(&pdev->dev,
+ "fail to get platform memory resource (%ld)\n",
+ PTR_ERR(resource));
+ return PTR_ERR(resource);
+ }
+
+ sensor->mmio_base = devm_ioremap_resource(&pdev->dev, resource);
+ if (IS_ERR(sensor->mmio_base)) {
+ dev_err(&pdev->dev, "failed to ioremap memory (%ld)\n",
+ PTR_ERR(sensor->mmio_base));
+ return PTR_ERR(sensor->mmio_base);
+ }
+
+ ret = of_property_read_u32(pdev->dev.of_node, "sensor-width",
+ &sensor->width);
+ if (ret < 0) {
+ sensor->width = THERMAL_MMIO_DEFAULT_SENSOR_WIDTH;
+ dev_dbg(&pdev->dev,
+ "sensor-width undefined (%d) - using default: %d\n",
+ ret, sensor->width);
+ }
+
+ ret = of_property_read_u32(pdev->dev.of_node, "sensor-mask",
+ &sensor->mask);
+ if (ret < 0) {
+
[PATCH 1/2] dt-bindings: thermal: thermal_mmio: Add binding documentation
Add thermal binding documentation for thermal MMIO driver. Signed-off-by: Talel Shenhar --- .../devicetree/bindings/thermal/thermal_mmio.txt | 173 + 1 file changed, 173 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/thermal_mmio.txt diff --git a/Documentation/devicetree/bindings/thermal/thermal_mmio.txt b/Documentation/devicetree/bindings/thermal/thermal_mmio.txt new file mode 100644 index 000..1f4d738 --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/thermal_mmio.txt @@ -0,0 +1,173 @@ +Generic Thermal MMIO Driver + +The generic thermal driver enables easy connectivity between "simple" +thermal HW devices to the thermal subsystem. "simple" - thermal HW that +doesn't need any configuration (such as power reset or clock enable) by a +driver and only exports the temperature via simple MMIO access, or, +alternatively, all the configuration is done by other entity (e.g. +bootloader). + +Any system that allows temperature reading via a single memory map read, be +it register or shared memory, is a potential candidate to work with this +driver. +This driver allows manipulations on the read value in order to allow some +flexibility for the various thermal HW, e.g. sensor-factor which will be +multiplied by the read value. + +This driver is most suitable for cases such as the following: +- The entire thermal HW setup (e.g. configure the thermal HW to work in + continuous mode) is done by another SW entity (e.g. bootloader) and all + that is left is to read the current temperature from a register +- The thermal reading is done by an external CPU (e.g. micro-controller) + and that CPU is exporting the reading via a shared memory +- The thermal HW setup and reading is done via CPLD, which exports the + current temperature to the system via a register +- The thermal HW is working out-of-the-box and only reports temperature via + a single register access + +Some examples for cases that this driver is not suitable for: +- Your HW need clock enabling to be done by thermal driver +- Your HW need some registers configurations in order to start reporting + temperatures and it is not doable by other entities (e.g. bootloader) +- Your HW allows reading of temperatures only between ADCs (or any other + timing constrains) +- In case your HW was configured by a bootloader and it lose the + configuration as part of low-power-state and need to be reconfigured. + (bootloader configuration typically is not sustained across low power + states) +- In case the reading from the MMIO require any type of locking + +So the only operation this driver will do is MMIO read for the temperature. +In case you are using HW that require some configurations this driver is +not suitable (you can decide to move all the configuration logic into your +bootloader and only leave the temperature reading to this driver but this +is up to you). + +An example of a system that uses this driver is the Amazon Nitro SoC in +which there is the main CPU and micro-controller: +(1) The micro-controller accesses an SBUS controller to read the thermal +sensor from an SBUS slave called Avago Technologies digital +Temperature/Voltage Sensor (ip16_SENS_thermvolt25_0) +(2) Once the micro-controller gathers the temperature it relays it to the +main CPU via a Shared Integrated RAM which is MMIO accessible by the CPU +(3) The thermal_mmio driver that runs on the main CPU will read the +temperature via an MMIO access to the Shared RAM + ++--+ ++ +| | (3) || +| Main CPU +->+ Integrated RAM | +| | || ++--+ +-^--+ +|(2) ++-+ +--+ +| | (1) | | +| SBUS controller <---+ micro-controller | +| | | | ++++ +--+ + | ++v-+ +| | +| Avago Technologies digital | +| Temperature/Voltage Sensor | +| (ip16_SENS_thermvolt25_0) | +| | ++--+ + +Required properties: +- compatible: "thermal-mmio". +- reg: The physical base address and length of the sensor's registers. +- #thermal-sensor-cells: Must be 1. See ./thermal.txt for a description. + +Optional properties: +- sensor-width: Width (in bytes) of each consecutive sensor. +Supported: 1, 2, 4. +(Default = 4) +- sensor-mask: Mask to be applied on the raw read value before any + calculation. + (Default = 0x) +- sensor-factor: Scale value by which to multiple the masked read value +This is a signed 32 bit value. +(Default = 1) +- sensor-bias: Bias value to be added to the scaled value. +
Re: [PATCH 01/20] asm-generic/mmiowb: Add generic implementation of mmiowb() tracking
Nicholas Piggin writes:
> Will Deacon's on March 2, 2019 12:03 am:
>> In preparation for removing all explicit mmiowb() calls from driver
>> code, implement a tracking system in asm-generic based loosely on the
>> PowerPC implementation. This allows architectures with a non-empty
>> mmiowb() definition to have the barrier automatically inserted in
>> spin_unlock() following a critical section containing an I/O write.
>
> Is there a reason to call this "mmiowb"? We already have wmb that
> orders cacheable stores vs mmio stores don't we?
>
> Yes ia64 "sn2" is broken in that case, but that can be fixed (if
> anyone really cares about the platform any more). Maybe that's
> orthogonal to what you're doing here, I just don't like seeing
> "mmiowb" spread.
>
> This series works for spin locks, but you would want a driver to
> be able to use wmb() to order locks vs mmio when using a bit lock
> or a mutex or whatever else. Calling your wmb-if-io-is-pending
> version io_mb_before_unlock() would kind of match with existing
> patterns.
>
>> +static inline void mmiowb_set_pending(void)
>> +{
>> +struct mmiowb_state *ms = __mmiowb_state();
>> +ms->mmiowb_pending = ms->nesting_count;
>> +}
>> +
>> +static inline void mmiowb_spin_lock(void)
>> +{
>> +struct mmiowb_state *ms = __mmiowb_state();
>> +ms->nesting_count++;
>> +}
>> +
>> +static inline void mmiowb_spin_unlock(void)
>> +{
>> +struct mmiowb_state *ms = __mmiowb_state();
>> +
>> +if (unlikely(ms->mmiowb_pending)) {
>> +ms->mmiowb_pending = 0;
>> +mmiowb();
>> +}
>> +
>> +ms->nesting_count--;
>> +}
>
> Humour me for a minute and tell me what this algorithm is doing, or
> what was broken about the powerpc one, which is basically:
>
> static inline void mmiowb_set_pending(void)
> {
> struct mmiowb_state *ms = __mmiowb_state();
> ms->mmiowb_pending = 1;
> }
>
> static inline void mmiowb_spin_lock(void)
> {
> }
The current powerpc code clears io_sync in spin_lock().
ie, it would be equivalent to:
static inline void mmiowb_spin_lock(void)
{
ms->mmiowb_pending = 0;
}
Which means that:
spin_lock(a);
writel(x, y);
spin_lock(b);
...
spin_unlock(b);
spin_unlock(a);
Does no barrier.
cheers
Re: kernel BUG at include/linux/mm.h:LINE! (4)
On Sat, Mar 2, 2019 at 9:05 PM Eric Biggers wrote: > > On Fri, Mar 01, 2019 at 11:05:05PM -0800, syzbot wrote: > > Hello, > > > > syzbot found the following crash on: > > > > HEAD commit:42fd8df9d1d9 Add linux-next specific files for 20190228 > > git tree: linux-next > > console output: https://syzkaller.appspot.com/x/log.txt?x=16c3cd5cc0 > > kernel config: https://syzkaller.appspot.com/x/.config?x=c0f38652d28b522f > > dashboard link: https://syzkaller.appspot.com/bug?extid=cc252aa9d2d3b576246f > > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > > > Unfortunately, I don't have any reproducer for this crash yet. > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > > Reported-by: syzbot+cc252aa9d2d3b5762...@syzkaller.appspotmail.com > > > > page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) == 0) > > page->mem_cgroup:888059786cc0 > > [ cut here ] > > kernel BUG at include/linux/mm.h:579! > > invalid opcode: [#1] PREEMPT SMP KASAN > > CPU: 0 PID: 22405 Comm: syz-executor.3 Not tainted 5.0.0-rc8-next-20190228 > > #45 > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > > Google 01/01/2011 > > RIP: 0010:put_page_testzero include/linux/mm.h:579 [inline] > > RIP: 0010:put_page include/linux/mm.h:1025 [inline] > > RIP: 0010:generic_pipe_buf_release+0x120/0x160 fs/pipe.c:224 > > Code: bd ff 4c 89 e7 e8 90 43 db ff e8 5b 07 bd ff 5b 41 5c 41 5d 5d c3 e8 > > 4f 07 bd ff 48 c7 c6 60 98 75 87 4c 89 e7 e8 c0 db e4 ff <0f> 0b e8 39 07 bd > > ff 4d 8d 65 ff e9 3d ff ff ff 48 89 df e8 e8 f8 > > RSP: 0018:888056c57920 EFLAGS: 00010246 > > RAX: 0004 RBX: ea0002283db4 RCX: c9000c456000 > > RDX: 0004 RSI: 81984e72 RDI: ed100ad8af08 > > RBP: 888056c57938 R08: 0021 R09: ed1015d05011 > > R10: ed1015d05010 R11: 8880ae828087 R12: ea0002283d80 > > R13: R14: 88809ad3c800 R15: 8880592ac928 > > FS: 7fb53aaf2700() GS:8880ae80() knlGS: > > CS: 0010 DS: ES: CR0: 80050033 > > CR2: 7fff65e0fdb8 CR3: 93a2f000 CR4: 001406f0 > > DR0: DR1: DR2: > > DR3: DR6: fffe0ff0 DR7: 0400 > > Call Trace: > > pipe_buf_release include/linux/pipe_fs_i.h:129 [inline] > > iter_file_splice_write+0x7d1/0xbe0 fs/splice.c:759 > > do_splice_from fs/splice.c:847 [inline] > > direct_splice_actor+0x126/0x1a0 fs/splice.c:1019 > > splice_direct_to_actor+0x369/0x970 fs/splice.c:974 > > do_splice_direct+0x1da/0x2a0 fs/splice.c:1062 > > do_sendfile+0x597/0xd00 fs/read_write.c:1442 > > __do_sys_sendfile64 fs/read_write.c:1503 [inline] > > __se_sys_sendfile64 fs/read_write.c:1489 [inline] > > __x64_sys_sendfile64+0x1dd/0x220 fs/read_write.c:1489 > > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290 > > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > RIP: 0033:0x457e29 > > Code: ad b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 > > 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff > > 0f 83 7b b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 > > RSP: 002b:7fb53aaf1c78 EFLAGS: 0246 ORIG_RAX: 0028 > > RAX: ffda RBX: 0004 RCX: 00457e29 > > RDX: RSI: 0003 RDI: 0003 > > RBP: 0073bf00 R08: R09: > > R10: 00010200 R11: 0246 R12: 7fb53aaf26d4 > > R13: 004c4dce R14: 004d8af8 R15: > > Modules linked in: > > ---[ end trace ce17ea3937b628f2 ]--- > > RIP: 0010:put_page_testzero include/linux/mm.h:579 [inline] > > RIP: 0010:put_page include/linux/mm.h:1025 [inline] > > RIP: 0010:generic_pipe_buf_release+0x120/0x160 fs/pipe.c:224 > > Code: bd ff 4c 89 e7 e8 90 43 db ff e8 5b 07 bd ff 5b 41 5c 41 5d 5d c3 e8 > > 4f 07 bd ff 48 c7 c6 60 98 75 87 4c 89 e7 e8 c0 db e4 ff <0f> 0b e8 39 07 bd > > ff 4d 8d 65 ff e9 3d ff ff ff 48 89 df e8 e8 f8 > > RSP: 0018:888056c57920 EFLAGS: 00010246 > > RAX: 0004 RBX: ea0002283db4 RCX: c9000c456000 > > RDX: 0004 RSI: 81984e72 RDI: ed100ad8af08 > > RBP: 888056c57938 R08: 0021 R09: ed1015d05011 > > R10: ed1015d05010 R11: 8880ae828087 R12: ea0002283d80 > > R13: R14: 88809ad3c800 R15: 8880592ac928 > > FS: 7fb53aaf2700() GS:8880ae90() knlGS: > > CS: 0010 DS: ES: CR0: 80050033 > > CR2: 001b30225000 CR3: 93a2f000 CR4: 001406e0 > > DR0: DR1: DR2: > > DR3: DR6: fffe0ff0 DR7: 0400 > > > > > > --- > > This bug is generated by a bot. It may contain errors. > > See https://goo.gl/tpsmEJ f
Re: [PATCH v2] RDMA/umem: minor bug fix and cleanup in error handling paths
On 02/03/2019 21:44, Ira Weiny wrote:
On Sat, Mar 02, 2019 at 12:24:35PM -0800, john.hubb...@gmail.com wrote:
From: John Hubbard
...
3. Dead code removal: the check for (user_virt & ~page_mask)
is checking for a condition that can never happen,
because earlier:
user_virt = user_virt & page_mask;
...so, remove that entire phrase.
bcnt -= min_t(size_t, npages << PAGE_SHIFT, bcnt);
mutex_lock(&umem_odp->umem_mutex);
for (j = 0; j < npages; j++, user_virt += PAGE_SIZE) {
- if (user_virt & ~page_mask) {
- p += PAGE_SIZE;
- if (page_to_phys(local_page_list[j]) != p) {
- ret = -EFAULT;
- break;
- }
- put_page(local_page_list[j]);
- continue;
- }
-
I think this is trying to account for compound pages. (ie page_mask could
represent more than PAGE_SIZE which is what user_virt is being incrimented by.)
But putting the page in that case seems to be the wrong thing to do?
Yes this was added by Artemy[1] now cc'ed.
Right, this is for huge pages, please keep it.
put_page() needed to decrement refcount of the head page.
Re: [PATCH 01/20] asm-generic/mmiowb: Add generic implementation of mmiowb() tracking
Linus Torvalds's on March 3, 2019 2:29 pm: > On Sat, Mar 2, 2019, 19:34 Nicholas Piggin wrote: > >> >> It doesn't have to be done all at once with this series, obviously this >> is a big improvement on its own. But why perpetuate the nomenclature >> and concept for new code added now? >> > > What nomenclature? > > Nobody will be using mmiowb(). That's the whole point of the patch series. > > It's now an entirely internal name, and nobody cares. Why even bother with it at all, "internal" or not? Just get rid of mmiowb, the concept is obsolete. > And none of this has anything to do with wmb(), since it's about IO being > ordered across cpu's by spin locks, not by barriers. > > So I'm not seeing what you're arguing about. Pretend ia64 doesn't exist for a minute. Now the regular mb/wmb barriers orders IO across CPUs with respect to their cacheable accesses. Regardless of whether that cacheable access is a spin lock, a bit lock, an atomic, a mutex... This is how it was before mmiowb came along. Nothing wrong with this series to make spinlocks order mmio, but why call it mmiowb? Another patch could rename ia64's mmiowb and then the name can be removed from the tree completely. Thanks, Nick
Re: [mt76/mt7603/mac] Question about missing variable assignment
On 2019-03-02 22:10, Gustavo A. R. Silva wrote:
> Hi all,
>
> The following piece of code in drivers/net/wireless/mediatek/mt76/mt7603/mac.c
> is missing a variable assignment before line 1058. Notice that there
> is a potential execution path in which variable *i* is compared against
> magic number 15 at line 1075 without being initialized previously
> (this was reported by Coverity):
>
> 1055 out:
> 1056 final_rate_flags = info->status.rates[final_idx].flags;
> 1057
> 1058 switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
> 1059 case MT_PHY_TYPE_CCK:
> 1060 cck = true;
> 1061 /* fall through */
> 1062 case MT_PHY_TYPE_OFDM:
> 1063 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
> 1064 sband = &dev->mt76.sband_5g.sband;
> 1065 else
> 1066 sband = &dev->mt76.sband_2g.sband;
> 1067 final_rate &= GENMASK(5, 0);
> 1068 final_rate = mt7603_get_rate(dev, sband, final_rate,
> cck);
> 1069 final_rate_flags = 0;
> 1070 break;
> 1071 case MT_PHY_TYPE_HT_GF:
> 1072 case MT_PHY_TYPE_HT:
> 1073 final_rate_flags |= IEEE80211_TX_RC_MCS;
> 1074 final_rate &= GENMASK(5, 0);
> 1075 if (i > 15)
> 1076 return false;
> 1077 break;
> 1078 default:
> 1079 return false;
> 1080 }
>
> My guess is that such missing assignment should be something similar
> to the one at line 566:
>
> i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);
>
> but I'm not sure what the proper arguments for macro FIELD_GET should
> be.
>
> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe
>
> What do you think?
Thanks for reporting this. The fix is simpler than that, the check
should be: if (final_rate > 15)
I will send a fix.
- Felix
Re: [PATCH 01/20] asm-generic/mmiowb: Add generic implementation of mmiowb() tracking
Michael Ellerman's on March 3, 2019 7:26 pm:
> Nicholas Piggin writes:
>> Will Deacon's on March 2, 2019 12:03 am:
>>> In preparation for removing all explicit mmiowb() calls from driver
>>> code, implement a tracking system in asm-generic based loosely on the
>>> PowerPC implementation. This allows architectures with a non-empty
>>> mmiowb() definition to have the barrier automatically inserted in
>>> spin_unlock() following a critical section containing an I/O write.
>>
>> Is there a reason to call this "mmiowb"? We already have wmb that
>> orders cacheable stores vs mmio stores don't we?
>>
>> Yes ia64 "sn2" is broken in that case, but that can be fixed (if
>> anyone really cares about the platform any more). Maybe that's
>> orthogonal to what you're doing here, I just don't like seeing
>> "mmiowb" spread.
>>
>> This series works for spin locks, but you would want a driver to
>> be able to use wmb() to order locks vs mmio when using a bit lock
>> or a mutex or whatever else. Calling your wmb-if-io-is-pending
>> version io_mb_before_unlock() would kind of match with existing
>> patterns.
>>
>>> +static inline void mmiowb_set_pending(void)
>>> +{
>>> + struct mmiowb_state *ms = __mmiowb_state();
>>> + ms->mmiowb_pending = ms->nesting_count;
>>> +}
>>> +
>>> +static inline void mmiowb_spin_lock(void)
>>> +{
>>> + struct mmiowb_state *ms = __mmiowb_state();
>>> + ms->nesting_count++;
>>> +}
>>> +
>>> +static inline void mmiowb_spin_unlock(void)
>>> +{
>>> + struct mmiowb_state *ms = __mmiowb_state();
>>> +
>>> + if (unlikely(ms->mmiowb_pending)) {
>>> + ms->mmiowb_pending = 0;
>>> + mmiowb();
>>> + }
>>> +
>>> + ms->nesting_count--;
>>> +}
>>
>> Humour me for a minute and tell me what this algorithm is doing, or
>> what was broken about the powerpc one, which is basically:
>>
>> static inline void mmiowb_set_pending(void)
>> {
>> struct mmiowb_state *ms = __mmiowb_state();
>> ms->mmiowb_pending = 1;
>> }
>>
>> static inline void mmiowb_spin_lock(void)
>> {
>> }
>
> The current powerpc code clears io_sync in spin_lock().
>
> ie, it would be equivalent to:
>
> static inline void mmiowb_spin_lock(void)
> {
> ms->mmiowb_pending = 0;
> }
Ah okay that's what I missed. How about we just not do that?
Thanks,
Nick
KASAN: use-after-free Read in unix_dgram_poll
Hello, syzbot found the following crash on: HEAD commit:7d762d69145a afs: Fix manually set volume location server .. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=131d832ac0 kernel config: https://syzkaller.appspot.com/x/.config?x=b76ec970784287c dashboard link: https://syzkaller.appspot.com/bug?extid=503d4cc169fcec1cb18c compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11934262c0 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+503d4cc169fcec1cb...@syzkaller.appspotmail.com == BUG: KASAN: use-after-free in unix_dgram_poll+0x5e1/0x690 net/unix/af_unix.c:2695 Read of size 4 at addr 88809292aae0 by task syz-executor.1/18946 CPU: 0 PID: 18946 Comm: syz-executor.1 Not tainted 5.0.0-rc8+ #88 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x172/0x1f0 lib/dump_stack.c:113 print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187 kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317 __asan_report_load4_noabort+0x14/0x20 mm/kasan/generic_report.c:134 unix_dgram_poll+0x5e1/0x690 net/unix/af_unix.c:2695 sock_poll+0x291/0x340 net/socket.c:1127 vfs_poll include/linux/poll.h:86 [inline] aio_poll fs/aio.c:1766 [inline] __io_submit_one fs/aio.c:1876 [inline] io_submit_one+0xe3e/0x1cf0 fs/aio.c:1909 __do_sys_io_submit fs/aio.c:1954 [inline] __se_sys_io_submit fs/aio.c:1924 [inline] __x64_sys_io_submit+0x1bd/0x580 fs/aio.c:1924 ? 0x8100 do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x457e29 Code: ad b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 7b b8 fb ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7fd43ca93c78 EFLAGS: 0246 ORIG_RAX: 00d1 RAX: ffda RBX: 0003 RCX: 00457e29 RDX: 2600 RSI: 1d70 RDI: 7fd43ca73000 RBP: 0073bf00 R08: R09: R10: R11: 0246 R12: 7fd43ca946d4 R13: 004bf02f R14: 004d09b0 R15: Allocated by task 18946: save_stack+0x45/0xd0 mm/kasan/common.c:73 set_track mm/kasan/common.c:85 [inline] __kasan_kmalloc mm/kasan/common.c:495 [inline] __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:468 kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:503 slab_post_alloc_hook mm/slab.h:440 [inline] slab_alloc mm/slab.c:3388 [inline] kmem_cache_alloc+0x11a/0x6f0 mm/slab.c:3548 sk_prot_alloc+0x67/0x2e0 net/core/sock.c:1471 sk_alloc+0x39/0xf70 net/core/sock.c:1531 unix_create1+0xc3/0x530 net/unix/af_unix.c:764 unix_create+0x103/0x1e0 net/unix/af_unix.c:825 __sock_create+0x3e6/0x750 net/socket.c:1275 sock_create net/socket.c:1315 [inline] __sys_socketpair+0x272/0x5e0 net/socket.c:1407 __do_sys_socketpair net/socket.c:1456 [inline] __se_sys_socketpair net/socket.c:1453 [inline] __x64_sys_socketpair+0x97/0xf0 net/socket.c:1453 do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe Freed by task 18944: save_stack+0x45/0xd0 mm/kasan/common.c:73 set_track mm/kasan/common.c:85 [inline] __kasan_slab_free+0x102/0x150 mm/kasan/common.c:457 kasan_slab_free+0xe/0x10 mm/kasan/common.c:465 __cache_free mm/slab.c:3494 [inline] kmem_cache_free+0x86/0x260 mm/slab.c:3754 sk_prot_free net/core/sock.c:1512 [inline] __sk_destruct+0x4b6/0x6d0 net/core/sock.c:1596 sk_destruct+0x7b/0x90 net/core/sock.c:1604 __sk_free+0xce/0x300 net/core/sock.c:1615 sk_free+0x42/0x50 net/core/sock.c:1626 sock_put include/net/sock.h:1707 [inline] unix_release_sock+0x921/0xbb0 net/unix/af_unix.c:573 unix_release+0x44/0x90 net/unix/af_unix.c:835 __sock_release+0xd3/0x250 net/socket.c:579 sock_close+0x1b/0x30 net/socket.c:1139 __fput+0x2df/0x8d0 fs/file_table.c:278 fput+0x16/0x20 fs/file_table.c:309 task_work_run+0x14a/0x1c0 kernel/task_work.c:113 tracehook_notify_resume include/linux/tracehook.h:188 [inline] exit_to_usermode_loop+0x273/0x2c0 arch/x86/entry/common.c:166 prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline] syscall_return_slowpath arch/x86/entry/common.c:268 [inline] do_syscall_64+0x52d/0x610 arch/x86/entry/common.c:293 entry_SYSCALL_64_after_hwframe+0x49/0xbe The buggy address belongs to the object at 88809292a740 which belongs to the cache UNIX(49:syz1) of size 1728 The buggy address is located 928 bytes inside of 1728-byte region [88809292a740, 88809292ae00) The buggy address belongs to the page: page:ea00024a4a80 count:1 mapcount:0 mapping:8880920c0800 index:0x0 flags: 0x1fffc000200(slab) raw: 01fffc000200
Re: [PATCH v3] mm/memory.c: do_fault: avoid usage of stale vm_area_struct
On Sun, Mar 03, 2019 at 08:28:04AM +0100, Jan Stancek wrote: > Cache mm_struct to avoid using potentially stale "vma". > > [1] > https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/mem/mtest06/mmap1.c > > Signed-off-by: Jan Stancek > Reviewed-by: Andrea Arcangeli Reviewed-by: Matthew Wilcox
Re: [PATCH] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
Hi,
On 01-03-19 17:40, m...@myself5.de wrote:
From: Christian Oder
Add touchscreen info for the CHUWUI Hi10 Air tablet.
Signed-off-by: Christian Oder
Thank you for the patch, patch looks good to me:
Reviewed-by: Hans de Goede
Regards,
Hans
---
drivers/platform/x86/touchscreen_dmi.c | 27 ++
1 file changed, 27 insertions(+)
diff --git a/drivers/platform/x86/touchscreen_dmi.c
b/drivers/platform/x86/touchscreen_dmi.c
index 167a156e3cc7..2d56ff7c8230 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
.properties = chuwi_hi8_pro_props,
};
+static const struct property_entry chuwi_hi10_air_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
+ PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
+ PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
+ PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct ts_dmi_data chuwi_hi10_air_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = chuwi_hi10_air_props,
+};
+
static const struct property_entry chuwi_vi8_props[] = {
PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
@@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[]
= {
DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
},
},
+ {
+ /* Chuwi Hi10 Air */
+ .driver_data = (void *)&chuwi_hi10_air_data,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
+ },
+ },
{
/* Chuwi Vi8 (CWI506) */
.driver_data = (void *)&chuwi_vi8_data,
Re: False positive "do_IRQ: #.55 No irq handler for vector" messages on AMD ryzen based laptops
Hi, On 21-02-19 13:30, Hans de Goede wrote: Hi, On 19-02-19 22:47, Lendacky, Thomas wrote: On 2/19/19 3:01 PM, Thomas Gleixner wrote: Hans, On Tue, 19 Feb 2019, Hans de Goede wrote: Cc+: ACPI/AMD folks Various people are reporting false positive "do_IRQ: #.55 No irq handler for vector" messages on AMD ryzen based laptops, see e.g.: https://bugzilla.redhat.com/show_bug.cgi?id=1551605 Which contains this dmesg snippet: Feb 07 20:14:29 localhost.localdomain kernel: smp: Bringing up secondary CPUs ... Feb 07 20:14:29 localhost.localdomain kernel: x86: Booting SMP configuration: Feb 07 20:14:29 localhost.localdomain kernel: node #0, CPUs: #1 Feb 07 20:14:29 localhost.localdomain kernel: do_IRQ: 1.55 No irq handler for vector Feb 07 20:14:29 localhost.localdomain kernel: #2 Feb 07 20:14:29 localhost.localdomain kernel: do_IRQ: 2.55 No irq handler for vector Feb 07 20:14:29 localhost.localdomain kernel: #3 Feb 07 20:14:29 localhost.localdomain kernel: do_IRQ: 3.55 No irq handler for vector Feb 07 20:14:29 localhost.localdomain kernel: smp: Brought up 1 node, 4 CPUs Feb 07 20:14:29 localhost.localdomain kernel: smpboot: Max logical packages: 1 Feb 07 20:14:29 localhost.localdomain kernel: smpboot: Total of 4 processors activated (15968.49 BogoMIPS) It seems that we get an IRQ for each CPU as we bring it online, which feels to me like it is some sorta false-positive. Sigh, that looks like BIOS value add again. It's not a false positive. Something _IS_ sending a vector 55 to these CPUs for whatever reason. I remember seeing something like this in the past and it turned out to be a BIOS issue. BIOS was enabling the APs to interact with the legacy 8259 interrupt controller when only the BSP should. During POST the APs were exposed to ExtINT/INTR events as a result of the mis-configuration (probably due to a UEFI timer-tick using the 8259) and this left a pending ExtINT/INTR interrupt latched on the APs. When the APs were started by the OS, the latched ExtINT/INTR interrupt is processed shortly after the OS enables interrupts. The AP then queries the 8259 to identify the vector number (which is the value of the 8259's ICW2 register + the IRQ level). The master 8259's ICW2 was set to 0x30 and, since no interrupts are actually pending, the 8259 will respond with IRQ7 (spurious interrupt) yielding a vector of 0x37 or 55. The OS was not expecting vector 55 and printed the message. From the Intel Developer's Manual: Vol 3a, Section 10.5.1: "Only one processor in the system should have an LVT entry configured to use the ExtINT delivery mode." Not saying this is the problem, but very well could be. That sounds like a likely candidate, esp. also since this only happens once per CPU when we first only the CPU. Can you provide me with a patch with some printk-s / pr_debugs to test for this, then I can build a kernel with that patch added and we can see if your hypothesis is right. Ping? I like your theory, can you provide some help with debugging this further (to prove that your theory is correct ) ? Regards, Hans
Re: [RFC PATCH 2/2] ceph: quota: fix quota subdir mounts
Jeff Layton writes:
> On Fri, 2019-03-01 at 17:57 +, Luis Henriques wrote:
>> The CephFS kernel client doesn't enforce quotas that are set in a
>> directory that isn't visible in the mount point. For example, given the
>> path '/dir1/dir2', if quotas are set in 'dir1' and the mount is done in with
>>
>> mount -t ceph ::/dir1/ /mnt
>>
>> then the client can't access the 'dir1' inode from the quota realm dir2
>> belongs to.
>>
>> This patch fixes this by simply doing an MDS LOOKUPINO Op and grabbing a
>> reference to it (so that it doesn't disappear again). This also requires an
>> extra field in ceph_snap_realm so that we know we have to release that
>> reference when destroying the realm.
>>
>> Links: https://tracker.ceph.com/issues/3848
>
> This bug looks unrelated to the patch description. Are you sure it's
> correct?
Ups! Looks like I truncated the bug number. Sorry about that, here's
the correct link:
https://tracker.ceph.com/issues/38482
Cheers,
--
Luis
>> Reported-by: Hendrik Peyerl
>> Signed-off-by: Luis Henriques
>> ---
>> fs/ceph/caps.c | 2 +-
>> fs/ceph/quota.c | 30 +++---
>> fs/ceph/snap.c | 3 +++
>> fs/ceph/super.h | 2 ++
>> 4 files changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ceph/caps.c b/fs/ceph/caps.c
>> index bba28a5034ba..e79994ff53d6 100644
>> --- a/fs/ceph/caps.c
>> +++ b/fs/ceph/caps.c
>> @@ -1035,7 +1035,7 @@ static void drop_inode_snap_realm(struct
>> ceph_inode_info *ci)
>> list_del_init(&ci->i_snap_realm_item);
>> ci->i_snap_realm_counter++;
>> ci->i_snap_realm = NULL;
>> -if (realm->ino == ci->i_vino.ino)
>> +if ((realm->ino == ci->i_vino.ino) && !realm->own_inode)
>> realm->inode = NULL;
>> spin_unlock(&realm->inodes_with_caps_lock);
>> ceph_put_snap_realm(ceph_sb_to_client(ci->vfs_inode.i_sb)->mdsc,
>> diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c
>> index 9455d3aef0c3..f6b972d222e4 100644
>> --- a/fs/ceph/quota.c
>> +++ b/fs/ceph/quota.c
>> @@ -22,7 +22,16 @@ void ceph_adjust_quota_realms_count(struct inode *inode,
>> bool inc)
>> static inline bool ceph_has_realms_with_quotas(struct inode *inode)
>> {
>> struct ceph_mds_client *mdsc = ceph_inode_to_client(inode)->mdsc;
>> -return atomic64_read(&mdsc->quotarealms_count) > 0;
>> +struct super_block *sb = mdsc->fsc->sb;
>> +
>> +if (atomic64_read(&mdsc->quotarealms_count) > 0)
>> +return true;
>> +/* if root is the real CephFS root, we don't have quota realms */
>> +if (sb->s_root->d_inode &&
>> +(sb->s_root->d_inode->i_ino == CEPH_INO_ROOT))
>> +return false;
>> +/* otherwise, we can't know for sure */
>> +return true;
>> }
>>
>> void ceph_handle_quota(struct ceph_mds_client *mdsc,
>> @@ -166,6 +175,7 @@ static bool check_quota_exceeded(struct inode *inode,
>> enum quota_check_op op,
>> return false;
>>
>> down_read(&mdsc->snap_rwsem);
>> +restart:
>> realm = ceph_inode(inode)->i_snap_realm;
>> if (realm)
>> ceph_get_snap_realm(mdsc, realm);
>> @@ -176,8 +186,22 @@ static bool check_quota_exceeded(struct inode *inode,
>> enum quota_check_op op,
>> spin_lock(&realm->inodes_with_caps_lock);
>> in = realm->inode ? igrab(realm->inode) : NULL;
>> spin_unlock(&realm->inodes_with_caps_lock);
>> -if (!in)
>> -break;
>> +if (!in) {
>> +up_read(&mdsc->snap_rwsem);
>> +in = ceph_lookup_inode(inode->i_sb, realm->ino);
>> +down_read(&mdsc->snap_rwsem);
>> +if (IS_ERR(in)) {
>> +pr_warn("Can't lookup inode %llx (err: %ld)\n",
>> +realm->ino, PTR_ERR(in));
>> +break;
>> +}
>> +spin_lock(&realm->inodes_with_caps_lock);
>> +realm->inode = in;
>> +realm->own_inode = true;
>> +spin_unlock(&realm->inodes_with_caps_lock);
>> +ceph_put_snap_realm(mdsc, realm);
>> +goto restart;
>> +}
>>
>> ci = ceph_inode(in);
>> spin_lock(&ci->i_ceph_lock);
>> diff --git a/fs/ceph/snap.c b/fs/ceph/snap.c
>> index f74193da0e09..c84ed8e8526a 100644
>> --- a/fs/ceph/snap.c
>> +++ b/fs/ceph/snap.c
>> @@ -117,6 +117,7 @@ static struct ceph_snap_realm *ceph_create_snap_realm(
>>
>> atomic_set(&realm->nref, 1);/* for caller */
>> realm->ino = ino;
>> +realm->own_inode = false;
>> INIT_LIST_HEAD(&realm->children);
>> INIT_LIST_HEAD(&realm->child_item);
>> INIT_LIST_HEAD(&realm->empty_item);
>> @@ -184,6 +185,8 @@ static void __destroy_snap_realm(struct ceph_mds_client
>> *mdsc,
>> kfree(realm->prior_parent_snaps);
>> kfree(realm->snaps);
>> ceph_put_snap_con
Re: [PATCH v2 1/4] staging: iio: ad5933: add SPDX identifier
On Thu, 28 Feb 2019 23:52:30 -0300
Marcelo Schmitt wrote:
> Add SPDX identifier of GPL-2.0 for the ad5933 driver.
> Organize imports.
> Make multi-line comments compliant with the preferred code style.
One type of change per patch please.
So this is at least 3 patches.
SPDX,
Imports,
comment style.
Actual contents looks fine.
Jonathan
>
> Signed-off-by: Marcelo Schmitt
> ---
> .../staging/iio/impedance-analyzer/ad5933.c | 30 ++-
> 1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
> b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 3134295f014f..d75bdfbf93de 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -1,27 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> /*
> * AD5933 AD5934 Impedance Converter, Network Analyzer
> *
> * Copyright 2011 Analog Devices Inc.
> - *
> - * Licensed under the GPL-2.
> */
>
> -#include
> +#include
> +#include
> #include
> -#include
> -#include
> +#include
> #include
> +#include
> +#include
> +#include
> #include
> +#include
> #include
> -#include
> -#include
> -#include
> -#include
>
> -#include
> -#include
> #include
> +#include
> #include
> +#include
>
> /* AD5933/AD5934 Registers */
> #define AD5933_REG_CONTROL_HB0x80/* R/W, 1 byte */
> @@ -474,7 +473,8 @@ static IIO_DEVICE_ATTR(out_voltage0_settling_cycles, 0644,
> ad5933_store,
> AD5933_OUT_SETTLING_CYCLES);
>
> -/* note:
> +/*
> + * note:
> * ideally we would handle the scale attributes via the iio_info
> * (read|write)_raw methods, however this part is a untypical since we
> * don't create dedicated sysfs channel attributes for out0 and in0.
> @@ -572,7 +572,8 @@ static int ad5933_ring_postenable(struct iio_dev
> *indio_dev)
> {
> struct ad5933_state *st = iio_priv(indio_dev);
>
> - /* AD5933_CTRL_INIT_START_FREQ:
> + /*
> + * AD5933_CTRL_INIT_START_FREQ:
>* High Q complex circuits require a long time to reach steady state.
>* To facilitate the measurement of such impedances, this mode allows
>* the user full control of the settling time requirement before
> @@ -663,7 +664,8 @@ static void ad5933_work(struct work_struct *work)
> }
>
> if (status & AD5933_STAT_SWEEP_DONE) {
> - /* last sample received - power down do
> + /*
> + * last sample received - power down do
>* nothing until the ring enable is toggled
>*/
> ad5933_cmd(st, AD5933_CTRL_POWER_DOWN);
Re: [PATCH v2 2/4] staging: iio: ad5933: change help rule message
On Thu, 28 Feb 2019 23:52:58 -0300 Marcelo Schmitt wrote: > Remove the previous comment about direct access via sysfs which would > lead one think ad5933 driver has limitations it actually doesn't. > > Signed-off-by: Marcelo Schmitt This is fine. Will pick up once the rest of the series is ready. Jonathan > --- > drivers/staging/iio/impedance-analyzer/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/impedance-analyzer/Kconfig > b/drivers/staging/iio/impedance-analyzer/Kconfig > index dd97b6bb3fd0..b9a679cdd146 100644 > --- a/drivers/staging/iio/impedance-analyzer/Kconfig > +++ b/drivers/staging/iio/impedance-analyzer/Kconfig > @@ -10,7 +10,7 @@ config AD5933 > select IIO_KFIFO_BUF > help > Say yes here to build support for Analog Devices Impedance Converter, > - Network Analyzer, AD5933/4, provides direct access via sysfs. > + Network Analyzer, AD5933/4. > > To compile this driver as a module, choose M here: the > module will be called ad5933.
Re: [PATCH v2 3/4] staging: iio: ad5933: add ABI documentation
On Thu, 28 Feb 2019 23:53:14 -0300 Marcelo Schmitt wrote: > Add an ABI documentation for the ad5933 driver. > > Signed-off-by: Marcelo Schmitt Hi Marcelo, The ABI that you have defined which is actually new is mostly fine, however it seems that some of the existing ABI is not used correctly and that will need to be fixed. Jonathan > --- > .../ABI/testing/sysfs-bus-iio-ad5933 | 91 +++ > 1 file changed, 91 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-ad5933 > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > b/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > new file mode 100644 > index ..81e3d3f6f724 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-ad5933 > @@ -0,0 +1,91 @@ > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_scale > +Date:February 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + The output peak-to-peak voltage range. Writting 0 sets range > + to 2.0V p-p typical, 1 sets range to 200mV p-p typical, 2 sets > + range to 400mV p-p typical, 3 sets range to 1.0V p-p typical. > + The p-p value of the ac output exitation voltage scales with > + supply voltage according to the following formula: > + Output Excitation Voltage (V p-p) = normalized_3v3 × [VDD/3.3] > + where normalized_3v3 is one of the four voltage range above and > + VDD is the supply voltage. Definitely not. The device must comply with standard ABI and this is not using it correctly (see below or the docs). > + > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_scale_available > +Date:February 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + Prints available peak-to-peak voltage range to buffer. I really hope it doesn't. Scale is a mulitplier not a range. It should be printing out the value by which the _raw reading should be multiplied to get the voltage in the base units for voltage (millivolts) > + > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_start > +Date:February 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + The start frequency. Set this to define de frequency point at > + which the device should start the next frequency sweep. Default > + start frequency point set to 1Hz. No need to specify the default. It's trivial to read and any user of this chip should set it to the value they want. > + > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_increment > +Date:February 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + The frequency sweep increment. Set this to define at which rate > + frequency sweep points are incremented. Rate is a temporal term. Perhaps "step by which the frequency is incremented"? > After the measurement at > + a frequency point is completed, the next measurement will be > + made with a frequency 'frequency increment'Hz higher than the > + previous point until the defined number of increments has been > + made. Default frequency increment set to 200Hz. No need to specify the default. People can just read the file to find that out. > + > + > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_freq_points > +Date:February 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + The number of increments. This defines the number of frequency > + points in the frequency sweep. Device stores a 9-bit integer > + number in binary format for this so the maximum number of > + increments that can be programmed is 511. I would relax this ABI description so that it doesn't describe the bit depth and ideally doesn't describe the range either. If we want to provide the range, then provide an available attribute to pair with this. The reason is that we should be looking to define interfaces in a way that allows them to be potentially generalised to similar devices in future. I suspect a lot of this is generic to impedance measuring ICs. > + > +What:/sys/bus/iio/devices/iio:deviceX/out_voltage0_settling_cycles > +Date:February 2019 > +KernelVersion: Kernel 4.19 > +Contact: linux-...@vger.kernel.org > +Description: > + Number of settling time cycles. This attribute is a 11 bit field > + divided in two parts. The 9 least significant bit define the > + number of output excitation cycles that are passed through the > + unknown impedance, after the receipt of a start frequency sweep, > +
Re: [PATCH v2 4/4] staging: iio: ad5933: move out of staging
On Thu, 28 Feb 2019 23:53:31 -0300 Marcelo Schmitt wrote: > Move ad5933 impedance-analyzer driver from staging to mainline. > > The ad5933 is a high precision impedance converter system solution that > combines an on-board frequency generator with an analog-to-digital > converter (ADC). This driver was designed to be compatible with both > ad5933 and ad5934 chips. > > Signed-off-by: Marcelo Schmitt Hi Marcelo. There seems to still be substantial changes needed related to the ABI, so I'm not going to review the final state until those are done. Do you have one of these to test? Or can we arrange for someone else to test the result? It's rather easy for complex rework like you are doing (or going to do) to have side effects that review might not catch - so I'd definitely like to know the end result has been tested on the actual hardware! Thanks for those docs. I suspected they would throw up some 'interesting' issues around the ABI. It tends to happen with these older drivers because we hadn't finished fully refining the interfaces when Michael was developing this! Jonathan > --- > drivers/iio/impedance-analyzer/Kconfig| 18 + > drivers/iio/impedance-analyzer/Makefile | 5 + > drivers/iio/impedance-analyzer/ad5933.c | 811 ++ > .../staging/iio/impedance-analyzer/Kconfig| 18 - > .../staging/iio/impedance-analyzer/Makefile | 5 - > .../staging/iio/impedance-analyzer/ad5933.c | 811 -- > 6 files changed, 834 insertions(+), 834 deletions(-) > create mode 100644 drivers/iio/impedance-analyzer/Kconfig > create mode 100644 drivers/iio/impedance-analyzer/Makefile > create mode 100644 drivers/iio/impedance-analyzer/ad5933.c > delete mode 100644 drivers/staging/iio/impedance-analyzer/Kconfig > delete mode 100644 drivers/staging/iio/impedance-analyzer/Makefile > delete mode 100644 drivers/staging/iio/impedance-analyzer/ad5933.c > > diff --git a/drivers/iio/impedance-analyzer/Kconfig > b/drivers/iio/impedance-analyzer/Kconfig > new file mode 100644 > index ..b9a679cdd146 > --- /dev/null > +++ b/drivers/iio/impedance-analyzer/Kconfig > @@ -0,0 +1,18 @@ > +# > +# Impedance Converter, Network Analyzer drivers > +# > +menu "Network Analyzer, Impedance Converters" > + > +config AD5933 > + tristate "Analog Devices AD5933, AD5934 driver" > + depends on I2C > + select IIO_BUFFER > + select IIO_KFIFO_BUF > + help > + Say yes here to build support for Analog Devices Impedance Converter, > + Network Analyzer, AD5933/4. > + > + To compile this driver as a module, choose M here: the > + module will be called ad5933. > + > +endmenu > diff --git a/drivers/iio/impedance-analyzer/Makefile > b/drivers/iio/impedance-analyzer/Makefile > new file mode 100644 > index ..7604d786583e > --- /dev/null > +++ b/drivers/iio/impedance-analyzer/Makefile > @@ -0,0 +1,5 @@ > +# > +# Makefile for Impedance Converter, Network Analyzer drivers > +# > + > +obj-$(CONFIG_AD5933) += ad5933.o > diff --git a/drivers/iio/impedance-analyzer/ad5933.c > b/drivers/iio/impedance-analyzer/ad5933.c > new file mode 100644 > index ..d75bdfbf93de > --- /dev/null > +++ b/drivers/iio/impedance-analyzer/ad5933.c > @@ -0,0 +1,811 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * AD5933 AD5934 Impedance Converter, Network Analyzer > + * > + * Copyright 2011 Analog Devices Inc. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > + > +/* AD5933/AD5934 Registers */ > +#define AD5933_REG_CONTROL_HB0x80/* R/W, 1 byte */ > +#define AD5933_REG_CONTROL_LB0x81/* R/W, 1 byte */ > +#define AD5933_REG_FREQ_START0x82/* R/W, 3 bytes */ > +#define AD5933_REG_FREQ_INC 0x85/* R/W, 3 bytes */ > +#define AD5933_REG_INC_NUM 0x88/* R/W, 2 bytes, 9 bit */ > +#define AD5933_REG_SETTLING_CYCLES 0x8A/* R/W, 2 bytes */ > +#define AD5933_REG_STATUS0x8F/* R, 1 byte */ > +#define AD5933_REG_TEMP_DATA 0x92/* R, 2 bytes*/ > +#define AD5933_REG_REAL_DATA 0x94/* R, 2 bytes*/ > +#define AD5933_REG_IMAG_DATA 0x96/* R, 2 bytes*/ > + > +/* AD5933_REG_CONTROL_HB Bits */ > +#define AD5933_CTRL_INIT_START_FREQ (0x1 << 4) > +#define AD5933_CTRL_START_SWEEP (0x2 << 4) > +#define AD5933_CTRL_INC_FREQ (0x3 << 4) > +#define AD5933_CTRL_REPEAT_FREQ (0x4 << 4) > +#define AD5933_CTRL_MEASURE_TEMP (0x9 << 4) > +#define AD5933_CTRL_POWER_DOWN (0xA << 4) > +#define AD5933_CTRL_STANDBY (0xB << 4) > + > +#define AD5933_CTRL_RANGE_2000mVpp (0x0 << 1) > +#define AD5933_CTRL_RANGE_200mVpp(0x1 << 1) > +#define AD5933_CTRL_RANGE_400mVpp(0x2 << 1) > +#define AD5933_CTRL_RANGE_1000mVpp (0x3 <<
Gooday To You,
Gooday To You, Please i need your kind Assistance. I will be very glad if you can assist me to receive this sum of ( $22. Million US dollars.) into your bank account for the benefit of our both families, reply me if you are ready to receive this fund.
Gooday To You,
Gooday To You, Please i need your kind Assistance. I will be very glad if you can assist me to receive this sum of ( $22. Million US dollars.) into your bank account for the benefit of our both families, reply me if you are ready to receive this fund.
Gooday To You,
Gooday To You, Please i need your kind Assistance. I will be very glad if you can assist me to receive this sum of ( $22. Million US dollars.) into your bank account for the benefit of our both families, reply me if you are ready to receive this fund.
Gooday To You,
Gooday To You, Please i need your kind Assistance. I will be very glad if you can assist me to receive this sum of ( $22. Million US dollars.) into your bank account for the benefit of our both families, reply me if you are ready to receive this fund.
[PATCH] arm64: dts: rockchip: decrease rising edge time of UART2
This patch increases drive strength of UART2 from 3mA to 12mA for
getting more faster rising edge.
RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
this setting, a bit width of UART is about 667ns.
In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
converter), falling time of RockPro64 UART2 is 40ns, but riging time
is over 650ns. So UART receiver will get wrong data, because receiver
read intermediate data of rising edge.
Rising time becomes 300ns from 650ns if apply this patch. This is not
perfect solution but better than now.
Signed-off-by: Katsuhiro Suzuki
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index beaa92744a64..e3c8f91ead50 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2000,6 +2000,11 @@
drive-strength = <8>;
};
+ pcfg_pull_up_12ma: pcfg-pull-up-12ma {
+ bias-pull-up;
+ drive-strength = <12>;
+ };
+
pcfg_pull_up_18ma: pcfg-pull-up-18ma {
bias-pull-up;
drive-strength = <18>;
@@ -2521,8 +2526,8 @@
uart2c {
uart2c_xfer: uart2c-xfer {
rockchip,pins =
- <4 RK_PC3 RK_FUNC_1 &pcfg_pull_up>,
- <4 RK_PC4 RK_FUNC_1 &pcfg_pull_none>;
+ <4 RK_PC3 RK_FUNC_1 &pcfg_pull_up_12ma>,
+ <4 RK_PC4 RK_FUNC_1
&pcfg_pull_none_12ma>;
};
};
--
2.20.1
[PATCH] docs: add extra integer types to printk-formats
A few commonly used integer types were absent from this table, so add them. Link: https://github.com/ClangBuiltLinux/linux/issues/378 Suggested-by: Nick Desaulniers Signed-off-by: Louis Taylor --- Documentation/core-api/printk-formats.rst | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/core-api/printk-formats.rst b/Documentation/core-api/printk-formats.rst index a7fae4538946..6f08b1b6240a 100644 --- a/Documentation/core-api/printk-formats.rst +++ b/Documentation/core-api/printk-formats.rst @@ -19,8 +19,16 @@ Integer types unsigned long %lu or %lx long long %lld or %llx unsigned long long %llu or %llx + short int %hd or %hx + unsigned short int %hu or %hx + char%hhd or %hhx + unsigned char %hhu or %hhx size_t %zu or %zx ssize_t %zd or %zx + s8 %hhd or %hhx + u8 %hhu or %hhx + s16 %hd or %hx + u16 %hu or %hx s32 %d or %x u32 %u or %x s64 %lld or %llx -- 2.20.1
Re: [PATCH v3 1/7] iio: imu: adis16480: Add support for configurable drdy indicator
On Wed, 27 Feb 2019 18:14:22 +0200
Stefan Popa wrote:
> The FNCTIO_CTRL register provides configuration control for each I/O pin
> (DIO1, DIO2, DIO3 and DIO4).
>
> This patch adds the option to configure each DIOx pin as data ready
> indicator with positive or negative polarity by reading the 'interrupts'
> and 'interrupt-names' properties from the devicetree. The
> 'interrupt-names' property is optional, if it is not specified, then the
> DIO1 pin is used as default data ready signal.
>
> Although the factory default assigns DIO2 as data ready signal, in the
> versions previous this patch, DIO1 pin was used. We should leave this
> configuration as is, since some devices might be expecting the interrupt
> on the wrong physical pin.
>
> Signed-off-by: Stefan Popa
Nice detailed description.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/imu/adis16480.c | 97
> -
> 1 file changed, 95 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index a27fe20..98a23ac 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -9,6 +9,8 @@
> *
> */
>
> +#include
> +#include
> #include
> #include
> #include
> @@ -107,6 +109,14 @@
> #define ADIS16480_FIR_COEF_C(x)
> ADIS16480_FIR_COEF(0x09, (x))
> #define ADIS16480_FIR_COEF_D(x)
> ADIS16480_FIR_COEF(0x0B, (x))
>
> +/* ADIS16480_REG_FNCTIO_CTRL */
> +#define ADIS16480_DRDY_SEL_MSK GENMASK(1, 0)
> +#define ADIS16480_DRDY_SEL(x)
> FIELD_PREP(ADIS16480_DRDY_SEL_MSK, x)
> +#define ADIS16480_DRDY_POL_MSK BIT(2)
> +#define ADIS16480_DRDY_POL(x)
> FIELD_PREP(ADIS16480_DRDY_POL_MSK, x)
> +#define ADIS16480_DRDY_EN_MSKBIT(3)
> +#define ADIS16480_DRDY_EN(x) FIELD_PREP(ADIS16480_DRDY_EN_MSK, x)
> +
> struct adis16480_chip_info {
> unsigned int num_channels;
> const struct iio_chan_spec *channels;
> @@ -116,12 +126,26 @@ struct adis16480_chip_info {
> unsigned int accel_max_scale;
> };
>
> +enum adis16480_int_pin {
> + ADIS16480_PIN_DIO1,
> + ADIS16480_PIN_DIO2,
> + ADIS16480_PIN_DIO3,
> + ADIS16480_PIN_DIO4
> +};
> +
> struct adis16480 {
> const struct adis16480_chip_info *chip_info;
>
> struct adis adis;
> };
>
> +static const char * const adis16480_int_pin_names[4] = {
> + [ADIS16480_PIN_DIO1] = "DIO1",
> + [ADIS16480_PIN_DIO2] = "DIO2",
> + [ADIS16480_PIN_DIO3] = "DIO3",
> + [ADIS16480_PIN_DIO4] = "DIO4",
> +};
> +
> #ifdef CONFIG_DEBUG_FS
>
> static ssize_t adis16480_show_firmware_revision(struct file *file,
> @@ -741,8 +765,17 @@ static int adis16480_stop_device(struct iio_dev
> *indio_dev)
>
> static int adis16480_enable_irq(struct adis *adis, bool enable)
> {
> - return adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL,
> - enable ? BIT(3) : 0);
> + uint16_t val;
> + int ret;
> +
> + ret = adis_read_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, &val);
> + if (ret < 0)
> + return ret;
> +
> + val &= ~ADIS16480_DRDY_EN_MSK;
> + val |= ADIS16480_DRDY_EN(enable);
> +
> + return adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, val);
> }
>
> static int adis16480_initial_setup(struct iio_dev *indio_dev)
> @@ -826,6 +859,62 @@ static const struct adis_data adis16480_data = {
> .enable_irq = adis16480_enable_irq,
> };
>
> +static int adis16480_config_irq_pin(struct device_node *of_node,
> + struct adis16480 *st)
> +{
> + struct irq_data *desc;
> + enum adis16480_int_pin pin;
> + unsigned int irq_type;
> + uint16_t val;
> + int i, irq = 0;
> +
> + desc = irq_get_irq_data(st->adis.spi->irq);
> + if (!desc) {
> + dev_err(&st->adis.spi->dev, "Could not find IRQ %d\n", irq);
> + return -EINVAL;
> + }
> +
> + /* Disable data ready since the default after reset is on */
> + val = ADIS16480_DRDY_EN(0);
> +
> + /*
> + * Get the interrupt from the devicetre by reading the interrupt-names
> + * property. If it is not specified, use DIO1 pin as default.
> + * According to the datasheet, the factory default assigns DIO2 as data
> + * ready signal. However, in the previous versions of the driver, DIO1
> + * pin was used. So, we should leave it as is since some devices might
> + * be expecting the interrupt on the wrong physical pin.
> + */
> + pin = ADIS16480_PIN_DIO1;
> + for (i = 0; i < ARRAY_SIZE(adis16480_int_pin_names); i++) {
> + irq = of_irq_get_byname(of_node, adis16480_int_pin_names[i]);
> + if (irq > 0) {
> + pin = i;
> + break;
> + }
> + }
> +
> + val |=
Re: [PATCH v3 2/7] iio: imu: adis16480: Add OF device ID table
On Wed, 27 Feb 2019 18:14:23 +0200
Stefan Popa wrote:
> The driver does not have a struct of_device_id table, but supported
> devices are registered via Device Trees. This patch adds OF device ID
> table.
>
> Signed-off-by: Stefan Popa
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/imu/adis16480.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index 98a23ac..150d814 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -991,9 +991,19 @@ static const struct spi_device_id adis16480_ids[] = {
> };
> MODULE_DEVICE_TABLE(spi, adis16480_ids);
>
> +static const struct of_device_id adis16480_of_match[] = {
> + { .compatible = "adi,adis16375" },
> + { .compatible = "adi,adis16480" },
> + { .compatible = "adi,adis16485" },
> + { .compatible = "adi,adis16488" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, adis16480_of_match);
> +
> static struct spi_driver adis16480_driver = {
> .driver = {
> .name = "adis16480",
> + .of_match_table = adis16480_of_match,
> },
> .id_table = adis16480_ids,
> .probe = adis16480_probe,
Re: [PATCH v3 3/7] iio: imu: adis16480: Treat temperature scale in a generic way
On Wed, 27 Feb 2019 18:14:24 +0200
Stefan Popa wrote:
> All supported devices provide internal temperature measurement from -40 C
> to +85 C, with +25 C representing value 0x00.
>
> This patch treats the temperature scale in a generic way, similar to the
> accelerometer and gyroscope scales. So far, there are no temperature max
> scale differences between the supported devices. However, devices that
> will make use of this feature will be added in the future.
>
> Signed-off-by: Stefan Popa
Applied. Thanks,
Jonathan
> ---
> drivers/iio/imu/adis16480.c | 18 +++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index 150d814..5a2864a 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -124,6 +124,7 @@ struct adis16480_chip_info {
> unsigned int gyro_max_scale;
> unsigned int accel_max_val;
> unsigned int accel_max_scale;
> + unsigned int temp_scale;
> };
>
> enum adis16480_int_pin {
> @@ -530,6 +531,7 @@ static int adis16480_read_raw(struct iio_dev *indio_dev,
> const struct iio_chan_spec *chan, int *val, int *val2, long info)
> {
> struct adis16480 *st = iio_priv(indio_dev);
> + unsigned int temp;
>
> switch (info) {
> case IIO_CHAN_INFO_RAW:
> @@ -549,8 +551,13 @@ static int adis16480_read_raw(struct iio_dev *indio_dev,
> *val2 = 100; /* 0.0001 gauss */
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_TEMP:
> - *val = 5;
> - *val2 = 65; /* 5.65 milli degree Celsius */
> + /*
> + * +85 degrees Celsius = temp_max_scale
> + * +25 degrees Celsius = 0
> + * LSB, 25 degrees Celsius = 60 / temp_max_scale
> + */
> + *val = st->chip_info->temp_scale / 1000;
> + *val2 = (st->chip_info->temp_scale % 1000) * 1000;
> return IIO_VAL_INT_PLUS_MICRO;
> case IIO_PRESSURE:
> *val = 0;
> @@ -561,7 +568,8 @@ static int adis16480_read_raw(struct iio_dev *indio_dev,
> }
> case IIO_CHAN_INFO_OFFSET:
> /* Only the temperature channel has a offset */
> - *val = 4425; /* 25 degree Celsius = 0x */
> + temp = 25 * 100LL; /* 25 degree Celsius = 0x */
> + *val = DIV_ROUND_CLOSEST_ULL(temp, st->chip_info->temp_scale);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_CALIBBIAS:
> return adis16480_get_calibbias(indio_dev, chan, val);
> @@ -717,6 +725,7 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .gyro_max_scale = 300,
> .accel_max_val = IIO_M_S_2_TO_G(21973),
> .accel_max_scale = 18,
> + .temp_scale = 5650, /* 5.65 milli degree Celsius */
> },
> [ADIS16480] = {
> .channels = adis16480_channels,
> @@ -725,6 +734,7 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .gyro_max_scale = 450,
> .accel_max_val = IIO_M_S_2_TO_G(12500),
> .accel_max_scale = 10,
> + .temp_scale = 5650, /* 5.65 milli degree Celsius */
> },
> [ADIS16485] = {
> .channels = adis16485_channels,
> @@ -733,6 +743,7 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .gyro_max_scale = 450,
> .accel_max_val = IIO_M_S_2_TO_G(2),
> .accel_max_scale = 5,
> + .temp_scale = 5650, /* 5.65 milli degree Celsius */
> },
> [ADIS16488] = {
> .channels = adis16480_channels,
> @@ -741,6 +752,7 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .gyro_max_scale = 450,
> .accel_max_val = IIO_M_S_2_TO_G(22500),
> .accel_max_scale = 18,
> + .temp_scale = 5650, /* 5.65 milli degree Celsius */
> },
> };
>
Re: [PATCH v4 2/6] uaccess: Use user_access_ok() in user_access_begin()
Hi Masami,
I love your patch! Yet something to improve:
[auto build test ERROR on tip/perf/core]
[also build test ERROR on v5.0-rc8 next-20190301]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Masami-Hiramatsu/tracing-probes-uaccess-Add-support-user-space-access/20190303-150015
config: riscv-defconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 8.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=riscv
All errors (new ones prefixed by >>):
In file included from arch/riscv/include/asm/bug.h:75,
from include/linux/bug.h:5,
from include/linux/thread_info.h:12,
from lib/strncpy_from_user.c:5:
lib/strncpy_from_user.c: In function 'strncpy_from_user':
>> arch/riscv/include/asm/uaccess.h:42:19: error: 'TASK_SIZE' undeclared (first
>> use in this function); did you mean 'TASK_SIZE_OF'?
#define USER_DS (TASK_SIZE)
^
include/asm-generic/bug.h:148:27: note: in definition of macro 'WARN_ON_ONCE'
int __ret_warn_once = !!(condition); \
^
include/linux/uaccess.h:26:16: note: in expansion of macro 'segment_eq'
WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)); \
^~
include/linux/uaccess.h:26:37: note: in expansion of macro 'USER_DS'
WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)); \
^~~
include/linux/uaccess.h:285:37: note: in expansion of macro 'user_access_ok'
#define user_access_begin(ptr, len) user_access_ok(ptr, len)
^~
lib/strncpy_from_user.c:117:7: note: in expansion of macro
'user_access_begin'
if (user_access_begin(src, max)) {
^
arch/riscv/include/asm/uaccess.h:42:19: note: each undeclared identifier is
reported only once for each function it appears in
#define USER_DS (TASK_SIZE)
^
include/asm-generic/bug.h:148:27: note: in definition of macro 'WARN_ON_ONCE'
int __ret_warn_once = !!(condition); \
^
include/linux/uaccess.h:26:16: note: in expansion of macro 'segment_eq'
WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)); \
^~
include/linux/uaccess.h:26:37: note: in expansion of macro 'USER_DS'
WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)); \
^~~
include/linux/uaccess.h:285:37: note: in expansion of macro 'user_access_ok'
#define user_access_begin(ptr, len) user_access_ok(ptr, len)
^~
lib/strncpy_from_user.c:117:7: note: in expansion of macro
'user_access_begin'
if (user_access_begin(src, max)) {
^
--
In file included from arch/riscv/include/asm/bug.h:75,
from include/linux/bug.h:5,
from arch/riscv/include/asm/current.h:21,
from include/linux/sched.h:12,
from include/linux/uaccess.h:5,
from lib/strnlen_user.c:4:
lib/strnlen_user.c: In function 'strnlen_user':
>> arch/riscv/include/asm/uaccess.h:42:19: error: 'TASK_SIZE' undeclared (first
>> use in this function); did you mean 'TASK_SIZE_OF'?
#define USER_DS (TASK_SIZE)
^
include/asm-generic/bug.h:148:27: note: in definition of macro 'WARN_ON_ONCE'
int __ret_warn_once = !!(condition); \
^
include/linux/uaccess.h:26:16: note: in expansion of macro 'segment_eq'
WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)); \
^~
include/linux/uaccess.h:26:37: note: in expansion of macro 'USER_DS'
WARN_ON_ONCE(!segment_eq(get_fs(), USER_DS)); \
^~~
include/linux/uaccess.h:285:37: note: in expansion of macro 'user_access_ok'
#define user_access_begin(ptr, len) user_access_ok(ptr, len)
^~
lib/strnlen_user.c:117:7: note: in expansion of macro 'user_access_begin'
if (user_access_begin(str, max)) {
^
arch/riscv/include/asm/uaccess.h:42:19: note: each undeclared identifier is
reported only once for each function it appears in
#define USER_DS (TASK_SIZE)
Re: [PATCH v3 4/7] iio: imu: adis16480: Calculate the sampling frequency in a generic way
On Wed, 27 Feb 2019 18:14:25 +0200
Stefan Popa wrote:
> The adis1648x devices have an internal clock of 2.46 kSPS. The sampling
> frequency is calculated by applying a decimation rate which can take the
> maximum value of 2047.
>
> Although all adis1648x devices are similar in this regard, devices that
> will use this feature will be added in the future.
>
> Signed-off-by: Stefan Popa
Straight forward refactor so fine.
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
A comment inline about something in the old code!
Thanks,
Jonathan
> ---
> drivers/iio/imu/adis16480.c | 18 ++
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index 5a2864a..92abc95 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -125,6 +125,8 @@ struct adis16480_chip_info {
> unsigned int accel_max_val;
> unsigned int accel_max_scale;
> unsigned int temp_scale;
> + unsigned int int_clk;
> + unsigned int max_dec_rate;
> };
>
> enum adis16480_int_pin {
> @@ -299,9 +301,9 @@ static int adis16480_set_freq(struct iio_dev *indio_dev,
> int val, int val2)
> if (t <= 0)
> return -EINVAL;
>
> - t = 246 / t;
> - if (t > 2048)
> - t = 2048;
> + t = st->chip_info->int_clk / t;
> + if (t > st->chip_info->max_dec_rate)
> + t = st->chip_info->max_dec_rate;
>
> if (t != 0)
> t--;
> @@ -320,7 +322,7 @@ static int adis16480_get_freq(struct iio_dev *indio_dev,
> int *val, int *val2)
> if (ret < 0)
> return ret;
>
> - freq = 246 / (t + 1);
> + freq = st->chip_info->int_clk / (t + 1);
I'm a little curious about why t + 1? Presumably to avoid weird rounding
issues, but maybe a nice addition would be a comment explaining this.
> *val = freq / 1000;
> *val2 = (freq % 1000) * 1000;
>
> @@ -726,6 +728,8 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .accel_max_val = IIO_M_S_2_TO_G(21973),
> .accel_max_scale = 18,
> .temp_scale = 5650, /* 5.65 milli degree Celsius */
> + .int_clk = 246,
> + .max_dec_rate = 2048,
> },
> [ADIS16480] = {
> .channels = adis16480_channels,
> @@ -735,6 +739,8 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .accel_max_val = IIO_M_S_2_TO_G(12500),
> .accel_max_scale = 10,
> .temp_scale = 5650, /* 5.65 milli degree Celsius */
> + .int_clk = 246,
> + .max_dec_rate = 2048,
> },
> [ADIS16485] = {
> .channels = adis16485_channels,
> @@ -744,6 +750,8 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .accel_max_val = IIO_M_S_2_TO_G(2),
> .accel_max_scale = 5,
> .temp_scale = 5650, /* 5.65 milli degree Celsius */
> + .int_clk = 246,
> + .max_dec_rate = 2048,
> },
> [ADIS16488] = {
> .channels = adis16480_channels,
> @@ -753,6 +761,8 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .accel_max_val = IIO_M_S_2_TO_G(22500),
> .accel_max_scale = 18,
> .temp_scale = 5650, /* 5.65 milli degree Celsius */
> + .int_clk = 246,
> + .max_dec_rate = 2048,
> },
> };
>
Re: [PATCH v3 5/7] iio: imu: adis16480: Deal with filter freq in a generic way
On Wed, 27 Feb 2019 18:14:26 +0200
Stefan Popa wrote:
> When setting the filter frequency, the driver looks into the
> adis16480_def_filter_freqs table for the best match. Pass this table to
> the chip_info struct since future devices will need to use a different
> table.
>
> Signed-off-by: Stefan Popa
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/imu/adis16480.c | 13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index 92abc95..c90375d 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -127,6 +127,7 @@ struct adis16480_chip_info {
> unsigned int temp_scale;
> unsigned int int_clk;
> unsigned int max_dec_rate;
> + const unsigned int *filter_freqs;
> };
>
> enum adis16480_int_pin {
> @@ -483,7 +484,7 @@ static int adis16480_get_filter_freq(struct iio_dev
> *indio_dev,
> if (!(val & enable_mask))
> *freq = 0;
> else
> - *freq = adis16480_def_filter_freqs[(val >> offset) & 0x3];
> + *freq = st->chip_info->filter_freqs[(val >> offset) & 0x3];
>
> return IIO_VAL_INT;
> }
> @@ -510,10 +511,10 @@ static int adis16480_set_filter_freq(struct iio_dev
> *indio_dev,
> val &= ~enable_mask;
> } else {
> best_freq = 0;
> - best_diff = 310;
> + best_diff = st->chip_info->filter_freqs[0];
> for (i = 0; i < ARRAY_SIZE(adis16480_def_filter_freqs); i++) {
> - if (adis16480_def_filter_freqs[i] >= freq) {
> - diff = adis16480_def_filter_freqs[i] - freq;
> + if (st->chip_info->filter_freqs[i] >= freq) {
> + diff = st->chip_info->filter_freqs[i] - freq;
> if (diff < best_diff) {
> best_diff = diff;
> best_freq = i;
> @@ -730,6 +731,7 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .temp_scale = 5650, /* 5.65 milli degree Celsius */
> .int_clk = 246,
> .max_dec_rate = 2048,
> + .filter_freqs = adis16480_def_filter_freqs,
> },
> [ADIS16480] = {
> .channels = adis16480_channels,
> @@ -741,6 +743,7 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .temp_scale = 5650, /* 5.65 milli degree Celsius */
> .int_clk = 246,
> .max_dec_rate = 2048,
> + .filter_freqs = adis16480_def_filter_freqs,
> },
> [ADIS16485] = {
> .channels = adis16485_channels,
> @@ -752,6 +755,7 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .temp_scale = 5650, /* 5.65 milli degree Celsius */
> .int_clk = 246,
> .max_dec_rate = 2048,
> + .filter_freqs = adis16480_def_filter_freqs,
> },
> [ADIS16488] = {
> .channels = adis16480_channels,
> @@ -763,6 +767,7 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .temp_scale = 5650, /* 5.65 milli degree Celsius */
> .int_clk = 246,
> .max_dec_rate = 2048,
> + .filter_freqs = adis16480_def_filter_freqs,
> },
> };
>
From Mrs. Elia Rodrigues,
Hello I am Mrs. Elia Rodrigues I have been diagnosed with cancer. It has defied all forms of medical treatment, and right now I have only about a few months to live, according to medical experts. I have not particularly lived my life so well, as I never really cared for anyone (not even myself) but my business. Though I am very rich, I was never generous. I was always hostile to People and only focused on my business as that was the only thing I cared for. But now I regret all this as I now know that there is more to life than just wanting to have or make all the money in the world. I believe when God gives me a second chance to come to this world I would live my life a different way from how I have lived it. I would want to have a Personal and Trustworthy Relationship with you, as I intend and willing to empower the change of ownership for the transfer of my Deposits to your personal possession for Charity Disbursement to the Less Privilege and Homeless. I want this fund to be used in liberation work Activities like, help to Orphanages, and Christian schools, helping widows and up keeping of the Churches and propagating the word of God and to endeavor that the house of God is maintained. I am not afraid of death hence I know where I am going. I know that I am going to be in the bosom of the Lord. Exodus 14 vs 14 says that the lord will fight my case and I shall hold my peace. Because of my health condition when i try to speak it leads to a serious coughing out of blood. I will send you the photos of me, As soon as I receive your reply I shall give you the details on how to contact the bank directly for onward transfer. I will also issue you an authority letter that will prove you as the present beneficiary of my funds. I want you and your family to always pray for me but the lord is my shepherd. Thank you for your due consideration. God be with you Yours Sister in the Lord. Mrs. Elia Rodrigues
Re: [PATCH v3 6/7] iio: imu: adis16480: Add support for ADIS1649x family of devices
On Wed, 27 Feb 2019 18:14:27 +0200
Stefan Popa wrote:
> The ADIS16495 and ADIS16497 are inertial systems that include a triaxis
> gyroscope and a triaxis accelerometer. The serial peripheral interface
> (SPI) provide a simple interface for data collection and configuration
> control. The devices are similar to ADIS16475, ADIS16480, ADIS16485 and
> ADIS16488, the main differences are highlighted below:
>
> * The temperature data scale is 0.00565 C/LSB for ADIS16475 and ADIS1648x
> devices, while for ADIS1649x 0.0125 C/LSB.
>
> * ADIS1649x devices support different gyroscope measurement ranges which
> are dependent on the dash number (-1, -2, -3), see Table 24 in the
> ADIS16495 datasheet. However, the ADIS16497 gyroscopes have the same
> scale as ADIS16495.
>
> * ADIS16495 devices support the acceleration maximum range of 8g, while
> ADIS16497 devices go up to 40g.
>
> * The internal clock for ADIS1649x devices is 4.25 kSPS. The sampling
> frequency is calculated by applying a decimation rate which can take a
> maximum value of 4250.
>
> * ADIS1649x devices support different default filter frequencies.
>
> Datasheets:
> Link:
> https://www.analog.com/media/en/technical-documentation/data-sheets/adis16495.pdf
> Link:
> https://www.analog.com/media/en/technical-documentation/data-sheets/adis16497.pdf
>
> Signed-off-by: Stefan Popa
Applied. Thanks,
Jonathan
> ---
> drivers/iio/imu/adis16480.c | 97
> +
> 1 file changed, 97 insertions(+)
>
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index c90375d..28cece3 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -453,6 +453,13 @@ static const unsigned int adis16480_def_filter_freqs[] =
> {
> 63,
> };
>
> +static const unsigned int adis16495_def_filter_freqs[] = {
> + 300,
> + 100,
> + 300,
> + 100,
> +};
> +
> static const unsigned int ad16480_filter_data[][2] = {
> [ADIS16480_SCAN_GYRO_X] = { ADIS16480_REG_FILTER_BNK0, 0 },
> [ADIS16480_SCAN_GYRO_Y] = { ADIS16480_REG_FILTER_BNK0, 3 },
> @@ -713,6 +720,12 @@ enum adis16480_variant {
> ADIS16480,
> ADIS16485,
> ADIS16488,
> + ADIS16495_1,
> + ADIS16495_2,
> + ADIS16495_3,
> + ADIS16497_1,
> + ADIS16497_2,
> + ADIS16497_3,
> };
>
> static const struct adis16480_chip_info adis16480_chip_info[] = {
> @@ -769,6 +782,78 @@ static const struct adis16480_chip_info
> adis16480_chip_info[] = {
> .max_dec_rate = 2048,
> .filter_freqs = adis16480_def_filter_freqs,
> },
> + [ADIS16495_1] = {
> + .channels = adis16485_channels,
> + .num_channels = ARRAY_SIZE(adis16485_channels),
> + .gyro_max_val = IIO_RAD_TO_DEGREE(2),
> + .gyro_max_scale = 125,
> + .accel_max_val = IIO_M_S_2_TO_G(32000),
> + .accel_max_scale = 8,
> + .temp_scale = 12500, /* 12.5 milli degree Celsius */
> + .int_clk = 425,
> + .max_dec_rate = 4250,
> + .filter_freqs = adis16495_def_filter_freqs,
> + },
> + [ADIS16495_2] = {
> + .channels = adis16485_channels,
> + .num_channels = ARRAY_SIZE(adis16485_channels),
> + .gyro_max_val = IIO_RAD_TO_DEGREE(18000),
> + .gyro_max_scale = 450,
> + .accel_max_val = IIO_M_S_2_TO_G(32000),
> + .accel_max_scale = 8,
> + .temp_scale = 12500, /* 12.5 milli degree Celsius */
> + .int_clk = 425,
> + .max_dec_rate = 4250,
> + .filter_freqs = adis16495_def_filter_freqs,
> + },
> + [ADIS16495_3] = {
> + .channels = adis16485_channels,
> + .num_channels = ARRAY_SIZE(adis16485_channels),
> + .gyro_max_val = IIO_RAD_TO_DEGREE(2),
> + .gyro_max_scale = 2000,
> + .accel_max_val = IIO_M_S_2_TO_G(32000),
> + .accel_max_scale = 8,
> + .temp_scale = 12500, /* 12.5 milli degree Celsius */
> + .int_clk = 425,
> + .max_dec_rate = 4250,
> + .filter_freqs = adis16495_def_filter_freqs,
> + },
> + [ADIS16497_1] = {
> + .channels = adis16485_channels,
> + .num_channels = ARRAY_SIZE(adis16485_channels),
> + .gyro_max_val = IIO_RAD_TO_DEGREE(2),
> + .gyro_max_scale = 125,
> + .accel_max_val = IIO_M_S_2_TO_G(32000),
> + .accel_max_scale = 40,
> + .temp_scale = 12500, /* 12.5 milli degree Celsius */
> + .int_clk = 425,
> + .max_dec_rate = 4250,
> + .filter_freqs = adis16495_def_filter_freqs,
> + },
> + [ADIS16497_2] = {
> + .channels = adis16485_channels,
> + .num_channels = ARRAY_SIZE(adis16485_channels),
> + .gyro_ma
Re: [PATCH v3 7/7] iio: imu: adis16480: Add docs for ADIS16480 IMU
On Wed, 27 Feb 2019 18:14:28 +0200
Stefan Popa wrote:
> Document support for ADIS16480 Inertial Measurement Unit.
>
> Signed-off-by: Stefan Popa
> Reviewed-by: Rob Herring
Applied.
Thanks,
Jonathan
> ---
> .../devicetree/bindings/iio/imu/adi,adis16480.txt | 49
> ++
> MAINTAINERS| 1 +
> 2 files changed, 50 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/iio/imu/adi,adis16480.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/adi,adis16480.txt
> b/Documentation/devicetree/bindings/iio/imu/adi,adis16480.txt
> new file mode 100644
> index 000..39ab016
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/imu/adi,adis16480.txt
> @@ -0,0 +1,49 @@
> +
> +Analog Devices ADIS16480 and similar IMUs
> +
> +Required properties for the ADIS16480:
> +
> +- compatible: Must be one of
> + * "adi,adis16375"
> + * "adi,adis16480"
> + * "adi,adis16485"
> + * "adi,adis16488"
> + * "adi,adis16495-1"
> + * "adi,adis16495-2"
> + * "adi,adis16495-3"
> + * "adi,adis16497-1"
> + * "adi,adis16497-2"
> + * "adi,adis16497-3"
> +- reg: SPI chip select number for the device
> +- spi-max-frequency: Max SPI frequency to use
> + see: Documentation/devicetree/bindings/spi/spi-bus.txt
> +- spi-cpha: See Documentation/devicetree/bindings/spi/spi-bus.txt
> +- spi-cpol: See Documentation/devicetree/bindings/spi/spi-bus.txt
> +- interrupts: interrupt mapping for IRQ, accepted values are:
> + * IRQF_TRIGGER_RISING
> + * IRQF_TRIGGER_FALLING
> +
> +Optional properties:
> +
> +- interrupt-names: Data ready line selection. Valid values are:
> + * DIO1
> + * DIO2
> + * DIO3
> + * DIO4
> + If this field is left empty, DIO1 is assigned as default data ready
> + signal.
> +- reset-gpios: must be the device tree identifier of the RESET pin. As the
> line
> + is active low, it should be marked GPIO_ACTIVE_LOW.
> +
> +Example:
> +
> + imu@0 {
> + compatible = "adi,adis16495-1";
> + reg = <0>;
> + spi-max-frequency = <320>;
> + spi-cpol;
> + spi-cpha;
> + interrupts = <25 IRQF_TRIGGER_FALLING>;
> + interrupt-parent = <&gpio>;
> + interrupt-names = "DIO2";
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e4091ac..beecd1e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -942,6 +942,7 @@ F:drivers/dma/dma-axi-dmac.c
> ANALOG DEVICES INC IIO DRIVERS
> M: Lars-Peter Clausen
> M: Michael Hennerich
> +M: Stefan Popa
> W: http://wiki.analog.com/
> W: http://ez.analog.com/community/linux-device-drivers
> S: Supported
Re: [PATCH v5 1/2] iio:temperature:max31856:Add device tree bind info
On Thu, 28 Feb 2019 16:15:55 +0100
Patrick Havelange wrote:
> On Thu, Feb 28, 2019 at 12:59 AM Rob Herring wrote:
> >
> > On Tue, Feb 26, 2019 at 04:02:13PM +0100, Patrick Havelange wrote:
> > > From: Paresh Chaudhary
> > >
> > > This patch added device tree binding info for MAX31856 driver.
> > >
> > > Signed-off-by: Paresh Chaudhary
> > > Signed-off-by: Matt Weber
> > > Signed-off-by: Patrick Havelange
> > > ---
> > > Changes
> > > v1 -> v2
> > > [Matt
> > > - Removed comment block and added possibilities of
> > >thermocouple type in device tree binding doc.
> > >
> > > v2 -> v3
> > > - Rebased
> > >
> > > v3 -> v4
> > > - Removed one-shot property related information.
> > > - Used standard name 'temp-sensor'
> > >
> > > v4 -> v5
> > > [Patrick
> > > - Rename thermocouple type to maxim,thermocouple-type for DT entry
> > > ---
> > > .../bindings/iio/temperature/max31856.txt | 29 +++
> > > 1 file changed, 29 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/iio/temperature/max31856.txt
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/iio/temperature/max31856.txt
> > > b/Documentation/devicetree/bindings/iio/temperature/max31856.txt
> > > new file mode 100644
> > > index ..b4396069b8fa
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/temperature/max31856.txt
> > > @@ -0,0 +1,29 @@
> > > +Maxim MAX31856 thermocouple support
> > > +
> > > +https://datasheets.maximintegrated.com/en/ds/MAX31856.pdf
> > > +
> > > +Required properties:
> > > + - compatible: must be "maxim,max31856"
> > > + - reg: SPI chip select number for the device
> > > + - spi-max-frequency: As per datasheet max. supported freq is 500
> > > + - spi-cpha: must be defined for max31856 to enable SPI mode 1
> > > + - maxim,thermocouple-type: Type of thermocouple (By default is
> > > K-Type)
> > > + 0x00 : TYPE_B
> > > + 0x01 : TYPE_E
> > > + 0x02 : TYPE_J
> > > + 0x03 : TYPE_K (default)
> > > + 0x04 : TYPE_N
> > > + 0x05 : TYPE_R
> > > + 0x06 : TYPE_S
> > > + 0x07 : TYPE_T
> >
> > These appear to be standard types. Perhaps this should be a common
> > property instead?
>
> A remark on the v4 of the patch recommended to add a vendor prefix. It
> also mentioned that it could be done as a generic type with a
> translation layer for each driver.
> Maybe this generic type could be introduced in a separate patch, or
> when another driver also uses that kind of thermocouple-type, as there
> is no other use of it for the moment it seems.
Hmm. If Rob is keen, it might be good to define this standard now.
The big advantage for you is that you can define the numbers to involve
no transition layer for your device.
A quick look at wikipedia suggests there are additional types:
TYPE_M
TYPE_C
TYPE_G
TYPE_P
+ some weird types without a letter.
We have missed the coming merge window for this one so have
a bit of time. Perhaps you could propose such a generic binding?
For now that binding could just include the values you care about
and new ones can be added when needed.
We would want a dt header to give them defined names to help
with readability.
Apologies for making you go around again on this.
Jonathan
>
>
> >
> > > +
> > > + Refer to spi/spi-bus.txt for generic SPI slave bindings.
> > > +
> > > + Example:
> > > + temp-sensor@0 {
> > > + compatible = "maxim,max31856";
> > > + reg = <0>;
> > > + spi-max-frequency = <500>;
> > > + spi-cpha;
> > > + maxim,thermocouple-type = <0x03>;
> > > + };
> > > --
> > > 2.19.1
> > >
Re: [PATCH v5 2/2] iio:temperature: Add MAX31856 thermocouple support
On Tue, 26 Feb 2019 16:02:14 +0100
Patrick Havelange wrote:
> From: Paresh Chaudhary
>
> This patch adds support for Maxim MAX31856 thermocouple
> temperature sensor support.
>
> More information can be found in:
> https://www.maximintegrated.com/en/ds/MAX31856.pdf
>
> NOTE: Driver support only Comparator Mode.
>
> Signed-off-by: Paresh Chaudhary
> Signed-off-by: Matt Weber
> Signed-off-by: Patrick Havelange
One totally trivial comment inline.
I wish we had a more general error reporting mechanism, but until
that comes about (it's debated every few years but I haven't heard
anything recently) the sysfs ABI will have to do.
Anyhow, just holding this to let us sort out the DT binding for
the thermocouple probe type.
Thanks,
Jonathan
> ---
> Changes
> v1 -> v2
> [Peter
> 1. Fixed all space & 'return' related comments
> 2. Removed 'sysfs_create_group' api because
> iio_device_register function is handling sysfs entry
> 3. Return -EIO if there is any fault
> 4. Added check for 'read_size' before spi read call
> 5. Removed license text from the source file
> 6. Added .o file in alphabetic order
> 7. Used #defines instead of magic bits
>
> v2 -> v3
> [Jonathan
> 1. Used bool for fault_oc and fault_ovuv
> 2. Changed 'max31856_read' function and use byte array to
> store registers value.
> 3. Used 'GENMASK' where required
> 4. Changed logic 'max31856_thermocouple_read' function. Used
> array to read registers value.
> 5. Used 'devm_iio_device_register' and removed 'max31856_remove'.
> 6. Fixed other cosmetic changes.
> 7. Added 'sysfs-bus-iio-temperature-max31856' file and updated
> 'MAINTAINERS' file.
>
> v3 -> v4
> [Jonathan
> 1. Removed unwanted logic
> 2. Updated code to handle return value of max31856_read call
> 3. Added devicetree id table
> 4. Removed one-shot support from driver as this support was not
> implemented with correct design.
>
> v4 -> v5
> [Patrick
> 1. Rename thermocouple type to maxim,thermocouple-type for DT entry
> 2. Don't cache values from the Fault Status Register
> 3. Simplify a bit max31856_init()
> 4. Use IIO_NO_MOD in switch case + default error case
> 5. Use dev_*() instead of pr_*()
> 6. Fix missing space in comments
> 7. Removed iio_info.driver_module assignment as no longer present
> 8. Don't keep read/write buffer into the internal driver struct
> 9. Updated kernel version, add missing space in documentation
>10. Updated (c) year
>11. Removed linux/init.h #include
>12. More use of BIT() macro
>13. Removed iio_chan_spec.address assignment as not used
>14. In max31856_thermocouple_read(), same switch case order as
> channels definition
>15. Refactor show_fault_*() functions
>16. Use u8 as register type in max31856_{read,write}()
> ---
> .../sysfs-bus-iio-temperature-max31856| 23 ++
> drivers/iio/temperature/Kconfig | 10 +
> drivers/iio/temperature/Makefile | 1 +
> drivers/iio/temperature/max31856.c| 332 ++
> 4 files changed, 366 insertions(+)
> create mode 100644
> Documentation/ABI/testing/sysfs-bus-iio-temperature-max31856
> create mode 100644 drivers/iio/temperature/max31856.c
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-temperature-max31856
> b/Documentation/ABI/testing/sysfs-bus-iio-temperature-max31856
> new file mode 100644
> index ..22659c91262c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-temperature-max31856
> @@ -0,0 +1,23 @@
> +What:/sys/bus/iio/devices/iio:deviceX/fault_oc
> +KernelVersion: 5.1
> +Contact: linux-...@vger.kernel.org
> +Description:
> + Open-circuit fault. The detection of open-circuit faults,
> + such as those caused by broken thermocouple wires.
> + Reading returns either '1' or '0'.
> + '1' = An open circuit such as broken thermocouple wires
> + has been detected.
> + '0' = No open circuit or broken thermocouple wires are detected
> +
> +What:/sys/bus/iio/devices/iio:deviceX/fault_ovuv
> +KernelVersion: 5.1
> +Contact: linux-...@vger.kernel.org
> +Description:
> + Overvoltage or Undervoltage Input Fault. The internal circuitry
> + is protected from excessive voltages applied to the thermocouple
> + cables by integrated MOSFETs at the T+ and T- inputs, and the
> + BIAS output. These MOSFETs turn off when the input voltage is
> + negative or greater than VDD.
> + Reading returns either '1' or '0'.
> + '1' = The input voltage is negative or greater than VDD.
> + '0' = The input voltage is positive and less than VDD (default).
Re: [PATCH] docs: driver-api: iio: fix errors in documentation
On Mon, 25 Feb 2019 21:23:26 +0100
Tomasz Duszynski wrote:
> Improve IIO documentation by fixing a few mistakes.
>
> Signed-off-by: Tomasz Duszynski
Acked-by: Jonathan Cameron
Thanks,
Jonathan
> ---
> Documentation/driver-api/iio/buffers.rst | 2 +-
> Documentation/driver-api/iio/core.rst| 6 +++---
> Documentation/driver-api/iio/hw-consumer.rst | 2 +-
> Documentation/driver-api/iio/triggers.rst| 2 +-
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/driver-api/iio/buffers.rst
> b/Documentation/driver-api/iio/buffers.rst
> index 02c99a6bee18..e9036ef9f8f4 100644
> --- a/Documentation/driver-api/iio/buffers.rst
> +++ b/Documentation/driver-api/iio/buffers.rst
> @@ -26,7 +26,7 @@ IIO buffer setup
>
>
> The meta information associated with a channel reading placed in a buffer is
> -called a scan element . The important bits configuring scan elements are
> +called a scan element. The important bits configuring scan elements are
> exposed to userspace applications via the
> :file:`/sys/bus/iio/iio:device{X}/scan_elements/*` directory. This file
> contains
> attributes of the following form:
> diff --git a/Documentation/driver-api/iio/core.rst
> b/Documentation/driver-api/iio/core.rst
> index 9a34ae03b679..54f193edbf8b 100644
> --- a/Documentation/driver-api/iio/core.rst
> +++ b/Documentation/driver-api/iio/core.rst
> @@ -2,8 +2,8 @@
> Core elements
> =
>
> -The Industrial I/O core offers a unified framework for writing drivers for
> -many different types of embedded sensors. a standard interface to user space
> +The Industrial I/O core offers both a unified framework for writing drivers
> for
> +many different types of embedded sensors and a standard interface to user
> space
> applications manipulating sensors. The implementation can be found under
> :file:`drivers/iio/industrialio-*`
>
> @@ -11,7 +11,7 @@ Industrial I/O Devices
> --
>
> * struct :c:type:`iio_dev` - industrial I/O device
> -* :c:func:`iio_device_alloc()` - alocate an :c:type:`iio_dev` from a driver
> +* :c:func:`iio_device_alloc()` - allocate an :c:type:`iio_dev` from a driver
> * :c:func:`iio_device_free()` - free an :c:type:`iio_dev` from a driver
> * :c:func:`iio_device_register()` - register a device with the IIO subsystem
> * :c:func:`iio_device_unregister()` - unregister a device from the IIO
> diff --git a/Documentation/driver-api/iio/hw-consumer.rst
> b/Documentation/driver-api/iio/hw-consumer.rst
> index 8facce6a6733..e0fe0b98230e 100644
> --- a/Documentation/driver-api/iio/hw-consumer.rst
> +++ b/Documentation/driver-api/iio/hw-consumer.rst
> @@ -1,7 +1,7 @@
> ===
> HW consumer
> ===
> -An IIO device can be directly connected to another device in hardware. in
> this
> +An IIO device can be directly connected to another device in hardware. In
> this
> case the buffers between IIO provider and IIO consumer are handled by
> hardware.
> The Industrial I/O HW consumer offers a way to bond these IIO devices without
> software buffer for data. The implementation can be found under
> diff --git a/Documentation/driver-api/iio/triggers.rst
> b/Documentation/driver-api/iio/triggers.rst
> index f89d37e7dd82..5c2156de6284 100644
> --- a/Documentation/driver-api/iio/triggers.rst
> +++ b/Documentation/driver-api/iio/triggers.rst
> @@ -38,7 +38,7 @@ There are two locations in sysfs related to triggers:
>
> * :file:`/sys/bus/iio/devices/iio:device{X}/trigger/*`, this directory is
>created once the device supports a triggered buffer. We can associate a
> - trigger with our device by writing the trigger's name in the
> + trigger with our device by writing the trigger's name in the
>:file:`current_trigger` file.
>
> IIO trigger setup
> --
> 2.20.1
>
Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2
Hi,
Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:
> This patch increases drive strength of UART2 from 3mA to 12mA for
> getting more faster rising edge.
>
> RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
> this setting, a bit width of UART is about 667ns.
>
> In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
> converter), falling time of RockPro64 UART2 is 40ns, but riging time
> is over 650ns. So UART receiver will get wrong data, because receiver
> read intermediate data of rising edge.
>
> Rising time becomes 300ns from 650ns if apply this patch. This is not
> perfect solution but better than now.
>
> Signed-off-by: Katsuhiro Suzuki
> ---
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
your changing a core rk3399 property here, so I'd really like to get
input from other board stakeholders on this before applying a core
change.
Could you either include the submitters of other rk3399-boards in the
recipient list so that they're aware or limit the change to rockpro64 for
the time being (aka overriding the property in the board-dts) please?
Thanks
Heiko
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index beaa92744a64..e3c8f91ead50 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -2000,6 +2000,11 @@
> drive-strength = <8>;
> };
>
> + pcfg_pull_up_12ma: pcfg-pull-up-12ma {
> + bias-pull-up;
> + drive-strength = <12>;
> + };
> +
> pcfg_pull_up_18ma: pcfg-pull-up-18ma {
> bias-pull-up;
> drive-strength = <18>;
> @@ -2521,8 +2526,8 @@
> uart2c {
> uart2c_xfer: uart2c-xfer {
> rockchip,pins =
> - <4 RK_PC3 RK_FUNC_1 &pcfg_pull_up>,
> - <4 RK_PC4 RK_FUNC_1 &pcfg_pull_none>;
> + <4 RK_PC3 RK_FUNC_1 &pcfg_pull_up_12ma>,
> + <4 RK_PC4 RK_FUNC_1
> &pcfg_pull_none_12ma>;
> };
> };
>
>
Re: [PATCH v3 1/3] staging: iio: frequency: ad9834: Move frequency to standard iio types
On Mon, 25 Feb 2019 21:17:30 +0200 Beniamin Bia wrote: > Frequency attribute is added with a standard type from iio framework > instead of custom attribute. This is a small step towards removing any > unnecessary custom attribute. Ad9834 will diverge from ad9833 in the > future, that is why we have two identical arrays for ad9834 and 9833. > > Signed-off-by: Beniamin Bia What happened to patch 3? I definitely want wider discussion of the ABI in here if anyone has time to comment! Note that I would prefer to separate out the ABI as a separate patch to aid discussion in future series. It's fine to have it after the patches that implement it. > --- > Changed in v3: > -based on Jonathan suggestion, i replaced default option with > Ad9834 DeviceId > -added a local variable in frequency to simplify the code > -added ABI documentation > > .../testing/sysfs-bus-iio-frequency-ad9834| 129 ++ > drivers/staging/iio/frequency/ad9834.c| 104 +++--- > 2 files changed, 216 insertions(+), 17 deletions(-) > create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834 > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834 > b/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834 > new file mode 100644 > index ..b912b49473a3 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834 > @@ -0,0 +1,129 @@ > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltage0_frequency > +KernelVersion: 3.5.0 > +Date:April 2012 > +Contact: linux-...@vger.kernel.org > +Description: > + Represents the value from frequency register 0 of device. The > + value is between 0 and clock frequency / 2. > + Reading returns the value of frequency written in register 0. > + > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltage1_frequency > +KernelVersion: 3.5.0 > +Date:April 2012 > +Contact: linux-...@vger.kernel.org > +Description: > + Represents the value from frequency register 1 of device. The > + value is between 0 and clock frequency / 2. > + Reading returns the value of frequency written in register 1. Hmm. So this is the fundamental question of this ABI. Is it fine to represent two different possible outputs that are (sort of) muxed onto one wire as separate channels? FSK / PSK as relevant. It may be that we need /sys/bus/iio/devices/iio:deviceX/out_voltage0_symbol0_frequency /sys/bus/iio/devices/iio:deviceX/out_voltage0_symbol1_frequency for example to more cleanly represent this? Perhaps using modifiers to hammer this into the existing framework. > + > +What:/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_phase0 > +KernelVersion: 3.5.0 > +Date:April 2012 > +Contact: linux-...@vger.kernel.org > +Description: > + Represents the value from phase register 0 of device. The value > + is between 0 and 4096 rad. > + Reading returns the value of phase written in register 0. > + > +What:/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_phase1 > +KernelVersion: 3.5.0 > +Date:April 2012 > +Date:February 2019 > +Contact: linux-...@vger.kernel.org > +Description: > + Represents the value from phase register 1 of device. > + The value is between 0 and 4096 rad > + Reading returns the value of phase written in register 1. > + > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out0_wavetype_available > +KernelVersion: 3.5.0 > +Date:April 2012 > +Contact: linux-...@vger.kernel.org > +Description: > + Reading returns the possible waveform of output: > + sine - for a sinewave > + triangle - for a triangle signal > + square - squarewave, this is only available on ad9833/7 > + > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out0_wavetype > +KernelVersion: 3.5.0 > +Date:April 2012 > +Contact: linux-...@vger.kernel.org > +Description: > + Represents the output waveform of channel: > + sine - for a sinewave > + triangle - for a triangle signal > + square - squarewave, this is only available on ad9833/7 So these are different channels? out0 and out1 different pins? I am particularly confused given on the ad9834 they are IOUT and IOUTB and seem to track together. > + > +What: > /sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out1_wavetype_available > +KernelVersion: 3.5.0 > +Date:April 2012 > +Contact: linux-...@vger.kernel.org > +Description: > + Reading returns the possible waveform type for accumulator > + output: >
Re: [PATCH v3 2/3] staging: iio: frequency: ad9834: Move phase and scale to standard iio attribute
On Mon, 25 Feb 2019 21:17:31 +0200
Beniamin Bia wrote:
> The custom phase and scale attributes were moved to standard iio types.
>
> Signed-off-by: Beniamin Bia
> ---
> Changes in v3:
> -abi documentation added
>
> .../testing/sysfs-bus-iio-frequency-ad9834| 10 ++--
> drivers/staging/iio/frequency/ad9834.c| 53 +++
> 2 files changed, 38 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834
> b/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834
> index b912b49473a3..656aa5b6d22b 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834
> @@ -1,3 +1,5 @@
> +What:/sys/bus/iio/devices/iio:deviceX/out_altvoltage_scale
> +
> What:
> /sys/bus/iio/devices/iio:deviceX/out_altvoltage0_frequency
> KernelVersion: 3.5.0
> Date:April 2012
> @@ -16,7 +18,7 @@ Description:
> value is between 0 and clock frequency / 2.
> Reading returns the value of frequency written in register 1.
>
> -What:/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_phase0
> +What:/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_phase
Ah, I see what you are doing here. Much better to just have the final ABI
documented to discuss than doing it in steps.
> KernelVersion: 3.5.0
> Date:April 2012
> Contact: linux-...@vger.kernel.org
> @@ -25,7 +27,7 @@ Description:
> is between 0 and 4096 rad.
> Reading returns the value of phase written in register 0.
>
> -What:/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_phase1
> +What:/sys/bus/iio/devices/iio:deviceX/out_altvoltage1_phase
> KernelVersion: 3.5.0
> Date:April 2012
> Date:February 2019
> @@ -106,9 +108,9 @@ Description:
> have two registers for frequency and phase but only one
> output. The user can select which one controls the output.
> 0 represents phase 0 which is mapped to
> - out_altvoltage0_phase0
> + out_altvoltage0_phase
> 1 represents phase 1 which is mapped to
> - out_altvoltage0_phase1
> + out_altvoltage1_phase
>
> What:
> /sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out0_enable
> KernelVersion: 3.5.0
> diff --git a/drivers/staging/iio/frequency/ad9834.c
> b/drivers/staging/iio/frequency/ad9834.c
> index 8465dac656dd..107d859dadd7 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -82,6 +82,7 @@ struct ad9834_state {
> struct mutexlock; /* protect sensor state */
>
> unsigned long frequency[2];
> + unsigned long phase[2];
>
> /*
>* DMA (thus cache coherency maintenance) requires the
> @@ -113,6 +114,8 @@ enum ad9834_supported_device_ids {
> .output = 1,\
> .channel = (chan), \
> .info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY) \
> + | BIT(IIO_CHAN_INFO_PHASE),\
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
> }
>
> static const struct iio_chan_spec ad9833_channels[] = {
> @@ -170,13 +173,26 @@ static int ad9834_write_frequency(struct ad9834_state
> *st,
> }
>
> static int ad9834_write_phase(struct ad9834_state *st,
> - unsigned long addr, unsigned long phase)
> + enum ad9834_ch_addr addr,
> + unsigned long phase)
> {
> + int ret;
> +
> if (phase > BIT(AD9834_PHASE_BITS))
> return -EINVAL;
> - st->data = cpu_to_be16(addr | phase);
>
> - return spi_sync(st->spi, &st->msg);
> + if (addr == AD9834_CHANNEL_ADDRESS0)
> + st->data = cpu_to_be16(AD9834_REG_PHASE0 | phase);
> + else
> + st->data = cpu_to_be16(AD9834_REG_PHASE1 | phase);
> +
> + ret = spi_sync(st->spi, &st->msg);
> + if (ret)
> + return ret;
> +
> + st->phase[(int)addr] = phase;
> +
> + return 0;
> }
>
> static int ad9834_read_raw(struct iio_dev *indio_dev,
> @@ -189,6 +205,13 @@ static int ad9834_read_raw(struct iio_dev *indio_dev,
> case IIO_CHAN_INFO_FREQUENCY:
> *val = st->frequency[chan->channel];
> return IIO_VAL_INT;
> + case IIO_CHAN_INFO_PHASE:
> + *val = st->phase[chan->channel];
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + /*1 hz */
> + *val = 1;
> + return IIO_VAL_INT;
> }
>
> return -EINVAL;
> @@ -205,6 +228,
Re: [PATCH v2] iio: chemical: sps30: fix attribute kernel version
On Mon, 25 Feb 2019 19:17:40 +0100 Tomasz Duszynski wrote: > 4.22 have never existed and this change was actually added to 5.0 > hence fix the numbering. > > Signed-off-by: Tomasz Duszynski Applied. Thanks. Jonathan > --- > v2: > * this is targeting 5.0 hence fix the version > > Documentation/ABI/testing/sysfs-bus-iio-sps30 | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-sps30 > b/Documentation/ABI/testing/sysfs-bus-iio-sps30 > index 143df8e89d08..bef52f3e47bc 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio-sps30 > +++ b/Documentation/ABI/testing/sysfs-bus-iio-sps30 > @@ -1,6 +1,6 @@ > What:/sys/bus/iio/devices/iio:deviceX/start_cleaning > Date:December 2018 > -KernelVersion: 4.22 > +KernelVersion: 5.0 > Contact: linux-...@vger.kernel.org > Description: > Writing 1 starts sensor self cleaning. Internal fan accelerates > -- > 2.20.1 >
Re: [PATCH] iio: adc: ad7766: Change alignment to match paranthesis
On Sun, 24 Feb 2019 18:40:10 -0300
Camylla Gonçalves Cantanheide wrote:
> This commit align broken line to match upper line parenthesis,
> in lines 80, 130, 237, 242, 255, 264 and 293. Solves the checkpatch.pl's
> message:
>
> CHECK: Alignment should match open parenthesis
>
> In lines 130, 255, 264 and 293 it was necessary to break a line.
Applied to the togreg branch of iio.git and pushed out as testing
because of other patches that are queued up.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7766.c | 22 +-
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7766.c b/drivers/iio/adc/ad7766.c
> index 3ae14fc8c649..101502435768 100644
> --- a/drivers/iio/adc/ad7766.c
> +++ b/drivers/iio/adc/ad7766.c
> @@ -77,7 +77,7 @@ static irqreturn_t ad7766_trigger_handler(int irq, void *p)
> goto done;
>
> iio_push_to_buffers_with_timestamp(indio_dev, ad7766->data,
> - pf->timestamp);
> +pf->timestamp);
> done:
> iio_trigger_notify_done(indio_dev->trig);
>
> @@ -127,7 +127,8 @@ static int ad7766_postdisable(struct iio_dev *indio_dev)
> }
>
> static int ad7766_read_raw(struct iio_dev *indio_dev,
> - const struct iio_chan_spec *chan, int *val, int *val2, long info)
> +const struct iio_chan_spec *chan, int *val,
> +int *val2, long info)
> {
> struct ad7766 *ad7766 = iio_priv(indio_dev);
> struct regulator *vref = ad7766->reg[AD7766_SUPPLY_VREF].consumer;
> @@ -234,12 +235,12 @@ static int ad7766_probe(struct spi_device *spi)
> ad7766->reg[AD7766_SUPPLY_VREF].supply = "vref";
>
> ret = devm_regulator_bulk_get(&spi->dev, ARRAY_SIZE(ad7766->reg),
> - ad7766->reg);
> + ad7766->reg);
> if (ret)
> return ret;
>
> ad7766->pd_gpio = devm_gpiod_get_optional(&spi->dev, "powerdown",
> - GPIOD_OUT_HIGH);
> + GPIOD_OUT_HIGH);
> if (IS_ERR(ad7766->pd_gpio))
> return PTR_ERR(ad7766->pd_gpio);
>
> @@ -252,7 +253,8 @@ static int ad7766_probe(struct spi_device *spi)
>
> if (spi->irq > 0) {
> ad7766->trig = devm_iio_trigger_alloc(&spi->dev, "%s-dev%d",
> - indio_dev->name, indio_dev->id);
> + indio_dev->name,
> + indio_dev->id);
> if (!ad7766->trig)
> return -ENOMEM;
>
> @@ -261,8 +263,9 @@ static int ad7766_probe(struct spi_device *spi)
> iio_trigger_set_drvdata(ad7766->trig, ad7766);
>
> ret = devm_request_irq(&spi->dev, spi->irq, ad7766_irq,
> - IRQF_TRIGGER_FALLING, dev_name(&spi->dev),
> - ad7766->trig);
> +IRQF_TRIGGER_FALLING,
> +dev_name(&spi->dev),
> +ad7766->trig);
> if (ret < 0)
> return ret;
>
> @@ -290,8 +293,9 @@ static int ad7766_probe(struct spi_device *spi)
> spi_message_add_tail(&ad7766->xfer, &ad7766->msg);
>
> ret = devm_iio_triggered_buffer_setup(&spi->dev, indio_dev,
> - &iio_pollfunc_store_time, &ad7766_trigger_handler,
> - &ad7766_buffer_setup_ops);
> + &iio_pollfunc_store_time,
> + &ad7766_trigger_handler,
> + &ad7766_buffer_setup_ops);
> if (ret)
> return ret;
>
Re: [PATCH 0/3] Add leds, ir and fix regulator name on rk3328-rock64
Am Samstag, 2. März 2019, 17:56:56 CET schrieb Jonas Karlman: > This patchset fixes two regulator names and adds nodes to > enable use of leds and ir on Rock64. > > Jonas Karlman (3): > arm64: dts: rockchip: fix regulator name on rk3328-rock64 > arm64: dts: rockchip: add leds node on rk3328-rock64 > arm64: dts: rockchip: add ir-receiver node on rk3328-rock64 I've moved the ir-receiver node to its alphabetical position in the file and applied all 3 patches for 5.2 . You might want to check your mail setup though, the mail threading was broken. While all mails did contain In-Reply-To headers these didn't match the message IDs outlook.com set at all. Heiko
[PATCH v2] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
Add touchscreen info for the CHUWUI Hi10 Air tablet.
Signed-off-by: Christian Oder
Reviewed-by: Hans de Goede
---
Changes in v2:
- added review tag
drivers/platform/x86/touchscreen_dmi.c | 27 ++
1 file changed, 27 insertions(+)
diff --git a/drivers/platform/x86/touchscreen_dmi.c
b/drivers/platform/x86/touchscreen_dmi.c
index 167a156e3cc7..2d56ff7c8230 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
.properties = chuwi_hi8_pro_props,
};
+static const struct property_entry chuwi_hi10_air_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
+ PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
+ PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
+ PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct ts_dmi_data chuwi_hi10_air_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = chuwi_hi10_air_props,
+};
+
static const struct property_entry chuwi_vi8_props[] = {
PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
@@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[]
= {
DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
},
},
+ {
+ /* Chuwi Hi10 Air */
+ .driver_data = (void *)&chuwi_hi10_air_data,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
+ },
+ },
{
/* Chuwi Vi8 (CWI506) */
.driver_data = (void *)&chuwi_vi8_data,
--
2.20.1
Re: [PATCH v4 0/9] staging: iio: ad7780: move out of staging
Hi Alexandru, Thanks for the review. Some questions inline. Thanks, Renato On 03/01, Ardelean, Alexandru wrote: On Thu, 2019-02-28 at 11:23 -0300, Renato Lui Geh wrote: The patch-series is a bit big. I guess that the intent is to move this out-of-staging, but various patches are holding this in it's place. For patch series above a certain size, you could get many re-spins [V2,3,4... so on]. You could send some of the changes as individual patches, or group them in series of 1,2 or 3 patches. That way, you "parallelize" patch sending, and when you get reviews on each patch, you can re-spin them individually. You'll find over time that certain patches get accepted on V1, others on V2 and some on V7 [ hopefully, there isn't any frustration at that point ]. On these subseries, should versioning follow this patchset (v5) or should they start anew (v1), ignoring this series version? Well, this is a technique I use to distribute some of my upstream-patch- work, so that I can switch easier between internal-work & upstreaming-work. Coming back to this patch-series. My general input, is that the patches are fine over-all; some are just cosmetics/noise/a-different-way-of-doing-things-for-this-driver, and those usually can be left to preference [of the maintainer usually]. I do suggest to not hurry when re-spinning patches, and not change too much the number of patches in a new series. That can complicate things sometimes. But, if doing small patch-series or individual patches, you won't have this problem too much. Thanks Alex This series of patches contains the following: - Adds user input for the 'gain' and 'filter' GPIO pins for the ad778x family chips; - Filter reading for the ad778x; - Sets pattern macro values and mask for PATTERN status bits; - Adds ID values for the ad7170, ad7171, ad7780 and ad7781 for ID status bits checking; - Moves regulator initialization to after GPIO init to maintain consistency between probe and remove; - Copyright edits, adding SPDX identifier and new copyright holder; - Moves the ad7780 driver out of staging to the mainline; - Adds device tree binding for the ad7780 driver. Renato Lui Geh (9): staging: iio: ad7780: add gain & filter gpio support staging: iio: ad7780: add filter reading to ad778x staging: iio: ad7780: set pattern values and masks directly staging:iio:ad7780: add chip ID values and mask staging: iio: ad7780: move regulator to after GPIO init staging: iio: ad7780: add SPDX identifier staging: iio: ad7780: add new copyright holder staging: iio: ad7780: moving ad7780 out of staging staging: iio: ad7780: add device tree binding Changelog: *v3 - SPDX and regulator init as patches - Renamed filter to odr and ad778x_filter to ad778x_odr_avail - Removed unnecessary regulator disabling - Removed unnecessary AD_SD_CHANNEL macro - Changed unsigned int to unsigned long long to avoid overflow *v4 - Split gain & filter patch into two, with the new commit adding only filter reading - Changed pattern values to direct values, and added pattern mask - Added ID values and mask - Added new copyright holder - Added device tree binding to the ad7780 driver .../bindings/iio/adc/adi,ad7780.txt | 48 +++ drivers/iio/adc/Kconfig | 12 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/ad7780.c | 365 ++ drivers/staging/iio/adc/Kconfig | 13 - drivers/staging/iio/adc/Makefile | 1 - drivers/staging/iio/adc/ad7780.c | 277 - 7 files changed, 426 insertions(+), 291 deletions(-) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7780.txt create mode 100644 drivers/iio/adc/ad7780.c delete mode 100644 drivers/staging/iio/adc/ad7780.c -- 2.21.0 -- You received this message because you are subscribed to the Google Groups "Kernel USP" group. To unsubscribe from this group and stop receiving emails from it, send an email to kernel-usp+unsubscr...@googlegroups.com. To post to this group, send email to kernel-...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kernel-usp/72a54cd5f58aeb9507b95b7e33ca3d9a38c853e9.camel%40analog.com. For more options, visit https://groups.google.com/d/optout.
Re: [PATCH] arm64: dts: rockchip: enable HDMI CEC on rk3328
Am Samstag, 2. März 2019, 17:37:09 CET schrieb Jonas Karlman: > This patch enables HDMI CEC on RK3328 devices > > Signed-off-by: Jonas Karlman applied for 5.2. I've added another sentence to the commit messsage for the origin of the (unusual) cec clock, marking the vendor-kernel as source. Heiko
Re: KASAN: use-after-free Read in unix_dgram_poll
On Sun, Mar 03, 2019 at 02:22:04AM -0800, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:7d762d69145a afs: Fix manually set volume location server .. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=131d832ac0 > kernel config: https://syzkaller.appspot.com/x/.config?x=b76ec970784287c > dashboard link: https://syzkaller.appspot.com/bug?extid=503d4cc169fcec1cb18c > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11934262c0 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+503d4cc169fcec1cb...@syzkaller.appspotmail.com > > == > BUG: KASAN: use-after-free in unix_dgram_poll+0x5e1/0x690 > net/unix/af_unix.c:2695 > Read of size 4 at addr 88809292aae0 by task syz-executor.1/18946 > > CPU: 0 PID: 18946 Comm: syz-executor.1 Not tainted 5.0.0-rc8+ #88 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x172/0x1f0 lib/dump_stack.c:113 > print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187 > kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317 > __asan_report_load4_noabort+0x14/0x20 mm/kasan/generic_report.c:134 > unix_dgram_poll+0x5e1/0x690 net/unix/af_unix.c:2695 > sock_poll+0x291/0x340 net/socket.c:1127 > vfs_poll include/linux/poll.h:86 [inline] > aio_poll fs/aio.c:1766 [inline] > __io_submit_one fs/aio.c:1876 [inline] > io_submit_one+0xe3e/0x1cf0 fs/aio.c:1909 > __do_sys_io_submit fs/aio.c:1954 [inline] > __se_sys_io_submit fs/aio.c:1924 [inline] > __x64_sys_io_submit+0x1bd/0x580 fs/aio.c:1924 > ? 0x8100 > do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290 > entry_SYSCALL_64_after_hwframe+0x49/0xbe Apparently, a call of ->poll() overlapping with call of ->release() (if not outright happening after it). Which should never happen... Maybe unrelated to this bug, but... What's to prevent a wakeup that happens just after we'd been added to a waitqueue by ->poll() triggering aio_poll_wake(), which gets to aio_poll_complete() with its fput() *before* we'd reached the end of ->poll() instance? I wonder if adding get_file(req->file); // make sure that early completion is safe right after req->file = fget(iocb->aio_fildes); if (unlikely(!req->file)) return -EBADF; paired with fput(req->file); right after out: in aio_poll() is needed... Am I missing something obvious here? Christoph?
Re: [PATCH v3 1/2] arm64: dts: rockchip: add #sound-dai-cells to HDMI of rk3328
Am Sonntag, 17. Februar 2019, 18:34:06 CET schrieb Katsuhiro Suzuki: > This patch adds #sound-dai-cells to use HDMI node as audio > codec from device tree of rk3328 boards. > > Signed-off-by: Katsuhiro Suzuki applied for 5.2 . I'm holding off of patch2 for a bit until you and Jonas can clarify that all is right as it is :-) Heiko
Re: [PATCH v2] platform/x86: touchscreen_dmi: Add info for the CHUWI Hi10 Air tablet
Hi Christian,
On 03-03-19 14:47, Christian Oder wrote:
Add touchscreen info for the CHUWUI Hi10 Air tablet.
Signed-off-by: Christian Oder
Reviewed-by: Hans de Goede
---
Changes in v2:
- added review tag
A quick note for any patches you may submit in the future, there
is no need to send a v2 just to add someones Reviewed-by or Acked-by,
these will be picked up automatically by the subsys-maintainer when
they add the patch to their tree.
Regards,
Hans
drivers/platform/x86/touchscreen_dmi.c | 27 ++
1 file changed, 27 insertions(+)
diff --git a/drivers/platform/x86/touchscreen_dmi.c
b/drivers/platform/x86/touchscreen_dmi.c
index 167a156e3cc7..2d56ff7c8230 100644
--- a/drivers/platform/x86/touchscreen_dmi.c
+++ b/drivers/platform/x86/touchscreen_dmi.c
@@ -72,6 +72,25 @@ static const struct ts_dmi_data chuwi_hi8_pro_data = {
.properties = chuwi_hi8_pro_props,
};
+static const struct property_entry chuwi_hi10_air_props[] = {
+ PROPERTY_ENTRY_U32("touchscreen-size-x", 1981),
+ PROPERTY_ENTRY_U32("touchscreen-size-y", 1271),
+ PROPERTY_ENTRY_U32("touchscreen-min-x", 99),
+ PROPERTY_ENTRY_U32("touchscreen-min-y", 9),
+ PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-x", 5),
+ PROPERTY_ENTRY_U32("touchscreen-fuzz-y", 4),
+ PROPERTY_ENTRY_STRING("firmware-name", "gsl1680-chuwi-hi10-air.fw"),
+ PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
+ { }
+};
+
+static const struct ts_dmi_data chuwi_hi10_air_data = {
+ .acpi_name = "MSSL1680:00",
+ .properties = chuwi_hi10_air_props,
+};
+
static const struct property_entry chuwi_vi8_props[] = {
PROPERTY_ENTRY_U32("touchscreen-min-x", 4),
PROPERTY_ENTRY_U32("touchscreen-min-y", 6),
@@ -546,6 +565,14 @@ static const struct dmi_system_id touchscreen_dmi_table[]
= {
DMI_MATCH(DMI_PRODUCT_NAME, "X1D3_C806N"),
},
},
+ {
+ /* Chuwi Hi10 Air */
+ .driver_data = (void *)&chuwi_hi10_air_data,
+ .matches = {
+ DMI_MATCH(DMI_BOARD_VENDOR, "Hampoo"),
+ DMI_MATCH(DMI_PRODUCT_SKU, "P1W6_C109D_B"),
+ },
+ },
{
/* Chuwi Vi8 (CWI506) */
.driver_data = (void *)&chuwi_vi8_data,
Re: [PATCH v4 4/9] staging:iio:ad7780: add chip ID values and mask
On 03/01, Ardelean, Alexandru wrote: On Thu, 2019-02-28 at 11:24 -0300, Renato Lui Geh wrote: The ad7780 supports both the ad778x and ad717x families. Each chip has a corresponding ID. This patch provides a mask for extracting ID values from the status bits and also macros for the correct values for the ad7170, ad7171, ad7780 and ad7781. Signed-off-by: Renato Lui Geh --- drivers/staging/iio/adc/ad7780.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c index 56c49e28f432..ad7617a3a141 100644 --- a/drivers/staging/iio/adc/ad7780.c +++ b/drivers/staging/iio/adc/ad7780.c @@ -26,10 +26,14 @@ #define AD7780_RDY BIT(7) #define AD7780_FILTER BIT(6) #define AD7780_ERR BIT(5) -#define AD7780_ID1 BIT(4) -#define AD7780_ID0 BIT(3) #define AD7780_GAINBIT(2) +#define AD7170_ID 0 +#define AD7171_ID 1 +#define AD7780_ID 1 +#define AD7781_ID 0 + +#define AD7780_ID_MASK (BIT(3) | BIT(4)) This also doesn't have any functionality change. The AD7170_ID, AD7171_ID, AD7780_ID & AD7781_ID IDs are also unused (maybe in a later patch they are ?). They aren't. I added them following a previous review suggestion. Should I remove them? I would also leave the AD7780_ID1 & AD7780_ID0 definitions in place, since they're easier matched with the datasheet. #define AD7780_PATTERN_GOOD1 #define AD7780_PATTERN_MASKGENMASK(1, 0) -- 2.21.0
Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2
Hello Heiko,
Thank you for comments.
On 2019/03/03 22:19, Heiko Stuebner wrote:
Hi,
Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:
This patch increases drive strength of UART2 from 3mA to 12mA for
getting more faster rising edge.
RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
this setting, a bit width of UART is about 667ns.
In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
converter), falling time of RockPro64 UART2 is 40ns, but riging time
is over 650ns. So UART receiver will get wrong data, because receiver
read intermediate data of rising edge.
Rising time becomes 300ns from 650ns if apply this patch. This is not
perfect solution but better than now.
Signed-off-by: Katsuhiro Suzuki
---
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
your changing a core rk3399 property here, so I'd really like to get
input from other board stakeholders on this before applying a core
change.
Could you either include the submitters of other rk3399-boards in the
recipient list so that they're aware or limit the change to rockpro64 for
the time being (aka overriding the property in the board-dts) please?
OK, I'm adding other boards members.
by ./scripts/get_maintainer.pl arch/arm64/boot/dts/rockchip/rk3399-*.dts
RockPro64 directly connect UART2 pins of RK3399 to external connector.
I think maybe other RK3399 boards are facing same problem, but I cannot
check it because I have RockPro64 only...
I'm happy if someone tell me other boards situation.
Best Regards,
Katsuhiro Suzuki
Thanks
Heiko
diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index beaa92744a64..e3c8f91ead50 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -2000,6 +2000,11 @@
drive-strength = <8>;
};
+ pcfg_pull_up_12ma: pcfg-pull-up-12ma {
+ bias-pull-up;
+ drive-strength = <12>;
+ };
+
pcfg_pull_up_18ma: pcfg-pull-up-18ma {
bias-pull-up;
drive-strength = <18>;
@@ -2521,8 +2526,8 @@
uart2c {
uart2c_xfer: uart2c-xfer {
rockchip,pins =
- <4 RK_PC3 RK_FUNC_1 &pcfg_pull_up>,
- <4 RK_PC4 RK_FUNC_1 &pcfg_pull_none>;
+ <4 RK_PC3 RK_FUNC_1 &pcfg_pull_up_12ma>,
+ <4 RK_PC4 RK_FUNC_1
&pcfg_pull_none_12ma>;
};
};
Re: [PATCH] rsi: Fix NULL pointer dereference in kmalloc
On Sat, 2019-03-02 at 14:31 -0600, Aditya Pakki wrote:
> kmalloc can fail in rsi_register_rates_channels but memcpy still attempts
> to write to channels. The patch checks and avoids such a situation.
>
> Signed-off-by: Aditya Pakki
> ---
> drivers/net/wireless/rsi/rsi_91x_mac80211.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/wireless/rsi/rsi_91x_mac80211.c
> b/drivers/net/wireless/rsi/rsi_91x_mac80211.c
> index e56fc83faf0e..59eb1f533d0e 100644
> --- a/drivers/net/wireless/rsi/rsi_91x_mac80211.c
> +++ b/drivers/net/wireless/rsi/rsi_91x_mac80211.c
> @@ -197,6 +197,11 @@ static void rsi_register_rates_channels(struct rsi_hw
> *adapter, int band)
>
> if (band == NL80211_BAND_2GHZ) {
> channels = kmalloc(sizeof(rsi_2ghz_channels), GFP_KERNEL);
> + if (!channels) {
> + rsi_dbg(ERR_ZONE, "Failed to allocate memory\n");
> + return;
> + }
> +
> memcpy(channels,
> rsi_2ghz_channels,
> sizeof(rsi_2ghz_channels));
Should probably be kmemdup() anyway though.
johannes
Re: [PATCH v4 0/9] staging: iio: ad7780: move out of staging
On Sun, Mar 3, 2019 at 3:52 PM Renato Lui Geh wrote: > > Hi Alexandru, > > Thanks for the review. Some questions inline. > > Thanks, > Renato > > On 03/01, Ardelean, Alexandru wrote: > >On Thu, 2019-02-28 at 11:23 -0300, Renato Lui Geh wrote: > >> > > > >The patch-series is a bit big. > >I guess that the intent is to move this out-of-staging, but various patches > >are holding this in it's place. > >For patch series above a certain size, you could get many re-spins > >[V2,3,4... so on]. > > > >You could send some of the changes as individual patches, or group them in > >series of 1,2 or 3 patches. That way, you "parallelize" patch sending, and > >when you get reviews on each patch, you can re-spin them individually. > >You'll find over time that certain patches get accepted on V1, others on V2 > >and some on V7 [ hopefully, there isn't any frustration at that point ]. > > On these subseries, should versioning follow this patchset (v5) or should > they start anew (v1), ignoring this series version? I guess, in this case it's fine to leave it as is [in this series]. The series has been reviewed now. But [for me typically], I delay doing a review if a patch-series is longer than 4-5 patches. And I think some reviewers may do the same. So, if I want more people to review/look at my code, I try to make things as easy to review, as possible. And one way, is to definitely keep things decoupled. If one patch can be independent of another [for the same driver/code], I send them as separate patches. This [of course], is a preference. Some reviewers don't mind longer series [than 4-5 patches]. > > > >Well, this is a technique I use to distribute some of my upstream-patch- > >work, so that I can switch easier between internal-work & upstreaming-work. > > > >Coming back to this patch-series. > >My general input, is that the patches are fine over-all; some are just > >cosmetics/noise/a-different-way-of-doing-things-for-this-driver, and those > >usually can be left to preference [of the maintainer usually]. > > > >I do suggest to not hurry when re-spinning patches, and not change too much > >the number of patches in a new series. That can complicate things > >sometimes. But, if doing small patch-series or individual patches, you > >won't have this problem too much. > > > >Thanks > >Alex > > > >> > >> This series of patches contains the following: > >> - Adds user input for the 'gain' and 'filter' GPIO pins for the ad778x > >>family chips; > >> - Filter reading for the ad778x; > >> - Sets pattern macro values and mask for PATTERN status bits; > >> - Adds ID values for the ad7170, ad7171, ad7780 and ad7781 for ID > >>status bits checking; > >> - Moves regulator initialization to after GPIO init to maintain > >>consistency between probe and remove; > >> - Copyright edits, adding SPDX identifier and new copyright holder; > >> - Moves the ad7780 driver out of staging to the mainline; > >> - Adds device tree binding for the ad7780 driver. > >> > >> Renato Lui Geh (9): > >> staging: iio: ad7780: add gain & filter gpio support > >> staging: iio: ad7780: add filter reading to ad778x > >> staging: iio: ad7780: set pattern values and masks directly > >> staging:iio:ad7780: add chip ID values and mask > >> staging: iio: ad7780: move regulator to after GPIO init > >> staging: iio: ad7780: add SPDX identifier > >> staging: iio: ad7780: add new copyright holder > >> staging: iio: ad7780: moving ad7780 out of staging > >> staging: iio: ad7780: add device tree binding > >> > >> Changelog: > >> *v3 > >> - SPDX and regulator init as patches > >> - Renamed filter to odr and ad778x_filter to ad778x_odr_avail > >> - Removed unnecessary regulator disabling > >> - Removed unnecessary AD_SD_CHANNEL macro > >> - Changed unsigned int to unsigned long long to avoid overflow > >> *v4 > >> - Split gain & filter patch into two, with the new commit adding only > >>filter reading > >> - Changed pattern values to direct values, and added pattern mask > >> - Added ID values and mask > >> - Added new copyright holder > >> - Added device tree binding to the ad7780 driver > >> > >> .../bindings/iio/adc/adi,ad7780.txt | 48 +++ > >> drivers/iio/adc/Kconfig | 12 + > >> drivers/iio/adc/Makefile | 1 + > >> drivers/iio/adc/ad7780.c | 365 ++ > >> drivers/staging/iio/adc/Kconfig | 13 - > >> drivers/staging/iio/adc/Makefile | 1 - > >> drivers/staging/iio/adc/ad7780.c | 277 - > >> 7 files changed, 426 insertions(+), 291 deletions(-) > >> create mode 100644 > >> Documentation/devicetree/bindings/iio/adc/adi,ad7780.txt > >> create mode 100644 drivers/iio/adc/ad7780.c > >> delete mode 100644 drivers/staging/iio/adc/ad7780.c > >> > >> -- > >> 2.21.0 > >> > > > >-- > >You received this message because you are subscribed to the Google Groups > >"Kernel USP" gr
[PATCH -next] workqueue: fix a memory leak in wq->lock_name
The linux-next commit 669de8bda87b ("kernel/workqueue: Use dynamic
lockdep keys for workqueues") introduced a memory leak as
wq_free_lockdep() calls kfree(wq->lock_name) but wq_init_lockdep() does
not point wq->lock_name to the newly allocated slab object. This can be
reproduced by running LTP fallocate04 followed by oom01 tests.
unreferenced object 0xc005876384d8 (size 64):
comm "fallocate04", pid 26972, jiffies 4297139141 (age 40370.480s)
hex dump (first 32 bytes):
28 77 71 5f 63 6f 6d 70 6c 65 74 69 6f 6e 29 65 (wq_completion)e
78 74 34 2d 72 73 76 2d 63 6f 6e 76 65 72 73 69 xt4-rsv-conversi
backtrace:
[] kvasprintf+0x6c/0xe0
[<4654ddac>] kasprintf+0x34/0x60
[<1c68f311>] alloc_workqueue+0x1f8/0x6ac
[<03c2ad83>] ext4_fill_super+0x23d4/0x3c80 [ext4]
[<06610538>] mount_bdev+0x25c/0x290
[] ext4_mount+0x28/0x50 [ext4]
[<16e08fd3>] legacy_get_tree+0x4c/0xb0
[<42b6a5fc>] vfs_get_tree+0x6c/0x190
[<268ab022>] do_mount+0xb9c/0x1100
[<698e6898>] ksys_mount+0x158/0x180
[<64e391fd>] sys_mount+0x20/0x30
[] system_call+0x5c/0x70
Signed-off-by: Qian Cai
---
kernel/workqueue.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 497900263dbc..e780bf73eced 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -3445,6 +3445,8 @@ static void wq_init_lockdep(struct workqueue_struct *wq)
lock_name = kasprintf(GFP_KERNEL, "%s%s", "(wq_completion)", wq->name);
if (!lock_name)
lock_name = wq->name;
+
+ wq->lock_name = lock_name;
lockdep_init_map(&wq->lockdep_map, lock_name, &wq->key, 0);
}
--
2.17.2 (Apple Git-113)
Re: [PATCH v2 1/3] iio: light: Add driver for ap3216c
On Thu, 28 Feb 2019 11:49:54 -0500 Sven Van Asbroeck wrote: > Hi, comments inline. > > On Sun, Feb 24, 2019 at 3:36 PM Robert Eshleman > wrote: > > > > This patch adds support for the ap3216c ambient light and proximity > > sensor. > > > > Supported features include: > > > > * Illuminance (lux) > > * Proximity (raw) > > * IR (raw) > > * Rising/falling threshold events for illuminance and proximity > > * Calibration scale for illuminance > > * Calibration bias for proximity > > > > Signed-off-by: Robert Eshleman Hmm. Just been thinking a bit about the events on here and wondered if it is possible to mask them through careful use of the threshold values - i.e. can we stop the hardware generating the interrupts for the ones we don't want. It would be unusual for hardware to be designed where this wasn't possible. From reading the datasheet I think we can, but obviously you will want to try checking! So for the lower thresholds the DS states: An interrupt is triggered when ALS ADC (Registers 0CH & 0DH) < ALS ADC Low Threshold. So if we set the low threshold to 0 that is never true. For the high thresholds An interrupt is triggered when ALS ADC (Registers 0CH and 0DH) > ALS ADC High Threshold So if we set the threshold to the maximum value that is never true either. So I'd propose a change to how you handle the thresholds. When the value is set, if the threshold is not currently enabled, cache it rather than directly writing. When enabling then write the threshold. This will get rid of any potential issues around the _en flags by moving it all into hardware. However it does depend on the datasheet being correct! Otherwise, a few additional comments from me. I piggy backed on Sven's review to hopefully avoid too much repetition! Jonathan > > --- > > Changes in v2: > > - Add mutex protection in IRQ handler > > - Support interrupt clearing for PS and ALS separately > > - Mask away reserved bits when reading device registers > > - Use regmap_bulk_read and regmap_bulk_write when possible > > - Style cleanup > > > > drivers/iio/light/Kconfig | 11 + > > drivers/iio/light/Makefile | 1 + > > drivers/iio/light/ap3216c.c | 809 > > 3 files changed, 821 insertions(+) > > create mode 100644 drivers/iio/light/ap3216c.c > > > > diff --git a/drivers/iio/light/Kconfig b/drivers/iio/light/Kconfig > > index 36f458433480..74688d19beb1 100644 > > --- a/drivers/iio/light/Kconfig > > +++ b/drivers/iio/light/Kconfig > > @@ -41,6 +41,17 @@ config AL3320A > > To compile this driver as a module, choose M here: the > > module will be called al3320a. > > > > +config AP3216C > > + tristate "AP3216C Ambient Light and Proximity sensor" > > + select REGMAP_I2C > > + depends on I2C > > + help > > + Say Y here to build a driver for the AP3216C Ambient Light and > > + Proximity sensor. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called ap3216c. > > + > > config APDS9300 > > tristate "APDS9300 ambient light sensor" > > depends on I2C > > diff --git a/drivers/iio/light/Makefile b/drivers/iio/light/Makefile > > index 286bf3975372..7d2f8fa0f30d 100644 > > --- a/drivers/iio/light/Makefile > > +++ b/drivers/iio/light/Makefile > > @@ -7,6 +7,7 @@ > > obj-$(CONFIG_ACPI_ALS) += acpi-als.o > > obj-$(CONFIG_ADJD_S311)+= adjd_s311.o > > obj-$(CONFIG_AL3320A) += al3320a.o > > +obj-$(CONFIG_AP3216C) += ap3216c.o > > obj-$(CONFIG_APDS9300) += apds9300.o > > obj-$(CONFIG_APDS9960) += apds9960.o > > obj-$(CONFIG_BH1750) += bh1750.o > > diff --git a/drivers/iio/light/ap3216c.c b/drivers/iio/light/ap3216c.c > > new file mode 100644 > > index ..aaa319b932e6 > > --- /dev/null > > +++ b/drivers/iio/light/ap3216c.c > > @@ -0,0 +1,809 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * AP3216C Ambient Light and Infrared Proximity Sensor > > + * > > + * Copyright (c) 2019, Robert Eshleman. > > + * > > + * Datasheet: > > https://pdf-datasheet-datasheet.netdna-ssl.com/pdf-down/A/P/3/AP3216C-LITE-ON.pdf > > + * > > + * 7-bit I2C slave address 0x1E > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define AP3216C_DRV_NAME "ap3216c" > > + > > +/* register addresses */ > > +#define AP3216C_SYS 0x0 > > +#define AP3216C_INT_STATUS 0x01 > > +#define AP3216C_INT_CLR 0x02 > > +#define AP3216C_IR_DATA_LO 0x0A > > +#define AP3216C_IR_DATA_HI 0x0B > > +#define AP3216C_ALS_DATA_LO 0x0C > > +#define AP3216C_ALS_DATA_HI 0x0D > > +#define AP3216C_PS_DATA_LO 0x0E > > +#define AP3216C_PS_DATA_HI 0x0F > > +#define AP3216C_ALS_CFG 0x10 > > +#define AP3216C_ALS_CALIB 0x19 > > +#define AP3216C_ALS_LO_THR_LO 0x1A > > +#define AP3216C_ALS_LO_THR_HI 0x1B > > +#def
Re: [PATCH 1/2] dt-bindings: iio: stm32-lptimer-counter: document pinctrl sleep state
On Mon, 25 Feb 2019 11:42:46 +0100
Fabrice Gasnier wrote:
> Add documentation for optional pinctrl sleep state that can be used by
> STM32 LPTimer encoder/counter.
>
> Signed-off-by: Fabrice Gasnier
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.
Note we are too late for the coming merge window so this will be
for the next one now.
Thanks,
Jonathan
> ---
> .../devicetree/bindings/iio/counter/stm32-lptimer-cnt.txt | 8
> +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/iio/counter/stm32-lptimer-cnt.txt
> b/Documentation/devicetree/bindings/iio/counter/stm32-lptimer-cnt.txt
> index a04aa5c..e90bc47 100644
> --- a/Documentation/devicetree/bindings/iio/counter/stm32-lptimer-cnt.txt
> +++ b/Documentation/devicetree/bindings/iio/counter/stm32-lptimer-cnt.txt
> @@ -10,8 +10,9 @@ See ../mfd/stm32-lptimer.txt for details about the parent
> node.
>
> Required properties:
> - compatible:Must be "st,stm32-lptimer-counter".
> -- pinctrl-names: Set to "default".
> -- pinctrl-0: List of phandles pointing to pin configuration
> nodes,
> +- pinctrl-names: Set to "default". An additional "sleep" state can be
> + defined to set pins in sleep state.
> +- pinctrl-n: List of phandles pointing to pin configuration
> nodes,
> to set IN1/IN2 pins in mode of operation for Low-Power
> Timer input on external pin.
>
> @@ -21,7 +22,8 @@ Example:
> ...
> counter {
> compatible = "st,stm32-lptimer-counter";
> - pinctrl-names = "default";
> + pinctrl-names = "default", "sleep";
> pinctrl-0 = <&lptim1_in_pins>;
> + pinctrl-1 = <&lptim1_sleep_in_pins>;
> };
> };
Re: [PATCH 2/2] iio: counter: stm32-lptimer: Add power management support
On Mon, 25 Feb 2019 11:42:47 +0100
Fabrice Gasnier wrote:
> Add suspend/resume PM sleep ops. When going to low power, disable
> active counter. Only active counter should be resumed: don't touch
> disabled counter, as it may be used by other LPTimer MFD child driver.
>
> Signed-off-by: Fabrice Gasnier
Applied. Thanks,
Jonathan
> ---
> drivers/iio/counter/stm32-lptimer-cnt.c | 55
> +
> 1 file changed, 55 insertions(+)
>
> diff --git a/drivers/iio/counter/stm32-lptimer-cnt.c
> b/drivers/iio/counter/stm32-lptimer-cnt.c
> index 42fb8ba..2a49cce 100644
> --- a/drivers/iio/counter/stm32-lptimer-cnt.c
> +++ b/drivers/iio/counter/stm32-lptimer-cnt.c
> @@ -14,6 +14,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> struct stm32_lptim_cnt {
> @@ -23,6 +24,7 @@ struct stm32_lptim_cnt {
> u32 preset;
> u32 polarity;
> u32 quadrature_mode;
> + bool enabled;
> };
>
> static int stm32_lptim_is_enabled(struct stm32_lptim_cnt *priv)
> @@ -50,6 +52,7 @@ static int stm32_lptim_set_enable_state(struct
> stm32_lptim_cnt *priv,
>
> if (!enable) {
> clk_disable(priv->clk);
> + priv->enabled = false;
> return 0;
> }
>
> @@ -79,6 +82,7 @@ static int stm32_lptim_set_enable_state(struct
> stm32_lptim_cnt *priv,
> regmap_write(priv->regmap, STM32_LPTIM_CR, 0);
> return ret;
> }
> + priv->enabled = true;
>
> /* Start LP timer in continuous mode */
> return regmap_update_bits(priv->regmap, STM32_LPTIM_CR,
> @@ -361,6 +365,56 @@ static int stm32_lptim_cnt_probe(struct platform_device
> *pdev)
> return devm_iio_device_register(&pdev->dev, indio_dev);
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int stm32_lptim_cnt_suspend(struct device *dev)
> +{
> + struct stm32_lptim_cnt *priv = dev_get_drvdata(dev);
> + int ret;
> +
> + /* Only take care of enabled counter: don't disturb other MFD child */
> + if (priv->enabled) {
> + ret = stm32_lptim_setup(priv, 0);
> + if (ret)
> + return ret;
> +
> + ret = stm32_lptim_set_enable_state(priv, 0);
> + if (ret)
> + return ret;
> +
> + /* Force enable state for later resume */
> + priv->enabled = true;
> + }
> +
> + return pinctrl_pm_select_sleep_state(dev);
> +}
> +
> +static int stm32_lptim_cnt_resume(struct device *dev)
> +{
> + struct stm32_lptim_cnt *priv = dev_get_drvdata(dev);
> + int ret;
> +
> + ret = pinctrl_pm_select_default_state(dev);
> + if (ret)
> + return ret;
> +
> + if (priv->enabled) {
> + priv->enabled = false;
> + ret = stm32_lptim_setup(priv, 1);
> + if (ret)
> + return ret;
> +
> + ret = stm32_lptim_set_enable_state(priv, 1);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(stm32_lptim_cnt_pm_ops, stm32_lptim_cnt_suspend,
> + stm32_lptim_cnt_resume);
> +
> static const struct of_device_id stm32_lptim_cnt_of_match[] = {
> { .compatible = "st,stm32-lptimer-counter", },
> {},
> @@ -372,6 +426,7 @@ static struct platform_driver stm32_lptim_cnt_driver = {
> .driver = {
> .name = "stm32-lptimer-counter",
> .of_match_table = stm32_lptim_cnt_of_match,
> + .pm = &stm32_lptim_cnt_pm_ops,
> },
> };
> module_platform_driver(stm32_lptim_cnt_driver);
Re: [PATCH 1/4] iio:adc:ad7923: Align broken line to parenthesis
On Fri, 22 Feb 2019 17:31:56 -0300
Bárbara Fernandes wrote:
> Get broken line aligned with parenthesis on upper line. Solves
> checkpatch.pl's message:
>
> CHECK: Alignment should match open parenthesis
>
> Signed-off-by: Bárbara Fernandes
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7923.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index d62dbb62be45..ebae7522710a 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -130,7 +130,7 @@ static const struct ad7923_chip_info ad7923_chip_info[] =
> {
> * ad7923_update_scan_mode() setup the spi transfer buffer for the new scan
> mask
> **/
> static int ad7923_update_scan_mode(struct iio_dev *indio_dev,
> - const unsigned long *active_scan_mask)
> +const unsigned long *active_scan_mask)
> {
> struct ad7923_state *st = iio_priv(indio_dev);
> int i, cmd, len;
> @@ -181,7 +181,7 @@ static irqreturn_t ad7923_trigger_handler(int irq, void
> *p)
> goto done;
>
> iio_push_to_buffers_with_timestamp(indio_dev, st->rx_buf,
> - iio_get_time_ns(indio_dev));
> +iio_get_time_ns(indio_dev));
>
> done:
> iio_trigger_notify_done(indio_dev->trig);
> @@ -314,7 +314,7 @@ static int ad7923_probe(struct spi_device *spi)
> return ret;
>
> ret = iio_triggered_buffer_setup(indio_dev, NULL,
> - &ad7923_trigger_handler, NULL);
> + &ad7923_trigger_handler, NULL);
> if (ret)
> goto error_disable_reg;
>
Re: [PATCH 2/4] iio:adc:ad7923: Use BIT macro instead of bitshift
On Fri, 22 Feb 2019 17:31:57 -0300 Bárbara Fernandes wrote: > Replace use of the operation '<<' by the BIT macro. Solves checkpath.pl's > message: > > CHECK: Prefer using the BIT macro > > Signed-off-by: Bárbara Fernandes Applied. Thanks, Jonathan > --- > drivers/iio/adc/ad7923.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c > index ebae7522710a..b39ea834cdd6 100644 > --- a/drivers/iio/adc/ad7923.c > +++ b/drivers/iio/adc/ad7923.c > @@ -24,9 +24,9 @@ > #include > #include > > -#define AD7923_WRITE_CR (1 << 11) /* write control > register */ > -#define AD7923_RANGE (1 << 1)/* range to REFin */ > -#define AD7923_CODING(1 << 0)/* coding is straight > binary */ > +#define AD7923_WRITE_CR BIT(11) /* write control > register */ > +#define AD7923_RANGE BIT(1) /* range to REFin */ > +#define AD7923_CODINGBIT(0) /* coding is straight > binary */ > #define AD7923_PM_MODE_AS(1) /* auto shutdown */ > #define AD7923_PM_MODE_FS(2) /* full shutdown */ > #define AD7923_PM_MODE_OPS (3) /* normal operation */
Re: [PATCH 3/4] iio:adc:ad7923: Put macro argument between ()'s
On Fri, 22 Feb 2019 17:31:58 -0300
Bárbara Fernandes wrote:
> Put macro argument between parenthesis in order to avoid precedence
> issues. Solves the following checkpath.pl's messages:
>
> CHECK: Macro argument 'mode' may be better as '(mode)' to avoid
> precedence issues
> CHECK: Macro argument 'channel' may be better as '(channel)' to
> avoid precedence issues
> CHECK: Macro argument reuse 'sequence' - possible side-effects?
> CHECK: Macro argument 'sequence' may be better as '(sequence)' to
> avoid precedence issues
> CHECK: Macro argument 'val' may be better as '(val)' to avoid
> precedence issues
> CHECK: Macro argument 'dec' may be better as '(dec)' to avoid precedence
> issues
> CHECK: Macro argument 'bits' may be better as '(bits)' to avoid
> precedence issues
>
> Signed-off-by: Bárbara Fernandes
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/adc/ad7923.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c
> index b39ea834cdd6..dbece44e26e4 100644
> --- a/drivers/iio/adc/ad7923.c
> +++ b/drivers/iio/adc/ad7923.c
> @@ -40,16 +40,16 @@
>
> #define AD7923_MAX_CHAN 4
>
> -#define AD7923_PM_MODE_WRITE(mode) (mode << 4) /* write mode */
> -#define AD7923_CHANNEL_WRITE(channel)(channel << 6) /* write
> channel */
> -#define AD7923_SEQUENCE_WRITE(sequence) (((sequence & 1) << 3) \
> - + ((sequence & 2) << 9))
> +#define AD7923_PM_MODE_WRITE(mode) ((mode) << 4)/* write mode */
> +#define AD7923_CHANNEL_WRITE(channel)((channel) << 6) /* write
> channel */
> +#define AD7923_SEQUENCE_WRITE(sequence) sequence) & 1) << 3) \
> + + (((sequence) & 2) << 9))
> /* write sequence fonction */
> /* left shift for CR : bit 11 transmit in first */
> #define AD7923_SHIFT_REGISTER4
>
> /* val = value, dec = left shift, bits = number of bits of the mask */
> -#define EXTRACT(val, dec, bits) ((val >> dec) & ((1 << bits) -
> 1))
> +#define EXTRACT(val, dec, bits) (((val) >> (dec)) & ((1 <<
> (bits)) - 1))
>
> struct ad7923_state {
> struct spi_device *spi;
Re: [PATCH 4/4] iio:adc:ad7923: Rewrite comparison to NULL
On Fri, 22 Feb 2019 17:31:59 -0300 Bárbara Fernandes wrote: > Solves checkpath.pl's message: > > CHECK: Comparison to NULL could be written "!indio_dev" > > Signed-off-by: Bárbara Fernandes Applied, thanks. It's nice to get some cleanup for drivers that aren't in staging sometimes. After all, in recent years the kernel coding style and tools to check it have improved considerably. Thanks, Jonathan > --- > drivers/iio/adc/ad7923.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/adc/ad7923.c b/drivers/iio/adc/ad7923.c > index dbece44e26e4..cb7b854df00c 100644 > --- a/drivers/iio/adc/ad7923.c > +++ b/drivers/iio/adc/ad7923.c > @@ -272,7 +272,7 @@ static int ad7923_probe(struct spi_device *spi) > int ret; > > indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st)); > - if (indio_dev == NULL) > + if (!indio_dev) > return -ENOMEM; > > st = iio_priv(indio_dev);
Re: [PATCH v4 4/9] staging:iio:ad7780: add chip ID values and mask
On Sun, 3 Mar 2019 11:01:09 -0300 Renato Lui Geh wrote: > On 03/01, Ardelean, Alexandru wrote: > >On Thu, 2019-02-28 at 11:24 -0300, Renato Lui Geh wrote: > >> > >> > >> The ad7780 supports both the ad778x and ad717x families. Each chip has > >> a corresponding ID. This patch provides a mask for extracting ID values > >> from the status bits and also macros for the correct values for the > >> ad7170, ad7171, ad7780 and ad7781. > >> > >> Signed-off-by: Renato Lui Geh > >> --- > >> drivers/staging/iio/adc/ad7780.c | 8 ++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/staging/iio/adc/ad7780.c > >> b/drivers/staging/iio/adc/ad7780.c > >> index 56c49e28f432..ad7617a3a141 100644 > >> --- a/drivers/staging/iio/adc/ad7780.c > >> +++ b/drivers/staging/iio/adc/ad7780.c > >> @@ -26,10 +26,14 @@ > >> #define AD7780_RDY BIT(7) > >> #define AD7780_FILTER BIT(6) > >> #define AD7780_ERR BIT(5) > >> -#define AD7780_ID1 BIT(4) > >> -#define AD7780_ID0 BIT(3) > >> #define AD7780_GAINBIT(2) > >> > >> +#define AD7170_ID 0 > >> +#define AD7171_ID 1 > >> +#define AD7780_ID 1 > >> +#define AD7781_ID 0 > >> + > >> +#define AD7780_ID_MASK (BIT(3) | BIT(4)) > > > >This also doesn't have any functionality change. > >The AD7170_ID, AD7171_ID, AD7780_ID & AD7781_ID IDs are also unused (maybe > >in a later patch they are ?). > > They aren't. I added them following a previous review suggestion. Should > I remove them? Can we check them? It's always useful to confirm that the device is the one you think it is. Then we can either use what is there with a suitable warning, or if that is tricky just fault out as the dt is giving us the wrong part number. J > > > >I would also leave the AD7780_ID1 & AD7780_ID0 definitions in place, since > >they're easier matched with the datasheet. > > > >> > >> #define AD7780_PATTERN_GOOD1 > >> #define AD7780_PATTERN_MASKGENMASK(1, 0) > >> -- > >> 2.21.0 > >>
Re: [PATCH v2 01/10] iio: Allow to read mount matrix from ACPI
On Thu, 21 Feb 2019 18:02:46 +0100
"H. Nikolaus Schaller" wrote:
> From: Andy Shevchenko
>
> Currently mount matrix is allowed in Device Tree, though there is
> no technical issue to extend it to support ACPI.
>
> Convert the function to use device_property_read_string_array() and
> thus allow to read mount matrix from ACPI if available.
>
> Example of use in _DSD method:
>
> Name (_DSD, Package ()
> {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package ()
> {
> Package () { "mount-matrix", Package() {
> "1", "0", "0",
> "0", "0.866", "0.5",
> "0", "-0.5", "0.866",
> } },
> }
> })
>
> At the same time drop the "of" prefix from its name and
> convert current users.
>
> No functional change intended.
>
> Signed-off-by: Andy Shevchenko
I'm not so very fussed about this one as Andy had posted it to the
list previously, but in theory you should have added your signed-off-by
as an intermediate person who handled the patch.
Otherwise, great and applied to the togreg branch of iio.git and
pushed out as testing for the autobuilders to see if we got anything
wrong.
Thanks,
Jonathan
> ---
> drivers/iio/accel/kxsd9.c | 4 +-
> drivers/iio/gyro/mpu3050-core.c| 3 +-
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 4 +-
> drivers/iio/industrialio-core.c| 46 +-
> drivers/iio/magnetometer/ak8974.c | 5 +--
> drivers/iio/magnetometer/ak8975.c | 5 +--
> include/linux/iio/iio.h| 4 +-
> 7 files changed, 28 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 0c0df4fce420..70c60db62247 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -420,9 +420,7 @@ int kxsd9_common_probe(struct device *dev,
> indio_dev->available_scan_masks = kxsd9_scan_masks;
>
> /* Read the mounting matrix, if present */
> - ret = of_iio_read_mount_matrix(dev,
> -"mount-matrix",
> -&st->orientation);
> + ret = iio_read_mount_matrix(dev, "mount-matrix", &st->orientation);
> if (ret)
> return ret;
>
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index 77fac81a3adc..8200e48f561b 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -1149,8 +1149,7 @@ int mpu3050_common_probe(struct device *dev,
> mpu3050->divisor = 99;
>
> /* Read the mounting matrix, if present */
> - ret = of_iio_read_mount_matrix(dev, "mount-matrix",
> -&mpu3050->orientation);
> + ret = iio_read_mount_matrix(dev, "mount-matrix", &mpu3050->orientation);
> if (ret)
> return ret;
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 1e428c196a82..533d1f8321ac 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -990,8 +990,8 @@ int inv_mpu_core_probe(struct regmap *regmap, int irq,
> const char *name,
>
> pdata = dev_get_platdata(dev);
> if (!pdata) {
> - result = of_iio_read_mount_matrix(dev, "mount-matrix",
> - &st->orientation);
> + result = iio_read_mount_matrix(dev, "mount-matrix",
> +&st->orientation);
> if (result) {
> dev_err(dev, "Failed to retrieve mounting matrix %d\n",
> result);
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 4f5cd9f60870..0a40f1df49c7 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -19,6 +19,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -525,8 +526,8 @@ ssize_t iio_show_mount_matrix(struct iio_dev *indio_dev,
> uintptr_t priv,
> EXPORT_SYMBOL_GPL(iio_show_mount_matrix);
>
> /**
> - * of_iio_read_mount_matrix() - retrieve iio device mounting matrix from
> - * device-tree "mount-matrix" property
> + * iio_read_mount_matrix() - retrieve iio device mounting matrix from
> + * device "mount-matrix" property
> * @dev: device the mounting matrix property is assigned to
> * @propname:device specific mounting matrix property name
> * @matrix: where to store retrieved matrix
> @@ -536,40 +537,29 @@ EXPORT_SYMBOL_GPL(iio_show_mount_matrix);
> *
> * Return: 0 if success, or a negative error code on failure.
> */
> -#ifdef CONFIG_OF
> -int of_iio_read_mount_matrix(const struct device *dev,
> - const char *propname,
> - s
Re: [PATCH] arm64: dts: rockchip: decrease rising edge time of UART2
On Sun, Mar 3, 2019 at 9:04 AM Katsuhiro Suzuki wrote:
>
> Hello Heiko,
>
> Thank you for comments.
>
> On 2019/03/03 22:19, Heiko Stuebner wrote:
> > Hi,
> >
> > Am Sonntag, 3. März 2019, 13:27:05 CET schrieb Katsuhiro Suzuki:
> >> This patch increases drive strength of UART2 from 3mA to 12mA for
> >> getting more faster rising edge.
> >>
> >> RockPro64 is using a very high speed rate (1.5Mbps) for UART2. In
> >> this setting, a bit width of UART is about 667ns.
> >>
> >> In my environment (RockPro64 UART2 with FTDI FT232RL UART-USB
> >> converter), falling time of RockPro64 UART2 is 40ns, but riging time
> >> is over 650ns. So UART receiver will get wrong data, because receiver
> >> read intermediate data of rising edge.
> >>
> >> Rising time becomes 300ns from 650ns if apply this patch. This is not
> >> perfect solution but better than now.
> >>
> >> Signed-off-by: Katsuhiro Suzuki
> >> ---
> >> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 9 +++--
> >> 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > your changing a core rk3399 property here, so I'd really like to get
> > input from other board stakeholders on this before applying a core
> > change.
> >
> > Could you either include the submitters of other rk3399-boards in the
> > recipient list so that they're aware or limit the change to rockpro64 for
> > the time being (aka overriding the property in the board-dts) please?
> >
>
> OK, I'm adding other boards members.
> by ./scripts/get_maintainer.pl arch/arm64/boot/dts/rockchip/rk3399-*.dts
>
>
> RockPro64 directly connect UART2 pins of RK3399 to external connector.
> I think maybe other RK3399 boards are facing same problem, but I cannot
> check it because I have RockPro64 only...
>
> I'm happy if someone tell me other boards situation.
I'm pulling out other rockchip boards momentarily to see what kind of
population we have.
Note these are not all running 5.x kernels, however none of them have
the UART2 drive levels modified to my knowledge, and regardless, none
show over 100 ns.
board:rise/fall
rk3399-roc-pc: 90ns/90ns
rk3399-rockpro64 V2.0: 90ns/45ns
rk3399-rockpro64 V2.1: 40ns/41ns
Please make sure there's not a large amount of flux or something
around the terminals on your board, that seems excessively high.
>
> Best Regards,
> Katsuhiro Suzuki
>
>
> > Thanks
> > Heiko
> >
> >
> >
> >> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >> b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >> index beaa92744a64..e3c8f91ead50 100644
> >> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> >> @@ -2000,6 +2000,11 @@
> >> drive-strength = <8>;
> >> };
> >>
> >> +pcfg_pull_up_12ma: pcfg-pull-up-12ma {
> >> +bias-pull-up;
> >> +drive-strength = <12>;
> >> +};
> >> +
> >> pcfg_pull_up_18ma: pcfg-pull-up-18ma {
> >> bias-pull-up;
> >> drive-strength = <18>;
> >> @@ -2521,8 +2526,8 @@
> >> uart2c {
> >> uart2c_xfer: uart2c-xfer {
> >> rockchip,pins =
> >> -<4 RK_PC3 RK_FUNC_1 &pcfg_pull_up>,
> >> -<4 RK_PC4 RK_FUNC_1 &pcfg_pull_none>;
> >> +<4 RK_PC3 RK_FUNC_1
> >> &pcfg_pull_up_12ma>,
> >> +<4 RK_PC4 RK_FUNC_1
> >> &pcfg_pull_none_12ma>;
> >> };
> >> };
> >>
> >>
> >
> >
> >
> >
> >
> >
>
>
> ___
> Linux-rockchip mailing list
> linux-rockc...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [RFC PATCH] KVM: arm64: Force a PTE mapping when logging is enabled
I think there're still some problems in this patch... Details below.
On Sat, Mar 2, 2019 at 11:39 AM Zenghui Yu wrote:
>
> The idea behind this is: we don't want to keep tracking of huge pages when
> logging_active is true, which will result in performance degradation. We
> still need to set vma_pagesize to PAGE_SIZE, so that we can make use of it
> to force a PTE mapping.
>
> Cc: Suzuki K Poulose
> Cc: Punit Agrawal
> Signed-off-by: Zenghui Yu
>
> ---
> Atfer looking into https://patchwork.codeaurora.org/patch/647985/ , the
> "vma_pagesize = PAGE_SIZE" logic was not intended to be deleted. As far
> as I can tell, we used to have "hugetlb" to force the PTE mapping, but
> we have "vma_pagesize" currently instead. We should set it properly for
> performance reasons (e.g, in VM migration). Did I miss something important?
>
> ---
> virt/kvm/arm/mmu.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/virt/kvm/arm/mmu.c b/virt/kvm/arm/mmu.c
> index 30251e2..7d41b16 100644
> --- a/virt/kvm/arm/mmu.c
> +++ b/virt/kvm/arm/mmu.c
> @@ -1705,6 +1705,13 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
> phys_addr_t fault_ipa,
> (vma_pagesize == PUD_SIZE && kvm_stage2_has_pmd(kvm))) &&
> !force_pte) {
> gfn = (fault_ipa & huge_page_mask(hstate_vma(vma))) >>
> PAGE_SHIFT;
> + } else {
> + /*
> +* Fallback to PTE if it's not one of the stage2
> +* supported hugepage sizes or the corresponding level
> +* doesn't exist, or logging is enabled.
First, Instead of "logging is enabled", it should be "force_pte is true",
since "force_pte" will be true when:
1) fault_supports_stage2_pmd_mappings() return false; or
2) "logging is enabled" (e.g, in VM migration).
Second, fallback some unsupported hugepage sizes (e.g, 64K hugepage with
4K pages) to PTE is somewhat strange. And it will then _unexpectedly_
reach transparent_hugepage_adjust(), though no real adjustment will happen
since commit fd2ef358282c ("KVM: arm/arm64: Ensure only THP is candidate
for adjustment"). Keeping "vma_pagesize" there as it is will be better,
right?
So I'd just simplify the logic like:
} else if (force_pte) {
vma_pagesize = PAGE_SIZE;
}
Will send a V2 later and waiting for your comments :)
thanks,
zenghui
> +*/
> + vma_pagesize = PAGE_SIZE;
> }
> up_read(¤t->mm->mmap_sem);
>
> --
> 1.8.3.1
>
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[PATCH] aio: prevent the final fput() in the middle of vfs_poll() (Re: KASAN: use-after-free Read in unix_dgram_poll)
On Sun, Mar 03, 2019 at 01:55:02PM +, Al Viro wrote:
> Maybe unrelated to this bug, but... What's to prevent a wakeup
> that happens just after we'd been added to a waitqueue by ->poll()
> triggering aio_poll_wake(), which gets to aio_poll_complete()
> with its fput() *before* we'd reached the end of ->poll() instance?
>
> I wonder if adding
> get_file(req->file); // make sure that early completion is safe
> right after
> req->file = fget(iocb->aio_fildes);
> if (unlikely(!req->file))
> return -EBADF;
> paired with
> fput(req->file);
> right after out: in aio_poll() is needed... Am I missing something
> obvious here? Christoph?
In more details - normally IOCB_CMD_POLL handling looks so:
1) io_submit(2) allocates aio_kiocb instance and passes it to aio_poll()
2) aio_poll() resolves the descriptor to struct file by
req->file = fget(iocb->aio_fildes)
3) aio_poll() sets ->woken to false and raises ->ki_refcnt of that
aio_kiocb to 2 (bumps by 1, that is).
4) aio_poll() calls vfs_poll(). After sanity checks (basically, "poll_wait()
had been called and only once") it locks the queue. That's what the extra
reference to iocb had been for - we know we can safely access it.
5) With queue locked, we check if ->woken has already been set to true
(by aio_poll_wake()) and, if it had been, we unlock the queue, drop
a reference to aio_kiocb and bugger off - at that point it's
a responsibility to aio_poll_wake() and the stuff called/scheduled by
it. That code will drop the reference to file in req->file, along
with the other reference to our aio_kiocb.
6) otherwise, we see whether we need to wait. If we do, we unlock
the queue, drop one reference to aio_kiocb and go away - eventual
wakeup (or cancel) will deal with the reference to file and with the
other reference to aio_kiocb
7) otherwise we remove ourselves from waitqueue (still under the queue
lock), so that wakeup won't get us. No async activity will be happening,
so we can safely drop req->file and iocb ourselves.
If wakeup happens while we are in vfs_poll(), we are fine - aio_kiocb
won't get freed under us, so we can do all the checks and locking safely.
And we don't touch ->file if we detect that case.
However, vfs_poll() most certainly *does* touch the file it had been
given. So wakeup coming while we are still in ->poll() might end up
doing fput() on that file. That case is not too rare, and usually
we are saved by the still present reference from descriptor table -
that fput() is not the final one. But if another thread closes that
descriptor right after our fget() and wakeup does happen before
->poll() returns, we are in trouble - final fput() done while we
are in the middle of a method.
What we need is to hold on to the file reference the same way we do with
aio_kiocb. No need to store the reference to what we'd got in a separate
variable - req->file is never reassigned and we'd already made sure that
req won't be freed under us, so dropping the extra reference with
fput(req->file) is fine in all cases.
Fixes: bfe4037e722ec
Cc: sta...@vger.kernel.org
Signed-off-by: Al Viro
---
diff --git a/fs/aio.c b/fs/aio.c
index 3083180a54c8..7e88bfabdac2 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1767,6 +1767,7 @@ static ssize_t aio_poll(struct aio_kiocb *aiocb, const
struct iocb *iocb)
/* one for removal from waitqueue, one for this function */
refcount_set(&aiocb->ki_refcnt, 2);
+ get_file(req->file);
mask = vfs_poll(req->file, &apt.pt) & req->events;
if (unlikely(!req->head)) {
@@ -1793,6 +1794,7 @@ static ssize_t aio_poll(struct aio_kiocb *aiocb, const
struct iocb *iocb)
spin_unlock_irq(&ctx->ctx_lock);
out:
+ fput(req->file);
if (unlikely(apt.error)) {
fput(req->file);
return apt.error;
Re: [PATCH v2 02/10] iio: document bindings for mounting matrices
On Thu, 21 Feb 2019 18:02:47 +0100
"H. Nikolaus Schaller" wrote:
> From: Linus Walleij
>
> The mounting matrix for sensors was introduced in
> commit dfc57732ad38 ("iio:core: mounting matrix support")
>
> However the device tree bindings are very terse and since this is
> a widely applicable property, we need a proper binding for it
> that the other bindings can reference. This will also be useful
> for other operating systems and sensor engineering at large.
>
> I think all 3D sensors should support it, the current situation
> is probably that the mounting information is confined in magic
> userspace components rather than using the mounting matrix, which
> is not good for portability and reuse.
>
> Cc: Linus Walleij
> Cc: Gregor Boirie
> Cc: Sebastian Reichel
> Cc: Samu Onkalo
> Cc: devicet...@vger.kernel.org
> Signed-off-by: Linus Walleij
> Signed-off-by: H. Nikolaus Schaller
Hi Nikolaus
A few minor notes inline.
> ---
> .../devicetree/bindings/iio/mount-matrix.txt | 204 ++
> 1 file changed, 204 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/mount-matrix.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/mount-matrix.txt
> b/Documentation/devicetree/bindings/iio/mount-matrix.txt
> new file mode 100644
> index ..1b64c8b1f689
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/mount-matrix.txt
> @@ -0,0 +1,204 @@
> +For discussion. Unclear are:
> +* is the definition of +/- values practical or counterintuitive?
> +* are the definitions unambiguous and easy to follow?
> +* are the examples correct?
> +* should we have HOWTO engineer a correct matrix for a new device (without
> comparing to a different one)?
> +
> +
> +
> +
> +Mounting matrix
> +
> +The mounting matrix is a device tree property used to orient any IIO device
Minor, but DT bindings are in theory not Linux specific and IIO is, so
should be "any device"
> +that produce three-dimensional data in relation to the world where it is
> +deployed.
> +
> +The purpose of the mounting matrix is to translate the sensor frame of
> +reference into the device frame of reference using a translation matrix as
> +defined in linear algebra.
> +
> +The typical usecase is that where a component has an internal representation
> +of the (x,y,z) triplets, such as different registers to read these
> coordinates,
> +and thus implying that the component should be mounted in a certain
> orientation
> +relative to some specific device frame of reference.
> +
> +For example a device with some kind of screen, where the user is supposed to
> +interact with the environment using an accelerometer, gyroscope or
> magnetometer
> +mounted on the same chassis as this screen, will likely take the screen as
> +reference to (x,y,z) orientation, with (x,y) corresponding to these axes on
> the
> +screen and (z) being depth, the axis perpendicular to the screen.
> +
> +For a screen you probably want (x) coordinates to go from negative on the
> left
> +to positive on the right, (y) from negative on the bottom to positive on top
> +and (z) depth to be negative under the screen and positive in front of it,
> +toward the face of the user.
> +
> +A sensor can be mounted in any angle along the axes relative to the frame of
> +reference. This means that the sensor may be flipped upside-down, left-right,
> +or tilted at any angle relative to the frame of reference.
> +
> +Another frame of reference is how the device with its sensor relates to the
> +external world, the environment where the device is deployed. Usually the
> data
> +from the sensor is used to figure out how the device is oriented with respect
> +to this world. When using the mounting matrix, the sensor and device
> orientation
> +becomes identical and we can focus on the data as it relates to the
> surrounding
> +world.
> +
> +Device-to-world examples for some three-dimensional sensor types:
> +
> +- Accelerometers have their world frame of reference toward the center of
> + gravity, usually to the core of the planet. A reading of the (x,y,z) values
> + from the sensor will give a projection of the gravity vector through the
> + device relative to the center of the planet, i.e. relative to its surface
> at
> + this point. Up and down in the world relative to the device frame of
> + reference can thus be determined. and users would likely expect a value of
> + 9.81 m/s^2 upwards along the (z) axis, i.e. out of the screen when the
> device
> + is held with its screen flat on the planets surface and 0 on the other
> axes,
> + as the gravity vector is projected 1:1 onto the sensors (z)-axis.
Nitpick: Screen is face down or face up? Someone might think a screen is
flat when looking up at them from the floor or the other way up.
I 'think' it's face down in the following...
> +
> + If you tilt the device, the g vector virtually coming out of the display
> + is projected onto the (x,y) plane of the display panel.
Re: [PATCH v2 03/10] iio: accel: bmc150: add mount matrix support
On Thu, 21 Feb 2019 18:02:48 +0100
"H. Nikolaus Schaller" wrote:
> This patch allows to read a mount-matrix device tree
> property and report to user-space or in-kernel iio
> clients.
>
> Signed-off-by: H. Nikolaus Schaller
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/accel/bmc150-accel-core.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/iio/accel/bmc150-accel-core.c
> b/drivers/iio/accel/bmc150-accel-core.c
> index 383c802eb5b8..b4e2d9b04e1d 100644
> --- a/drivers/iio/accel/bmc150-accel-core.c
> +++ b/drivers/iio/accel/bmc150-accel-core.c
> @@ -204,6 +204,7 @@ struct bmc150_accel_data {
> int ev_enable_state;
> int64_t timestamp, old_timestamp; /* Only used in hw fifo mode. */
> const struct bmc150_accel_chip_info *chip_info;
> + struct iio_mount_matrix orientation;
> };
>
> static const struct {
> @@ -796,6 +797,20 @@ static ssize_t bmc150_accel_get_fifo_state(struct device
> *dev,
> return sprintf(buf, "%d\n", state);
> }
>
> +static const struct iio_mount_matrix *
> +bmc150_accel_get_mount_matrix(const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct bmc150_accel_data *data = iio_priv(indio_dev);
> +
> + return &data->orientation;
> +}
> +
> +static const struct iio_chan_spec_ext_info bmc150_accel_ext_info[] = {
> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmc150_accel_get_mount_matrix),
> + { }
> +};
> +
> static IIO_CONST_ATTR(hwfifo_watermark_min, "1");
> static IIO_CONST_ATTR(hwfifo_watermark_max,
> __stringify(BMC150_ACCEL_FIFO_LENGTH));
> @@ -978,6 +993,7 @@ static const struct iio_event_spec bmc150_accel_event = {
> .shift = 16 - (bits), \
> .endianness = IIO_LE, \
> }, \
> + .ext_info = bmc150_accel_ext_info, \
> .event_spec = &bmc150_accel_event, \
> .num_event_specs = 1\
> }
> @@ -1555,6 +1571,11 @@ int bmc150_accel_core_probe(struct device *dev, struct
> regmap *regmap, int irq,
>
> data->regmap = regmap;
>
> + ret = iio_read_mount_matrix(dev, "mount-matrix",
> + &data->orientation);
> + if (ret)
> + return ret;
> +
> ret = bmc150_accel_chip_init(data);
> if (ret < 0)
> return ret;
Re: [PATCH][next] net/mlx5e: Remove redundant assignment
On 02/03/2019 21:39, Gustavo A. R. Silva wrote:
> Remove redundant assignment to tun_entropy->enabled.
>
> Addesses-Coverity-ID: 1477328 ("Unused value")
> Fixes: 97417f6182f8 ("net/mlx5e: Fix GRE key by controlling port tunnel
> entropy calculation")
the commit doesn't fix any real issue but is more of a cleanup.
so I'm not sure if fixes line is relevant or not.
beside that looks ok.
Reviewed-by: Roi Dayan
> Signed-off-by: Gustavo A. R. Silva
> ---
> drivers/net/ethernet/mellanox/mlx5/core/lib/port_tun.c | 6 ++
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/port_tun.c
> b/drivers/net/ethernet/mellanox/mlx5/core/lib/port_tun.c
> index 40f4a19b1ce1..be69c1d7941a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/port_tun.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/port_tun.c
> @@ -80,10 +80,8 @@ void mlx5_init_port_tun_entropy(struct mlx5_tun_entropy
> *tun_entropy,
> mlx5_query_port_tun_entropy(mdev, &entropy_flags);
> tun_entropy->num_enabling_entries = 0;
> tun_entropy->num_disabling_entries = 0;
> - tun_entropy->enabled = entropy_flags.calc_enabled;
> - tun_entropy->enabled =
> - (entropy_flags.calc_supported) ?
> - entropy_flags.calc_enabled : true;
> + tun_entropy->enabled = entropy_flags.calc_supported ?
> +entropy_flags.calc_enabled : true;
> }
>
> static int mlx5_set_entropy(struct mlx5_tun_entropy *tun_entropy,
>
Re: [PATCH v2 04/10] iio: accel: bma180: add mount matrix support
On Thu, 21 Feb 2019 18:02:49 +0100
"H. Nikolaus Schaller" wrote:
> This patch allows to read a mount-matrix device tree
> property and report to user-space or in-kernel iio
> clients.
>
> Signed-off-by: H. Nikolaus Schaller
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/accel/bma180.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
> index cb9765a3de60..f9720a1e8a7c 100644
> --- a/drivers/iio/accel/bma180.c
> +++ b/drivers/iio/accel/bma180.c
> @@ -116,6 +116,7 @@ struct bma180_data {
> struct i2c_client *client;
> struct iio_trigger *trig;
> const struct bma180_part_info *part_info;
> + struct iio_mount_matrix orientation;
> struct mutex mutex;
> bool sleep_state;
> int scale;
> @@ -561,6 +562,15 @@ static int bma180_set_power_mode(struct iio_dev
> *indio_dev,
> return ret;
> }
>
> +static const struct iio_mount_matrix *
> +bma180_accel_get_mount_matrix(const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct bma180_data *data = iio_priv(indio_dev);
> +
> + return &data->orientation;
> +}
> +
> static const struct iio_enum bma180_power_mode_enum = {
> .items = bma180_power_modes,
> .num_items = ARRAY_SIZE(bma180_power_modes),
> @@ -571,7 +581,8 @@ static const struct iio_enum bma180_power_mode_enum = {
> static const struct iio_chan_spec_ext_info bma180_ext_info[] = {
> IIO_ENUM("power_mode", true, &bma180_power_mode_enum),
> IIO_ENUM_AVAILABLE("power_mode", &bma180_power_mode_enum),
> - { },
> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bma180_accel_get_mount_matrix),
> + { }
> };
>
> #define BMA180_ACC_CHANNEL(_axis, _bits) { \
> @@ -722,6 +733,11 @@ static int bma180_probe(struct i2c_client *client,
> chip = id->driver_data;
> data->part_info = &bma180_part_info[chip];
>
> + ret = iio_read_mount_matrix(&client->dev, "mount-matrix",
> + &data->orientation);
> + if (ret)
> + return ret;
> +
> ret = data->part_info->chip_config(data);
> if (ret < 0)
> goto err_chip_disable;
Re: [PATCH v2 05/10] iio: gyro: bmg160: add mount matrix support
On Thu, 21 Feb 2019 18:02:50 +0100
"H. Nikolaus Schaller" wrote:
> This patch allows to read a mount-matrix device tree
> property and report to user-space or in-kernel iio
> clients.
>
> Signed-off-by: H. Nikolaus Schaller
Applied.
Thanks,
> ---
> drivers/iio/gyro/bmg160_core.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
> index 63ca31628a93..e7b38adee39a 100644
> --- a/drivers/iio/gyro/bmg160_core.c
> +++ b/drivers/iio/gyro/bmg160_core.c
> @@ -102,6 +102,7 @@ struct bmg160_data {
> struct regmap *regmap;
> struct iio_trigger *dready_trig;
> struct iio_trigger *motion_trig;
> + struct iio_mount_matrix orientation;
> struct mutex mutex;
> s16 buffer[8];
> u32 dps_range;
> @@ -794,6 +795,20 @@ static int bmg160_write_event_config(struct iio_dev
> *indio_dev,
> return 0;
> }
>
> +static const struct iio_mount_matrix *
> +bmg160_get_mount_matrix(const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct bmg160_data *data = iio_priv(indio_dev);
> +
> + return &data->orientation;
> +}
> +
> +static const struct iio_chan_spec_ext_info bmg160_ext_info[] = {
> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmg160_get_mount_matrix),
> + { }
> +};
> +
> static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("100 200 400 1000 2000");
>
> static IIO_CONST_ATTR(in_anglvel_scale_available,
> @@ -831,6 +846,7 @@ static const struct iio_event_spec bmg160_event = {
> .storagebits = 16, \
> .endianness = IIO_LE, \
> }, \
> + .ext_info = bmg160_ext_info,\
> .event_spec = &bmg160_event,\
> .num_event_specs = 1\
> }
> @@ -1075,6 +1091,11 @@ int bmg160_core_probe(struct device *dev, struct
> regmap *regmap, int irq,
> data->irq = irq;
> data->regmap = regmap;
>
> + ret = iio_read_mount_matrix(dev, "mount-matrix",
> + &data->orientation);
> + if (ret)
> + return ret;
> +
> ret = bmg160_chip_init(data);
> if (ret < 0)
> return ret;
Re: [PATCH v2 06/10] iio: gyro: itg3200: add mount matrix support
On Thu, 21 Feb 2019 18:02:51 +0100
"H. Nikolaus Schaller" wrote:
> This patch allows to read a mount-matrix device tree
> property and report to user-space or in-kernel iio
> clients.
>
> Signed-off-by: H. Nikolaus Schaller
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/gyro/itg3200_core.c | 20
> include/linux/iio/gyro/itg3200.h | 1 +
> 2 files changed, 21 insertions(+)
>
> diff --git a/drivers/iio/gyro/itg3200_core.c b/drivers/iio/gyro/itg3200_core.c
> index 7adecb562c81..203a6be33b70 100644
> --- a/drivers/iio/gyro/itg3200_core.c
> +++ b/drivers/iio/gyro/itg3200_core.c
> @@ -242,6 +242,20 @@ static int itg3200_initial_setup(struct iio_dev
> *indio_dev)
> return ret;
> }
>
> +static const struct iio_mount_matrix *
> +itg3200_get_mount_matrix(const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct itg3200 *data = iio_priv(indio_dev);
> +
> + return &data->orientation;
> +}
> +
> +static const struct iio_chan_spec_ext_info itg3200_ext_info[] = {
> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, itg3200_get_mount_matrix),
> + { }
> +};
> +
> #define ITG3200_ST \
> { .sign = 's', .realbits = 16, .storagebits = 16, .endianness = IIO_BE }
>
> @@ -255,6 +269,7 @@ static int itg3200_initial_setup(struct iio_dev
> *indio_dev)
> .address = ITG3200_REG_GYRO_ ## _mod ## OUT_H, \
> .scan_index = ITG3200_SCAN_GYRO_ ## _mod, \
> .scan_type = ITG3200_ST, \
> + .ext_info = itg3200_ext_info, \
> }
>
> static const struct iio_chan_spec itg3200_channels[] = {
> @@ -297,6 +312,11 @@ static int itg3200_probe(struct i2c_client *client,
>
> st = iio_priv(indio_dev);
>
> + ret = iio_read_mount_matrix(&client->dev, "mount-matrix",
> + &st->orientation);
> + if (ret)
> + return ret;
> +
> i2c_set_clientdata(client, indio_dev);
> st->i2c = client;
>
> diff --git a/include/linux/iio/gyro/itg3200.h
> b/include/linux/iio/gyro/itg3200.h
> index 2a820850f284..0a30fddccfb3 100644
> --- a/include/linux/iio/gyro/itg3200.h
> +++ b/include/linux/iio/gyro/itg3200.h
> @@ -104,6 +104,7 @@
> struct itg3200 {
> struct i2c_client *i2c;
> struct iio_trigger *trig;
> + struct iio_mount_matrix orientation;
> };
>
> enum ITG3200_SCAN_INDEX {
Re: [PATCH v2 07/10] iio: magnetometer: bmc150: add mount matrix support
On Thu, 21 Feb 2019 18:02:52 +0100
"H. Nikolaus Schaller" wrote:
> This patch allows to read a mount-matrix device tree
> property and report to user-space or in-kernel iio
> clients.
>
> Signed-off-by: H. Nikolaus Schaller
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/magnetometer/bmc150_magn.c | 21 +
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/iio/magnetometer/bmc150_magn.c
> b/drivers/iio/magnetometer/bmc150_magn.c
> index d91cb845e3d6..b0d8b036d9bb 100644
> --- a/drivers/iio/magnetometer/bmc150_magn.c
> +++ b/drivers/iio/magnetometer/bmc150_magn.c
> @@ -143,6 +143,7 @@ struct bmc150_magn_data {
>*/
> struct mutex mutex;
> struct regmap *regmap;
> + struct iio_mount_matrix orientation;
> /* 4 x 32 bits for x, y z, 4 bytes align, 64 bits timestamp */
> s32 buffer[6];
> struct iio_trigger *dready_trig;
> @@ -612,6 +613,20 @@ static ssize_t bmc150_magn_show_samp_freq_avail(struct
> device *dev,
> return len;
> }
>
> +static const struct iio_mount_matrix *
> +bmc150_magn_get_mount_matrix(const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct bmc150_magn_data *data = iio_priv(indio_dev);
> +
> + return &data->orientation;
> +}
> +
> +static const struct iio_chan_spec_ext_info bmc150_magn_ext_info[] = {
> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmc150_magn_get_mount_matrix),
> + { }
> +};
> +
> static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(bmc150_magn_show_samp_freq_avail);
>
> static struct attribute *bmc150_magn_attributes[] = {
> @@ -638,6 +653,7 @@ static const struct attribute_group
> bmc150_magn_attrs_group = {
> .storagebits = 32, \
> .endianness = IIO_LE\
> }, \
> + .ext_info = bmc150_magn_ext_info, \
> }
>
> static const struct iio_chan_spec bmc150_magn_channels[] = {
> @@ -861,6 +877,11 @@ int bmc150_magn_probe(struct device *dev, struct regmap
> *regmap,
> data->irq = irq;
> data->dev = dev;
>
> + ret = iio_read_mount_matrix(dev, "mount-matrix",
> + &data->orientation);
> + if (ret)
> + return ret;
> +
> if (!name && ACPI_HANDLE(dev))
> name = bmc150_magn_match_acpi_device(dev);
>
Re: [PATCH v2 08/10] iio: magnetometer: hmc5843: add mount matrix support
On Thu, 21 Feb 2019 18:02:53 +0100
"H. Nikolaus Schaller" wrote:
> This patch allows to read a mount-matrix device tree
> property and report to user-space or in-kernel iio
> clients.
>
> Signed-off-by: H. Nikolaus Schaller
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/magnetometer/hmc5843.h | 1 +
> drivers/iio/magnetometer/hmc5843_core.c | 20 ++--
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/hmc5843.h
> b/drivers/iio/magnetometer/hmc5843.h
> index a75224cf99df..e3e22d2508d3 100644
> --- a/drivers/iio/magnetometer/hmc5843.h
> +++ b/drivers/iio/magnetometer/hmc5843.h
> @@ -43,6 +43,7 @@ struct hmc5843_data {
> struct mutex lock;
> struct regmap *regmap;
> const struct hmc5843_chip_info *variant;
> + struct iio_mount_matrix orientation;
> __be16 buffer[8];
> };
>
> diff --git a/drivers/iio/magnetometer/hmc5843_core.c
> b/drivers/iio/magnetometer/hmc5843_core.c
> index ada142fb7aa3..05629ec56d80 100644
> --- a/drivers/iio/magnetometer/hmc5843_core.c
> +++ b/drivers/iio/magnetometer/hmc5843_core.c
> @@ -237,6 +237,15 @@ int hmc5843_set_measurement_configuration(struct iio_dev
> *indio_dev,
> return hmc5843_set_meas_conf(data, meas_conf);
> }
>
> +static const struct iio_mount_matrix *
> +hmc5843_get_mount_matrix(const struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan)
> +{
> + struct hmc5843_data *data = iio_priv(indio_dev);
> +
> + return &data->orientation;
> +}
> +
> static const struct iio_enum hmc5843_meas_conf_enum = {
> .items = hmc5843_meas_conf_modes,
> .num_items = ARRAY_SIZE(hmc5843_meas_conf_modes),
> @@ -247,7 +256,8 @@ static const struct iio_enum hmc5843_meas_conf_enum = {
> static const struct iio_chan_spec_ext_info hmc5843_ext_info[] = {
> IIO_ENUM("meas_conf", true, &hmc5843_meas_conf_enum),
> IIO_ENUM_AVAILABLE("meas_conf", &hmc5843_meas_conf_enum),
> - { },
> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, hmc5843_get_mount_matrix),
> + { }
> };
>
> static const struct iio_enum hmc5983_meas_conf_enum = {
> @@ -260,7 +270,8 @@ static const struct iio_enum hmc5983_meas_conf_enum = {
> static const struct iio_chan_spec_ext_info hmc5983_ext_info[] = {
> IIO_ENUM("meas_conf", true, &hmc5983_meas_conf_enum),
> IIO_ENUM_AVAILABLE("meas_conf", &hmc5983_meas_conf_enum),
> - { },
> + IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, hmc5843_get_mount_matrix),
> + { }
> };
>
> static
> @@ -635,6 +646,11 @@ int hmc5843_common_probe(struct device *dev, struct
> regmap *regmap,
> data->variant = &hmc5843_chip_info_tbl[id];
> mutex_init(&data->lock);
>
> + ret = iio_read_mount_matrix(dev, "mount-matrix",
> + &data->orientation);
> + if (ret)
> + return ret;
> +
> indio_dev->dev.parent = dev;
> indio_dev->name = name;
> indio_dev->info = &hmc5843_info;
Re: [mt76/mt7603/mac] Question about missing variable assignment
On 3/3/19 4:05 AM, Felix Fietkau wrote:
> On 2019-03-02 22:10, Gustavo A. R. Silva wrote:
>> Hi all,
>>
>> The following piece of code in
>> drivers/net/wireless/mediatek/mt76/mt7603/mac.c
>> is missing a variable assignment before line 1058. Notice that there
>> is a potential execution path in which variable *i* is compared against
>> magic number 15 at line 1075 without being initialized previously
>> (this was reported by Coverity):
>>
>> 1055 out:
>> 1056 final_rate_flags = info->status.rates[final_idx].flags;
>> 1057
>> 1058 switch (FIELD_GET(MT_TX_RATE_MODE, final_rate)) {
>> 1059 case MT_PHY_TYPE_CCK:
>> 1060 cck = true;
>> 1061 /* fall through */
>> 1062 case MT_PHY_TYPE_OFDM:
>> 1063 if (dev->mt76.chandef.chan->band == NL80211_BAND_5GHZ)
>> 1064 sband = &dev->mt76.sband_5g.sband;
>> 1065 else
>> 1066 sband = &dev->mt76.sband_2g.sband;
>> 1067 final_rate &= GENMASK(5, 0);
>> 1068 final_rate = mt7603_get_rate(dev, sband, final_rate,
>> cck);
>> 1069 final_rate_flags = 0;
>> 1070 break;
>> 1071 case MT_PHY_TYPE_HT_GF:
>> 1072 case MT_PHY_TYPE_HT:
>> 1073 final_rate_flags |= IEEE80211_TX_RC_MCS;
>> 1074 final_rate &= GENMASK(5, 0);
>> 1075 if (i > 15)
>> 1076 return false;
>> 1077 break;
>> 1078 default:
>> 1079 return false;
>> 1080 }
>>
>> My guess is that such missing assignment should be something similar
>> to the one at line 566:
>>
>> i = FIELD_GET(MT_RXV1_TX_RATE, rxdg0);
>>
>> but I'm not sure what the proper arguments for macro FIELD_GET should
>> be.
>>
>> This code was introduced by commit c8846e1015022d2531ac4c895783e400b3e5babe
>>
>> What do you think?
> Thanks for reporting this. The fix is simpler than that, the check
> should be: if (final_rate > 15)
> I will send a fix.
>
Great. Glad to help. :)
Thanks
--
Gustavo
Re: [PATCH v2 10/10] iio: ak8975: improve code readability
On Thu, 21 Feb 2019 18:02:55 +0100
"H. Nikolaus Schaller" wrote:
> - use temporary variable in get_mount_matrix()
> - remove , after { }
>
> Signed-off-by: H. Nikolaus Schaller
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/magnetometer/ak8975.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/magnetometer/ak8975.c
> b/drivers/iio/magnetometer/ak8975.c
> index db7214ac514c..43d08c089792 100644
> --- a/drivers/iio/magnetometer/ak8975.c
> +++ b/drivers/iio/magnetometer/ak8975.c
> @@ -746,12 +746,14 @@ static const struct iio_mount_matrix *
> ak8975_get_mount_matrix(const struct iio_dev *indio_dev,
> const struct iio_chan_spec *chan)
> {
> - return &((struct ak8975_data *)iio_priv(indio_dev))->orientation;
> + struct ak8975_data *data = iio_priv(indio_dev);
> +
> + return &data->orientation;
> }
>
> static const struct iio_chan_spec_ext_info ak8975_ext_info[] = {
> IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, ak8975_get_mount_matrix),
> - { },
> + { }
> };
>
> #define AK8975_CHANNEL(axis, index) \
> @@ -792,7 +794,7 @@ static const struct acpi_device_id ak_acpi_match[] = {
> {"AK09911", AK09911},
> {"AKM9911", AK09911},
> {"AK09912", AK09912},
> - { },
> + { }
> };
> MODULE_DEVICE_TABLE(acpi, ak_acpi_match);
> #endif
Re: [PATCH v2 09/10] iio: mpu6050: improve code readability
On Thu, 21 Feb 2019 18:02:54 +0100
"H. Nikolaus Schaller" wrote:
> - use temporary variable in get_mount_matrix()
> - remove , after { }
>
> Signed-off-by: H. Nikolaus Schaller
Applied.
Thanks,
Jonathan
> ---
> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> index 533d1f8321ac..e7721b6fb441 100644
> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
> @@ -765,12 +765,14 @@ static const struct iio_mount_matrix *
> inv_get_mount_matrix(const struct iio_dev *indio_dev,
>const struct iio_chan_spec *chan)
> {
> - return &((struct inv_mpu6050_state *)iio_priv(indio_dev))->orientation;
> + struct inv_mpu6050_state *data = iio_priv(indio_dev);
> +
> + return &data->orientation;
> }
>
> static const struct iio_chan_spec_ext_info inv_ext_info[] = {
> IIO_MOUNT_MATRIX(IIO_SHARED_BY_TYPE, inv_get_mount_matrix),
> - { },
> + { }
> };
>
> #define INV_MPU6050_CHAN(_type, _channel2, _index)\
Re: [PATCH v2 00/10] iio mount matrix - revitalize missing bindings documentation and provide code for bmc150, bmg160, bma180, itg3200, hmc584x
On Fri, 22 Feb 2019 16:48:14 +0200 Andy Shevchenko wrote: > On Thu, Feb 21, 2019 at 7:04 PM H. Nikolaus Schaller > wrote: > > > > Fixes V2: > > * make get_mount_matrix() functions more readable (use temp variable) > > (suggested by Jonathan and Andy) > > * add these readability improvements also for ak8975 and mpu6050 > > (suggested by Jonathan and Andy) > > * squash bindings documentation into single commit for better discussion > > (suggested by Linus) > > * FOR DISCUSSION: add some more clarifications to the bindings documentation > > and an attempt to define the magnetometer orientation > > * add "iio: Allow to read mount matrix from ACPI" to the beginning of > > the series to make it compile > > (suggested by Andy) > > * replace of_iio_read_mount_matrix() by iio_read_mount_matrix() > > (required by "iio: Allow to read mount matrix from ACPI") > > * drop patch to convert bma180 to devm (potential race) > > (suggested by Jonathan) > > > > PATCH V1 2019-02-20 15:01:02: > > This patch series adds the mount-matrix to several iio sensor drivers > > used in handheld devices. > > > > The mount-matrix translates the quite arbitrary orientation of the sensor > > on some printed circuit board to user-tangible orientation in handheld > > devices that relates to typical screen orientation. > > > > There was a bindings documentation by Linus Walleij but the patch > > did not make it into mainline. Therefore I resend it here. > > > > Next I have added some clarifications (at least I hope it clarifies) > > in a second patch. > > > > Finally, the patch set implements the hooks for the mount matrix > > in several iio drivers: bmc150, bma180, bmg160, itg3200, hmc5843. > > This includes also one patch for the bma180 to convert it to devm API. > > > > We use them in different variants of the omap3-gta04 so a separate > > patch set will provide device tree additions for them. > > > > Thank you, it looks nice. > FWIW, > Reviewed-by: Andy Shevchenko > Thanks to Linus and Andy for reviews. I've picked up all but they documentation patch. I think it is 'very nearly there', but would like a few really minor tweaks. Seemed silly to stall the other patches on that though! Thanks, Jonathan
Re: [PATCH v2 4/4] staging: iio: ad5933: move out of staging
Hi Marcelo, Thank you for the patch! Yet something to improve: [auto build test ERROR on iio/togreg] [cannot apply to v5.0-rc8] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Marcelo-Schmitt/staging-iio-ad5933-move-out-of-staging/20190303-232039 base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg config: xtensa-allyesconfig compiler: xtensa-linux-gcc (GCC) 8.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross GCC_VERSION=8.2.0 make.cross ARCH=xtensa allyesconfig GCC_VERSION=8.2.0 make.cross ARCH=xtensa All errors (new ones prefixed by >>): >> drivers/staging/iio/Kconfig:12: can't open file >> "drivers/staging/iio/impedance-analyzer/Kconfig" make[2]: *** [allyesconfig] Error 1 make[1]: *** [allyesconfig] Error 2 make: *** [sub-make] Error 2 -- >> drivers/staging/iio/Kconfig:12: can't open file >> "drivers/staging/iio/impedance-analyzer/Kconfig" make[2]: *** [oldconfig] Error 1 make[1]: *** [oldconfig] Error 2 make: *** [sub-make] Error 2 -- >> drivers/staging/iio/Kconfig:12: can't open file >> "drivers/staging/iio/impedance-analyzer/Kconfig" make[2]: *** [olddefconfig] Error 1 make[1]: *** [olddefconfig] Error 2 make: *** [sub-make] Error 2 vim +12 drivers/staging/iio/Kconfig f94aa354d6 Michael Hennerich 2011-08-02 @12 source "drivers/staging/iio/impedance-analyzer/Kconfig" 09434ef7c2 Barry Song2010-10-27 13 source "drivers/staging/iio/meter/Kconfig" f46d9f154a Graf Yang 2010-10-27 14 source "drivers/staging/iio/resolver/Kconfig" 7f3a1fb998 Jonathan Cameron 2009-08-18 15 :: The code at line 12 was first introduced by commit :: f94aa354d676532448e8e222e737fdd0755fc786 iio: impedance-analyzer: New driver for AD5933/4 Impedance Converter, Network Analyzer :: TO: Michael Hennerich :: CC: Greg Kroah-Hartman --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
This report is for an older version of the patch so ignore it. The issue is already resolved. On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot wrote: > > Hi Joel, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [also build test ERROR on v5.0-rc8] > [cannot apply to next-20190301] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850 > config: sh-allmodconfig (attached as .config) > compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0 > reproduce: > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=8.2.0 make.cross ARCH=sh > > All errors (new ones prefixed by >>): > > >> find: 'arch/sh/kernel/module.lds': No such file or directory > >> find: 'arch/sh/kernel/module.lds': No such file or directory > > --- > 0-DAY kernel test infrastructureOpen Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation > > -- > You received this message because you are subscribed to the Google Groups > "kernel-team" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to kernel-team+unsubscr...@android.com.
Re: [PATCH][next] iio: st_accel: remove redundant unsigned less than zero check
On Thu, 21 Feb 2019 10:46:36 + Colin King wrote: > From: Colin Ian King > > The check that variable val is less than zero is redundant since val > is an unsigned int and hence can never be less than zero. Remove it. > > Signed-off-by: Colin Ian King Applied. Thanks, Jonathan > --- > drivers/iio/accel/st_accel_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/accel/st_accel_core.c > b/drivers/iio/accel/st_accel_core.c > index a3c0916479fa..9930edf423bf 100644 > --- a/drivers/iio/accel/st_accel_core.c > +++ b/drivers/iio/accel/st_accel_core.c > @@ -992,7 +992,7 @@ static int apply_acpi_orientation(struct iio_dev > *indio_dev, > goto out; > > val = elements[i].integer.value; > - if (val < 0 || val > 2) > + if (val > 2) > goto out; > > /* Avoiding full matrix multiplication, we simply reorder the
Re: [PATCH] io: accel: kxcjk1013: restore the range after resume.
On Thu, 21 Feb 2019 03:13:44 + "He, Bo" wrote: > restore the range register in case kxcjk1013 power is off after suspend > > we see the issue on some laptops, after system suspend and resume, > the CTRL_REG1 register changed from 0xc8 to 0x80, so acceleration range > is changed, the patch is to restore the acceleration range after resume. > > Signed-off-by: he, bo > Signed-off-by: Chen, Hu Please don't do the ret += trick, it obscures the return value which we may want to know if anything goes wrong. Handle each error independently. Thanks, Jonathan > --- > drivers/iio/accel/kxcjk-1013.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c > index 7506bd9..c6bb3be 100644 > --- a/drivers/iio/accel/kxcjk-1013.c > +++ b/drivers/iio/accel/kxcjk-1013.c > @@ -1340,6 +1340,7 @@ static int kxcjk1013_resume(struct device *dev) > > mutex_lock(&data->mutex); > ret = kxcjk1013_set_mode(data, OPERATION); > + ret += kxcjk1013_set_range(data, data->range); > mutex_unlock(&data->mutex); > > return ret;
Re: KASAN: use-after-free Read in get_mem_cgroup_from_mm
Hi, guys I also hit the following issue. but it fails to reproduce the issue by the log. it seems to the case that we access the mm->owner and deference it will result in the UAF. But it should not be possible that we specify the incomplete process to be the mm->owner. Any thoughts? Thanks, zhong jiang On 2018/12/4 23:43, syzbot wrote: > syzbot has found a reproducer for the following crash on: > > HEAD commit:0072a0c14d5b Merge tag 'media/v4.20-4' of git://git.kernel.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=11c885a340 > kernel config: https://syzkaller.appspot.com/x/.config?x=b9cc5a440391cbfd > dashboard link: https://syzkaller.appspot.com/bug?extid=cbb52e396df3e565ab02 > compiler: gcc (GCC) 8.0.1 20180413 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12835e2540 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=172fa5a340 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+cbb52e396df3e565a...@syzkaller.appspotmail.com > > cgroup: fork rejected by pids controller in /syz2 > == > BUG: KASAN: use-after-free in __read_once_size include/linux/compiler.h:182 > [inline] > BUG: KASAN: use-after-free in task_css include/linux/cgroup.h:477 [inline] > BUG: KASAN: use-after-free in mem_cgroup_from_task mm/memcontrol.c:815 > [inline] > BUG: KASAN: use-after-free in get_mem_cgroup_from_mm.part.62+0x6d7/0x880 > mm/memcontrol.c:844 > Read of size 8 at addr 8881b72af310 by task syz-executor198/9332 > > CPU: 0 PID: 9332 Comm: syz-executor198 Not tainted 4.20.0-rc5+ #142 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS > Google 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x244/0x39d lib/dump_stack.c:113 > print_address_description.cold.7+0x9/0x1ff mm/kasan/report.c:256 > kasan_report_error mm/kasan/report.c:354 [inline] > kasan_report.cold.8+0x242/0x309 mm/kasan/report.c:412 > __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433 > __read_once_size include/linux/compiler.h:182 [inline] > task_css include/linux/cgroup.h:477 [inline] > mem_cgroup_from_task mm/memcontrol.c:815 [inline] > get_mem_cgroup_from_mm.part.62+0x6d7/0x880 mm/memcontrol.c:844 > get_mem_cgroup_from_mm mm/memcontrol.c:834 [inline] > mem_cgroup_try_charge+0x608/0xe20 mm/memcontrol.c:5888 > mcopy_atomic_pte mm/userfaultfd.c:71 [inline] > mfill_atomic_pte mm/userfaultfd.c:418 [inline] > __mcopy_atomic mm/userfaultfd.c:559 [inline] > mcopy_atomic+0xb08/0x2c70 mm/userfaultfd.c:609 > userfaultfd_copy fs/userfaultfd.c:1705 [inline] > userfaultfd_ioctl+0x29fb/0x5610 fs/userfaultfd.c:1851 > vfs_ioctl fs/ioctl.c:46 [inline] > file_ioctl fs/ioctl.c:509 [inline] > do_vfs_ioctl+0x1de/0x1790 fs/ioctl.c:696 > ksys_ioctl+0xa9/0xd0 fs/ioctl.c:713 > __do_sys_ioctl fs/ioctl.c:720 [inline] > __se_sys_ioctl fs/ioctl.c:718 [inline] > __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718 > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > RIP: 0033:0x44c7e9 > Code: 5d c5 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 > 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f > 83 2b c5 fb ff c3 66 2e 0f 1f 84 00 00 00 00 > RSP: 002b:7f906b69fdb8 EFLAGS: 0246 ORIG_RAX: 0010 > RAX: ffda RBX: 006e4a08 RCX: 0044c7e9 > RDX: 2100 RSI: c028aa03 RDI: 0004 > RBP: 006e4a00 R08: R09: > R10: R11: 0246 R12: 006e4a0c > R13: 7ffdfd47813f R14: 7f906b6a09c0 R15: 002d > > Allocated by task 9325: > save_stack+0x43/0xd0 mm/kasan/kasan.c:448 > set_track mm/kasan/kasan.c:460 [inline] > kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553 > kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490 > kmem_cache_alloc_node+0x144/0x730 mm/slab.c:3644 > alloc_task_struct_node kernel/fork.c:158 [inline] > dup_task_struct kernel/fork.c:843 [inline] > copy_process+0x2026/0x87a0 kernel/fork.c:1751 > _do_fork+0x1cb/0x11d0 kernel/fork.c:2216 > __do_sys_clone kernel/fork.c:2323 [inline] > __se_sys_clone kernel/fork.c:2317 [inline] > __x64_sys_clone+0xbf/0x150 kernel/fork.c:2317 > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > > Freed by task 9325: > save_stack+0x43/0xd0 mm/kasan/kasan.c:448 > set_track mm/kasan/kasan.c:460 [inline] > __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521 > kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528 > __cache_free mm/slab.c:3498 [inline] > kmem_cache_free+0x83/0x290 mm/slab.c:3760 > free_task_struct kernel/fork.c:163 [inline] > free_task+0x16e/0x1f0 kernel/fork.c:457 > copy_process+0x1dcc/0x87a0 kernel/fork.c:2148 > _do_fork+0x1cb/0x11d0 ker
Re: [PATCH] iio: cros_ec_accel_legacy: Refactor code in cros_ec_accel_legacy_probe
On Thu, 21 Feb 2019 09:03:39 +0100
Enric Balletbo i Serra wrote:
> On 21/2/19 8:00, Kees Cook wrote:
> > On Wed, Feb 20, 2019 at 6:06 PM Gustavo A. R. Silva
> > wrote:
> >>
> >> Refactor some code in order to fix both the technical implementation
> >> and the following warnings:
> >>
> >> drivers/iio/accel/cros_ec_accel_legacy.c: In function
> >> ‘cros_ec_accel_legacy_probe’:
> >> drivers/iio/accel/cros_ec_accel_legacy.c:387:36: warning: this statement
> >> may fall through [-Wimplicit-fallthrough=]
> >> ec_accel_channels[X].scan_index = Y;
> >> ^~~
> >> drivers/iio/accel/cros_ec_accel_legacy.c:388:3: note: here
> >>case Y:
> >>^~~~
> >> drivers/iio/accel/cros_ec_accel_legacy.c:389:36: warning: this statement
> >> may fall through [-Wimplicit-fallthrough=]
> >> ec_accel_channels[Y].scan_index = X;
> >> ^~~
> >> drivers/iio/accel/cros_ec_accel_legacy.c:390:3: note: here
> >>case Z:
> >>^~~~
> >>
> >> Notice that neither the for loop nor the switch statement is needed.
> >> Also, "state->sign[Y] = 1" should be unconditional.
> >>
> >> This patch is part of the ongoing efforts to enable
> >> -Wimplicit-fallthrough.
> >>
> >> Signed-off-by: Gustavo A. R. Silva
> >
> > Acked-by: Kees Cook
> >
>
> Acked-by: Enric Balletbo i Serra
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
This will probably only make it upstream at the 'next' merge window
given the timing.
Jonathan
>
> Thanks,
> Enric
>
> > -Kees
> >
> >> ---
> >> drivers/iio/accel/cros_ec_accel_legacy.c | 27 +++-
> >> 1 file changed, 12 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> index 063e89eff791..021f9f5cd3bb 100644
> >> --- a/drivers/iio/accel/cros_ec_accel_legacy.c
> >> +++ b/drivers/iio/accel/cros_ec_accel_legacy.c
> >> @@ -353,7 +353,7 @@ static int cros_ec_accel_legacy_probe(struct
> >> platform_device *pdev)
> >> struct cros_ec_sensor_platform *sensor_platform =
> >> dev_get_platdata(dev);
> >> struct iio_dev *indio_dev;
> >> struct cros_ec_accel_legacy_state *state;
> >> - int ret, i;
> >> + int ret;
> >>
> >> if (!ec || !ec->ec_dev) {
> >> dev_warn(&pdev->dev, "No EC device found.\n");
> >> @@ -381,20 +381,17 @@ static int cros_ec_accel_legacy_probe(struct
> >> platform_device *pdev)
> >> * Present the channel using HTML5 standard:
> >> * need to invert X and Y and invert some lid axis.
> >> */
> >> - for (i = X ; i < MAX_AXIS; i++) {
> >> - switch (i) {
> >> - case X:
> >> - ec_accel_channels[X].scan_index = Y;
> >> - case Y:
> >> - ec_accel_channels[Y].scan_index = X;
> >> - case Z:
> >> - ec_accel_channels[Z].scan_index = Z;
> >> - }
> >> - if (state->sensor_num == MOTIONSENSE_LOC_LID && i != Y)
> >> - state->sign[i] = -1;
> >> - else
> >> - state->sign[i] = 1;
> >> - }
> >> + ec_accel_channels[X].scan_index = Y;
> >> + ec_accel_channels[Y].scan_index = X;
> >> + ec_accel_channels[Z].scan_index = Z;
> >> +
> >> + state->sign[Y] = 1;
> >> +
> >> + if (state->sensor_num == MOTIONSENSE_LOC_LID)
> >> + state->sign[X] = state->sign[Z] = -1;
> >> + else
> >> + state->sign[X] = state->sign[Z] = 1;
> >> +
> >> indio_dev->num_channels = ARRAY_SIZE(ec_accel_channels);
> >> indio_dev->dev.parent = &pdev->dev;
> >> indio_dev->info = &cros_ec_accel_legacy_info;
> >> --
> >> 2.20.1
> >>
> >
> >
Re: [PATCH] net: ipv6: add socket option IPV6_ROUTER_ALERT_ISOLATE
On 3/1/19 4:31 PM, Francesco Ruggeri wrote: > By default IPv6 socket with IPV6_ROUTER_ALERT socket option set will > receive all IPv6 RA packets from all namespaces. > IPV6_ROUTER_ALERT_ISOLATE socket option restricts packets received by > the socket to be only from the socket's namespace. > > Signed-off-by: Maxim Martynov > Signed-off-by: Francesco Ruggeri > --- > include/linux/ipv6.h | 3 ++- > include/uapi/linux/in6.h | 1 + > net/ipv6/ip6_output.c| 6 ++ > net/ipv6/ipv6_sockglue.c | 10 ++ > 4 files changed, 19 insertions(+), 1 deletion(-) > Reviewed-by: David Ahern
[PATCH 9/9] HID: intel-ish-hid: Use the new interface functions in HID ish client
Only include intel-ish-client-if.h, which has all interfaces required to
implment ISHTP client. There is no longer any direct field access from
core ISHTP only include files.
Signed-off-by: Srinivas Pandruvada
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 68 +---
drivers/hid/intel-ish-hid/ishtp-hid.c| 2 -
drivers/hid/intel-ish-hid/ishtp-hid.h| 6 +-
3 files changed, 35 insertions(+), 41 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 9084fea4c247..5b98e36e3a81 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -17,8 +17,6 @@
#include
#include
#include
-#include "ishtp/ishtp-dev.h"
-#include "ishtp/client.h"
#include "ishtp-hid.h"
/* Rx ring buffer pool size */
@@ -40,7 +38,7 @@ static void report_bad_packet(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
size_t cur_pos, size_t payload_len)
{
struct hostif_msg *recv_msg = recv_buf;
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
dev_err(cl_data_to_dev(client_data), "[hid-ish]: BAD packet %02X\n"
"total_bad=%u cur_pos=%u\n"
@@ -77,7 +75,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void
*recv_buf,
struct report_list *reports_list;
char *reports;
size_t report_len;
- struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl);
int curr_hid_dev = client_data->cur_hid_dev;
payload = recv_buf + sizeof(struct hostif_msg_hdr);
@@ -91,7 +89,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void
*recv_buf,
(unsigned int)data_len,
(unsigned int)sizeof(struct hostif_msg_hdr));
++client_data->bad_recv_cnt;
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
@@ -104,7 +102,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
++client_data->bad_recv_cnt;
report_bad_packet(hid_ishtp_cl, recv_msg, cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
@@ -119,7 +117,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+
ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
client_data->hid_dev_count = (unsigned int)*payload;
@@ -168,7 +166,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+
ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
if (!client_data->hid_descr[curr_hid_dev])
@@ -193,7 +191,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
report_bad_packet(hid_ishtp_cl, recv_msg,
cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+
ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
if (!client_data->report_descr[curr_hid_dev])
@@ -298,7 +296,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
++client_data->bad_recv_cnt;
report_bad_packet(hid_ishtp_cl, recv_msg, cur_pos,
payload_len);
- ish_hw_reset(hid_ishtp_cl->dev);
+ ish_hw_reset(ishtp_get_ishtp_device(hid_ishtp_cl));
break;
}
@@ -478,7 +476,7 @@ int ishtp_hid_link_ready_wait(struct ishtp_cl_data
*client_data)
[PATCH 5/9] HID: intel-ish-hid: Store ishtp_cl_device instance in device
Store ishtp_cl_device pointer in device struct private data. In this way we can get ishtp_cl_device * from device struct pointer. Signed-off-by: Srinivas Pandruvada --- drivers/hid/intel-ish-hid/ishtp/bus.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c index 95a97534fcda..2ca65864192f 100644 --- a/drivers/hid/intel-ish-hid/ishtp/bus.c +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c @@ -466,6 +466,7 @@ static struct ishtp_cl_device *ishtp_bus_add_device(struct ishtp_device *dev, } ishtp_device_ready = true; + dev_set_drvdata(&device->dev, device); return device; } -- 2.17.2
[PATCH 4/9] HID: intel-ish-hid: Move driver registry functions
Move the driver registry with the ishtp bus to the common interface
file, which clients can include.
Also rename __ishtp_cl_driver_register() to ishtp_cl_driver_register()
and removed define for ishtp_cl_driver_register.
Signed-off-by: Srinivas Pandruvada
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 2 +-
drivers/hid/intel-ish-hid/ishtp/bus.c| 8 +++---
drivers/hid/intel-ish-hid/ishtp/bus.h| 27 +---
include/linux/intel-ish-client-if.h | 25 ++
4 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 287d71338b91..9084fea4c247 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -938,7 +938,7 @@ static int __init ish_hid_init(void)
int rv;
/* Register ISHTP client device driver with ISHTP Bus */
- rv = ishtp_cl_driver_register(&hid_ishtp_cl_driver);
+ rv = ishtp_cl_driver_register(&hid_ishtp_cl_driver, THIS_MODULE);
return rv;
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c
b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 308853eab91c..95a97534fcda 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -485,7 +485,7 @@ static void ishtp_bus_remove_device(struct ishtp_cl_device
*device)
}
/**
- * __ishtp_cl_driver_register() - Client driver register
+ * ishtp_cl_driver_register() - Client driver register
* @driver:the client driver instance
* @owner: Owner of this driver module
*
@@ -494,8 +494,8 @@ static void ishtp_bus_remove_device(struct ishtp_cl_device
*device)
*
* Return: Return value of driver_register or -ENODEV if not ready
*/
-int __ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
- struct module *owner)
+int ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
+struct module *owner)
{
int err;
@@ -512,7 +512,7 @@ int __ishtp_cl_driver_register(struct ishtp_cl_driver
*driver,
return 0;
}
-EXPORT_SYMBOL(__ishtp_cl_driver_register);
+EXPORT_SYMBOL(ishtp_cl_driver_register);
/**
* ishtp_cl_driver_unregister() - Client driver unregister
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h
b/drivers/hid/intel-ish-hid/ishtp/bus.h
index c96e7fb42f01..4aed195719de 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -17,6 +17,7 @@
#include
#include
+#include
struct ishtp_cl;
struct ishtp_cl_device;
@@ -52,26 +53,6 @@ struct ishtp_cl_device {
void (*event_cb)(struct ishtp_cl_device *device);
};
-/**
- * struct ishtp_cl_device - ISHTP device handle
- * @driver:driver instance on a bus
- * @name: Name of the device for probe
- * @probe: driver callback for device probe
- * @remove:driver callback on device removal
- *
- * Client drivers defines to get probed/removed for ISHTP client device.
- */
-struct ishtp_cl_driver {
- struct device_driver driver;
- const char *name;
- const guid_t *guid;
- int (*probe)(struct ishtp_cl_device *dev);
- int (*remove)(struct ishtp_cl_device *dev);
- int (*reset)(struct ishtp_cl_device *dev);
- const struct dev_pm_ops *pm;
-};
-
-
intishtp_bus_new_client(struct ishtp_device *dev);
void ishtp_remove_all_clients(struct ishtp_device *dev);
intishtp_cl_device_bind(struct ishtp_cl *cl);
@@ -105,12 +86,6 @@ voidishtp_get_device(struct ishtp_cl_device *);
void ishtp_set_drvdata(struct ishtp_cl_device *cl_device, void *data);
void *ishtp_get_drvdata(struct ishtp_cl_device *cl_device);
-int__ishtp_cl_driver_register(struct ishtp_cl_driver *driver,
- struct module *owner);
-#define ishtp_cl_driver_register(driver) \
- __ishtp_cl_driver_register(driver, THIS_MODULE)
-void ishtp_cl_driver_unregister(struct ishtp_cl_driver *driver);
-
intishtp_register_event_cb(struct ishtp_cl_device *device,
void (*read_cb)(struct ishtp_cl_device *));
intishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t *cuuid);
diff --git a/include/linux/intel-ish-client-if.h
b/include/linux/intel-ish-client-if.h
index 11e285172735..abc0b8122f07 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -10,6 +10,31 @@
struct ishtp_cl_device;
+/**
+ * struct ishtp_cl_device - ISHTP device handle
+ * @driver:driver instance on a bus
+ * @name: Name of the device for probe
+ * @probe: driver callback for device probe
+ * @remove:driver callback on device removal
+ *
+ * Client drivers defines to get probed/removed for ISHTP client device.
+ */
+struct ishtp_cl_driver {
+ struct device_driver driver;
+ const char *name;
+ const guid_t *guid;
+
[PATCH 7/9] HID: intel-ish-hid: Add interface functions for struct ishtp_cl
Instead of directly accessing members of struct ishtp_cl, create interface
functions to access them.
Signed-off-by: Srinivas Pandruvada
---
drivers/hid/intel-ish-hid/ishtp/client.c | 42
include/linux/intel-ish-client-if.h | 7
2 files changed, 49 insertions(+)
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c
b/drivers/hid/intel-ish-hid/ishtp/client.c
index 657b46dcefa6..b7ac5e3b1e82 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -1063,3 +1063,45 @@ void recv_ishtp_cl_msg_dma(struct ishtp_device *dev,
void *msg,
eoi:
return;
}
+
+void *ishtp_get_client_data(struct ishtp_cl *cl)
+{
+ return cl->client_data;
+}
+EXPORT_SYMBOL(ishtp_get_client_data);
+
+void ishtp_set_client_data(struct ishtp_cl *cl, void *data)
+{
+ cl->client_data = data;
+}
+EXPORT_SYMBOL(ishtp_set_client_data);
+
+struct ishtp_device *ishtp_get_ishtp_device(struct ishtp_cl *cl)
+{
+ return cl->dev;
+}
+EXPORT_SYMBOL(ishtp_get_ishtp_device);
+
+void ishtp_set_tx_ring_size(struct ishtp_cl *cl, int size)
+{
+ cl->tx_ring_size = size;
+}
+EXPORT_SYMBOL(ishtp_set_tx_ring_size);
+
+void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size)
+{
+ cl->rx_ring_size = size;
+}
+EXPORT_SYMBOL(ishtp_set_rx_ring_size);
+
+void ishtp_set_connection_state(struct ishtp_cl *cl, int state)
+{
+ cl->state = state;
+}
+EXPORT_SYMBOL(ishtp_set_connection_state);
+
+void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id)
+{
+ cl->fw_client_id = fw_client_id;
+}
+EXPORT_SYMBOL(ishtp_cl_set_fw_client_id);
diff --git a/include/linux/intel-ish-client-if.h
b/include/linux/intel-ish-client-if.h
index 7ce172f656f8..526e3048e09f 100644
--- a/include/linux/intel-ish-client-if.h
+++ b/include/linux/intel-ish-client-if.h
@@ -87,5 +87,12 @@ int ishtp_cl_flush_queues(struct ishtp_cl *cl);
int ishtp_cl_io_rb_recycle(struct ishtp_cl_rb *rb);
bool ishtp_cl_tx_empty(struct ishtp_cl *cl);
struct ishtp_cl_rb *ishtp_cl_rx_get_rb(struct ishtp_cl *cl);
+void *ishtp_get_client_data(struct ishtp_cl *cl);
+void ishtp_set_client_data(struct ishtp_cl *cl, void *data);
+struct ishtp_device *ishtp_get_ishtp_device(struct ishtp_cl *cl);
+void ishtp_set_tx_ring_size(struct ishtp_cl *cl, int size);
+void ishtp_set_rx_ring_size(struct ishtp_cl *cl, int size);
+void ishtp_set_connection_state(struct ishtp_cl *cl, int state);
+void ishtp_cl_set_fw_client_id(struct ishtp_cl *cl, int fw_client_id);
#endif /* _INTEL_ISH_CLIENT_IF_H_ */
--
2.17.2
Re: [PATCH] iio: cros_ec: Fix gyro scale calculation
On Fri, 22 Feb 2019 11:24:24 +0100
Enric Balletbo i Serra wrote:
> Hi Jonathan,
>
> On 20/2/19 17:01, Jonathan Cameron wrote:
> > On Wed, 20 Feb 2019 16:03:00 +0100
> > Enric Balletbo i Serra wrote:
> >
> >> From: Gwendal Grignou
> >>
> >> Calculation was copied from IIO_DEGREE_TO_RAD, but offset added to avoid
> >> rounding error is wrong. It should be only half of the divider.
> >>
> >> Fixes: c14dca07a31d ("iio: cros_ec_sensors: add ChromeOS EC Contiguous
> >> Sensors driver")
> >> Signed-off-by: Gwendal Grignou
> >> Signed-off-by: Enric Balletbo i Serra
> >
> > This one is kind of interesting. See below.
> >
> >> ---
> >>
> >> drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> >> b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> >> index 89cb0066a6e0..600942af9f9c 100644
> >> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> >> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> >> @@ -103,7 +103,7 @@ static int cros_ec_sensors_read(struct iio_dev
> >> *indio_dev,
> >> * Do not use IIO_DEGREE_TO_RAD to avoid precision
> >> * loss. Round to the nearest integer.
> >> */
> >> - *val = div_s64(val64 * 314159 + 900ULL, 1000);
> >> + *val = div_s64(val64 * 314159 + 500ULL, 1000);
> > That is only one of two divides going on. Firstly we divide by 1000 here,
> > then we provide it in fractional form which means that the actual value you
> > get
> > from sysfs etc is
> > val/val2. It's this one we are protecting against rounding error on I
> > guess.
> > Now this is even less obviously because it's not 18000 either, but
> > 18000 * 2^CROS_EC_SENSOR_BITS.
> >
> > Which ultimately means neither answer is correct. Hmm.
> > Not totally sure what the right answer actually is..
> >
>
> If I understood well the Gwendal's patch the problem that we're trying to
> solve
> is that current calculation is not closer from the float calculation.
>
> For 1000dps, the result should be:
>
> (1000 * pi ) / 180 >> 15 ~= 0.000532632218
>
> But with current calculation we get
>
> $ cat scale
> 0.000547890
>
> With that patch (modifying the offset to avoid the rounding error) we get a
> closer result
>
> $ cat scale
> 0.000532631
>
> So, what we're trying to do is have val/val2 closer to the real value. Makes
> this sense to you or I'm missing something? I can improve the commit message
> if
> it's not clear.
I think we are in enough of a mess here with the different dividers that we
should just do the maths here, then we can avoid the bia.
aiming for nano value.
val * pi * 10e12 / (180 * 2^15)
div_s64(val * 3141592653000 + 2949120, 5898240) = 532632
vs 532632 for floating point division.
Then use IIO_INT_PLUS_NANO to return it.
Even then I suspect the +2949120 is only effecting the last digit so
you could probably drop it safely enough.
I'd certainly rather we had all the magic in one place rather than
trying to correct for divisions that aren't apparent here.
>
> -- Enric
>
> > Jonathan
> >
> >>*val2 = 18000 << (CROS_EC_SENSOR_BITS - 1);
> >>ret = IIO_VAL_FRACTIONAL;
> >>break;
> >
[PATCH 1/9] HID: intel-ish-hid: Add match callback to ishtp bus type
From: Hong Liu
Currently we depend on the guid check in ishtp_cl_driver.probe to match
the device and driver. However Linux device core first calls the match()
callback to decide the matching of driver and device, and then does some
preparation before calling the driver probe function. If we return error
in the driver probe, it needs to tear down all the preparation work and
retry with next driver.
Adding the match callback can avoid the unnecessary entry into unmatched
driver probe function for ishtp clients reported by FW.
Signed-off-by: Hong Liu
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +
drivers/hid/intel-ish-hid/ishtp/bus.c| 21
drivers/hid/intel-ish-hid/ishtp/bus.h| 1 +
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index 30fe0c5e6fad..fbb9d85ba6df 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -788,10 +788,6 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device
*cl_device)
if (!cl_device)
return -ENODEV;
- if (!guid_equal(&hid_ishtp_guid,
- &cl_device->fw_client->props.protocol_name))
- return -ENODEV;
-
client_data = devm_kzalloc(&cl_device->dev, sizeof(*client_data),
GFP_KERNEL);
if (!client_data)
@@ -922,6 +918,7 @@ static const struct dev_pm_ops hid_ishtp_pm_ops = {
static struct ishtp_cl_driver hid_ishtp_cl_driver = {
.name = "ish-hid",
+ .guid = &hid_ishtp_guid,
.probe = hid_ishtp_cl_probe,
.remove = hid_ishtp_cl_remove,
.reset = hid_ishtp_cl_reset,
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c
b/drivers/hid/intel-ish-hid/ishtp/bus.c
index d5f4b6438d86..6348fee8aadc 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -219,6 +219,26 @@ static int ishtp_cl_device_probe(struct device *dev)
return driver->probe(device);
}
+/**
+ * ishtp_cl_bus_match() - Bus match() callback
+ * @dev: the device structure
+ * @drv: the driver structure
+ *
+ * This is a bus match callback, called when a new ishtp_cl_device is
+ * registered during ishtp bus client enumeration. Use the guid_t in
+ * drv and dev to decide whether they match or not.
+ *
+ * Return: 1 if dev & drv matches, 0 otherwise.
+ */
+static int ishtp_cl_bus_match(struct device *dev, struct device_driver *drv)
+{
+ struct ishtp_cl_device *device = to_ishtp_cl_device(dev);
+ struct ishtp_cl_driver *driver = to_ishtp_cl_driver(drv);
+
+ return guid_equal(driver->guid,
+ &device->fw_client->props.protocol_name);
+}
+
/**
* ishtp_cl_device_remove() - Bus remove() callback
* @dev: the device structure
@@ -372,6 +392,7 @@ static struct bus_type ishtp_cl_bus_type = {
.name = "ishtp",
.dev_groups = ishtp_cl_dev_groups,
.probe = ishtp_cl_device_probe,
+ .match = ishtp_cl_bus_match,
.remove = ishtp_cl_device_remove,
.pm = &ishtp_cl_bus_dev_pm_ops,
.uevent = ishtp_cl_uevent,
diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h
b/drivers/hid/intel-ish-hid/ishtp/bus.h
index 4cf7ad586c37..c96e7fb42f01 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.h
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
@@ -64,6 +64,7 @@ struct ishtp_cl_device {
struct ishtp_cl_driver {
struct device_driver driver;
const char *name;
+ const guid_t *guid;
int (*probe)(struct ishtp_cl_device *dev);
int (*remove)(struct ishtp_cl_device *dev);
int (*reset)(struct ishtp_cl_device *dev);
--
2.17.2
[PATCH 2/9] HID: intel-ish-hid: Hide members of struct ishtp_cl_device
ISH clients don't need to access any field of struct ishtp_cl_device. To
avoid this create an interface functions instead where it is required.
In the case of ishtp_cl_allocate(), modify the parameters so that the
clients don't have to dereference.
Clients can also use tracing, here a new interface is added to get the
common trace function pointer, instead of direct call.
The new interface functions defined in one external header file, named
intel-ish-client-if.h. This is the only header files all ISHTP clients
must include.
Signed-off-by: Srinivas Pandruvada
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 56 +++-
drivers/hid/intel-ish-hid/ishtp-hid.c| 4 +-
drivers/hid/intel-ish-hid/ishtp-hid.h| 2 +-
drivers/hid/intel-ish-hid/ishtp/bus.c| 27 ++
drivers/hid/intel-ish-hid/ishtp/client.c | 4 +-
drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
include/linux/intel-ish-client-if.h | 18 +++
7 files changed, 84 insertions(+), 29 deletions(-)
create mode 100644 include/linux/intel-ish-client-if.h
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index fbb9d85ba6df..ffed7c91bebf 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -15,6 +15,7 @@
#include
#include
+#include
#include
#include "ishtp/ishtp-dev.h"
#include "ishtp/client.h"
@@ -24,6 +25,8 @@
#define HID_CL_RX_RING_SIZE32
#define HID_CL_TX_RING_SIZE16
+#define cl_data_to_dev(client_data) ishtp_device(client_data->cl_device)
+
/**
* report_bad_packets() - Report bad packets
* @hid_ishtp_cl: Client instance to get stats
@@ -39,7 +42,7 @@ static void report_bad_packet(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
struct hostif_msg *recv_msg = recv_buf;
struct ishtp_cl_data *client_data = hid_ishtp_cl->client_data;
- dev_err(&client_data->cl_device->dev, "[hid-ish]: BAD packet %02X\n"
+ dev_err(cl_data_to_dev(client_data), "[hid-ish]: BAD packet %02X\n"
"total_bad=%u cur_pos=%u\n"
"[%02X %02X %02X %02X]\n"
"payload_len=%u\n"
@@ -83,7 +86,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void
*recv_buf,
do {
if (cur_pos + sizeof(struct hostif_msg) > total_len) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: error, received %u which is less
than data header %u\n",
(unsigned int)data_len,
(unsigned int)sizeof(struct hostif_msg_hdr));
@@ -122,12 +125,12 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
client_data->hid_dev_count = (unsigned int)*payload;
if (!client_data->hid_devices)
client_data->hid_devices = devm_kcalloc(
- &client_data->cl_device->dev,
+ cl_data_to_dev(client_data),
client_data->hid_dev_count,
sizeof(struct device_info),
GFP_KERNEL);
if (!client_data->hid_devices) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"Mem alloc failed for hid device info\n");
wake_up_interruptible(&client_data->init_wait);
break;
@@ -135,7 +138,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
for (i = 0; i < client_data->hid_dev_count; ++i) {
if (1 + sizeof(struct device_info) * i >=
payload_len) {
- dev_err(&client_data->cl_device->dev,
+ dev_err(cl_data_to_dev(client_data),
"[hid-ish]: [ENUM_DEVICES]:
content size %zu is bigger than payload_len %zu\n",
1 + sizeof(struct device_info)
* i, payload_len);
@@ -170,7 +173,7 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl,
void *recv_buf,
}
if (!client_data->hid_descr[curr_hid_dev])
client_data->hid_descr[curr_hid_dev] =
- devm_kmalloc(&client_data->cl_device->dev,
+ devm_kmalloc(cl_data_to_dev(client_data),
[PATCH 0/9] HID: intel-ish-hid: Clean up external interfaces
FOR kernel v5.2+. No functional changes are expected with this series. I am posting this now because of usage of ISH in ChromeOS Embedded Controller. https://lkml.org/lkml/2019/2/24/26 I want to make sure that API is restricted before more development and posting there. Currently only one ISH client is using ISH transport. But it is changing now with the development of other clients using ISH transport. Some of these clients which are targeted for Linux based OS only laptops, are not using HID to export sensors. As more clients are getting developed it is important that the external interface for ISH transport only allows what clients need. Currently the header files used by clients "client.h" and "ishtp-dev.h" include other ISH header files. Also clients access fields from structure which also has other fields which are only used by ISH transort. So this series introduces one header file "linux/intel-ish-client-if.h". This header files doesn't include any other ISH transport header files. There are interface functions defined so that clients never have to directly access any ISH transort structures. Also clients don't have to match there GUID in probe. They will be only probbed if their GUID matches, which is passed as driver registry. Hong Liu (1): HID: intel-ish-hid: Add match callback to ishtp bus type Srinivas Pandruvada (8): HID: intel-ish-hid: Hide members of struct ishtp_cl_device HID: intel-ish-hid: Simplify ishtp_cl_link() HID: intel-ish-hid: Move driver registry functions HID: intel-ish-hid: Store ishtp_cl_device instance in device HID: intel-ish-hid: Move the common functions from client.h HID: intel-ish-hid: Add interface functions for struct ishtp_cl HID: intel-ish-hid: Move functions related to bus and device HID: intel-ish-hid: Use the new interface functions in HID ish client drivers/hid/intel-ish-hid/ishtp-hid-client.c | 131 ++- drivers/hid/intel-ish-hid/ishtp-hid.c| 6 +- drivers/hid/intel-ish-hid/ishtp-hid.h| 6 +- drivers/hid/intel-ish-hid/ishtp/bus.c| 83 +++- drivers/hid/intel-ish-hid/ishtp/bus.h| 37 +- drivers/hid/intel-ish-hid/ishtp/client.c | 60 +++-- drivers/hid/intel-ish-hid/ishtp/client.h | 24 drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 31 - include/linux/intel-ish-client-if.h | 110 9 files changed, 310 insertions(+), 178 deletions(-) create mode 100644 include/linux/intel-ish-client-if.h -- 2.17.2
[PATCH 3/9] HID: intel-ish-hid: Simplify ishtp_cl_link()
All callers will only use ISHTP_HOST_CLIENT_ID_ANY, so get rid of
option to pass this additional id.
Signed-off-by: Srinivas Pandruvada
---
drivers/hid/intel-ish-hid/ishtp-hid-client.c | 2 +-
drivers/hid/intel-ish-hid/ishtp/client.c | 14 --
drivers/hid/intel-ish-hid/ishtp/client.h | 2 +-
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
index ffed7c91bebf..287d71338b91 100644
--- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
+++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
@@ -637,7 +637,7 @@ static int hid_ishtp_cl_init(struct ishtp_cl *hid_ishtp_cl,
int reset)
dev_dbg(cl_data_to_dev(client_data), "%s\n", __func__);
hid_ishtp_trace(client_data, "%s reset flag: %d\n", __func__, reset);
- rv = ishtp_cl_link(hid_ishtp_cl, ISHTP_HOST_CLIENT_ID_ANY);
+ rv = ishtp_cl_link(hid_ishtp_cl);
if (rv) {
dev_err(cl_data_to_dev(client_data),
"ishtp_cl_link failed\n");
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c
b/drivers/hid/intel-ish-hid/ishtp/client.c
index 760e48a368a8..657b46dcefa6 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.c
+++ b/drivers/hid/intel-ish-hid/ishtp/client.c
@@ -168,9 +168,6 @@ EXPORT_SYMBOL(ishtp_cl_free);
/**
* ishtp_cl_link() - Reserve a host id and link the client instance
* @cl: client device instance
- * @id: host client id to use. It can be ISHTP_HOST_CLIENT_ID_ANY if any
- * id from the available can be used
- *
*
* This allocates a single bit in the hostmap. This function will make sure
* that not many client sessions are opened at the same time. Once allocated
@@ -179,11 +176,11 @@ EXPORT_SYMBOL(ishtp_cl_free);
*
* Return: 0 or error code on failure
*/
-int ishtp_cl_link(struct ishtp_cl *cl, int id)
+int ishtp_cl_link(struct ishtp_cl *cl)
{
struct ishtp_device *dev;
- unsigned long flags, flags_cl;
- int ret = 0;
+ unsigned long flags, flags_cl;
+ int id, ret = 0;
if (WARN_ON(!cl || !cl->dev))
return -EINVAL;
@@ -197,10 +194,7 @@ int ishtp_cl_link(struct ishtp_cl *cl, int id)
goto unlock_dev;
}
- /* If Id is not assigned get one*/
- if (id == ISHTP_HOST_CLIENT_ID_ANY)
- id = find_first_zero_bit(dev->host_clients_map,
- ISHTP_CLIENTS_MAX);
+ id = find_first_zero_bit(dev->host_clients_map, ISHTP_CLIENTS_MAX);
if (id >= ISHTP_CLIENTS_MAX) {
spin_unlock_irqrestore(&dev->device_lock, flags);
diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h
b/drivers/hid/intel-ish-hid/ishtp/client.h
index 992625891a6c..afa8b1f521d0 100644
--- a/drivers/hid/intel-ish-hid/ishtp/client.h
+++ b/drivers/hid/intel-ish-hid/ishtp/client.h
@@ -172,7 +172,7 @@ static inline bool ishtp_cl_cmp_id(const struct ishtp_cl
*cl1,
/* exported functions from ISHTP under client management scope */
struct ishtp_cl *ishtp_cl_allocate(struct ishtp_cl_device *cl_device);
void ishtp_cl_free(struct ishtp_cl *cl);
-int ishtp_cl_link(struct ishtp_cl *cl, int id);
+int ishtp_cl_link(struct ishtp_cl *cl);
void ishtp_cl_unlink(struct ishtp_cl *cl);
int ishtp_cl_disconnect(struct ishtp_cl *cl);
int ishtp_cl_connect(struct ishtp_cl *cl);
--
2.17.2
