Re: [PATCH v2 3/4] media: i2c: Add TDA1997x HDMI receiver driver
On 10/18/2017 02:04 PM, Hans Verkuil wrote:
> Hi Tim,
>
> Here is my review of this v2:
>
> On 10/12/17 06:45, Tim Harvey wrote:
>> Add support for the TDA1997x HDMI receivers.
>>
>> Cc: Hans Verkuil
>> Signed-off-by: Tim Harvey
>> ---
>> v2:
>> - implement dv timings enum/cap
>> - remove deprecated g_mbus_config op
>> - fix dv_query_timings
>> - add EDID get/set handling
>> - remove max-pixel-rate support
>> - add audio codec DAI support
>> - use new audio bindings
>>
>> ---
>> drivers/media/i2c/Kconfig|9 +
>> drivers/media/i2c/Makefile |1 +
>> drivers/media/i2c/tda1997x.c | 3336
>> ++
>> include/dt-bindings/media/tda1997x.h | 78 +
>> include/media/i2c/tda1997x.h | 53 +
>> 5 files changed, 3477 insertions(+)
>> create mode 100644 drivers/media/i2c/tda1997x.c
>> create mode 100644 include/dt-bindings/media/tda1997x.h
>> create mode 100644 include/media/i2c/tda1997x.h
>>
>> +/* parse an infoframe and do some sanity checks on it */
>> +static unsigned int
>> +tda1997x_parse_infoframe(struct tda1997x_state *state, u16 addr)
>> +{
>> +struct v4l2_subdev *sd = &state->sd;
>> +union hdmi_infoframe frame;
>> +u8 buffer[40];
>> +u8 reg;
>> +int len, err;
>> +
>> +/* read data */
>> +len = io_readn(sd, addr, sizeof(buffer), buffer);
>> +err = hdmi_infoframe_unpack(&frame, buffer);
>> +if (err) {
>> +v4l_err(state->client,
>> +"failed parsing %d byte infoframe: 0x%04x/0x%02x\n",
>> +len, addr, buffer[0]);
>> +return err;
>> +}
>> +if (debug > 1)
>> +hdmi_infoframe_log(KERN_INFO, &state->client->dev, &frame);
>> +switch (frame.any.type) {
>> +/* Audio InfoFrame: see HDMI spec 8.2.2 */
>> +case HDMI_INFOFRAME_TYPE_AUDIO:
>> +/* sample rate */
>> +switch (frame.audio.sample_frequency) {
>> +case HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
>> +state->audio_samplerate = 32000;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
>> +state->audio_samplerate = 44100;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
>> +state->audio_samplerate = 48000;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
>> +state->audio_samplerate = 88200;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
>> +state->audio_samplerate = 96000;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_FREQUENCY_176400:
>> +state->audio_samplerate = 176400;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_FREQUENCY_192000:
>> +state->audio_samplerate = 192000;
>> +break;
>> +default:
>> +case HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM:
>> +break;
>> +}
>> +
>> +/* sample size */
>> +switch (frame.audio.sample_size) {
>> +case HDMI_AUDIO_SAMPLE_SIZE_16:
>> +state->audio_samplesize = 16;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_SIZE_20:
>> +state->audio_samplesize = 20;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_SIZE_24:
>> +state->audio_samplesize = 24;
>> +break;
>> +case HDMI_AUDIO_SAMPLE_SIZE_STREAM:
>> +default:
>> +break;
>> +}
>> +
>> +/* Channel Count */
>> +state->audio_channels = frame.audio.channels;
>> +if (frame.audio.channel_allocation &&
>> +frame.audio.channel_allocation != state->audio_ch_alloc) {
>> +/* use the channel assignment from the infoframe */
>> +state->audio_ch_alloc = frame.audio.channel_allocation;
>> +tda1997x_configure_audout(sd, state->audio_ch_alloc);
>> +/* reset the audio FIFO */
>> +tda1997x_hdmi_info_reset(sd, RESET_AUDIO, false);
>> +}
>> +break;
>> +
>> +/* Source Product Descriptor information (SPD) */
>> +case HDMI_INFOFRAME_TYPE_SPD:
>> +strncpy(frame.spd.vendor, state->vendor,
>> +sizeof(frame.spd.vendor));
>> +strncpy(frame.spd.product, state->product,
>> +sizeof(frame.spd.product));
>> +v4l_info(state->client, "Source Product Descriptor: %s %s\n",
>> + state->vendor, state->product);
>
> Use hdmi_infoframe_log() for logging infoframes.
>
>> +break;
>> +
>> +/* Auxiliary Video information (AVI) InfoFrame: see HDMI spec 8.2.1 */
>> +case HDMI_INF
Re: [PATCH 0/2] [media] rc/keymaps: add support for two RCs of hisilicon boards.
On 10/18/2017 12:54 PM, Jiancheng Xue wrote: > Add support for two remote controllers of hisilicon boards. > > Younian Wang (2): > [media] rc/keymaps: add support for RC of hisilicon TV demo boards > [media] rc/keymaps: add support for RC of hisilicon poplar board > > drivers/media/rc/keymaps/Makefile | 2 + > drivers/media/rc/keymaps/rc-hisi-poplar.c | 58 + > drivers/media/rc/keymaps/rc-hisi-tv-demo.c | 70 > ++ > 3 files changed, 130 insertions(+) > create mode 100644 drivers/media/rc/keymaps/rc-hisi-poplar.c > create mode 100644 drivers/media/rc/keymaps/rc-hisi-tv-demo.c > Did you make a mistake? You reposted these two patches, but still without any copyright statement... I think something went wrong here. Regards, Hans
[PATCH v2 1/4] completion: Add support for initializing completion with lockdep_map
Sometimes, we want to initialize completions with sparate lockdep maps
to assign lock classes under control. For example, the workqueue code
manages lockdep maps, as it can classify lockdep maps properly.
Provided a function for that purpose.
Signed-off-by: Byungchul Park
---
include/linux/completion.h | 8
1 file changed, 8 insertions(+)
diff --git a/include/linux/completion.h b/include/linux/completion.h
index cae5400..182d56e 100644
--- a/include/linux/completion.h
+++ b/include/linux/completion.h
@@ -49,6 +49,13 @@ static inline void complete_release_commit(struct completion
*x)
lock_commit_crosslock((struct lockdep_map *)&x->map);
}
+#define init_completion_with_map(x, m) \
+do { \
+ lockdep_init_map_crosslock((struct lockdep_map *)&(x)->map, \
+ (m)->name, (m)->key, 0);
\
+ __init_completion(x); \
+} while (0)
+
#define init_completion(x) \
do { \
static struct lock_class_key __key; \
@@ -58,6 +65,7 @@ static inline void complete_release_commit(struct completion
*x)
__init_completion(x); \
} while (0)
#else
+#define init_completion_with_map(x, m) __init_completion(x)
#define init_completion(x) __init_completion(x)
static inline void complete_acquire(struct completion *x) {}
static inline void complete_release(struct completion *x) {}
--
1.9.1
[PATCH v2 0/4] Fix false positives by cross-release feature
I attached this patchset into another thread for patches fixing a performance regression by cross-release, so that the cross-release can be re-enabled easily as the last, after fixing false positives as well. Changes from v1 - Separate a patch removing white space Byungchul Park (4): completion: Add support for initializing completion with lockdep_map lockdep: Remove unnecessary acquisitions wrt workqueue flush genhd.h: Remove trailing white space lockdep: Assign a lock_class per gendisk used for wait_for_completion() block/bio.c| 2 +- block/genhd.c | 13 + include/linux/completion.h | 8 include/linux/genhd.h | 26 ++ include/linux/workqueue.h | 4 ++-- kernel/workqueue.c | 20 6 files changed, 42 insertions(+), 31 deletions(-) -- 1.9.1
[PATCH v2 4/4] lockdep: Assign a lock_class per gendisk used for wait_for_completion()
Darrick and Dave Chinner posted the following warning:
> ==
> WARNING: possible circular locking dependency detected
> 4.14.0-rc1-fixes #1 Tainted: GW
> --
> loop0/31693 is trying to acquire lock:
> (&(&ip->i_mmaplock)->mr_lock){}, at: []
> xfs_ilock+0x23c/0x330 [xfs]
>
> but now in release context of a crosslock acquired at the following:
> ((complete)&ret.event){+.+.}, at: []
> submit_bio_wait+0x7f/0xb0
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #2 ((complete)&ret.event){+.+.}:
>lock_acquire+0xab/0x200
>wait_for_completion_io+0x4e/0x1a0
>submit_bio_wait+0x7f/0xb0
>blkdev_issue_zeroout+0x71/0xa0
>xfs_bmapi_convert_unwritten+0x11f/0x1d0 [xfs]
>xfs_bmapi_write+0x374/0x11f0 [xfs]
>xfs_iomap_write_direct+0x2ac/0x430 [xfs]
>xfs_file_iomap_begin+0x20d/0xd50 [xfs]
>iomap_apply+0x43/0xe0
>dax_iomap_rw+0x89/0xf0
>xfs_file_dax_write+0xcc/0x220 [xfs]
>xfs_file_write_iter+0xf0/0x130 [xfs]
>__vfs_write+0xd9/0x150
>vfs_write+0xc8/0x1c0
>SyS_write+0x45/0xa0
>entry_SYSCALL_64_fastpath+0x1f/0xbe
>
> -> #1 (&xfs_nondir_ilock_class){}:
>lock_acquire+0xab/0x200
>down_write_nested+0x4a/0xb0
>xfs_ilock+0x263/0x330 [xfs]
>xfs_setattr_size+0x152/0x370 [xfs]
>xfs_vn_setattr+0x6b/0x90 [xfs]
>notify_change+0x27d/0x3f0
>do_truncate+0x5b/0x90
>path_openat+0x237/0xa90
>do_filp_open+0x8a/0xf0
>do_sys_open+0x11c/0x1f0
>entry_SYSCALL_64_fastpath+0x1f/0xbe
>
> -> #0 (&(&ip->i_mmaplock)->mr_lock){}:
>up_write+0x1c/0x40
>xfs_iunlock+0x1d0/0x310 [xfs]
>xfs_file_fallocate+0x8a/0x310 [xfs]
>loop_queue_work+0xb7/0x8d0
>kthread_worker_fn+0xb9/0x1f0
>
> Chain exists of:
> &(&ip->i_mmaplock)->mr_lock --> &xfs_nondir_ilock_class -->
> (complete)&ret.event
>
> Possible unsafe locking scenario by crosslock:
>
>CPU0CPU1
>
> lock(&xfs_nondir_ilock_class);
> lock((complete)&ret.event);
>lock(&(&ip->i_mmaplock)->mr_lock);
>unlock((complete)&ret.event);
>
>*** DEADLOCK ***
The warning is a false positive, caused by the fact that all
wait_for_completion()s in submit_bio_wait() are waiting with the same
lock class.
However, some bios have nothing to do with others, for example, the case
might happen while using loop devices, between bios of an upper device
and a lower device(=loop device).
The safest way to assign different lock classes to different devices is
to do it for each gendisk. In other words, this patch assigns a
lockdep_map per gendisk and uses it when initializing completion in
submit_bio_wait().
Of course, it might be too conservative. But, making it safest for now
and extended by block layer experts later is good, atm.
Signed-off-by: Byungchul Park
---
block/bio.c | 2 +-
block/genhd.c | 13 +
include/linux/genhd.h | 22 --
3 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 101c2a9..6dd640d 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -945,7 +945,7 @@ int submit_bio_wait(struct bio *bio)
{
struct submit_bio_ret ret;
- init_completion(&ret.event);
+ init_completion_with_map(&ret.event, &bio->bi_disk->lockdep_map);
bio->bi_private = &ret;
bio->bi_end_io = submit_bio_wait_endio;
bio->bi_opf |= REQ_SYNC;
diff --git a/block/genhd.c b/block/genhd.c
index dd305c6..f195d22 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1354,13 +1354,7 @@ dev_t blk_lookup_devt(const char *name, int partno)
}
EXPORT_SYMBOL(blk_lookup_devt);
-struct gendisk *alloc_disk(int minors)
-{
- return alloc_disk_node(minors, NUMA_NO_NODE);
-}
-EXPORT_SYMBOL(alloc_disk);
-
-struct gendisk *alloc_disk_node(int minors, int node_id)
+struct gendisk *__alloc_disk_node(int minors, int node_id, struct
lock_class_key *key, const char *lock_name)
{
struct gendisk *disk;
struct disk_part_tbl *ptbl;
@@ -1409,9 +1403,12 @@ struct gendisk *alloc_disk_node(int minors, int node_id)
disk_to_dev(disk)->type = &disk_type;
device_initialize(disk_to_dev(disk));
}
+
+ lockdep_init_map(&disk->lockdep_map, lock_name, key, 0);
+
return disk;
}
-EXPORT_SYMBOL(alloc_disk_node);
+EXPORT_SYMBOL(__alloc_disk_node);
struct kobject *get_disk(struct gendisk *disk)
{
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 6d85a75..9832e3c 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -206,6 +206,9 @@ struct gendisk {
#endif /* CONFIG_BLK_DEV_INTEGRIT
Re: [PATCH 01/11] Initialize the mapping of KASan shadow memory
On 2017.10.12 7:43AM Dmitry Osipenko [mailto:dig...@gmail.com] wrote: >Shouldn't all __pgprot's contain L_PTE_MT_WRITETHROUGH ? > >[...] > >-- >Dmitry Thanks for your review. I'm sorry that my replay is so late. I don't think L_PTE_MT_WRITETHROUGH is need for all arm soc. So I think kasan's mapping can use PAGE_KERNEL which can be initialized for different arm soc and __pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY)). I don't think the mapping table flags in kasan_early_init need be changed because of the follow reason: 1) PAGE_KERNEL can't be used in early_kasan_init because the pgprot_kernel which is used to define PAGE_KERNEL doesn't be initialized. 2) all of the kasan shadow's mapping table is going to be created again in kasan_init function. All what I say is: I think only the mapping table flags in kasan_init function need to be changed into PAGE_KERNEL or __pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY)). Here is the code, I has already tested: --- a/arch/arm/mm/kasan_init.c +++ b/arch/arm/mm/kasan_init.c @@ -124,7 +124,7 @@ pte_t * __meminit kasan_pte_populate(pmd_t *pmd, unsigned long addr, int node) void *p = kasan_alloc_block(PAGE_SIZE, node); if (!p) return NULL; - entry = pfn_pte(virt_to_pfn(p), __pgprot(_L_PTE_DEFAULT | L_PTE_DIRTY | L_PTE_XN)); + entry = pfn_pte(virt_to_pfn(p), __pgprot(pgprot_val(PAGE_KERNEL))); set_pte_at(&init_mm, addr, pte, entry); } return pte; @@ -253,7 +254,7 @@ void __init kasan_init(void) set_pte_at(&init_mm, KASAN_SHADOW_START + i*PAGE_SIZE, &kasan_zero_pte[i], pfn_pte( virt_to_pfn(kasan_zero_page), -__pgprot(_L_PTE_DEFAULT | L_PTE_DIRTY | L_PTE_XN | L_PTE_RDONLY))); + __pgprot(pgprot_val(PAGE_KERNEL) | L_PTE_RDONLY))); memset(kasan_zero_page, 0, PAGE_SIZE); cpu_set_ttbr0(orig_ttbr0); flush_cache_all();
[PATCH v2 2/4] lockdep: Remove unnecessary acquisitions wrt workqueue flush
The workqueue added manual acquisitions to catch deadlock cases.
Now crossrelease was introduced, some of those are redundant, since
wait_for_completion() already includes the acquisition for itself.
Removed it.
Signed-off-by: Byungchul Park
---
include/linux/workqueue.h | 4 ++--
kernel/workqueue.c| 20
2 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index f3c47a0..1455b5e 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -218,7 +218,7 @@ static inline void destroy_delayed_work_on_stack(struct
delayed_work *work) { }
\
__init_work((_work), _onstack); \
(_work)->data = (atomic_long_t) WORK_DATA_INIT(); \
- lockdep_init_map(&(_work)->lockdep_map, #_work, &__key, 0); \
+ lockdep_init_map(&(_work)->lockdep_map, "(complete)"#_work,
&__key, 0); \
INIT_LIST_HEAD(&(_work)->entry);\
(_work)->func = (_func);\
} while (0)
@@ -399,7 +399,7 @@ enum {
static struct lock_class_key __key; \
const char *__lock_name;\
\
- __lock_name = #fmt#args;\
+ __lock_name = "(complete)"#fmt#args;\
\
__alloc_workqueue_key((fmt), (flags), (max_active), \
&__key, __lock_name, ##args); \
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index c77fdf6..8cef533 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -2496,15 +2496,8 @@ static void insert_wq_barrier(struct pool_workqueue *pwq,
INIT_WORK_ONSTACK(&barr->work, wq_barrier_func);
__set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&barr->work));
- /*
-* Explicitly init the crosslock for wq_barrier::done, make its lock
-* key a subkey of the corresponding work. As a result we won't
-* build a dependency between wq_barrier::done and unrelated work.
-*/
- lockdep_init_map_crosslock((struct lockdep_map *)&barr->done.map,
- "(complete)wq_barr::done",
- target->lockdep_map.key, 1);
- __init_completion(&barr->done);
+ init_completion_with_map(&barr->done, &target->lockdep_map);
+
barr->task = current;
/*
@@ -2610,16 +2603,14 @@ void flush_workqueue(struct workqueue_struct *wq)
struct wq_flusher this_flusher = {
.list = LIST_HEAD_INIT(this_flusher.list),
.flush_color = -1,
- .done = COMPLETION_INITIALIZER_ONSTACK(this_flusher.done),
};
int next_color;
+ init_completion_with_map(&this_flusher.done, &wq->lockdep_map);
+
if (WARN_ON(!wq_online))
return;
- lock_map_acquire(&wq->lockdep_map);
- lock_map_release(&wq->lockdep_map);
-
mutex_lock(&wq->mutex);
/*
@@ -2882,9 +2873,6 @@ bool flush_work(struct work_struct *work)
if (WARN_ON(!wq_online))
return false;
- lock_map_acquire(&work->lockdep_map);
- lock_map_release(&work->lockdep_map);
-
if (start_flush_work(work, &barr)) {
wait_for_completion(&barr.done);
destroy_work_on_stack(&barr.work);
--
1.9.1
[PATCH v2 3/4] genhd.h: Remove trailing white space
Trailing white space is not accepted in kernel coding style. Remove
them.
Signed-off-by: Byungchul Park
---
include/linux/genhd.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index ea652bf..6d85a75 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -3,7 +3,7 @@
/*
* genhd.h Copyright (C) 1992 Drew Eckhardt
- * Generic hard disk header file by
+ * Generic hard disk header file by
* Drew Eckhardt
*
*
@@ -471,7 +471,7 @@ struct bsd_disklabel {
__s16 d_type; /* drive type */
__s16 d_subtype; /* controller/d_type specific */
chard_typename[16]; /* type name, e.g. "eagle" */
- chard_packname[16]; /* pack identifier */
+ chard_packname[16]; /* pack identifier */
__u32 d_secsize; /* # of bytes per sector */
__u32 d_nsectors; /* # of data sectors per track */
__u32 d_ntracks; /* # of tracks per cylinder */
--
1.9.1
Re: [PATCH 1/2] misc: rtsx: Move Realtek Card Reader Driver to misc
On Thu, Oct 19, 2017 at 5:22 AM, wrote:
> From: Rui Feng
>
> Because Realtek PCIE card reader driver is a pcie driver,
> and it bridges mmc subsystem and memstick subsystem, it's
> not a mfd driver. Greg and Lee Jones had a discussion about
> where to put the driver, the result is that misc is a good
> place for it, so I move all files to misc. If I don't move
> it to a right place, I can't add any patch for this driver.
>
> Signed-off-by: Rui Feng
> ---
> drivers/memstick/host/rtsx_pci_ms.c | 2 +-
> drivers/mfd/Kconfig | 11 ---
> drivers/mfd/Makefile | 2 --
> drivers/misc/Kconfig | 1 +
> drivers/misc/Makefile| 1 +
> drivers/misc/realtek/Kconfig | 10 ++
> drivers/misc/realtek/Makefile| 3 +++
> drivers/{mfd => misc/realtek}/rtl8411.c | 1 -
> drivers/{mfd => misc/realtek}/rts5209.c | 1 -
> drivers/{mfd => misc/realtek}/rts5227.c | 1 -
Thanks for redoing the patch. If Lee and Greg want to see it moved
here, I won't object, but I'd still like to understand the distinction better,
since there are likely going to be other drivers in the same situation
(either adding into misc or mfd) and it would be good if we can all
give consistent advice.
Greg, Lee: can you come up with a simple definition of what an
MFD is that this driver does not fit into? What about other
PCI drivers like intel-lpss-pci and lpc_ich? Are they different
from this one or should they also be moved out of MFD?
Arnd
Re: [PATCH 0/9] Intel Processor Trace virtulization enabling
On 19/10/2017 07:54, Kang, Luwei wrote: >>> Get it. I have feedback to hardware architect. I hope it can be applied but >>> it may need wait a long time. >> Note that the hardware need not do anything. However it would be nice if >> the SDM can define a bit _for the hypervisors_ to >> enforce the above constraint and fail vmentry if they are not respected. > > Hi Paolo, > Thanks for your response. I have a question want to ask for you. As > you mentioned in previous mail " We would like the nested hypervisor > to be forced to set the "use GPA for processor tracing"". Is there > have any problem if we don't set "use GPA for processor tracing" in > nested hypervisor? If the nested hypervisor doesn't set "use GPA for processor tracing", the processor should use L1 addresses for processor tracing. This however is not possible without shadowing the ToPA, same as in non-nested virtualization for <=Skylake processors. So we have | mode |--- nested? | system-wide | host-guest +---+--- no | use HPA for tracing | use GPA for tracing | (no EPT) | (EPT is GPA->HPA) +---+--- yes | use GPA for tracing | use nGPA for tracing | (EPT is nGPA->HPA!!) | (EPT is nGPA->HPA, so ok) (for nested, L0 mode of course must be host-guest). If the nested hypervisor wants to use system-wide tracing, it cannot use "use GPA for tracing" because the EPT table doesn't have the right mapping of L1->L0 physical address. So if you want to do system-wide L1 tracing you have to disable EPT for L1, and if you want to do host-guest L1 tracing you have to enable it. Paolo > If yes, what should we do? In patch 9, I pass > though PT MSRs ( IA32_RTIT_* ) to guest.
Re: [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages
On Thu 19-10-17 11:51:11, Joonsoo Kim wrote: > On Fri, Oct 13, 2017 at 02:00:12PM +0200, Michal Hocko wrote: > > From: Michal Hocko > > > > Michael has noticed that the memory offline tries to migrate kernel code > > pages when doing > > echo 0 > /sys/devices/system/memory/memory0/online > > > > The current implementation will fail the operation after several failed > > page migration attempts but we shouldn't even attempt to migrate > > that memory and fail right away because this memory is clearly not > > migrateable. This will become a real problem when we drop the retry loop > > counter resp. timeout. > > > > The real problem is in has_unmovable_pages in fact. We should fail if > > there are any non migrateable pages in the area. In orther to guarantee > > that remove the migrate type checks because MIGRATE_MOVABLE is not > > guaranteed to contain only migrateable pages. It is merely a heuristic. > > Similarly MIGRATE_CMA does guarantee that the page allocator doesn't > > allocate any non-migrateable pages from the block but CMA allocations > > themselves are unlikely to migrateable. Therefore remove both checks. > > Hello, > > This patch will break the CMA user. As you mentioned, CMA allocation > itself isn't migrateable. So, after a single page is allocated through > CMA allocation, has_unmovable_pages() will return true for this > pageblock. Then, futher CMA allocation request to this pageblock will > fail because it requires isolating the pageblock. Hmm, does this mean that the CMA allocation path depends on has_unmovable_pages to return false here even though the memory is not movable? This sounds really strange to me and kind of abuse of this function. Which path is that? Can we do the migrate type test theres? -- Michal Hocko SUSE Labs
Re: [PATCH 0/2] [media] rc/keymaps: add support for two RCs of hisilicon boards.
Hi Hans， On 2017/10/19 15:01, Hans Verkuil wrote: > On 10/18/2017 12:54 PM, Jiancheng Xue wrote: >> Add support for two remote controllers of hisilicon boards. >> >> Younian Wang (2): >> [media] rc/keymaps: add support for RC of hisilicon TV demo boards >> [media] rc/keymaps: add support for RC of hisilicon poplar board >> >> drivers/media/rc/keymaps/Makefile | 2 + >> drivers/media/rc/keymaps/rc-hisi-poplar.c | 58 + >> drivers/media/rc/keymaps/rc-hisi-tv-demo.c | 70 >> ++ >> 3 files changed, 130 insertions(+) >> create mode 100644 drivers/media/rc/keymaps/rc-hisi-poplar.c >> create mode 100644 drivers/media/rc/keymaps/rc-hisi-tv-demo.c >> > > Did you make a mistake? You reposted these two patches, but still without any > copyright statement... > > I think something went wrong here. > I haven't reposted them so far. This is still the first version. I am waiting to see if there are any more comments. If not, I can repost them soon. Thank you. Regards, Jiancheng
[PATCH v3 2/2] platform/x86: silead_dmi: Add silead,home-button property to some tablets
Add "silead,home-button" property to entries for tablets which have
a capacitive home button (typically a windows logo on the front).
This new property is checked for by the new capacitive home button
support in the silead touchscreen driver.
Signed-off-by: Hans de Goede
---
drivers/platform/x86/silead_dmi.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/platform/x86/silead_dmi.c
b/drivers/platform/x86/silead_dmi.c
index fadfce9b3f5f..ac304f2318cc 100644
--- a/drivers/platform/x86/silead_dmi.c
+++ b/drivers/platform/x86/silead_dmi.c
@@ -58,6 +58,7 @@ static const struct property_entry dexp_ursus_7w_props[] = {
PROPERTY_ENTRY_U32("touchscreen-size-y", 630),
PROPERTY_ENTRY_STRING("firmware-name", "gsl1686-dexp-ursus-7w.fw"),
PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
{ }
};
@@ -72,6 +73,7 @@ static const struct property_entry
surftab_wintron70_st70416_6_props[] = {
PROPERTY_ENTRY_STRING("firmware-name",
"gsl1686-surftab-wintron70-st70416-6.fw"),
PROPERTY_ENTRY_U32("silead,max-fingers", 10),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
{ }
};
@@ -116,6 +118,7 @@ static const struct property_entry
pov_mobii_wintab_p800w_props[] = {
PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
PROPERTY_ENTRY_STRING("firmware-name",
"gsl3692-pov-mobii-wintab-p800w.fw"),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
{ }
};
@@ -143,6 +146,7 @@ static const struct property_entry chuwi_hi8_pro_props[] = {
PROPERTY_ENTRY_U32("touchscreen-size-y", 1148),
PROPERTY_ENTRY_BOOL("touchscreen-swapped-x-y"),
PROPERTY_ENTRY_STRING("firmware-name", "gsl3680-chuwi-hi8-pro.fw"),
+ PROPERTY_ENTRY_BOOL("silead,home-button"),
{ }
};
--
2.14.2
[PATCH v3 1/2] Input: silead - Add support for capactive home button found on some x86 tablets
On some x86 tablets with a silead touchscreen the windows logo on the
front is a capacitive home button. Touching this button results in a touch
with bits 12-15 of the Y coordinates set, while normally only the lower 12
are used.
Detect this and report a KEY_LEFTMETA press when this happens. Note for
now we only respond to the Y coordinate bits 12-15 containing 0x01, on some
tablets *without* a capacative button I've noticed these bits containing
0x04 when crossing the edges of the screen.
Cc: Rob Herring
Signed-off-by: Hans de Goede
---
Changes in v2:
-Only enable support for the home-button if a "silead,home-button"
boolean device-property is set on the device
Changes in v3:
-Document the new silead,home-button property in:
Documentation/devicetree/bindings/input/touchscreen/goodix.txt
---
.../bindings/input/touchscreen/silead_gsl1680.txt | 2 +
drivers/input/touchscreen/silead.c | 46 --
2 files changed, 37 insertions(+), 11 deletions(-)
diff --git
a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt
b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt
index 6aa625e0cb8d..84752de12412 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/silead_gsl1680.txt
@@ -23,6 +23,8 @@ Optional properties:
- touchscreen-inverted-y : See touchscreen.txt
- touchscreen-swapped-x-y : See touchscreen.txt
- silead,max-fingers : maximum number of fingers the touchscreen can detect
+- silead,home-button : Boolean, set to true on devices which have a
+ capacitive home-button build into the touchscreen
- vddio-supply : regulator phandle for controller VDDIO
- avdd-supply: regulator phandle for controller AVDD
diff --git a/drivers/input/touchscreen/silead.c
b/drivers/input/touchscreen/silead.c
index 0dbcf105f7db..646b1e768e6b 100644
--- a/drivers/input/touchscreen/silead.c
+++ b/drivers/input/touchscreen/silead.c
@@ -56,7 +56,7 @@
#define SILEAD_POINT_Y_MSB_OFF 0x01
#define SILEAD_POINT_X_OFF 0x02
#define SILEAD_POINT_X_MSB_OFF 0x03
-#define SILEAD_TOUCH_ID_MASK 0xF0
+#define SILEAD_EXTRA_DATA_MASK 0xF0
#define SILEAD_CMD_SLEEP_MIN 1
#define SILEAD_CMD_SLEEP_MAX 2
@@ -109,6 +109,9 @@ static int silead_ts_request_input_dev(struct
silead_ts_data *data)
INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED |
INPUT_MT_TRACK);
+ if (device_property_read_bool(dev, "silead,home-button"))
+ input_set_capability(data->input, EV_KEY, KEY_LEFTMETA);
+
data->input->name = SILEAD_TS_NAME;
data->input->phys = "input/ts";
data->input->id.bustype = BUS_I2C;
@@ -139,7 +142,8 @@ static void silead_ts_read_data(struct i2c_client *client)
struct input_dev *input = data->input;
struct device *dev = &client->dev;
u8 *bufp, buf[SILEAD_TS_DATA_LEN];
- int touch_nr, error, i;
+ int touch_nr, softbutton, error, i;
+ bool softbutton_pressed = false;
error = i2c_smbus_read_i2c_block_data(client, SILEAD_REG_DATA,
SILEAD_TS_DATA_LEN, buf);
@@ -148,21 +152,40 @@ static void silead_ts_read_data(struct i2c_client *client)
return;
}
- touch_nr = buf[0];
- if (touch_nr > data->max_fingers) {
+ if (buf[0] > data->max_fingers) {
dev_warn(dev, "More touches reported then supported %d > %d\n",
-touch_nr, data->max_fingers);
- touch_nr = data->max_fingers;
+buf[0], data->max_fingers);
+ buf[0] = data->max_fingers;
}
+ touch_nr = 0;
bufp = buf + SILEAD_POINT_DATA_LEN;
- for (i = 0; i < touch_nr; i++, bufp += SILEAD_POINT_DATA_LEN) {
- /* Bits 4-7 are the touch id */
- data->id[i] = (bufp[SILEAD_POINT_X_MSB_OFF] &
- SILEAD_TOUCH_ID_MASK) >> 4;
- touchscreen_set_mt_pos(&data->pos[i], &data->prop,
+ for (i = 0; i < buf[0]; i++, bufp += SILEAD_POINT_DATA_LEN) {
+ softbutton = (bufp[SILEAD_POINT_Y_MSB_OFF] &
+ SILEAD_EXTRA_DATA_MASK) >> 4;
+
+ if (softbutton) {
+ /*
+* For now only respond to softbutton == 0x01, some
+* tablets *without* a capacative button send 0x04
+* when crossing the edges of the screen.
+*/
+ if (softbutton == 0x01)
+ softbutton_pressed = true;
+
+ continue;
+ }
+
+ /*
+* Bits 4-7 are the touch id, note not all models have
+* hardware touch ids so atm we don't use
Re: [PATCH v2 3/4] media: i2c: Add TDA1997x HDMI receiver driver
On Wed, Oct 18, 2017 at 5:04 AM, Hans Verkuil wrote:
> Hi Tim,
>
> Here is my review of this v2:
>
> On 10/12/17 06:45, Tim Harvey wrote:
>> Add support for the TDA1997x HDMI receivers.
>>
>> +
>> +/*
>> + * Video Input formats
>> + */
>> +struct vhref_values {
>> + u16 href_start;
>> + u16 href_end;
>> + u16 vref_f1_start;
>> + u8 vref_f1_width;
>> + u16 vref_f2_start;
>> + u8 vref_f2_width;
>> + u16 fieldref_f1_start;
>> + u8 fieldPolarity;
>> + u16 fieldref_f2_start;
>
> Since we don't support interlaced (yet) I'd just drop the 'f2' fields.
> Ditto for fieldPolarity.
>
> Can't these href/vref values be calculated from the timings?
>
The values in this struct are used to configure the tda1997x VHREF
timing generator in tda1997x_configure_input_resolution() for
generating the video output bus timings so I can't really drop them
unless I can calculate them. Let me look into this - should be
possible.
>> +};
>> +
>> +/* parse an infoframe and do some sanity checks on it */
>> +static unsigned int
>> +tda1997x_parse_infoframe(struct tda1997x_state *state, u16 addr)
>> +{
>> + struct v4l2_subdev *sd = &state->sd;
>> + union hdmi_infoframe frame;
>> + u8 buffer[40];
>> + u8 reg;
>> + int len, err;
>> +
>> + /* read data */
>> + len = io_readn(sd, addr, sizeof(buffer), buffer);
>> + err = hdmi_infoframe_unpack(&frame, buffer);
>> + if (err) {
>> + v4l_err(state->client,
>> + "failed parsing %d byte infoframe: 0x%04x/0x%02x\n",
>> + len, addr, buffer[0]);
>> + return err;
>> + }
>> + if (debug > 1)
>> + hdmi_infoframe_log(KERN_INFO, &state->client->dev, &frame);
>> + switch (frame.any.type) {
>> + /* Audio InfoFrame: see HDMI spec 8.2.2 */
>> + case HDMI_INFOFRAME_TYPE_AUDIO:
>> + /* sample rate */
>> + switch (frame.audio.sample_frequency) {
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
>> + state->audio_samplerate = 32000;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
>> + state->audio_samplerate = 44100;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
>> + state->audio_samplerate = 48000;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
>> + state->audio_samplerate = 88200;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
>> + state->audio_samplerate = 96000;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_176400:
>> + state->audio_samplerate = 176400;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_192000:
>> + state->audio_samplerate = 192000;
>> + break;
>> + default:
>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM:
>> + break;
>> + }
>> +
>> + /* sample size */
>> + switch (frame.audio.sample_size) {
>> + case HDMI_AUDIO_SAMPLE_SIZE_16:
>> + state->audio_samplesize = 16;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_SIZE_20:
>> + state->audio_samplesize = 20;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_SIZE_24:
>> + state->audio_samplesize = 24;
>> + break;
>> + case HDMI_AUDIO_SAMPLE_SIZE_STREAM:
>> + default:
>> + break;
>> + }
>> +
>> + /* Channel Count */
>> + state->audio_channels = frame.audio.channels;
>> + if (frame.audio.channel_allocation &&
>> + frame.audio.channel_allocation != state->audio_ch_alloc) {
>> + /* use the channel assignment from the infoframe */
>> + state->audio_ch_alloc = frame.audio.channel_allocation;
>> + tda1997x_configure_audout(sd, state->audio_ch_alloc);
>> + /* reset the audio FIFO */
>> + tda1997x_hdmi_info_reset(sd, RESET_AUDIO, false);
>> + }
>> + break;
>> +
>> + /* Source Product Descriptor information (SPD) */
>> + case HDMI_INFOFRAME_TYPE_SPD:
>> + strncpy(frame.spd.vendor, state->vendor,
>> + sizeof(frame.spd.vendor));
>> + strncpy(frame.spd.product, state->product,
>> + sizeof(frame.spd.product));
>> + v4l_info(state->client, "Source Product Descriptor: %s %s\n",
>> + state->vendor, state->product);
>
> Use hdmi_infoframe_log() for logging infoframes.
ok - I will always call hdmi_infoframe_log() above and refrain from
ou
Re: [PATCH 0/2] [media] rc/keymaps: add support for two RCs of hisilicon boards.
On 10/19/2017 09:12 AM, Jiancheng Xue wrote: > Hi Hans， > > On 2017/10/19 15:01, Hans Verkuil wrote: >> On 10/18/2017 12:54 PM, Jiancheng Xue wrote: >>> Add support for two remote controllers of hisilicon boards. >>> >>> Younian Wang (2): >>> [media] rc/keymaps: add support for RC of hisilicon TV demo boards >>> [media] rc/keymaps: add support for RC of hisilicon poplar board >>> >>> drivers/media/rc/keymaps/Makefile | 2 + >>> drivers/media/rc/keymaps/rc-hisi-poplar.c | 58 + >>> drivers/media/rc/keymaps/rc-hisi-tv-demo.c | 70 >>> ++ >>> 3 files changed, 130 insertions(+) >>> create mode 100644 drivers/media/rc/keymaps/rc-hisi-poplar.c >>> create mode 100644 drivers/media/rc/keymaps/rc-hisi-tv-demo.c >>> >> >> Did you make a mistake? You reposted these two patches, but still without any >> copyright statement... >> >> I think something went wrong here. >> > I haven't reposted them so far. This is still the first version. I am waiting > to see if there are any more comments. If not, I can repost them soon. Ah, now I see why I got confused: something is wrong with the email dates: Sean's reply has an earlier timestamp than your patch series. So I mistakenly thought that the patch series was a v2 series. Regards, Hans
Re: [PATCH resend] [media] uvcvideo: zero seq number when disabling stream
Hi Laurent and Guennadi,
On 2017年10月18日 16:52, Guennadi Liakhovetski wrote:
Hi Laurent,
On Mon, 16 Oct 2017, Laurent Pinchart wrote:
Hi Hans,
(CC'ing Guennadi Liakhovetski)
Thank you for the patch.
On Friday, 15 September 2017 09:27:51 EEST Hans Yang wrote:
For bulk-based devices, when disabling the video stream,
in addition to issue CLEAR_FEATURE(HALT), it is better to set
alternate setting 0 as well or the sequnce number in host
side will probably not reset to zero.
The USB 2.0 specificatin states in the description of the SET_INTERFACE
request that "If a device only supports a default setting for the specified
interface, then a STALL may be returned in the Status stage of the request".
The Linux implementation of usb_set_interface() deals with this by handling
STALL conditions and manually clearing HALT for all endpoints in the
interface, but I'm still concerned that this change could break existing bulk-
based cameras. Do you know what other operating systems do when disabling the
stream on bulk cameras ? According to a comment in the driver Windows calls
CLEAR_FEATURE(HALT), but the situation might have changed since that was
tested.
Sorry I did not mention that it is about SS bulk-based cameras using
sequence number technique.
From my observations, invoking usb_clear_halt() will reset the endpoint
in the device side via CLEAR_FEATURE(HALT)
and reset the sequence number as well; however usb_reset_endpoint() does
not reset the host side endpoint with xhci driver,
then the sequence number will mismatch in next time stream enable.
I can always observe this through a bus analyzer on Linux implementation
whatever X86 or ARM based.
This is not observed on the Windows.
Guennadi, how do your bulk-based cameras handle this ?
I don't know what design decisions are implemented there, but I tested a
couple of cameras for sending a STREAMOFF after a few captured buffers,
sleeping for a couple of seconds, requeuing buffers, sending STREAMON and
capturing a few more - that seems to have worked. "Seems" because I only
used a modified version of capture-example, I haven't checked buffer
contents.
BTW, Hans, may I ask - what cameras did you use?
I have three SS bulk-based cameras as follows:
e-con Systems See3CAM_CU130 (2560:c1d0)
Leopard ZED (2b03:f580)
Intel(R) RealSense(TM) Camera SR300 (8086:0aa5)
I can observe the same issue on all above;
besides, to reproduce issue do not let the camera enter suspend because
it will *help* to reset the sequence number through usb_set_interface(0).
Thanks
Guennadi
Then in next time video stream start, the device will expect
host starts packet from 0 sequence number but host actually
continue the sequence number from last transaction and this
causes transaction errors.
Do you mean the DATA0/DATA1 toggle ? Why does the host continue toggling the
PID from the last transation ? The usb_clear_halt() function calls
usb_reset_endpoint() which should reset the DATA PID toggle.
This commit fixes this by adding set alternate setting 0 back
as what isoch-based devices do.
Below error message will also be eliminated for some devices:
uvcvideo: Non-zero status (-71) in video completion handler.
Signed-off-by: Hans Yang
---
drivers/media/usb/uvc/uvc_video.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c
b/drivers/media/usb/uvc/uvc_video.c index fb86d6af398d..ad80c2a6da6a 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1862,10 +1862,9 @@ int uvc_video_enable(struct uvc_streaming *stream,
int enable)
if (!enable) {
uvc_uninit_video(stream, 1);
- if (stream->intf->num_altsetting > 1) {
- usb_set_interface(stream->dev->udev,
+ usb_set_interface(stream->dev->udev,
stream->intfnum, 0);
- } else {
+ if (stream->intf->num_altsetting == 1) {
/* UVC doesn't specify how to inform a bulk-based device
* when the video stream is stopped. Windows sends a
* CLEAR_FEATURE(HALT) request to the video streaming
--
Regards,
Laurent Pinchart
Regards,
Hans Yang
--nvpublic--
Re: [PATCH] f2fs: limit # of inmemory pages
On Wed, Oct 18, 2017 at 07:15:16PM -0700, Jaegeuk Kim wrote: > If some abnormal users try lots of atomic write operations, f2fs is able to > produce pinned pages in the main memory which affects system performance. > This patch limits that as 20% over total memory size, and if f2fs reaches > to the limit, it will drop all the inmemory pages. Can you please document your "atomic" writes and how they pin memory in a mail to linux-abi? And how they compare to my O_ATOMIC implement as well as the papers it is based on? I tried to take a quick look at the code and it looks very bogus to me in addition to being sneaked in through fs-specific ioctls without any review whatsoever.
Re: [PATCH 3/3] mm: oom: show unreclaimable slab info when unreclaimable slabs > user memory
On Tue 17-10-17 15:39:08, David Rientjes wrote: > On Wed, 18 Oct 2017, Yang Shi wrote: > > > > Yes, this should catch occurrences of "huge unreclaimable slabs", right? > > > > Yes, it sounds so. Although single "huge" unreclaimable slab might not > > result > > in excessive slabs use in a whole, but this would help to filter out "small" > > unreclaimable slab. > > > > Keep in mind this is regardless of SLAB_RECLAIM_ACCOUNT: your patch has > value beyond only unreclaimable slab, it can also be used to show > instances where the oom killer was invoked without properly reclaiming > slab. If the total footprint of a slab cache exceeds 5%, I think a line > should be emitted unconditionally to the kernel log. agreed. I am not sure 5% is the greatest fit but we can tune that later. -- Michal Hocko SUSE Labs
Re: [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages
On Thu, Oct 19, 2017 at 09:15:03AM +0200, Michal Hocko wrote: > On Thu 19-10-17 11:51:11, Joonsoo Kim wrote: > > On Fri, Oct 13, 2017 at 02:00:12PM +0200, Michal Hocko wrote: > > > From: Michal Hocko > > > > > > Michael has noticed that the memory offline tries to migrate kernel code > > > pages when doing > > > echo 0 > /sys/devices/system/memory/memory0/online > > > > > > The current implementation will fail the operation after several failed > > > page migration attempts but we shouldn't even attempt to migrate > > > that memory and fail right away because this memory is clearly not > > > migrateable. This will become a real problem when we drop the retry loop > > > counter resp. timeout. > > > > > > The real problem is in has_unmovable_pages in fact. We should fail if > > > there are any non migrateable pages in the area. In orther to guarantee > > > that remove the migrate type checks because MIGRATE_MOVABLE is not > > > guaranteed to contain only migrateable pages. It is merely a heuristic. > > > Similarly MIGRATE_CMA does guarantee that the page allocator doesn't > > > allocate any non-migrateable pages from the block but CMA allocations > > > themselves are unlikely to migrateable. Therefore remove both checks. > > > > Hello, > > > > This patch will break the CMA user. As you mentioned, CMA allocation > > itself isn't migrateable. So, after a single page is allocated through > > CMA allocation, has_unmovable_pages() will return true for this > > pageblock. Then, futher CMA allocation request to this pageblock will > > fail because it requires isolating the pageblock. > > Hmm, does this mean that the CMA allocation path depends on > has_unmovable_pages to return false here even though the memory is not > movable? This sounds really strange to me and kind of abuse of this Your understanding is correct. Perhaps, abuse or wrong function name. > function. Which path is that? Can we do the migrate type test theres? alloc_contig_range() -> start_isolate_page_range() -> set_migratetype_isolate() -> has_unmovable_pages() We can add one argument, 'XXX' to set_migratetype_isolate() and change it to check migrate type rather than has_unmovable_pages() if 'XXX' is specified. Thanks.
Re: [v6,1/2] pid: Replace pid bitmap implementation with IDR API
Hi Gargi,
This patch breaks CRIU, because it changes a meaning of ns_last_pid.
== Run zdtm/static/env00 in h ==
DEP env00.d
CCenv00.o
LINK env00
Start test
./env00 --pidfile=env00.pid --outfile=env00.out --envname=ENV_00_TEST
Run criu dump
Run criu restore
=[log]=> dump/zdtm/static/env00/52/1/restore.log
grep Error
(00.000587) No mountpoints-6.img image
(00.000593) mnt: Reading mountpoint images (id 6 pid 52)
(00.000653) Forking task with 52 pid (flags 0x0)
(00.007568) PID: real 51 virt 52
(00.010363) 52: Error (criu/cr-restore.c:1787): Pid 51 do not match
expected 52 (task 52)
(00.010474) Error (criu/cr-restore.c:2449): Restoring FAILED.
ERROR OVER
Before this patch, ns_last_pid contains a pid of a last process. With
this patch, it contains a pid of a "next" process.
In CRIU we use ns_last_pid to restore a process with a specified pid,
and now this logic is broken:
$ uname -a
Linux laptop 4.11.11-200.fc25.x86_64 #1 SMP Mon Jul 17 17:41:12 UTC 2017 x86_64
x86_64 x86_64 GNU/Linux
$ echo 1 > /proc/sys/kernel/ns_last_pid && sh -c 'echo $$'
2
$ uname -a
Linux fc24 4.14.0-rc5-next-20171018 #1 SMP Wed Oct 18 23:52:43 PDT 2017 x86_64
x86_64 x86_64 GNU/Linux
$ echo 1 > /proc/sys/kernel/ns_last_pid && sh -c 'echo $$'
1
Thanks,
Andrei
On Wed, Oct 11, 2017 at 06:19:38PM -0400, Gargi Sharma wrote:
> This patch replaces the current bitmap implemetation for
> Process ID allocation. Functions that are no longer required,
> for example, free_pidmap(), alloc_pidmap(), etc. are removed.
> The rest of the functions are modified to use the IDR API.
> The change was made to make the PID allocation less complex by
> replacing custom code with calls to generic API.
>
> Signed-off-by: Gargi Sharma
> Reviewed-by: Rik van Riel
> ---
> arch/powerpc/platforms/cell/spufs/sched.c | 2 +-
> fs/proc/loadavg.c | 2 +-
> include/linux/pid_namespace.h | 14 +--
> init/main.c | 2 +-
> kernel/pid.c | 201
> ++
> kernel/pid_namespace.c| 44 +++
> 6 files changed, 57 insertions(+), 208 deletions(-)
>
> diff --git a/arch/powerpc/platforms/cell/spufs/sched.c
> b/arch/powerpc/platforms/cell/spufs/sched.c
> index 1fbb5da..e47761c 100644
> --- a/arch/powerpc/platforms/cell/spufs/sched.c
> +++ b/arch/powerpc/platforms/cell/spufs/sched.c
> @@ -1093,7 +1093,7 @@ static int show_spu_loadavg(struct seq_file *s, void
> *private)
> LOAD_INT(c), LOAD_FRAC(c),
> count_active_contexts(),
> atomic_read(&nr_spu_contexts),
> - task_active_pid_ns(current)->last_pid);
> + idr_get_cursor(&task_active_pid_ns(current)->idr));
> return 0;
> }
>
> diff --git a/fs/proc/loadavg.c b/fs/proc/loadavg.c
> index 983fce5..ba3d0e2 100644
> --- a/fs/proc/loadavg.c
> +++ b/fs/proc/loadavg.c
> @@ -23,7 +23,7 @@ static int loadavg_proc_show(struct seq_file *m, void *v)
> LOAD_INT(avnrun[1]), LOAD_FRAC(avnrun[1]),
> LOAD_INT(avnrun[2]), LOAD_FRAC(avnrun[2]),
> nr_running(), nr_threads,
> - task_active_pid_ns(current)->last_pid);
> + idr_get_cursor(&task_active_pid_ns(current)->idr));
> return 0;
> }
>
> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
> index b09136f..f4db4a7 100644
> --- a/include/linux/pid_namespace.h
> +++ b/include/linux/pid_namespace.h
> @@ -9,15 +9,8 @@
> #include
> #include
> #include
> +#include
>
> -struct pidmap {
> - atomic_t nr_free;
> - void *page;
> -};
> -
> -#define BITS_PER_PAGE(PAGE_SIZE * 8)
> -#define BITS_PER_PAGE_MASK (BITS_PER_PAGE-1)
> -#define PIDMAP_ENTRIES
> ((PID_MAX_LIMIT+BITS_PER_PAGE-1)/BITS_PER_PAGE)
>
> struct fs_pin;
>
> @@ -29,9 +22,8 @@ enum { /* definitions for pid_namespace's hide_pid field */
>
> struct pid_namespace {
> struct kref kref;
> - struct pidmap pidmap[PIDMAP_ENTRIES];
> + struct idr idr;
> struct rcu_head rcu;
> - int last_pid;
> unsigned int nr_hashed;
> struct task_struct *child_reaper;
> struct kmem_cache *pid_cachep;
> @@ -105,6 +97,6 @@ static inline int reboot_pid_ns(struct pid_namespace
> *pid_ns, int cmd)
>
> extern struct pid_namespace *task_active_pid_ns(struct task_struct *tsk);
> void pidhash_init(void);
> -void pidmap_init(void);
> +void pid_idr_init(void);
>
> #endif /* _LINUX_PID_NS_H */
> diff --git a/init/main.c b/init/main.c
> index 0ee9c686..9f4db20 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -667,7 +667,7 @@ asmlinkage __visible void __init start_kernel(void)
> if (late_time_init)
> late_time_init();
> calibrate_delay();
>
Re: [PATCH v2 1/1] arm: topology: remove cpu_efficiency
Hi Dietmar,
On 18 October 2017 at 12:37, Dietmar Eggemann wrote:
> Hi Vincent,
>
> On 17/10/17 13:28, Vincent Guittot wrote:
>> Hi Dietmar,
>>
>> On 12 October 2017 at 16:00, Dietmar Eggemann
>> wrote:
>>> Remove the 'cpu_efficiency/clock-frequency dt property' based solution
>>> to set cpu capacity which was only working for Cortex-A15/A7 arm
>>> big.LITTLE systems.
>>>
>>> I.e. the 'capacity-dmips-mhz' based solution is now the only one. It is
>>> shared between arm and arm64 and works for every big.LITTLE system no
>>> matter which core types it consists of.
>>>
>>> Cc: Russell King
>>> Cc: Vincent Guittot
>>> Cc: Juri Lelli
>>> Signed-off-by: Dietmar Eggemann
>
> [...]
>
>>> @@ -111,76 +50,15 @@ static void __init parse_dt_topology(void)
>>> continue;
>>> }
>>>
>>> - if (topology_parse_cpu_capacity(cn, cpu)) {
>>> + if (topology_parse_cpu_capacity(cn, cpu))
>>> of_node_put(cn);
>>
>> We should call the of_node_put unconditionally to balance the
>> of_get_cpu_node, isn't it ?
>> Note that this problem is also present without your change
>
> Thanks for the review. Brendan mentioned this the other day already.
>
> I could add an additional patch to the v3 with this code change. What do
> you think?
The changes looks good to me
>
> diff --git a/arch/arm/kernel/topology.c b/arch/arm/kernel/topology.c
> index 15cc131ae387..81ec42333489 100644
> --- a/arch/arm/kernel/topology.c
> +++ b/arch/arm/kernel/topology.c
> @@ -41,6 +41,7 @@ static void __init parse_dt_topology(void)
> pr_err("No CPU information found in DT\n");
> return;
> }
> + of_node_put(cn);
>
> for_each_possible_cpu(cpu) {
> /* too early to use cpu->of_node */
> @@ -50,8 +51,8 @@ static void __init parse_dt_topology(void)
> continue;
> }
>
> - if (topology_parse_cpu_capacity(cn, cpu))
> - of_node_put(cn);
> + topology_parse_cpu_capacity(cn, cpu);
> + of_node_put(cn);
> }
>
> topology_normalize_cpu_scale();
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
Re: [Update][PATCH v2 01/12] PM / core: Add NEVER_SKIP and SMART_PREPARE driver flags
On Thu, Oct 19, 2017 at 01:17:31AM +0200, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki > > The motivation for this change is to provide a way to work around > a problem with the direct-complete mechanism used for avoiding > system suspend/resume handling for devices in runtime suspend. > > The problem is that some middle layer code (the PCI bus type and > the ACPI PM domain in particular) returns positive values from its > system suspend ->prepare callbacks regardless of whether the driver's > ->prepare returns a positive value or 0, which effectively prevents > drivers from being able to control the direct-complete feature. > Some drivers need that control, however, and the PCI bus type has > grown its own flag to deal with this issue, but since it is not > limited to PCI, it is better to address it by adding driver flags at > the core level. > > To that end, add a driver_flags field to struct dev_pm_info for flags > that can be set by device drivers at the probe time to inform the PM > core and/or bus types, PM domains and so on on the capabilities and/or > preferences of device drivers. Also add two static inline helpers > for setting that field and testing it against a given set of flags > and make the driver core clear it automatically on driver remove > and probe failures. > > Define and document two PM driver flags related to the direct- > complete feature: NEVER_SKIP and SMART_PREPARE that can be used, > respectively, to indicate to the PM core that the direct-complete > mechanism should never be used for the device and to inform the > middle layer code (bus types, PM domains etc) that it can only > request the PM core to use the direct-complete mechanism for > the device (by returning a positive value from its ->prepare > callback) if it also has been requested by the driver. > > While at it, make the core check pm_runtime_suspended() when > setting power.direct_complete so that it doesn't need to be > checked by ->prepare callbacks. > > Signed-off-by: Rafael J. Wysocki Acked-by: Greg Kroah-Hartman
Re: [PATCH 1/4][PoC][RFC] sched: Allow to get() and put() signal struct
On Wed, Oct 18, 2017 at 10:32:27PM +0200, Krzysztof Opasiak wrote: > Allow to get() and put() signal struct from different > parts of kernel core, not only from signal.c. That says what this does, but not _why_. Who would ever want to have access to these internal-only functions? thanks, greg k-h
[PATCH] perf test: use stat() to check type of file instead of readdir
according to man-page: http://man7.org/linux/man-pages/man3/readdir.3.html
d_type is not supported by all filesystems
-
Currently, only some filesystems (among them: Btrfs, ext2,
ext3, and ext4) have full support for returning the file type
in d_type. All applications must properly handle a return of
DT_UNKNOWN.
-
so if we run the perf test under xfs, all cases under tests/shell/ will not be
covered previously.
Signed-off-by: Li Zhijian
---
tools/perf/tests/builtin-test.c | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 377bea0..0bf1c27 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -409,9 +409,16 @@ static const char *shell_test__description(char
*description, size_t size,
return description ? trim(description + 1) : NULL;
}
-#define for_each_shell_test(dir, ent) \
- while ((ent = readdir(dir)) != NULL)\
- if (ent->d_type == DT_REG && ent->d_name[0] != '.')
+#define for_each_shell_test(path, dir, ent)\
+ while ((ent = readdir(dir)) != NULL) { \
+ int ret;\
+ struct stat sb; \
+ char pathname[4096];\
+ snprintf(pathname, 4096, "%s/%s", path, ent->d_name); \
+ ret = stat(pathname, &sb); \
+ if (ret == 0 && S_ISREG(sb.st_mode) && ent->d_name[0] != '.')
+
+#define for_each_shell_test_end() }
static const char *shell_tests__dir(char *path, size_t size)
{
@@ -450,7 +457,7 @@ static int shell_tests__max_desc_width(void)
if (!dir)
return -1;
- for_each_shell_test(dir, ent) {
+ for_each_shell_test(path, dir, ent) {
char bf[256];
const char *desc = shell_test__description(bf, sizeof(bf),
path, ent->d_name);
@@ -461,6 +468,7 @@ static int shell_tests__max_desc_width(void)
width = len;
}
}
+ for_each_shell_test_end()
closedir(dir);
return width;
@@ -502,7 +510,7 @@ static int run_shell_tests(int argc, const char *argv[],
int i, int width)
if (!dir)
return -1;
- for_each_shell_test(dir, ent) {
+ for_each_shell_test(st.dir, dir, ent) {
int curr = i++;
char desc[256];
struct test test = {
@@ -518,6 +526,7 @@ static int run_shell_tests(int argc, const char *argv[],
int i, int width)
pr_info("%2d: %-*s:", i, width, test.desc);
test_and_print(&test, false, -1);
}
+ for_each_shell_test_end()
closedir(dir);
return 0;
@@ -612,7 +621,7 @@ static int perf_test__list_shell(int argc, const char
**argv, int i)
if (!dir)
return -1;
- for_each_shell_test(dir, ent) {
+ for_each_shell_test(path, dir, ent) {
int curr = i++;
char bf[256];
struct test t = {
@@ -624,6 +633,7 @@ static int perf_test__list_shell(int argc, const char
**argv, int i)
pr_info("%2d: %s\n", i, t.desc);
}
+ for_each_shell_test_end()
closedir(dir);
return 0;
--
2.7.4
Re: linux-next: manual merge of the usb-gadget tree with the usb tree
Hi,
Kees Cook writes:
> On Wed, Oct 18, 2017 at 7:42 AM, Mark Brown wrote:
>> Hi Felipe,
>>
>> Today's linux-next merge of the usb-gadget tree got a conflict in:
>>
>> drivers/usb/gadget/udc/snps_udc_core.c
>>
>> between commit:
>>
>> 29bce57723351f63d ("usb/gadget/snps_udc_core: Convert timers to use
>> timer_setup()")
>>
>> from the usb tree and commit:
>>
>> 46b614affda8667f9 ("usb: gadget: udc: snps_udc_core: use setup_timer()
>> helper.")
>>
>> from the usb-gadget tree.
>>
>> I fixed it up by taking the USB version and can carry the fix as
>> necessary. This is now fixed as far as linux-next is concerned, but any
>> non trivial conflicts should be mentioned to your upstream maintainer
>> when your tree is submitted for merging. You may also want to consider
>> cooperating with the maintainer of the conflicting tree to minimise any
>> particularly complex conflicts.
>
> FWIW, timer_setup() should be preferred over setup_timer().
I'll drop the one from my tree.
thanks
--
balbi
signature.asc
Description: PGP signature
Re: [PATCH v2 3/4] media: i2c: Add TDA1997x HDMI receiver driver
On 10/19/2017 09:20 AM, Tim Harvey wrote:
> On Wed, Oct 18, 2017 at 5:04 AM, Hans Verkuil wrote:
>> Hi Tim,
>>
>> Here is my review of this v2:
>>
>> On 10/12/17 06:45, Tim Harvey wrote:
>>> Add support for the TDA1997x HDMI receivers.
>>>
>
>>> +
>>> +/*
>>> + * Video Input formats
>>> + */
>>> +struct vhref_values {
>>> + u16 href_start;
>>> + u16 href_end;
>>> + u16 vref_f1_start;
>>> + u8 vref_f1_width;
>>> + u16 vref_f2_start;
>>> + u8 vref_f2_width;
>>> + u16 fieldref_f1_start;
>>> + u8 fieldPolarity;
>>> + u16 fieldref_f2_start;
>>
>> Since we don't support interlaced (yet) I'd just drop the 'f2' fields.
>> Ditto for fieldPolarity.
>>
>> Can't these href/vref values be calculated from the timings?
>>
>
> The values in this struct are used to configure the tda1997x VHREF
> timing generator in tda1997x_configure_input_resolution() for
> generating the video output bus timings so I can't really drop them
> unless I can calculate them. Let me look into this - should be
> possible.
>
>>> +};
>>> +
>
>>> +/* parse an infoframe and do some sanity checks on it */
>>> +static unsigned int
>>> +tda1997x_parse_infoframe(struct tda1997x_state *state, u16 addr)
>>> +{
>>> + struct v4l2_subdev *sd = &state->sd;
>>> + union hdmi_infoframe frame;
>>> + u8 buffer[40];
>>> + u8 reg;
>>> + int len, err;
>>> +
>>> + /* read data */
>>> + len = io_readn(sd, addr, sizeof(buffer), buffer);
>>> + err = hdmi_infoframe_unpack(&frame, buffer);
>>> + if (err) {
>>> + v4l_err(state->client,
>>> + "failed parsing %d byte infoframe: 0x%04x/0x%02x\n",
>>> + len, addr, buffer[0]);
>>> + return err;
>>> + }
>>> + if (debug > 1)
>>> + hdmi_infoframe_log(KERN_INFO, &state->client->dev, &frame);
>>> + switch (frame.any.type) {
>>> + /* Audio InfoFrame: see HDMI spec 8.2.2 */
>>> + case HDMI_INFOFRAME_TYPE_AUDIO:
>>> + /* sample rate */
>>> + switch (frame.audio.sample_frequency) {
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
>>> + state->audio_samplerate = 32000;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
>>> + state->audio_samplerate = 44100;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
>>> + state->audio_samplerate = 48000;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
>>> + state->audio_samplerate = 88200;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
>>> + state->audio_samplerate = 96000;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_176400:
>>> + state->audio_samplerate = 176400;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_192000:
>>> + state->audio_samplerate = 192000;
>>> + break;
>>> + default:
>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM:
>>> + break;
>>> + }
>>> +
>>> + /* sample size */
>>> + switch (frame.audio.sample_size) {
>>> + case HDMI_AUDIO_SAMPLE_SIZE_16:
>>> + state->audio_samplesize = 16;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_SIZE_20:
>>> + state->audio_samplesize = 20;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_SIZE_24:
>>> + state->audio_samplesize = 24;
>>> + break;
>>> + case HDMI_AUDIO_SAMPLE_SIZE_STREAM:
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + /* Channel Count */
>>> + state->audio_channels = frame.audio.channels;
>>> + if (frame.audio.channel_allocation &&
>>> + frame.audio.channel_allocation != state->audio_ch_alloc) {
>>> + /* use the channel assignment from the infoframe */
>>> + state->audio_ch_alloc =
>>> frame.audio.channel_allocation;
>>> + tda1997x_configure_audout(sd, state->audio_ch_alloc);
>>> + /* reset the audio FIFO */
>>> + tda1997x_hdmi_info_reset(sd, RESET_AUDIO, false);
>>> + }
>>> + break;
>>> +
>>> + /* Source Product Descriptor information (SPD) */
>>> + case HDMI_INFOFRAME_TYPE_SPD:
>>> + strncpy(frame.spd.vendor, state->vendor,
>>> + sizeof(frame.spd.vendor));
>>> + strncpy(frame.spd.product, state->product,
>>> + sizeof(frame.spd.product));
>>> + v4l_info(state->client, "Source Product Descriptor: %s %s\n",
Re: [PATCH 02/14] soundwire: Add SoundWire bus type
On Thu, 19 Oct 2017 05:03:18 +0200,
Vinod Koul wrote:
>
> --- /dev/null
> +++ b/drivers/soundwire/Kconfig
> @@ -0,0 +1,22 @@
> +#
> +# SoundWire subsystem configuration
> +#
> +
> +menuconfig SOUNDWIRE
> + bool "SoundWire support"
> + ---help---
> + SoundWire is a 2-Pin interface with data and clock line ratified
> + by the MIPI Alliance. SoundWire is used for transporting data
> + typically related to audio functions. SoundWire interface is
> + optimized to integrate audio devices in mobile or mobile inspired
> + systems
> +
> +if SOUNDWIRE
> +
> +comment "SoundWire Devices"
> +
> +config SOUNDWIRE_BUS
> + tristate
> + default SOUNDWIRE
> +
Does it make sense to be tristate?
Since CONFIG_SOUNDWIRE is a bool, the above would be also only either
Y or N. If it's Y and others select M, it'll be still Y.
> --- /dev/null
> +++ b/drivers/soundwire/bus_type.c
> +/**
> + * sdw_get_device_id: find the matching SoundWire device id
> + *
> + * @slave: SoundWire Slave device
> + * @drv: SoundWire Slave Driver
Inconsistent upper/lower letters in these two lines.
> + * The match is done by comparing the mfg_id and part_id from the
> + * struct sdw_device_id. class_id is unused, as it is a placeholder
> + * in MIPI Spec.
> + */
> +static const struct sdw_device_id *
> +sdw_get_device_id(struct sdw_slave *slave, struct sdw_driver *drv)
> +{
> + const struct sdw_device_id *id = drv->id_table;
> +
> + while (id && id->mfg_id) {
> + if (slave->id.mfg_id == id->mfg_id &&
> + slave->id.part_id == id->part_id) {
Please indentation properly.
> + return id;
> + }
Superfluous braces for a single-line.
> + id++;
> + }
> +
> + return NULL;
> +}
> +
> +static int sdw_bus_match(struct device *dev, struct device_driver *ddrv)
> +{
> + struct sdw_slave *slave = dev_to_sdw_dev(dev);
> + struct sdw_driver *drv = drv_to_sdw_driver(ddrv);
> +
> + return !!sdw_get_device_id(slave, drv);
> +}
> +
> +int sdw_slave_modalias(struct sdw_slave *slave, char *buf, size_t size)
I'd put const to slave argument, as it won't be modified.
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -228,6 +228,13 @@ struct hda_device_id {
> unsigned long driver_data;
> };
>
> +struct sdw_device_id {
> + __u16 mfg_id;
> + __u16 part_id;
> + __u8 class_id;
> + kernel_ulong_t driver_data;
Better to think of alignment.
> --- /dev/null
> +++ b/include/linux/soundwire/sdw.h
> +/**
> + * struct sdw_bus: SoundWire bus
> + *
> + * @dev: Master linux device
> + * @link_id: Link id number, can be 0 to N, unique for each Master
> + * @slaves: list of Slaves on this bus
> + * @assigned: logical addresses assigned, Index 0 (broadcast) would be unused
> + * @bus_lock: bus lock
> + */
> +struct sdw_bus {
> + struct device *dev;
> + unsigned int link_id;
> + struct list_head slaves;
> + bool assigned[SDW_MAX_DEVICES + 1];
Why not a bitmap?
thanks,
Takashi
Re: [linux-sunxi] [PATCH v2 5/5] ARM: sun8i: v40: enable USB host ports for Banana Pi M2 Berry
Hi, On Wed, Oct 18, 2017 at 9:06 PM, Icenowy Zheng wrote: > Banana Pi M2 Berry has an on-board USB Hub that provides 4 USB Type-A > ports, and it's connected to the USB1 port of the SoC. > > Enable it. What is the USB hub chip's model? If it's a USB 2.0 hub, then you don't need to enable the OHCI. ChenYu
Re: [PATCH 3/4][PoC][RFC] Connect rlimit-events with process life cycle
On Wed, Oct 18, 2017 at 10:32:29PM +0200, Krzysztof Opasiak wrote: > Add rlimit-events call to process lifecycle to ensure that > we get notified whenever process dies (to cleanup our watch > levels) or forks (to implement watch levels inheritance). > > Signed-off-by: Krzysztof Opasiak > --- > kernel/exit.c | 4 > kernel/fork.c | 16 +++- > 2 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/kernel/exit.c b/kernel/exit.c > index 516acdb0e0ec..c7e435ac4428 100644 > --- a/kernel/exit.c > +++ b/kernel/exit.c > @@ -62,6 +62,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -858,6 +859,9 @@ void __noreturn do_exit(long code) > if (group_dead) > tty_audit_exit(); > audit_free(tsk); > +#ifdef CONFIG_RLIMIT_NOTIFICATION > + rlimit_noti_task_exit(tsk); > +#endif #ifdef should not be needed in a .c file :(
Re: [PATCH 2/4][PoC][RFC] Add rlimit-events framework
Meta-comments on the code, I'm not commenting on the content, just
normal code review things that I always see in kernel code...
On Wed, Oct 18, 2017 at 10:32:28PM +0200, Krzysztof Opasiak wrote:
> diff --git a/include/linux/rlimit_noti_kern.h
> b/include/linux/rlimit_noti_kern.h
> new file mode 100644
> index ..e49fddaa21c0
> --- /dev/null
> +++ b/include/linux/rlimit_noti_kern.h
> @@ -0,0 +1,54 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
I have to ask, do you really mean "any later version" for this, and the
other new files you created?
And, it is nice to use SPDX for new files to identify their license.
It's not that prevelant, but is getting there...
> --- a/include/uapi/linux/netlink.h
> +++ b/include/uapi/linux/netlink.h
> @@ -28,6 +28,7 @@
> #define NETLINK_RDMA 20
> #define NETLINK_CRYPTO 21 /* Crypto layer */
> #define NETLINK_SMC 22 /* SMC monitoring */
> +#define NETLINK_RLIMIT_EVENTS 23 /* rlimit notification */
No tabs?
> --- /dev/null
> +++ b/include/uapi/linux/rlimit_noti.h
> @@ -0,0 +1,71 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
GPLv2+ in a user api header file? You are really brave :)
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef _UAPI_LINUX_RLIMIT_NOTI_H_
> +#define _UAPI_LINUX_RLIMIT_NOTI_H_
> +
> +#ifdef __KERNEL__
> +#include
> +#include
> +#else
> +#include
> +#endif
> +
> +#define RLIMIT_GET_NOTI_FD 1000
> +
> +/* ioctl's */
> +#define RLIMIT_ADD_NOTI_LVL 1
> +#define RLIMIT_RM_NOTI_LVL 2
> +
> +#define RLIMIT_SET_NOTI_ALL 3
> +#define RLIMIT_CLEAR_NOTI_ALL 4
No tabs?
> +
> +/*
> + * For future (notify every 5, 10 units change):
> + * #define RLIMIT_SET_NOTI_STEP 5
> + */
> +
> +#define RLIMIT_GET_NOTI_LVLS 6
> +#define RLIMIT_GET_NOTI_LVL_COUNT 7
> +
> +/* Flags for ioctl's */
> +#define RLIMIT_FLAG_NO_INHERIT (1u << 0)
> +
> +/* Event types */
> +enum {
> + RLIMIT_EVENT_TYPE_RES_CHANGED,
> + RLIMIT_EVENT_TYPE_MAX
> +};
> +
> +/* TODO take care of padding (packed) */
> +struct rlimit_noti_subject {
> + pid_t pid;
> + uint32_t resource;
> +};
For structures that cross the user/kernel boundry, you have to use the
correct variable types. And that is never "unit32_t" and such, use
"__u32" and the other "__" types.
And are you _sure_ that pid_t is able to be exported to userspace
correctly?
> +
> +struct rlimit_noti_level {
> + struct rlimit_noti_subject subj;
> + uint64_t value;
__u64
> + uint32_t flags;
__u32
And so on for all others.
You don't seem to describe an ioctl here in the "normal" method, but
only use vague numbers up above, that's odd, why?
> diff --git a/init/Kconfig b/init/Kconfig
> index 1d3475fc9496..4bc44fa86640 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -332,6 +332,12 @@ config AUDIT_TREE
> depends on AUDITSYSCALL
> select FSNOTIFY
>
> +config RLIMIT_NOTIFICATION
> + bool "Support fd notifications on given resource usage"
> + depends on NET
> + help
> + Enable this to monitor process resource changes usage via fd.
Mix of tab and spaces :(
thanks,
greg k-h
Re: [PATCH v3] dmaengine: rcar-dmac: use TCRB instead of TCR for residue
On Thu, Oct 19, 2017 at 3:15 AM, Kuninori Morimoto
wrote:
> From: Hiroyuki Yokoyama
>
> SYS/RT/Audio DMAC includes independent data buffers for reading
> and writing. Therefore, the read transfer counter and write transfer
> counter have different values.
> TCR indicates read counter, and TCRB indicates write counter.
> The relationship is like below.
>
> TCR TCRB
> [SOURCE] -> [DMAC] -> [SINK]
>
> In the MEM_TO_DEV direction, what really matters is how much data has
> been written to the device. If the DMA is interrupted between read and
> write, then, the data doesn't end up in the destination, so shouldn't
> be counted. TCRB is thus the register we should use in this cases.
>
> In the DEV_TO_MEM direction, the situation is more complex. Both the
> read and write side are important. What matters from a data consumer
> point of view is how much data has been written to memory.
> On the other hand, if the transfer is interrupted between read and
> write, we'll end up losing data. It can also be important to report.
>
> In the MEM_TO_MEM direction, what matters is of course how much data
> has been written to memory from data consumer point of view.
> Here, because read and write have independent data buffers, it will
> take a while for TCR and TCRB to become equal. Thus we should check
> TCRB in this case, too.
>
> Thus, all cases we should check TCRB instead of TCR.
>
> Without this patch, Sound Capture has noise after PluseAudio support
> (= 07b7acb51d2 ("ASoC: rsnd: update pointer more accurate")), because
> the recorder will use wrong residue counter which indicates transferred
> from sound device, but in reality the data was not yet put to memory
> and recorder will record it.
>
> Signed-off-by: Hiroyuki Yokoyama
> [Kuninori: added detail information in log]
> Signed-off-by: Kuninori Morimoto
> ---
> v2 -> v3
>
> - Code is back to same as v1
> - log has more detail explanation
> - From: is back to Yokoyama-san again
Thanks for the update!
Reviewed-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [linux-sunxi] [PATCH v2 0/5] Allwinner R40 USB host support (DT part)
On Wed, Oct 18, 2017 at 9:06 PM, Icenowy Zheng wrote: > This patchset adds support for the USB host ports on Allwiner R40, and > enable them on Banana Pi M2 Ultra and Berry boards. > > The first patch adds USB PHY and EHCI/OHCI nodes to the R40 DTSI. > > The second and third patch adds 5V regulator for the two boards, and > the fourth and fifth patch finally adds USB host ports support. > > Icenowy Zheng (5): > ARM: sun8i: r40: add USB host port nodes for R40 > ARM: sun8i: r40: add 5V regulator for Banana Pi M2 Ultra > ARM: sun8i: v40: add 5V regulator for Banana Pi M2 Berry > ARM: sun8i: r40: enable USB host for Banana Pi M2 Ultra For the four patches, Acked-by: Chen-Yu Tsai > ARM: sun8i: v40: enable USB host ports for Banana Pi M2 Berry See my comments on OHCI and USB hub for the last patch. ChenYu > > arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts | 31 ++ > arch/arm/boot/dts/sun8i-r40.dtsi | 72 > +++ > arch/arm/boot/dts/sun8i-v40-bananapi-m2-berry.dts | 22 +++ > 3 files changed, 125 insertions(+) > > -- > 2.13.6 > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to linux-sunxi+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
[PATCH] platform/x86: silead_dmi: Fix GP-electronic T701 entry
The GP-electronic T701 has its LCD panel mounted upside-down, initially
my plan was to fix this by transparently rotating the image in the i915
driver (my "drm/i915: Deal with upside-down mounted LCD" patch), but
that approach has been rejected instead the kernel will now export
a "panel orientation" property on the drm-connector for the panel and
let userspace deal with it.
But userspace expects the touchscreen coordinates to match the panel
coordinates *before* applying any rotation, so now that we no longer hide
the upside-down-ness of the LCD panel from userspace the coordinates being
generated are wrong and we need to apply a rotation of 180 degrees to the
coordinates to fix this.
Signed-off-by: Hans de Goede
---
drivers/platform/x86/silead_dmi.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/platform/x86/silead_dmi.c
b/drivers/platform/x86/silead_dmi.c
index a0f80f971081..ac304f2318cc 100644
--- a/drivers/platform/x86/silead_dmi.c
+++ b/drivers/platform/x86/silead_dmi.c
@@ -85,6 +85,8 @@ static const struct silead_ts_dmi_data
surftab_wintron70_st70416_6_data = {
static const struct property_entry gp_electronic_t701_props[] = {
PROPERTY_ENTRY_U32("touchscreen-size-x", 960),
PROPERTY_ENTRY_U32("touchscreen-size-y", 640),
+ PROPERTY_ENTRY_BOOL("touchscreen-inverted-x"),
+ PROPERTY_ENTRY_BOOL("touchscreen-inverted-y"),
PROPERTY_ENTRY_STRING("firmware-name",
"gsl1680-gp-electronic-t701.fw"),
{ }
--
2.14.2
Re: Representative Request
Good day My name is Lutz Albrecht, an attorney, I have searched for several years to locate any possible relative of a late client of mine who died intestate but left a huge financial estate. I am contacting you because you have the same last name to present you as the beneficiary of the estate. If you are interested to find out more, please get back to me. Thanks Lutz
Re: [PATCH v6 6/7] KVM: arm64: allow get exception information from userspace
On 2017/10/7 1:31, James Morse wrote: > Hi gengdongjiu, > > On 27/09/17 12:07, gengdongjiu wrote: >> On 2017/9/23 0:51, James Morse wrote: >>> If this wasn't a firmware-first notification, then you're right KVM hands >>> the >>> guest an asynchronous external abort. This could be considered a bug in >>> KVM. (we >>> can discuss with Marc and Christoffer what it should do), but: >>> >>> I'm not sure what scenario you could see this in: surely all your >>> CPU:external-aborts are taken to EL3 by SCR_EL3.EA and become firmware-first >>> notifications. So they should always be claimed by APEI. > >> Yes, if it is firmware-first we should not exist such issue. > > [...] > >>> What you may be seeing is some awkwardness with the change in the SError ESR >>> with v8.2. Previously the VSE mechanism injected an impdef SError, (but they >>> were all impdef so it didn't matter). >>> With VSESR_EL2 KVM has to specify one, and all-zeros is a bad choice as this >>> means 'classified as a RAS error ... unknown!'. > >>> I have a patch in the upcoming SError/RAS series that changes KVMs >>> virtual-abort >>> code to specify an impdef ESR for this path. > > https://www.spinics.net/lists/arm-kernel/msg609589.html > > >> Before I remember Marc and you suggest me specify the an impdef ESR (set the >> vsesr_el2) in >> the userspace, > > If Qemu/kvmtool wants to emulate a machine that notifies the guest about > firmware-first RAS Errors using SError/SEI, it needs to decide when to send > these SError and what ESR to specify. yes, it is. I agree. > > >> now do you want to specify an impdef ESR in KVM instead of usrspace? > > No, that patch is just trying to fixup the existing cases where KVM already > injects an SError. I'm just trying to keep the behaviour the-same: > > Before the RAS Extensions the guest would always get an impdef SError ESR. > After the RAS Extensions KVM has to pick an ESR, as otherwise the guest gets > the > hardware-reset value of VSESR_EL2. On the FVP this is all-zeros, which is a > RAS > encoding. That patch changes it to still be an impdef SError ESR. > > >> if setting the vsesr_el2 in the KVM, whether user-space need to specify? > > I think we need a KVM CAP API to do this. With that patch it can be wired into > pend_guest_serror(), which takes the ESR to make pending if the CPU supports > it. For this CAP API, I have made a patch in the new series patches. > > It's not clear to me whether its useful for user-space to make an SError > pending > even if it can't set the ESR why it can not set the ESR? In the KVM, we can return a encoding fault info to userspace, then user space can specify its own ESR for guest. I already made a patch for it. > > [...] > >>> Because the SEI notification depends on v8.2 I'd like to get the SError/RAS >>> series posted (currently re-testing), then I'll pick up enough of the >>> patches >>> you've posted for a consolidated version of the series, and we can take the >>> discussion from there. > >> James, it is great, we can make a consolidated version of the series. > > We need to get some of the wider issues fixed first, the big-ugly one is > memory_failure_queue() not being NMI safe. (this isn't a problem for SEA as > this > would only become re-entrant if the kernel text was corrupt). It is a problem > for SEI and SDEI. For memory_failure_queue(), I think the big problem is it is in a process context, not error handling context. there are two context. and the memory_failure_queue() is scheduled later than the error handling. > > >>> I'd still like to know what your firmware does if the normal-world believes >>> its >>> masked physical-SError and you want to hand it an SEI notification. > > Aha, thanks for this: > >> firstly the physical-SError that happened in the EL2/EL1/EL1 can not be >> masked if SCR_EL3.EA is set. > > Masked for the CPU because the CPU can deliver the SError to EL3. > > What about software? Code at EL1 and EL2 each have a PSTATE.A bit they may > have > set. HCR_EL2.VSE respects EL1's PSTATE.A ... the question is does your > firmware > respect the PSTATE.A value of the exception level that SError are routed to? Before route to the target EL, software set the spsr_el3.A to 1, then "eret", the PSTATE.A will be to 1. Note: PSTATE.A is shared by different EL, in the hardware, it is one register, not many registers. spsr_elx has more registers, such as spsr_el1, spsr_el2, spsr_el3. > > >> when trap to EL3, firmware will record the error to APEI CPER from reading >> ERR* RAS registers. >> >> (1) if HCR_EL2.TEA is set to 1, exception come from EL0, El1. firmware knows >> this > > HCR_EL2.TEA covers synchronous-external-aborts. For SError you need to check > HCR_EL2.AMO. Some crazy hypervisor may set one and not the other. sorry, it is typo issue, should check HCR_EL2.AMO > > >> SError come from guest OS, copy the elr_el3 to elr_el2, copy ESR_El3 to >> ESR_EL2. > > The EC value in the ELR des
Re: [PATCH v5] mm, sysctl: make NUMA stats configurable
On Wed 18-10-17 09:42:07, Kemi Wang wrote: > This is the second step which introduces a tunable interface that allow > numa stats configurable for optimizing zone_statistics(), as suggested by > Dave Hansen and Ying Huang. > > = > When page allocation performance becomes a bottleneck and you can tolerate > some possible tool breakage and decreased numa counter precision, you can > do: > echo 0 > /proc/sys/vm/numa_stat > In this case, numa counter update is ignored. We can see about > *4.8%*(185->176) drop of cpu cycles per single page allocation and reclaim > on Jesper's page_bench01 (single thread) and *8.1%*(343->315) drop of cpu > cycles per single page allocation and reclaim on Jesper's page_bench03 (88 > threads) running on a 2-Socket Broadwell-based server (88 threads, 126G > memory). > > Benchmark link provided by Jesper D Brouer(increase loop times to > 1000): > https://github.com/netoptimizer/prototype-kernel/tree/master/kernel/mm/ > bench > > = > When page allocation performance is not a bottleneck and you want all > tooling to work, you can do: > echo 1 > /proc/sys/vm/numa_stat > This is system default setting. > > Many thanks to Michal Hocko, Dave Hansen, Ying Huang and Vlastimil Babka > for comments to help improve the original patch. > > ChangeLog: > V4->V5 > a) Scope vm_numa_stat_lock into the sysctl handler function, as suggested > by Michal Hocko; > b) Only allow 0/1 value when setting a value to numa_stat at userspace, > that would keep the possibility for add auto mode in future (e.g. 2 for > auto mode), as suggested by Michal Hocko. > > V3->V4 > a) Get rid of auto mode of numa stats, and may add it back if necessary, > as alignment before; > b) Skip NUMA_INTERLEAVE_HIT counter update when numa stats is disabled, > as reported by Andrey Ryabinin. See commit "de55c8b2519" for details > c) Remove extern declaration for those clear_numa_ function, and make > them static in vmstat.c, as suggested by Vlastimil Babka. > > V2->V3: > a) Propose a better way to use jump label to eliminate the overhead of > branch selection in zone_statistics(), as inspired by Ying Huang; > b) Add a paragraph in commit log to describe the way for branch target > selection; > c) Use a more descriptive name numa_stats_mode instead of vmstat_mode, > and change the description accordingly, as suggested by Michal Hocko; > d) Make this functionality NUMA-specific via ifdef > > V1->V2: > a) Merge to one patch; > b) Use jump label to eliminate the overhead of branch selection; > c) Add a single-time log message at boot time to help tell users what > happened. > > Reported-by: Jesper Dangaard Brouer > Suggested-by: Dave Hansen > Suggested-by: Ying Huang > Signed-off-by: Kemi Wang Acked-by: Michal Hocko Thanks! > --- > Documentation/sysctl/vm.txt | 16 +++ > include/linux/vmstat.h | 10 +++ > kernel/sysctl.c | 9 ++ > mm/mempolicy.c | 3 ++ > mm/page_alloc.c | 6 > mm/vmstat.c | 70 > + > 6 files changed, 114 insertions(+) > > diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt > index 9baf66a..f65c5c7 100644 > --- a/Documentation/sysctl/vm.txt > +++ b/Documentation/sysctl/vm.txt > @@ -58,6 +58,7 @@ Currently, these files are in /proc/sys/vm: > - percpu_pagelist_fraction > - stat_interval > - stat_refresh > +- numa_stat > - swappiness > - user_reserve_kbytes > - vfs_cache_pressure > @@ -792,6 +793,21 @@ with no ill effects: errors and warnings on these stats > are suppressed.) > > == > > +numa_stat > + > +This interface allows runtime configuration of numa statistics. > + > +When page allocation performance becomes a bottleneck and you can tolerate > +some possible tool breakage and decreased numa counter precision, you can > +do: > + echo 0 > /proc/sys/vm/numa_stat > + > +When page allocation performance is not a bottleneck and you want all > +tooling to work, you can do: > + echo 1 > /proc/sys/vm/numa_stat > + > +== > + > swappiness > > This control is used to define how aggressive the kernel will swap > diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h > index ade7cb5..c605c94 100644 > --- a/include/linux/vmstat.h > +++ b/include/linux/vmstat.h > @@ -6,9 +6,19 @@ > #include > #include > #include > +#include > > extern int sysctl_stat_interval; > > +#ifdef CONFIG_NUMA > +#define ENABLE_NUMA_STAT 1 > +#define DISABLE_NUMA_STAT 0 > +extern int sysctl_vm_numa_stat; > +DECLARE_STATIC_KEY_TRUE(vm_numa_stat_key); > +extern int sysctl_vm_numa_stat_handler(struct ctl_table *table, > +
Re: [PATCH] clk: tegra: dfll: Fix drvdata overwriting issue
On Thu, Oct 12, 2017 at 04:09:59PM -0700, Nicolin Chen wrote:
> Both tegra124-dfll and clk-dfll are using platform_set_drvdata
> to set drvdata of the exact same pdev while they use different
> pointers for the drvdata. Once the drvdata has been overwritten
> by tegra124-dfll, clk-dfll will never get its td pointer as it
> expects.
>
> Since tegra124-dfll merely needs its soc pointer in its remove
> function, this patch fixes the bug by removing the overwriting
> in the tegra124-dfll file and letting the tegra_dfll_unregister
> return an soc pointer for it.
>
> Signed-off-by: Nicolin Chen
Acked-By: Peter De Schrijver
> ---
>
> As I don't have a T124 platform, I can't verify it on a board.
> If someone could provide a Tested-by, it would be very helpful.
> The test could be a simple system suspend/resume that activates
> runtime_suspend/resume() where a platform_get_drvdata() would
> be called. -- Nicolin
>
> drivers/clk/tegra/clk-dfll.c | 10 +-
> drivers/clk/tegra/clk-dfll.h | 2 +-
> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 12 +---
> 3 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-dfll.c b/drivers/clk/tegra/clk-dfll.c
> index 2c44aeb..0a7deee 100644
> --- a/drivers/clk/tegra/clk-dfll.c
> +++ b/drivers/clk/tegra/clk-dfll.c
> @@ -1728,10 +1728,10 @@ EXPORT_SYMBOL(tegra_dfll_register);
> * @pdev: DFLL platform_device *
> *
> * Unbind this driver from the DFLL hardware device represented by
> - * @pdev. The DFLL must be disabled for this to succeed. Returns 0
> - * upon success or -EBUSY if the DFLL is still active.
> + * @pdev. The DFLL must be disabled for this to succeed. Returns a
> + * soc pointer upon success or -EBUSY if the DFLL is still active.
> */
> -int tegra_dfll_unregister(struct platform_device *pdev)
> +struct tegra_dfll_soc_data *tegra_dfll_unregister(struct platform_device
> *pdev)
> {
> struct tegra_dfll *td = platform_get_drvdata(pdev);
>
> @@ -1739,7 +1739,7 @@ int tegra_dfll_unregister(struct platform_device *pdev)
> if (td->mode != DFLL_DISABLED) {
> dev_err(&pdev->dev,
> "must disable DFLL before removing driver\n");
> - return -EBUSY;
> + return ERR_PTR(-EBUSY);
> }
>
> debugfs_remove_recursive(td->debugfs_dir);
> @@ -1753,6 +1753,6 @@ int tegra_dfll_unregister(struct platform_device *pdev)
>
> reset_control_assert(td->dvco_rst);
>
> - return 0;
> + return td->soc;
> }
> EXPORT_SYMBOL(tegra_dfll_unregister);
> diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
> index ed2ad88..83352c8 100644
> --- a/drivers/clk/tegra/clk-dfll.h
> +++ b/drivers/clk/tegra/clk-dfll.h
> @@ -43,7 +43,7 @@ struct tegra_dfll_soc_data {
>
> int tegra_dfll_register(struct platform_device *pdev,
> struct tegra_dfll_soc_data *soc);
> -int tegra_dfll_unregister(struct platform_device *pdev);
> +struct tegra_dfll_soc_data *tegra_dfll_unregister(struct platform_device
> *pdev);
> int tegra_dfll_runtime_suspend(struct device *dev);
> int tegra_dfll_runtime_resume(struct device *dev);
>
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index ad1c1cc..269d359 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -125,19 +125,17 @@ static int tegra124_dfll_fcpu_probe(struct
> platform_device *pdev)
> return err;
> }
>
> - platform_set_drvdata(pdev, soc);
> -
> return 0;
> }
>
> static int tegra124_dfll_fcpu_remove(struct platform_device *pdev)
> {
> - struct tegra_dfll_soc_data *soc = platform_get_drvdata(pdev);
> - int err;
> + struct tegra_dfll_soc_data *soc;
>
> - err = tegra_dfll_unregister(pdev);
> - if (err < 0)
> - dev_err(&pdev->dev, "failed to unregister DFLL: %d\n", err);
> + soc = tegra_dfll_unregister(pdev);
> + if (IS_ERR(soc))
> + dev_err(&pdev->dev, "failed to unregister DFLL: %ld\n",
> + PTR_ERR(soc));
>
> tegra_cvb_remove_opp_table(soc->dev, soc->cvb, soc->max_freq);
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V8 5/5] libata: Align DMA buffer to dma_get_cache_alignment()
On 18/10/17 14:03, Tejun Heo wrote:
On Tue, Oct 17, 2017 at 04:05:42PM +0800, Huacai Chen wrote:
In non-coherent DMA mode, kernel uses cache flushing operations to
maintain I/O coherency, so in ata_do_dev_read_id() the DMA buffer
should be aligned to ARCH_DMA_MINALIGN. Otherwise, If a DMA buffer
and a kernel structure share a same cache line, and if the kernel
structure has dirty data, cache_invalidate (no writeback) will cause
data corruption.
Cc: sta...@vger.kernel.org
Signed-off-by: Huacai Chen
---
drivers/ata/libata-core.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ee4c1ec..e134955 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1833,8 +1833,19 @@ static u32 ata_pio_mask_no_iordy(const struct ata_device
*adev)
unsigned int ata_do_dev_read_id(struct ata_device *dev,
struct ata_taskfile *tf, u16 *id)
{
- return ata_exec_internal(dev, tf, NULL, DMA_FROM_DEVICE,
-id, sizeof(id[0]) * ATA_ID_WORDS, 0);
+ u16 *devid;
+ int res, size = sizeof(u16) * ATA_ID_WORDS;
+
+ if (IS_ALIGNED((unsigned long)id, dma_get_cache_alignment(&dev->tdev)))
+ res = ata_exec_internal(dev, tf, NULL, DMA_FROM_DEVICE, id,
size, 0);
+ else {
+ devid = kmalloc(size, GFP_KERNEL);
+ res = ata_exec_internal(dev, tf, NULL, DMA_FROM_DEVICE, devid,
size, 0);
+ memcpy(id, devid, size);
+ kfree(devid);
+ }
+
+ return res;
Hmm... I think it'd be a lot better to ensure that the buffers are
aligned properly to begin with. There are only two buffers which are
used for id reading - ata_port->sector_buf and ata_device->id. Both
are embedded arrays but making them separately allocated aligned
buffers shouldn't be difficult.
Thanks.
FWIW, I agree that the buffers used for DMA should be split out from the
structure. We ran into this problem on MIPS last year,
4ee34ea3a12396f35b26d90a094c75db95080baa ("libata: Align ata_device's id
on a cacheline") partially fixed it, but likely should have also
cacheline aligned the following devslp_timing in the struct such that we
guarantee that members of the struct not used for DMA do not share the
same cacheline as the DMA buffer. Not having this means that
architectures, such as MIPS, which in some cases have to perform manual
invalidation of DMA buffer can clobber valid adjacent data if it is in
the same cacheline.
Thanks,
Matt
Re: [PATCH 6/7] gpio: brcmstb: consolidate interrupt domains
Hi Doug,
On Wed, Oct 04, 2017 at 02:24:37PM -0700, Doug Berger wrote:
> On 10/03/2017 08:03 PM, Gregory Fong wrote:
> > On Fri, Sep 29, 2017 at 8:40 PM, Doug Berger wrote:
> >> The GPIOLIB IRQ chip helpers were very appealing, but badly broke
> >> the 1:1 mapping between a GPIO controller's device_node and its
> >> interrupt domain.
> >
> > Out of curiosity, what sort of problems have you seen from this?
> >
>
> [snip]
>
> When another device-tree node references a GPIO device as its interrupt
> parent, the irq_create_of_mapping() function looks for the irq domain of
> the GPIO device and since all bank irq domains reference the same GPIO
> device node it always resolves to the irq domain of the first bank
> regardless of which bank the number of the GPIO should resolve. This
> domain can only map hwirq numbers 0-31 so interrupts on GPIO above that
> can't be mapped by the device-tree.
Thanks for clarifying. This would be great information to include in
the commit message.
>
> >>
> >> This commit consolidates the per bank irq domains to a version
> >> where we have one larger interrupt domain per GPIO controller
> >> instance spanning multiple GPIO banks.
> >
> > This works (and is reminiscent to my initially submitted
> > implementation at [1]), but I think it might make sense to keep as-is
> > (using the gpiolib irqchip helpers), and instead allocate an irqchip
> > fwnode per bank and use to_of_node() to set it as the of_node for the
> > gpiochip before calling gpiochip_irqchip_add(). OTOH, that capability
> > might go away...
> >
> > Linus, can you comment on the FIXME in gpiochip_irqchip_add_key() that
> > says "get rid of this and use gpiochip->parent->of_node everywhere"?
> > It seems like it would still be beneficial to be able to override the
> > associated node for a gpiochip, since that's what's used for the
> > irqdomain, but if that's going away, obviously we don't want to start
> > using that now.
> >
>
> Yes, this is effectively a reversion to an earlier implementation. I
> produced an implementation based on the generic irqchip libraries, but
> that was stripped from this submission when I discovered that no support
> exists within the generic irqchip libraries for removal of domain
> generic chips and we wanted to preserve the module support of this driver.
Considering this is heavily based on my initial implementation (several
functions are exactly the same), it'd be nice to have some small
attribution in the commit message. :-)
>
> It is conceivable that the current GPIO device-tree nodes could be
> broken down into separate devices per bank, but it is believed that this
> would only confuse things for users of the device as the concept
> diverges from the concept expressed in device documentation.
OK, that sounds like a worse alternative. And since these are all
actually using the same parent IRQ, it does make sense to keep them all
in the same IRQ domain.
On Fri, Sep 29, 2017 at 08:40:56PM -0700, Doug Berger wrote:
> [snip]
> diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
> index e2fff559c1ca..752a46ce3589 100644
> --- a/drivers/gpio/gpio-brcmstb.c
> +++ b/drivers/gpio/gpio-brcmstb.c
> [snip]
> @@ -77,12 +79,18 @@ brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank
> *bank)
> return status;
> }
>
> +static int brcmstb_gpio_hwirq_to_offset(irq_hw_number_t hwirq,
> + struct brcmstb_gpio_bank *bank)
> +{
> + return hwirq - (bank->gc.base - bank->parent_priv->gpio_base);
> +}
> +
> static void brcmstb_gpio_set_imask(struct brcmstb_gpio_bank *bank,
> unsigned int offset, bool enable)
> {
> struct gpio_chip *gc = &bank->gc;
> struct brcmstb_gpio_priv *priv = bank->parent_priv;
> - u32 mask = gc->pin2mask(gc, offset);
> + u32 mask = BIT(brcmstb_gpio_hwirq_to_offset(offset, bank));
Consider renaming "offset" to "hwirq".
> u32 imask;
> unsigned long flags;
>
> @@ -96,6 +104,17 @@ static void brcmstb_gpio_set_imask(struct
> brcmstb_gpio_bank *bank,
> spin_unlock_irqrestore(&gc->bgpio_lock, flags);
> }
>
> +static int brcmstb_gpio_to_irq(struct gpio_chip *gc, unsigned gc_offset)
> +{
> + struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
> + /* gc_offset is relative to this gpio_chip; want real offset */
> + int offset = gc_offset + (gc->base - priv->gpio_base);
Consider renaming "gc_offset" to "offset" and "offset" to "hwirq" to
keep things consistent.
> +
> + if (offset >= priv->num_gpios)
> + return -ENXIO;
> + return irq_create_mapping(priv->irq_domain, offset);
> +}
> +
>
> [snip]
>
> @@ -226,18 +245,19 @@ static irqreturn_t brcmstb_gpio_wake_irq_handler(int
> irq, void *data)
> static void brcmstb_gpio_irq_bank_handler(struct brcmstb_gpio_bank *bank)
> {
> struct brcmstb_gpio_priv *priv = bank->parent_priv;
> - struct irq_domain *irq_domain = bank->gc.irqdomain;
> + struc
Re: [PATCH] virtio_balloon: fix deadlock on OOM
On 10/19/2017 01:19 AM, Michael S. Tsirkin wrote: On Fri, Oct 13, 2017 at 11:06:23PM +0900, Tetsuo Handa wrote: Michael S. Tsirkin wrote: This is a replacement for [PATCH] virtio: avoid possible OOM lockup at virtballoon_oom_notify() but unlike that patch it actually deflates on oom even in presence of lock contention. But Wei Wang is proposing VIRTIO_BALLOON_F_SG which will try to allocate memory, isn't he? Hopefully that can be fixed by allocating outside the lock. I think that would still have an issue even without the lock, because we can't do any memory allocation in the OOM code path. Probably, we could write a separate function, leak_balloon_oom() for the oom notifier, which puts the oom deflating pages to the vq one by one, and kick when the vq is full. In this case, we would need to stop the normal leak_balloon while oom deflating starts. However, a better optimization I think would be to do some kind of consolidation, since leak_balloon is already deflating, leak_ballon_oom can just count the number of pages that have been deflated by leak_balloon and return when it reaches oom_pages. Best, Wei
Re: [PATCH 08/10] PCI: dwc: artpec6: add support for endpoint mode
On Mon, Oct 16, 2017 at 06:43:26PM -0500, Bjorn Helgaas wrote: > understand it. I guess the idea is to build pcie-artpec6.o if either > CONFIG_PCIE_ARTPEC6_HOST or CONFIG_PCIE_ARTPEC6_EP is set (or both). > > Is this really the simplest way to express that in Kconfig? Both the > "if PCIE_ARTPEC6" and this ifneq thing are complicated and relatively > unusual. I thijnk the right style is to make PCIE_ARTPEC6 a hidden kconfig symbol without help text, and then have PCIE_ARTPEC6_HOST and PCIE_ARTPEC6_EP select it.
Re: [Qemu-devel] [RFC 2/2] KVM: add virtio-pmem driver
On Wed, Oct 18, 2017 at 08:51:37AM -0700, Dan Williams wrote: > On Wed, Oct 18, 2017 at 6:03 AM, Stefan Hajnoczi wrote: > > On Tue, Oct 17, 2017 at 04:30:41AM -0400, Pankaj Gupta wrote: > >> > >> > > Are you saying do it as existing i.e ACPI pmem like interface? > >> > > The reason we have created this new driver is exiting pmem driver > >> > > does not define proper semantics for guest flushing requests. > >> > > >> > At this point I'm caring about the Linux-internal interface, and > >> > for that it should be integrated into the nvdimm subsystem and not > >> > a block driver. How the host <-> guest interface looks is a different > >> > idea. > >> > > >> > > > >> > > Regarding block support of driver, we want to achieve DAX support > >> > > to bypass guest page cache. Also, we want to utilize existing DAX > >> > > capable file-system interfaces(e.g fsync) from userspace file API's > >> > > to trigger the host side flush request. > >> > > >> > Well, if you want to support XFS+DAX better don't make it a block > >> > devices, because I'll post patches soon to stop using the block device > >> > entirely for the DAX case. > >> > >> o.k I will look at your patches once they are in mailing list. > >> Thanks for the heads up. > >> > >> If I am guessing it right, we don't need block device additional features > >> for pmem? We can bypass block device features like blk device cache flush > >> etc. > >> Also, still we would be supporting ext4 & XFS filesystem with pmem? > >> > >> If there is time to your patches can you please elaborate on this a bit. > > > > I think the idea is that the nvdimm subsystem already adds block device > > semantics on top of the struct nvdimms that it manages. See > > drivers/nvdimm/blk.c. > > > > So it would be cleaner to make virtio-pmem an nvdimm bus. This will > > eliminate the duplication between your driver and drivers/nvdimm/ code. > > Try "git grep nvdimm_bus_register" to find drivers that use the nvdimm > > subsystem. > > This use case is not "Persistent Memory". Persistent Memory is > something you can map and make persistent with CPU instructions. > Anything that requires a driver call is device driver managed "Shared > Memory". Dan, in that case do you have ideas regarding Christoph Hellwig's comment that this driver should be integrated into the nvdimm subsystem instead of a new block driver? Stefan
Re: [Qemu-devel] [RFC 2/2] KVM: add virtio-pmem driver
On Wed, Oct 18, 2017 at 08:51:37AM -0700, Dan Williams wrote: > This use case is not "Persistent Memory". Persistent Memory is > something you can map and make persistent with CPU instructions. > Anything that requires a driver call is device driver managed "Shared > Memory". How is this any different than the existing nvdimm_flush()? If you really care about the not driver thing it could easily be a write to a doorbell page or a hypercall, but in the end that's just semantics.
Re: [PATCH] hwmon: (coretemp) remove duplicated coretemp for same core id
On Wed, 18 Oct 2017, Guenter Roeck wrote:
> On 10/18/2017 07:28 PM, Shu Wang wrote:
> > > > > > Fix kernel warning on my 4cpus 2core_id system. The cpu0 and cpu1
> > > > > > have same core_id 0, so both cpu0 and cpu1 will try to create file
> > > > > > temp2_label when it's online.
> > > > > >
> > > > >
> > > > > > - coretemp_cpu_online(cpu=0)
> > > > > > - create_core_data(cpu=0, attr_no=2)
> > > > > > - create_core_attrs(attr_no=2)
> > > > > > - coretemp_cpu_online(cpu=1)
> > > > > > - create_core_data(cpu=1, attr_no=2)
> > > > > > - create_core_attrs(attr_no=2)
> > > > > >
> > > > > > $ grep -e processor -e 'core id' /proc/cpuinfo
> > > > > > processor : 0
> > > > > > core id : 0
> > > > > > processor : 1
> > > > > > core id : 0
> > > > > > processor : 2
> > > > > > core id : 1
> > > > > > processor : 3
> > > > > > core id : 1
> > > > >
> > > > > Complete output of /proc/cpuinfo might be helpful.
> > > >
> > > > $ cat /proc/cpuinfo
> > > > processor : 0
> > > > vendor_id : GenuineIntel
> > > > cpu family : 6
> > > > model : 61
> > > > model name : Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz
> > >
> > > This is a hyperthreading CPU, which should already be handled,
> >
> > Do you mean that for my system, coretemp_cpu_online should only
> > be called twice instead of four times to create two core attrs?
> >
>
> coretemp_add_core() should only be called twice, and cpumask_intersects()
> should filter out the duplicate ones.
>
> /*
> * Check whether a thread sibling is already online. If not add the
> * interface for this CPU core.
> */
> if (!cpumask_intersects(&pdata->cpumask,
> topology_sibling_cpumask(cpu)))
> coretemp_add_core(pdev, cpu, 0);
>
> Thomas, is it possible that something is wrong with this code ?
Hrmm. Not that I can see. The only thing I can think of is that the logical
package association of the CPUs is screwed.
Debug patch below.
Thanks,
tglx
8<
--- a/drivers/hwmon/coretemp.c
+++ b/drivers/hwmon/coretemp.c
@@ -611,6 +611,9 @@ static int coretemp_cpu_online(unsigned
if (cpuhp_tasks_frozen)
return 0;
+ pr_info("CPU %u %u pkgid %d\n", cpu, smp_processor_id(),
+ topology_logical_package_id(cpu));
+
/*
* CPUID.06H.EAX[0] indicates whether the CPU has thermal
* sensors. We check this bit only, all the early CPUs
[PATCH 1/1] xen/time: do not decrease steal time after live migration on xen
After guest live migration on xen, steal time in /proc/stat
(cpustat[CPUTIME_STEAL]) might decrease because steal returned by
xen_steal_lock() might be less than this_rq()->prev_steal_time which is
derived from previous return value of xen_steal_clock().
For instance, steal time of each vcpu is 335 before live migration.
cpu 198 0 368 200064 1962 0 0 1340 0 0
cpu0 38 0 81 50063 492 0 0 335 0 0
cpu1 65 0 97 49763 634 0 0 335 0 0
cpu2 38 0 81 50098 462 0 0 335 0 0
cpu3 56 0 107 50138 374 0 0 335 0 0
After live migration, steal time is reduced to 312.
cpu 200 0 370 200330 1971 0 0 1248 0 0
cpu0 38 0 82 50123 500 0 0 312 0 0
cpu1 65 0 97 49832 634 0 0 312 0 0
cpu2 39 0 82 50167 462 0 0 312 0 0
cpu3 56 0 107 50207 374 0 0 312 0 0
The code in this patch is borrowed from do_stolen_accounting() which has
already been removed from linux source code since commit ecb23dc6f2ef
("xen: add steal_clock support on x86"). The core idea of both
do_stolen_accounting() and this patch is to avoid accounting new steal
clock if it is smaller than previous old steal clock.
Similar and more severe issue would impact prior linux 4.8-4.10 as
discussed by Michael Las at
https://0xstubs.org/debugging-a-flaky-cpu-steal-time-counter-on-a-paravirtualized-xen-guest,
which would overflow steal time and lead to 100% st usage in top command
for linux 4.8-4.10. A backport of this patch would fix that issue.
References:
https://0xstubs.org/debugging-a-flaky-cpu-steal-time-counter-on-a-paravirtualized-xen-guest
Signed-off-by: Dongli Zhang
---
drivers/xen/time.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/time.c b/drivers/xen/time.c
index ac5f23f..2b3a996 100644
--- a/drivers/xen/time.c
+++ b/drivers/xen/time.c
@@ -19,6 +19,8 @@
/* runstate info updated by Xen */
static DEFINE_PER_CPU(struct vcpu_runstate_info, xen_runstate);
+static DEFINE_PER_CPU(u64, xen_old_steal);
+
/* return an consistent snapshot of 64-bit time/counter value */
static u64 get64(const u64 *p)
{
@@ -83,9 +85,20 @@ bool xen_vcpu_stolen(int vcpu)
u64 xen_steal_clock(int cpu)
{
struct vcpu_runstate_info state;
+ u64 xen_new_steal;
+ s64 steal_delta;
xen_get_runstate_snapshot_cpu(&state, cpu);
- return state.time[RUNSTATE_runnable] + state.time[RUNSTATE_offline];
+ xen_new_steal = state.time[RUNSTATE_runnable]
+ + state.time[RUNSTATE_offline];
+ steal_delta = xen_new_steal - per_cpu(xen_old_steal, cpu);
+
+ if (steal_delta < 0)
+ xen_new_steal = per_cpu(xen_old_steal, cpu);
+ else
+ per_cpu(xen_old_steal, cpu) = xen_new_steal;
+
+ return xen_new_steal;
}
void xen_setup_runstate_info(int cpu)
--
2.7.4
Re: [PATCH] HID: rmi: Check that a device is a RMI device before calling RMI functions
On Wed, 18 Oct 2017, Hendrik Langer wrote: > Tested-by: Hendrik Langer > > Confirmed, the patch fixes the probem for me. > All freezes/Oops and rmi or input related error messages are gone. Even > keyboard and touchpad are working now! Thanks a lot for confirmation. Now queued in for-4.14/upstream-fixes. -- Jiri Kosina SUSE Labs
Re: [PATCH v16 5/5] virtio-balloon: VIRTIO_BALLOON_F_CTRL_VQ
On 10/13/2017 09:38 PM, Michael S. Tsirkin wrote: On Thu, Oct 12, 2017 at 11:54:56AM +0800, Wei Wang wrote: But I think flushing is very fragile. You will easily run into races if one of the actors gets out of sync and keeps adding data. I think adding an ID in the free vq stream is a more robust approach. Adding ID to the free vq would need the device to distinguish whether it receives an ID or a free page hint, Not really. It's pretty simple: a 64 bit buffer is an ID. A 4K and bigger one is a page. I think we can also use the previous method, free page via in_buf, and id via out_buf. Best, Wei
RE: [RFC PATCH] can: m_can: Support higher speed CAN-FD bitrates
> >>> On 08/18/2017 02:39 PM, Franklin S Cooper Jr wrote: > During test transmitting using CAN-FD at high bitrates (4 Mbps) > only resulted in errors. Scoping the signals I noticed that > only a single bit was being transmitted and with a bit more > investigation realized the actual MCAN IP would go back to > initialization mode automatically. > > It appears this issue is due to the MCAN needing to use the > Transmitter Delay Compensation Mode as defined in the MCAN > User's Guide. When this mode is used the User's Guide indicates > that the Transmitter Delay Compensation Offset register should > be set. The document mentions that this register should be set > to (1/dbitrate)/2*(Func Clk Freq). > > Additional CAN-CIA's "Bit Time Requirements for CAN FD" > document indicates that this TDC mode is only needed for data > bit rates above 2.5 Mbps. > Therefore, only enable this mode and only set TDCO when the > data bit rate is above 2.5 Mbps. > > Signed-off-by: Franklin S Cooper Jr > --- > I'm pretty surprised that this hasn't been implemented already > since the primary purpose of CAN-FD is to go beyond 1 Mbps and > the MCAN IP supports up to 10 Mbps. > > So it will be nice to get comments from users of this driver to > understand if they have been able to use CAN-FD beyond 2.5 Mbps > without this patch. > If they haven't what did they do to get around it if they > needed higher speeds. > > Meanwhile I plan on testing this using a more "realistic" CAN > bus to insure everything still works at 5 Mbps which is the max > speed of my CAN transceiver. > >>> ping. Anyone has any thoughts on this? > >> I added Dong who authored the m_can driver and Wenyou who added > >> the only in-kernel user of the driver for any help. > > I tested it on SAMA5D2 Xplained board both with and without this > > patch, both work with the 4M bps data bit rate. > Thank you for testing this out. Its interesting that you have been > able to use higher speeds without this patch. What is the CAN > transceiver being used on the SAMA5D2 Xplained board? I tried > looking at the schematic but it seems the CAN signals are used on > an extension board which I can't find the schematic for. Also do > you mind sharing your test setup? Were you doing a short point to > point test? > > Thank You, > Franklin > >>> Hello Franklin, > >>> > >>> your patch definitely makes sense. > >>> > >>> I forgot the TDC in my patches because it was not present in the > >>> previous driver versions and because I didn't encounter any problems > >>> when testing it myself. > >>> > >>> The error is highly dependent on the hardware (transceiver) setup. > >>> So it is definitely possible that some people don't encounter errors > >>> without your patch. > >> > >> So the Transmission Delay Compensation feature Value register is > >> suppose to take into consideration the transceiver delay > >> automatically and add the value of TDCO on top of that. So why would > >> TDCO be dependent on the transceiver? I've heard conflicting things > >> regarding TDC so any clarification on what actually impacts it would be > appreciated. > >> > >> Also part of the issue I'm having is how can we properly configure > TDCO? > >> Configuring TDCO is essentially figuring out what Secondary Sample > >> Point to use. However, it is unclear what value to set SSP to and > >> which use cases a given SSP will work or doesn't work. I've seen > >> various recommendations from Bosch on choosing SSP but ultimately it > >> seems they suggestion "real world testing" to come up with a proper > >> value. Not setting TDCO causes problems for my device and improperly > >> setting TDCO causes problems for my device. So its likely any value I > >> use could end up breaking something for someone else. > >> > >> Currently I leaning to a DT property that can be used for setting SSP. > >> Perhaps use a generic default value and allow individuals to override > >> it via DT? > > > > Sounds reasonable. What's the status of this series? > > I have had some offline discussions with Franklin on this, and I am not > fully convinced that DT is the way to go here (although I don't have the > agreement with Franklin there). > > There are two components in configuring the secondary sample point. It is > the transceiver loopback delay and an offset (example half of the bit time > in data phase). > > While the transceiver loopback delay is pretty board dependent (and thus > amenable to DT encoding), I am not quite sure the offset can be configured > in DT because its not really board dependent. > > Unfortunately, offset calculation does not seem to be an exa
Re: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default
* Byungchul Park wrote: > On Thu, Oct 19, 2017 at 08:22:12AM +0200, Ingo Molnar wrote: > > There's no current crash regression that I know of - I'm just outlining the > > conditions of getting all this re-enabled in the next merge window. > > > > Instead of sending two series, could you please send a series that includes > > both > > these fixing + re-enabling patches, plus the false positive fixes? > > > > In particular I think the cross-release re-enabling should be done as the > > last > > patch, so that any future bisections of new false positives won't be made > > more > > difficult by re-introducing the old false positives near the end of the > > bisection. > > I agree. But I already sent v2 before you told me.. > > Do you want me to send patches fixing false positives in the thread > fixing performance regression? No need, I'll reorder them and let you know if there's any problem left. Thanks, Ingo
Re: [PATCH v6 4/5] clocksource: stm32: add clocksource support
2017-10-18 20:59 GMT+02:00 Thomas Gleixner :
> On Wed, 18 Oct 2017, Benjamin Gaignard wrote:
>
>> Rework driver code to be able to implement clocksource and clockevent
>> on the same hardware block.
>> Before this patch only the counter of the hardware block was used to
>> generate clock events. Now counter will be used to provide a 32 bits
>> clock source and a comparator will provide clock events.
>
> Again. Read, understand and comply with the patch submission
> documentation. Proper changelogs are not optional.
>
> "Before this patch " is bogus because it suggests that the patch is
> already applied which is obviously not the case.
>
> Let me give you an example.
>
> The stm32 timer hardware is currently only used as a clock event device,
> but it can be utilized as a clocksource as well.
>
> Implement this by enabling the free running counter in the hardware block
> and converting the clock event part from a count down event timer to a
> comparator based timer.
>
> Can you see the difference?
I will rework the commit message
>
>> -static int stm32_clock_event_set_periodic(struct clock_event_device *evt)
>> +static int stm32_clock_event_set_next_event(unsigned long evt,
>> + struct clock_event_device *clkevt)
>> {
>> - struct timer_of *to = to_timer_of(evt);
>> + struct timer_of *to = to_timer_of(clkevt);
>> + unsigned long cnt;
>>
>> - writel_relaxed(timer_of_period(to), timer_of_base(to) + TIM_ARR);
>> - writel_relaxed(TIM_CR1_ARPE | TIM_CR1_CEN, timer_of_base(to) +
>> TIM_CR1);
>> + cnt = readl_relaxed(timer_of_base(to) + TIM_CNT);
>> + writel_relaxed(cnt + evt, timer_of_base(to) + TIM_CCR1);
>> + writel_relaxed(TIM_DIER_CC1IE, timer_of_base(to) + TIM_DIER);
>
> This implementation is doomed. You cannot rely on the assumption that the
> read/modify/write sequence is 'atomic'.
>
> Bus/pipeline delays, FIQs, hypervisor exits and whatever can delay it
> enough so that the write comes too late which means that you have to wait
> for a full wraparound of the counter for the next interrupt.
>
> See the big fat comment in hpet_next_event() for gory details of issues
> caused by comparator based timers.
Other drivers like prima2 have the same problem.
They have solve it by checking if the current time is still below next event
time after wirting it, so the function will be like that:
unsigned long now, next;
next = readl_relaxed(timer_of_base(to) + TIM_CNT) + evt;
writel_relaxed(next, timer_of_base(to) + TIM_CCR1);
writel_relaxed(TIM_DIER_CC1IE, timer_of_base(to) + TIM_DIER);
now = readl_relaxed(timer_of_base(to) + TIM_CNT);
return next - now > delta ? -ETIME : 0;
>
> Your change of min delay in one of the previous patches is papering over
> this problem and I really wonder if your argumentation of 'required because
> the CPU can't keep up otherwise' is just wrong and you failed to decode the
> RMW issue proper.
The CPU is a CortexM4 @ 200MHZ and the clocks driving the timers are at 90MHZ
with a min delta at 1 you could have an interrupt each 0.01 ms which
is really to much.
By increase it to 0x60 it give time to CPU to handle the interrupt.
Also want to remove 16 bits counters because the maximum period is around 750 ms
which is a short period for a clocksource.
With 32 bits this period is close 47 secondes.
>
> Though fact is, that your implementation CANNOT cover all potential RMW
> disturbances safely. You need a proper safety net for these cases.
>
> To be honest. I prefer having a slow, inaccurate down counting timer over a
> fast comparator based one any time as long as the comparator is not
> cleverly implemented and can do less than equal comparisons which take the
> wraparound of the counter into account. It's not rocket science to do that,
> it just takes a few more gates, but hardware people can't be bothered to
> think about the consequences of their cheap implementations ever.
I will forward you point of to the hardware designer but I will have to deal the
hardware I have anyway.
Benjamin
>
> Thanks,
>
> tglx
>
>
Re: [PATCH v4 06/12] fs/pipe.c: export create_pipe_files() and replace_fd()
On Mon, Oct 16, 2017 at 10:18:49PM +0300, Jarkko Sakkinen wrote: > Export create_pipe_files() and replace_fd() so that the SGX driver is > able to create stdin and stdout pipes. Err, no way.
Re: [PATCH] MAINTAINERS: update TPM driver infrastructure changes
On 15/09/17 23:38, Jarkko Sakkinen wrote: > Signed-off-by: Jarkko Sakkinen > --- > CREDITS | 7 +++ > MAINTAINERS | 13 ++--- > 2 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/CREDITS b/CREDITS > index 5d09c26d69cd..dcd761885ea8 100644 > --- a/CREDITS > +++ b/CREDITS > @@ -4130,6 +4130,13 @@ D: MD driver > D: EISA/sysfs subsystem > S: France > > +N: Marcel Selhorst > +E: tp...@selhorst.net > +D: TPM driver > + > +N: Ashley Lai > +E: ashleyd...@gmail.com > +D: IBM VTPM driver > > # Don't add your name here, unless you really _are_ after Marc > # alphabetically. Leonard used to be very proud of being the Given the comment here, can these two entries be moved to their legitimate location? I'm quite proud (just like Leonard was before I pushed him up) to be at the bottom of that file, and my family has worked hard over generations to ensure its place at the end of most alphabetical lists. I want my patch of CREDIT back! ;-) Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default
* Byungchul Park wrote: > On Thu, Oct 19, 2017 at 03:11:12PM +0900, Byungchul Park wrote: > > On Thu, Oct 19, 2017 at 07:57:30AM +0200, Ingo Molnar wrote: > > > > > > * Byungchul Park wrote: > > > > > > > On Wed, Oct 18, 2017 at 12:09:44PM +0200, Ingo Molnar wrote: > > > > > BTW., have you attempted limiting the depth of the stack traces? I > > > > > suspect more > > > > > than 2-4 are rarely required to disambiguate the calling context. > > > > > > > > I did it for you. Let me show you the result. > > > > > > > > 1. No lockdep: 2.756558155 seconds time > > > > elapsed( +- 0.09% ) > > > > 2. Lockdep: 2.968710420 seconds > > > > time elapsed( +- 0.12% ) > > > > 3. Lockdep + Crossrelease 5 entries:3.153839636 seconds > > > > time elapsed( +- 0.31% ) > > > > 4. Lockdep + Crossrelease 3 entries:3.137205534 seconds > > > > time elapsed( +- 0.87% ) > > > > 5. Lockdep + Crossrelease + This patch: 2.963669551 seconds time > > > > elapsed( +- 0.11% ) > > > > > > I think the lockdep + crossrelease + full-stack numbers are missing? > > > > Ah, the last version of crossrelease merged into vanilla, records 5 > > entries, since I thought it overloads too much if full stack is used, > > and 5 entries are enough. Don't you think so? > > > > > But yeah, looks like single-entry-stacktrace crossrelease only has a > > > +0.2% > > > performance cost (with 0.1% noise), while lockdep itself has a +7.7% cost. > > > > > > That's very reasonable and we can keep the single-entry cross-release > > > feature > > > enabled by default as part of CONFIG_PROVE_LOCKING=y - assuming all the > > > crashes > > > > BTW, is there any crash by cross-release I don't know? Of course, I know > > cases of false positives, but I don't about crash. > > Are you talking about the oops by 'null pointer dereference' by unwinder a > few weeks ago? > > At the time, cross-release was falsely accused. AFAIK, cross-release has > not crashed system yet. I'm talking about the crash fixed here: 8b405d5c5d09: locking/lockdep: Fix stacktrace mess Which was introduced by your patch: ce07a9415f26: locking/lockdep: Make check_prev_add() able to handle external stack_trace ... which was a preparatory patch for cross-release. So 'technically' it's not a cross-release crash, but was very much related. It even says so in the changelog: Actually crossrelease needs to do other than saving a stack_trace. So pass a stack_trace and callback to handle it, to check_prev_add(). ... so let's not pretend it wasn't related, ok? Thanks, Ingo
Re: [PATCH 2/8] mm, truncate: Do not check mapping for every page being truncated
On Wed 18-10-17 08:59:46, Mel Gorman wrote: > During truncation, the mapping has already been checked for shmem and dax > so it's known that workingset_update_node is required. This patch avoids > the checks on mapping for each page being truncated. In all other cases, > a lookup helper is used to determine if workingset_update_node() needs > to be called. The one danger is that the API is slightly harder to use as > calling workingset_update_node directly without checking for dax or shmem > mappings could lead to surprises. However, the API rarely needs to be used > and hopefully the comment is enough to give people the hint. > > sparsetruncate (tiny) > 4.14.0-rc4 4.14.0-rc4 > oneirq-v1r1pickhelper-v1r1 > Min Time 141.00 ( 0.00%) 140.00 ( 0.71%) > 1st-qrtleTime 142.00 ( 0.00%) 141.00 ( 0.70%) > 2nd-qrtleTime 142.00 ( 0.00%) 142.00 ( 0.00%) > 3rd-qrtleTime 143.00 ( 0.00%) 143.00 ( 0.00%) > Max-90% Time 144.00 ( 0.00%) 144.00 ( 0.00%) > Max-95% Time 147.00 ( 0.00%) 145.00 ( 1.36%) > Max-99% Time 195.00 ( 0.00%) 191.00 ( 2.05%) > Max Time 230.00 ( 0.00%) 205.00 ( 10.87%) > AmeanTime 144.37 ( 0.00%) 143.82 ( 0.38%) > Stddev Time 10.44 ( 0.00%)9.00 ( 13.74%) > CoeffTime7.23 ( 0.00%)6.26 ( 13.41%) > Best99%Amean Time 143.72 ( 0.00%) 143.34 ( 0.26%) > Best95%Amean Time 142.37 ( 0.00%) 142.00 ( 0.26%) > Best90%Amean Time 142.19 ( 0.00%) 141.85 ( 0.24%) > Best75%Amean Time 141.92 ( 0.00%) 141.58 ( 0.24%) > Best50%Amean Time 141.69 ( 0.00%) 141.31 ( 0.27%) > Best25%Amean Time 141.38 ( 0.00%) 140.97 ( 0.29%) > > As you'd expect, the gain is marginal but it can be detected. The differences > in bonnie are all within the noise which is not surprising given the impact > on the microbenchmark. > > radix_tree_update_node_t is a callback for some radix operations that > optionally passes in a private field. The only user of the callback is > workingset_update_node and as it no longer requires a mapping, the private > field is removed. > > Signed-off-by: Mel Gorman > Acked-by: Johannes Weiner The patch looks good to me. You can add: Reviewed-by: Jan Kara Honza > --- > fs/dax.c | 2 +- > include/linux/radix-tree.h| 7 +++ > include/linux/swap.h | 13 - > lib/idr.c | 2 +- > lib/radix-tree.c | 30 +- > mm/filemap.c | 7 --- > mm/shmem.c| 2 +- > mm/truncate.c | 2 +- > mm/workingset.c | 10 ++ > tools/testing/radix-tree/multiorder.c | 2 +- > 10 files changed, 39 insertions(+), 38 deletions(-) > > diff --git a/fs/dax.c b/fs/dax.c > index f001d8c72a06..3318ae9046e6 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -565,7 +565,7 @@ static void *dax_insert_mapping_entry(struct > address_space *mapping, > ret = __radix_tree_lookup(page_tree, index, &node, &slot); > WARN_ON_ONCE(ret != entry); > __radix_tree_replace(page_tree, node, slot, > - new_entry, NULL, NULL); > + new_entry, NULL); > entry = new_entry; > } > > diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h > index 567ebb5eaab0..0ca448c1cb42 100644 > --- a/include/linux/radix-tree.h > +++ b/include/linux/radix-tree.h > @@ -301,18 +301,17 @@ void *__radix_tree_lookup(const struct radix_tree_root > *, unsigned long index, > void *radix_tree_lookup(const struct radix_tree_root *, unsigned long); > void __rcu **radix_tree_lookup_slot(const struct radix_tree_root *, > unsigned long index); > -typedef void (*radix_tree_update_node_t)(struct radix_tree_node *, void *); > +typedef void (*radix_tree_update_node_t)(struct radix_tree_node *); > void __radix_tree_replace(struct radix_tree_root *, struct radix_tree_node *, > void __rcu **slot, void *entry, > - radix_tree_update_node_t update_node, void *private); > + radix_tree_update_node_t update_node); > void radix_tree_iter_replace(struct radix_tree_root *, > const struct radix_tree_iter *, void __rcu **slot, void *entry); > void radix_tree_replace_slot(struct radix_tree_root *, >void __rcu **slot, void *entry); > void __radix_tree_delete_node(struct radix_tree_root *, > struct radix_
Re: [PATCH] mmc:host:sdhci-pci:V2-Addition of Arasan PCI controller with integrated phy.
On 10/10/17 20:42, Atul Garg wrote:
> The Arasan controller is based on a FPGA platform and has integrated phy
> with specific phy registers used during the initialization and
> management of different modes. The phy and the controller are integrated
> and registers are very specific to Arasan.
>
> Arasan being an IP provider, licenses these IPs to various companies for
> integration of IP in custom SOCs. The custom SOCs define own register
> map depending on how bits are tied inside the SOC for phy registers,
> depending on SOC memory plan and hence will require own platform drivers.
>
> If more details on phy registers are required, an interface document is
> hosted at https://arasan.com/NF/eMMC5.1 PHY Programming in Linux.pdf.
>
> Signed-off-by: Atul Garg
Sekhar Noir already covered some things. There are some more comments below.
> ---
> drivers/mmc/host/Makefile | 18 +-
> drivers/mmc/host/sdhci-pci-arasan.c | 325
>
> drivers/mmc/host/sdhci-pci-arasan.h | 80 +
> drivers/mmc/host/sdhci-pci-core.c | 16 ++
> drivers/mmc/host/sdhci-pci.h| 4 +
> 5 files changed, 431 insertions(+), 12 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-pci-arasan.c
> create mode 100644 drivers/mmc/host/sdhci-pci-arasan.h
>
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index 2b5a813..b9b2c6c 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -2,15 +2,14 @@
> # Makefile for MMC/SD host controller drivers
> #
>
> -obj-$(CONFIG_MMC_ARMMMCI) += armmmci.o
> -armmmci-y := mmci.o
> -armmmci-$(CONFIG_MMC_QCOM_DML) += mmci_qcom_dml.o
> +obj-$(CONFIG_MMC_ARMMMCI)+= mmci.o
> +obj-$(CONFIG_MMC_QCOM_DML) += mmci_qcom_dml.o
> obj-$(CONFIG_MMC_PXA)+= pxamci.o
> obj-$(CONFIG_MMC_MXC)+= mxcmmc.o
> obj-$(CONFIG_MMC_MXS)+= mxs-mmc.o
> obj-$(CONFIG_MMC_SDHCI) += sdhci.o
> obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
> -sdhci-pci-y += sdhci-pci-core.o sdhci-pci-o2micro.o
> +sdhci-pci-y += sdhci-pci-core.o sdhci-pci-o2micro.o
> sdhci-pci-arasan.o
> obj-$(subst m,y,$(CONFIG_MMC_SDHCI_PCI)) += sdhci-pci-data.o
> obj-$(CONFIG_MMC_SDHCI_ACPI) += sdhci-acpi.o
> obj-$(CONFIG_MMC_SDHCI_PXAV3)+= sdhci-pxav3.o
> @@ -37,13 +36,9 @@ obj-$(CONFIG_MMC_S3C) += s3cmci.o
> obj-$(CONFIG_MMC_SDRICOH_CS) += sdricoh_cs.o
> obj-$(CONFIG_MMC_TMIO) += tmio_mmc.o
> obj-$(CONFIG_MMC_TMIO_CORE) += tmio_mmc_core.o
> -obj-$(CONFIG_MMC_SDHI) += renesas_sdhi_core.o
> -ifeq ($(subst m,y,$(CONFIG_MMC_SDHI_SYS_DMAC)),y)
> -obj-$(CONFIG_MMC_SDHI) += renesas_sdhi_sys_dmac.o
> -endif
> -ifeq ($(subst m,y,$(CONFIG_MMC_SDHI_INTERNAL_DMAC)),y)
> -obj-$(CONFIG_MMC_SDHI) += renesas_sdhi_internal_dmac.o
> -endif
> +tmio_mmc_core-y := tmio_mmc_pio.o
> +tmio_mmc_core-$(subst m,y,$(CONFIG_MMC_SDHI))+= tmio_mmc_dma.o
> +obj-$(CONFIG_MMC_SDHI) += sh_mobile_sdhi.o
> obj-$(CONFIG_MMC_CB710) += cb710-mmc.o
> obj-$(CONFIG_MMC_VIA_SDMMC) += via-sdmmc.o
> obj-$(CONFIG_SDH_BFIN) += bfin_sdh.o
> @@ -89,7 +84,6 @@ obj-$(CONFIG_MMC_SDHCI_MSM) += sdhci-msm.o
> obj-$(CONFIG_MMC_SDHCI_ST) += sdhci-st.o
> obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32) += sdhci-pic32.o
> obj-$(CONFIG_MMC_SDHCI_BRCMSTB) += sdhci-brcmstb.o
> -obj-$(CONFIG_MMC_SDHCI_OMAP) += sdhci-omap.o
>
> ifeq ($(CONFIG_CB710_DEBUG),y)
> CFLAGS-cb710-mmc+= -DDEBUG
> diff --git a/drivers/mmc/host/sdhci-pci-arasan.c
> b/drivers/mmc/host/sdhci-pci-arasan.c
> new file mode 100644
> index 000..57fbf8f
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-pci-arasan.c
> @@ -0,0 +1,325 @@
> +/*
> + * Copyright (C) 2017 Arasan Chip Systems Inc.,
> + *
> + * Authors: Atul Garg
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include
> +#include
> +
> +#include "sdhci.h"
> +#include "sdhci-pci.h"
> +#include "sdhci-pci-arasan.h"
> +
> +static int arasan_phy_write(struct sdhci_host *host, u8 data, u8 offset)
> +{
> + u32 timeout;
> + u8 busy;
> +
> + sdhci_writew(host, data, HOST_PHY_DATA_REG);
> + sdhci_writew(host, ((1<<8) | offset), HOST_PHY_ADDR_REG);
> + timeout = 20;
> + do {
> + busy = sdhci_readw(host, HOST_PHY_ADDR_REG) & (1<<9);
> + if (!busy)
> + break;
> +
Re: [PATCH v6 0/3] livepatch callbacks
On Fri, 13 Oct 2017, Joe Lawrence wrote: > Another turn of the livepatch callback crank. This version cleans up > some minor issues found in v5 and was rebased on top of Jiri's branches > listed below. > > Unfortunately, I didn't have time to test out Petr's suggestion to move > the pre-patch callback inside klp_patch_object() and the post-unpatch > callback into klp_unpatch_object()... let me know if this is a > deal-breaker (I think it would be possible to revisit this later if > need be.) > > Thanks for all the feedback and suggestions thus far! Applied to for-4.15/callbacks branch, thanks a lot. -- Jiri Kosina SUSE Labs
Re: [PATCH 1/3] clocksource/mips-gic-timer: Fix rcu_sched timeouts from multithreading
On 18/10/17 21:34, Thomas Gleixner wrote: On Wed, 11 Oct 2017, Matt Redfearn wrote: When the MIPS GIC clockevent code was written, it appears to have inherited the 0x300 cycle min delta from the MIPS CPU timer driver. This is suboptimal for two reasons. Firstly, the CPU timer counts once every other cycle (i.e. half the clock rate). The GIC counts once per clock. Assuming that the GIC and CPU share the same clock this means the GIC is counting twice as fast, and so the min delta should be (at least) doubled. Fix this by doubling the min delta to 0x600. Secondly, the fixed min delta ignores the fact that with MIPS multithreading active, execution resource within a core is shared between the hardware threads within that core. An inconvenienly timed switch of executing thread within gic_next_event, between the read and write of updated count, can result in the CPU writing an event in the past, and subsequently not receiving a tick interrupt until the counter wraps. This stalls the CPU from the RCU scheduler. Other CPUs detect this and print rcu_sched timeout messages in the kernel log. It can lead to other issues as well if the CPU is holding locks or other resources at the point at which it stalls. Fix this by scaling the min delta for the timer based on the number of threads in the core (smp_num_siblings). This accounts for the greater average runtime of CPUs within a multithreading core. I don't understand why this is not catched by the check at the end of the next_event() function: res = ((int)(gic_read_count() - cnt) >= 0) ? -ETIME : 0; Btw, the local_irq_save() in this function is pointless as this function is always called with interrupts disabled from the core code. Thanks, tglx Hi tglx, This is an issue because in some cases (hrtimer_reprogram -> clockevents_program_event -> clockevents_program_min_delta, when CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=n) there is no retry performed in the case of -ETIME. There has been a patch pending for some time https://patchwork.kernel.org/patch/8909491/ which ought to address this and retry in the case of an event in the past on this call path. But in the meantime this patch vastly improves the situation. Thanks, Matt
Re: [PATCH 3/4] Staging: rtl8188eu: core: rtw_mlme_ext: Use list_for_each_entry_safe
On Thu, Oct 19, 2017 at 01:01:39AM +0530, Srishti Sharma wrote:
> This is a cleanup patch and doesn't change runtime behaviour. It
> changes an open coded list traversal to use list_for_each_entry_safe.
> Done using the following semantic patch by coccinelle.
>
> @r@
> struct list_head* l;
> expression e;
> identifier m,list_del_init,f;
> type T1;
> T1* pos;
> iterator name list_for_each_entry_safe;
> @@
>
> f(...){
>
> +T1* tmp;
> <+...
> -while(...)
> +list_for_each_entry_safe(pos,tmp,l,m)
> {
> ...
> -pos = container_of(l,T1,m);
> ...
> -l=e;
> <+...
> list_del_init(&pos->m)
> ...+>
> }
> ...+>
>
> }
>
> Signed-off-by: Srishti Sharma
> ---
> drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 9 -
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> index 685c071..cf85fb4 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> @@ -5463,6 +5463,8 @@ u8 mlme_evt_hdl(struct adapter *padapter, unsigned char
> *pbuf)
>
> u8 tx_beacon_hdl(struct adapter *padapter, unsigned char *pbuf)
> {
> + struct xmit_frame *tmp;
> +
> if (send_beacon(padapter) == _FAIL) {
> DBG_88E("issue_beacon, fail!\n");
> return H2C_PARAMETERS_ERROR;
> @@ -5486,11 +5488,8 @@ u8 tx_beacon_hdl(struct adapter *padapter, unsigned
> char *pbuf)
> xmitframe_phead = get_list_head(&psta_bmc->sleep_q);
> xmitframe_plist = xmitframe_phead->next;
>
> - while (xmitframe_phead != xmitframe_plist) {
> - pxmitframe = container_of(xmitframe_plist,
> struct xmit_frame, list);
> -
> - xmitframe_plist = xmitframe_plist->next;
> -
> + list_for_each_entry_safe(pxmitframe, tmp,
> + xmitframe_plist, list) {
I'm pretty sure we should get rid of the xmitframe_plist variable and
use xmitframe_phead here.
regards,
dan carpenter
Re: [PATCH] panel: display: Add support for Mitsubishi aa070mc01 TFT panel
On Fri, Sep 08, 2017 at 11:43:02AM +0200, Lukasz Majewski wrote:
> This commit adds support for Mitsubishi aa070mc01 TFT panel working
> with 8 bit ISP mode (pin 19 "mode" HIGH for 20 pin TFT connector).
>
> Signed-off-by: Lukasz Majewski
> ---
> drivers/gpu/drm/panel/panel-simple.c | 35 +++
> 1 file changed, 35 insertions(+)
This is missing a device tree binding.
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index 3d2cb8b..0c64ec6 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1141,6 +1141,38 @@ static const struct panel_desc innolux_g121x1_l03 = {
> },
> };
>
> +static const struct drm_display_mode mitsubishi_aa070mc01_mode = {
> + .clock = 30400,
> + .hdisplay = 800,
> + .hsync_start = 800 + 0,
> + .hsync_end = 800 + 1,
> + .htotal = 800 + 0 + 1 + 160,
> + .vdisplay = 480,
> + .vsync_start = 480 + 0,
> + .vsync_end = 480 + 48 + 1,
> + .vtotal = 480 + 48 + 1 + 0,
> + .vrefresh = 60,
> + .flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC,
> +};
> +
> +static const struct panel_desc mitsubishi_aa070mc01 = {
> + .modes = &mitsubishi_aa070mc01_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 152,
> + .height = 91,
> + },
> +
> + .delay = {
> + .enable = 200,
> + .unprepare = 200,
> + .disable = 400,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> + .bus_flags = DRM_BUS_FLAG_DE_HIGH,
> +};
And this isn't sorted properly.
> +
> static const struct drm_display_mode innolux_n116bge_mode = {
> .clock = 76420,
> .hdisplay = 1366,
> @@ -2029,6 +2061,9 @@ static const struct of_device_id platform_of_match[] = {
> .compatible = "innolux,g121x1-l03",
> .data = &innolux_g121x1_l03,
> }, {
> + .compatible = "mitsubishi,aa070mc01-ca1",
> + .data = &mitsubishi_aa070mc01,
> + }, {
Same here.
Thierry
signature.asc
Description: PGP signature
Re: [PATCH 2/4] Staging: rtl8188eu: core: rtw_ap: Use list_for_each_entry_safe
On Thu, Oct 19, 2017 at 01:01:14AM +0530, Srishti Sharma wrote:
> This is a cleanup patch and doesn't change runtime behaviour. It
> changes an open coded list traversal to use list_for_each_entry_safe.
> Done using the following semantic patch by coccinelle.
>
> @r@
> struct list_head* l;
> expression e;
> identifier m,list_del_init,f;
> type T1;
> T1* pos;
> iterator name list_for_each_entry_safe;
> @@
>
> f(...){
>
> +T1* tmp;
> <+...
> -while(...)
> +list_for_each_entry_safe(pos,tmp,l,m)
> {
> ...
> -pos = container_of(l,T1,m);
> ...
> -l=e;
> <+...
> list_del_init(&pos->m)
> ...+>
> }
> ...+>
>
> }
>
> Signed-off-by: Srishti Sharma
> ---
> drivers/staging/rtl8188eu/core/rtw_ap.c | 34
> +
> 1 file changed, 9 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c
> b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index 32a4837..551af9e 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -280,7 +280,7 @@ void expire_timeout_chk(struct adapter *padapter)
> {
> struct list_head *phead, *plist;
> u8 updated = 0;
> - struct sta_info *psta = NULL;
> + struct sta_info *psta = NULL, *tmp;
> struct sta_priv *pstapriv = &padapter->stapriv;
> u8 chk_alive_num = 0;
> char chk_alive_list[NUM_STA];
> @@ -292,10 +292,7 @@ void expire_timeout_chk(struct adapter *padapter)
> plist = phead->next;
>
> /* check auth_queue */
> - while (phead != plist) {
> - psta = container_of(plist, struct sta_info, auth_list);
> - plist = plist->next;
> -
> + list_for_each_entry_safe(psta, tmp, plist, auth_list) {
This one as well. (I'm reviewing in reverse order) We don't need
plist, just phead.
regards,
dan carpenter
Re: [linux-sunxi] [PATCH v2 6/8] ARM: dts: sun8i: a83t: Move mmc1 pinctrl setting to dtsi file
在 2017-10-19 14:48，Chen-Yu Tsai 写道： On Wed, Oct 18, 2017 at 11:00 PM, Joonas Kylmälä wrote: Hi, Chen-Yu Tsai: mmc1 only has 1 possible pinmux setting. What if someone is using the MMC with bus width 1 and then using the remaining 3 pins for something else? I would very much like to see such a design. Currently the devices we see all follow Allwinner's reference design, with only minor modifications. As such, mmc1 is used exclusively for connecting SDIO-based WiFi modules. If such a radical(?) design is done, the vendor can always add a "mmc1-1bit-pins" setting and override the default. I think this kind of thing happened on A13 -- the MMC2 of A13 is 8-bit, but Lichee Pi One wires it to a SD card slot (4-bit); then the remaining 4 data lines are wired out as GPIO. (Lichee Pi's do not like obeying reference design ;-) ) But I think a MMC/SDIO device under 1-bit mode is too slow that maybe no one will use such a setup. ChenYu
Re: [PATCH v6 4/5] clocksource: stm32: add clocksource support
On Thu, 19 Oct 2017, Benjamin Gaignard wrote:
> 2017-10-18 20:59 GMT+02:00 Thomas Gleixner :
> >> -static int stm32_clock_event_set_periodic(struct clock_event_device *evt)
> >> +static int stm32_clock_event_set_next_event(unsigned long evt,
> >> + struct clock_event_device
> >> *clkevt)
> >> {
> >> - struct timer_of *to = to_timer_of(evt);
> >> + struct timer_of *to = to_timer_of(clkevt);
> >> + unsigned long cnt;
> >>
> >> - writel_relaxed(timer_of_period(to), timer_of_base(to) + TIM_ARR);
> >> - writel_relaxed(TIM_CR1_ARPE | TIM_CR1_CEN, timer_of_base(to) +
> >> TIM_CR1);
> >> + cnt = readl_relaxed(timer_of_base(to) + TIM_CNT);
> >> + writel_relaxed(cnt + evt, timer_of_base(to) + TIM_CCR1);
> >> + writel_relaxed(TIM_DIER_CC1IE, timer_of_base(to) + TIM_DIER);
> >
> > This implementation is doomed. You cannot rely on the assumption that the
> > read/modify/write sequence is 'atomic'.
> >
> > Bus/pipeline delays, FIQs, hypervisor exits and whatever can delay it
> > enough so that the write comes too late which means that you have to wait
> > for a full wraparound of the counter for the next interrupt.
> >
> > See the big fat comment in hpet_next_event() for gory details of issues
> > caused by comparator based timers.
>
> Other drivers like prima2 have the same problem.
That does not make it any better.
> > Your change of min delay in one of the previous patches is papering over
> > this problem and I really wonder if your argumentation of 'required because
> > the CPU can't keep up otherwise' is just wrong and you failed to decode the
> > RMW issue proper.
>
> The CPU is a CortexM4 @ 200MHZ and the clocks driving the timers are at 90MHZ
> with a min delta at 1 you could have an interrupt each 0.01 ms which
> is really to much.
> By increase it to 0x60 it give time to CPU to handle the interrupt.
Fair enough, but exactly this information wants to be in the changelog. And
still, if the hardware only supports 16 bit you still can use the clock
events part and not initialize the clocksource.
> Also want to remove 16 bits counters because the maximum period is around
> 750 ms which is a short period for a clocksource. With 32 bits this
> period is close 47 secondes.
Again. The changelog is missing this information. You cannot expect
reviewers to crystal ball your reasonings.
> > To be honest. I prefer having a slow, inaccurate down counting timer over a
> > fast comparator based one any time as long as the comparator is not
> > cleverly implemented and can do less than equal comparisons which take the
> > wraparound of the counter into account. It's not rocket science to do that,
> > it just takes a few more gates, but hardware people can't be bothered to
> > think about the consequences of their cheap implementations ever.
>
> I will forward you point of to the hardware designer but I will have to deal
> the
> hardware I have anyway.
I know that it's to late. Just wanted to mention it as a general note.
Thanks,
tglx
Re: [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages
On Thu 19-10-17 16:33:56, Joonsoo Kim wrote:
> On Thu, Oct 19, 2017 at 09:15:03AM +0200, Michal Hocko wrote:
> > On Thu 19-10-17 11:51:11, Joonsoo Kim wrote:
[...]
> > > Hello,
> > >
> > > This patch will break the CMA user. As you mentioned, CMA allocation
> > > itself isn't migrateable. So, after a single page is allocated through
> > > CMA allocation, has_unmovable_pages() will return true for this
> > > pageblock. Then, futher CMA allocation request to this pageblock will
> > > fail because it requires isolating the pageblock.
> >
> > Hmm, does this mean that the CMA allocation path depends on
> > has_unmovable_pages to return false here even though the memory is not
> > movable? This sounds really strange to me and kind of abuse of this
>
> Your understanding is correct. Perhaps, abuse or wrong function name.
>
> > function. Which path is that? Can we do the migrate type test theres?
>
> alloc_contig_range() -> start_isolate_page_range() ->
> set_migratetype_isolate() -> has_unmovable_pages()
I see. It seems that the CMA and memory hotplug have a very different
view on what should happen during isolation.
> We can add one argument, 'XXX' to set_migratetype_isolate() and change
> it to check migrate type rather than has_unmovable_pages() if 'XXX' is
> specified.
Can we use the migratetype argument and do the special thing for
MIGRATE_CMA? Like the following diff?
---
diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index d4cd2014fa6f..fa9db0c7b54e 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -30,7 +30,7 @@ static inline bool is_migrate_isolate(int migratetype)
#endif
bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
-bool skip_hwpoisoned_pages);
+int migratetype, bool skip_hwpoisoned_pages);
void set_pageblock_migratetype(struct page *page, int migratetype);
int move_freepages_block(struct zone *zone, struct page *page,
int migratetype, int *num_movable);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index bc50d746a82f..ad2ea7069d14 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7362,6 +7362,7 @@ void *__init alloc_large_system_hash(const char
*tablename,
* race condition. So you can't expect this function should be exact.
*/
bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
+int migratetype,
bool skip_hwpoisoned_pages)
{
unsigned long pfn, iter, found;
@@ -7373,6 +7374,15 @@ bool has_unmovable_pages(struct zone *zone, struct page
*page, int count,
if (zone_idx(zone) == ZONE_MOVABLE)
return false;
+ /*
+* CMA allocations (alloc_contig_range) really need to mark isolate
+* CMA pageblocks even when they are not movable in fact so consider
+* them movable here.
+*/
+ if (is_migrate_cma(migratetype) &&
+ is_migrate_cma(get_pageblock_migratetype(page)))
+ return false;
+
pfn = page_to_pfn(page);
for (found = 0, iter = 0; iter < pageblock_nr_pages; iter++) {
unsigned long check = pfn + iter;
@@ -7458,7 +7468,7 @@ bool is_pageblock_removable_nolock(struct page *page)
if (!zone_spans_pfn(zone, pfn))
return false;
- return !has_unmovable_pages(zone, page, 0, true);
+ return !has_unmovable_pages(zone, page, 0, MIGRATE_MOVABLE, true);
}
#if (defined(CONFIG_MEMORY_ISOLATION) && defined(CONFIG_COMPACTION)) ||
defined(CONFIG_CMA)
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 757410d9f758..8616f5332c77 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -14,7 +14,7 @@
#define CREATE_TRACE_POINTS
#include
-static int set_migratetype_isolate(struct page *page,
+static int set_migratetype_isolate(struct page *page, int migratetype,
bool skip_hwpoisoned_pages)
{
struct zone *zone;
@@ -51,7 +51,7 @@ static int set_migratetype_isolate(struct page *page,
* FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
* We just check MOVABLE pages.
*/
- if (!has_unmovable_pages(zone, page, arg.pages_found,
+ if (!has_unmovable_pages(zone, page, arg.pages_found, migratetype,
skip_hwpoisoned_pages))
ret = 0;
@@ -63,14 +63,14 @@ static int set_migratetype_isolate(struct page *page,
out:
if (!ret) {
unsigned long nr_pages;
- int migratetype = get_pageblock_migratetype(page);
+ int mt = get_pageblock_migratetype(page);
set_pageblock_migratetype(page, MIGRATE_ISOLATE);
zone->nr_isolate_pageblock++;
nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE,
Re: [PATCH v2] pinctrl: mcp23s08: fix interrupt handling regression
On Wed, Oct 18, 2017 at 4:21 PM, Dmitry Mastykin wrote:
> interrupt handling was broken with conversion to using regmap caching.
> cached_gpio value was updated by boolean status instead of gpio reading.
>
> Fixes: 8f38910ba4f6 ("pinctrl: mcp23s08: switch to regmap caching")
> Tested-by: Phil Reid
> Signed-off-by: Dmitry Mastykin
Patch applied for fixes, adding Sebastians review tag.
Yours,
Linus Walleij
Re: [PATCH 1/3] clocksource/mips-gic-timer: Fix rcu_sched timeouts from multithreading
On Thu, 19 Oct 2017, Matt Redfearn wrote: > On 18/10/17 21:34, Thomas Gleixner wrote: > > On Wed, 11 Oct 2017, Matt Redfearn wrote: > > > Secondly, the fixed min delta ignores the fact that with MIPS > > > multithreading active, execution resource within a core is shared > > > between the hardware threads within that core. An inconvenienly timed > > > switch of executing thread within gic_next_event, between the read and > > > write of updated count, can result in the CPU writing an event in the > > > past, and subsequently not receiving a tick interrupt until the counter > > > wraps. This stalls the CPU from the RCU scheduler. Other CPUs detect > > > this and print rcu_sched timeout messages in the kernel log. It can > > > lead to other issues as well if the CPU is holding locks or other > > > resources at the point at which it stalls. Fix this by scaling the min > > > delta for the timer based on the number of threads in the core > > > (smp_num_siblings). This accounts for the greater average runtime of > > > CPUs within a multithreading core. > > > > I don't understand why this is not catched by the check at the end of the > > next_event() function: > > > > res = ((int)(gic_read_count() - cnt) >= 0) ? -ETIME : 0; > > > > Btw, the local_irq_save() in this function is pointless as this function is > > always called with interrupts disabled from the core code. > > This is an issue because in some cases (hrtimer_reprogram -> > clockevents_program_event -> clockevents_program_min_delta, when > CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=n) there is no retry performed in the > case of -ETIME. There has been a patch pending for some time > https://patchwork.kernel.org/patch/8909491/ which ought to address this and > retry in the case of an event in the past on this call path. But in the > meantime this patch vastly improves the situation. I somehow missed that one. Care to repost so we get that solved at the place where it should be solved. Thanks, tglx
Re: [PATCH] clk: tegra: dfll: Fix drvdata overwriting issue
On Thu, Oct 12, 2017 at 04:09:59PM -0700, Nicolin Chen wrote: > Both tegra124-dfll and clk-dfll are using platform_set_drvdata > to set drvdata of the exact same pdev while they use different > pointers for the drvdata. Once the drvdata has been overwritten > by tegra124-dfll, clk-dfll will never get its td pointer as it > expects. > > Since tegra124-dfll merely needs its soc pointer in its remove > function, this patch fixes the bug by removing the overwriting > in the tegra124-dfll file and letting the tegra_dfll_unregister > return an soc pointer for it. > > Signed-off-by: Nicolin Chen > --- > > As I don't have a T124 platform, I can't verify it on a board. > If someone could provide a Tested-by, it would be very helpful. > The test could be a simple system suspend/resume that activates > runtime_suspend/resume() where a platform_get_drvdata() would > be called. -- Nicolin > > drivers/clk/tegra/clk-dfll.c | 10 +- > drivers/clk/tegra/clk-dfll.h | 2 +- > drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 12 +--- > 3 files changed, 11 insertions(+), 13 deletions(-) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [PATCH] arm64: defconfig: Enable Tegra PCI controller
On Wed, Oct 18, 2017 at 05:46:20PM +0300, Tuomas Tynkkynen wrote: > The driver has supported the 64-bit Tegra210 for a while now, so enable > it in the defconfig. > > Signed-off-by: Tuomas Tynkkynen > --- > I didn't personally test this but got a report from our distro users > that it works. > > arch/arm64/configs/defconfig | 1 + > 1 file changed, 1 insertion(+) Applied, thanks. Thierry signature.asc Description: PGP signature
Re: [linux-sunxi] [PATCH v2 6/8] ARM: dts: sun8i: a83t: Move mmc1 pinctrl setting to dtsi file
On Thu, Oct 19, 2017 at 4:18 PM, wrote: > 在 2017-10-19 14:48，Chen-Yu Tsai 写道： >> >> On Wed, Oct 18, 2017 at 11:00 PM, Joonas Kylmälä >> wrote: >>> >>> Hi, >>> >>> Chen-Yu Tsai: mmc1 only has 1 possible pinmux setting. >>> >>> >>> What if someone is using the MMC with bus width 1 and then using the >>> remaining 3 pins for something else? >> >> >> I would very much like to see such a design. Currently the devices >> we see all follow Allwinner's reference design, with only minor >> modifications. As such, mmc1 is used exclusively for connecting >> SDIO-based WiFi modules. >> >> If such a radical(?) design is done, the vendor can always add >> a "mmc1-1bit-pins" setting and override the default. > > > I think this kind of thing happened on A13 -- the MMC2 of A13 is > 8-bit, but Lichee Pi One wires it to a SD card slot (4-bit); then > the remaining 4 data lines are wired out as GPIO. (Lichee Pi's > do not like obeying reference design ;-) ) For MMC2 we have mmc2_pins (4-bit) and mmc2_8bit_emmc_pins (8-bit + emmc reset pin). ChenYu > > But I think a MMC/SDIO device under 1-bit mode is too slow that > maybe no one will use such a setup. > >> >> ChenYu > > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to linux-sunxi+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout.
Re: usb/net/rt2x00: warning in rt2800_eeprom_word_index
On Mon, Oct 16, 2017 at 2:19 PM, Dmitry Vyukov wrote: > On Mon, Oct 16, 2017 at 11:40 AM, Stanislaw Gruszka > wrote: >> Hi Dmitry >> >> On Sat, Oct 14, 2017 at 04:38:03PM +0200, Dmitry Vyukov wrote: >>> On Thu, Oct 12, 2017 at 9:25 AM, Stanislaw Gruszka >>> wrote: >>> > Hi >>> > >>> > On Mon, Oct 09, 2017 at 07:50:53PM +0200, Andrey Konovalov wrote: >>> >> I've got the following report while fuzzing the kernel with syzkaller. >>> >> >>> >> On commit 8a5776a5f49812d29fe4b2d0a2d71675c3facf3f (4.14-rc4). >>> >> >>> >> I'm not sure whether this is a bug in the driver, or just a way to >>> >> report misbehaving device. In the latter case this shouldn't be a >>> >> WARN() call, since WARN() means bug in the kernel. >>> > >>> > This is about wrong EEPROM, which reported 3 tx streams on >>> > non 3 antenna device. I think WARN() is justified and thanks >>> > to the call trace I was actually able to to understand what >>> > happened. >>> > >>> > In general I do not think WARN() only means a kernel bug, it >>> > can be F/W or H/W bug too. >>> >>> Hi Stanislaw, >>> >>> Printing messages is fine. Printing stacks is fine. Just please make >>> them distinguishable from kernel bugs and don't kill the whole >>> possibility of automated Linux kernel testing. That's an important >>> capability. >> >> We do not distinguish between bugs and other problems when WARN() is >> used in (wireless) drivers, what I think is correct, taking comment from >> include/asm-generic/bug.h : >> >> /* >> * WARN(), WARN_ON(), WARN_ON_ONCE, and so on can be used to report >> * significant issues that need prompt attention if they should ever >> * appear at runtime. Use the versions with printk format strings >> * to provide better diagnostics. >> */ >> >> Historically we have BUG() to mark the bugs, but usage if it is not >> recommended as it can kill the system, so for anything that can >> be recovered in runtime - WARN() is recommended. >> >> Perhaps we can introduce another helper like PROBLEM() for marking >> situations when something is wrong, but it is not a bug. However I'm >> not even sure at what extent it can be used, since for many cases >> if not the most, driver author can not tell apriori if the problem >> is a bug in the driver or HW/FW misbehaviour (or maybe particular >> issue can happen because of both). > > I will write a separate email to LKML. Sent a mail titled "Distinguishing kernel bugs from invalid inputs" to LKML. Here is a copy: https://groups.google.com/forum/#!topic/syzkaller/dGh7qtbu14Q
Distinguishing kernel bugs from invalid inputs
Hello, As you may know we are doing some automated kernel testing with syzkaller fuzzer. For that we need to be able to distinguish kernel bugs (something to notify kernel mailing lists about) from console messages provoked by various invalid inputs to kernel (effectively EINVAL coming from user-space or devices, either real or test). From time to time we have problems with "WARNING:" messages. Most of the time they do mean kernel bugs (just not fatal), and we found 100+ bugs based on WARNING messages and kernel mostly follows this meaning of WARNING. But every now and then they are used for invalid inputs and we see some push back from developers saying that it's fine to use WARNING for, say, bad data coming from a USB device. Comments in include/asm-generic/bug.h are not definitive in this regard. So I would like kernel community to define some policy around console output that allows automatically detecting when there is a bug in kernel, and then document it so that we don't need to get back to this question again and again. I think it will also be useful for administrators and users staring at dmesg. And have obvious implications when panic_on_warn is set (not sure if it's used by anybody in production, though). I also heard about effective (unintentional) local DoS caused by buggy programs provoking WARNINGs in tight loop when serial output is actually always captured. I don't have strong preference as to how exactly it should look like. And to make it clear, printing messages and stacks, if necessary, on invalid inputs if fine, it just needs to be distinguishable from kernel bugs. We could use pr_err (not containing "WARNING"!), or there was a mention of a new macro a-la PROBLEM(). Other options? Thoughts? Thanks
Re: [PATCH v3 2/2] livepatch: add atomic replace
On Wed, 18 Oct 2017, Josh Poimboeuf wrote: > On Wed, Oct 18, 2017 at 03:36:42PM +0200, Jiri Kosina wrote: > > On Wed, 18 Oct 2017, Miroslav Benes wrote: > > > > > 3. Drop immediate. It causes problems only and its advantages on x86_64 > > > are theoretical. You would still need to solve the interaction with > > > atomic > > > replace on other architecture with immediate preserved, but that may be > > > easier. Or we can be aggressive and drop immediate completely. The force > > > transition I proposed earlier could achieve the same. > > > > After brief off-thread discussion, I've been thinking about this a bit > > more and I also think that we should claim immediate "an experiment that > > failed", especially as the force functionality (which provides equal > > functionality from the userspace POV) will likely be there sonnish. > > Agreed. > > To clarify, we'll need the force patch before removing > klp_patch.immediate, so we don't break non-x86 arches in the meantime. Yes, that is correct. I've just started to work on the force patch. > On the other hand I think we could remove klp_func.immediate > immediately. > > > While we're on the subject of removing code... :-) > > > I've mentioned this several times before, but the more features we add, > the more obvious this point becomes: if we could figure out how to get > rid of the "patching unloaded modules" feature, the code would be so > much better, and I'd actually be able to fit the code in my brain. Then > we could get rid of all these sneaky bugs that Miroslav and Petr keep > finding, and I wouldn't get an uneasy feeling everytime somebody posts a > new feature. > > Here's one vague idea for how to achieve that. More ideas welcome. > > 1) Make the consistency model synchronous with respect to modules: don't >allow any modules to load or unload until the patch transition is >complete. > > 2) Instead of one big uber patch module which patches vmlinux and >modules at the same time, make each patch module specific to a single >object. Then bundle the patch modules together somehow into a "patch >bundle" so they're treated as a single atomic unit. > > 3) The patch bundle, when loaded, would load some of its patch modules >immediately (for those objects which are already loaded). For >unloaded objects, the corresponding patch modules will be copied into >memory but not loaded. > > 4) Then, when a to-be-patched module is loaded, the module loader loads >it into memory and relocates it, but doesn't make it live. Then it >loads the patch module from the memory blob, makes the patch module >live, and then makes the to-be-patched module live. > > (A variation would be to create a way for user space to load a module in > the paused state. Then user space can handle the dependencies and do > the patch juggling. I think that would mean depmod/modprobe would need > to be involved.) It might be easier to do at least part of it in userspace. > It could be a terrible idea, though it might be worth looking into. I'm worried that we would only move the complexity elsewhere, but maybe not. We'd remove the code from kernel/livepatch/, but some non-trivial changes would go to the module loader. Jessica, would that be possible? We need to discuss it with packaging if that'd be possible to deliver it. Anyway, interesting idea. Miroslav
perf test vfs_getname failed
Hi Arnaldo i encounter some problem when running perf test, the details arelike below: - [lizj@rhel71 perf]$ sudo ./perf test vfs_getname -v 61: Add vfs_getname probe to get syscall args filenames : --- start --- test child forked, pid 5958 Failed to find 'result' in this function. Error: Failed to add events. test child finished with -1 end Add vfs_getname probe to get syscall args filenames: FAILED! 62: Use vfs_getname probe to get syscall args filenames : --- start --- test child forked, pid 5979 Failed to find 'result' in this function. Error: Failed to add events. test child finished with -1 end Use vfs_getname probe to get syscall args filenames: FAILED! 63: Check open filename arg using perf trace + vfs_getname: --- start --- test child forked, pid 6001 Failed to find 'result' in this function. Error: Failed to add events. test child finished with -1 end Check open filename arg using perf trace + vfs_getname: FAILED! - my environment: 1. perf features the enabled features are: - Auto-detecting system features: ... dwarf: [ on ] ...dwarf_getlocations: [ on ] ... glibc: [ on ] ... gtk2: [ on ] ... libaudit: [ on ] ...libbfd: [ on ] ...libelf: [ on ] ... libnuma: [ on ] ...numa_num_possible_cpus: [ on ] ... libperl: [ on ] ... libpython: [ on ] ... libslang: [ OFF ] ... libcrypto: [ on ] ... libunwind: [ on ] ...libdw-dwarf-unwind: [ on ] ... zlib: [ on ] ... lzma: [ on ] ... get_cpuid: [ on ] ... bpf: [ on ] - 2. host [lizj@rhel71 perf]$ cat /etc/redhat-release Red Hat Enterprise Linux Server release 7.1 Beta (Maipo) 3. kernel [lizj@rhel71 perf]$ uname -a Linux rhel71 4.14.0-rc1+ #2 SMP Wed Oct 18 23:49:30 EDT 2017 x86_64 x86_64 x86_64 GNU/Linux if i miss something, fell free to let me know :) Thanks Zhijian
Re: [alsa-devel] [PATCH 02/14] soundwire: Add SoundWire bus type
On Thu, 19 Oct 2017 09:40:06 +0200, Takashi Iwai wrote: > > On Thu, 19 Oct 2017 05:03:18 +0200, > Vinod Koul wrote: > > > > --- /dev/null > > +++ b/drivers/soundwire/Kconfig > > @@ -0,0 +1,22 @@ > > +# > > +# SoundWire subsystem configuration > > +# > > + > > +menuconfig SOUNDWIRE > > + bool "SoundWire support" > > + ---help--- > > + SoundWire is a 2-Pin interface with data and clock line ratified > > + by the MIPI Alliance. SoundWire is used for transporting data > > + typically related to audio functions. SoundWire interface is > > + optimized to integrate audio devices in mobile or mobile inspired > > + systems > > + > > +if SOUNDWIRE > > + > > +comment "SoundWire Devices" > > + > > +config SOUNDWIRE_BUS > > + tristate > > + default SOUNDWIRE > > + > > Does it make sense to be tristate? > Since CONFIG_SOUNDWIRE is a bool, the above would be also only either > Y or N. If it's Y and others select M, it'll be still Y. I found a later patch selecting SOUNDWIRE_BUS. So just drop this "default" line, and always let others selecting the bus. Takashi
[PATCH] tracing: always define trace_{irq,preempt}_{enable_disable}
We get a build error in the irqsoff tracer in some configurations:
kernel/trace/trace_irqsoff.c: In function 'trace_preempt_on':
kernel/trace/trace_irqsoff.c:855:2: error: implicit declaration of function
'trace_preempt_enable_rcuidle'; did you mean 'trace_irq_enable_rcuidle'?
[-Werror=implicit-function-declaration]
trace_preempt_enable_rcuidle(a0, a1);
The problem is that trace_preempt_enable_rcuidle() has different
definition based on multiple Kconfig symbols, but not all combinations
have a valid definition.
This changes the conditions so that we always get exactly one
definition of each of the four tracing macros. I have not tried
to verify that these definitions are sensible, but now we
can build all randconfig combinations again.
Fixes: d59158162e03 ("tracing: Add support for preempt and irq enable/disable
events")
Signed-off-by: Arnd Bergmann
---
include/trace/events/preemptirq.h | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/trace/events/preemptirq.h
b/include/trace/events/preemptirq.h
index f5024c560d8f..9c4eb33c5a1d 100644
--- a/include/trace/events/preemptirq.h
+++ b/include/trace/events/preemptirq.h
@@ -56,15 +56,18 @@ DEFINE_EVENT(preemptirq_template, preempt_enable,
#include
-#else /* !CONFIG_PREEMPTIRQ_EVENTS */
+#endif /* !CONFIG_PREEMPTIRQ_EVENTS */
+#if !defined(CONFIG_PREEMPTIRQ_EVENTS) || defined(CONFIG_PROVE_LOCKING)
#define trace_irq_enable(...)
#define trace_irq_disable(...)
-#define trace_preempt_enable(...)
-#define trace_preempt_disable(...)
#define trace_irq_enable_rcuidle(...)
#define trace_irq_disable_rcuidle(...)
+#endif
+
+#if !defined(CONFIG_PREEMPTIRQ_EVENTS) || !defined(CONFIG_DEBUG_PREEMPT)
+#define trace_preempt_enable(...)
+#define trace_preempt_disable(...)
#define trace_preempt_enable_rcuidle(...)
#define trace_preempt_disable_rcuidle(...)
-
#endif
--
2.9.0
[RFC] mm/swap: Rename pagevec_lru_move_fn() as pagevec_lruvec_move_fn()
The function pagevec_lru_move_fn() actually moves pages from various
per cpu pagevecs into per node lruvecs with a custom function which
knows how to handle individual pages present in any given pagevec.
Because it does movement between pagevecs and lruvecs as whole not
to an individual list element, the name should reflect it.
Signed-off-by: Anshuman Khandual
---
mm/swap.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/mm/swap.c b/mm/swap.c
index a77d68f..fcd82bc 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -185,7 +185,7 @@ int get_kernel_page(unsigned long start, int write, struct
page **pages)
}
EXPORT_SYMBOL_GPL(get_kernel_page);
-static void pagevec_lru_move_fn(struct pagevec *pvec,
+static void pagevec_lruvec_move_fn(struct pagevec *pvec,
void (*move_fn)(struct page *page, struct lruvec *lruvec, void *arg),
void *arg)
{
@@ -235,7 +235,7 @@ static void pagevec_move_tail(struct pagevec *pvec)
{
int pgmoved = 0;
- pagevec_lru_move_fn(pvec, pagevec_move_tail_fn, &pgmoved);
+ pagevec_lruvec_move_fn(pvec, pagevec_move_tail_fn, &pgmoved);
__count_vm_events(PGROTATED, pgmoved);
}
@@ -294,7 +294,7 @@ static void activate_page_drain(int cpu)
struct pagevec *pvec = &per_cpu(activate_page_pvecs, cpu);
if (pagevec_count(pvec))
- pagevec_lru_move_fn(pvec, __activate_page, NULL);
+ pagevec_lruvec_move_fn(pvec, __activate_page, NULL);
}
static bool need_activate_page_drain(int cpu)
@@ -310,7 +310,7 @@ void activate_page(struct page *page)
get_page(page);
if (!pagevec_add(pvec, page) || PageCompound(page))
- pagevec_lru_move_fn(pvec, __activate_page, NULL);
+ pagevec_lruvec_move_fn(pvec, __activate_page, NULL);
put_cpu_var(activate_page_pvecs);
}
}
@@ -620,11 +620,11 @@ void lru_add_drain_cpu(int cpu)
pvec = &per_cpu(lru_deactivate_file_pvecs, cpu);
if (pagevec_count(pvec))
- pagevec_lru_move_fn(pvec, lru_deactivate_file_fn, NULL);
+ pagevec_lruvec_move_fn(pvec, lru_deactivate_file_fn, NULL);
pvec = &per_cpu(lru_lazyfree_pvecs, cpu);
if (pagevec_count(pvec))
- pagevec_lru_move_fn(pvec, lru_lazyfree_fn, NULL);
+ pagevec_lruvec_move_fn(pvec, lru_lazyfree_fn, NULL);
activate_page_drain(cpu);
}
@@ -650,7 +650,8 @@ void deactivate_file_page(struct page *page)
struct pagevec *pvec = &get_cpu_var(lru_deactivate_file_pvecs);
if (!pagevec_add(pvec, page) || PageCompound(page))
- pagevec_lru_move_fn(pvec, lru_deactivate_file_fn, NULL);
+ pagevec_lruvec_move_fn(pvec,
+ lru_deactivate_file_fn, NULL);
put_cpu_var(lru_deactivate_file_pvecs);
}
}
@@ -670,7 +671,7 @@ void mark_page_lazyfree(struct page *page)
get_page(page);
if (!pagevec_add(pvec, page) || PageCompound(page))
- pagevec_lru_move_fn(pvec, lru_lazyfree_fn, NULL);
+ pagevec_lruvec_move_fn(pvec, lru_lazyfree_fn, NULL);
put_cpu_var(lru_lazyfree_pvecs);
}
}
@@ -901,7 +902,7 @@ static void __pagevec_lru_add_fn(struct page *page, struct
lruvec *lruvec,
*/
void __pagevec_lru_add(struct pagevec *pvec)
{
- pagevec_lru_move_fn(pvec, __pagevec_lru_add_fn, NULL);
+ pagevec_lruvec_move_fn(pvec, __pagevec_lru_add_fn, NULL);
}
EXPORT_SYMBOL(__pagevec_lru_add);
--
1.8.5.2
Re: [PATCH 0/12] PM / sleep: Driver flags for system suspend/resume
On 18 October 2017 at 23:48, Rafael J. Wysocki wrote: > On Wednesday, October 18, 2017 9:45:11 PM CEST Grygorii Strashko wrote: >> >> On 10/18/2017 09:11 AM, Ulf Hansson wrote: > > [...] > >> >>> That's the point. We know pm_runtime_force_* works nicely for the >> >>> trivial middle-layer cases. >> >> >> >> In which cases the middle-layer callbacks don't exist, so it's just like >> >> reusing driver callbacks directly. :-) >> >> I'd like to ask you clarify one point here and provide some info which I >> hope can be useful - >> what's exactly means "trivial middle-layer cases"? >> >> Is it when systems use "drivers/base/power/clock_ops.c - Generic clock >> manipulation PM callbacks" as dev_pm_domain (arm davinci/keystone), or OMAP >> device framework struct dev_pm_domain omap_device_pm_domain >> (arm/mach-omap2/omap_device.c) or static const struct dev_pm_ops >> tegra_aconnect_pm_ops? >> >> if yes all above have PM runtime callbacks. > > Trivial ones don't actually do anything meaningful in their PM callbacks. > > Things like the platform bus type, spi bus type, i2c bus type and similar. > > If the middle-layer callbacks manipulate devices in a significant way, then > they aren't trivial. I fully agree with Rafael's description above, but let me also clarify one more thing. We have also been discussing PM domains as being trivial and non-trivial. In some statements I even think the PM domain has been a part the middle-layer terminology, which may have been a bit confusing. In this regards as we consider genpd being a trivial PM domain, those examples your bring up above is too me also examples of trivial PM domains. Especially because they don't deal with wakeups, as that is taken care of by the drivers, right!? Kind regards Uffe
Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name
Hi Frank, > On Oct 19, 2017, at 00:46 , Frank Rowand wrote: > > On 10/18/17 11:30, Rob Herring wrote: >> On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou >> wrote: >>> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote: On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull wrote: > On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand > wrote: >> On 10/17/17 14:46, Rob Herring wrote: >>> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull wrote: On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring wrote: Hi Rob, > With dependencies on a statically allocated full path name converted > to > use %pOF format specifier, we can store just the basename of node, and > the unflattening of the FDT can be simplified. > > This commit will affect the remaining users of full_name. After > analyzing these users, the remaining cases should only change some > print > messages. The main users of full_name are providing a name for struct > resource. The resource names shouldn't be important other than > providing > /proc/iomem names. > > We no longer distinguish between pre and post 0x10 dtb formats as > either > a full path or basename will work. However, less than 0x10 formats > have > been broken since the conversion to use libfdt (and no one has cared). > The conversion of the unflattening code to be non-recursive also broke > pre 0x10 formats as the populate_node function would return 0 in that > case. > > Signed-off-by: Rob Herring > --- > v2: > - rebase to linux-next > > drivers/of/fdt.c | 69 > +--- > 1 file changed, 11 insertions(+), 58 deletions(-) I've just updated to the latest next branch and am finding problems applying overlays. Reverting this commit alleviates things. The errors I get are: [ 88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0 [ 88.513447] OF: overlay: apply failed '/__symbols__' [ 88.518423] create_overlay: Failed to create overlay (err=-12) >>> >>> Frank's series with overlay updates should fix this. >> >> Yes, it does: >> >> [PATCH v3 11/12] of: overlay: remove a dependency on device node >> full_name > > Thanks for the fast response. I fetched the dt/next branch to test > this but there are sufficient changes that Pantelis' "OF: DT-Overlay > configfs interface (v7)" is broken now. I've been adding that > downstream since 4.4. We're using it as an interface for applying > overlays to program FPGAs. If we fix it again, is there any chance > that can go upstream now? With a drive-by posting once every few years, no. >>> >>> I take offense to that. There's nothing changed in the patch for years. >>> Reposting the same patch without changes would achieve nothing. >> >> Are you still expecting review comments on it or something? >> Furthermore, If something is posted infrequently, then I'm not >> inclined to comment or care if the next posting is going to be after I >> forget what I previously said (which is not very long). >> >> I'm just saying, don't expect to forward port, post and it will be >> accepted. Below is minimally one of the issues that needs to be >> addressed. >> > > The issue remains that the kernel is not really setup to deal with any random property or node to be changed at any point in run-time. I think there needs to be some restrictions around what the overlays can touch. We can't have it be wide open and then lock things down later and break users. > > That paragraph is key to any discussion of accepting code to apply overlays. > Solving that issue has been stated to be a gating factor for such code from > the beginning of overlay development. > Overlays are not only used only for cases where you have external expansion boards, or FPGAs where every change is contained under a few designated nodes, so that’s why I’m pushing for a in-kernel validator that’s more flexible than a single whitelist. An eBPF validator would handle a whitelist trivially easy, and would be flexible enough for any other more complicated use case. > One example of what you could do is you can only add sub-trees to whitelisted nodes. That's probably acceptable for your usecase. >>> >>> Defining what can and what cannot be changed is not as trivial as a >>> list of white-listed nodes. >> >> No, but we have to start somewhere and we are not starting with any >> change allowed anywhere at anytime. If that is what people want, then >> they are going to get to maintain that out of tree. >> >>> In some cases there is a whole node hiera
[PATCH] mtd: nand: omap2: Fix subpage write
Since v4.12, NAND subpage writes were causing a NULL pointer
dereference on OMAP platforms (omap2-nand) using OMAP_ECC_BCH4_CODE_HW,
OMAP_ECC_BCH8_CODE_HW and OMAP_ECC_BCH16_CODE_HW.
This is because for those ECC modes, omap_calculate_ecc_bch()
generates ECC bytes for the entire (multi-sector) page and this can
overflow the ECC buffer provided by nand_write_subpage_hwecc()
as it expects ecc.calculate() to return ECC bytes for just one sector.
However, the root cause of the problem is present much before
v4.12 but was not seen then as NAND buffers were being allocated
as one big chunck prior to
commit 3deb9979c731 ("mtd: nand: allocate aligned buffers if NAND_OWN_BUFFERS
is unset")
Fix the issue by providing a OMAP optimized write_subpage() implementation.
cc: # v4.12+
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/omap2.c | 338 +++
1 file changed, 225 insertions(+), 113 deletions(-)
diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
index 54540c8..a0bd456 100644
--- a/drivers/mtd/nand/omap2.c
+++ b/drivers/mtd/nand/omap2.c
@@ -1133,129 +1133,172 @@ static u8 bch8_polynomial[] = {0xef, 0x51, 0x2e,
0x09, 0xed, 0x93, 0x9a, 0xc2,
0x97, 0x79, 0xe5, 0x24, 0xb5};
/**
- * omap_calculate_ecc_bch - Generate bytes of ECC bytes
+ * _omap_calculate_ecc_bch - Generate ECC bytes for one sector
* @mtd: MTD device structure
* @dat: The pointer to data on which ecc is computed
* @ecc_code: The ecc_code buffer
+ * @i: The sector number (for a multi sector page)
*
- * Support calculating of BCH4/8 ecc vectors for the page
+ * Support calculating of BCH4/8/16 ECC vectors for one sector
+ * within a page. Sector number is in @i.
*/
-static int __maybe_unused omap_calculate_ecc_bch(struct mtd_info *mtd,
- const u_char *dat, u_char *ecc_calc)
+static int _omap_calculate_ecc_bch(struct mtd_info *mtd,
+ const u_char *dat, u_char *ecc_calc, int i)
{
struct omap_nand_info *info = mtd_to_omap(mtd);
int eccbytes= info->nand.ecc.bytes;
struct gpmc_nand_regs *gpmc_regs = &info->reg;
u8 *ecc_code;
- unsigned long nsectors, bch_val1, bch_val2, bch_val3, bch_val4;
+ unsigned long bch_val1, bch_val2, bch_val3, bch_val4;
u32 val;
- int i, j;
+ int j;
+
+ ecc_code = ecc_calc;
+ switch (info->ecc_opt) {
+ case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
+ case OMAP_ECC_BCH8_CODE_HW:
+ bch_val1 = readl(gpmc_regs->gpmc_bch_result0[i]);
+ bch_val2 = readl(gpmc_regs->gpmc_bch_result1[i]);
+ bch_val3 = readl(gpmc_regs->gpmc_bch_result2[i]);
+ bch_val4 = readl(gpmc_regs->gpmc_bch_result3[i]);
+ *ecc_code++ = (bch_val4 & 0xFF);
+ *ecc_code++ = ((bch_val3 >> 24) & 0xFF);
+ *ecc_code++ = ((bch_val3 >> 16) & 0xFF);
+ *ecc_code++ = ((bch_val3 >> 8) & 0xFF);
+ *ecc_code++ = (bch_val3 & 0xFF);
+ *ecc_code++ = ((bch_val2 >> 24) & 0xFF);
+ *ecc_code++ = ((bch_val2 >> 16) & 0xFF);
+ *ecc_code++ = ((bch_val2 >> 8) & 0xFF);
+ *ecc_code++ = (bch_val2 & 0xFF);
+ *ecc_code++ = ((bch_val1 >> 24) & 0xFF);
+ *ecc_code++ = ((bch_val1 >> 16) & 0xFF);
+ *ecc_code++ = ((bch_val1 >> 8) & 0xFF);
+ *ecc_code++ = (bch_val1 & 0xFF);
+ break;
+ case OMAP_ECC_BCH4_CODE_HW_DETECTION_SW:
+ case OMAP_ECC_BCH4_CODE_HW:
+ bch_val1 = readl(gpmc_regs->gpmc_bch_result0[i]);
+ bch_val2 = readl(gpmc_regs->gpmc_bch_result1[i]);
+ *ecc_code++ = ((bch_val2 >> 12) & 0xFF);
+ *ecc_code++ = ((bch_val2 >> 4) & 0xFF);
+ *ecc_code++ = ((bch_val2 & 0xF) << 4) |
+ ((bch_val1 >> 28) & 0xF);
+ *ecc_code++ = ((bch_val1 >> 20) & 0xFF);
+ *ecc_code++ = ((bch_val1 >> 12) & 0xFF);
+ *ecc_code++ = ((bch_val1 >> 4) & 0xFF);
+ *ecc_code++ = ((bch_val1 & 0xF) << 4);
+ break;
+ case OMAP_ECC_BCH16_CODE_HW:
+ val = readl(gpmc_regs->gpmc_bch_result6[i]);
+ ecc_code[0] = ((val >> 8) & 0xFF);
+ ecc_code[1] = ((val >> 0) & 0xFF);
+ val = readl(gpmc_regs->gpmc_bch_result5[i]);
+ ecc_code[2] = ((val >> 24) & 0xFF);
+ ecc_code[3] = ((val >> 16) & 0xFF);
+ ecc_code[4] = ((val >> 8) & 0xFF);
+ ecc_code[5] = ((val >> 0) & 0xFF);
+ val = readl(gpmc_regs->gpmc_bch_result4[i]);
+ ecc_code[6] = ((val >> 24) & 0xFF);
+ ecc_code[7] = ((val >> 16) & 0xFF);
+ ecc_code[8] = ((val >> 8) & 0xFF);
+ ecc_code[9] = ((val >> 0)
Re: [PATCH] mmc: sdhci-cadence: use bitfield access macros for cleanup
On 13/10/17 15:50, Masahiro Yamada wrote:
> Accessing register fields generally need mask and shift part.
> Defining them separately, like SDHCI_CDNS_HRS06_TUNE_{SHIFT,MASK},
> is tedious.
>
> Register fields can be always defined by GENMASK (or, BIT if it it
> a single bit). They are nicely handled by FIELD_* macros.
>
> Signed-off-by: Masahiro Yamada
Acked-by: Adrian Hunter
> ---
>
> drivers/mmc/host/sdhci-cadence.c | 28 ++--
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-cadence.c
> b/drivers/mmc/host/sdhci-cadence.c
> index 56529c3..0f589e2 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -13,6 +13,7 @@
> * GNU General Public License for more details.
> */
>
> +#include
> #include
> #include
> #include
> @@ -27,15 +28,14 @@
> #define SDHCI_CDNS_HRS04_ACK BIT(26)
> #define SDHCI_CDNS_HRS04_RDBIT(25)
> #define SDHCI_CDNS_HRS04_WRBIT(24)
> -#define SDHCI_CDNS_HRS04_RDATA_SHIFT 16
> -#define SDHCI_CDNS_HRS04_WDATA_SHIFT 8
> -#define SDHCI_CDNS_HRS04_ADDR_SHIFT0
> +#define SDHCI_CDNS_HRS04_RDATA GENMASK(23, 16)
> +#define SDHCI_CDNS_HRS04_WDATA GENMASK(15, 8)
> +#define SDHCI_CDNS_HRS04_ADDR GENMASK(5, 0)
>
> #define SDHCI_CDNS_HRS06 0x18/* eMMC control */
> #define SDHCI_CDNS_HRS06_TUNE_UP BIT(15)
> -#define SDHCI_CDNS_HRS06_TUNE_SHIFT8
> -#define SDHCI_CDNS_HRS06_TUNE_MASK 0x3f
> -#define SDHCI_CDNS_HRS06_MODE_MASK 0x7
> +#define SDHCI_CDNS_HRS06_TUNE GENMASK(13, 8)
> +#define SDHCI_CDNS_HRS06_MODE GENMASK(2, 0)
> #define SDHCI_CDNS_HRS06_MODE_SD 0x0
> #define SDHCI_CDNS_HRS06_MODE_MMC_SDR 0x2
> #define SDHCI_CDNS_HRS06_MODE_MMC_DDR 0x3
> @@ -105,8 +105,8 @@ static int sdhci_cdns_write_phy_reg(struct
> sdhci_cdns_priv *priv,
> u32 tmp;
> int ret;
>
> - tmp = (data << SDHCI_CDNS_HRS04_WDATA_SHIFT) |
> - (addr << SDHCI_CDNS_HRS04_ADDR_SHIFT);
> + tmp = FIELD_PREP(SDHCI_CDNS_HRS04_WDATA, data) |
> + FIELD_PREP(SDHCI_CDNS_HRS04_ADDR, addr);
> writel(tmp, reg);
>
> tmp |= SDHCI_CDNS_HRS04_WR;
> @@ -189,8 +189,8 @@ static void sdhci_cdns_set_emmc_mode(struct
> sdhci_cdns_priv *priv, u32 mode)
>
> /* The speed mode for eMMC is selected by HRS06 register */
> tmp = readl(priv->hrs_addr + SDHCI_CDNS_HRS06);
> - tmp &= ~SDHCI_CDNS_HRS06_MODE_MASK;
> - tmp |= mode;
> + tmp &= ~SDHCI_CDNS_HRS06_MODE;
> + tmp |= FIELD_PREP(SDHCI_CDNS_HRS06_MODE, mode);
> writel(tmp, priv->hrs_addr + SDHCI_CDNS_HRS06);
> }
>
> @@ -199,7 +199,7 @@ static u32 sdhci_cdns_get_emmc_mode(struct
> sdhci_cdns_priv *priv)
> u32 tmp;
>
> tmp = readl(priv->hrs_addr + SDHCI_CDNS_HRS06);
> - return tmp & SDHCI_CDNS_HRS06_MODE_MASK;
> + return FIELD_GET(SDHCI_CDNS_HRS06_MODE, tmp);
> }
>
> static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
> @@ -254,12 +254,12 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host
> *host, unsigned int val)
> void __iomem *reg = priv->hrs_addr + SDHCI_CDNS_HRS06;
> u32 tmp;
>
> - if (WARN_ON(val > SDHCI_CDNS_HRS06_TUNE_MASK))
> + if (WARN_ON(!FIELD_FIT(SDHCI_CDNS_HRS06_TUNE, val)))
> return -EINVAL;
>
> tmp = readl(reg);
> - tmp &= ~(SDHCI_CDNS_HRS06_TUNE_MASK << SDHCI_CDNS_HRS06_TUNE_SHIFT);
> - tmp |= val << SDHCI_CDNS_HRS06_TUNE_SHIFT;
> + tmp &= ~SDHCI_CDNS_HRS06_TUNE;
> + tmp |= FIELD_PREP(SDHCI_CDNS_HRS06_TUNE, val);
> tmp |= SDHCI_CDNS_HRS06_TUNE_UP;
> writel(tmp, reg);
>
>
Re: [PATCH] USB: add SPDX identifiers to all files in drivers/usb/
Hi Greg,
On Thu, Oct 19, 2017 at 10:38 AM, Greg Kroah-Hartman
wrote:
> It's good to have SPDX identifiers in all files to make it easier to
> audit the kernel tree for correct licenses. This patch adds these
> identifiers to all files in drivers/usb/ based on a script and data from
> Thomas Gleixner, Philippe Ombredanne, and Kate Stewart.
>
> Cc: Thomas Gleixner
> Cc: Kate Stewart
> Cc: Philippe Ombredanne
> Signed-off-by: Greg Kroah-Hartman
> ---
> Unless someone really complains, I'm going to add this to my tree for
> 4.15-rc1.
It would be good to include a diffstat, esp. when touching +600 files.
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 9650b351c26c..cb8d902b801d 100644
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] USB: add SPDX identifiers to all files in drivers/usb/
On Thu, 19 Oct 2017, Greg Kroah-Hartman wrote:
> It's good to have SPDX identifiers in all files to make it easier to
> audit the kernel tree for correct licenses. This patch adds these
> identifiers to all files in drivers/usb/ based on a script and data from
> Thomas Gleixner, Philippe Ombredanne, and Kate Stewart.
>
> Cc: Thomas Gleixner
> Cc: Kate Stewart
> Cc: Philippe Ombredanne
> Signed-off-by: Greg Kroah-Hartman
> ---
> Unless someone really complains, I'm going to add this to my tree for
> 4.15-rc1.
>
>
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 9650b351c26c..cb8d902b801d 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -1,6 +1,7 @@
> #
> # Makefile for the kernel USB device drivers.
> #
> +# SPDX-License-Identifier: GPL-2.0
The last discussion about this was to add the identifier as the first line
of the file or as the second in case of files with a shebang in the first
one.
I think you missed the last version of the script. Attached.
Thanks,
tglx
#!/usr/bin/env python
#
import sys
import os
def insert_at(srclines, pos, tag, style):
srclines.insert(pos, '%s SPDX-License-Identifier: %s\n' %(style, tag))
return True
def handle_c_h(srclines, tag):
return insert_at(srclines, 0, tag, '//')
def handle_asm(srclines, tag):
# Stupid search for a proper style to comment the SPDX tag
pos = 0
style = None
for line in srclines:
pos += 1
if line.startswith(';;;'):
style = ';;;'
elif line.startswith(';;'):
style = ';;'
elif line.startswith(';'):
style = ';'
elif line.startswith('|'):
style = '|'
elif line.startswith('!'):
style = '!'
elif line.startswith('//'):
style = '//'
elif line.startswith("/*"):
style = '//'
else:
continue
return insert_at(srclines, 0, tag, style)
return False
def handle_sh(srclines, tag):
return insert_at(srclines, 1, tag, '#')
tf = open(sys.argv[1])
for entry in tf.readlines():
if len(entry.strip()) == 0:
continue
nr, fname, tag = entry.strip().split(',')
# FIXME: Use a proper encoder
fname = fname.replace('%2C',',')
if tag == 'NOTAG':
print("Skipping %s" %fname)
continue
if not os.path.isfile(fname):
print("FAIL: File %s does not exist anymore" %fname)
continue
srclines = open(fname).readlines()
done = False
for line in srclines:
if line.find('SPDX-License-Identifier') >= 0:
done = True
break
if done:
print("SPDX id exists already in %s" %fname)
continue
ok = False
if fname.endswith('.c') or fname.endswith('.h') or fname.endswith('.uc'):
ok = handle_c_h(srclines, tag)
elif fname.endswith('.S'):
ok = handle_asm(srclines, tag)
elif fname.endswith('.cocci'):
ok = insert_at(srclines, 0, tag, '//')
elif fname.endswith('.dts') or fname.endswith('.dtsi'):
ok = insert_at(srclines, 0, tag, '//')
elif fname.endswith('.py') or fname.endswith('.tc') or fname.endswith('.sh'):
ok = insert_at(srclines, 1, tag, '#')
elif fname.endswith('Makefile') or fname.find('Kconfig') > 0 or fname.find('Kbuild') > 0:
ok = insert_at(srclines, 0, tag, '#')
else:
print("Unhandled or ignored file %s" %fname)
continue
if ok:
open(fname, 'w').writelines(srclines)
print("Inserted %s into %s" %(tag, fname))
else:
print("FAIL: No place for insertion found %s" %fname)
[PATCH] mtd: spi-nor: fix style issue
Fixed a space coding style issue, found via checkpatch.
Signed-off-by: Philipp Puschmann
---
drivers/mtd/spi-nor/spi-nor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 19c000722cbc..386de64e5a86 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -964,7 +964,7 @@ static const struct flash_info spi_nor_ids[] = {
{ "f25l64qa", INFO(0x8c4117, 0, 64 * 1024, 128, SECT_4K |
SPI_NOR_HAS_LOCK) },
/* Everspin */
- { "mr25h256", CAT25_INFO( 32 * 1024, 1, 256, 2, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) },
+ { "mr25h256", CAT25_INFO(32 * 1024, 1, 256, 2, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) },
{ "mr25h10", CAT25_INFO(128 * 1024, 1, 256, 3, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) },
{ "mr25h40", CAT25_INFO(512 * 1024, 1, 256, 3, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) },
--
2.14.1
Re: [PATCH] MAINTAINERS: Split Cavium EDAC entry and add myself
On Thu, Oct 19, 2017 at 08:41:29AM +0200, Jan Glauber wrote: > Split the Cavium EDAC entry into MIPS and ARM drivers because > they have different maintainers and mailing lists. > > Add myself as additional maintainer to the ThunderX driver. > > Signed-off-by: Jan Glauber > --- > MAINTAINERS | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) Applied, thanks. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH v2 5/5] of/fdt: only store the device node basename in full_name
Hi Rob, > On Oct 18, 2017, at 21:30 , Rob Herring wrote: > > On Wed, Oct 18, 2017 at 10:53 AM, Pantelis Antoniou > wrote: >> On Wed, 2017-10-18 at 10:44 -0500, Rob Herring wrote: >>> On Wed, Oct 18, 2017 at 10:12 AM, Alan Tull wrote: On Tue, Oct 17, 2017 at 6:51 PM, Frank Rowand wrote: > On 10/17/17 14:46, Rob Herring wrote: >> On Tue, Oct 17, 2017 at 4:32 PM, Alan Tull wrote: >>> On Mon, Aug 21, 2017 at 10:16 AM, Rob Herring wrote: >>> >>> Hi Rob, >>> With dependencies on a statically allocated full path name converted to use %pOF format specifier, we can store just the basename of node, and the unflattening of the FDT can be simplified. This commit will affect the remaining users of full_name. After analyzing these users, the remaining cases should only change some print messages. The main users of full_name are providing a name for struct resource. The resource names shouldn't be important other than providing /proc/iomem names. We no longer distinguish between pre and post 0x10 dtb formats as either a full path or basename will work. However, less than 0x10 formats have been broken since the conversion to use libfdt (and no one has cared). The conversion of the unflattening code to be non-recursive also broke pre 0x10 formats as the populate_node function would return 0 in that case. Signed-off-by: Rob Herring --- v2: - rebase to linux-next drivers/of/fdt.c | 69 +--- 1 file changed, 11 insertions(+), 58 deletions(-) >>> >>> I've just updated to the latest next branch and am finding problems >>> applying overlays. Reverting this commit alleviates things. The >>> errors I get are: >>> >>> [ 88.498704] OF: overlay: Failed to apply prop @/__symbols__/clk_0 >>> [ 88.513447] OF: overlay: apply failed '/__symbols__' >>> [ 88.518423] create_overlay: Failed to create overlay (err=-12) >> >> Frank's series with overlay updates should fix this. > > Yes, it does: > > [PATCH v3 11/12] of: overlay: remove a dependency on device node > full_name Thanks for the fast response. I fetched the dt/next branch to test this but there are sufficient changes that Pantelis' "OF: DT-Overlay configfs interface (v7)" is broken now. I've been adding that downstream since 4.4. We're using it as an interface for applying overlays to program FPGAs. If we fix it again, is there any chance that can go upstream now? >>> >>> With a drive-by posting once every few years, no. >>> >> >> I take offense to that. There's nothing changed in the patch for years. >> Reposting the same patch without changes would achieve nothing. > > Are you still expecting review comments on it or something? > Furthermore, If something is posted infrequently, then I'm not > inclined to comment or care if the next posting is going to be after I > forget what I previously said (which is not very long). > > I'm just saying, don't expect to forward port, post and it will be > accepted. Below is minimally one of the issues that needs to be > addressed. > >>> The issue remains that the kernel is not really setup to deal with any >>> random property or node to be changed at any point in run-time. I >>> think there needs to be some restrictions around what the overlays can >>> touch. We can't have it be wide open and then lock things down later >>> and break users. One example of what you could do is you can only add >>> sub-trees to whitelisted nodes. That's probably acceptable for your >>> usecase. >>> >> >> Defining what can and what cannot be changed is not as trivial as a >> list of white-listed nodes. > > No, but we have to start somewhere and we are not starting with any > change allowed anywhere at anytime. If that is what people want, then > they are going to get to maintain that out of tree. > I am still not sold on this ‘dangerous’ idea. No-one is crazy enough to suggest overlays to be loadable by an unprivileged user. It’s exactly the same ‘danger’ as loading a kernel module, while is sure capable of much greater mischief. >> In some cases there is a whole node hierarchy being inserted (like in >> a FPGA). > > Yes, so you'd have a target fpga region. That sounds fine to me. Maybe > its not a static whitelist, but drivers have to register target > nodes/paths. > >> In others, it's merely changing a status property to "okay" and >> a few device parameters. > > That seems fine too. Disabled nodes could be allowed. But what if you > add/change properties on a node that is not disabled? Once a node is > enabled, who is responsible for registering the device? > > What about changing a
Re: [PATCH] net: l2tp: mark expected switch fall-through
On Tue, Oct 17, 2017 at 05:42:53PM -0500, Gustavo A. R. Silva wrote: > This code was tested by compilation only (GCC 7.2.0 was used). > No worry, this case really has to fall through. > diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c > index 7135f46..f517942 100644 > --- a/net/l2tp/l2tp_netlink.c > +++ b/net/l2tp/l2tp_netlink.c > @@ -406,7 +406,7 @@ static int l2tp_nl_tunnel_send(struct sk_buff *skb, u32 > portid, u32 seq, int fla > if (nla_put_u16(skb, L2TP_ATTR_UDP_SPORT, > ntohs(inet->inet_sport)) || > nla_put_u16(skb, L2TP_ATTR_UDP_DPORT, > ntohs(inet->inet_dport))) > goto nla_put_failure; > - /* NOBREAK */ > + /* fall through */ > case L2TP_ENCAPTYPE_IP: > Nit: extra whitespace before '*/' (just couldn't restrain myself from noticing). Acked-by: Guillaume Nault
Re: [PATCH] USB: add SPDX identifiers to all files in drivers/usb/
On Thu, Oct 19, 2017 at 10:49:47AM +0200, Geert Uytterhoeven wrote: > Hi Greg, > > On Thu, Oct 19, 2017 at 10:38 AM, Greg Kroah-Hartman > wrote: > > It's good to have SPDX identifiers in all files to make it easier to > > audit the kernel tree for correct licenses. This patch adds these > > identifiers to all files in drivers/usb/ based on a script and data from > > Thomas Gleixner, Philippe Ombredanne, and Kate Stewart. > > > > Cc: Thomas Gleixner > > Cc: Kate Stewart > > Cc: Philippe Ombredanne > > Signed-off-by: Greg Kroah-Hartman > > --- > > Unless someone really complains, I'm going to add this to my tree for > > 4.15-rc1. > > It would be good to include a diffstat, esp. when touching +600 files. I wanted to, but I was worried it would be too long to prevent the patch from hitting the list :) Here it is: drivers/usb/Makefile |1 + drivers/usb/atm/Makefile |1 + drivers/usb/atm/cxacru.c |1 + drivers/usb/atm/speedtch.c |1 + drivers/usb/atm/ueagle-atm.c |2 ++ drivers/usb/atm/usbatm.c |1 + drivers/usb/atm/usbatm.h |1 + drivers/usb/atm/xusbatm.c |1 + drivers/usb/c67x00/c67x00-drv.c|2 ++ drivers/usb/c67x00/c67x00-hcd.c|2 ++ drivers/usb/c67x00/c67x00-hcd.h|2 ++ drivers/usb/c67x00/c67x00-ll-hpi.c |2 ++ drivers/usb/c67x00/c67x00-sched.c |2 ++ drivers/usb/c67x00/c67x00.h|2 ++ drivers/usb/chipidea/Makefile |1 + drivers/usb/chipidea/bits.h|2 ++ drivers/usb/chipidea/ci.h |2 ++ drivers/usb/chipidea/ci_hdrc_imx.c |2 ++ drivers/usb/chipidea/ci_hdrc_imx.h |2 ++ drivers/usb/chipidea/ci_hdrc_msm.c |2 ++ drivers/usb/chipidea/ci_hdrc_pci.c |2 ++ drivers/usb/chipidea/ci_hdrc_usb2.c|2 ++ drivers/usb/chipidea/ci_hdrc_zevio.c |2 ++ drivers/usb/chipidea/core.c|2 ++ drivers/usb/chipidea/debug.c |1 + drivers/usb/chipidea/host.c|2 ++ drivers/usb/chipidea/host.h|1 + drivers/usb/chipidea/otg.c |2 ++ drivers/usb/chipidea/otg.h |2 ++ drivers/usb/chipidea/otg_fsm.c |2 ++ drivers/usb/chipidea/otg_fsm.h |2 ++ drivers/usb/chipidea/udc.c |2 ++ drivers/usb/chipidea/udc.h |2 ++ drivers/usb/chipidea/ulpi.c|2 ++ drivers/usb/chipidea/usbmisc_imx.c |2 ++ drivers/usb/class/cdc-acm.c|2 ++ drivers/usb/class/cdc-acm.h|2 ++ drivers/usb/class/cdc-wdm.c|2 ++ drivers/usb/class/usblp.c |2 ++ drivers/usb/class/usbtmc.c |2 ++ drivers/usb/common/Makefile|1 + drivers/usb/common/common.c|2 ++ drivers/usb/common/led.c |2 ++ drivers/usb/common/ulpi.c |2 ++ drivers/usb/common/usb-otg-fsm.c |2 ++ drivers/usb/core/Makefile |1 + drivers/usb/core/devices.c |2 ++ drivers/usb/core/devio.c |6 +- drivers/usb/core/hcd-pci.c |2 ++ drivers/usb/core/hcd.c |2 ++ drivers/usb/core/hub.h |2 ++ drivers/usb/core/ledtrig-usbport.c |2 ++ drivers/usb/core/of.c |2 ++ drivers/usb/core/otg_whitelist.h |2 ++ drivers/usb/core/port.c|2 ++ drivers/usb/core/quirks.c |2 ++ drivers/usb/core/usb-acpi.c|2 ++ drivers/usb/dwc2/Makefile |1 + drivers/usb/dwc2/core.c|2 ++ drivers/usb/dwc2/core.h|2 ++ drivers/usb/dwc2/core_intr.c |2 ++ drivers/usb/dwc2/debug.h |2 ++ drivers/usb/dwc2/debugfs.c |2 ++ drivers/usb/dwc2/gadget.c |2 ++ drivers/usb/dwc2/hcd.c |2 ++ drivers/usb/dwc2/hcd.h |2 ++ drivers/usb/dwc2/hcd_ddma.c|2 ++ drivers/usb/dwc2/hcd_intr.c|2 ++ drivers/usb/dwc2/hcd_queue.c |2 ++ drivers/usb/dwc2/hw.h |2 ++ drivers/usb/dwc2/params.c |2 ++ drivers/usb/dwc2/pci.c |2 ++
Re: [PATCH] wireless: iwlegacy: Convert timers to use timer_setup()
On Mon, Oct 16, 2017 at 04:37:44PM -0700, Kees Cook wrote: > In preparation for unconditionally passing the struct timer_list pointer to > all timer callbacks, switch to using the new timer_setup() and from_timer() > to pass the timer pointer explicitly. > > Cc: Stanislaw Gruszka > Cc: Kalle Valo > Cc: linux-wirel...@vger.kernel.org > Cc: net...@vger.kernel.org > Signed-off-by: Kees Cook Acked-by: Stanislaw Gruszka
Re: [alsa-devel] [PATCH 03/14] soundwire: Add Master registration
On Thu, 19 Oct 2017 05:03:19 +0200,
Vinod Koul wrote:
>
> +/**
> + * sdw_add_bus_master: add a bus Master instance
> + *
> + * @bus: bus instance
> + *
> + * Initializes the bus instance, read properties and create child
> + * devices.
> + */
> +int sdw_add_bus_master(struct sdw_bus *bus)
> +{
> + int ret;
> +
> + if (!bus->dev) {
> + pr_err("SoundWire bus has no device");
> + return -ENODEV;
> + }
> +
> + mutex_init(&bus->bus_lock);
> + INIT_LIST_HEAD(&bus->slaves);
> +
> + /*
> + * SDW is an enumerable bus, but devices can be powered off. So,
> + * they won't be able to report as present.
> + *
> + * Create Slave devices based on Slaves described in
> + * the respective firmware (ACPI/DT)
> + */
> +
> + if (IS_ENABLED(CONFIG_ACPI) && bus->dev && ACPI_HANDLE(bus->dev))
> + ret = sdw_acpi_find_slaves(bus);
> + else if (IS_ENABLED(CONFIG_OF) && bus->dev && bus->dev->of_node)
The bus->dev NULL check is already done at the beginning of the
function, so here are superfluous.
> +static int sdw_delete_slave(struct device *dev, void *data)
> +{
> + struct sdw_slave *slave = dev_to_sdw_dev(dev);
> + struct sdw_bus *bus = slave->bus;
> +
> + mutex_lock(&bus->bus_lock);
> + if (!list_empty(&bus->slaves))
> + list_del(&slave->node);
You can perform list_del_init() without empty check.
> +void sdw_extract_slave_id(struct sdw_bus *bus,
> + unsigned long long addr, struct sdw_slave_id *id)
Use u64 instead.
> +{
> + dev_dbg(bus->dev, "SDW Slave Addr: %llx", addr);
> +
> + /*
> + * Spec definition
> + * Register Bit Contents
> + * DevId_0 [7:4] 47:44 sdw_version
> + * DevId_0 [3:0] 43:40 unique_id
> + * DevId_139:32 mfg_id [15:8]
> + * DevId_231:24 mfg_id [7:0]
> + * DevId_323:16 part_id [15:8]
> + * DevId_415:08 part_id [7:0]
> + * DevId_507:00 class_id
> + */
> + id->sdw_version = (addr >> 44) & GENMASK(3, 0);
> + id->unique_id = (addr >> 40) & GENMASK(3, 0);
> + id->mfg_id = (addr >> 24) & GENMASK(15, 0);
> + id->part_id = (addr >> 8) & GENMASK(15, 0);
> + id->class_id = addr & GENMASK(7, 0);
> +
> + dev_info(bus->dev,
> + "SDW Slave class_id %x, part_id %x, mfg_id %x, unique_id %x,
> version %x",
> + id->class_id, id->part_id, id->mfg_id,
> + id->unique_id, id->sdw_version);
> +
Do we want to print a message always at each invocation?
> +static int sdw_slave_add(struct sdw_bus *bus,
> + struct sdw_slave_id *id, struct fwnode_handle *fwnode)
> +{
> + struct sdw_slave *slave;
> + char name[32];
> + int ret;
> +
> + slave = kzalloc(sizeof(*slave), GFP_KERNEL);
> + if (!slave)
> + return -ENOMEM;
> +
> + /* Initialize data structure */
> + memcpy(&slave->id, id, sizeof(*id));
> +
> + /* name shall be sdw:link:mfg:part:class:unique */
> + snprintf(name, sizeof(name), "sdw:%x:%x:%x:%x:%x",
> + bus->link_id, id->mfg_id, id->part_id,
> + id->class_id, id->unique_id);
You can set the name directly via dev_set_name(). It's printf format,
after all.
> + slave->dev.parent = bus->dev;
> + slave->dev.fwnode = fwnode;
> + dev_set_name(&slave->dev, "%s", name);
> + slave->dev.release = sdw_slave_release;
> + slave->dev.bus = &sdw_bus_type;
> + slave->bus = bus;
> + slave->status = SDW_SLAVE_UNATTACHED;
> + slave->dev_num = 0;
> +
> + mutex_lock(&bus->bus_lock);
> + list_add_tail(&slave->node, &bus->slaves);
> + mutex_unlock(&bus->bus_lock);
> +
> + ret = device_register(&slave->dev);
> + if (ret) {
> + dev_err(bus->dev, "Failed to add slave: ret %d\n", ret);
> +
> + /*
> + * On err, don't free but drop ref as this will be freed
> + * when release method is invoked.
> + */
> + put_device(&slave->dev);
Wouldn't it leave a stale link to bus?
thanks,
Takashi
[PATCH] mtd: spi-nor: Add support for mr25h128
Add Everspin mr25h128 16KB MRAM to the list of supported chips.
Signed-off-by: Philipp Puschmann
---
Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 +
drivers/mtd/devices/m25p80.c| 1 +
drivers/mtd/spi-nor/spi-nor.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
index 9ce35af8507c..956bb046e599 100644
--- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
+++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
@@ -13,6 +13,7 @@ Required properties:
at25df321a
at25df641
at26df081a
+ mr25h128
mr25h256
mr25h10
mr25h40
diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 00eea6fd379c..dbe6a1de2bb8 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -359,6 +359,7 @@ static const struct spi_device_id m25p_ids[] = {
{"m25p32-nonjedec"},{"m25p64-nonjedec"},{"m25p128-nonjedec"},
/* Everspin MRAMs (non-JEDEC) */
+ { "mr25h128" }, /* 128 Kib, 40 MHz */
{ "mr25h256" }, /* 256 Kib, 40 MHz */
{ "mr25h10" }, /* 1 Mib, 40 MHz */
{ "mr25h40" }, /* 4 Mib, 40 MHz */
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 386de64e5a86..a835bb890166 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -964,6 +964,7 @@ static const struct flash_info spi_nor_ids[] = {
{ "f25l64qa", INFO(0x8c4117, 0, 64 * 1024, 128, SECT_4K |
SPI_NOR_HAS_LOCK) },
/* Everspin */
+ { "mr25h128", CAT25_INFO(16 * 1024, 1, 256, 2, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) },
{ "mr25h256", CAT25_INFO(32 * 1024, 1, 256, 2, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) },
{ "mr25h10", CAT25_INFO(128 * 1024, 1, 256, 3, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) },
{ "mr25h40", CAT25_INFO(512 * 1024, 1, 256, 3, SPI_NOR_NO_ERASE |
SPI_NOR_NO_FR) },
--
2.15.0.rc1
Re: [PATCH V2 2/3] pseries/findnodes: Find nodes with memory for memoryless nodes
Hi Michael,
Michael Bringmann writes:
> pseries/findnodes: On pseries systems which allow 'hot-add' of
This isn't a powerpc or pseries patch, so the subject/prefix is wrong.
Also because you're changing generic code you need to provide an
explanation that makes sense in general, across all architectures, not
just in terms of what the pseries platform does.
> resources, we may boot configurations that have CPUs, but no memory
> associated to a node by the affinity calculations.
This is called a "memory-less node" and is understood by the generic
code.
> Previously, the
> software took a shortcut to collapse initialization and references
What software? What shortcut?
> to such memoryless nodes with other nodes that did have memory
> associated with them at boot. This patch is based on fixes that
What fixes?
> allow the proper initialization and distinguishment of memoryless
> and memory-plus nodes after NUMA initialization.
What exactly is unproper about the current code?
> It extends the
> use of the 'node_to_mem_node()' API from 'topology.h' to modules
The term "modules" has a specific meaning in Linux which is not correct
here. We would just say "in two functions" or "in two files".
> that are allocating node-specific memory at boot, and allows such
> references to find available memory in another node.
> diff --git a/block/blk-mq-cpumap.c b/block/blk-mq-cpumap.c
> index 9f8cffc..a27a31f 100644
> --- a/block/blk-mq-cpumap.c
> +++ b/block/blk-mq-cpumap.c
> @@ -73,7 +73,8 @@ int blk_mq_hw_queue_to_node(unsigned int *mq_map, unsigned
> int index)
>
> for_each_possible_cpu(i) {
> if (index == mq_map[i])
> - return local_memory_node(cpu_to_node(i));
> + return local_memory_node(
> + node_to_mem_node(cpu_to_node(i)));
What is this trying to do?
local_memory_node() is supposed to return a "local" node for nodes with
no memory.
And in fact the comment says:
* Used for initializing percpu 'numa_mem'
Which is what we do:
set_numa_mem(local_memory_node(numa_cpu_lookup_table[cpu]));
And is what's returned by node_to_mem_node():
static inline void set_numa_mem(int node)
{
this_cpu_write(_numa_mem_, node);
_node_numa_mem_[numa_node_id()] = node;
}
static inline int node_to_mem_node(int node)
{
return _node_numa_mem_[node];
}
So your change effectively ends up doing:
return local_memory_node(local_memory_node(cpu_to_node(i)));
Which doesn't look right.
cheers
Re: [PATCH] USB: add SPDX identifiers to all files in drivers/usb/
On Thu, Oct 19, 2017 at 10:50:44AM +0200, Thomas Gleixner wrote: > On Thu, 19 Oct 2017, Greg Kroah-Hartman wrote: > > > It's good to have SPDX identifiers in all files to make it easier to > > audit the kernel tree for correct licenses. This patch adds these > > identifiers to all files in drivers/usb/ based on a script and data from > > Thomas Gleixner, Philippe Ombredanne, and Kate Stewart. > > > > Cc: Thomas Gleixner > > Cc: Kate Stewart > > Cc: Philippe Ombredanne > > Signed-off-by: Greg Kroah-Hartman > > --- > > Unless someone really complains, I'm going to add this to my tree for > > 4.15-rc1. > > > > > > diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile > > index 9650b351c26c..cb8d902b801d 100644 > > --- a/drivers/usb/Makefile > > +++ b/drivers/usb/Makefile > > @@ -1,6 +1,7 @@ > > # > > # Makefile for the kernel USB device drivers. > > # > > +# SPDX-License-Identifier: GPL-2.0 > > The last discussion about this was to add the identifier as the first line > of the file or as the second in case of files with a shebang in the first > one. > > I think you missed the last version of the script. Attached. Oh, I did, thanks, let me run this again...
Re: [PATCH] EDAC, sb_edac: fix missing break in switch
On Mon, Oct 16, 2017 at 12:40:29PM -0500, Gustavo A. R. Silva wrote: > Add missing break statement in order to prevent the code from falling > through. > > Signed-off-by: Gustavo A. R. Silva > --- > drivers/edac/sb_edac.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c > index 72b98a0..f34430f 100644 > --- a/drivers/edac/sb_edac.c > +++ b/drivers/edac/sb_edac.c > @@ -2485,6 +2485,7 @@ static int ibridge_mci_bind_devs(struct mem_ctl_info > *mci, > case PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_TA: > case PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA1_TA: > pvt->pci_ta = pdev; > + break; > case PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA0_RAS: > case PCI_DEVICE_ID_INTEL_IBRIDGE_IMC_HA1_RAS: > pvt->pci_ras = pdev; > -- Applied, thanks. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH] USB: add SPDX identifiers to all files in drivers/usb/
On Thu, Oct 19, 2017 at 10:50:44AM +0200, Thomas Gleixner wrote: > On Thu, 19 Oct 2017, Greg Kroah-Hartman wrote: > > > It's good to have SPDX identifiers in all files to make it easier to > > audit the kernel tree for correct licenses. This patch adds these > > identifiers to all files in drivers/usb/ based on a script and data from > > Thomas Gleixner, Philippe Ombredanne, and Kate Stewart. > > > > Cc: Thomas Gleixner > > Cc: Kate Stewart > > Cc: Philippe Ombredanne > > Signed-off-by: Greg Kroah-Hartman > > --- > > Unless someone really complains, I'm going to add this to my tree for > > 4.15-rc1. > > > > > > diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile > > index 9650b351c26c..cb8d902b801d 100644 > > --- a/drivers/usb/Makefile > > +++ b/drivers/usb/Makefile > > @@ -1,6 +1,7 @@ > > # > > # Makefile for the kernel USB device drivers. > > # > > +# SPDX-License-Identifier: GPL-2.0 > > The last discussion about this was to add the identifier as the first line > of the file or as the second in case of files with a shebang in the first > one. > > I think you missed the last version of the script. Attached. Ugh, that's ugly, creating stuff like this: --- a/drivers/usb/atm/speedtch.c +++ b/drivers/usb/atm/speedtch.c @@ -1,3 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0+ /** * speedtch.c - Alcatel SpeedTouch USB xDSL modem driver * diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c Are we really ok with the '//' comments? thanks, greg k-h
Re: [PATCH] USB: add SPDX identifiers to all files in drivers/usb/
On Thu, 19 Oct 2017, Greg Kroah-Hartman wrote: > On Thu, Oct 19, 2017 at 10:50:44AM +0200, Thomas Gleixner wrote: > > The last discussion about this was to add the identifier as the first line > > of the file or as the second in case of files with a shebang in the first > > one. > > > > I think you missed the last version of the script. Attached. > > Ugh, that's ugly, creating stuff like this: > > --- a/drivers/usb/atm/speedtch.c > +++ b/drivers/usb/atm/speedtch.c > @@ -1,3 +1,4 @@ > +// SPDX-License-Identifier: GPL-2.0+ > > /** > * speedtch.c - Alcatel SpeedTouch USB xDSL modem driver > * > diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c > > > Are we really ok with the '//' comments? That's what Linus suggested so it stands out. Thanks, tglx
Re: [alsa-devel] [PATCH 04/14] soundwire: Add MIPI DisCo property helpers
On Thu, 19 Oct 2017 05:03:20 +0200,
Vinod Koul wrote:
>
> diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
> index a14d1de80afa..baad4ad3be44 100644
> --- a/drivers/soundwire/bus_type.c
> +++ b/drivers/soundwire/bus_type.c
> @@ -139,12 +139,28 @@ static int sdw_drv_probe(struct device *dev)
> return ret;
> }
>
> + slave->ops = drv->ops;
> +
> ret = drv->probe(slave, id);
> if (ret) {
> dev_err(dev, "Probe of %s failed: %d\n", drv->name, ret);
> return ret;
> }
>
> + /* device is probed so let's read the properties now */
> + if (slave->ops && slave->ops->read_prop)
> + slave->ops->read_prop(slave);
> +
> + /*
> + * Check for valid clk_stop_timeout, use DisCo worst case value of
> + * 300ms
> + */
> + if (slave->prop.clk_stop_timeout == 0)
> + slave->prop.clk_stop_timeout = 300;
> +
> + slave->bus->clk_stop_timeout = max_t(u32, slave->bus->clk_stop_timeout,
> + slave->prop.clk_stop_timeout);
Isn't it racy?
Also what happens after removing a driver? The clk_stop_timeout is
kept high?
> +
> +int sdw_slave_read_dpn(struct sdw_slave *slave,
> + struct sdw_dpn_prop *dpn, int count, int ports, char *type)
Missing comment for a public API function.
thanks,
Takashi
Re: [PATCH v2 2/4] rtc: mediatek: add driver for RTC on MT7622 SoC
On 19/10/2017 at 10:55:49 +0800, Sean Wang wrote: > Hi, both > > On Wed, 2017-10-18 at 14:57 +0200, Alexandre Belloni wrote: > > On 18/10/2017 at 19:12:06 +0800, Yingjoe Chen wrote: > > > On Tue, 2017-10-17 at 17:40 +0800, sean.w...@mediatek.com wrote: > > > > From: Sean Wang > > > > > > > > This patch introduces the driver for the RTC on MT7622 SoC. > > > > > > > > Signed-off-by: Sean Wang > > > > --- > > > > drivers/rtc/Kconfig | 10 ++ > > > > drivers/rtc/Makefile | 1 + > > > > drivers/rtc/rtc-mt7622.c | 418 > > > > +++ > > > > 3 files changed, 429 insertions(+) > > > > create mode 100644 drivers/rtc/rtc-mt7622.c > > > > > > > > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig > > > > index e0e58f3..4226295 100644 > > > > --- a/drivers/rtc/Kconfig > > > > +++ b/drivers/rtc/Kconfig > > > > @@ -1705,6 +1705,16 @@ config RTC_DRV_MOXART > > > >This driver can also be built as a module. If so, the module > > > >will be called rtc-moxart > > > > > > > > +config RTC_DRV_MEDIATEK > > > > > > How about changing this to RTC_DRV_MT7622 or RTC_DRV_MEDIATEK_SOC? > > > It is confusing to have both RTC_DRV_MEDIATEK & RTC_DRV_MT6397 here. > > > > > > > Yes, this has to be RTC_DRV_MT7622. It doesn't matter if it support > > future SoCs named differently, it will be less confusing than using > > anything with only mediatek in it. > > > > Agreed on. RTC_DRV_MT7622 will be applied instead to align the usage on > MT6397 and to get rid of such kind of confusion. > > > > > > + return -EINVAL; > > > > + > > > > + /* Keep yr_base used to calculate the calculate year when > > > > userspace > > > > +* queries and extend the maximum year the RTC can count. > > > > +*/ > > > > + hw->yr_base[MTK_TC] = tm->tm_year - MTK_RTC_TM_YR_L - > > > > + (tm->tm_year % MTK_RTC_HW_YR_LIMIT); > > > > > > > > > I'm not sure this worth it. > > > If maximum year it can hold is 99, I'd bet it won't support leap year > > > correctly after 2100. This make the RTC useless after that. > > > > > > Also, yr_base is lost after power cycle, so you can't get correct year > > > back anyway. > > > > > > > I agree, the best you can do here is to only support 2000 to 2099. > > > > O.K. I will remove those yr_base extension and only consider only > support from 2000 to 2099 because of no much gain we can get from > yr_base. > > The only gain is yr_base I thought just allows people have the > opportunity to set up rtc after 2100. However, it appears to not much > practical to foresee these things after 2100 and rtc must be setup again > when either year overflowing or power cycle happens after 2100 as Joe.C > mentioned. > > In addition, I also found the rtc hardware would take year == 0 as not > leap year that works for 2100, 2200, 2300, but not for 2000, 2400, > 2800,... and thus 2000 is also needed to be excluded in both set_time > and set_alarm if only 2000 to 2099 is supported. > So you can make it work from 2001 to 2100 but I'm not sure it is worth it. -- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
RE: [PATCH 1/2] lockdep: Introduce CROSSRELEASE_STACK_TRACE and make it not unwind as default
> -Original Message- > From: Ingo Molnar [mailto:mingo.kernel@gmail.com] On Behalf Of Ingo > > > At the time, cross-release was falsely accused. AFAIK, cross-release has > > not crashed system yet. > > I'm talking about the crash fixed here: > > 8b405d5c5d09: locking/lockdep: Fix stacktrace mess > > Which was introduced by your patch: > > ce07a9415f26: locking/lockdep: Make check_prev_add() able to handle > external stack_trace > > ... which was a preparatory patch for cross-release. So 'technically' it's > not a > cross-release crash, but was very much related. It even says so in the > changelog: > > Actually crossrelease needs to do other than saving a stack_trace. > So pass a stack_trace and callback to handle it, to check_prev_add(). > > ... so let's not pretend it wasn't related, ok? I don't want to pretend I'm perfect. Of course, I can make mistakes. I'm just saying that *I have not seen* any crash by cross-release. In that case you pointed out, likewise, the crash was caused by ae813308f: lockdep: Avoid creating redundant links, which is not related to the feature actually. It was also falsely accused at the time again... Of course, it's my fault not to have made the design more robust so that others can modify lockdep code caring less after cross-release commit. That's what I'm sorry for. I already mentioned the above in the thread talking about the issue you are pointing now. Of course, I basically appreciate all comments and suggestions you have given, but you seem to have mis-understood some issues wrt cross-release feature. Thanks, Byungchul
Re: [PATCH 7/7] gpio: brcmstb: implement suspend/resume/shutdown
Hi Doug,
Nice description of the problem with dealing with a pending disabled
wake interrupt and the solution. A few remarks:
On Fri, Sep 29, 2017 at 08:40:57PM -0700, Doug Berger wrote:
> diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
> index 752a46ce3589..c964ed71a68d 100644
> --- a/drivers/gpio/gpio-brcmstb.c
> +++ b/drivers/gpio/gpio-brcmstb.c
> @@ -19,17 +19,29 @@
> #include
> #include
> #include
> -#include
> -
> -#define GIO_BANK_SIZE 0x20
> -#define GIO_ODEN(bank) (((bank) * GIO_BANK_SIZE) + 0x00)
> -#define GIO_DATA(bank) (((bank) * GIO_BANK_SIZE) + 0x04)
> -#define GIO_IODIR(bank) (((bank) * GIO_BANK_SIZE) + 0x08)
> -#define GIO_EC(bank)(((bank) * GIO_BANK_SIZE) + 0x0c)
> -#define GIO_EI(bank)(((bank) * GIO_BANK_SIZE) + 0x10)
> -#define GIO_MASK(bank) (((bank) * GIO_BANK_SIZE) + 0x14)
> -#define GIO_LEVEL(bank) (((bank) * GIO_BANK_SIZE) + 0x18)
> -#define GIO_STAT(bank) (((bank) * GIO_BANK_SIZE) + 0x1c)
> +
> +enum gio_reg_index {
> + GIO_REG_ODEN = 0,
> + GIO_REG_DATA,
> + GIO_REG_IODIR,
> + GIO_REG_EC,
> + GIO_REG_EI,
> + GIO_REG_MASK,
> + GIO_REG_LEVEL,
> + GIO_REG_STAT,
> + GIO_REG_COUNT
Please change the name or add a comment to make it clear that this is
not for an actual register.
> +};
> +
> [snip]
> @@ -37,6 +49,8 @@ struct brcmstb_gpio_bank {
> struct gpio_chip gc;
> struct brcmstb_gpio_priv *parent_priv;
> u32 width;
> + u32 wake_active;
> + u32 regs[GIO_REG_STAT]; /* Don't save and restore GIO_REG_STAT */
Consider naming to make it clearer that this is for save/restore, not
some kind of shadow reg implementation or anything like that.
> };
>
> struct brcmstb_gpio_priv {
> @@ -49,7 +63,6 @@ struct brcmstb_gpio_priv {
> int gpio_base;
> int num_gpios;
> int parent_wake_irq;
> - struct notifier_block reboot_notifier;
> };
>
> #define MAX_GPIO_PER_BANK 32
> @@ -65,15 +78,22 @@ brcmstb_gpio_gc_to_priv(struct gpio_chip *gc)
> }
>
> static unsigned long
> -brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
> +__brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
> {
> void __iomem *reg_base = bank->parent_priv->reg_base;
> +
> + return bank->gc.read_reg(reg_base + GIO_STAT(bank->id)) &
> +bank->gc.read_reg(reg_base + GIO_MASK(bank->id));
> +}
> +
> +static unsigned long
> +brcmstb_gpio_get_active_irqs(struct brcmstb_gpio_bank *bank)
> +{
> unsigned long status;
> unsigned long flags;
>
> spin_lock_irqsave(&bank->gc.bgpio_lock, flags);
> - status = bank->gc.read_reg(reg_base + GIO_STAT(bank->id)) &
> - bank->gc.read_reg(reg_base + GIO_MASK(bank->id));
> + status = __brcmstb_gpio_get_active_irqs(bank);
> spin_unlock_irqrestore(&bank->gc.bgpio_lock, flags);
>
> return status;
> @@ -209,11 +229,6 @@ static int brcmstb_gpio_priv_set_wake(struct
> brcmstb_gpio_priv *priv,
> {
> int ret = 0;
>
> - /*
> - * Only enable wake IRQ once for however many hwirqs can wake
> - * since they all use the same wake IRQ. Mask will be set
> - * up appropriately thanks to IRQCHIP_MASK_ON_SUSPEND flag.
> - */
> if (enable)
> ret = enable_irq_wake(priv->parent_wake_irq);
> else
> @@ -227,7 +242,17 @@ static int brcmstb_gpio_priv_set_wake(struct
> brcmstb_gpio_priv *priv,
> static int brcmstb_gpio_irq_set_wake(struct irq_data *d, unsigned int enable)
> {
> struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> - struct brcmstb_gpio_priv *priv = brcmstb_gpio_gc_to_priv(gc);
> + struct brcmstb_gpio_bank *bank = gpiochip_get_data(gc);
> + struct brcmstb_gpio_priv *priv = bank->parent_priv;
> + u32 mask = BIT(brcmstb_gpio_hwirq_to_offset(d->hwirq, bank));
> +
> + /* Do not do anything specific for now, suspend/resume callbacks will
> + * configure the interrupt mask appropriately
> + */
Please fix comment format (this is the network subsystem style).
>
> [snip]
> @@ -508,6 +511,99 @@ static int brcmstb_gpio_irq_setup(struct platform_device
> *pdev,
> [...]
> +static void brcmstb_gpio_shutdown(struct platform_device *pdev)
> +{
> + /* Enable GPIO for S5 cold boot */
> + brcmstb_gpio_quiesce(&pdev->dev, 0);
nit: use false instead of 0 (it's a bool).
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> [snip]
> +static int brcmstb_gpio_resume(struct device *dev)
> +{
> + struct brcmstb_gpio_priv *priv = dev_get_drvdata(dev);
> + struct brcmstb_gpio_bank *bank;
> + u32 wake_mask = 0;
This isn't really being used as a mask, contrary to appearances. It's
just tracking whether any active IRQs were seen. Please change to use a
bool instead and adjust the name accordingly.
> +
> + list_for_each_entry(bank, &priv->bank_list, node) {
> + wake_mask |= __brcmstb_gpio_get_active_irqs(bank
[PATCH v4 2/2] crypto: stm32 - Support for STM32 CRYP crypto module
This module registers block cipher algorithms that make use of the
STMicroelectronics STM32 crypto "CRYP1" hardware.
The following algorithms are supported:
- aes: ecb, cbc, ctr
- des: ecb, cbc
- tdes: ecb, cbc
Signed-off-by: Fabien Dessenne
---
drivers/crypto/stm32/Kconfig |9 +
drivers/crypto/stm32/Makefile |3 +-
drivers/crypto/stm32/stm32-cryp.c | 1188 +
3 files changed, 1199 insertions(+), 1 deletion(-)
create mode 100644 drivers/crypto/stm32/stm32-cryp.c
diff --git a/drivers/crypto/stm32/Kconfig b/drivers/crypto/stm32/Kconfig
index 602332e..61ef00b 100644
--- a/drivers/crypto/stm32/Kconfig
+++ b/drivers/crypto/stm32/Kconfig
@@ -18,3 +18,12 @@ config HASH_DEV_STM32
help
This enables support for the HASH hw accelerator which can be found
on STMicroelectronics STM32 SOC.
+
+config CRYP_DEV_STM32
+ tristate "Support for STM32 cryp accelerators"
+ depends on ARCH_STM32
+ select CRYPTO_HASH
+ select CRYPTO_ENGINE
+ help
+ This enables support for the CRYP (AES/DES/TDES) hw accelerator which
+ can be found on STMicroelectronics STM32 SOC.
diff --git a/drivers/crypto/stm32/Makefile b/drivers/crypto/stm32/Makefile
index 73cd56c..2c19fc1 100644
--- a/drivers/crypto/stm32/Makefile
+++ b/drivers/crypto/stm32/Makefile
@@ -1,2 +1,3 @@
obj-$(CONFIG_CRC_DEV_STM32) += stm32_crc32.o
-obj-$(CONFIG_HASH_DEV_STM32) += stm32-hash.o
\ No newline at end of file
+obj-$(CONFIG_HASH_DEV_STM32) += stm32-hash.o
+obj-$(CONFIG_CRYP_DEV_STM32) += stm32-cryp.o
diff --git a/drivers/crypto/stm32/stm32-cryp.c
b/drivers/crypto/stm32/stm32-cryp.c
new file mode 100644
index 000..ede2995
--- /dev/null
+++ b/drivers/crypto/stm32/stm32-cryp.c
@@ -0,0 +1,1188 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2017
+ * Author: Fabien Dessenne
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME "stm32-cryp"
+
+/* Bit [0] encrypt / decrypt */
+#define FLG_ENCRYPT BIT(0)
+/* Bit [8..1] algo & operation mode */
+#define FLG_AES BIT(1)
+#define FLG_DES BIT(2)
+#define FLG_TDESBIT(3)
+#define FLG_ECB BIT(4)
+#define FLG_CBC BIT(5)
+#define FLG_CTR BIT(6)
+/* Mode mask = bits [15..0] */
+#define FLG_MODE_MASK GENMASK(15, 0)
+
+/* Registers */
+#define CRYP_CR 0x
+#define CRYP_SR 0x0004
+#define CRYP_DIN0x0008
+#define CRYP_DOUT 0x000C
+#define CRYP_DMACR 0x0010
+#define CRYP_IMSCR 0x0014
+#define CRYP_RISR 0x0018
+#define CRYP_MISR 0x001C
+#define CRYP_K0LR 0x0020
+#define CRYP_K0RR 0x0024
+#define CRYP_K1LR 0x0028
+#define CRYP_K1RR 0x002C
+#define CRYP_K2LR 0x0030
+#define CRYP_K2RR 0x0034
+#define CRYP_K3LR 0x0038
+#define CRYP_K3RR 0x003C
+#define CRYP_IV0LR 0x0040
+#define CRYP_IV0RR 0x0044
+#define CRYP_IV1LR 0x0048
+#define CRYP_IV1RR 0x004C
+
+/* Registers values */
+#define CR_DEC_NOT_ENC 0x0004
+#define CR_TDES_ECB 0x
+#define CR_TDES_CBC 0x0008
+#define CR_DES_ECB 0x0010
+#define CR_DES_CBC 0x0018
+#define CR_AES_ECB 0x0020
+#define CR_AES_CBC 0x0028
+#define CR_AES_CTR 0x0030
+#define CR_AES_KP 0x0038
+#define CR_AES_UNKNOWN 0x
+#define CR_ALGO_MASK0x00080038
+#define CR_DATA32 0x
+#define CR_DATA16 0x0040
+#define CR_DATA80x0080
+#define CR_DATA10x00C0
+#define CR_KEY128 0x
+#define CR_KEY192 0x0100
+#define CR_KEY256 0x0200
+#define CR_FFLUSH 0x4000
+#define CR_CRYPEN 0x8000
+
+#define SR_BUSY 0x0010
+#define SR_OFNE 0x0004
+
+#define IMSCR_INBIT(0)
+#define IMSCR_OUT BIT(1)
+
+#define MISR_IN BIT(0)
+#define MISR_OUTBIT(1)
+
+/* Misc */
+#define AES_BLOCK_32(AES_BLOCK_SIZE / sizeof(u32))
+#define _walked_in (cryp->in_walk.offset - cryp->in_sg->offset)
+#define _walked_out (cryp->out_walk.offset - cryp->out_sg->offset)
+
+struct stm32_cryp_ctx {
+ struct stm32_cryp *cryp;
+ int keylen;
+ u32 key[AES_KEYSIZE_
