[PATCH v12 02/10] perf/amd/iommu: Clean up bitwise operations

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit 

Clean up register initializaton and make use of BIT_ULL(x)
where appropriate. This should not affect logic and functionality.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 8d8ed40..e112f49 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -164,11 +164,11 @@ static int get_next_avail_iommu_bnk_cntr(struct 
perf_amd_iommu *perf_iommu)
for (bank = 0, shift = 0; bank < max_banks; bank++) {
for (cntr = 0; cntr < max_cntrs; cntr++) {
shift = bank + (bank*3) + cntr;
-   if (perf_iommu->cntr_assign_mask & (1ULLcntr_assign_mask |= (1ULL

[PATCH v12 00/10] perf/amd/iommu: Enable multi-IOMMU support

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit 

This patch series modifies the existing IOMMU and Perf drivers to support
systems with multiple IOMMUs by allocating an amd_iommu PMU per IOMMU instance.
This allows users to specify performance events and filters separately for each
IOMMU.

This has been tested on the new family17h-based server w/ multiple IOMMUs.

Git branch containing this patch series is available here:

https://github.com/ssuthiku/linux.git  perf-iommu-v12

Changes from V11 (https://lkml.org/lkml/2017/2/24/114)
  * Cleaning up in patch 9 and 10 per Boris' suggestions.

Changes from V10 (https://lkml.org/lkml/2017/2/15/1202)
  * Introduce amd_iommu specific struct in struct hw_perf_event
(patch 9/10).
  * Minor clean up on pr_info (patch 1/10 per Joe Perches).

Changes from V9 (https://lkml.org/lkml/2017/2/7/105)
  * Since each IOMMU PMU in a system functions independently from 
one another, the amd_iommu_pc_init() function has been modified
to allow certain IOMMU PMU initialization to fail w/o disabling
the others (patch 9/9).
  * Define a static pmu (iommu_pmu), and stuct assignment directly.
(per Boris)
  * Refactor code clean up into a separate patch (4/9) (per Boris)

Thanks,
Suravee

Suravee Suthikulpanit (10):
  perf/amd/iommu: Declare pr_fmt and remove unnecessary pr_debug
  perf/amd/iommu: Clean up bitwise operations
  perf/amd/iommu: Clean up perf_iommu_read()
  iommu/amd: Clean up iommu_pc_get_set_reg()
  iommu/amd: Introduce amd_iommu_get_num_iommus()
  perf/amd/iommu: Modify functions to query max banks and counters
  perf/amd/iommu: Modify amd_iommu_pc_get_set_reg_val() to allow
specifying IOMMU
  perf/amd/iommu: Fix sysfs perf attribute groups
  perf/amd/iommu: Introduce amd_iommu-specific struct in struct
hw_perf_event
  perf/amd/iommu: Enable support for multiple IOMMUs

 arch/x86/events/amd/iommu.c | 321 +++-
 arch/x86/events/amd/iommu.h |  18 ++-
 drivers/iommu/amd_iommu.c   |   6 +-
 drivers/iommu/amd_iommu_init.c  | 101 -
 drivers/iommu/amd_iommu_proto.h |   8 +-
 drivers/iommu/amd_iommu_types.h |   3 -
 include/linux/perf_event.h  |   7 +
 7 files changed, 237 insertions(+), 227 deletions(-)

-- 
1.8.3.1

[PATCH v12 01/10] perf/amd/iommu: Declare pr_fmt and remove unnecessary pr_debug

[Suravee Suthikulpanit]

Declare pr_fmt for perf/amd_iommu and remove unnecessary pr_debug.

Also check return value when _init_events_attrs fails.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index b28200d..8d8ed40 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -11,6 +11,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt)"perf/amd_iommu: " fmt
+
 #include 
 #include 
 #include 
@@ -298,7 +300,6 @@ static void perf_iommu_start(struct perf_event *event, int 
flags)
 {
struct hw_perf_event *hwc = &event->hw;
 
-   pr_debug("perf: amd_iommu:perf_iommu_start\n");
if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
return;
 
@@ -323,7 +324,6 @@ static void perf_iommu_read(struct perf_event *event)
u64 prev_raw_count = 0ULL;
u64 delta = 0ULL;
struct hw_perf_event *hwc = &event->hw;
-   pr_debug("perf: amd_iommu:perf_iommu_read\n");
 
amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
_GET_BANK(event), _GET_CNTR(event),
@@ -349,8 +349,6 @@ static void perf_iommu_stop(struct perf_event *event, int 
flags)
struct hw_perf_event *hwc = &event->hw;
u64 config;
 
-   pr_debug("perf: amd_iommu:perf_iommu_stop\n");
-
if (hwc->state & PERF_HES_UPTODATE)
return;
 
@@ -372,7 +370,6 @@ static int perf_iommu_add(struct perf_event *event, int 
flags)
struct perf_amd_iommu *perf_iommu =
container_of(event->pmu, struct perf_amd_iommu, pmu);
 
-   pr_debug("perf: amd_iommu:perf_iommu_add\n");
event->hw.state = PERF_HES_UPTODATE | PERF_HES_STOPPED;
 
/* request an iommu bank/counter */
@@ -393,7 +390,6 @@ static void perf_iommu_del(struct perf_event *event, int 
flags)
struct perf_amd_iommu *perf_iommu =
container_of(event->pmu, struct perf_amd_iommu, pmu);
 
-   pr_debug("perf: amd_iommu:perf_iommu_del\n");
perf_iommu_stop(event, PERF_EF_UPDATE);
 
/* clear the assigned iommu bank/counter */
@@ -444,27 +440,27 @@ static __init int _init_perf_amd_iommu(
 
raw_spin_lock_init(&perf_iommu->lock);
 
-   /* Init format attributes */
perf_iommu->format_group = &amd_iommu_format_group;
 
/* Init cpumask attributes to only core 0 */
cpumask_set_cpu(0, &iommu_cpumask);
perf_iommu->cpumask_group = &amd_iommu_cpumask_group;
 
-   /* Init events attributes */
-   if (_init_events_attrs(perf_iommu) != 0)
-   pr_err("perf: amd_iommu: Only support raw events.\n");
+   ret = _init_events_attrs(perf_iommu);
+   if (ret) {
+   pr_err("Error initializing AMD IOMMU perf events.\n");
+   return ret;
+   }
 
-   /* Init null attributes */
perf_iommu->null_group = NULL;
perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;
 
ret = perf_pmu_register(&perf_iommu->pmu, name, -1);
if (ret) {
-   pr_err("perf: amd_iommu: Failed to initialized.\n");
+   pr_err("Error initializing AMD IOMMU perf counters.\n");
amd_iommu_pc_exit();
} else {
-   pr_info("perf: amd_iommu: Detected. (%d banks, %d 
counters/bank)\n",
+   pr_info("Detected AMD IOMMU (%d banks, %d counters/bank).\n",
amd_iommu_pc_get_max_banks(IOMMU_BASE_DEVID),
amd_iommu_pc_get_max_counters(IOMMU_BASE_DEVID));
}
-- 
1.8.3.1

[PATCH v12 04/10] iommu/amd: Clean up iommu_pc_get_set_reg()

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit 

Clean up coding style and fix a bug in the 64-bit register read
logic since it overwrites the upper 32-bit when reading the lower 32-bit.

Signed-off-by: Suravee Suthikulpanit 
---
 drivers/iommu/amd_iommu_init.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 6130278..ce65a47 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2763,22 +2763,25 @@ static int iommu_pc_get_set_reg_val(struct amd_iommu 
*iommu,
if (WARN_ON((fxn > 0x28) || (fxn & 7)))
return -ENODEV;
 
-   offset = (u32)(((0x40|bank) << 12) | (cntr << 8) | fxn);
+   offset = (u32)(((0x40 | bank) << 12) | (cntr << 8) | fxn);
 
/* Limit the offset to the hw defined mmio region aperture */
-   max_offset_lim = (u32)(((0x40|iommu->max_banks) << 12) |
+   max_offset_lim = (u32)(((0x40 | iommu->max_banks) << 12) |
(iommu->max_counters << 8) | 0x28);
if ((offset < MMIO_CNTR_REG_OFFSET) ||
(offset > max_offset_lim))
return -EINVAL;
 
if (is_write) {
-   writel((u32)*value, iommu->mmio_base + offset);
-   writel((*value >> 32), iommu->mmio_base + offset + 4);
+   u64 val = *value & GENMASK_ULL(47, 0);
+
+   writel((u32)val, iommu->mmio_base + offset);
+   writel((val >> 32), iommu->mmio_base + offset + 4);
} else {
*value = readl(iommu->mmio_base + offset + 4);
*value <<= 32;
-   *value = readl(iommu->mmio_base + offset);
+   *value |= readl(iommu->mmio_base + offset);
+   *value &= GENMASK_ULL(47, 0);
}
 
return 0;
-- 
1.8.3.1

[PATCH v12 05/10] iommu/amd: Introduce amd_iommu_get_num_iommus()

[Suravee Suthikulpanit]

Introduce amd_iommu_get_num_iommus(), which returns the value of
amd_iommus_present. The function is used to replace direct access to
the variable, which is now declared as static.

This function will also be used by Perf AMD IOMMU driver.

Cc: Borislav Petkov 
Cc: Joerg Roedel 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.h |  2 ++
 drivers/iommu/amd_iommu.c   |  6 +++---
 drivers/iommu/amd_iommu_init.c  | 11 +--
 drivers/iommu/amd_iommu_proto.h |  1 +
 drivers/iommu/amd_iommu_types.h |  3 ---
 5 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
index 845d173..5c5c932 100644
--- a/arch/x86/events/amd/iommu.h
+++ b/arch/x86/events/amd/iommu.h
@@ -28,6 +28,8 @@
 #define IOMMU_BASE_DEVID   0x
 
 /* amd_iommu_init.c external support functions */
+extern int amd_iommu_get_num_iommus(void);
+
 extern bool amd_iommu_pc_supported(void);
 
 extern u8 amd_iommu_pc_get_max_banks(u16 devid);
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 98940d1..5d4dd94 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1234,7 +1234,7 @@ static void __domain_flush_pages(struct protection_domain 
*domain,
 
build_inv_iommu_pages(&cmd, address, size, domain->id, pde);
 
-   for (i = 0; i < amd_iommus_present; ++i) {
+   for (i = 0; i < amd_iommu_get_num_iommus(); ++i) {
if (!domain->dev_iommu[i])
continue;
 
@@ -1278,7 +1278,7 @@ static void domain_flush_complete(struct 
protection_domain *domain)
 {
int i;
 
-   for (i = 0; i < amd_iommus_present; ++i) {
+   for (i = 0; i < amd_iommu_get_num_iommus(); ++i) {
if (domain && !domain->dev_iommu[i])
continue;
 
@@ -3363,7 +3363,7 @@ static int __flush_pasid(struct protection_domain 
*domain, int pasid,
 * IOMMU TLB needs to be flushed before Device TLB to
 * prevent device TLB refill from IOMMU TLB
 */
-   for (i = 0; i < amd_iommus_present; ++i) {
+   for (i = 0; i < amd_iommu_get_num_iommus(); ++i) {
if (domain->dev_iommu[i] == 0)
continue;
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index ce65a47..d3ec9c3 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -167,7 +167,9 @@ struct ivmd_header {
 
 /* Array to assign indices to IOMMUs*/
 struct amd_iommu *amd_iommus[MAX_IOMMUS];
-int amd_iommus_present;
+
+/* Number of IOMMUs present in the system */
+static int amd_iommus_present;
 
 /* IOMMUs have a non-present cache? */
 bool amd_iommu_np_cache __read_mostly;
@@ -272,6 +274,11 @@ static inline unsigned long tbl_size(int entry_size)
return 1UL << shift;
 }
 
+int amd_iommu_get_num_iommus(void)
+{
+   return amd_iommus_present;
+}
+
 /* Access to l1 and l2 indexed register spaces */
 
 static u32 iommu_read_l1(struct amd_iommu *iommu, u16 l1, u8 address)
@@ -1336,7 +1343,7 @@ static int __init init_iommu_one(struct amd_iommu *iommu, 
struct ivhd_header *h)
 
/* Add IOMMU to internal data structures */
list_add_tail(&iommu->list, &amd_iommu_list);
-   iommu->index = amd_iommus_present++;
+   iommu->index = amd_iommus_present++;
 
if (unlikely(iommu->index >= MAX_IOMMUS)) {
WARN(1, "AMD-Vi: System has more IOMMUs than supported by this 
driver\n");
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 7eb60c1..e8f0710 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -21,6 +21,7 @@
 
 #include "amd_iommu_types.h"
 
+extern int amd_iommu_get_num_iommus(void);
 extern int amd_iommu_init_dma_ops(void);
 extern int amd_iommu_init_passthrough(void);
 extern irqreturn_t amd_iommu_int_thread(int irq, void *data);
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 003f3ce..4de8f41 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -611,9 +611,6 @@ struct devid_map {
  */
 extern struct amd_iommu *amd_iommus[MAX_IOMMUS];
 
-/* Number of IOMMUs present in the system */
-extern int amd_iommus_present;
-
 /*
  * Declarations for the global list of all protection domains
  */
-- 
1.8.3.1

[PATCH v12 08/10] perf/amd/iommu: Fix sysfs perf attribute groups

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit 

Introduce static amd_iommu_attr_groups to simplify the
sysfs attributes initialization code.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 81 ++---
 1 file changed, 32 insertions(+), 49 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 88fbc80..7ac8138 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -43,14 +43,8 @@ struct perf_amd_iommu {
u8 max_counters;
u64 cntr_assign_mask;
raw_spinlock_t lock;
-   const struct attribute_group *attr_groups[4];
 };
 
-#define format_group   attr_groups[0]
-#define cpumask_group  attr_groups[1]
-#define events_group   attr_groups[2]
-#define null_group attr_groups[3]
-
 /*-
  * sysfs format attributes
  *-*/
@@ -81,6 +75,10 @@ struct perf_amd_iommu {
 /*-
  * sysfs events attributes
  *-*/
+static struct attribute_group amd_iommu_events_group = {
+   .name = "events",
+};
+
 struct amd_iommu_event_desc {
struct kobj_attribute attr;
const char *event;
@@ -384,76 +382,60 @@ static void perf_iommu_del(struct perf_event *event, int 
flags)
perf_event_update_userpage(event);
 }
 
-static __init int _init_events_attrs(struct perf_amd_iommu *perf_iommu)
+static __init int _init_events_attrs(void)
 {
-   struct attribute **attrs;
-   struct attribute_group *attr_group;
int i = 0, j;
+   struct attribute **attrs;
 
while (amd_iommu_v2_event_descs[i].attr.attr.name)
i++;
 
-   attr_group = kzalloc(sizeof(struct attribute *)
-   * (i + 1) + sizeof(*attr_group), GFP_KERNEL);
-   if (!attr_group)
+   attrs = kzalloc(sizeof(struct attribute **) * (i + 1), GFP_KERNEL);
+   if (!attrs)
return -ENOMEM;
 
-   attrs = (struct attribute **)(attr_group + 1);
for (j = 0; j < i; j++)
attrs[j] = &amd_iommu_v2_event_descs[j].attr.attr;
 
-   attr_group->name = "events";
-   attr_group->attrs = attrs;
-   perf_iommu->events_group = attr_group;
-
+   amd_iommu_events_group.attrs = attrs;
return 0;
 }
 
 static __init void amd_iommu_pc_exit(void)
 {
-   if (__perf_iommu.events_group != NULL) {
-   kfree(__perf_iommu.events_group);
-   __perf_iommu.events_group = NULL;
-   }
+   kfree(amd_iommu_events_group.attrs);
 }
 
-static __init int _init_perf_amd_iommu(
-   struct perf_amd_iommu *perf_iommu, char *name)
+const struct attribute_group *amd_iommu_attr_groups[] = {
+   &amd_iommu_format_group,
+   &amd_iommu_cpumask_group,
+   &amd_iommu_events_group,
+   NULL,
+};
+
+static __init int
+_init_perf_amd_iommu(struct perf_amd_iommu *perf_iommu, char *name)
 {
int ret;
 
raw_spin_lock_init(&perf_iommu->lock);
 
-   perf_iommu->format_group = &amd_iommu_format_group;
-
/* Init cpumask attributes to only core 0 */
cpumask_set_cpu(0, &iommu_cpumask);
-   perf_iommu->cpumask_group = &amd_iommu_cpumask_group;
-
-   ret = _init_events_attrs(perf_iommu);
-   if (ret) {
-   pr_err("Error initializing AMD IOMMU perf events.\n");
-   return ret;
-   }
 
perf_iommu->max_banks= amd_iommu_pc_get_max_banks(0);
perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
if (!perf_iommu->max_banks || !perf_iommu->max_counters)
return -EINVAL;
 
-   perf_iommu->null_group = NULL;
-   perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;
-
+   perf_iommu->pmu.attr_groups = amd_iommu_attr_groups;
ret = perf_pmu_register(&perf_iommu->pmu, name, -1);
-   if (ret) {
+   if (ret)
pr_err("Error initializing AMD IOMMU perf counters.\n");
-   amd_iommu_pc_exit();
-   } else {
+   else
pr_info("Detected AMD IOMMU (%d banks, %d counters/bank).\n",
amd_iommu_pc_get_max_banks(0),
amd_iommu_pc_get_max_counters(0));
-   }
-
return ret;
 }
 
@@ -467,24 +449,25 @@ static __init int _init_perf_amd_iommu(
.stop   = perf_iommu_stop,
.read   = perf_iommu_read,
},
-   .max_banks  = 0x00,
-   .max_counters   = 0x00,
-   .cntr_assign_mask   = 0ULL,
-   .format_group   = NULL,
-   .cpumask_group  = NULL,
-   .events_group   = NULL,
-   .null_group = NULL,
 };
 
 static __init int amd_iommu_pc_init(void)
 {
+   int ret;
+
/* Make sure the IOMMU PC resource is available */
if (!amd_iommu_pc_suppor

[PATCH v12 07/10] perf/amd/iommu: Modify amd_iommu_pc_get_set_reg_val() to allow specifying IOMMU

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit 

The current amd_iommu_pc_get_set_reg_val() cannot support multiple IOMMUs.
So, modify it to allow callers to specify IOMMU. This prepares the driver
for supporting multi-IOMMU in subsequent patch.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Cc: Joerg Roedel 

Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 38 
 arch/x86/events/amd/iommu.h |  9 +++--
 drivers/iommu/amd_iommu_init.c  | 43 +++--
 drivers/iommu/amd_iommu_proto.h |  5 -
 4 files changed, 48 insertions(+), 47 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 10f67d3..88fbc80 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -248,49 +248,45 @@ static int perf_iommu_event_init(struct perf_event *event)
 
 static void perf_iommu_enable_event(struct perf_event *ev)
 {
+   struct amd_iommu *iommu = get_amd_iommu(0);
u8 csource = _GET_CSOURCE(ev);
u16 devid = _GET_DEVID(ev);
+   u8 bank = _GET_BANK(ev);
+   u8 cntr = _GET_CNTR(ev);
u64 reg = 0ULL;
 
reg = csource;
-   amd_iommu_pc_get_set_reg_val(devid,
-   _GET_BANK(ev), _GET_CNTR(ev) ,
-IOMMU_PC_COUNTER_SRC_REG, ®, true);
+   amd_iommu_pc_set_reg(iommu, bank, cntr, IOMMU_PC_COUNTER_SRC_REG, ®);
 
reg = devid | (_GET_DEVID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_get_set_reg_val(devid,
-   _GET_BANK(ev), _GET_CNTR(ev) ,
-IOMMU_PC_DEVID_MATCH_REG, ®, true);
+   amd_iommu_pc_set_reg(iommu, bank, cntr, IOMMU_PC_DEVID_MATCH_REG, ®);
 
reg = _GET_PASID(ev) | (_GET_PASID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_get_set_reg_val(devid,
-   _GET_BANK(ev), _GET_CNTR(ev) ,
-IOMMU_PC_PASID_MATCH_REG, ®, true);
+   amd_iommu_pc_set_reg(iommu, bank, cntr, IOMMU_PC_PASID_MATCH_REG, ®);
 
reg = _GET_DOMID(ev) | (_GET_DOMID_MASK(ev) << 32);
if (reg)
reg |= BIT(31);
-   amd_iommu_pc_get_set_reg_val(devid,
-   _GET_BANK(ev), _GET_CNTR(ev) ,
-IOMMU_PC_DOMID_MATCH_REG, ®, true);
+   amd_iommu_pc_set_reg(iommu, bank, cntr, IOMMU_PC_DOMID_MATCH_REG, ®);
 }
 
 static void perf_iommu_disable_event(struct perf_event *event)
 {
+   struct amd_iommu *iommu = get_amd_iommu(0);
u64 reg = 0ULL;
 
-   amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-   _GET_BANK(event), _GET_CNTR(event),
-   IOMMU_PC_COUNTER_SRC_REG, ®, true);
+   amd_iommu_pc_set_reg(iommu, _GET_BANK(event), _GET_CNTR(event),
+IOMMU_PC_COUNTER_SRC_REG, ®);
 }
 
 static void perf_iommu_start(struct perf_event *event, int flags)
 {
struct hw_perf_event *hwc = &event->hw;
+   struct amd_iommu *iommu = get_amd_iommu(0);
 
if (WARN_ON_ONCE(!(hwc->state & PERF_HES_STOPPED)))
return;
@@ -300,9 +296,8 @@ static void perf_iommu_start(struct perf_event *event, int 
flags)
 
if (flags & PERF_EF_RELOAD) {
u64 prev_raw_count =  local64_read(&hwc->prev_count);
-   amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-   _GET_BANK(event), _GET_CNTR(event),
-   IOMMU_PC_COUNTER_REG, &prev_raw_count, true);
+   amd_iommu_pc_set_reg(iommu, _GET_BANK(event), _GET_CNTR(event),
+IOMMU_PC_COUNTER_REG, &prev_raw_count);
}
 
perf_iommu_enable_event(event);
@@ -314,10 +309,11 @@ static void perf_iommu_read(struct perf_event *event)
 {
u64 count, prev, delta;
struct hw_perf_event *hwc = &event->hw;
+   struct amd_iommu *iommu = get_amd_iommu(0);
 
-   amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
-   _GET_BANK(event), _GET_CNTR(event),
-   IOMMU_PC_COUNTER_REG, &count, false);
+   if (amd_iommu_pc_get_reg(iommu, _GET_BANK(event), _GET_CNTR(event),
+IOMMU_PC_COUNTER_REG, &count))
+   return;
 
/* IOMMU pc counter register is only 48 bits */
count &= GENMASK_ULL(47, 0);
diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
index b775107..62e0702 100644
--- a/arch/x86/events/amd/iommu.h
+++ b/arch/x86/events/amd/iommu.h
@@ -24,6 +24,8 @@
 #define PC_MAX_SPEC_BNKS   64
 #define PC_MAX_SPEC_CNTRS  16
 
+struct amd_iommu;
+
 /* amd_iommu_init.c external support functions */
 extern int amd_iommu_get_num_iommus(void);
 
@@ -33,8 +35,11 @@
 
 extern u8 amd_iommu_pc_get_max_counters(unsigned int idx);
 
-extern int amd_iommu_pc_get_set_reg_val(u16

[PATCH v12 03/10] perf/amd/iommu: Clean up perf_iommu_read()

[Suravee Suthikulpanit]

Fix coding style and make use of GENMASK_ULL macro.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Cc: Joerg Roedel 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index e112f49..d4375da 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -320,9 +320,7 @@ static void perf_iommu_start(struct perf_event *event, int 
flags)
 
 static void perf_iommu_read(struct perf_event *event)
 {
-   u64 count = 0ULL;
-   u64 prev_raw_count = 0ULL;
-   u64 delta = 0ULL;
+   u64 count, prev, delta;
struct hw_perf_event *hwc = &event->hw;
 
amd_iommu_pc_get_set_reg_val(_GET_DEVID(event),
@@ -330,18 +328,16 @@ static void perf_iommu_read(struct perf_event *event)
IOMMU_PC_COUNTER_REG, &count, false);
 
/* IOMMU pc counter register is only 48 bits */
-   count &= 0xULL;
+   count &= GENMASK_ULL(47, 0);
 
-   prev_raw_count =  local64_read(&hwc->prev_count);
-   if (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
-   count) != prev_raw_count)
+   prev = local64_read(&hwc->prev_count);
+   if (local64_cmpxchg(&hwc->prev_count, prev, count) != prev)
return;
 
-   /* Handling 48-bit counter overflowing */
-   delta = (count << COUNTER_SHIFT) - (prev_raw_count << COUNTER_SHIFT);
+   /* Handle 48-bit counter overflow */
+   delta = (count << COUNTER_SHIFT) - (prev << COUNTER_SHIFT);
delta >>= COUNTER_SHIFT;
local64_add(delta, &event->count);
-
 }
 
 static void perf_iommu_stop(struct perf_event *event, int flags)
-- 
1.8.3.1

[PATCH v12 09/10] perf/amd/iommu: Introduce amd_iommu-specific struct in struct hw_perf_event

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit 

Current AMD IOMMU Perf PMU inappropriately uses hardware struct
inside the union inside the struct hw_perf_event, mainly the use of
extra_reg.

Instead, introduce amd_iommu-specific struct with required
parameters to be programmed onto the IOMMU performance counter
control register.

Also update the pasid field from 16 to 20 bits.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 102 +++-
 include/linux/perf_event.h  |   7 +++
 2 files changed, 52 insertions(+), 57 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 7ac8138..915a20c 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -23,17 +23,16 @@
 
 #define COUNTER_SHIFT  16
 
-#define _GET_BANK(ev)   ((u8)(ev->hw.extra_reg.reg >> 8))
-#define _GET_CNTR(ev)   ((u8)(ev->hw.extra_reg.reg))
-
-/* iommu pmu config masks */
-#define _GET_CSOURCE(ev)((ev->hw.config & 0xFFULL))
-#define _GET_DEVID(ev)  ((ev->hw.config >> 8)  & 0xULL)
-#define _GET_PASID(ev)  ((ev->hw.config >> 24) & 0xULL)
-#define _GET_DOMID(ev)  ((ev->hw.config >> 40) & 0xULL)
-#define _GET_DEVID_MASK(ev) ((ev->hw.extra_reg.config)  & 0xULL)
-#define _GET_PASID_MASK(ev) ((ev->hw.extra_reg.config >> 16) & 0xULL)
-#define _GET_DOMID_MASK(ev) ((ev->hw.extra_reg.config >> 32) & 0xULL)
+/* iommu pmu conf masks */
+#define GET_CSOURCE(x) (x->conf & 0xFFULL)
+#define GET_DEVID(x)   ((x->conf >> 8)  & 0xULL)
+#define GET_DOMID(x)   ((x->conf >> 24) & 0xULL)
+#define GET_PASID(x)   ((x->conf >> 40) & 0xFULL)
+
+/* iommu pmu conf1 masks */
+#define GET_DEVID_MASK(x)  (x->conf1  & 0xULL)
+#define GET_DOMID_MASK(x)  ((x->conf1 >> 16) & 0xULL)
+#define GET_PASID_MASK(x)  ((x->conf1 >> 32) & 0xFULL)
 
 static struct perf_amd_iommu __perf_iommu;
 
@@ -50,11 +49,11 @@ struct perf_amd_iommu {
  *-*/
 PMU_FORMAT_ATTR(csource,"config:0-7");
 PMU_FORMAT_ATTR(devid,  "config:8-23");
-PMU_FORMAT_ATTR(pasid,  "config:24-39");
-PMU_FORMAT_ATTR(domid,  "config:40-55");
+PMU_FORMAT_ATTR(domid,  "config:24-39");
+PMU_FORMAT_ATTR(pasid,  "config:40-59");
 PMU_FORMAT_ATTR(devid_mask, "config1:0-15");
-PMU_FORMAT_ATTR(pasid_mask, "config1:16-31");
-PMU_FORMAT_ATTR(domid_mask, "config1:32-47");
+PMU_FORMAT_ATTR(domid_mask, "config1:16-31");
+PMU_FORMAT_ATTR(pasid_mask, "config1:32-51");
 
 static struct attribute *iommu_format_attrs[] = {
&format_attr_csource.attr,
@@ -150,10 +149,13 @@ static ssize_t _iommu_cpumask_show(struct device *dev,
 
 /*-*/
 
-static int get_next_avail_iommu_bnk_cntr(struct perf_amd_iommu *perf_iommu)
+static int get_next_avail_iommu_bnk_cntr(struct perf_event *event)
 {
unsigned long flags;
-   int shift, bank, cntr, retval;
+   u32 shift, bank, cntr;
+   int retval;
+   struct perf_amd_iommu *perf_iommu =
+   container_of(event->pmu, struct perf_amd_iommu, pmu);
int max_banks = perf_iommu->max_banks;
int max_cntrs = perf_iommu->max_counters;
 
@@ -166,7 +168,9 @@ static int get_next_avail_iommu_bnk_cntr(struct 
perf_amd_iommu *perf_iommu)
continue;
} else {
perf_iommu->cntr_assign_mask |= BIT_ULL(shift);
-   retval = ((bank & 0xFF) << 8) | (cntr & 0xFF);
+   event->hw.iommu_bank = bank;
+   event->hw.iommu_cntr = cntr;
+   retval = 0;
goto out;
}
}
@@ -202,8 +206,6 @@ static int clear_avail_iommu_bnk_cntr(struct perf_amd_iommu 
*perf_iommu,
 static int perf_iommu_event_init(struct perf_event *event)
 {
struct hw_perf_event *hwc = &event->hw;
-   struct perf_amd_iommu *perf_iommu;
-   u64 config, config1;
 
/* test the event attr type check for PMU enumeration */
if (event->attr.type != event->pmu->type)
@@ -225,21 +227,9 @@ static int perf_iommu_event_init(struct perf_event *event)
if (event->cpu < 0)
return -EINVAL;
 
-   perf_iommu = &__perf_iommu;
-
-   if (event->pmu != &perf_iommu->pmu)
-   return -ENOENT;
-
-   if (perf_iommu) {
-   config = event->attr.config;
-   config1 = event->attr.config1;
-   } else {
-   return -EINVAL;
-   }
-
/* update the hw_perf_event struct with the iommu config data */
-   hwc->config = config;
-   hwc->extra_reg.config = config1;
+   hwc->conf  = event->attr.config;
+   hwc->conf1 = event->attr.config1;
 
return 0;
 }
@@ -247,26 +237,28 @@ static int perf_iommu_event_in

[PATCH v12 06/10] perf/amd/iommu: Modify functions to query max banks and counters

[Suravee Suthikulpanit]

Currently, amd_iommu_pc_get_max_[banks|counters]() use end-point
device ID to locate an IOMMU and check the reported max banks/counters.
The logic assumes that the IOMMU_BASE_DEVID belongs to the first IOMMU,
and uses it to acquire a reference to the first IOMMU, which does not work
on certain systems. Instead, modify the function to take an IOMMU index,
and use it to query the corresponding AMD IOMMU instance.

Currently, hardcodes the IOMMU index to 0 since the current AMD IOMMU
perf implementation only supports a single IOMMU. A subsequent
patch will add support for multiple IOMMUs, and will use a proper IOMMU index.

Cc: Peter Zijlstra 
Cc: Borislav Petkov 
Cc: Joerg Roedel 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 17 +++--
 arch/x86/events/amd/iommu.h |  9 -
 drivers/iommu/amd_iommu_init.c  | 34 --
 drivers/iommu/amd_iommu_proto.h |  2 --
 4 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index d4375da..10f67d3 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -239,14 +239,6 @@ static int perf_iommu_event_init(struct perf_event *event)
return -EINVAL;
}
 
-   /* integrate with iommu base devid (), assume one iommu */
-   perf_iommu->max_banks =
-   amd_iommu_pc_get_max_banks(IOMMU_BASE_DEVID);
-   perf_iommu->max_counters =
-   amd_iommu_pc_get_max_counters(IOMMU_BASE_DEVID);
-   if ((perf_iommu->max_banks == 0) || (perf_iommu->max_counters == 0))
-   return -EINVAL;
-
/* update the hw_perf_event struct with the iommu config data */
hwc->config = config;
hwc->extra_reg.config = config1;
@@ -448,6 +440,11 @@ static __init int _init_perf_amd_iommu(
return ret;
}
 
+   perf_iommu->max_banks= amd_iommu_pc_get_max_banks(0);
+   perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
+   if (!perf_iommu->max_banks || !perf_iommu->max_counters)
+   return -EINVAL;
+
perf_iommu->null_group = NULL;
perf_iommu->pmu.attr_groups = perf_iommu->attr_groups;
 
@@ -457,8 +454,8 @@ static __init int _init_perf_amd_iommu(
amd_iommu_pc_exit();
} else {
pr_info("Detected AMD IOMMU (%d banks, %d counters/bank).\n",
-   amd_iommu_pc_get_max_banks(IOMMU_BASE_DEVID),
-   amd_iommu_pc_get_max_counters(IOMMU_BASE_DEVID));
+   amd_iommu_pc_get_max_banks(0),
+   amd_iommu_pc_get_max_counters(0));
}
 
return ret;
diff --git a/arch/x86/events/amd/iommu.h b/arch/x86/events/amd/iommu.h
index 5c5c932..b775107 100644
--- a/arch/x86/events/amd/iommu.h
+++ b/arch/x86/events/amd/iommu.h
@@ -24,19 +24,18 @@
 #define PC_MAX_SPEC_BNKS   64
 #define PC_MAX_SPEC_CNTRS  16
 
-/* iommu pc reg masks*/
-#define IOMMU_BASE_DEVID   0x
-
 /* amd_iommu_init.c external support functions */
 extern int amd_iommu_get_num_iommus(void);
 
 extern bool amd_iommu_pc_supported(void);
 
-extern u8 amd_iommu_pc_get_max_banks(u16 devid);
+extern u8 amd_iommu_pc_get_max_banks(unsigned int idx);
 
-extern u8 amd_iommu_pc_get_max_counters(u16 devid);
+extern u8 amd_iommu_pc_get_max_counters(unsigned int idx);
 
 extern int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr,
u8 fxn, u64 *value, bool is_write);
 
+extern struct amd_iommu *get_amd_iommu(int idx);
+
 #endif /*_PERF_EVENT_AMD_IOMMU_H_*/
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index d3ec9c3..8fdf109 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2718,6 +2718,18 @@ bool amd_iommu_v2_supported(void)
 }
 EXPORT_SYMBOL(amd_iommu_v2_supported);
 
+struct amd_iommu *get_amd_iommu(unsigned int idx)
+{
+   unsigned int i = 0;
+   struct amd_iommu *iommu;
+
+   for_each_iommu(iommu)
+   if (i++ == idx)
+   return iommu;
+   return NULL;
+}
+EXPORT_SYMBOL(get_amd_iommu);
+
 /
  *
  * IOMMU EFR Performance Counter support functionality. This code allows
@@ -2725,17 +2737,14 @@ bool amd_iommu_v2_supported(void)
  *
  /
 
-u8 amd_iommu_pc_get_max_banks(u16 devid)
+u8 amd_iommu_pc_get_max_banks(unsigned int idx)
 {
-   struct amd_iommu *iommu;
-   u8 ret = 0;
+   struct amd_iommu *iommu = get_amd_iommu(idx);
 
-   /* locate the iommu governing the devid */
-   iommu = amd_iommu_rlookup_table[devid];
if (iommu)
-   ret = iommu->max_banks;
+   return iommu->max_banks;
 
-   return ret;
+   return 0;
 }

[PATCH v12 10/10] perf/amd/iommu: Enable support for multiple IOMMUs

[Suravee Suthikulpanit]

From: Suravee Suthikulpanit 

Add multi-IOMMU support for perf by exposing an AMD IOMMU PMU
for each IOMMU found in the system via:

  /bus/event_source/devices/amd_iommu_x

where x is the IOMMU index. This allows users to specify
different events to be programmed onto performance counters
of each IOMMU.

Cc: Borislav Petkov 
Cc: Peter Zijlstra 
Signed-off-by: Suravee Suthikulpanit 
---
 arch/x86/events/amd/iommu.c | 111 
 1 file changed, 70 insertions(+), 41 deletions(-)

diff --git a/arch/x86/events/amd/iommu.c b/arch/x86/events/amd/iommu.c
index 915a20c..9a12077 100644
--- a/arch/x86/events/amd/iommu.c
+++ b/arch/x86/events/amd/iommu.c
@@ -34,16 +34,21 @@
 #define GET_DOMID_MASK(x)  ((x->conf1 >> 16) & 0xULL)
 #define GET_PASID_MASK(x)  ((x->conf1 >> 32) & 0xFULL)
 
-static struct perf_amd_iommu __perf_iommu;
+#define PERF_AMD_IOMMU_NAME_SIZE   16
 
 struct perf_amd_iommu {
+   struct list_head list;
struct pmu pmu;
+   struct amd_iommu *iommu;
+   char name[PERF_AMD_IOMMU_NAME_SIZE];
u8 max_banks;
u8 max_counters;
u64 cntr_assign_mask;
raw_spinlock_t lock;
 };
 
+static LIST_HEAD(perf_amd_iommu_list);
+
 /*-
  * sysfs format attributes
  *-*/
@@ -234,9 +239,14 @@ static int perf_iommu_event_init(struct perf_event *event)
return 0;
 }
 
+static inline struct amd_iommu *perf_event_2_iommu(struct perf_event *ev)
+{
+   return (container_of(ev->pmu, struct perf_amd_iommu, pmu))->iommu;
+}
+
 static void perf_iommu_enable_event(struct perf_event *ev)
 {
-   struct amd_iommu *iommu = get_amd_iommu(0);
+   struct amd_iommu *iommu = perf_event_2_iommu(ev);
struct hw_perf_event *hwc = &ev->hw;
u8 bank = hwc->iommu_bank;
u8 cntr = hwc->iommu_cntr;
@@ -266,7 +276,7 @@ static void perf_iommu_enable_event(struct perf_event *ev)
 
 static void perf_iommu_disable_event(struct perf_event *event)
 {
-   struct amd_iommu *iommu = get_amd_iommu(0);
+   struct amd_iommu *iommu = perf_event_2_iommu(event);
struct hw_perf_event *hwc = &event->hw;
u64 reg = 0ULL;
 
@@ -286,7 +296,7 @@ static void perf_iommu_start(struct perf_event *event, int 
flags)
 
if (flags & PERF_EF_RELOAD) {
u64 prev_raw_count = local64_read(&hwc->prev_count);
-   struct amd_iommu *iommu = get_amd_iommu(0);
+   struct amd_iommu *iommu = perf_event_2_iommu(event);
 
amd_iommu_pc_set_reg(iommu, hwc->iommu_bank, hwc->iommu_cntr,
 IOMMU_PC_COUNTER_REG, &prev_raw_count);
@@ -301,7 +311,7 @@ static void perf_iommu_read(struct perf_event *event)
 {
u64 count, prev, delta;
struct hw_perf_event *hwc = &event->hw;
-   struct amd_iommu *iommu = get_amd_iommu(0);
+   struct amd_iommu *iommu = perf_event_2_iommu(event);
 
if (amd_iommu_pc_get_reg(iommu, hwc->iommu_bank, hwc->iommu_cntr,
 IOMMU_PC_COUNTER_REG, &count))
@@ -389,11 +399,6 @@ static __init int _init_events_attrs(void)
return 0;
 }
 
-static __init void amd_iommu_pc_exit(void)
-{
-   kfree(amd_iommu_events_group.attrs);
-}
-
 const struct attribute_group *amd_iommu_attr_groups[] = {
&amd_iommu_format_group,
&amd_iommu_cpumask_group,
@@ -401,47 +406,56 @@ static __init void amd_iommu_pc_exit(void)
NULL,
 };
 
+static struct pmu iommu_pmu = {
+   .event_init = perf_iommu_event_init,
+   .add= perf_iommu_add,
+   .del= perf_iommu_del,
+   .start  = perf_iommu_start,
+   .stop   = perf_iommu_stop,
+   .read   = perf_iommu_read,
+   .task_ctx_nr= perf_invalid_context,
+   .attr_groups= amd_iommu_attr_groups,
+};
+
 static __init int
-_init_perf_amd_iommu(struct perf_amd_iommu *perf_iommu, char *name)
+init_one_iommu(unsigned int idx)
 {
int ret;
+   struct perf_amd_iommu *perf_iommu;
 
-   raw_spin_lock_init(&perf_iommu->lock);
+   perf_iommu = kzalloc(sizeof(struct perf_amd_iommu), GFP_KERNEL);
+   if (!perf_iommu)
+   return -ENOMEM;
 
-   /* Init cpumask attributes to only core 0 */
-   cpumask_set_cpu(0, &iommu_cpumask);
+   raw_spin_lock_init(&perf_iommu->lock);
 
-   perf_iommu->max_banks= amd_iommu_pc_get_max_banks(0);
-   perf_iommu->max_counters = amd_iommu_pc_get_max_counters(0);
-   if (!perf_iommu->max_banks || !perf_iommu->max_counters)
+   perf_iommu->pmu  = iommu_pmu;
+   perf_iommu->iommu= get_amd_iommu(idx);
+   perf_iommu->max_banks= amd_iommu_pc_get_max_banks(idx);
+   perf_iommu->max_counters = amd_iommu_pc_get_max_counters(idx);
+   if (!perf_iommu->iommu || !perf_iommu->max_banks ||
+   !perf_iommu->max_counters) {
+  

RE: [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration

[Piotr Sroka]

Hi Masahiro

2017-03-21 08:3 AM  Masahiro Yamada :
> Hi Piotr,
> 
> 2017-03-21 16:01 GMT+09:00 Piotr Sroka :
> >
> > Hi Masahiro
> >
> > 2017-03-21 02:46 AM Masahiro Yamada :
> >> Hi Piotr,
> >>
> >>
> >> 2017-03-20 20:20 GMT+09:00 Piotr Sroka :
> >> > DTS properties are used instead of fixed data
> >> > because PHY settings can be different for different chips/boards.
> >> >
> >> > Signed-off-by: Piotr Sroka 
> >>
> >>
> >> I found this version is a problem for me.
> >>
> >>
> >> > +
> >> > +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = {
> >> > +   { "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, 
> >> > },
> >> > +   { "cdns,phy-input-delay-sd-legacy", 
> >> > SDHCI_CDNS_PHY_DLY_SD_DEFAULT, },
> >> > +   { "cdns,phy-input-delay-sd-uhs-sdr12", 
> >> > SDHCI_CDNS_PHY_DLY_UHS_SDR12, },
> >> > +   { "cdns,phy-input-delay-sd-uhs-sdr25", 
> >> > SDHCI_CDNS_PHY_DLY_UHS_SDR25, },
> >> > +   { "cdns,phy-input-delay-sd-uhs-sdr50", 
> >> > SDHCI_CDNS_PHY_DLY_UHS_SDR50, },
> >> > +   { "cdns,phy-input-delay-sd-uhs-ddr50", 
> >> > SDHCI_CDNS_PHY_DLY_UHS_DDR50, },
> >> > +   { "cdns,phy-input-delay-mmc-highspeed", 
> >> > SDHCI_CDNS_PHY_DLY_EMMC_SDR, },
> >> > +   { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, },
> >> > +   { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, },
> >> > +   { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, },
> >> > +   { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, },
> >> > +};
> >> > +
> >>
> >>
> >> I see mmc-legacy property in v1,
> >> but it is missing now.
> >>
> >>
> >> >  static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv,
> >> > u8 addr, u8 data)
> >> >  {
> >> > @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct 
> >> > sdhci_cdns_priv *priv,
> >> > return 0;
> >> >  }
> >> >
> >> > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv)
> >> > +static int sdhci_cdns_phy_init(struct device_node *np,
> >> > +  struct sdhci_cdns_priv *priv)
> >> >  {
> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4);
> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 4);
> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 
> >> > 9);
> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2);
> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3);
> >>
> >>
> >> I need to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY to 9 for my SoC.
> >>
> >> Maybe, do we need a DT property for this, too?
> >>
> > I can add it but could you check if you realy need it? There is no 
> > selection MMC legacy mode
> > in this driver.
> >
> 
> Ah, you are right.
> 
> For Linux, SD-Legacy and MMC-Legacy share the same timing flag.
> The legacy mode is covered by SDHCI_CDNS_PHY_DLY_SD_DEFAULT,
> so we need not to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY.
> 
> So, I have no problem with patch.
> 
> Reviewed-by: Masahiro Yamada 
> 
> 
> 
> 
> From here, just my minor comments.
> The binding should not be too Linux-oriented.
> Device Tree is hardware description language, and project-neutral from
> its concept.
> I wonder if there is a project that handles SD-Legacy and MMC-Legacy 
> separately.
> I am not sure about this, and I do not have a strong opinion, either.
> 
> I leave this to you  (and other developers).
> 

Thanks for your comments.

Because patch adding HS400 enhanced strobe support was applied 
for next branch I needed to create v5. BTW I added one extra patch modifing
probe function as you suggested.
I also modify binding to be consistent with linux MMC timing names.
The driver does not use MMC legacy at all so I think there is no need 
to distinguish between SD Legacy and MMC Legacy. So there are no 
contraindications
for being consistent with Linux.


Best Regards
Piotr Sroka

Re: [PATCH v3 02/11] ARM: sun8i: a33: add all operating points

[Maxime Ripard]

On Wed, Mar 22, 2017 at 11:31:54AM +0800, Chen-Yu Tsai wrote:
> On Tue, Mar 21, 2017 at 11:36 PM, Quentin Schulz
>  wrote:
> > This adds almost all operating points allowed for the A33 as defined by
> > fex files available at:
> > https://github.com/linux-sunxi/sunxi-boards/tree/master/sys_config/a33
> >
> > There are more possible frequencies in this patch than there are in the
> > fex files because the fex files only give an interval of possible
> > frequencies for a given voltage. All supported frequencies are defined
> > in the original driver code in Allwinner vendor tree.
> >
> > There are two missing frequencies though: 1104MHz and 1200MHz which
> > require the CPU to have 1.32V supplied, which is higher than the default
> > voltage.
> >
> > Without all A33 boards defining the CPU regulator, we cannot have these
> > two frequencies as it would cause the CPU to try to run a higher
> > frequency without "overvolting" which is very likely to crash the CPU.
> >
> > Therefore, these two frequencies must be enabled on a per-board basis.
> >
> > Signed-off-by: Quentin Schulz 
> 
> Acked-by: Chen-Yu Tsai 

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH] ARM: dts: sun6i: sina31s: Enable SPDIF out

[Maxime Ripard]

On Wed, Mar 22, 2017 at 01:47:10PM +0800, Chen-Yu Tsai wrote:
> The SinA31s has a coaxial SPDIF output. Enable it.
> 
> Signed-off-by: Chen-Yu Tsai 

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH v2 2/2] iio: Aspeed AST2400/AST2500 ADC

[Quentin Schulz]

Hi,

On 21/03/2017 21:48, Rick Altherr wrote:
> Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. Low
> and high threshold interrupts are supported by the hardware but are not
> currently implemented.
> 
> Signed-off-by: Rick Altherr 
> ---
> 
> Changes in v2:
> - Rewritten as an IIO device
> - Renamed register macros to describe the register's purpose
> - Replaced awkward reading of 16-bit data registers with readw()
> - Added Kconfig dependency on COMPILE_TEST
> 
[...]
> +struct aspeed_adc_data {
> + struct device   *dev;
> + void __iomem*base;
> +

Useless empty line?

> + spinlock_t  clk_lock;
> + struct clk_hw   *clk_prescaler;
> + struct clk_hw   *clk_scaler;
> +};
> +
> +#define ASPEED_ADC_CHAN(_idx, _addr) {   \
> + .type = IIO_VOLTAGE,\
> + .indexed = 1,   \
> + .channel = (_idx),  \
> + .address = (_addr), \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |  \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ),   \
> +}
> +
> +static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
> + ASPEED_ADC_CHAN(0, 0x10),
> + ASPEED_ADC_CHAN(1, 0x12),
> + ASPEED_ADC_CHAN(2, 0x14),
> + ASPEED_ADC_CHAN(3, 0x16),
> + ASPEED_ADC_CHAN(4, 0x18),
> + ASPEED_ADC_CHAN(5, 0x1A),
> + ASPEED_ADC_CHAN(6, 0x1C),
> + ASPEED_ADC_CHAN(7, 0x1E),
> + ASPEED_ADC_CHAN(8, 0x20),
> + ASPEED_ADC_CHAN(9, 0x22),
> + ASPEED_ADC_CHAN(10, 0x24),
> + ASPEED_ADC_CHAN(11, 0x26),
> + ASPEED_ADC_CHAN(12, 0x28),
> + ASPEED_ADC_CHAN(13, 0x2A),
> + ASPEED_ADC_CHAN(14, 0x2C),
> + ASPEED_ADC_CHAN(15, 0x2E),

It would make sense to name the registers (the _addr parameter of your
macro) so it's easier to understand what it refers to.

[...]
> +static int aspeed_adc_probe(struct platform_device *pdev)
> +{
> + struct iio_dev *indio_dev;
> + struct aspeed_adc_data *data;
> + struct resource *res;
> + const char *clk_parent_name;
> + int ret;
> + u32 adc_engine_control_reg_val;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*data));
> + if (!indio_dev) {
> + dev_err(&pdev->dev, "Failed allocating iio device\n");
> + return -ENOMEM;
> + }
> +
> + data = iio_priv(indio_dev);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + data->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(data->base)) {
> + dev_err(&pdev->dev, "Failed allocating device resources\n");
> + ret = PTR_ERR(data->base);
> + goto resource_error;
> + }
> +
> + /* Register ADC clock prescaler with source specified by device tree. */
> + spin_lock_init(&data->clk_lock);
> + clk_parent_name = of_clk_get_parent_name(pdev->dev.of_node, 0);
> +
> + data->clk_prescaler = clk_hw_register_divider(
> + &pdev->dev, "prescaler", clk_parent_name, 0,
> + data->base + ASPEED_ADC_REG_CLOCK_CONTROL,
> + 17, 15, 0, &data->clk_lock);
> + if (IS_ERR(data->clk_prescaler)) {
> + dev_err(&pdev->dev, "Failed allocating prescaler clock\n");
> + ret = PTR_ERR(data->clk_prescaler);
> + goto prescaler_error;
> + }
> +
> + /*
> +  * Register ADC clock scaler downstream from the prescaler. Allow rate
> +  * setting to adjust the prescaler as well.
> +  */
> + data->clk_scaler = clk_hw_register_divider(
> + &pdev->dev, "scaler", "prescaler",
> + CLK_SET_RATE_PARENT,
> + data->base + ASPEED_ADC_REG_CLOCK_CONTROL,
> + 0, 10, 0, &data->clk_lock);
> + if (IS_ERR(data->clk_scaler)) {
> + dev_err(&pdev->dev, "Failed allocating scaler clock\n");
> + ret = PTR_ERR(data->clk_scaler);
> + goto scaler_error;
> + }
> +
> + /* Start all channels in normal mode. */
> + clk_prepare_enable(data->clk_scaler->clk);
> + adc_engine_control_reg_val = GENMASK(31, 16) |
> + ASPEED_ADC_OPERATION_MODE_NORMAL | ASPEED_ADC_ENGINE_ENABLE;
> + writel(adc_engine_control_reg_val,
> + data->base + ASPEED_ADC_REG_ENGINE_CONTROL);
> +
> + indio_dev->name = dev_name(&pdev->dev);

This isn't good practice (cf.: https://lkml.org/lkml/2017/1/28/145 end
of the mail in the probe function). Better name it aspeed-adc or even
better to have a different name per compatible: ast2400-adc or ast2500-adc.

> + indio_dev->dev.parent = &pdev->dev;
> + indio_dev->info = &aspeed_adc_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->ch

Re: [RESEND PATCH v4 0/9] add support for AXP20X and AXP22X power supply drivers

[Maxime Ripard]

On Mon, Mar 20, 2017 at 09:16:44AM +0100, Quentin Schulz wrote:
> These patches haven't changed since v4. The goal is to split the series in
> two series, one (this one) with patches ready to be merged and the other
> one (everything impacting the battery power supply) which depends on an
> ongoing series by Liam Breck to be merged once the ongoing series has a
> consensus.
> 
> Therefore, this series of patch is ready to be merged. I don't see any
> dependency between subsystems.

Merged the patches 5-8, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [v4 3/3] mmc: sdhci-cadence: Update PHY delay configuration

[Masahiro Yamada]

2017-03-22 16:08 GMT+09:00 Piotr Sroka :
> Hi Masahiro
>
> 2017-03-21 08:3 AM  Masahiro Yamada :
>> Hi Piotr,
>>
>> 2017-03-21 16:01 GMT+09:00 Piotr Sroka :
>> >
>> > Hi Masahiro
>> >
>> > 2017-03-21 02:46 AM Masahiro Yamada :
>> >> Hi Piotr,
>> >>
>> >>
>> >> 2017-03-20 20:20 GMT+09:00 Piotr Sroka :
>> >> > DTS properties are used instead of fixed data
>> >> > because PHY settings can be different for different chips/boards.
>> >> >
>> >> > Signed-off-by: Piotr Sroka 
>> >>
>> >>
>> >> I found this version is a problem for me.
>> >>
>> >>
>> >> > +
>> >> > +static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = {
>> >> > +   { "cdns,phy-input-delay-sd-highspeed", 
>> >> > SDHCI_CDNS_PHY_DLY_SD_HS, },
>> >> > +   { "cdns,phy-input-delay-sd-legacy", 
>> >> > SDHCI_CDNS_PHY_DLY_SD_DEFAULT, },
>> >> > +   { "cdns,phy-input-delay-sd-uhs-sdr12", 
>> >> > SDHCI_CDNS_PHY_DLY_UHS_SDR12, },
>> >> > +   { "cdns,phy-input-delay-sd-uhs-sdr25", 
>> >> > SDHCI_CDNS_PHY_DLY_UHS_SDR25, },
>> >> > +   { "cdns,phy-input-delay-sd-uhs-sdr50", 
>> >> > SDHCI_CDNS_PHY_DLY_UHS_SDR50, },
>> >> > +   { "cdns,phy-input-delay-sd-uhs-ddr50", 
>> >> > SDHCI_CDNS_PHY_DLY_UHS_DDR50, },
>> >> > +   { "cdns,phy-input-delay-mmc-highspeed", 
>> >> > SDHCI_CDNS_PHY_DLY_EMMC_SDR, },
>> >> > +   { "cdns,phy-input-delay-mmc-ddr", SDHCI_CDNS_PHY_DLY_EMMC_DDR, 
>> >> > },
>> >> > +   { "cdns,phy-dll-delay-sdclk", SDHCI_CDNS_PHY_DLY_SDCLK, },
>> >> > +   { "cdns,phy-dll-delay-sdclk-hsmmc", SDHCI_CDNS_PHY_DLY_HSMMC, },
>> >> > +   { "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, },
>> >> > +};
>> >> > +
>> >>
>> >>
>> >> I see mmc-legacy property in v1,
>> >> but it is missing now.
>> >>
>> >>
>> >> >  static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv,
>> >> > u8 addr, u8 data)
>> >> >  {
>> >> > @@ -90,13 +113,26 @@ static int sdhci_cdns_write_phy_reg(struct 
>> >> > sdhci_cdns_priv *priv,
>> >> > return 0;
>> >> >  }
>> >> >
>> >> > -static void sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv)
>> >> > +static int sdhci_cdns_phy_init(struct device_node *np,
>> >> > +  struct sdhci_cdns_priv *priv)
>> >> >  {
>> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_HS, 4);
>> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_SD_DEFAULT, 
>> >> > 4);
>> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_LEGACY, 
>> >> > 9);
>> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_SDR, 2);
>> >> > -   sdhci_cdns_write_phy_reg(priv, SDHCI_CDNS_PHY_DLY_EMMC_DDR, 3);
>> >>
>> >>
>> >> I need to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY to 9 for my SoC.
>> >>
>> >> Maybe, do we need a DT property for this, too?
>> >>
>> > I can add it but could you check if you realy need it? There is no 
>> > selection MMC legacy mode
>> > in this driver.
>> >
>>
>> Ah, you are right.
>>
>> For Linux, SD-Legacy and MMC-Legacy share the same timing flag.
>> The legacy mode is covered by SDHCI_CDNS_PHY_DLY_SD_DEFAULT,
>> so we need not to set SDHCI_CDNS_PHY_DLY_EMMC_LEGACY.
>>
>> So, I have no problem with patch.
>>
>> Reviewed-by: Masahiro Yamada 
>>
>>
>>
>>
>> From here, just my minor comments.
>> The binding should not be too Linux-oriented.
>> Device Tree is hardware description language, and project-neutral from
>> its concept.
>> I wonder if there is a project that handles SD-Legacy and MMC-Legacy 
>> separately.
>> I am not sure about this, and I do not have a strong opinion, either.
>>
>> I leave this to you  (and other developers).
>>
>
> Thanks for your comments.
>
> Because patch adding HS400 enhanced strobe support was applied
> for next branch I needed to create v5. BTW I added one extra patch modifing
> probe function as you suggested.
> I also modify binding to be consistent with linux MMC timing names.
> The driver does not use MMC legacy at all so I think there is no need
> to distinguish between SD Legacy and MMC Legacy. So there are no 
> contraindications
> for being consistent with Linux.


OK.  Make sense.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 02/10] x86: assembly, FUNC_START for fn, DATA_START for data

[Ingo Molnar]

* Pavel Machek  wrote:

> Hi!
> 
> > -ENTRY(saved_rbp)   .quad   0
> > -ENTRY(saved_rsi)   .quad   0
> > -ENTRY(saved_rdi)   .quad   0
> > -ENTRY(saved_rbx)   .quad   0
> > +SYM_DATA_START(saved_rbp)  .quad   0
> > +SYM_DATA_START(saved_rsi)  .quad   0
> > +SYM_DATA_START(saved_rdi)  .quad   0
> > +SYM_DATA_START(saved_rbx)  .quad   0
> 
> Does it make sense to call it SYM_DATA_*START* when there's no
> corresponding end?

That looks like a bug - I think we should strive for them to always be in pairs.

Jiri, Josh, could objtool help here perhaps, to detect 'non-terminated' 
SYM_*_START() uses? This could be done by emitting debug data into a special 
section and then analyzing that section for unpaired entries. The section can 
be 
discarded in the final link, it won't show up in the kernel image.

We don't ever nest symbols, right?

> Plus... it looks like saved_rsi (and friends) are only used inside
> wakeup_64.S. Could we just delete the "ENTRY" annotations?

That appears to make sense as well.

Thanks,

Ingo

Re: [v5 1/4] mmc: sdhci-cadence: Fix writing PHY delay

[Masahiro Yamada]

2017-03-21 23:32 GMT+09:00 Piotr Sroka :
> Add polling for ACK to be sure that data are written to PHY register.
>
> Signed-off-by: Piotr Sroka 


Reviewed-by: Masahiro Yamada 


-- 
Best Regards
Masahiro Yamada

Re: [v5 4/4] mmc: sdhci-cadence: refactor probe function

[Masahiro Yamada]

2017-03-21 23:33 GMT+09:00 Piotr Sroka :
> Use added dev variable for devm_clk_get.
>
> Signed-off-by: Piotr Sroka 
> ---
> Changes for v5:
> - patch created in v5
> ---


Reviewed-by: Masahiro Yamada 


-- 
Best Regards
Masahiro Yamada

Re: [v5 3/4] mmc: sdhci-cadence: Update PHY delay configuration

[Masahiro Yamada]

2017-03-21 23:33 GMT+09:00 Piotr Sroka :
> DTS properties are used instead of fixed data
> because PHY settings can be different for different chips/boards.
>
> Signed-off-by: Piotr Sroka 
> ---
> Changes for v2:
> - dts part was removed from this patch
> - most delays were moved from dts file
>   to data associated with an SoC specific compatible
> - remove unrelated changes
> ---
> Changes for v3:
> - move all delays back to dts because they are also boards dependent
> - prefix all of the Cadence-specific properties with cdns prefix
> - put checking delay properties inside the for loop
>   instead of using a lot of single if expressions
> ---
> Changes for v4:
> - remove unecessary declaration of sdhci_cdns_match
> - format fix (blank line removed)
> ---
> Changes for v5:
> - use driver version from next branch, with applied enhanced strobe feature 
> support.
> - change name of property to be consistent with timing modes
>   available in Linux
> ---


Reviewed-by: Masahiro Yamada 

-- 
Best Regards
Masahiro Yamada

Re: [v5 2/4] dt-bindings: mmc: add description of PHY delays for sdhci-cadence

[Masahiro Yamada]

2017-03-21 23:33 GMT+09:00 Piotr Sroka :
> DTS properties are used instead of fixed data
> because PHY settings can be different for different chips/boards.
> Add description of new DLL PHY delays.
>
> Signed-off-by: Piotr Sroka 
> ---
> Changes for v2:
> - file was created in v2. It was a part of driver source file patch.
> - most delays were moved from dts file
>   to data associated with an SoC specific compatible
> - description of delays was updated to be more clearly
> ---
> Changes for v3:
> - move all delays back to dts because they are also boards dependent
> - prefix all of the Cadence-specific properties with cdns prefix
> ---
> Changes for v4:
> - change the beginning of the commit subject
> ---
> Changes for v5:
> - change name of property to be consistent with timing modes
>   available in Linux


As I gave Reviewed-by in v4 already, this looks good to me.


Reviewed-by: Masahiro Yamada 


As I said before,
once you get Reviewed/Acked tags,
please include them in your later version.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 4/6] x86/kasan: Prepare clear_pgds() to switch to

[Ingo Molnar]

* Andrey Ryabinin  wrote:

> 
> 
> On 03/17/2017 09:55 PM, Kirill A. Shutemov wrote:
> > With folded p4d, pgd_clear() is nop. Change clear_pgds() to use
> > p4d_clear() instead.
> > 
> 
> You could probably just use set_pgd(pgd_offset_k(start), __pgd(0)); instead 
> of pgd_clear()
> as we already do in arm64.
> It's basically pgd_clear() except it's not a nop wih p4d folded.

Makes sense. Kirill, if you agree, mind spinning a v2 patch?

Thanks,

Ingo

Re: [PATCH 4/4] perf annotate: More exactly grep -v of the objdump command

[Taeung Song]

On 03/22/2017 03:32 AM, Arnaldo Carvalho de Melo wrote:

Em Tue, Mar 21, 2017 at 03:29:50PM -0300, Arnaldo Carvalho de Melo escreveu:

Em Wed, Mar 22, 2017 at 01:19:49AM +0900, Taeung Song escreveu:

On 03/21/2017 11:37 PM, Arnaldo Carvalho de Melo wrote:

+++ b/tools/perf/util/annotate.c
@@ -1443,7 +1443,7 @@ int symbol__disassemble(struct symbol *sym, struct map 
*map, const char *arch_na
snprintf(command, sizeof(command),
 "%s %s%s --start-address=0x%016" PRIx64
 " --stop-address=0x%016" PRIx64
-" -l -d %s %s -C %s 2>/dev/null|grep -v %s|expand",
+" -l -d %s %s -C %s 2>/dev/null|grep -v %s: |expand",



Are you sure this works as you expect? I.e. both lines will have '%s:'



Yes, if we do, we can exactly the only below line without the side effect
IMHO.


yeah, because the .c is not there, ok gotcha, will get your new commit
log message, massage it a bit and apply the patch.


Nah, with all this in mind, the original message is good enough, sorry
for the confusion with the .c and the space after ':' (that I removed,
needless and potentially confusing, as we saw here).

- Arnaldo


I got it!

Thanks for your review,
will also resend v2 with the rest bugfix patches making
the commit log messages of them be more clearly. :)

Thanks,
Taeung

Re: [PATCH v2 03/10] x86: assembly, use SYM_FUNC_END for functions

[Ingo Molnar]

* Josh Poimboeuf  wrote:

> On Mon, Mar 20, 2017 at 01:32:15PM +0100, Jiri Slaby wrote:
> >  ENTRY(ftrace_caller)
> > /* save_mcount_regs fills in first two parameters */
> > @@ -184,11 +184,12 @@ GLOBAL(ftrace_epilogue)
> >  GLOBAL(ftrace_graph_call)
> > jmp ftrace_stub
> >  #endif
> > +SYM_FUNC_END(ftrace_caller)
> >  
> >  /* This is weak to keep gas from relaxing the jumps */
> >  WEAK(ftrace_stub)
> > retq
> > -END(ftrace_caller)
> > +SYM_FUNC_END(ftrace_caller)
> 
> This gives ftrace_caller() two ends.

Such errors too could be detected automatically via objtool or some other 
symbol 
debug mechanism.

The reason it might be a good fit for objtool is to make the checking optional 
(no 
need to burden a regular build with it), plus objtool already looks at the .o 
from 
first principles - a symbol checking sub-functionality could analyze the symbol 
names in the same pass.

If we want to complicate things with CFI then we absolutely should increase the 
quality of our symbol names space.

Thanks,

Ingo

[PATCH] net: vmxnet3: use new api ethtool_{get|set}_link_ksettings

[Philippe Reynes]

The ethtool api {get|set}_settings is deprecated.
We move this driver to new api {get|set}_link_ksettings.

Signed-off-by: Philippe Reynes 
---
 drivers/net/vmxnet3/vmxnet3_ethtool.c |   25 ++---
 1 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/net/vmxnet3/vmxnet3_ethtool.c 
b/drivers/net/vmxnet3/vmxnet3_ethtool.c
index f88ffaf..2ff2731 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethtool.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethtool.c
@@ -471,22 +471,25 @@ int vmxnet3_set_features(struct net_device *netdev, 
netdev_features_t features)
 
 
 static int
-vmxnet3_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
+vmxnet3_get_link_ksettings(struct net_device *netdev,
+  struct ethtool_link_ksettings *ecmd)
 {
struct vmxnet3_adapter *adapter = netdev_priv(netdev);
 
-   ecmd->supported = SUPPORTED_1baseT_Full | SUPPORTED_1000baseT_Full |
- SUPPORTED_TP;
-   ecmd->advertising = ADVERTISED_TP;
-   ecmd->port = PORT_TP;
-   ecmd->transceiver = XCVR_INTERNAL;
+   ethtool_link_ksettings_zero_link_mode(ecmd, supported);
+   ethtool_link_ksettings_add_link_mode(ecmd, supported, 1baseT_Full);
+   ethtool_link_ksettings_add_link_mode(ecmd, supported, 1000baseT_Full);
+   ethtool_link_ksettings_add_link_mode(ecmd, supported, TP);
+   ethtool_link_ksettings_zero_link_mode(ecmd, advertising);
+   ethtool_link_ksettings_add_link_mode(ecmd, advertising, TP);
+   ecmd->base.port = PORT_TP;
 
if (adapter->link_speed) {
-   ethtool_cmd_speed_set(ecmd, adapter->link_speed);
-   ecmd->duplex = DUPLEX_FULL;
+   ecmd->base.speed = adapter->link_speed;
+   ecmd->base.duplex = DUPLEX_FULL;
} else {
-   ethtool_cmd_speed_set(ecmd, SPEED_UNKNOWN);
-   ecmd->duplex = DUPLEX_UNKNOWN;
+   ecmd->base.speed = SPEED_UNKNOWN;
+   ecmd->base.duplex = DUPLEX_UNKNOWN;
}
return 0;
 }
@@ -880,7 +883,6 @@ int vmxnet3_set_features(struct net_device *netdev, 
netdev_features_t features)
 }
 
 static const struct ethtool_ops vmxnet3_ethtool_ops = {
-   .get_settings  = vmxnet3_get_settings,
.get_drvinfo   = vmxnet3_get_drvinfo,
.get_regs_len  = vmxnet3_get_regs_len,
.get_regs  = vmxnet3_get_regs,
@@ -900,6 +902,7 @@ int vmxnet3_set_features(struct net_device *netdev, 
netdev_features_t features)
.get_rxfh  = vmxnet3_get_rss,
.set_rxfh  = vmxnet3_set_rss,
 #endif
+   .get_link_ksettings = vmxnet3_get_link_ksettings,
 };
 
 void vmxnet3_set_ethtool_ops(struct net_device *netdev)
-- 
1.7.4.4

Re: [lkp-robot] [x86] 69218e4799: BUG:kernel_hang_in_boot_stage

[Ingo Molnar]

* Thomas Garnier  wrote:

> >  static inline void setup_fixmap_gdt(int cpu)
> >  {
> > __set_fixmap(get_cpu_gdt_ro_index(cpu),
> > -__pa(get_cpu_gdt_rw(cpu)), pg_fixmap_gdt_flags);
> > +slow_virt_to_phys(get_cpu_gdt_rw(cpu)),
> > +pg_fixmap_gdt_flags);
> >  }
> >
> >  /* Load the original GDT from the per-cpu structure */
> >
> > This makes UP boot for me, but SMP (2 cpus) is still busted.
> 
> This change fixed boot for me:
> 
> diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
> index b65155cc3760..4e30707d9f9a 100644
> --- a/arch/x86/include/asm/fixmap.h
> +++ b/arch/x86/include/asm/fixmap.h
> @@ -104,7 +104,9 @@ enum fixed_addresses {
> FIX_GDT_REMAP_BEGIN,
> FIX_GDT_REMAP_END = FIX_GDT_REMAP_BEGIN + NR_CPUS - 1,
> 
> -   __end_of_permanent_fixed_addresses,
> +   __end_of_permanent_fixed_addresses =
> +   (FIX_GDT_REMAP_END + PTRS_PER_PTE - 1) &
> +   -PTRS_PER_PTE,
> 
> Just ensure PKMAP_BASE & FIX_WP_TEST are on a different PMD.
> 
> I don't think that the right fix but it might help understand the
> exact root cause.

Could this be related to the permission bits in the PMD itself getting out of 
sync 
with the PTEs? WP test marks a page writable/unwritable, and maybe we mess up 
the 
restoration. If they are on separate PMDs then this is worked around because 
the 
fixmap GDT is on a separate PMD.

Thanks,

Ingo

Re: [PATCH] tpm: make check_locality return bool

[Jarkko Sakkinen]

On Sat, Mar 18, 2017 at 01:59:57AM -0700, Jerry Snitselaar wrote:
> Since check_locality is checking to see if a certain
> locality is active, return true if active otherwise
> return false.
> 
> Cc: Christophe Ricard 
> Cc: Jason Gunthorpe 
> Cc: Marcel Selhorst 
> Cc: Jarkko Sakkinen 
> Cc: Peter Huewe 
> Signed-off-by: Jerry Snitselaar 
> ---
> Tested tpm_tis, but don't have the ability to
> test Infineon or STMicro drivers.

Does not matter as this one does not have connection to the used
hardware. I'll have to test this.

/Jarkko

> 
>  drivers/char/tpm/st33zp24/st33zp24.c | 12 ++--
>  drivers/char/tpm/tpm_i2c_infineon.c  | 12 ++--
>  drivers/char/tpm/tpm_tis_core.c  | 20 +++-
>  3 files changed, 23 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/char/tpm/st33zp24/st33zp24.c 
> b/drivers/char/tpm/st33zp24/st33zp24.c
> index e8e0f7c02686..4d1dc8b46877 100644
> --- a/drivers/char/tpm/st33zp24/st33zp24.c
> +++ b/drivers/char/tpm/st33zp24/st33zp24.c
> @@ -117,9 +117,9 @@ static u8 st33zp24_status(struct tpm_chip *chip)
>  /*
>   * check_locality if the locality is active
>   * @param: chip, the tpm chip description
> - * @return: the active locality or -EACCESS.
> + * @return: true if LOCALITY0 is active, otherwise false
>   */
> -static int check_locality(struct tpm_chip *chip)
> +static bool check_locality(struct tpm_chip *chip)
>  {
>   struct st33zp24_dev *tpm_dev = dev_get_drvdata(&chip->dev);
>   u8 data;
> @@ -129,9 +129,9 @@ static int check_locality(struct tpm_chip *chip)
>   if (status && (data &
>   (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
>   (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID))
> - return tpm_dev->locality;
> + return true;
>  
> - return -EACCES;
> + return false;
>  } /* check_locality() */
>  
>  /*
> @@ -146,7 +146,7 @@ static int request_locality(struct tpm_chip *chip)
>   long ret;
>   u8 data;
>  
> - if (check_locality(chip) == tpm_dev->locality)
> + if (check_locality(chip))
>   return tpm_dev->locality;
>  
>   data = TPM_ACCESS_REQUEST_USE;
> @@ -158,7 +158,7 @@ static int request_locality(struct tpm_chip *chip)
>  
>   /* Request locality is usually effective after the request */
>   do {
> - if (check_locality(chip) >= 0)
> + if (check_locality(chip))
>   return tpm_dev->locality;
>   msleep(TPM_TIMEOUT);
>   } while (time_before(jiffies, stop));
> diff --git a/drivers/char/tpm/tpm_i2c_infineon.c 
> b/drivers/char/tpm/tpm_i2c_infineon.c
> index 62ee44e57ddc..dc47fa222a26 100644
> --- a/drivers/char/tpm/tpm_i2c_infineon.c
> +++ b/drivers/char/tpm/tpm_i2c_infineon.c
> @@ -278,22 +278,22 @@ enum tis_defaults {
>  #define  TPM_DATA_FIFO(l)(0x0005 | ((l) << 4))
>  #define  TPM_DID_VID(l)  (0x0006 | ((l) << 4))
>  
> -static int check_locality(struct tpm_chip *chip, int loc)
> +static bool check_locality(struct tpm_chip *chip, int loc)
>  {
>   u8 buf;
>   int rc;
>  
>   rc = iic_tpm_read(TPM_ACCESS(loc), &buf, 1);
>   if (rc < 0)
> - return rc;
> + return false;
>  
>   if ((buf & (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) ==
>   (TPM_ACCESS_ACTIVE_LOCALITY | TPM_ACCESS_VALID)) {
>   tpm_dev.locality = loc;
> - return loc;
> + return true;
>   }
>  
> - return -EIO;
> + return false;
>  }
>  
>  /* implementation similar to tpm_tis */
> @@ -315,7 +315,7 @@ static int request_locality(struct tpm_chip *chip, int 
> loc)
>   unsigned long stop;
>   u8 buf = TPM_ACCESS_REQUEST_USE;
>  
> - if (check_locality(chip, loc) >= 0)
> + if (check_locality(chip, loc))
>   return loc;
>  
>   iic_tpm_write(TPM_ACCESS(loc), &buf, 1);
> @@ -323,7 +323,7 @@ static int request_locality(struct tpm_chip *chip, int 
> loc)
>   /* wait for burstcount */
>   stop = jiffies + chip->timeout_a;
>   do {
> - if (check_locality(chip, loc) >= 0)
> + if (check_locality(chip, loc))
>   return loc;
>   usleep_range(TPM_TIMEOUT_US_LOW, TPM_TIMEOUT_US_HI);
>   } while (time_before(jiffies, stop));
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index c0f296b5d413..93d641996249 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -56,7 +56,7 @@ static int wait_startup(struct tpm_chip *chip, int l)
>   return -1;
>  }
>  
> -static int check_locality(struct tpm_chip *chip, int l)
> +static bool check_locality(struct tpm_chip *chip, int l)
>  {
>   struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
>   int rc;
> @@ -64,13 +64,15 @@ static int check_locality(struct tpm_chip *chip, int l)
>  
>   rc = tpm_tis_read8(priv, TPM_ACCESS(l), &access);
>

Re: [PATCH v2 02/10] x86: assembly, FUNC_START for fn, DATA_START for data

[Jiri Slaby]

On 03/22/2017, 08:25 AM, Ingo Molnar wrote:
> 
> * Pavel Machek  wrote:
> 
>> Hi!
>>
>>> -ENTRY(saved_rbp)   .quad   0
>>> -ENTRY(saved_rsi)   .quad   0
>>> -ENTRY(saved_rdi)   .quad   0
>>> -ENTRY(saved_rbx)   .quad   0
>>> +SYM_DATA_START(saved_rbp)  .quad   0
>>> +SYM_DATA_START(saved_rsi)  .quad   0
>>> +SYM_DATA_START(saved_rdi)  .quad   0
>>> +SYM_DATA_START(saved_rbx)  .quad   0
>>
>> Does it make sense to call it SYM_DATA_*START* when there's no
>> corresponding end?
> 
> That looks like a bug - I think we should strive for them to always be in 
> pairs.
> 
> Jiri, Josh, could objtool help here perhaps, to detect 'non-terminated' 
> SYM_*_START() uses? This could be done by emitting debug data into a special 
> section and then analyzing that section for unpaired entries. The section can 
> be 
> discarded in the final link, it won't show up in the kernel image.

It should be easier than that. No introduction of other info needed --
every global symbol without a ".type" or ".size" (i.e. SYM_*_END) should
be a bug now.

> We don't ever nest symbols, right?

AFAI could see so far, correct.

>> Plus... it looks like saved_rsi (and friends) are only used inside
>> wakeup_64.S. Could we just delete the "ENTRY" annotations?
> 
> That appears to make sense as well.

+1, will fix this.

thanks,
-- 
js
suse labs

Re: [PATCH v2 02/10] x86: assembly, FUNC_START for fn, DATA_START for data

[Ingo Molnar]

* Jiri Slaby  wrote:

> On 03/22/2017, 08:25 AM, Ingo Molnar wrote:
> > 
> > * Pavel Machek  wrote:
> > 
> >> Hi!
> >>
> >>> -ENTRY(saved_rbp) .quad   0
> >>> -ENTRY(saved_rsi) .quad   0
> >>> -ENTRY(saved_rdi) .quad   0
> >>> -ENTRY(saved_rbx) .quad   0
> >>> +SYM_DATA_START(saved_rbp).quad   0
> >>> +SYM_DATA_START(saved_rsi).quad   0
> >>> +SYM_DATA_START(saved_rdi).quad   0
> >>> +SYM_DATA_START(saved_rbx).quad   0
> >>
> >> Does it make sense to call it SYM_DATA_*START* when there's no
> >> corresponding end?
> > 
> > That looks like a bug - I think we should strive for them to always be in 
> > pairs.
> > 
> > Jiri, Josh, could objtool help here perhaps, to detect 'non-terminated' 
> > SYM_*_START() uses? This could be done by emitting debug data into a 
> > special 
> > section and then analyzing that section for unpaired entries. The section 
> > can be 
> > discarded in the final link, it won't show up in the kernel image.
> 
> It should be easier than that. No introduction of other info needed --
> every global symbol without a ".type" or ".size" (i.e. SYM_*_END) should
> be a bug now.

I'm all for that!

Can we detect double ends as well - i.e. do a build check of the full syntax of 
these symbol definition primitives?

Thanks,

Ingo

[PATCH] drivers/staging/dgnc: Updating dev_dbg function

[Pushkar Jambhlekar]

Making use of __func__ instead of using function name directly in dev_dbg.

Signed-off-by: Pushkar Jambhlekar 
---
 drivers/staging/dgnc/dgnc_tty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index f7e754e..b0c88b8 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -1858,13 +1858,13 @@ static void dgnc_tty_send_xchar(struct tty_struct *tty, 
char c)
if (!bd || bd->magic != DGNC_BOARD_MAGIC)
return;
 
-   dev_dbg(tty->dev, "dgnc_tty_send_xchar start\n");
+   dev_dbg(tty->dev, "%s start\n", __func__);
 
spin_lock_irqsave(&ch->ch_lock, flags);
bd->bd_ops->send_immediate_char(ch, c);
spin_unlock_irqrestore(&ch->ch_lock, flags);
 
-   dev_dbg(tty->dev, "dgnc_tty_send_xchar finish\n");
+   dev_dbg(tty->dev, "%s finish\n", __func__);
 }
 
 /* Return modem signals to ld. */
-- 
2.7.4

Re: [PATCH v2] x86: mostly disable '-maccumulate-outgoing-args'

[Ingo Molnar]

* Josh Poimboeuf  wrote:

> The gcc '-maccumulate-outgoing-args' flag is enabled for most configs,
> mostly because of issues which are no longer relevant.  For most
> configs, and with most recent versions of gcc, it's no longer needed.
> 
> Clarify which cases need it, and only enable it for those cases.  Also
> produce a compile-time error for the ftrace graph + mcount + '-Os' case,
> which will otherwise cause runtime failures.
> 
> The main benefit of '-maccumulate-outgoing-args' is that it prevents an
> ugly prologue for functions which have aligned stacks.  But removing the
> option also has some benefits: more readable argument saves, smaller
> text size, and (presumably) slightly improved performance.
> 
> Here are the object size savings for 32-bit and 64-bit defconfig
> kernels:
> 
>   text   data bss  dechex filename
>   100067103543328 1773568 15323606 e9d1d6 vmlinux.x86-32.before
>97063583547424 1773568 15027350 e54c96 vmlinux.x86-32.after
> 
>   text   data bss  dechex filename
>   106521054537576  843776 16033457 f4a6b1 vmlinux.x86-64.before
>   106396294537576  843776 16020981 f475f5 vmlinux.x86-64.after
> 
> That comes out to a 3% text size improvement on x86-32 and a 0.1% text
> size improvement on x86-64.
> 
> Signed-off-by: Josh Poimboeuf 
> ---
> v2:
> - improve readability of the comments
> - add comment about why gcc version needs to be checked
> - use cc-option-yn instead of cc-option
> 
>  arch/x86/Makefile| 31 +++
>  arch/x86/Makefile_32.cpu | 18 --
>  arch/x86/kernel/ftrace.c |  6 ++
>  scripts/Kbuild.include   |  4 
>  4 files changed, 37 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 2d44933..04c87be 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -120,10 +120,6 @@ else
>  # -funit-at-a-time shrinks the kernel .text considerably
>  # unfortunately it makes reading oopses harder.
>  KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
> -
> -# this works around some issues with generating unwind tables in 
> older gccs
> -# newer gccs do it by default
> -KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
>  endif
>  
>  ifdef CONFIG_X86_X32
> @@ -147,6 +143,33 @@ ifeq ($(CONFIG_KMEMCHECK),y)
>   KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
>  endif
>  
> +# If the function graph tracer is used with mcount instead of fentry,
> +# '-maccumulate-outgoing-args' is needed to prevent a gcc bug
> +# (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109)
> +ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +  ifndef CONFIG_HAVE_FENTRY
> + ACCUMULATE_OUTGOING_ARGS := 1
> +  else
> +ifeq ($(call cc-option-yn, -mfentry), n)
> + ACCUMULATE_OUTGOING_ARGS := 1
> +endif
> +  endif
> +endif
> +
> +# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
> +# gcc bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226).  There's no 
> way
> +# to test for this bug at compile-time because the test case needs to 
> execute,
> +# which is a no-go for cross compilers.  So check the gcc version instead.

In documentation please refer to GCC the compiler as 'GCC' (upper case), while 
gcc 
the command as 'gcc'.

Also, even in Kbuild try to follow the kernel style - which for comments would 
be 
something like:

#
# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
# GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226).  There's no way
# to test for this bug at compile-time because the test case needs to execute,
# which is a no-go for cross compilers.  So check the GCC version instead.
#

... even though there's plenty of bad example in the Makefile you are changing.

> +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
> + !defined(CC_USING_FENTRY) && \
> + !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
> +# error Your compiler does not support function graph tracing
> +#endif

Might make sense to add the compiler option that is missing, i.e. something 
like:

  # error Your compiler does not support function graph tracing (-mfentry)

(or whatever compiler feature is missing.)

Thanks,

Ingo

Re: [PATCH v3 01/11] ARM: sun8i: a33: add operating-points-v2 property to all nodes

[Maxime Ripard]

On Wed, Mar 22, 2017 at 11:32:28AM +0800, Chen-Yu Tsai wrote:
> On Tue, Mar 21, 2017 at 11:36 PM, Quentin Schulz
>  wrote:
> > The OPP are declared as shared but no operating points are declared for
> > cpu1, 2 and 3. Thus, the following error happens during the boot:
> >
> > cpu cpu1: dev_pm_opp_of_get_sharing_cpus: Couldn't find tcpu_dev node.
> >
> > This patch applies the operating points to each cpu of the A33.
> >
> 
> This should have
> 
> Fixes: 03749eb88e63 ("ARM: dts: sun8i: add opp-v2 table for A33")
> 
> and be merged as fixes for 4.11.
> 
> > Signed-off-by: Quentin Schulz 
> 
> Acked-by: Chen-Yu Tsai 

I just queued it as a fix for 4.11, with the fixes tag.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

[PATCH 2/2] pinctrl: samsung: Remove unused pin-bank macro

[Chanwoo Choi]

After merged commit 1259feddd0f8 ("pinctrl: samsung: Fix the width
of PINCFG_TYPE_DRV bitfields for Exynos5433"), EXYNOS_PIN_BANK_EINTW
was replaced by EXYNOS5433_PIN_BANK_EINTW_EXT. This patch removes
the unused pin-bank macro (EXYNOS_PIN_BANK_EINTW).

Signed-off-by: Chanwoo Choi 
---
 drivers/pinctrl/samsung/pinctrl-exynos.h | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.h 
b/drivers/pinctrl/samsung/pinctrl-exynos.h
index a473092fb8d2..cd046eb7d705 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.h
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.h
@@ -79,17 +79,6 @@
.name   = id\
}
 
-#define EXYNOS_PIN_BANK_EINTW_EXT(pins, reg, id, offs, pctl_idx) \
-   {   \
-   .type   = &bank_type_alive, \
-   .pctl_offset= reg,  \
-   .nr_pins= pins, \
-   .eint_type  = EINT_TYPE_WKUP,   \
-   .eint_offset= offs, \
-   .name   = id,   \
-   .pctl_res_idx   = pctl_idx, \
-   }   \
-
 #define EXYNOS5433_PIN_BANK_EINTG(pins, reg, id, offs) \
{   \
.type   = &exynos5433_bank_type_off,\
-- 
1.9.1

Re: [PATCH v2 0/5] mm: support parallel free of memory

[Aaron Lu]

On Tue, Mar 21, 2017 at 07:54:37AM -0700, Dave Hansen wrote:
> On 03/16/2017 02:07 AM, Michal Hocko wrote:
> > On Wed 15-03-17 14:38:34, Tim Chen wrote:
> >> max_active:   time
> >> 1 8.9s   ±0.5%
> >> 2 5.65s  ±5.5%
> >> 4 4.84s  ±0.16%
> >> 8 4.77s  ±0.97%
> >> 164.85s  ±0.77%
> >> 326.21s  ±0.46%
> > 
> > OK, but this will depend on the HW, right? Also now that I am looking at
> > those numbers more closely. This was about unmapping 320GB area and
> > using 4 times more CPUs you managed to half the run time. Is this really
> > worth it? Sure if those CPUs were idle then this is a clear win but if
> > the system is moderately busy then it doesn't look like a clear win to
> > me.
> 
> This still suffers from zone lock contention.  It scales much better if
> we are freeing memory from more than one zone.  We would expect any
> other generic page allocator scalability improvements to really help
> here, too.
> 
> Aaron, could you make sure to make sure that the memory being freed is
> coming from multiple NUMA nodes?  It might also be interesting to boot

The test machine has 4 nodes and each has 128G memory.
With the test size of 320G, at least 3 nodes are involved.

But since the test is done on an idle system, I *guess* the allocated
memory is physically continuous. Then when they are freed in virtually
continuous order, it's likely that one after another physically continous
1G chunk are sent to the free kworkers. So roughly for the first
128 1G chunks, those workers will all be contending on the same zone.
(well, it shouldn't be 128 kworkers all runnable contending for the same
lock since early launched kworkers will have exited after finishing its
job before some later launched kworkers start).

> with a fake NUMA configuration with a *bunch* of nodes to see what the
> best case looks like when zone lock contention isn't even in play where
> one worker would be working on its own zone.

Good idea, will post results here once I finished the test.

Thanks.

Re: [PATCH v3 03/11] ARM: dts: sun8i: sina33: add cpu-supply

[Maxime Ripard]

On Wed, Mar 22, 2017 at 11:33:15AM +0800, Chen-Yu Tsai wrote:
> On Tue, Mar 21, 2017 at 11:36 PM, Quentin Schulz
>  wrote:
> > This adds the cpu-supply DT property to the cpu0 DT node needed by
> > the board to adapt the regulator voltage depending on the currently used
> > OPP.
> >
> > Signed-off-by: Quentin Schulz 
> 
> Acked-by: Chen-Yu Tsai 

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH] scsi: lpfc: fix linking against modular NVMe support

[Arnd Bergmann]

On Wed, Mar 22, 2017 at 3:25 AM, James Smart  wrote:
>
> On 3/21/2017 7:23 PM, James Smart wrote:
>>
>> Arnd,
>>
>> All of the build issues, including building as modules, should have been
>> resolved by the following patch:
>> http://www.spinics.net/lists/linux-scsi/msg106102.html
>>
>> Am I missing something ?
>
> Note: the patch I referenced
> (http://www.spinics.net/lists/linux-scsi/msg106102.html) replaced the one I
> think you referenced below
> (http://www.spinics.net/lists/linux-scsi/msg106024.html)

The build error I fixed was on yesterday's linux-next, which has the
http://www.spinics.net/lists/linux-scsi/msg106102.html patch, this is
the one I referenced as 7d7080335f8d ("scsi: lpfc: Finalize Kconfig
options for nvme").

The earlier version from linux-next-20170310 (there was no linux-next
last week) had the same bug but needed a different workaround:

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 4bf55b5d78be..52245eb83295 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -1256,12 +1256,14 @@ config SCSI_LPFC_DEBUG_FS
 config LPFC_NVME_INITIATOR
  bool "Emulex LightPulse Fibre Channel NVME Initiator Support"
  depends on SCSI_LPFC && NVME_FC
+ depends on SCSI_LPFC=m || NVME_FC=y
  ---help---
   This enables NVME Initiator support in the Emulex lpfc driver.

 config LPFC_NVME_TARGET
  bool "Emulex LightPulse Fibre Channel NVME Initiator Support"
  depends on SCSI_LPFC && NVME_TARGET_FC
+ depends on SCSI_LPFC=m || NVME_TARGET_FC=y
  ---help---
   This enables NVME Target support in the Emulex lpfc driver.
   Target enablement must still be enabled on a per adapter

As a side-note, the patch introduces a slightly unusual construct
using #if (IS_ENABLED(CONFIG_NVME_FC)). This can usually
expressed in a more readable way like this:

--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -2190,13 +2192,12 @@ lpfc_nvme_create_localport(struct lpfc_vport *vport)
 void
 lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
 {
-#if (IS_ENABLED(CONFIG_NVME_FC))
struct nvme_fc_local_port *localport;
struct lpfc_nvme_lport *lport;
struct lpfc_nvme_rport *rport = NULL, *rport_next = NULL;
int ret;

-   if (vport->nvmei_support == 0)
+   if (!IS_ENABLED(CONFIG_NVME_FC) || vport->nvmei_support == 0)
return;

localport = vport->localport;
@@ -2243,7 +2244,6 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport)
 "Failed, status x%x\n",
 ret);
}
-#endif
 }

 void


You could also use if(IS_REACHABLE()) here to work around
the link error when CONFIG_NVME_FC=m, but that would make
things a little more confusing for users as it is not immediately
clear why it fails to work at runtime.

  Arnd

[PATCH 0/2] pinctrl: samsung: Fix PINCFG_TYPE_DRV bitfields for Exynos5433

[Chanwoo Choi]

This patches add the missing part about PINCFG_TYPE_DRV bitfields
for Exynos5433 when merged the multiple patches for pinctrl-exynos.c
and this patch removes the unused EXYNOS_PIN_BANK_EINTW_EXT macro.

Also, patch1 and patch2 will be possible squashed if the separate patch
is not necesasry.

Chanwoo Choi (2):
  pinctrl: samsung: Add missing part for PINCFG_TYPE_DRV of Exynos5433
  pinctrl: samsung: Remove unused pin-bank macro

 drivers/pinctrl/samsung/pinctrl-exynos.c | 80 
 drivers/pinctrl/samsung/pinctrl-exynos.h | 11 -
 2 files changed, 40 insertions(+), 51 deletions(-)

-- 
1.9.1

[PATCH 1/2] pinctrl: samsung: Add missing part for PINCFG_TYPE_DRV of Exynos5433

[Chanwoo Choi]

The commit 1259feddd0f8("pinctrl: samsung: Fix the width of
PINCFG_TYPE_DRV bitfields for Exynos5433") already fixed
the different width of PINCFG_TYPE_DRV from previous Exynos SoC.

But, when merged the multiple patches, the missing parts occurred.
So, this patch adds the missing part using the EXYNOS5433_PIN_BANK_* macro.

Fixes: 7f36f5d11cda ("Merge tag 'v4.10-rc6' into devel")
Signed-off-by: Chanwoo Choi 
---
 drivers/pinctrl/samsung/pinctrl-exynos.c | 80 
 1 file changed, 40 insertions(+), 40 deletions(-)

diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c 
b/drivers/pinctrl/samsung/pinctrl-exynos.c
index f9b49967f512..63e51b56a22a 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -1468,82 +1468,82 @@ static void exynos_retention_disable(struct 
samsung_pinctrl_drv_data *drvdata)
 
 /* pin banks of exynos5433 pin-controller - ALIVE */
 static const struct samsung_pin_bank_data exynos5433_pin_banks0[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTW(8, 0x000, "gpa0", 0x00),
-   EXYNOS_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04),
-   EXYNOS_PIN_BANK_EINTW(8, 0x040, "gpa2", 0x08),
-   EXYNOS_PIN_BANK_EINTW(8, 0x060, "gpa3", 0x0c),
-   EXYNOS_PIN_BANK_EINTW_EXT(8, 0x020, "gpf1", 0x1004, 1),
-   EXYNOS_PIN_BANK_EINTW_EXT(4, 0x040, "gpf2", 0x1008, 1),
-   EXYNOS_PIN_BANK_EINTW_EXT(4, 0x060, "gpf3", 0x100c, 1),
-   EXYNOS_PIN_BANK_EINTW_EXT(8, 0x080, "gpf4", 0x1010, 1),
-   EXYNOS_PIN_BANK_EINTW_EXT(8, 0x0a0, "gpf5", 0x1014, 1),
+   EXYNOS5433_PIN_BANK_EINTW(8, 0x000, "gpa0", 0x00),
+   EXYNOS5433_PIN_BANK_EINTW(8, 0x020, "gpa1", 0x04),
+   EXYNOS5433_PIN_BANK_EINTW(8, 0x040, "gpa2", 0x08),
+   EXYNOS5433_PIN_BANK_EINTW(8, 0x060, "gpa3", 0x0c),
+   EXYNOS5433_PIN_BANK_EINTW_EXT(8, 0x020, "gpf1", 0x1004, 1),
+   EXYNOS5433_PIN_BANK_EINTW_EXT(4, 0x040, "gpf2", 0x1008, 1),
+   EXYNOS5433_PIN_BANK_EINTW_EXT(4, 0x060, "gpf3", 0x100c, 1),
+   EXYNOS5433_PIN_BANK_EINTW_EXT(8, 0x080, "gpf4", 0x1010, 1),
+   EXYNOS5433_PIN_BANK_EINTW_EXT(8, 0x0a0, "gpf5", 0x1014, 1),
 };
 
 /* pin banks of exynos5433 pin-controller - AUD */
 static const struct samsung_pin_bank_data exynos5433_pin_banks1[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTG(7, 0x000, "gpz0", 0x00),
-   EXYNOS_PIN_BANK_EINTG(4, 0x020, "gpz1", 0x04),
+   EXYNOS5433_PIN_BANK_EINTG(7, 0x000, "gpz0", 0x00),
+   EXYNOS5433_PIN_BANK_EINTG(4, 0x020, "gpz1", 0x04),
 };
 
 /* pin banks of exynos5433 pin-controller - CPIF */
 static const struct samsung_pin_bank_data exynos5433_pin_banks2[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTG(2, 0x000, "gpv6", 0x00),
+   EXYNOS5433_PIN_BANK_EINTG(2, 0x000, "gpv6", 0x00),
 };
 
 /* pin banks of exynos5433 pin-controller - eSE */
 static const struct samsung_pin_bank_data exynos5433_pin_banks3[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTG(3, 0x000, "gpj2", 0x00),
+   EXYNOS5433_PIN_BANK_EINTG(3, 0x000, "gpj2", 0x00),
 };
 
 /* pin banks of exynos5433 pin-controller - FINGER */
 static const struct samsung_pin_bank_data exynos5433_pin_banks4[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTG(4, 0x000, "gpd5", 0x00),
+   EXYNOS5433_PIN_BANK_EINTG(4, 0x000, "gpd5", 0x00),
 };
 
 /* pin banks of exynos5433 pin-controller - FSYS */
 static const struct samsung_pin_bank_data exynos5433_pin_banks5[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTG(6, 0x000, "gph1", 0x00),
-   EXYNOS_PIN_BANK_EINTG(7, 0x020, "gpr4", 0x04),
-   EXYNOS_PIN_BANK_EINTG(5, 0x040, "gpr0", 0x08),
-   EXYNOS_PIN_BANK_EINTG(8, 0x060, "gpr1", 0x0c),
-   EXYNOS_PIN_BANK_EINTG(2, 0x080, "gpr2", 0x10),
-   EXYNOS_PIN_BANK_EINTG(8, 0x0a0, "gpr3", 0x14),
+   EXYNOS5433_PIN_BANK_EINTG(6, 0x000, "gph1", 0x00),
+   EXYNOS5433_PIN_BANK_EINTG(7, 0x020, "gpr4", 0x04),
+   EXYNOS5433_PIN_BANK_EINTG(5, 0x040, "gpr0", 0x08),
+   EXYNOS5433_PIN_BANK_EINTG(8, 0x060, "gpr1", 0x0c),
+   EXYNOS5433_PIN_BANK_EINTG(2, 0x080, "gpr2", 0x10),
+   EXYNOS5433_PIN_BANK_EINTG(8, 0x0a0, "gpr3", 0x14),
 };
 
 /* pin banks of exynos5433 pin-controller - IMEM */
 static const struct samsung_pin_bank_data exynos5433_pin_banks6[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTG(8, 0x000, "gpf0", 0x00),
+   EXYNOS5433_PIN_BANK_EINTG(8, 0x000, "gpf0", 0x00),
 };
 
 /* pin banks of exynos5433 pin-controller - NFC */
 static const struct samsung_pin_bank_data exynos5433_pin_banks7[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTG(3, 0x000, "gpj0", 0x00),
+   EXYNOS5433_PIN_BANK_EINTG(3, 0x000, "gpj0", 0x00),
 };
 
 /* pin banks of exynos5433 pin-controller - PERIC */
 static const struct samsung_pin_bank_data exynos5433_pin_banks8[] __initconst 
= {
-   EXYNOS_PIN_BANK_EINTG(6, 0x000, "gpv7", 0x00),
-   EXYNOS_PIN_BANK_EINTG(5, 0x020, "gpb0", 0x04),
-   EXYNOS_PIN_BANK_EINTG(8, 0x040, "gpc0", 0x08),
-   EXYNOS_PIN_BANK_EINTG(2, 0x060, "gpc1", 0x0c),
-   EXYN

Re: [PATCH v8 2/3] dmaengine: pl330: remove pdata based initialization

[Marek Szyprowski]

Hi Vinod


On 2017-02-09 15:22, Marek Szyprowski wrote:

This driver is now used only on platforms which support device tree, so
it is safe to remove legacy platform data based initialization code.

Signed-off-by: Marek Szyprowski 
Reviewed-by: Ulf Hansson 
Acked-by: Arnd Bergmann 
For plat-samsung:
Acked-by: Krzysztof Kozlowski 


Vinod: This patch is completely independent from the rest of the changes
from that patchset. Could you apply it, or do you want me to resend it
separately? Runtime pm related changes will wait until a new DMA engine API
is ready.


---
  arch/arm/plat-samsung/devs.c |  1 -
  drivers/dma/pl330.c  | 42 --
  include/linux/amba/pl330.h   | 35 ---
  3 files changed, 8 insertions(+), 70 deletions(-)
  delete mode 100644 include/linux/amba/pl330.h

diff --git a/arch/arm/plat-samsung/devs.c b/arch/arm/plat-samsung/devs.c
index 03fac123676d..dc269d9143bc 100644
--- a/arch/arm/plat-samsung/devs.c
+++ b/arch/arm/plat-samsung/devs.c
@@ -10,7 +10,6 @@
   * published by the Free Software Foundation.
  */
  
-#include 

  #include 
  #include 
  #include 
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index f37f4978dabb..8b0da7fa520d 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -22,7 +22,6 @@
  #include 
  #include 
  #include 
-#include 
  #include 
  #include 
  #include 
@@ -2077,18 +2076,6 @@ static void pl330_tasklet(unsigned long data)
}
  }
  
-bool pl330_filter(struct dma_chan *chan, void *param)

-{
-   u8 *peri_id;
-
-   if (chan->device->dev->driver != &pl330_driver.drv)
-   return false;
-
-   peri_id = chan->private;
-   return *peri_id == (unsigned long)param;
-}
-EXPORT_SYMBOL(pl330_filter);
-
  static struct dma_chan *of_dma_pl330_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma)
  {
@@ -2833,7 +2820,6 @@ static int __maybe_unused pl330_resume(struct device *dev)
  static int
  pl330_probe(struct amba_device *adev, const struct amba_id *id)
  {
-   struct dma_pl330_platdata *pdat;
struct pl330_config *pcfg;
struct pl330_dmac *pl330;
struct dma_pl330_chan *pch, *_p;
@@ -2843,8 +2829,6 @@ static int __maybe_unused pl330_resume(struct device *dev)
int num_chan;
struct device_node *np = adev->dev.of_node;
  
-	pdat = dev_get_platdata(&adev->dev);

-
ret = dma_set_mask_and_coherent(&adev->dev, DMA_BIT_MASK(32));
if (ret)
return ret;
@@ -2857,7 +2841,7 @@ static int __maybe_unused pl330_resume(struct device *dev)
pd = &pl330->ddma;
pd->dev = &adev->dev;
  
-	pl330->mcbufsz = pdat ? pdat->mcbuf_sz : 0;

+   pl330->mcbufsz = 0;
  
  	/* get quirk */

for (i = 0; i < ARRAY_SIZE(of_quirks); i++)
@@ -2901,10 +2885,7 @@ static int __maybe_unused pl330_resume(struct device 
*dev)
INIT_LIST_HEAD(&pd->channels);
  
  	/* Initialize channel parameters */

-   if (pdat)
-   num_chan = max_t(int, pdat->nr_valid_peri, pcfg->num_chan);
-   else
-   num_chan = max_t(int, pcfg->num_peri, pcfg->num_chan);
+   num_chan = max_t(int, pcfg->num_peri, pcfg->num_chan);
  
  	pl330->num_peripherals = num_chan;
  
@@ -2916,11 +2897,8 @@ static int __maybe_unused pl330_resume(struct device *dev)
  
  	for (i = 0; i < num_chan; i++) {

pch = &pl330->peripherals[i];
-   if (!adev->dev.of_node)
-   pch->chan.private = pdat ? &pdat->peri_id[i] : NULL;
-   else
-   pch->chan.private = adev->dev.of_node;
  
+		pch->chan.private = adev->dev.of_node;

INIT_LIST_HEAD(&pch->submitted_list);
INIT_LIST_HEAD(&pch->work_list);
INIT_LIST_HEAD(&pch->completed_list);
@@ -2933,15 +2911,11 @@ static int __maybe_unused pl330_resume(struct device 
*dev)
list_add_tail(&pch->chan.device_node, &pd->channels);
}
  
-	if (pdat) {

-   pd->cap_mask = pdat->cap_mask;
-   } else {
-   dma_cap_set(DMA_MEMCPY, pd->cap_mask);
-   if (pcfg->num_peri) {
-   dma_cap_set(DMA_SLAVE, pd->cap_mask);
-   dma_cap_set(DMA_CYCLIC, pd->cap_mask);
-   dma_cap_set(DMA_PRIVATE, pd->cap_mask);
-   }
+   dma_cap_set(DMA_MEMCPY, pd->cap_mask);
+   if (pcfg->num_peri) {
+   dma_cap_set(DMA_SLAVE, pd->cap_mask);
+   dma_cap_set(DMA_CYCLIC, pd->cap_mask);
+   dma_cap_set(DMA_PRIVATE, pd->cap_mask);
}
  
  	pd->device_alloc_chan_resources = pl330_alloc_chan_resources;

diff --git a/include/linux/amba/pl330.h b/include/linux/amba/pl330.h
deleted file mode 100644
index fe93758e8403..
--- a/include/linux/amba/pl330.h
+++ /dev/null
@@ -1,35 +0,0 @@
-/* linux/include/linux/amba/pl330.h
- *
- * Copyrigh

Re: [PATCH] HID: Accutouch: Add driver for ELO Accutouch 2216 USB Touchscreens

[Jiri Kosina]

On Tue, 21 Mar 2017, Jiri Kosina wrote:

> Sorry, I messed up. I exchanged this patch with another one that fixed 
> report descriptor (which we can't yet, although I have been thinking about 
> ways how to do this properly).

FTR, this is now also dropped from hid.git. Thanks again for catching my 
brainfart, Dmitry.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v3 4/4] arm64: dts: rockchip: add regulator info for Kevin digitizer

[Heiko Stuebner]

Am Montag, 20. März 2017, 16:53:44 CET schrieb Brian Norris:
> We need to enable this regulator before the digitizer can be used. Wacom
> recommended waiting for 100 ms before talking to the HID.
> 
> Signed-off-by: Brian Norris 
> ---
> v3:
>  * reintroduced into patch set; bindings are in linux-next
> ---
>  arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts index
> 2f720cf2fd07..f4714c4606f3 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-kevin.dts
> @@ -232,6 +232,9 @@ ap_i2c_dig: &i2c2 {

according to the hid-over-i2c binding, this should use a special compatible 
like

compatible = "wacom,w9013", "hid-over-i2c"

As the binding patch is from Caesar, I assume that is the Kevin component and
I can add that myself, just need confirmation that this is actually the correct 
component :-) .


Thanks
Heiko

>   pinctrl-names = "default";
>   pinctrl-0 = <&cpu1_dig_irq_l &cpu1_dig_pdct_l>;
> 
> + vdd-supply = <&p3_3v_dig>;
> + post-power-on-delay-ms = <100>;
> +
>   interrupt-parent = <&gpio2>;
>   interrupts = <4 IRQ_TYPE_LEVEL_LOW>;

Re: [v2 PATCH 4/4] powernv: Recover correct PACA on wakeup from a stop on P9 DD1

[Nicholas Piggin]

On Wed, 22 Mar 2017 11:28:46 +0530
Gautham R Shenoy  wrote:

> On Tue, Mar 21, 2017 at 02:59:46AM +1000, Nicholas Piggin wrote:
> > On Mon, 20 Mar 2017 21:24:18 +0530
> > "Gautham R. Shenoy"  wrote:
> >   
> > > From: "Gautham R. Shenoy" 
> > > 
> > > POWER9 DD1.0 hardware has an issue due to which the SPRs of a thread
> > > waking up from stop 0,1,2 with ESL=1 can endup being misplaced in the
> > > core. Thus the HSPRG0 of a thread waking up from can contain the paca
> > > pointer of its sibling.
> > > 
> > > This patch implements a context recovery framework within threads of a
> > > core, by provisioning space in paca_struct for saving every sibling
> > > threads's paca pointers. Basically, we should be able to arrive at the
> > > right paca pointer from any of the thread's existing paca pointer.
> > > 
> > > At bootup, during powernv idle-init, we save the paca address of every
> > > CPU in each one its siblings paca_struct in the slot corresponding to
> > > this CPU's index in the core.
> > > 
> > > On wakeup from a stop, the thread will determine its index in the core
> > > from the lower 2 bits of the PIR register and recover its PACA pointer
> > > by indexing into the correct slot in the provisioned space in the
> > > current PACA.
> > > 
> > > Furthermore, ensure that the NVGPRs are restored from the stack on the
> > > way out by setting the NAPSTATELOST in paca.  
> > 
> > Thanks for expanding on this, it makes the patch easier to follow :)
> > 
> > As noted before, I think if we use PACA_EXNMI for system reset, then
> > *hopefully* there should be minimal races with the initial use of other
> > thread's PACA at the start of the exception. So I'll work on getting
> > that in, but it need not prevent this patch from being merged first
> > IMO.
> >   
> > > [Changelog written with inputs from sva...@linux.vnet.ibm.com]
> > > 
> > > Signed-off-by: Gautham R. Shenoy 
> > > ---
> > >  arch/powerpc/include/asm/paca.h   |  5 
> > >  arch/powerpc/kernel/asm-offsets.c |  1 +
> > >  arch/powerpc/kernel/idle_book3s.S | 49 
> > > ++-
> > >  arch/powerpc/platforms/powernv/idle.c | 22 
> > >  4 files changed, 76 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/powerpc/include/asm/paca.h 
> > > b/arch/powerpc/include/asm/paca.h
> > > index 708c3e5..4405630 100644
> > > --- a/arch/powerpc/include/asm/paca.h
> > > +++ b/arch/powerpc/include/asm/paca.h
> > > @@ -172,6 +172,11 @@ struct paca_struct {
> > >   u8 thread_mask;
> > >   /* Mask to denote subcore sibling threads */
> > >   u8 subcore_sibling_mask;
> > > + /*
> > > +  * Pointer to an array which contains pointer
> > > +  * to the sibling threads' paca.
> > > +  */
> > > + struct paca_struct *thread_sibling_pacas[8];  
> 
> > 
> > Is 8 the right number? I wonder if we have a define for it.  
> 
> Thats the maximum number of threads per core that we have had on POWER
> so far.
> 
> Perhaps, I can make this
> 
>struct paca_struct **thread_sibling_pacas;
> 
> and allocate threads_per_core number of slots in
> pnv_init_idle_states. Sounds ok ?

I guess that would minimise PACA overhead for non-DD1 machines,
so if it's not too much trouble, that might be good.


> > > +power9_dd1_recover_paca:
> > > + mfspr   r4, SPRN_PIR
> > > + clrldi  r4, r4, 62  
> > 
> > Does SPRN_TIR work?  
> 
> I wasn't aware of SPRN_TIR!
> 
> I can check this. If my reading of the ISA is correct, TIR should
> contain the thread number which are in the range [0..3].

Yep.


> > Reviewed-by: Nicholas Piggin 
> >   
> 
> Thanks for reviewing the patch.

No problems. Don't worry about the machine check wakeup for the moment
either. It's more important to just get the normal wakeup fix in I think.
We can revisit what to do there after my machine check patches go in
(idle machine check does not really work right now for POWER9 anyway).

Thanks,
Nick

[PATCH v2] staging: iio: Replace a bit shift by a use of BIT.

[Arushi Singhal]

This patch replaces bit shifting on 1 with the BIT(x) macro.
This was done with coccinelle:
@@
constant c;
@@

-1 << c
+BIT(c)

Signed-off-by: Arushi Singhal 
---
changes in v2
 -change the cc list

 drivers/staging/iio/adc/ad7816.c |  2 +-
 drivers/staging/iio/cdc/ad7150.c |  2 +-
 drivers/staging/iio/cdc/ad7746.c | 22 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c
index 17d280581e24..42f637ca0251 100644
--- a/drivers/staging/iio/adc/ad7816.c
+++ b/drivers/staging/iio/adc/ad7816.c
@@ -222,7 +222,7 @@ static ssize_t ad7816_show_value(struct device *dev,
value = (s8)((data >> AD7816_TEMP_FLOAT_OFFSET) - 103);
data &= AD7816_TEMP_FLOAT_MASK;
if (value < 0)
-   data = (1 << AD7816_TEMP_FLOAT_OFFSET) - data;
+   data = (BIT(AD7816_TEMP_FLOAT_OFFSET)) - data;
return sprintf(buf, "%d.%.2d\n", value, data * 25);
}
return sprintf(buf, "%u\n", data);
diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index ca72af3e9d4b..c5574b3ee939 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -232,7 +232,7 @@ static int ad7150_write_event_config(struct iio_dev 
*indio_dev,
if (ret < 0)
goto error_ret;
 
-   cfg = ret & ~((0x03 << 5) | (0x1 << 7));
+   cfg = ret & ~((0x03 << 5) | (BIT(7)));
 
switch (type) {
case IIO_EV_TYPE_MAG_ADAPTIVE:
diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index 81f8b9ee1120..8c573817826f 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -45,20 +45,20 @@
 #define AD7746_STATUS_RDYCAP   BIT(0)
 
 /* Capacitive Channel Setup Register Bit Designations (AD7746_REG_CAP_SETUP) */
-#define AD7746_CAPSETUP_CAPEN  (1 << 7)
-#define AD7746_CAPSETUP_CIN2   (1 << 6) /* AD7746 only */
-#define AD7746_CAPSETUP_CAPDIFF(1 << 5)
-#define AD7746_CAPSETUP_CACHOP (1 << 0)
+#define AD7746_CAPSETUP_CAPEN  (BIT(7))
+#define AD7746_CAPSETUP_CIN2   (BIT(6)) /* AD7746 only */
+#define AD7746_CAPSETUP_CAPDIFF(BIT(5))
+#define AD7746_CAPSETUP_CACHOP (BIT(0))
 
 /* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) */
-#define AD7746_VTSETUP_VTEN(1 << 7)
+#define AD7746_VTSETUP_VTEN(BIT(7))
 #define AD7746_VTSETUP_VTMD_INT_TEMP   (0 << 5)
-#define AD7746_VTSETUP_VTMD_EXT_TEMP   (1 << 5)
+#define AD7746_VTSETUP_VTMD_EXT_TEMP   (BIT(5))
 #define AD7746_VTSETUP_VTMD_VDD_MON(2 << 5)
 #define AD7746_VTSETUP_VTMD_EXT_VIN(3 << 5)
-#define AD7746_VTSETUP_EXTREF  (1 << 4)
-#define AD7746_VTSETUP_VTSHORT (1 << 1)
-#define AD7746_VTSETUP_VTCHOP  (1 << 0)
+#define AD7746_VTSETUP_EXTREF  (BIT(4))
+#define AD7746_VTSETUP_VTSHORT (BIT(1))
+#define AD7746_VTSETUP_VTCHOP  (BIT(0))
 
 /* Excitation Setup Register Bit Designations (AD7746_REG_EXC_SETUP) */
 #define AD7746_EXCSETUP_CLKCTRLBIT(7)
@@ -75,14 +75,14 @@
 #define AD7746_CONF_VTFS_MASK  GENMASK(7, 6)
 #define AD7746_CONF_CAPFS_MASK GENMASK(5, 3)
 #define AD7746_CONF_MODE_IDLE  (0 << 0)
-#define AD7746_CONF_MODE_CONT_CONV (1 << 0)
+#define AD7746_CONF_MODE_CONT_CONV (BIT(0))
 #define AD7746_CONF_MODE_SINGLE_CONV   (2 << 0)
 #define AD7746_CONF_MODE_PWRDN (3 << 0)
 #define AD7746_CONF_MODE_OFFS_CAL  (5 << 0)
 #define AD7746_CONF_MODE_GAIN_CAL  (6 << 0)
 
 /* CAPDAC Register Bit Designations (AD7746_REG_CAPDACx) */
-#define AD7746_CAPDAC_DACEN(1 << 7)
+#define AD7746_CAPDAC_DACEN(BIT(7))
 #define AD7746_CAPDAC_DACP(x)  ((x) & 0x7F)
 
 /*
-- 
2.11.0

[BUG][REGRESSION] i915 gpu hangs under load

[Martin Kepplinger]

Hi

I know something similar is here: 
https://bugs.freedesktop.org/show_bug.cgi?id=100110 too.


But this is rc3 and my machine is totally *not usable*. Let me be 
annoying :) I hope I can help:


Since rc1 I get gpu hangs and resets under load: This is almost 
certainly a kernel issue. 4.10 is fine.
I keep a debian stable userspace. nouveau is running on this machine 
too.


Mar 22 09:17:01 martin-laptop kernel: [ 2409.538706] [drm] GPU HANG: 
ecode 7:0:0xf3ce, in gnome-shell [1869], reason: Hang on render 
ring, action: reset
Mar 22 09:17:01 martin-laptop kernel: [ 2409.538711] [drm] GPU hangs can 
indicate a bug anywhere in the entire gfx stack, including userspace.
Mar 22 09:17:01 martin-laptop kernel: [ 2409.538713] [drm] Please file a 
_new_ bug report on bugs.freedesktop.org against DRI -> DRM/Intel
Mar 22 09:17:01 martin-laptop kernel: [ 2409.538714] [drm] drm/i915 
developers can then reassign to the right component if it's not a kernel 
issue.
Mar 22 09:17:01 martin-laptop kernel: [ 2409.538715] [drm] The gpu crash 
dump is required to analyze gpu hangs, so please always attach it.
Mar 22 09:17:01 martin-laptop kernel: [ 2409.538716] [drm] GPU crash 
dump saved to /sys/class/drm/card0/error
Mar 22 09:17:01 martin-laptop kernel: [ 2409.538768] drm/i915: Resetting 
chip after gpu hang
Mar 22 09:17:09 martin-laptop kernel: [ 2417.537886] drm/i915: Resetting 
chip after gpu hang
Mar 22 09:17:17 martin-laptop kernel: [ 2425.537152] drm/i915: Resetting 
chip after gpu hang
Mar 22 09:17:25 martin-laptop kernel: [ 2433.536407] drm/i915: Resetting 
chip after gpu hang
Mar 22 09:17:33 martin-laptop kernel: [ 2441.539674] drm/i915: Resetting 
chip after gpu hang



Furthermore, there are weird, small display distortions occuring. I 
don't get any log about them and
don't have a screenshot. Well. Nevermind. Please fix 4.11 and CC anyone 
I forgot.



thanks

 martin
GPU HANG: ecode 7:0:0xf3ce, in gnome-shell [1869], reason: Hang on render 
ring, action: reset
Kernel: 4.11.0-rc3-3-gbc61cd2
Time: 1490170621 s 524489 us
Boottime: 2409 s 756155 us
Uptime: 2395 s 323536 us
is_mobile: no
is_lp: no
is_alpha_support: no
has_64bit_reloc: no
has_aliasing_ppgtt: yes
has_csr: no
has_ddi: yes
has_decoupled_mmio: no
has_dp_mst: yes
has_fbc: yes
has_fpga_dbg: yes
has_full_ppgtt: yes
has_full_48bit_ppgtt: no
has_gmbus_irq: yes
has_gmch_display: no
has_guc: no
has_hotplug: yes
has_hw_contexts: yes
has_l3_dpf: yes
has_llc: yes
has_logical_ring_contexts: no
has_overlay: no
has_pipe_cxsr: no
has_pooled_eu: no
has_psr: yes
has_rc6: yes
has_rc6p: no
has_resource_streamer: yes
has_runtime_pm: yes
has_snoop: no
cursor_needs_physical: no
hws_needs_physical: no
overlay_needs_physical: no
supports_tv: no
Active process (on ring render): gnome-shell [1869], context bans 0
Reset count: 0
Suspend count: 0
Platform: HASWELL
PCI ID: 0x0416
PCI Revision: 0x06
PCI Subsystem: 10cf:17ac
IOMMU enabled?: 0
EIR: 0x
IER: 0xfc002529
GTIER: 0x00401821
PGTBL_ER: 0x
FORCEWAKE: 0x0001
DERRMR: 0x
CCID: 0x00ef410d
Missed interrupts: 0x
  fence[0] = 
  fence[1] = 
  fence[2] = 
  fence[3] = 
  fence[4] = 
  fence[5] = 
  fence[6] = 
  fence[7] = 
  fence[8] = 
  fence[9] = 
  fence[10] = 
  fence[11] = 
  fence[12] = 
  fence[13] = 
  fence[14] = 
  fence[15] = 
  fence[16] = 
  fence[17] = 
  fence[18] = 4b530770374a001
  fence[19] = 
  fence[20] = 
  fence[21] = 
  fence[22] = 
  fence[23] = 
  fence[24] = 
  fence[25] = 
  fence[26] = 
  fence[27] = 
  fence[28] = 
  fence[29] = 
  fence[30] = 
  fence[31] = 
ERROR: 0x0109
DONE_REG: 0x
ERR_INT: 0x
render command stream:
  START: 0x007ea000
  HEAD:  0x07a1f6dc [0x0001f648]
  TAIL:  0x0001f8f8 [0x0001f728, 0x0001f760]
  CTL:   0x0001f001
  MODE:  0x4000
  HWS:   0x7fff
  ACTHD: 0x 07a1f6dc
  IPEIR: 0x
  IPEHR: 0x0c00
  INSTDONE: 0xffce
  SC_INSTDONE: 0x
  SAMPLER_INSTDONE[0][0]: 0x
  ROW_INSTDONE[0][0]: 0x
  BBADDR: 0x_7fa48330
  BB_STATE: 0x
  INSTPS: 0x0500
  INSTPM: 0x6080
  FADDR: 0x 008096d8
  RC PSMI: 0x0010
  FAULT_REG: 0x00c5
  SYNC_0: 0x
  SYNC_1: 0x0001c2a1
  SYNC_2: 0x
  GFX_MODE: 0x2a00
  PP_DIR_BASE: 0x7fdf
  seqno: 0x0001c29a
  last_seqno: 0x0001c2a2
  waiting: yes
  ring->head: 0x00016e60
  ring->tail: 0x0001f8f8
  hangcheck stall: yes
  hangcheck action: dead
  hangcheck action timestamp: 4295493232, 204600 ms ago
blt command stream:
  START: 0x0080a000
  HEAD:  0x07e0e8d0 [0x]
  TAIL:  0xe8d0 [0x, 0x]
  CTL:   0x0001f001
  MODE:  0x0200
  HWS:   0x7fff1000
  ACTHD: 0x 07e0e8d0
  IPEIR: 0x
  IPEHR: 0

Re: [PATCH v2 3/5] mm: use a dedicated workqueue for the free workers

[Aaron Lu]

On Wed, Mar 22, 2017 at 03:33:35PM +0900, Minchan Kim wrote:
> Hi,
> 
> On Wed, Mar 15, 2017 at 05:00:02PM +0800, Aaron Lu wrote:
> > Introduce a workqueue for all the free workers so that user can fine
> > tune how many workers can be active through sysfs interface: max_active.
> > More workers will normally lead to better performance, but too many can
> > cause severe lock contention.
> 
> Let me ask a question.
> 
> How well can workqueue distribute the jobs in multiple CPU?

I would say it's good enough for my needs.
After all, it doesn't need many kworkers to achieve the 50% time
decrease: 2-4 kworkers for EP and 4-8 kworkers for EX are enough from
previous attched data.

> I don't ask about currency but parallelism.
> I guess benefit you are seeing comes from the parallelism and
> for your goal, unbound wq should spawn a thread per cpu and
> doing the work in every each CPU. does it work?

I don't think a unbound workqueue will spawn a thread per CPU, that
seems too much a cost to have a unbound workqueue.

My understanding of the unbound workqueue is that it will create a
thread pool for each node, versus each CPU as in the bound workqueue
case, and use threads from the thread pool(create threads if not enough)
to do the work.

I guess you want to ask if the unbound workqueue can spawn enough
threads to do the job? From the output of 'vmstat 1' during the free()
test, I can see some 70+ processes in runnable state when I didn't
set an upper limit for max_active of the workqueue.

Thanks,
Aaron

Re: [PATCH RT] tty: serial: st-asc: Make the locking RT aware

[Lionel DEBIEVE]

Just to agree with Thomas.

Do you want me to resend the patch without RT tag?

BR,
Lionel


On 03/21/2017 09:15 PM, Steven Rostedt wrote:
> On Tue, 21 Mar 2017 19:51:47 +0100 (CET)
> Thomas Gleixner  wrote:
>
>> On Tue, 21 Mar 2017, Steven Rostedt wrote:
/*
 * Disable interrupts so we don't get the IRQ line
 bouncing
>>> I'm nervous about the above comment, which in full is:
>>>
>>> /*
>>>  * Disable interrupts so we don't get the IRQ line bouncing
>>>  * up and down while interrupts are disabled.
>>>  */
>>>
>>> I'm not sure if disabling interrupts helps on an SMP system. This
>>> patch does change what happens when port->sysrq is set. But I'm not
>>> sure we care.
>> It disables interrupts at the device level which obviously helps
>> whether on SMP or not.
>>
> OK, so this has nothing to do with the local_irq_save() that is being
> removed, which would be fine then.
>
>
> -- Steve
>

Re: [PATCH 1/5] x86: Implement __WARN using UD0

[Peter Zijlstra]

A little like so then.

---
Subject: x86: Implement __WARN using UD0
From: Peter Zijlstra 
Date: Thu Feb 2 14:43:51 CET 2017

By using "UD0" for WARNs we remove the function call and its possible
__FILE__ and __LINE__ immediate arguments from the instruction stream.

Total image size will not change much, what we win in the instruction
stream we'll loose because of the __bug_table entries. Still, saves on
I$ footprint and the total image size does go down a bit.

   textdata bss dec hex filename
107021234530992  843776 16076891 f5505b 
defconfig-build/vmlinux.0
106824604530992  843776 16057228 f5038c 
defconfig-build/vmlinux.1

(um didn't seem to use GENERIC_BUG at all, so remove it)

Cc: Linus Torvalds 
Cc: Richard Weinberger 
Cc: Arjan van de Ven 
Cc: Borislav Petkov 
Cc: Josh Poimboeuf 

Signed-off-by: Peter Zijlstra (Intel) 
---
 arch/um/Kconfig.common |5 --
 arch/x86/include/asm/bug.h |   73 +++--
 arch/x86/kernel/dumpstack.c|3 -
 arch/x86/kernel/dumpstack_32.c |   12 --
 arch/x86/kernel/dumpstack_64.c |   10 -
 arch/x86/kernel/traps.c|   46 ++---
 arch/x86/um/Makefile   |2 -
 arch/x86/um/bug.c  |   21 ---
 8 files changed, 97 insertions(+), 75 deletions(-)

--- a/arch/um/Kconfig.common
+++ b/arch/um/Kconfig.common
@@ -50,11 +50,6 @@ config GENERIC_CALIBRATE_DELAY
bool
default y
 
-config GENERIC_BUG
-   bool
-   default y
-   depends on BUG
-
 config HZ
int
default 100
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -1,36 +1,75 @@
 #ifndef _ASM_X86_BUG_H
 #define _ASM_X86_BUG_H
 
+#include 
+
 #define HAVE_ARCH_BUG
 
-#ifdef CONFIG_DEBUG_BUGVERBOSE
+/*
+ * Since some emulators terminate on UD2, we cannot use it for WARN.
+ * Since various instruction decoders disagree on the length of UD1,
+ * we cannot use it either. So use UD0 for WARN.
+ *
+ * (binutils knows about "ud1" but {en,de}codes it as 2 bytes, whereas
+ *  our kernel decoder thinks it takes a ModRM byte, which seems consistent
+ *  with various things like the Intel SDM instruction encoding rules)
+ */
+
+#define ASM_UD0".byte 0x0f, 0xff"
+#define ASM_UD1".byte 0x0f, 0xb9" /* + ModRM */
+#define ASM_UD2".byte 0x0f, 0x0b"
+
+#define INSN_UD0   0xff0f
+#define INSN_UD2   0x0b0f
+
+#define LEN_UD02
 
 #ifdef CONFIG_X86_32
-# define __BUG_C0  "2:\t.long 1b, %c0\n"
+# define __BUG_REL(val)".long " __stringify(val)
 #else
-# define __BUG_C0  "2:\t.long 1b - 2b, %c0 - 2b\n"
+# define __BUG_REL(val)".long " __stringify(val) " - 2b"
 #endif
 
-#define BUG()  \
-do {   \
-   asm volatile("1:\tud2\n"\
-".pushsection __bug_table,\"a\"\n" \
-__BUG_C0   \
-"\t.word %c1, 0\n" \
-"\t.org 2b+%c2\n"  \
-".popsection"  \
-: : "i" (__FILE__), "i" (__LINE__),\
-"i" (sizeof(struct bug_entry)));   \
-   unreachable();  \
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+
+#define _BUG_FLAGS(ins, flags) \
+do {   \
+   asm volatile("1:\t" ins "\n"\
+".pushsection __bug_table,\"a\"\n" \
+"2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"   \
+"\t"  __BUG_REL(%c0) "\t# bug_entry::file\n"   \
+"\t.word %c1""\t# bug_entry::line\n"   \
+"\t.word %c2""\t# bug_entry::flags\n"  \
+"\t.org 2b+%c3\n"  \
+".popsection"  \
+: : "i" (__FILE__), "i" (__LINE__),\
+"i" (flags),   \
+"i" (sizeof(struct bug_entry)));   \
 } while (0)
 
-#else
+#else /* !CONFIG_DEBUG_BUGVERBOSE */
+
+#define _BUG_FLAGS(ins, flags) \
+do {   \
+   asm volatile("1:\t" ins "\n"\
+".pushsection __bug_table,\"a\"\n" \
+"2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"   \
+"\t.word %c0""\t# bug_entry::flag

Re: [PATCH v3 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

[Xunlei Pang]

On 03/21/2017 at 11:33 AM, Eric W. Biederman wrote:
> Xunlei Pang  writes:
>
>> As Eric said,
>> "what we need to do is move the variable vmcoreinfo_note out
>> of the kernel's .bss section.  And modify the code to regenerate
>> and keep this information in something like the control page.
>>
>> Definitely something like this needs a page all to itself, and ideally
>> far away from any other kernel data structures.  I clearly was not
>> watching closely the data someone decided to keep this silly thing
>> in the kernel's .bss section."
>>
>> This patch allocates extra pages for these vmcoreinfo_XXX variables,
>> one advantage is that it enhances some safety of vmcoreinfo, because
>> vmcoreinfo now is kept far away from other kernel data structures.
> Can you preceed this patch with a patch that removes CRASHTIME from
> vmcoreinfo?  If someone actually cares we can add a separate note that holds
> a 64bit crashtime in the per cpu notes.  

Hi Eric,

Thanks for your review, I took some time and did some investigation.

Removing "CRASHTIME=X" from vmcoreinfo_note will break user-space tools.
For example, makedumpfile gets vmcoreinfo note information by reading
"/sys/kernel/vmcoreinfo"  its PA, then get its "VA = PA | PAGE_OFFSET",
and then get the timestamp. This operates in the first kernel even before
kdump is loaded.

Actually, even moving vmcoreinfo_note[] into the crash memory, it
may have problems, for example, on s390 system the crash memory
range will be unmapped, so I guess it may cause some risks.

Additionally, there is no available way for us to allocate a page from the
crash memory during kernel initialization, we only can achieve this during
the kexec syscalls. There is not a neat way to implement a function to
allocate pages from the crash memory during kernel initialization without
some hack code added, because user-space tools(like kexec-tools) can
allocate the crash segment by their own ways from the crash memory.

That's why I only copy vmcoreinfo_data[] into the crash memory, and
not touch vmcoreinfo_note, so vmcoreinfo_data is well protected in
the crash memory copy, then in crash_save_vmcoreinfo(), we copy
this guaranteed copy into vmcoreinfo_note[], so the correctness of
vmcoreinfo_note[] is guaranteed. This is what [PATCH v3 3/3] does.

The current crash_save_vmcoreinfo() only involves memory(memcpy)
operations even for get_seconds(no locks), the only risk I can think
of now is that vmcoreinfo_note pointer may be corrupted. If it is a concern,
I guess we can put it into struct kimage" just like vmcoreinfo_XXX_copy
in this patch. After all if kimage structure was corrupted when crash happens,
we can do nothing but have to accept the fate.

So does it really deserve to eliminate crash_save_vmcoreinfo()?

Regards,
Xunlei

>
> As we are looking at reliability concerns removing CRASHTIME should make
> everything in vmcoreinfo a boot time constant.  Which should simplify
> everything considerably.
>
> Which means we only need to worry abou the per-cpu notes being written
> at the time of a crash.
>
>> Suggested-by: Eric Biederman 
>> Signed-off-by: Xunlei Pang 
>> ---
>>  arch/ia64/kernel/machine_kexec.c |  5 -
>>  arch/x86/kernel/crash.c  |  2 +-
>>  include/linux/kexec.h|  2 +-
>>  kernel/kexec_core.c  | 29 -
>>  kernel/ksysfs.c  |  2 +-
>>  5 files changed, 27 insertions(+), 13 deletions(-)
>>
>> diff --git a/arch/ia64/kernel/machine_kexec.c 
>> b/arch/ia64/kernel/machine_kexec.c
>> index 599507b..c14815d 100644
>> --- a/arch/ia64/kernel/machine_kexec.c
>> +++ b/arch/ia64/kernel/machine_kexec.c
>> @@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void)
>>  #endif
>>  }
>>  
>> -phys_addr_t paddr_vmcoreinfo_note(void)
>> -{
>> -return ia64_tpa((unsigned long)(char *)&vmcoreinfo_note);
>> -}
>> -
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 3741461..4d35fbb 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -456,7 +456,7 @@ static int prepare_elf64_headers(struct crash_elf_data 
>> *ced,
>>  bufp += sizeof(Elf64_Phdr);
>>  phdr->p_type = PT_NOTE;
>>  phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
>> -phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note);
>> +phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>>  (ehdr->e_phnum)++;
>>  
>>  #ifdef CONFIG_X86_64
>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>> index e98e546..f1c601b 100644
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -317,7 +317,7 @@ extern void *kexec_purgatory_get_symbol_addr(struct 
>> kimage *image,
>>  extern struct resource crashk_low_res;
>>  typedef u32 note_buf_t[KEXEC_NOTE_BYTES/4];
>>  extern note_buf_t __percpu *crash_notes;
>> -extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
>> +extern u32 *vmcoreinfo_note;
>>  extern size_t vmcoreinfo_size;
>>  extern size_t vmcoreinfo_max_size;
>>  
>> 

Re: staging: iio: Replace a bit shift by a use of BIT.

[Arnd Bergmann]

On Wed, Mar 22, 2017 at 3:49 AM, Arushi Singhal
 wrote:

> @@ -232,7 +232,7 @@ static int ad7150_write_event_config(struct iio_dev 
> *indio_dev,
> if (ret < 0)
> goto error_ret;
>
> -   cfg = ret & ~((0x03 << 5) | (0x1 << 7));
> +   cfg = ret & ~((0x03 << 5) | (BIT(7)));
>
> switch (type) {
> case IIO_EV_TYPE_MAG_ADAPTIVE:

While the change in the #defines is not unusual, this particular change
here makes it harder to read, so I'd recommend leaving it alone, or changing
it to use symbolic names instead of the bit masks.

>  /* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) 
> */
> -#define AD7746_VTSETUP_VTEN(1 << 7)
> +#define AD7746_VTSETUP_VTEN(BIT(7))
>  #define AD7746_VTSETUP_VTMD_INT_TEMP   (0 << 5)
> -#define AD7746_VTSETUP_VTMD_EXT_TEMP   (1 << 5)
> +#define AD7746_VTSETUP_VTMD_EXT_TEMP   (BIT(5))

Why did you leave one of them unchanged?

Arnd

Re: [PATCH v2 3/5] mm: use a dedicated workqueue for the free workers

[Minchan Kim]

On Wed, Mar 22, 2017 at 04:41:04PM +0800, Aaron Lu wrote:
> On Wed, Mar 22, 2017 at 03:33:35PM +0900, Minchan Kim wrote:
> > Hi,
> > 
> > On Wed, Mar 15, 2017 at 05:00:02PM +0800, Aaron Lu wrote:
> > > Introduce a workqueue for all the free workers so that user can fine
> > > tune how many workers can be active through sysfs interface: max_active.
> > > More workers will normally lead to better performance, but too many can
> > > cause severe lock contention.
> > 
> > Let me ask a question.
> > 
> > How well can workqueue distribute the jobs in multiple CPU?
> 
> I would say it's good enough for my needs.
> After all, it doesn't need many kworkers to achieve the 50% time
> decrease: 2-4 kworkers for EP and 4-8 kworkers for EX are enough from
> previous attched data.
> 
> > I don't ask about currency but parallelism.
> > I guess benefit you are seeing comes from the parallelism and
> > for your goal, unbound wq should spawn a thread per cpu and
> > doing the work in every each CPU. does it work?
> 
> I don't think a unbound workqueue will spawn a thread per CPU, that
> seems too much a cost to have a unbound workqueue.
> 
> My understanding of the unbound workqueue is that it will create a
> thread pool for each node, versus each CPU as in the bound workqueue
> case, and use threads from the thread pool(create threads if not enough)
> to do the work.

Yes, that was my understand so I read code and found that

insert_work:
..
if (__need_more_worker(pool))
wake_up_worker(pool); 

so I thought if there is a running thread in that node, workqueue
will not wake any other threads so parallelism should be max 2.
AFAIK, if the work goes sleep, scheduler will spawn new worker
thread so the active worker could be a lot but I cannot see any
significant sleepable point in that work(ie, batch_free_work).

> 
> I guess you want to ask if the unbound workqueue can spawn enough
> threads to do the job? From the output of 'vmstat 1' during the free()
> test, I can see some 70+ processes in runnable state when I didn't
> set an upper limit for max_active of the workqueue.

Hmm, it seems I was wrong. I am really curious how we can have
70+ active workers in that. Could you explain what I am missing?

Thanks.

> 
> Thanks,
> Aaron
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org";> em...@kvack.org 

[PATCH] arm64: kaslr: Add 2MB correction for aligning kernel image

[Srinivas Ramana]

From: Neeraj Upadhyay 

If kernel image extends across alignment boundary, existing
code increases the KASLR offset by size of kernel image. The
offset is masked after resizing. There are cases, where after
masking, we may still have kernel image extending across
boundary. This eventually results in only 2MB block getting
mapped while creating the page tables. This results in data aborts
while accessing unmapped regions during second relocation (with
kaslr offset) in __primary_switch. To fix this problem, add a
2MB correction to offset along with the correction of kernel
image size, before applying mask.

For example consider below case, where kernel image still crosses
1GB alignment boundary, after masking the offset, which is fixed
by adding 2MB correction.

SWAPPER_TABLE_SHIFT = 30
Swapper using section maps with section size 2MB.
CONFIG_PGTABLE_LEVELS = 3
VA_BITS = 39

_text  : 0xff800808
_end   : 0xff800aa1b000
offset : 0x1f3560
mask = ((1UL << (VA_BITS - 2)) - 1) & ~(SZ_2M - 1)

(_text + offset) >> SWAPPER_TABLE_SHIFT = 0x3fe7c
(_end + offset) >> SWAPPER_TABLE_SHIFT  = 0x3fe7d

offset after existing correction (before mask) = 0x1f37f9b000
(_text + offset) >> SWAPPER_TABLE_SHIFT = 0x3fe7d
(_end + offset) >> SWAPPER_TABLE_SHIFT  = 0x3fe7d

offset (after mask) = 0x1f37e0
(_text + offset) >> SWAPPER_TABLE_SHIFT = 0x3fe7c
(_end + offset) >> SWAPPER_TABLE_SHIFT  = 0x3fe7d

new offset w/ 2MB correction (before mask) = 0x1f37819b00
new offset w/ 2MB correction (after mask) = 0x1f3800
(_text + offset) >> SWAPPER_TABLE_SHIFT = 0x3fe7d
(_end + offset) >> SWAPPER_TABLE_SHIFT  = 0x3fe7d

Fixes: f80fb3a3d508 ("arm64: add support for kernel ASLR")
Signed-off-by: Neeraj Upadhyay 
Signed-off-by: Srinivas Ramana 
---
 arch/arm64/kernel/kaslr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 769f24ef628c..7b8af985e497 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -135,7 +135,7 @@ u64 __init kaslr_early_init(u64 dt_phys, u64 modulo_offset)
 */
if u64)_text + offset + modulo_offset) >> SWAPPER_TABLE_SHIFT) !=
(((u64)_end + offset + modulo_offset) >> SWAPPER_TABLE_SHIFT))
-   offset = (offset + (u64)(_end - _text)) & mask;
+   offset = (offset + (u64)(_end - _text) + SZ_2M) & mask;
 
if (IS_ENABLED(CONFIG_KASAN))
/*
-- 
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc., 
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v3 1/4] dt-bindings: Document rk3399 Gru/Kevin

[Heiko Stuebner]

Am Montag, 20. März 2017, 16:53:41 CET schrieb Brian Norris:
> Gru is a base dev board for a family of devices, including Kevin. Both
> utilize Rockchip RK3399, and they share much of their design.
> 
> Signed-off-by: Brian Norris 
> Acked-by: Rob Herring 

applied for 4.12, after adding a "(Samsung Chromebook Plus)" hint, as that may 
help people identifying the correct dts and also is public knowledge (some 
blogs but also /proc/device-tree/compatible on the device itself).


Heiko

> ---
> v3:
>  * added Rob's ack
> ---
>  Documentation/devicetree/bindings/arm/rockchip.txt | 20
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/rockchip.txt
> b/Documentation/devicetree/bindings/arm/rockchip.txt index
> 226ee9131078..a18308c5bdf1 100644
> --- a/Documentation/devicetree/bindings/arm/rockchip.txt
> +++ b/Documentation/devicetree/bindings/arm/rockchip.txt
> @@ -59,6 +59,17 @@ Rockchip platforms device tree bindings
>- compatible = "google,veyron-brain-rev0", "google,veyron-brain",
>"google,veyron", "rockchip,rk3288";
> 
> +- Google Gru (dev-board):
> +Required root node properties:
> +  - compatible = "google,gru-rev15", "google,gru-rev14",
> +  "google,gru-rev13", "google,gru-rev12",
> +  "google,gru-rev11", "google,gru-rev10",
> +  "google,gru-rev9", "google,gru-rev8",
> +  "google,gru-rev7", "google,gru-rev6",
> +  "google,gru-rev5", "google,gru-rev4",
> +  "google,gru-rev3", "google,gru-rev2",
> +  "google,gru", "rockchip,rk3399";
> +
>  - Google Jaq (Haier Chromebook 11 and more):
>  Required root node properties:
>- compatible = "google,veyron-jaq-rev5", "google,veyron-jaq-rev4",
> @@ -73,6 +84,15 @@ Rockchip platforms device tree bindings
>"google,veyron-jerry-rev3", "google,veyron-jerry",
>"google,veyron", "rockchip,rk3288";
> 
> +- Google Kevin:
> +Required root node properties:
> +  - compatible = "google,kevin-rev15", "google,kevin-rev14",
> +  "google,kevin-rev13", "google,kevin-rev12",
> +  "google,kevin-rev11", "google,kevin-rev10",
> +  "google,kevin-rev9", "google,kevin-rev8",
> +  "google,kevin-rev7", "google,kevin-rev6",
> +  "google,kevin", "google,gru", "rockchip,rk3399";
> +
>  - Google Mickey (Asus Chromebit CS10):
>  Required root node properties:
>- compatible = "google,veyron-mickey-rev8",
> "google,veyron-mickey-rev7",

Re: [PATCH v3] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability

[David Hildenbrand]

On 22.03.2017 02:19, Wanpeng Li wrote:
> From: Wanpeng Li 
> 
> This can be reproduced by running L2 on L1, and disable VPID on L0 if w/o 
> commit "KVM: nVMX: Fix nested VPID vmx exec control", the L2 crash as below:
> 
> KVM: entry failed, hardware error 0x7
> EAX= EBX= ECX= EDX=000306c3
> ESI= EDI= EBP= ESP=
> EIP=fff0 EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =   9300
> CS =f000   9b00
> SS =   9300
> DS =   9300
> FS =   9300
> GS =   9300
> LDT=   8200
> TR =   8b00
> GDT=  
> IDT=  
> CR0=6010 CR2= CR3= CR4=
> DR0= DR1= DR2= 
> DR3= 
> DR6=0ff0 DR7=0400
> EFER=
> 
> Reference SDM 30.3 INVVPID:
>  
> Protected Mode Exceptions
> #UD 
>   - If not in VMX operation.
>   - If the logical processor does not support VPIDs 
> (IA32_VMX_PROCBASED_CTLS2[37]=0).
>   - If the logical processor supports VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=1) 
> but does 
> not support the INVVPID instruction (IA32_VMX_EPT_VPID_CAP[32]=0).
> 
> So we should check both VPID enable bit in vmx exec control and INVVPID 
> support bit 
> in vmx capability MSRs to enable VPID. This patch adds the guarantee to not 
> enable VPID
> if INVVPID is not exposed in vmx capability MSRs.
> 
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Signed-off-by: Wanpeng Li 
> ---
>  arch/x86/kvm/vmx.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 8795a70..f2b912e 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
>   return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
>  }
>  
> +static inline bool cpu_has_vmx_invvpid(void)
> +{
> + return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
> +}
> +
>  static inline bool cpu_has_vmx_ept(void)
>  {
>   return vmcs_config.cpu_based_2nd_exec_ctrl &
> @@ -6518,8 +6523,9 @@ static __init int hardware_setup(void)
>   if (boot_cpu_has(X86_FEATURE_NX))
>   kvm_enable_efer_bits(EFER_NX);
>  
> - if (!cpu_has_vmx_vpid())
> + if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid())
>   enable_vpid = 0;
> +
>   if (!cpu_has_vmx_shadow_vmcs())
>   enable_shadow_vmcs = 0;
>   if (enable_shadow_vmcs)
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David

Re: [PATCH v5 1/2] usb: host: plat: Enable xhci plat runtime PM

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> I don't yet understand why we can't just keep runtime pm disabled as a
> default for xhci platform devices.
> It could be enabled by whatever creates the platform device by setting 
> some
> device property
> (or equivalent), which would be checked in xhci_plat_probe() before 
> enabling
> runtime pm. It
> could then optionally be set in sysfs via power/control entry.

 I think runtime pm is not one hardware property, is it suitable if we
 introduce one device property to enable/disable runtime pm?
>>>
>>> As I said, runtime pm is not one hardware property, I think it is not
>>> suitable if we introduce one device property to enable/disable runtime
>>> pm.
>>
>> we already this functionality exposed on sysfs.
>
> From my understanding, Mathias suggested me to add one device property
> (name like "usb-host-runtimePM") by platform_device_add_properties()
> to enable/disable runtime PM when creating platform device, like
> usb3_lpm_capable:
>
> if (dwc->usb3_lpm_capable)
>   props[prop_idx++].name = "usb3-lpm-capable";
>
> ret = platform_device_add_properties(xhci, props);
> if (ret) {
>   dev_err(dwc->dev, "failed to add properties to xHCI\n");
>   goto err1;
> }
>
> What I think It is not suitable to introduce one device property like
> above to enable/disable runtime PM, it is not one hardware attribute.

yeah, that's silly. We already have means for doing that:

my_probe()
{
[...]

pm_runtime_enable(dev);
pm_runtime_forbid(dev);

[...]

return 0;
}

>>> Secondly, we only can resume the xhci platform device by getting the
>>> xhci usage counter from gadget driver, since the cable plug in/out
>>> events only can be notified to glue layer of gadget driver(like dwc3
>>> glue layer). That means if we want to suspend xhci platform device, we
>>
>> this is a problem with the glue layer, IMO. It should be something like
>> so:
>>
>> static irqreturn_t dwc3_foobar_wakeup(int irq, void *_glue)
>> {
>> struct dwc3_foobar_glue *glue = _glue;
>> u32 reg;
>>
>> reg = real(glue->base, OFFSET);
>> if (reg & CONNECT)
>> pm_runtime_resume(&glue->dwc3);
>>
>> return IRQ_HANDLED;
>> }
>>
>> then dwc3's ->runtime_resume() should check if the event is supposed to
>> be handled by host or peripheral by checking which mode it was before
>> suspend and making the assumption that we don't change modes while
>> suspended. Something like:
>>
>> static int dwc3_runtime_resume(struct device *dev)
>> {
>> struct dwc3 *dwc = dev_get_drvdata(dev);
>>
>> [...]
>>
>> if (dwc->is_host)
>> pm_runtime_resume(dwc->xhci.dev);
>>
>> [...]
>>
>> return 0;
>> }
>
> Yeah, if we don't need to get xhci usage counter in xhci_plat_probe()
> to avoid affecting other controller's runtime PM, we can do like this
> and do not need to get/put counter.

why do you need to get xhci's usage counter in xhci_plat_probe() ?

And why would one xhci affect the other? They are different struct
device instances and, thus, have different pm usage counter. How would
one xhci's pm_runtime affect another?

PM runtime is not per driver, it's per device.

-- 
balbi


signature.asc
Description: PGP signature

Re: [2/2] zd1211rw: fix NULL-deref at probe

[Kalle Valo]

Johan Hovold  wrote:
> Make sure to check the number of endpoints to avoid dereferencing a
> NULL-pointer or accessing memory beyond the endpoint array should a
> malicious device lack the expected endpoints.
> 
> Fixes: a1030e92c150 ("[PATCH] zd1211rw: Convert installer CDROM device into 
> WLAN device")
> Cc: Daniel Drake 
> Signed-off-by: Johan Hovold 

Patch applied to wireless-drivers-next.git, thanks.

ca260ece6a57 zd1211rw: fix NULL-deref at probe

-- 
https://patchwork.kernel.org/patch/9620721/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 2/2 v2] xen/acpi: upload PM state from init-domain to Xen

[Stanislaw Gruszka]

On Tue, Mar 21, 2017 at 03:43:38PM -0700, Ankur Arora wrote:
> This was broken in commit cd979883b9ede90643e019f33cb317933eb867b4.
> do_suspend (from xen/manage.c) and thus xen_resume_notifier never get
> called on the initial-domain at resume (it is if running as guest.)
> 
> The rationale for the breaking change was that upload_pm_data()
> potentially does blocking work in syscore_resume(). This patch
> addresses the original issue by scheduling upload_pm_data() to
> execute in workqueue context.

It is ok to do upload_pm_data() with delay i.e. after some other
resume actions are done and possibly xen-acpi-processor is in
running state ?

Stanislaw

Re: [RESEND PATCH v3 1/2] dt-bindings: i2c: Add Mediatek MT2701 i2c binding

[Wolfram Sang]

On Thu, Mar 09, 2017 at 11:13:04AM +0800, Jun Gao wrote:
> From: Jun Gao 
> 
> Add MT2701 i2c binding to i2c-mt6577.txt and there is no need to
> modify i2c driver.
> 
> Signed-off-by: Jun Gao 

There is still the issue which Matthias Brugger pointed out on January,
24th: you need to describe the fallback compatibles.

> ---
>  .../devicetree/bindings/i2c/i2c-mt6577.txt |   11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt 
> b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> index 0ce6fa3..27dbbf9 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-mt6577.txt
> @@ -4,11 +4,12 @@ The Mediatek's I2C controller is used to interface with I2C 
> devices.
>  
>  Required properties:
>- compatible: value should be either of the following.
> -  (a) "mediatek,mt6577-i2c", for i2c compatible with mt6577 i2c.
> -  (b) "mediatek,mt6589-i2c", for i2c compatible with mt6589 i2c.
> -  (c) "mediatek,mt8127-i2c", for i2c compatible with mt8127 i2c.
> -  (d) "mediatek,mt8135-i2c", for i2c compatible with mt8135 i2c.
> -  (e) "mediatek,mt8173-i2c", for i2c compatible with mt8173 i2c.
> + "mediatek,mt2701-i2c"
> + "mediatek,mt6577-i2c"
> + "mediatek,mt6589-i2c"
> + "mediatek,mt8127-i2c"
> + "mediatek,mt8135-i2c"
> + "mediatek,mt8173-i2c"
>- reg: physical base address of the controller and dma base, length of 
> memory
>  mapped region.
>- interrupts: interrupt number to the cpu.
> -- 
> 1.7.9.5
> 


signature.asc
Description: PGP signature

Re: [PATCH] Add initial SX3000b platform code to MIPS arch

[Marc Zyngier]

Hi Amit,

On 22/03/17 05:38, Amit Kama IL wrote:
> Add initial support for boards based on Satixfy's SX3000b (Catniss) SoC.
> The SoC includes a MIPS interAptiv dual core 4 VPE processor and boots 
> using device-tree.
> 
> Signed-off-by: Amit Kama 
> 
> The irqchip file (irq-sx3000b.c) is pertinent to the platform. 
> IRQCHIP maintainers - is it possible to merge this through MIPS tree? 

Can you please send something that is the result of "git format-patch"?
You should have a diff-stat before the patch itself...

Also, this file (irq-sx3000b.c) doesn't seem to be in this huge patch,
which doesn't seem to concern the irqchip subsystem (it all looks
platform code to me). Wrong patch?

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v2 2/6] dt-bindings: i2c: rk3x: add support for rk3328

[Wolfram Sang]

On Thu, Mar 16, 2017 at 09:17:20PM +0800, c...@rock-chips.com wrote:
> From: Liang Chen 
> 
> The rk3328 i2c is the same as rk3399
> 
> Signed-off-by: Liang Chen 

Applied to for-next, thanks!



signature.asc
Description: PGP signature

Re: [PATCH v3 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

[Xunlei Pang]

On 03/22/2017 at 04:55 PM, Xunlei Pang wrote:
> On 03/21/2017 at 11:33 AM, Eric W. Biederman wrote:
>> Xunlei Pang  writes:
>>
>>> As Eric said,
>>> "what we need to do is move the variable vmcoreinfo_note out
>>> of the kernel's .bss section.  And modify the code to regenerate
>>> and keep this information in something like the control page.
>>>
>>> Definitely something like this needs a page all to itself, and ideally
>>> far away from any other kernel data structures.  I clearly was not
>>> watching closely the data someone decided to keep this silly thing
>>> in the kernel's .bss section."
>>>
>>> This patch allocates extra pages for these vmcoreinfo_XXX variables,
>>> one advantage is that it enhances some safety of vmcoreinfo, because
>>> vmcoreinfo now is kept far away from other kernel data structures.
>> Can you preceed this patch with a patch that removes CRASHTIME from
>> vmcoreinfo?  If someone actually cares we can add a separate note that holds
>> a 64bit crashtime in the per cpu notes.  
> Hi Eric,
>
> Thanks for your review, I took some time and did some investigation.
>
> Removing "CRASHTIME=X" from vmcoreinfo_note will break user-space tools.
> For example, makedumpfile gets vmcoreinfo note information by reading
> "/sys/kernel/vmcoreinfo"  its PA, then get its "VA = PA | PAGE_OFFSET",
> and then get the timestamp. This operates in the first kernel even before
> kdump is loaded.

Think more, this is not a problem for "makedumpfile --mem-usage",
as the system doesn't have "CRASHTIME" before crash. But still we
may have the following concerns.

>
> Actually, even moving vmcoreinfo_note[] into the crash memory, it
> may have problems, for example, on s390 system the crash memory
> range will be unmapped, so I guess it may cause some risks.
>
> Additionally, there is no available way for us to allocate a page from the
> crash memory during kernel initialization, we only can achieve this during
> the kexec syscalls. There is not a neat way to implement a function to
> allocate pages from the crash memory during kernel initialization without
> some hack code added, because user-space tools(like kexec-tools) can
> allocate the crash segment by their own ways from the crash memory.
>
> That's why I only copy vmcoreinfo_data[] into the crash memory, and
> not touch vmcoreinfo_note, so vmcoreinfo_data is well protected in
> the crash memory copy, then in crash_save_vmcoreinfo(), we copy
> this guaranteed copy into vmcoreinfo_note[], so the correctness of
> vmcoreinfo_note[] is guaranteed. This is what [PATCH v3 3/3] does.
>
> The current crash_save_vmcoreinfo() only involves memory(memcpy)
> operations even for get_seconds(no locks), the only risk I can think
> of now is that vmcoreinfo_note pointer may be corrupted. If it is a concern,
> I guess we can put it into struct kimage" just like vmcoreinfo_XXX_copy
> in this patch. After all if kimage structure was corrupted when crash happens,
> we can do nothing but have to accept the fate.
>
> So does it really deserve to eliminate crash_save_vmcoreinfo()?
>
> Regards,
> Xunlei
>
>> As we are looking at reliability concerns removing CRASHTIME should make
>> everything in vmcoreinfo a boot time constant.  Which should simplify
>> everything considerably.
>>
>> Which means we only need to worry abou the per-cpu notes being written
>> at the time of a crash.
>>
>>> Suggested-by: Eric Biederman 
>>> Signed-off-by: Xunlei Pang 
>>> ---
>>>  arch/ia64/kernel/machine_kexec.c |  5 -
>>>  arch/x86/kernel/crash.c  |  2 +-
>>>  include/linux/kexec.h|  2 +-
>>>  kernel/kexec_core.c  | 29 -
>>>  kernel/ksysfs.c  |  2 +-
>>>  5 files changed, 27 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/ia64/kernel/machine_kexec.c 
>>> b/arch/ia64/kernel/machine_kexec.c
>>> index 599507b..c14815d 100644
>>> --- a/arch/ia64/kernel/machine_kexec.c
>>> +++ b/arch/ia64/kernel/machine_kexec.c
>>> @@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void)
>>>  #endif
>>>  }
>>>  
>>> -phys_addr_t paddr_vmcoreinfo_note(void)
>>> -{
>>> -   return ia64_tpa((unsigned long)(char *)&vmcoreinfo_note);
>>> -}
>>> -
>>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>>> index 3741461..4d35fbb 100644
>>> --- a/arch/x86/kernel/crash.c
>>> +++ b/arch/x86/kernel/crash.c
>>> @@ -456,7 +456,7 @@ static int prepare_elf64_headers(struct crash_elf_data 
>>> *ced,
>>> bufp += sizeof(Elf64_Phdr);
>>> phdr->p_type = PT_NOTE;
>>> phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
>>> -   phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note);
>>> +   phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>>> (ehdr->e_phnum)++;
>>>  
>>>  #ifdef CONFIG_X86_64
>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>> index e98e546..f1c601b 100644
>>> --- a/include/linux/kexec.h
>>> +++ b/include/linux/kexec.h
>>> @@ -317,7 +317,7 @@ extern void *kexec_purgatory_get_sym

Re: [PATCH v3 2/3] clk: meson-gxbb: Add MALI clocks

[Neil Armstrong]

On 03/22/2017 12:31 AM, Michael Turquette wrote:
> Hi Neil,
> 
> Quoting Neil Armstrong (2017-03-09 04:53:46)
>> The Mali is clocked by two identical clock paths behind a glitch free mux
>> to safely change frequency while running.
>>
>> The two "mali_0" and "mali_1" clocks are composed of a mux, divider and gate.
>> Expose these two clocks trees using generic clocks.
>> Finally the glitch free mux is added as "mali" clock.
>>
>> Signed-off-by: Neil Armstrong 
>> ---
>>  drivers/clk/meson/gxbb.c | 139 
>> +++
>>  1 file changed, 139 insertions(+)
>>
>> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
>> index 5059c7b..d9f7fef 100644
>> --- a/drivers/clk/meson/gxbb.c
>> +++ b/drivers/clk/meson/gxbb.c
>> @@ -634,6 +634,131 @@
>> },
>>  };
>>  
>> +/*
>> + * The MALI IP is clocked by two identical clocks (mali_0 and mali_1)
>> + * muxed by a glitch-free switch.
>> + */
>> +
>> +static u32 mux_table_mali_0_1[] = {0, 1, 2, 3, 4, 5, 6, 7};
>> +static const char *gxbb_mali_0_1_parent_names[] = {
>> +   "xtal", "gp0_pll", "mpll2", "mpll1", "fclk_div7",
>> +   "fclk_div4", "fclk_div3", "fclk_div5"
>> +};
>> +
>> +static struct clk_mux gxbb_mali_0_sel = {
>> +   .reg = (void *)HHI_MALI_CLK_CNTL,
>> +   .mask = 0x7,
>> +   .shift = 9,
>> +   .table = mux_table_mali_0_1,
>> +   .lock = &clk_lock,
>> +   .hw.init = &(struct clk_init_data){
>> +   .name = "mali_0_sel",
>> +   .ops = &clk_mux_ops,
>> +   /*
>> +* bits 10:9 selects from 8 possible parents:
>> +* xtal, gp0_pll, mpll2, mpll1, fclk_div7,
>> +* fclk_div4, fclk_div3, fclk_div5
>> +*/
>> +   .parent_names = gxbb_mali_0_1_parent_names,
>> +   .num_parents = 8,
>> +   .flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
> 
> Are all of these CLK_IGNORE_UNUSED flags necessary? If so, why? I'm just
> wondering if this is yet another display-related use case where a
> hand-off mechanism would work better?

Hi,

I'm just precocious about how these clocks will be handled, since the Mali 
driver
is out of tree, I'm not sure about how they will be handled at all.

But I can test and remove these if we consider my out-of-tree driver platform
code as reference.

Neil

> 
> Thanks,
> Mike
> 
>> +   },
>> +};
>> +
>> +static struct clk_divider gxbb_mali_0_div = {
>> +   .reg = (void *)HHI_MALI_CLK_CNTL,
>> +   .shift = 0,
>> +   .width = 7,
>> +   .lock = &clk_lock,
>> +   .hw.init = &(struct clk_init_data){
>> +   .name = "mali_0_div",
>> +   .ops = &clk_divider_ops,
>> +   .parent_names = (const char *[]){ "mali_0_sel" },
>> +   .num_parents = 1,
>> +   .flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
>> +   },
>> +};
>> +
>> +static struct clk_gate gxbb_mali_0 = {
>> +   .reg = (void *)HHI_MALI_CLK_CNTL,
>> +   .bit_idx = 8,
>> +   .lock = &clk_lock,
>> +   .hw.init = &(struct clk_init_data){
>> +   .name = "mali_0",
>> +   .ops = &clk_gate_ops,
>> +   .parent_names = (const char *[]){ "mali_0_div" },
>> +   .num_parents = 1,
>> +   .flags = (CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED),
>> +   },
>> +};
>> +
>> +static struct clk_mux gxbb_mali_1_sel = {
>> +   .reg = (void *)HHI_MALI_CLK_CNTL,
>> +   .mask = 0x7,
>> +   .shift = 25,
>> +   .table = mux_table_mali_0_1,
>> +   .lock = &clk_lock,
>> +   .hw.init = &(struct clk_init_data){
>> +   .name = "mali_1_sel",
>> +   .ops = &clk_mux_ops,
>> +   /*
>> +* bits 10:9 selects from 8 possible parents:
>> +* xtal, gp0_pll, mpll2, mpll1, fclk_div7,
>> +* fclk_div4, fclk_div3, fclk_div5
>> +*/
>> +   .parent_names = gxbb_mali_0_1_parent_names,
>> +   .num_parents = 8,
>> +   .flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
>> +   },
>> +};
>> +
>> +static struct clk_divider gxbb_mali_1_div = {
>> +   .reg = (void *)HHI_MALI_CLK_CNTL,
>> +   .shift = 16,
>> +   .width = 7,
>> +   .lock = &clk_lock,
>> +   .hw.init = &(struct clk_init_data){
>> +   .name = "mali_1_div",
>> +   .ops = &clk_divider_ops,
>> +   .parent_names = (const char *[]){ "mali_1_sel" },
>> +   .num_parents = 1,
>> +   .flags = (CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED),
>> +   },
>> +};
>> +
>> +static struct clk_gate gxbb_mali_1 = {
>> +   .reg = (void *)HHI_MALI_CLK_CNTL,
>> +   .bit_idx = 24,
>> +   .lock = &clk_lock,
>> +   .hw.init = &(struct clk_init_data){
>> +   .name = "mali_1",
>> +   .ops = &clk_gate_ops,
>> +   .parent_names = (const 

Re: [PATCH 1/5] clk: meson: Add support for parameters for specific PLLs

[Neil Armstrong]

On 03/22/2017 12:43 AM, Michael Turquette wrote:
> Quoting Neil Armstrong (2017-03-13 06:26:40)
>> In recent Amlogic GXBB, GXL and GXM SoCs, the GP0 PLL needs some specific
>> parameters in order to initialize and lock correctly.
>>
>> This patch adds an optional PARAM table used to initialize the PLL to a
>> default value with it's parameters in order to achieve to desired frequency.
>>
>> The GP0 PLL in GXBB, GXL/GXM also needs some tweaks in the initialization
>> steps, and these are exposed along the PARAM table.
>>
>> Signed-off-by: Neil Armstrong 
>> ---
>>  drivers/clk/meson/clk-pll.c | 52 
>> +++--
>>  drivers/clk/meson/clkc.h| 23 
>>  2 files changed, 73 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>> index 4adc1e8..aff223b 100644
>> --- a/drivers/clk/meson/clk-pll.c
>> +++ b/drivers/clk/meson/clk-pll.c
>> @@ -116,6 +116,29 @@ static const struct pll_rate_table 
>> *meson_clk_get_pll_settings(struct meson_clk_
>> return NULL;
>>  }
>>  
>> +/* Specific wait loop for GXL/GXM GP0 PLL */
>> +static int meson_clk_pll_wait_lock_reset(struct meson_clk_pll *pll,
>> +struct parm *p_n)
>> +{
>> +   int delay = 100;
>> +   u32 reg;
>> +
>> +   while (delay > 0) {
>> +   reg = readl(pll->base + p_n->reg_off);
>> +   writel(reg | MESON_PLL_RESET, pll->base + p_n->reg_off);
>> +   udelay(10);
>> +   writel(reg & ~MESON_PLL_RESET, pll->base + p_n->reg_off);
>> +
>> +   mdelay(1);
> 
> Can you add a comment explaining where the delay values come from? Can
> they vary from chip to chip?

Hi,

Ok, the code comes from the vendor tree, and seems to be generic enough to 
handle
other chips.
But I'm not sure about this particular delay, 1ms seems large enough for a PLL 
locking...

Neil

> 
>> +
>> +   reg = readl(pll->base + p_n->reg_off);
>> +   if (reg & MESON_PLL_LOCK)
>> +   return 0;
>> +   delay--;
>> +   }
>> +   return -ETIMEDOUT;
>> +}
>> +
>>  static int meson_clk_pll_wait_lock(struct meson_clk_pll *pll,
>>struct parm *p_n)
>>  {
>> @@ -132,6 +155,15 @@ static int meson_clk_pll_wait_lock(struct meson_clk_pll 
>> *pll,
>> return -ETIMEDOUT;
>>  }
>>  
>> +static void meson_clk_pll_init_params(struct meson_clk_pll *pll)
>> +{
>> +   int i;
>> +
>> +   for (i = 0 ; i < pll->params.params_count ; ++i)
>> +   writel(pll->params.params_table[i].value,
>> +  pll->base + pll->params.params_table[i].reg_off);
>> +}
>> +
>>  static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>>   unsigned long parent_rate)
>>  {
>> @@ -151,10 +183,16 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, 
>> unsigned long rate,
>> if (!rate_set)
>> return -EINVAL;
>>  
>> +   /* Initialize the PLL in a clean state if specified */
>> +   if (pll->params.params_count)
>> +   meson_clk_pll_init_params(pll);
>> +
>> /* PLL reset */
>> p = &pll->n;
>> reg = readl(pll->base + p->reg_off);
>> -   writel(reg | MESON_PLL_RESET, pll->base + p->reg_off);
>> +   /* If no_init_reset is provided, avoid resetting at this point */
>> +   if (!pll->params.no_init_reset)
>> +   writel(reg | MESON_PLL_RESET, pll->base + p->reg_off);
>>  
>> reg = PARM_SET(p->width, p->shift, reg, rate_set->n);
>> writel(reg, pll->base + p->reg_off);
>> @@ -184,7 +222,17 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, 
>> unsigned long rate,
>> }
>>  
>> p = &pll->n;
>> -   ret = meson_clk_pll_wait_lock(pll, p);
>> +   /* If unreset_for_lock is provided, remove the reset bit here */
>> +   if (pll->params.unreset_for_lock) {
> 
> Small nitpick, but I find "unreset" to be confusing. Since 'reset' here
> is a bit that can be set and unset, maybe use clear_reset_for_lock
> instead?
> 
> Regards,
> Mike
> 
>> +   reg = readl(pll->base + p->reg_off);
>> +   writel(reg & ~MESON_PLL_RESET, pll->base + p->reg_off);
>> +   }
>> +
>> +   /* If reset_lock_loop, use a special loop including resetting */
>> +   if (pll->params.reset_lock_loop)
>> +   ret = meson_clk_pll_wait_lock_reset(pll, p);
>> +   else
>> +   ret = meson_clk_pll_wait_lock(pll, p);
>> if (ret) {
>> pr_warn("%s: pll did not lock, trying to restore old rate 
>> %lu\n",
>> __func__, old_rate);
>> diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
>> index 9bb70e7..5f1c12d 100644
>> --- a/drivers/clk/meson/clkc.h
>> +++ b/drivers/clk/meson/clkc.h
>> @@ -62,6 +62,28 @@ struct pll_rate_table {
>> .

[tip:x86/urgent] x86/platform/uv: Fix calculation of Global Physical Address

[tip-bot for Mike Travis]

Commit-ID:  ad4830051aac0b967add82ac168f49edf11f01a0
Gitweb: http://git.kernel.org/tip/ad4830051aac0b967add82ac168f49edf11f01a0
Author: Mike Travis 
AuthorDate: Tue, 21 Mar 2017 18:16:47 -0500
Committer:  Ingo Molnar 
CommitDate: Wed, 22 Mar 2017 07:41:10 +0100

x86/platform/uv: Fix calculation of Global Physical Address

The calculation of the global physical address (GPA) on UV4 is
incorrect.  The gnode_extra/upper global offset should only be
applied for fixed address space systems (UV1..3).

Tested-by: John Estabrook 
Signed-off-by: Mike Travis 
Cc: Dimitri Sivanich 
Cc: John Estabrook 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Russ Anderson 
Cc: Thomas Gleixner 
Link: http://lkml.kernel.org/r/20170321231646.667689...@asylum.americas.sgi.com
Signed-off-by: Ingo Molnar 
---
 arch/x86/include/asm/uv/uv_hub.h   | 8 +---
 arch/x86/kernel/apic/x2apic_uv_x.c | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/uv/uv_hub.h b/arch/x86/include/asm/uv/uv_hub.h
index 72e8300..9cffb44 100644
--- a/arch/x86/include/asm/uv/uv_hub.h
+++ b/arch/x86/include/asm/uv/uv_hub.h
@@ -485,15 +485,17 @@ static inline unsigned long 
uv_soc_phys_ram_to_gpa(unsigned long paddr)
 
if (paddr < uv_hub_info->lowmem_remap_top)
paddr |= uv_hub_info->lowmem_remap_base;
-   paddr |= uv_hub_info->gnode_upper;
-   if (m_val)
+
+   if (m_val) {
+   paddr |= uv_hub_info->gnode_upper;
paddr = ((paddr << uv_hub_info->m_shift)
>> uv_hub_info->m_shift) |
((paddr >> uv_hub_info->m_val)
<< uv_hub_info->n_lshift);
-   else
+   } else {
paddr |= uv_soc_phys_ram_to_nasid(paddr)
<< uv_hub_info->gpa_shift;
+   }
return paddr;
 }
 
diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c 
b/arch/x86/kernel/apic/x2apic_uv_x.c
index e9f8f8c..86f20cc 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -1105,7 +1105,8 @@ void __init uv_init_hub_info(struct uv_hub_info_s *hi)
node_id.v   = uv_read_local_mmr(UVH_NODE_ID);
uv_cpuid.gnode_shift= max_t(unsigned int, uv_cpuid.gnode_shift, 
mn.n_val);
hi->gnode_extra = (node_id.s.node_id & ~((1 << 
uv_cpuid.gnode_shift) - 1)) >> 1;
-   hi->gnode_upper = (unsigned long)hi->gnode_extra << mn.m_val;
+   if (mn.m_val)
+   hi->gnode_upper = (u64)hi->gnode_extra << mn.m_val;
 
if (uv_gp_table) {
hi->global_mmr_base = uv_gp_table->mmr_base;

[tip:x86/urgent] x86/mce: Fix copy/paste error in exception table entries

[tip-bot for Tony Luck]

Commit-ID:  26a37ab319a26d330bab298770d692bb9c852aff
Gitweb: http://git.kernel.org/tip/26a37ab319a26d330bab298770d692bb9c852aff
Author: Tony Luck 
AuthorDate: Mon, 20 Mar 2017 14:40:30 -0700
Committer:  Ingo Molnar 
CommitDate: Wed, 22 Mar 2017 08:43:25 +0100

x86/mce: Fix copy/paste error in exception table entries

Back in commit:

  92b0729c34cab ("x86/mm, x86/mce: Add memcpy_mcsafe()")

... I made a copy/paste error setting up the exception table entries
and ended up with two for label .L_cache_w3 and none for .L_cache_w2.

This means that if we take a machine check on:

  .L_cache_w2: movq 2*8(%rsi), %r10

then we don't have an exception table entry for this instruction
and we can't recover.

Fix: s/3/2/

Signed-off-by: Tony Luck 
Cc: 
Cc: Andy Lutomirski 
Cc: Borislav Petkov 
Cc: Brian Gerst 
Cc: Denys Vlasenko 
Cc: H. Peter Anvin 
Cc: Josh Poimboeuf 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Fixes: 92b0729c34cab ("x86/mm, x86/mce: Add memcpy_mcsafe()")
Link: 
http://lkml.kernel.org/r/1490046030-25862-1-git-send-email-tony.l...@intel.com
Signed-off-by: Ingo Molnar 
---
 arch/x86/lib/memcpy_64.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/lib/memcpy_64.S b/arch/x86/lib/memcpy_64.S
index 779782f..9a53a06 100644
--- a/arch/x86/lib/memcpy_64.S
+++ b/arch/x86/lib/memcpy_64.S
@@ -290,7 +290,7 @@ EXPORT_SYMBOL_GPL(memcpy_mcsafe_unrolled)
_ASM_EXTABLE_FAULT(.L_copy_leading_bytes, .L_memcpy_mcsafe_fail)
_ASM_EXTABLE_FAULT(.L_cache_w0, .L_memcpy_mcsafe_fail)
_ASM_EXTABLE_FAULT(.L_cache_w1, .L_memcpy_mcsafe_fail)
-   _ASM_EXTABLE_FAULT(.L_cache_w3, .L_memcpy_mcsafe_fail)
+   _ASM_EXTABLE_FAULT(.L_cache_w2, .L_memcpy_mcsafe_fail)
_ASM_EXTABLE_FAULT(.L_cache_w3, .L_memcpy_mcsafe_fail)
_ASM_EXTABLE_FAULT(.L_cache_w4, .L_memcpy_mcsafe_fail)
_ASM_EXTABLE_FAULT(.L_cache_w5, .L_memcpy_mcsafe_fail)

Re: [PATCH v3 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

[Xunlei Pang]

On 03/22/2017 at 04:55 PM, Xunlei Pang wrote:
> On 03/21/2017 at 11:33 AM, Eric W. Biederman wrote:
>> Xunlei Pang  writes:
>>
>>> As Eric said,
>>> "what we need to do is move the variable vmcoreinfo_note out
>>> of the kernel's .bss section.  And modify the code to regenerate
>>> and keep this information in something like the control page.
>>>
>>> Definitely something like this needs a page all to itself, and ideally
>>> far away from any other kernel data structures.  I clearly was not
>>> watching closely the data someone decided to keep this silly thing
>>> in the kernel's .bss section."
>>>
>>> This patch allocates extra pages for these vmcoreinfo_XXX variables,
>>> one advantage is that it enhances some safety of vmcoreinfo, because
>>> vmcoreinfo now is kept far away from other kernel data structures.
>> Can you preceed this patch with a patch that removes CRASHTIME from
>> vmcoreinfo?  If someone actually cares we can add a separate note that holds
>> a 64bit crashtime in the per cpu notes.  
> Hi Eric,
>
> Thanks for your review, I took some time and did some investigation.
>
> Removing "CRASHTIME=X" from vmcoreinfo_note will break user-space tools.
> For example, makedumpfile gets vmcoreinfo note information by reading
> "/sys/kernel/vmcoreinfo"  its PA, then get its "VA = PA | PAGE_OFFSET",
> and then get the timestamp. This operates in the first kernel even before
> kdump is loaded.

Think more, this is not a problem for "makedumpfile --mem-usage",
as the system doesn't have "CRASHTIME" before crash. But still we
may have the following concerns.

>
> Actually, even moving vmcoreinfo_note[] into the crash memory, it
> may have problems, for example, on s390 system the crash memory
> range will be unmapped, so I guess it may cause some risks.
>
> Additionally, there is no available way for us to allocate a page from the
> crash memory during kernel initialization, we only can achieve this during
> the kexec syscalls. There is not a neat way to implement a function to
> allocate pages from the crash memory during kernel initialization without
> some hack code added, because user-space tools(like kexec-tools) can
> allocate the crash segment by their own ways from the crash memory.
>
> That's why I only copy vmcoreinfo_data[] into the crash memory, and
> not touch vmcoreinfo_note, so vmcoreinfo_data is well protected in
> the crash memory copy, then in crash_save_vmcoreinfo(), we copy
> this guaranteed copy into vmcoreinfo_note[], so the correctness of
> vmcoreinfo_note[] is guaranteed. This is what [PATCH v3 3/3] does.
>
> The current crash_save_vmcoreinfo() only involves memory(memcpy)
> operations even for get_seconds(no locks), the only risk I can think
> of now is that vmcoreinfo_note pointer may be corrupted. If it is a concern,
> I guess we can put it into struct kimage" just like vmcoreinfo_XXX_copy
> in this patch. After all if kimage structure was corrupted when crash happens,
> we can do nothing but have to accept the fate.
>
> So does it really deserve to eliminate crash_save_vmcoreinfo()?
>
> Regards,
> Xunlei
>
>> As we are looking at reliability concerns removing CRASHTIME should make
>> everything in vmcoreinfo a boot time constant.  Which should simplify
>> everything considerably.
>>
>> Which means we only need to worry abou the per-cpu notes being written
>> at the time of a crash.
>>
>>> Suggested-by: Eric Biederman 
>>> Signed-off-by: Xunlei Pang 
>>> ---
>>>  arch/ia64/kernel/machine_kexec.c |  5 -
>>>  arch/x86/kernel/crash.c  |  2 +-
>>>  include/linux/kexec.h|  2 +-
>>>  kernel/kexec_core.c  | 29 -
>>>  kernel/ksysfs.c  |  2 +-
>>>  5 files changed, 27 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/arch/ia64/kernel/machine_kexec.c 
>>> b/arch/ia64/kernel/machine_kexec.c
>>> index 599507b..c14815d 100644
>>> --- a/arch/ia64/kernel/machine_kexec.c
>>> +++ b/arch/ia64/kernel/machine_kexec.c
>>> @@ -163,8 +163,3 @@ void arch_crash_save_vmcoreinfo(void)
>>>  #endif
>>>  }
>>>  
>>> -phys_addr_t paddr_vmcoreinfo_note(void)
>>> -{
>>> -   return ia64_tpa((unsigned long)(char *)&vmcoreinfo_note);
>>> -}
>>> -
>>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>>> index 3741461..4d35fbb 100644
>>> --- a/arch/x86/kernel/crash.c
>>> +++ b/arch/x86/kernel/crash.c
>>> @@ -456,7 +456,7 @@ static int prepare_elf64_headers(struct crash_elf_data 
>>> *ced,
>>> bufp += sizeof(Elf64_Phdr);
>>> phdr->p_type = PT_NOTE;
>>> phdr->p_offset = phdr->p_paddr = paddr_vmcoreinfo_note();
>>> -   phdr->p_filesz = phdr->p_memsz = sizeof(vmcoreinfo_note);
>>> +   phdr->p_filesz = phdr->p_memsz = VMCOREINFO_NOTE_SIZE;
>>> (ehdr->e_phnum)++;
>>>  
>>>  #ifdef CONFIG_X86_64
>>> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
>>> index e98e546..f1c601b 100644
>>> --- a/include/linux/kexec.h
>>> +++ b/include/linux/kexec.h
>>> @@ -317,7 +317,7 @@ extern void *kexec_purgatory_get_sym

Re: [PATCH v2 01/18] arm64: Allow checking of a CPU-local erratum

[Daniel Lezcano]

On Mon, Mar 20, 2017 at 05:48:12PM +, Marc Zyngier wrote:
> this_cpu_has_cap() only checks the feature array, and not the errata
> one. In order to be able to check for a CPU-local erratum, allow it
> to inspect the latter as well.
> 
> This is consistent with cpus_have_cap()'s behaviour, which includes
> errata already.
> 
> Reviewed-by: Suzuki K Poulose 
> Signed-off-by: Marc Zyngier 
> ---

Acked-by: Daniel Lezcano 

-- 

  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] staging: wilc1000: use kernel define byte order macros

[Dan Carpenter]

On Tue, Mar 21, 2017 at 03:40:10PM -0600, Robert Perry Hooker wrote:
> Thanks for taking a look, Dan. Sorry if I missed the mark here.
> 
> Can you tell me a bit more about the bug this would introduce?
> 
> I see that ieee80211_is_action is defined like this: static inline bool 
> ieee80211_is_action(__le16 fc)
> 
> ...and that buff[FRAME_TYPE_ID]is a u8 (since FRAME_TYPE_ID = 0).
> 
> Is there an issue with calling cpu_to_le16 on a u8 that isn't encountered by 
> implicitly casting a u8 to __le16? Or am I
> missing something else?
> 

Oh...  Hm.  You're right.  I just was thinking that since buff was a
little endian buffer but it's only reading a u8.  It should probably
be reading a le16...  The buff likely is just a regular ieee80211_hdr
struct.

So you're fixing a bug, but probably not in the right way.  We should
instead just say "struct ieee80211_hdr *hdr = buff;" and instead of
treating it like an array of u8.  Probably it requires testing...

regards,
dan carpenter

Re: [PATCHv2 1/2] mfd: cpcap: Add missing include dependencies

[Pavel Machek]

On Wed 2017-03-22 01:09:11, Sebastian Reichel wrote:
> This fixes compilation for files, that try to include the
> cpcap header in alphabetically sorted #include lists.
> 
> Signed-off-by: Sebastian Reichel 

Acked-by: Pavel Machek 

> ---
> This patch is new, since PATCHv1 did not order the includes
> in the led driver alphabetically.
> ---
>  include/linux/mfd/motorola-cpcap.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/mfd/motorola-cpcap.h 
> b/include/linux/mfd/motorola-cpcap.h
> index 7629e0d24d26..b8916953a537 100644
> --- a/include/linux/mfd/motorola-cpcap.h
> +++ b/include/linux/mfd/motorola-cpcap.h
> @@ -14,6 +14,9 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include 
> +#include 
> +
>  #define CPCAP_VENDOR_ST  0
>  #define CPCAP_VENDOR_TI  1
>  

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely

[Peter Zijlstra]

On Wed, Mar 22, 2017 at 12:08:50AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> The way the schedutil governor uses the PELT metric causes it to
> underestimate the CPU utilization in some cases.
> 
> That can be easily demonstrated by running kernel compilation on
> a Sandy Bridge Intel processor, running turbostat in parallel with
> it and looking at the values written to the MSR_IA32_PERF_CTL
> register.  Namely, the expected result would be that when all CPUs
> were 100% busy, all of them would be requested to run in the maximum
> P-state, but observation shows that this clearly isn't the case.
> The CPUs run in the maximum P-state for a while and then are
> requested to run slower and go back to the maximum P-state after
> a while again.  That causes the actual frequency of the processor to
> visibly oscillate below the sustainable maximum in a jittery fashion
> which clearly is not desirable.
> 
> That has been attributed to CPU utilization metric updates on task
> migration that cause the total utilization value for the CPU to be
> reduced by the utilization of the migrated task.  If that happens,
> the schedutil governor may see a CPU utilization reduction and will
> attempt to reduce the CPU frequency accordingly right away.  That
> may be premature, though, for example if the system is generally
> busy and there are other runnable tasks waiting to be run on that
> CPU already.
> 
> This is unlikely to be an issue on systems where cpufreq policies are
> shared between multiple CPUs, because in those cases the policy
> utilization is computed as the maximum of the CPU utilization values
> over the whole policy and if that turns out to be low, reducing the
> frequency for the policy most likely is a good idea anyway.  On
> systems with one CPU per policy, however, it may affect performance
> adversely and even lead to increased energy consumption in some cases.
> 
> On those systems it may be addressed by taking another utilization
> metric into consideration, like whether or not the CPU whose
> frequency is about to be reduced has been idle recently, because if
> that's not the case, the CPU is likely to be busy in the near future
> and its frequency should not be reduced.
> 
> To that end, use the counter of idle calls in the timekeeping code.
> Namely, make the schedutil governor look at that counter for the
> current CPU every time before its frequency is about to be reduced.
> If the counter has not changed since the previous iteration of the
> governor computations for that CPU, the CPU has been busy for all
> that time and its frequency should not be decreased, so if the new
> frequency would be lower than the one set previously, the governor
> will skip the frequency update.
> 
> Signed-off-by: Rafael J. Wysocki 

Right; this makes sense to me. Of course it would be good to have some
more measurements on this, but in principle:

Acked-by: Peter Zijlstra (Intel) 

Re: [PATCH] arm64: kaslr: Add 2MB correction for aligning kernel image

[Ard Biesheuvel]

> On 22 Mar 2017, at 08:55, Srinivas Ramana  wrote:
> 
> From: Neeraj Upadhyay 
> 
> If kernel image extends across alignment boundary, existing
> code increases the KASLR offset by size of kernel image. The
> offset is masked after resizing. There are cases, where after
> masking, we may still have kernel image extending across
> boundary. This eventually results in only 2MB block getting
> mapped while creating the page tables. This results in data aborts
> while accessing unmapped regions during second relocation (with
> kaslr offset) in __primary_switch. To fix this problem, add a
> 2MB correction to offset along with the correction of kernel
> image size, before applying mask.
> 
> For example consider below case, where kernel image still crosses
> 1GB alignment boundary, after masking the offset, which is fixed
> by adding 2MB correction.
> 
> SWAPPER_TABLE_SHIFT = 30
> Swapper using section maps with section size 2MB.
> CONFIG_PGTABLE_LEVELS = 3
> VA_BITS = 39
> 
> _text  : 0xff800808
> _end   : 0xff800aa1b000
> offset : 0x1f3560
> mask = ((1UL << (VA_BITS - 2)) - 1) & ~(SZ_2M - 1)
> 
> (_text + offset) >> SWAPPER_TABLE_SHIFT = 0x3fe7c
> (_end + offset) >> SWAPPER_TABLE_SHIFT  = 0x3fe7d
> 
> offset after existing correction (before mask) = 0x1f37f9b000
> (_text + offset) >> SWAPPER_TABLE_SHIFT = 0x3fe7d
> (_end + offset) >> SWAPPER_TABLE_SHIFT  = 0x3fe7d
> 
> offset (after mask) = 0x1f37e0
> (_text + offset) >> SWAPPER_TABLE_SHIFT = 0x3fe7c
> (_end + offset) >> SWAPPER_TABLE_SHIFT  = 0x3fe7d
> 
> new offset w/ 2MB correction (before mask) = 0x1f37819b00
> new offset w/ 2MB correction (after mask) = 0x1f3800
> (_text + offset) >> SWAPPER_TABLE_SHIFT = 0x3fe7d
> (_end + offset) >> SWAPPER_TABLE_SHIFT  = 0x3fe7d
> 
> Fixes: f80fb3a3d508 ("arm64: add support for kernel ASLR")
> Signed-off-by: Neeraj Upadhyay 
> Signed-off-by: Srinivas Ramana 
> ---
> arch/arm64/kernel/kaslr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
> index 769f24ef628c..7b8af985e497 100644
> --- a/arch/arm64/kernel/kaslr.c
> +++ b/arch/arm64/kernel/kaslr.c
> @@ -135,7 +135,7 @@ u64 __init kaslr_early_init(u64 dt_phys, u64 
> modulo_offset)
> */
>if u64)_text + offset + modulo_offset) >> SWAPPER_TABLE_SHIFT) !=
>(((u64)_end + offset + modulo_offset) >> SWAPPER_TABLE_SHIFT))
> -offset = (offset + (u64)(_end - _text)) & mask;
> +offset = (offset + (u64)(_end - _text) + SZ_2M) & mask;
> 
>if (IS_ENABLED(CONFIG_KASAN))
>/*


Hi,

Thanks for spotting this!

Instead of adding 2 MB, could we round up _end  - _text to a SWAPPER_BLOCK_SIZE 
multiple instead?

[PATCH 2/2] f2fs: don't track volatile file in dirty inode list

[Chao Yu]

Don't track volatile file in dirty inode list, otherwise with data_flush
option, background thread will entry into endless loop for flushing
journal file's pages.

Signed-off-by: Chao Yu 
---
 fs/f2fs/checkpoint.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 85831a0af463..9aba0bb340a0 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -817,7 +817,9 @@ static void __add_dirty_inode(struct inode *inode, enum 
inode_type type)
return;
 
set_inode_flag(inode, flag);
-   list_add_tail(&F2FS_I(inode)->dirty_list, &sbi->inode_list[type]);
+   if (!f2fs_is_volatile_file(inode))
+   list_add_tail(&F2FS_I(inode)->dirty_list,
+   &sbi->inode_list[type]);
stat_inc_dirty_inode(sbi, type);
 }
 
-- 
2.8.2.295.g3f1c1d0

[PATCH 1/2] f2fs: show the max number of volatile operations

[Chao Yu]

This patch adds to show the max number of volatile operations which are
conducting concurrently.

Signed-off-by: Chao Yu 
---
 fs/f2fs/debug.c | 10 --
 fs/f2fs/f2fs.h  | 18 +-
 fs/f2fs/file.c  |  5 +
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
index ef1179df05d9..0baa3ee39392 100644
--- a/fs/f2fs/debug.c
+++ b/fs/f2fs/debug.c
@@ -51,7 +51,9 @@ static void update_general_status(struct f2fs_sb_info *sbi)
si->ndirty_all = sbi->ndirty_inode[DIRTY_META];
si->inmem_pages = get_pages(sbi, F2FS_INMEM_PAGES);
si->aw_cnt = atomic_read(&sbi->aw_cnt);
+   si->vw_cnt = atomic_read(&sbi->vw_cnt);
si->max_aw_cnt = atomic_read(&sbi->max_aw_cnt);
+   si->max_vw_cnt = atomic_read(&sbi->max_vw_cnt);
si->nr_wb_cp_data = get_pages(sbi, F2FS_WB_CP_DATA);
si->nr_wb_data = get_pages(sbi, F2FS_WB_DATA);
if (SM_I(sbi) && SM_I(sbi)->fcc_info)
@@ -337,8 +339,10 @@ static int stat_show(struct seq_file *s, void *v)
seq_printf(s, "  - IO (CP: %4d, Data: %4d, Flush: %4d, Discard: 
%4d)\n",
   si->nr_wb_cp_data, si->nr_wb_data,
   si->nr_flush, si->nr_discard);
-   seq_printf(s, "  - inmem: %4d, atomic IO: %4d (Max. %4d)\n",
-  si->inmem_pages, si->aw_cnt, si->max_aw_cnt);
+   seq_printf(s, "  - inmem: %4d, atomic IO: %4d (Max. %4d), "
+   "volatile IO: %4d (Max. %4d)\n",
+  si->inmem_pages, si->aw_cnt, si->max_aw_cnt,
+  si->vw_cnt, si->max_vw_cnt);
seq_printf(s, "  - nodes: %4d in %4d\n",
   si->ndirty_node, si->node_pages);
seq_printf(s, "  - dents: %4d in dirs:%4d (%4d)\n",
@@ -438,7 +442,9 @@ int f2fs_build_stats(struct f2fs_sb_info *sbi)
atomic_set(&sbi->inplace_count, 0);
 
atomic_set(&sbi->aw_cnt, 0);
+   atomic_set(&sbi->vw_cnt, 0);
atomic_set(&sbi->max_aw_cnt, 0);
+   atomic_set(&sbi->max_vw_cnt, 0);
 
mutex_lock(&f2fs_stat_mutex);
list_add_tail(&si->stat_list, &f2fs_stat_list);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 82ddf5649d68..9ea230f7a0c3 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -921,7 +921,9 @@ struct f2fs_sb_info {
atomic_t inline_inode;  /* # of inline_data inodes */
atomic_t inline_dir;/* # of inline_dentry inodes */
atomic_t aw_cnt;/* # of atomic writes */
+   atomic_t vw_cnt;/* # of volatile writes */
atomic_t max_aw_cnt;/* max # of atomic writes */
+   atomic_t max_vw_cnt;/* max # of volatile writes */
int bg_gc;  /* background gc calls */
unsigned int ndirty_inode[NR_INODE_TYPE];   /* # of dirty inodes */
 #endif
@@ -2334,7 +2336,7 @@ struct f2fs_stat_info {
int total_count, utilization;
int bg_gc, nr_wb_cp_data, nr_wb_data, nr_flush, nr_discard;
int inline_xattr, inline_inode, inline_dir, append, update, orphans;
-   int aw_cnt, max_aw_cnt;
+   int aw_cnt, max_aw_cnt, vw_cnt, max_vw_cnt;
unsigned int valid_count, valid_node_count, valid_inode_count, 
discard_blks;
unsigned int bimodal, avg_vblocks;
int util_free, util_valid, util_invalid;
@@ -2417,6 +2419,17 @@ static inline struct f2fs_stat_info *F2FS_STAT(struct 
f2fs_sb_info *sbi)
if (cur > max)  \
atomic_set(&F2FS_I_SB(inode)->max_aw_cnt, cur); \
} while (0)
+#define stat_inc_volatile_write(inode) \
+   (atomic_inc(&F2FS_I_SB(inode)->vw_cnt))
+#define stat_dec_volatile_write(inode) \
+   (atomic_dec(&F2FS_I_SB(inode)->vw_cnt))
+#define stat_update_max_volatile_write(inode)  \
+   do {\
+   int cur = atomic_read(&F2FS_I_SB(inode)->vw_cnt);   \
+   int max = atomic_read(&F2FS_I_SB(inode)->max_vw_cnt);   \
+   if (cur > max)  \
+   atomic_set(&F2FS_I_SB(inode)->max_vw_cnt, cur); \
+   } while (0)
 #define stat_inc_seg_count(sbi, type, gc_type) \
do {\
struct f2fs_stat_info *si = F2FS_STAT(sbi); \
@@ -2473,6 +2486,9 @@ void f2fs_destroy_root_stats(void);
 #define stat_inc_atomic_write(inode)
 #define stat_dec_atomic_write(inode)
 #define stat_update_max_atomic_write(inode)
+#define stat_inc_volatile_write(inode)
+#define stat_dec_volatile_write(inode)
+#define stat_update_max_vo

Re: [PATCH] cpufreq: schedutil: Always trace frequency if it does not change

[Peter Zijlstra]

On Wed, Mar 22, 2017 at 01:56:53AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> sugov_update_commit() calls trace_cpu_frequency() to record the
> current CPU frequency if it has not changed in the fast switch case
> to prevent utilities from getting confused (they may report that the
> CPU is idle if the frequency has not been recorded for too long, for
> example).

That seems like buggy tools; we should then fix the tools, not the
kernel to emit more superfluous information.

> However, the same problem may occur for a cpufreq driver that doesn't
> support fast frequency switching and implements the ->target callback
> (that is, it doesn't use frequency tables), but trace_cpu_frequency()
> is not called if frequency updates are skipped in that case.

I'm having trouble parsing that; am I right in understanding this is the
exact same scenario where only superfluous changes are omitted?

> For this reason, modify sugov_update_commit() to always call
> trace_cpu_frequency() when the new frequency to be set is equal to
> the one that was set previously and reorganize the code in there to
> reduce duplication somewhat.

So why not fix the tools?

Re: [PATCHv2 02/11] Bluetooth: hci_uart: add serdev driver support library

[Pavel Machek]

On Tue 2017-03-21 23:32:07, Sebastian Reichel wrote:
> From: Rob Herring 
> 
> This adds library functions for serdev based BT drivers. This is largely
> copied from hci_ldisc.c and modified to use serdev calls. There's a little
> bit of duplication, but I avoided intermixing this as the ldisc code should
> eventually go away.
> 
> Signed-off-by: Rob Herring 
> Cc: Marcel Holtmann 
> Cc: Gustavo Padovan 
> Cc: Johan Hedberg 
> Cc: linux-blueto...@vger.kernel.org
> [Fix style issues reported by Pavel]
> Signed-off-by: Sebastian Reichel 

Acked-by: Pavel Machek 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH 3/5] clk: meson-gxbb: Add GXL/GXM GP0 Variant

[Neil Armstrong]

On 03/22/2017 12:49 AM, Michael Turquette wrote:
> Hi Neil,
> 
> Quoting Neil Armstrong (2017-03-13 06:26:42)
>> @@ -821,6 +893,7 @@ struct pll_params_table gxbb_gp0_params_table[] = {
>> &gxbb_hdmi_pll,
>> &gxbb_sys_pll,
>> &gxbb_gp0_pll,
>> +   &gxl_gp0_pll,

Yes, because this is the table used to change the register base, this won't 
harm in any way
to add SoC variant clocks, since they they are initialized using the 
gxbb_hw_onecell_data table.

> 
> Is there a reason for adding the pointer to this array here? It seems to
> me that the gxbb_gp0_pll and gxl_gp0_pll are mutually exclusive, so
> perhaps two different tables should be used?
> 
>>  };
>>  
>>  static struct meson_clk_mpll *const gxbb_clk_mplls[] = {
>> @@ -923,6 +996,10 @@ static int gxbb_clkc_probe(struct platform_device *pdev)
>> struct clk *parent_clk;
>> struct device *dev = &pdev->dev;
>>  
>> +   /* Override GP0 clock for GXL/GXM */
>> +   if (of_device_is_compatible(dev->of_node, "amlogic,gxl-clkc"))
>> +   gxbb_hw_onecell_data.hws[CLKID_GP0_PLL] = &gxl_gp0_pll.hw;
> 
> Similarly, this above is a little ugly compared to dedicated tables for
> each variant.

Here is the true uglyness, but would you like to have the exact same 
gxbb_hw_onecell_data
duplicated for only two different clocks ?
The gxbb_hw_onecell_data table is 105 lines, and when adding new clocks, we 
will need to
make sure they are still synchronized.

If you have a better idea... I can still push a v2 with such table with also a
separate gxbb_clk_plls table stored in a struct given from 
of_device_get_match_data()

Neil

> 
> Regards,
> Mike
> 
>> +
>> /*  Generic clocks and PLLs */
>> clk_base = of_iomap(dev->of_node, 0);
>> if (!clk_base) {
>> @@ -996,6 +1073,7 @@ static int gxbb_clkc_probe(struct platform_device *pdev)
>>  
>>  static const struct of_device_id gxbb_clkc_match_table[] = {
>> { .compatible = "amlogic,gxbb-clkc" },
>> +   { .compatible = "amlogic,gxl-clkc" },
>> { }
>>  };
>>  
>> diff --git a/drivers/clk/meson/gxbb.h b/drivers/clk/meson/gxbb.h
>> index 8ee2022..7f99bf6 100644
>> --- a/drivers/clk/meson/gxbb.h
>> +++ b/drivers/clk/meson/gxbb.h
>> @@ -71,6 +71,8 @@
>>  #define HHI_GP0_PLL_CNTL2  0x44 /* 0x11 offset in data sheet */
>>  #define HHI_GP0_PLL_CNTL3  0x48 /* 0x12 offset in data sheet */
>>  #define HHI_GP0_PLL_CNTL4  0x4c /* 0x13 offset in data sheet */
>> +#defineHHI_GP0_PLL_CNTL5   0x50 /* 0x14 offset in data 
>> sheet */
>> +#defineHHI_GP0_PLL_CNTL1   0x58 /* 0x16 offset in data 
>> sheet */
>>  
>>  #define HHI_XTAL_DIVN_CNTL 0xbc /* 0x2f offset in data sheet */
>>  #define HHI_TIMER90K   0xec /* 0x3b offset in data sheet */
>> -- 
>> 1.9.1
>>

Re: [PATCH v3 1/3] kexec: Move vmcoreinfo out of the kernel's .bss section

[Xunlei Pang]

On 03/22/2017 at 12:30 PM, Dave Young wrote:
> On 03/21/17 at 10:18pm, Eric W. Biederman wrote:
>> Dave Young  writes:
>>
>>> On 03/20/17 at 10:33pm, Eric W. Biederman wrote:
 Xunlei Pang  writes:

> As Eric said,
> "what we need to do is move the variable vmcoreinfo_note out
> of the kernel's .bss section.  And modify the code to regenerate
> and keep this information in something like the control page.
>
> Definitely something like this needs a page all to itself, and ideally
> far away from any other kernel data structures.  I clearly was not
> watching closely the data someone decided to keep this silly thing
> in the kernel's .bss section."
>
> This patch allocates extra pages for these vmcoreinfo_XXX variables,
> one advantage is that it enhances some safety of vmcoreinfo, because
> vmcoreinfo now is kept far away from other kernel data structures.
 Can you preceed this patch with a patch that removes CRASHTIME from
 vmcoreinfo?  If someone actually cares we can add a separate note that 
 holds
 a 64bit crashtime in the per cpu notes.  
>>> I think makedumpfile is using it, but I also vote to remove the
>>> CRASHTIME. It is better not to do this while crashing and a makedumpfile
>>> userspace patch is needed to drop the use of it.
>>>

By moving the CRASHTIME info to the cpu note of crashed cpu may be a good
way. In kdump kernel, notes of vmcore elfhdr will be merged into one big note
section, I don't know how makedumpfile or crash handle the big note section?
If they process the note in some order, breakage will definitely happen...

There is also a fadump may be affected.

Regards,
Xunlei

 As we are looking at reliability concerns removing CRASHTIME should make
 everything in vmcoreinfo a boot time constant.  Which should simplify
 everything considerably.
>>> It is a nice improvement..
>> We also need to take a close look at what s390 is doing with vmcoreinfo.
>> As apparently it is reading it in a different kind of crashdump process.
> Yes, need careful review from s390 and maybe ppc64 especially about
> patch 2/3, better to have comments from IBM about s390 dump tool and ppc
> fadump. Added more cc.
>
> Thanks
> Dave
>
> ___
> kexec mailing list
> ke...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] sched/fair: Fix ftq noise bench regression

[Vincent Guittot]

On 21 March 2017 at 18:46, Dietmar Eggemann  wrote:
> Hi Vincent,
>
> On 17/03/17 13:47, Vincent Guittot wrote:
>
> [...]
>
>> Reported-by: ying.hu...@linux.intel.com
>> Signed-off-by: Vincent Guittot 
>> Fixes: 4e5160766fcc ("sched/fair: Propagate asynchrous detach")
>
> I thought I can see a difference by running:
>
>  perf stat --null --repeat 10 -- perf bench sched messaging -g 50 -l
>  5000
>
> on an Ubuntu 16.10 server system.
>
> Number of entries in the rq->leaf_cfs_rq_list per cpu: ~40
>
> Target: Intel i5-3320M (4 logical cpus)
>
> tip/sched/core: 42.119140365 seconds time elapsed ( +-  0.33% )
>
> + patch   : 42.089557108 seconds time elapsed ( +-  0.37% )
>
> Maybe I need a CPU with more 'logical cpus' or a different test case.
> Anyway, couldn't spot any regression.

ok. I haven't been able to reproduce the regression on my platforms too

>
>> ---
>>  kernel/sched/fair.c | 39 ---
>>  1 file changed, 36 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 2805bd7..007df59 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -3173,6 +3173,36 @@ static inline int propagate_entity_load_avg(struct 
>> sched_entity *se)
>>   return 1;
>>  }
>>
>> +/*
>> + * Check if we need to update the load and the utilization of a blocked
>> + * group_entity
>> + */
>> +static inline bool skip_blocked_update(struct sched_entity *se)
>> +{
>> + struct cfs_rq *gcfs_rq = group_cfs_rq(se);
>> +
>> + /*
>> +  * If sched_entity still have not null load or utilization, we have to
>> +  * decay it.
>> +  */
>> + if (se->avg.load_avg || se->avg.util_avg)
>> + return false;
>> +
>> + /*
>> +  * If there is a pending propagation, we have to update the load and
>> +  * the utilizaion of the sched_entity
>
> nit pick: s/utilizaion/utilization

yes


>
>> +  */
>> + if (gcfs_rq->propagate_avg)
>> + return false;
>> +
>> + /*
>> +  * Other wise, the load and the utilization of the sched_entity is
>> +  * already null and there is no pending propagation so it will be a
>> +  * waste of time to try to decay it.
>> +  */
>> + return true;
>> +}
>> +
>>  #else /* CONFIG_FAIR_GROUP_SCHED */
>>
>>  static inline void update_tg_load_avg(struct cfs_rq *cfs_rq, int force) {}
>> @@ -6961,6 +6991,8 @@ static void update_blocked_averages(int cpu)
>>* list_add_leaf_cfs_rq() for details.
>>*/
>>   for_each_leaf_cfs_rq(rq, cfs_rq) {
>> + struct sched_entity *se;
>> +
>>   /* throttled entities do not contribute to load */
>>   if (throttled_hierarchy(cfs_rq))
>>   continue;
>> @@ -6968,9 +7000,10 @@ static void update_blocked_averages(int cpu)
>>   if (update_cfs_rq_load_avg(cfs_rq_clock_task(cfs_rq), cfs_rq, 
>> true))
>>   update_tg_load_avg(cfs_rq, 0);
>>
>> - /* Propagate pending load changes to the parent */
>> - if (cfs_rq->tg->se[cpu])
>> - update_load_avg(cfs_rq->tg->se[cpu], 0);
>> + /* Propagate pending load changes to the parent if any */
>> + se = cfs_rq->tg->se[cpu];
>> + if (se && !skip_blocked_update(se))
>> + update_load_avg(se, 0);
>>   }
>>   rq_unlock_irqrestore(rq, &rf);
>>  }
>>
>
> Why not turn skip_blocked_update(se) into blocked_update_needed(cpu, cfs_rq)?
> Saves a couple of patch lines:

Are you sure that we are saving some patch lines ?

I tend to agree on the name and but not on parameters.
IMO, it's easier to understand the purpose of
blocked_update_needed(se) compared to blocked_update_needed(cpu,
cfs_rq)


>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 007df5953d1a..8001eeb4ec18 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -3177,30 +3177,34 @@ static inline int propagate_entity_load_avg(struct 
> sched_entity *se)
>   * Check if we need to update the load and the utilization of a blocked
>   * group_entity
>   */
> -static inline bool skip_blocked_update(struct sched_entity *se)
> +static inline bool blocked_update_needed(int cpu, struct cfs_rq *cfs_rq)
>  {
> -   struct cfs_rq *gcfs_rq = group_cfs_rq(se);
> +   struct sched_entity *se = cfs_rq->tg->se[cpu];
> +
> +   /* cfs_rq of a root task_group has no sched_entity counterpart */
> +   if (!se)
> +   return false;
>
> /*
>  * If sched_entity still have not null load or utilization, we have to
>  * decay it.
>  */
> if (se->avg.load_avg || se->avg.util_avg)
> -   return false;
> +   return true;
>
> /*
>  * If there is a pending propagation, we have to update the load and
>  * the utilizaion of the sched_entity
>  */
> -   if (gcfs_rq->propagate_avg)
> -   return false;
>

Re: [PATCHv2 04/11] tty: serial: omap: add UPF_BOOT_AUTOCONF flag for DT init

[Pavel Machek]

On Tue 2017-03-21 23:32:09, Sebastian Reichel wrote:
> The UPF_BOOT_AUTOCONF flag is needed for proper
> flow control support.
> 
> Signed-off-by: Sebastian Reichel 

Acked-by: Pavel Machek 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH] usb: gadget: Correct usb EP argument for BOT status request

[Felipe Balbi]

Hi,

"Nicholas A. Bellinger"  writes:
> Hi Manish,
>
> (Added target-devel CC')
>
> On Mon, 2017-03-20 at 15:05 +0530, Manish Narani wrote:
>> This patch corrects the argument in usb_ep_free_request as it is
>> mistakenly set to ep_out. It should be ep_in for status request.
>> 
>> Signed-off-by: Manish Narani 
>> ---
>>  drivers/usb/gadget/function/f_tcm.c |2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/drivers/usb/gadget/function/f_tcm.c 
>> b/drivers/usb/gadget/function/f_tcm.c
>> index d235113..a82e2bd 100644
>> --- a/drivers/usb/gadget/function/f_tcm.c
>> +++ b/drivers/usb/gadget/function/f_tcm.c
>> @@ -373,7 +373,7 @@ static void bot_cleanup_old_alt(struct f_uas *fu)
>>  usb_ep_free_request(fu->ep_in, fu->bot_req_in);
>>  usb_ep_free_request(fu->ep_out, fu->bot_req_out);
>>  usb_ep_free_request(fu->ep_out, fu->cmd.req);
>> -usb_ep_free_request(fu->ep_out, fu->bot_status.req);
>> +usb_ep_free_request(fu->ep_in, fu->bot_status.req);
>>  
>>  kfree(fu->cmd.buf);
>>  
>
> Applied to target-pending/master.

Oh, so you're taking patches for f_tcm? Fair enough:

Acked-by: Felipe Balbi 

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCHv2.1] serdev: implement get/set tiocm

[Pavel Machek]

On Wed 2017-03-22 01:58:37, Sebastian Reichel wrote:
> Add method for getting and setting tiocm.
> 
> Signed-off-by: Sebastian Reichel 

Acked-by: Pavel Machek 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: staging: iio: Replace a bit shift by a use of BIT.

[Dan Carpenter]

On Wed, Mar 22, 2017 at 08:19:42AM +0530, Arushi Singhal wrote:
> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
> 
> -1 << c
> +BIT(c)
> 
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/iio/adc/ad7816.c |  2 +-
>  drivers/staging/iio/cdc/ad7150.c |  2 +-
>  drivers/staging/iio/cdc/ad7746.c | 22 +++---
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7816.c 
> b/drivers/staging/iio/adc/ad7816.c
> index 17d280581e24..42f637ca0251 100644
> --- a/drivers/staging/iio/adc/ad7816.c
> +++ b/drivers/staging/iio/adc/ad7816.c
> @@ -222,7 +222,7 @@ static ssize_t ad7816_show_value(struct device *dev,
>   value = (s8)((data >> AD7816_TEMP_FLOAT_OFFSET) - 103);
>   data &= AD7816_TEMP_FLOAT_MASK;
>   if (value < 0)
> - data = (1 << AD7816_TEMP_FLOAT_OFFSET) - data;
> + data = (BIT(AD7816_TEMP_FLOAT_OFFSET)) - data;

Remove the extra parentheses from all of these:

data = BIT(AD7816_TEMP_FLOAT_OFFSET) - data;

regards,
dan carpenter

Re: [PATCH v2 2/2] iio: Aspeed AST2400/AST2500 ADC

[Joel Stanley]

On Wed, Mar 22, 2017 at 7:18 AM, Rick Altherr  wrote:
> Aspeed AST2400/AST2500 BMC SoCs include a 16 channel, 10-bit ADC. Low
> and high threshold interrupts are supported by the hardware but are not
> currently implemented.
>
> Signed-off-by: Rick Altherr 

Looks good Rick. I gave it a review from the perspective of the Aspeed
soc. I also gave it a spin on the Aspeed AST2500 EVB which mostly
worked, but uncovered some things that need addressing.

My device tree additions looked like this:

  adc: adc@1e6e9000 {
  compatible = "aspeed,ast2500-adc";
  reg = <0x1e6e9000 0xb0>;
  clocks = <&clk_apb>;
  #io-channel-cells = <1>;

  pinctrl-names = "default";
  pinctrl-0 = <&pinctrl_adc0_default>;
  };

  iio-hwmon {
  compatible = "iio-hwmon";
  io-channels = <&adc 0>;
  };

I got this output from lm-sensors when booted:

iio_hwmon-isa-
Adapter: ISA adapter
in1:  +1.28 V

I then wired up ADC0 to ADC_12V_TW on the EVB. The above changed to:

 iio_hwmon-isa-
Adapter: ISA adapter
in1:  +1.80 V

ADC_12V_TW is the 12V rail sampled through a voltage divider. The
voltage should be: 12 * 680 / ( 5600 + 680) = 1.299

cat /sys/bus/iio/devices/iio:device0/in_voltage1_raw
738

738 / 1023 * 1.8 = 1.2975

Looks like the first channel is working! However our reference is
incorrect. Your driver has ASPEED_ADC_REF_VOLTAGE but doesn't use it.
It does hardcode 2500 in the aspeed_adc_read_raw callback:

  case IIO_CHAN_INFO_SCALE:
  *val = 2500; // mV
  *val2 = 10;
  return IIO_VAL_FRACTIONAL_LOG2;

Should this value the the constant you define?

Regardless, I don't think the reference voltage should be a constant.
This is going to vary from system to system. Can we put it in the
device tree? I notice other devices have vref-supply in their
bindings.

I noticed that in_voltage_scale is writable. However, it did not
accept any of the values I give it. Is this because we do not handle
it in aspeed_adc_write_raw?

I suggest we add the reference in the device tree bindings, and also
allow the value to be updated from userspace.

> ---
>
> Changes in v2:
> - Rewritten as an IIO device
> - Renamed register macros to describe the register's purpose
> - Replaced awkward reading of 16-bit data registers with readw()
> - Added Kconfig dependency on COMPILE_TEST
>
>  drivers/iio/adc/Kconfig  |  10 ++
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/aspeed_adc.c | 271 
> +++
>  3 files changed, 282 insertions(+)
>  create mode 100644 drivers/iio/adc/aspeed_adc.c
>
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2268a6fb9865..9672d799a3fb 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -130,6 +130,16 @@ config AD799X
>   To compile this driver as a module, choose M here: the module will 
> be
>   called ad799x.
>
> +config ASPEED_ADC
> +   tristate "Aspeed AST2400/AST2500 ADC"

You could just say Aspeed ADC to save us having to update it when the
ast2600 comes out.

> +   depends on ARCH_ASPEED || COMPILE_TEST
> +   help
> + If you say yes here you get support for the Aspeed AST2400/AST2500
> + ADC.
> +
> + To compile this driver as a module, choose M here: the module will 
> be
> + called aspeed_adc.

Don't forget to test compiling as a module.


> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> new file mode 100644
> index ..9220909aefd4
> --- /dev/null

> +#define ASPEED_ADC_NUM_CHANNELS16
> +#define ASPEED_ADC_REF_VOLTAGE 2500 /* millivolts */
> +#define ASPEED_ADC_RESOLUTION_BITS 10
> +#define ASPEED_ADC_MIN_SAMP_RATE   1
> +#define ASPEED_ADC_MAX_SAMP_RATE   50
> +#define ASPEED_ADC_CLOCKS_PER_SAMPLE   12
> +
> +#define ASPEED_ADC_REG_ENGINE_CONTROL  0x00
> +#define ASPEED_ADC_REG_INTERRUPT_CONTROL   0x04
> +#define ASPEED_ADC_REG_VGA_DETECT_CONTROL  0x08
> +#define ASPEED_ADC_REG_CLOCK_CONTROL   0x0C
> +#define ASPEED_ADC_REG_MAX 0xC0
> +
> +#define ASPEED_ADC_OPERATION_MODE_POWER_DOWN   (0x0 << 1)
> +#define ASPEED_ADC_OPERATION_MODE_STANDBY  (0x1 << 1)
> +#define ASPEED_ADC_OPERATION_MODE_NORMAL   (0x7 << 1)
> +
> +#define ASPEED_ADC_ENGINE_ENABLE   BIT(0)

Nit: You could chose to label these with a shorter prefix. Drop the
aspeed or adc, or both.

> +
> +static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
> +  struct iio_chan_spec const *chan,
> +  int *val, int *val2, long mask)
> +{
> +   struct aspeed_adc_data *data = iio_priv(indio_dev);
> +
> +   switch (mask) {
> +   case IIO_CHAN_INFO_RAW:
> +   *val = readw(data->base + chan->address);
> + 

[GIT PULL] extcon fixes for v4.11-rc3

[Chanwoo Choi]

Dear Greg,

This is extcon-fixes pull request for v4.11-rc3. I add detailed description of
this pull request on below. Please pull extcon with following updates.

Best Regards,
Chanwoo Choi

The following changes since commit 4495c08e84729385774601b5146d51d9e5849f81:

  Linux 4.11-rc2 (2017-03-12 14:47:08 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/extcon.git 
tags/extcon-fixes-for-4.11-rc3

for you to fetch changes up to 70216fd937fea79c20705400540fa48f86ddf1c5:

  extcon: int3496: Set the id pin to direction-input if necessary (2017-03-22 
18:29:48 +0900)


Update extcon for v4.11-rc3

This patch fixes issues for Intel INT3496 ACPI device as following:
- Propagate error code of gpiod_to_irq().
- Set the ID pin as the input direction if firmware has the bug
- Rename the gpio name for bining according to the documentation.
- Add gpio acpi mapping table and the dependency on X86.
- Use the devm_gpiod_get() instead of gpiod_get_index because of
  acpi mapping table.


Andy Shevchenko (3):
  extcon: int3496: Propagate error code of gpiod_to_irq()
  extcon: int3496: Rename GPIO pins in accordance with binding
  extcon: int3496: Add GPIO ACPI mapping table

Hans de Goede (2):
  extcon: int3496: Use gpiod_get instead of gpiod_get_index
  extcon: int3496: Set the id pin to direction-input if necessary

Peter Robinson (1):
  extcon: int3496: Add dependency on X86 as it's Intel specific

 Documentation/extcon/intel-int3496.txt |  5 +
 drivers/extcon/Kconfig |  2 +-
 drivers/extcon/extcon-intel-int3496.c  | 39 --
 3 files changed, 34 insertions(+), 12 deletions(-)

Re: [PATCHv2 05/11] serdev: add serdev_device_wait_until_sent

[Pavel Machek]

Hi!

> @@ -47,10 +48,13 @@ static void ttyport_write_wakeup(struct tty_port *port)
>   struct serport *serport = serdev_controller_get_drvdata(ctrl);
>  
>   if (!test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &port->tty->flags))
> - return;
> + goto out;
>  
>   if (test_bit(SERPORT_ACTIVE, &serport->flags))
>   serdev_controller_write_wakeup(ctrl);
> +
> +out:

I'd do "if (test_and_clear_bit() && test_bit()) serdev_()"
here. Otherwise it looks fine.

Acked-by: Pavel Machek 

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v2 1/9] Coccinelle: locks: identify callers of spin_lock{,_irq,_irqsave}() in irqchip implementations

[Julia Lawall]

On Tue, 21 Mar 2017, Julia Cartwright wrote:

> On PREEMPT_RT, the spinlock_t type becomes an object which sleeps under
> contention.  The codepaths used to support scheduling (irq dispatching, arch
> code, the scheduler, timers) therefore must make use of the
> raw_spin_lock{,_irq,_irqsave}() variations which preserve the non-sleeping
> spinlock behavior.
>
> Because the irq_chip callbacks are invoked in the process of interrupt
> dispatch, they cannot therefore make use of spin_lock_t type.  Instead, the
> usage of raw_spinlock_t is appropriate.
>
> Provide a spatch to identify (and attempt to patch) such problematic irqchip
> implementations.
>
> Note to those generating patches using this spatch; in order to maintain
> correct semantics w/ PREEMPT_RT, it is necessary to audit the
> raw_spinlock_t-protected codepaths to ensure their execution is bounded and
> minimal.  This is a manual audit process.
>
> See commit 47b03ca903fb0 ("pinctrl: qcom: Use raw spinlock variants") as an
> example of _one_ such instance, which fixed a real bug seen in the field.
>
> Cc: Thomas Gleixner 
> Cc: Sebastian Andrzej Siewior 
> Cc: Linus Walleij 
> Signed-off-by: Julia Cartwright 

Acked-by: Julia Lawall 

> ---
> v1 -> v2:
>   - Make use of 'exists' on match to find any codepath which acquires a lock. 
> (via Julia Lawall)
>   - Use 'when any' on '...' matches to loosen constraint (via Julia Lawall).
>
>  .../coccinelle/locks/irq_chip_raw_spinlock.cocci   | 96 
> ++
>  1 file changed, 96 insertions(+)
>  create mode 100644 scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci
>
> diff --git a/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci 
> b/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci
> new file mode 100644
> index ..0294831297d3
> --- /dev/null
> +++ b/scripts/coccinelle/locks/irq_chip_raw_spinlock.cocci
> @@ -0,0 +1,96 @@
> +// Copyright: (C) 2017 National Instruments Corp. GPLv2.
> +// Author: Julia Cartwright 
> +//
> +// Identify callers of non-raw spinlock_t functions in hardirq irq_chip
> +// callbacks.
> +//
> +// spin_lock{_irq,_irqsave}(), w/ PREEMPT_RT are "sleeping" spinlocks, and so
> +// therefore "sleep" under contention; identify (and potentially patch) 
> callers
> +// to use raw_spinlock_t instead.
> +//
> +// Confidence: Moderate
> +
> +virtual report
> +virtual patch
> +
> +@match@
> +identifier __irqchip;
> +identifier __irq_mask;
> +@@
> + static struct irq_chip __irqchip = {
> + .irq_mask   = __irq_mask,
> + };
> +
> +@match2 depends on match exists@
> +identifier match.__irq_mask;
> +identifier data;
> +identifier x;
> +identifier l;
> +type T;
> +position j0;
> +expression flags;
> +@@
> + static void __irq_mask(struct irq_data *data)
> + {
> + ... when any
> + T *x;
> + ... when any
> +(
> + spin_lock_irqsave(&x->l@j0, flags);
> +|
> + spin_lock_irq(&x->l@j0);
> +|
> + spin_lock(&x->l@j0);
> +)
> + ... when any
> + }
> +
> +@match3 depends on match2 && patch@
> +type match2.T;
> +identifier match2.l;
> +@@
> + T {
> + ...
> +-spinlock_t  l;
> ++raw_spinlock_t  l;
> + ...
> + };
> +
> +@match4 depends on match2 && patch@
> +type match2.T;
> +identifier match2.l;
> +expression flags;
> +T *x;
> +@@
> +
> +(
> +-spin_lock(&x->l)
> ++raw_spin_lock(&x->l)
> +|
> +-spin_lock_irqsave(&x->l, flags)
> ++raw_spin_lock_irqsave(&x->l, flags)
> +|
> +-spin_lock_irq(&x->l)
> ++raw_spin_lock_irq(&x->l)
> +|
> +-spin_unlock(&x->l)
> ++raw_spin_unlock(&x->l)
> +|
> +-spin_unlock_irq(&x->l)
> ++raw_spin_unlock_irq(&x->l)
> +|
> +-spin_unlock_irqrestore(&x->l, flags)
> ++raw_spin_unlock_irqrestore(&x->l, flags)
> +|
> +-spin_lock_init(&x->l)
> ++raw_spin_lock_init(&x->l)
> +)
> +
> +@script:python wat depends on match2 && report@
> +j0 << match2.j0;
> +t << match2.T;
> +l << match2.l;
> +@@
> +
> +msg = "Use of non-raw spinlock is illegal in this context (%s::%s)" % (t, l)
> +coccilib.report.print_report(j0[0], msg)
> --
> 2.12.0
>
>

Re: [PATCH v12 4/9] mmc: cavium: Work-around hardware bug on cn6xxx and cnf7xxx

[Jan Glauber]

On Tue, Mar 21, 2017 at 01:22:05PM -0700, David Daney wrote:
> On 03/21/2017 12:49 PM, Arnd Bergmann wrote:
> >On Tue, Mar 21, 2017 at 4:19 PM, David Daney  
> >wrote:
> >>On 03/21/2017 01:58 AM, Arnd Bergmann wrote:
> >>>
> >>>On Mon, Mar 20, 2017 at 9:45 PM, David Daney 
> >>>wrote:
> 
> On 03/17/2017 07:13 AM, Ulf Hansson wrote:
> >
> >My point is really that we should avoid exporting SoC specific APIs
> >which shall be called from drivers. This is old fashion.
> 
> 
> 
> Some people find it objectionable to see 1-off architecture specific
> in-line
> asm in a driver file, but I agree that putting it as close to the user as
> possible makes sense.
> >>>
> >>>
> >>>The proper solution might be to create an architecture independent
> >>>interface
> >>>for it, what it is that the function does. Can you explain what the
> >>>purpose
> >>>of locking/unlocking the cache line for MMC is? Is this something that
> >>>could be done more generally in the dma_map_ops implementation?
> >>
> >>
> >>It is a 1-off erratum workaround that is only needed on fewer than five
> >>models/revisions of a mips64 based SoC family.  As such, creating a general
> >>purpose, architecture independent, framework is clearly not the proper
> >>approach.
> >
> >If this is just for maintaining coherency of the DMA operation inbetween,
> >then there is already a generic API for that, which the driver calls.
> >Adding the workaround into octeon_dma_map_sg() would be a way
> >to abstract the platform erratum from the driver.
> >
> 
> Either I am bad at explaining things, or you are not reading what I wrote.
> 
> These are two facts about the bug:
> 
> 1) The bug has nothing to do with coherency management, so hacking
> something into dma_map* is the wrong thing to do.
> 
> 2) The bug effects exactly one device, so hacking something into
> common code that is used by other devices is the wrong thing to do.
> 
> Suggesting that we use an alternate set of facts, although an
> interesting exercise, doesn't get us closer to answering the
> question of which source code file should contain the code.
> 
> This is one opinion about the bug:
> 
> 1) The bug is in the device, not the "platform", so putting the
> workaround code in the driver for the device may be the cleanest
> approach.

I've moved the code into the octeon driver
(drivers/mmc/host/cavium-octeon.c) and think this is the cleanest
way to do it.

--Jan

> David Daney

Re: [PATCH] staging: rts5208: Replace a bit shift by a use of BIT.

[Dan Carpenter]

On Wed, Mar 22, 2017 at 08:04:47AM +0530, Arushi Singhal wrote:
> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
> 
> -1 << c
> +BIT(c)
> 
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/rts5208/rtsx_chip.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rts5208/rtsx_chip.c 
> b/drivers/staging/rts5208/rtsx_chip.c
> index 3511157a2c78..06a61800b71a 100644
> --- a/drivers/staging/rts5208/rtsx_chip.c
> +++ b/drivers/staging/rts5208/rtsx_chip.c
> @@ -1490,7 +1490,7 @@ int rtsx_write_register(struct rtsx_chip *chip, u16 
> addr, u8 mask, u8 data)
>  
>   for (i = 0; i < MAX_RW_REG_CNT; i++) {
>   val = rtsx_readl(chip, RTSX_HAIMR);
> - if ((val & (1 << 31)) == 0) {
> + if ((val & (BIT(31))) == 0) {

Extra parens.  Please check all your patches again.

regards,
dan carpenter

Re: [RFC][PATCH v3 2/2] cpufreq: schedutil: Avoid reducing frequency of busy CPUs prematurely

[Viresh Kumar]

On 22-03-17, 00:08, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> The way the schedutil governor uses the PELT metric causes it to
> underestimate the CPU utilization in some cases.
> 
> That can be easily demonstrated by running kernel compilation on
> a Sandy Bridge Intel processor, running turbostat in parallel with
> it and looking at the values written to the MSR_IA32_PERF_CTL
> register.  Namely, the expected result would be that when all CPUs
> were 100% busy, all of them would be requested to run in the maximum
> P-state, but observation shows that this clearly isn't the case.
> The CPUs run in the maximum P-state for a while and then are
> requested to run slower and go back to the maximum P-state after
> a while again.  That causes the actual frequency of the processor to
> visibly oscillate below the sustainable maximum in a jittery fashion
> which clearly is not desirable.
> 
> That has been attributed to CPU utilization metric updates on task
> migration that cause the total utilization value for the CPU to be
> reduced by the utilization of the migrated task.  If that happens,
> the schedutil governor may see a CPU utilization reduction and will
> attempt to reduce the CPU frequency accordingly right away.  That
> may be premature, though, for example if the system is generally
> busy and there are other runnable tasks waiting to be run on that
> CPU already.
> 
> This is unlikely to be an issue on systems where cpufreq policies are
> shared between multiple CPUs, because in those cases the policy
> utilization is computed as the maximum of the CPU utilization values
> over the whole policy and if that turns out to be low, reducing the
> frequency for the policy most likely is a good idea anyway.  On
> systems with one CPU per policy, however, it may affect performance
> adversely and even lead to increased energy consumption in some cases.
> 
> On those systems it may be addressed by taking another utilization
> metric into consideration, like whether or not the CPU whose
> frequency is about to be reduced has been idle recently, because if
> that's not the case, the CPU is likely to be busy in the near future
> and its frequency should not be reduced.
> 
> To that end, use the counter of idle calls in the timekeeping code.
> Namely, make the schedutil governor look at that counter for the
> current CPU every time before its frequency is about to be reduced.
> If the counter has not changed since the previous iteration of the
> governor computations for that CPU, the CPU has been busy for all
> that time and its frequency should not be decreased, so if the new
> frequency would be lower than the one set previously, the governor
> will skip the frequency update.
> 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  include/linux/tick.h |1 +
>  kernel/sched/cpufreq_schedutil.c |   27 +++
>  kernel/time/tick-sched.c |   12 
>  3 files changed, 40 insertions(+)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH v2 2/2] iio: Aspeed AST2400/AST2500 ADC

[Joel Stanley]

Hello Quentin,

On Wed, Mar 22, 2017 at 5:51 PM, Quentin Schulz
 wrote:

>> +
>> +#define ASPEED_ADC_CHAN(_idx, _addr) {   \
>> + .type = IIO_VOLTAGE,\
>> + .indexed = 1,   \
>> + .channel = (_idx),  \
>> + .address = (_addr), \
>> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),   \
>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |  \
>> + BIT(IIO_CHAN_INFO_SAMP_FREQ),   \
>> +}
>> +
>> +static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
>> + ASPEED_ADC_CHAN(0, 0x10),
>> + ASPEED_ADC_CHAN(1, 0x12),
>> + ASPEED_ADC_CHAN(2, 0x14),
>> + ASPEED_ADC_CHAN(3, 0x16),
>> + ASPEED_ADC_CHAN(4, 0x18),
>> + ASPEED_ADC_CHAN(5, 0x1A),
>> + ASPEED_ADC_CHAN(6, 0x1C),
>> + ASPEED_ADC_CHAN(7, 0x1E),
>> + ASPEED_ADC_CHAN(8, 0x20),
>> + ASPEED_ADC_CHAN(9, 0x22),
>> + ASPEED_ADC_CHAN(10, 0x24),
>> + ASPEED_ADC_CHAN(11, 0x26),
>> + ASPEED_ADC_CHAN(12, 0x28),
>> + ASPEED_ADC_CHAN(13, 0x2A),
>> + ASPEED_ADC_CHAN(14, 0x2C),
>> + ASPEED_ADC_CHAN(15, 0x2E),
>
> It would make sense to name the registers (the _addr parameter of your
> macro) so it's easier to understand what it refers to.

I appreciate the desire to not have magic values. However, I think
these are okay. We don't use them anywhere else, and it is obvious
from reading that they are the registers containing the ADC values for
each channel.

The alternative would look like this:

+ ASPEED_ADC_CHAN(14, ASPEED_ADC_CHAN_14_DATA),
+ ASPEED_ADC_CHAN(15, ASPEED_ADC_CHAN_15_DATA),

Which doesn't really help me as someone reading the code.

>> + /* Start all channels in normal mode. */
>> + clk_prepare_enable(data->clk_scaler->clk);
>> + adc_engine_control_reg_val = GENMASK(31, 16) |
>> + ASPEED_ADC_OPERATION_MODE_NORMAL | ASPEED_ADC_ENGINE_ENABLE;
>> + writel(adc_engine_control_reg_val,
>> + data->base + ASPEED_ADC_REG_ENGINE_CONTROL);
>> +
>> + indio_dev->name = dev_name(&pdev->dev);
>
> This isn't good practice (cf.: https://lkml.org/lkml/2017/1/28/145 end
> of the mail in the probe function). Better name it aspeed-adc or even
> better to have a different name per compatible: ast2400-adc or ast2500-adc.

The label comes out as "adc.1e6e9000". This is the reg property and
the node name from the device tree, which seems sensible, even if it
is a bit strange to be grabbing the name of the parent device for it.

Could the iio core fill in a sensible name for us here? Rick is the
31st person to make this mistake, so it would be nice to fix properly.

Cheers,

Joel

Re: [PATCH] MAINTAINERS: pinctrl: Add git tree to Samsung pinctrl entry

[Sylwester Nawrocki]

On 03/20/2017 05:48 PM, Krzysztof Kozlowski wrote:

Add a Git tree on @kernel.org for maintaining the Samsung pinctrl
drivers.  The tree will be maintained in a shared model between current
Samsung pinctrl maintainers.  Pull requests will be going to Linus
Walleij.

Also add the patchwork for linux-samsung-soc mailing list which will be
used for handling the patches.

Cc: Tomasz Figa 
Cc: Sylwester Nawrocki 
Cc: Linus Walleij 
Signed-off-by: Krzysztof Kozlowski 


Thanks for your effort Krzysztof,

Acked-by: Sylwester Nawrocki 

Re: [PATCH] tpm/tpm_crb: mark PM functions as __maybe_unused

[Jarkko Sakkinen]

On Mon, Mar 20, 2017 at 10:17:19AM +0100, Arnd Bergmann wrote:
> When CONFIG_PM_SLEEP is disabled, we get a warning about unused functions:
> 
> drivers/char/tpm/tpm_crb.c:551:12: error: 'crb_pm_resume' defined but not 
> used [-Werror=unused-function]
> drivers/char/tpm/tpm_crb.c:540:12: error: 'crb_pm_suspend' defined but not 
> used [-Werror=unused-function]
> 
> We could solve this with more sophistated #ifdefs, but a simpler and safer
> way is to just mark them as __maybe_unused.
> 
> Fixes: 848efcfb560c ("tpm/tpm_crb: enter the low power state upon device 
> suspend")
> Signed-off-by: Arnd Bergmann 

Hi, I somehow missed this and applied very similar patch. Sorry about
that.

/Jarkko

> ---
>  drivers/char/tpm/tpm_crb.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 1dfc37e33c02..15f1118982a6 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -519,8 +519,7 @@ static int crb_acpi_remove(struct acpi_device *device)
>   return 0;
>  }
>  
> -#ifdef CONFIG_PM
> -static int crb_pm_runtime_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_suspend(struct device *dev)
>  {
>   struct tpm_chip *chip = dev_get_drvdata(dev);
>   struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> @@ -528,7 +527,7 @@ static int crb_pm_runtime_suspend(struct device *dev)
>   return crb_go_idle(dev, priv);
>  }
>  
> -static int crb_pm_runtime_resume(struct device *dev)
> +static __maybe_unused int crb_pm_runtime_resume(struct device *dev)
>  {
>   struct tpm_chip *chip = dev_get_drvdata(dev);
>   struct crb_priv *priv = dev_get_drvdata(&chip->dev);
> @@ -536,7 +535,7 @@ static int crb_pm_runtime_resume(struct device *dev)
>   return crb_cmd_ready(dev, priv);
>  }
>  
> -static int crb_pm_suspend(struct device *dev)
> +static __maybe_unused int crb_pm_suspend(struct device *dev)
>  {
>   int ret;
>  
> @@ -547,7 +546,7 @@ static int crb_pm_suspend(struct device *dev)
>   return crb_pm_runtime_suspend(dev);
>  }
>  
> -static int crb_pm_resume(struct device *dev)
> +static __maybe_unused int crb_pm_resume(struct device *dev)
>  {
>   int ret;
>  
> @@ -558,8 +557,6 @@ static int crb_pm_resume(struct device *dev)
>   return tpm_pm_resume(dev);
>  }
>  
> -#endif /* CONFIG_PM */
> -
>  static const struct dev_pm_ops crb_pm = {
>   SET_SYSTEM_SLEEP_PM_OPS(crb_pm_suspend, crb_pm_resume)
>   SET_RUNTIME_PM_OPS(crb_pm_runtime_suspend, crb_pm_runtime_resume, NULL)
> -- 
> 2.9.0
> 

Re: [PATCH] cpufreq: schedutil: Always trace frequency if it does not change

[Viresh Kumar]

On 22-03-17, 10:29, Peter Zijlstra wrote:
> On Wed, Mar 22, 2017 at 01:56:53AM +0100, Rafael J. Wysocki wrote:

> > However, the same problem may occur for a cpufreq driver that doesn't
> > support fast frequency switching and implements the ->target callback
> > (that is, it doesn't use frequency tables), but trace_cpu_frequency()
> > is not called if frequency updates are skipped in that case.
> 
> I'm having trouble parsing that; am I right in understanding this is the
> exact same scenario where only superfluous changes are omitted?

Yes. Just for a different driver type.

-- 
viresh

Re: [PATCH 1/5] clk: meson: Add support for parameters for specific PLLs

[Neil Armstrong]

On 03/22/2017 12:43 AM, Michael Turquette wrote:
> Quoting Neil Armstrong (2017-03-13 06:26:40)
[..]
>>  
>> p = &pll->n;
>> -   ret = meson_clk_pll_wait_lock(pll, p);
>> +   /* If unreset_for_lock is provided, remove the reset bit here */
>> +   if (pll->params.unreset_for_lock) {
> 
> Small nitpick, but I find "unreset" to be confusing. Since 'reset' here
> is a bit that can be set and unset, maybe use clear_reset_for_lock
> instead?
> 
> Regards,
> Mike
> 

You are right, I'll rename it.

Neil

Re: [Outreachy kernel] [PATCH] staging: media: Replace a bit shift by a use of BIT.

[Julia Lawall]

On Wed, 22 Mar 2017, Arushi Singhal wrote:

> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
>
> -1 << c
> +BIT(c)
>
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/octeon/ethernet-tx.c | 4 ++--

What is the connection to media?  You also seem to have picked up the
Lustre maintainers, who are not relevant.

julia

>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/octeon/ethernet-tx.c 
> b/drivers/staging/octeon/ethernet-tx.c
> index ff4119e8de42..f00186ac4e27 100644
> --- a/drivers/staging/octeon/ethernet-tx.c
> +++ b/drivers/staging/octeon/ethernet-tx.c
> @@ -381,7 +381,7 @@ int cvm_oct_xmit(struct sk_buff *skb, struct net_device 
> *dev)
>   (ip_hdr(skb)->version == 4) &&
>   (ip_hdr(skb)->ihl == 5) &&
>   ((ip_hdr(skb)->frag_off == 0) ||
> -  (ip_hdr(skb)->frag_off == htons(1 << 14))) &&
> +  (ip_hdr(skb)->frag_off == htons(BIT(14 &&
>   ((ip_hdr(skb)->protocol == IPPROTO_TCP) ||
>(ip_hdr(skb)->protocol == IPPROTO_UDP))) {
>   /* Use hardware checksum calc */
> @@ -613,7 +613,7 @@ int cvm_oct_xmit_pow(struct sk_buff *skb, struct 
> net_device *dev)
>  #endif
>   work->word2.s.is_frag = !((ip_hdr(skb)->frag_off == 0) ||
> (ip_hdr(skb)->frag_off ==
> -   1 << 14));
> +   BIT(14)));
>  #if 0
>   /* Assume Linux is sending a good packet */
>   work->word2.s.IP_exc = 0;
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170322042028.GA22848%40arushi-HP-Pavilion-Notebook.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [PATCHv2 2/2] super1: check and output faulty dev role

[Jinpu Wang]

On Tue, Mar 21, 2017 at 8:55 PM, NeilBrown  wrote:
> On Mon, Mar 20 2017, Gioh Kim wrote:
>
>> From: Jack Wang 
>>
>> Output the real dev role in examine_super1, it will help to
>> find problem.
>>
>> Signed-off-by: Jack Wang 
>> Reviewed-by: Gioh Kim 
>> ---
>>  super1.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/super1.c b/super1.c
>> index f3520ac..c903371 100644
>> --- a/super1.c
>> +++ b/super1.c
>> @@ -501,8 +501,10 @@ static void examine_super1(struct supertype *st, char 
>> *homehost)
>>  #endif
>>   printf("   Device Role : ");
>>   role = role_from_sb(sb);
>> - if (role >= MD_DISK_ROLE_FAULTY)
>> - printf("spare\n");
>> + if (role == MD_DISK_ROLE_SPARE)
>> + printf("Spare\n");
>> + else if (role == MD_DISK_ROLE_FAULTY)
>> + printf("Faulty\n");
>>   else if (role == MD_DISK_ROLE_JOURNAL)
>>   printf("Journal\n");
>>   else if (sb->feature_map & __cpu_to_le32(MD_FEATURE_REPLACEMENT))
>> --
>> 2.5.0
>
> I don't think the distinction between "faulty" and "spare" is really
> useful here.  I used to report the difference and it turned out to be
> confusing, so we stopped.
>
> This is information stored on some other disk, not the one that is
> spare-or-faulty.  All it needs to know if what other devices are
> working.  It doesn't need to know about which devices aren't working and
> why.
> The distinction between 'faulty' and 'spare' is only relevant to the
> device itself, and to the array as a whole.
>
> We should probably get rid of the distinction between
> MD_DISK_ROLE_FAULTY and MD_DISK_ROLE_SPARE.
> Most places that test for it just test >= MD_DISK_ROLE_FAULTY.
>
> NeilBrown

The reason why I did this change, was during debugging the problem, we
 notice the dev_role was wrong, but
when I print in mdadm it said 'spare', which lead me to check other
kernel code path, so spent more time until, I found
examine_super1, treat >=MD_DISK_ROLE_FAULTY as 'spare'.

I thought if the output was right, it could have saved me or maybe
also other developer some time.

But if this cause confusing in the past, we can drop it, the first is
the real bugfix.

Thanks!
-- 
Jack Wang
Linux Kernel Developer

[PATCH v5] vfio error recovery: kernel support

[Cao jin]

From: "Michael S. Tsirkin" 

0. What happens now (PCIE AER only)
   Fatal errors cause a link reset. Non fatal errors don't.
   All errors stop the QEMU guest eventually, but not immediately,
   because it's detected and reported asynchronously.
   Interrupts are forwarded as usual.
   Correctable errors are not reported to user at all.

   Note:
   PPC EEH is different, but this approach won' affect EEH, because
   EEH treat all errors as fatal ones in AER, will still signal user
   via the legacy eventfd. And all devices/functions in a PE belongs to
   the same IOMMU group, so the slot_reset handler in this approach
   won't affect EEH either.

1. Correctable errors
   Hardware can correct these errors without software intervention,
   clear the error status is enough, this is what already done now.
   No need to recover it, nothing changed, leave it as it is.

2. Fatal errors
   They will induce a link reset. This is troublesome when user is
   a QEMU guest. This approach doens't touch the existing mechanism.

3. Non-fatal errors
   Before, they are signalled to user the same as fatal ones. In this approach,
   a new eventfd is introduced only for non-fatal error notification. By
   splitting non-fatal ones out, it will benefit AER recovery of a QEMU guest
   user by reporting them to guest saparately.

   To maintain backwards compatibility with userspace, non-fatal errors
   will continue to trigger via the existing error interrupt index if a
   non-fatal signaling mechanism has not been registered.

   Note:
   In case of a multi-function device which has different device driver
   for each of them, and one of the functions is bound to vfio while
   others doesn't(i.e., functions belong to different IOMMU group), a new
   slot_reset handler & another new eventfd are introduced. This is
   useful when device driver wants a slot reset while vfio-pci doesn't,
   which means vfio-pci device will got a passive reset. Signal user
   via another new eventfd names passive_reset_trigger, this helps to
   avoid signalling user twice via the same legacy error trigger.

For the original design and discussion, refer:
https://www.spinics.net/lists/linux-virtualization/msg29843.html


Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Cao jin 
---

v5 changelog:
1. Add another new eventfd passive_reset_trigger & the boilerplate code,
   used in slot_reset. Add comment for slot_reset().
2. Rewrite the commit log.

 drivers/vfio/pci/vfio_pci.c | 49 +++--
 drivers/vfio/pci/vfio_pci_intrs.c   | 38 
 drivers/vfio/pci/vfio_pci_private.h |  2 ++
 include/uapi/linux/vfio.h   |  2 ++
 4 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 324c52e..375ba20 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -441,7 +441,9 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device 
*vdev, int irq_type)
 
return (flags & PCI_MSIX_FLAGS_QSIZE) + 1;
}
-   } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX) {
+   } else if (irq_type == VFIO_PCI_ERR_IRQ_INDEX ||
+  irq_type == VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX ||
+  irq_type == VFIO_PCI_PASSIVE_RESET_IRQ_INDEX) {
if (pci_is_pcie(vdev->pdev))
return 1;
} else if (irq_type == VFIO_PCI_REQ_IRQ_INDEX) {
@@ -796,6 +798,8 @@ static long vfio_pci_ioctl(void *device_data,
case VFIO_PCI_REQ_IRQ_INDEX:
break;
case VFIO_PCI_ERR_IRQ_INDEX:
+   case VFIO_PCI_NON_FATAL_ERR_IRQ_INDEX:
+   case VFIO_PCI_PASSIVE_RESET_IRQ_INDEX:
if (pci_is_pcie(vdev->pdev))
break;
/* pass thru to return error */
@@ -1282,7 +1286,9 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct 
pci_dev *pdev,
 
mutex_lock(&vdev->igate);
 
-   if (vdev->err_trigger)
+   if (state == pci_channel_io_normal && vdev->non_fatal_err_trigger)
+   eventfd_signal(vdev->non_fatal_err_trigger, 1);
+   else if (vdev->err_trigger)
eventfd_signal(vdev->err_trigger, 1);
 
mutex_unlock(&vdev->igate);
@@ -1292,8 +1298,47 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct 
pci_dev *pdev,
return PCI_ERS_RESULT_CAN_RECOVER;
 }
 
+/*
+ * In case of a function/device is bound to vfio, while other collateral ones
+ * are still controlled by device driver(i.e., they belongs to different iommu
+ * group), and device driver want a slot reset when seeing AER errors while
+ * vfio pci doesn't, signal user via with proprietary eventfd in precedence to
+ * the legacy one.
+ */
+static pci_ers_result_t vfio_pci_aer_slot_reset(struct pci_dev *pdev)
+{
+   struct vfio_pci_device *vdev;
+   struct vfio_device *device;
+   static 

[PATCH v4 3/3] ARM64: dts: meson-gx: Add MALI nodes for GXBB and GXL

[Neil Armstrong]

The same MALI-450 MP3 GPU is present in the GXBB and GXL SoCs.

The node is simply added in the meson-gxbb.dtsi file.

For GXL, since a lot is shared with the GXM that has a MALI-T820 IP, this
patch adds a new meson-gxl-mali.dtsi and is included in the SoC specific
dtsi files.

Signed-off-by: Neil Armstrong 
---
 arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi  | 37 
 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi  | 43 
 arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi |  1 +
 arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi |  1 +
 4 files changed, 82 insertions(+)
 create mode 100644 arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi

diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
index 04b3324..0617a3d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi
@@ -478,6 +478,43 @@
};
 };
 
+&apb {
+   mali: gpu@c {
+   compatible = "amlogic,meson-gxbb-mali", "arm,mali-450";
+   reg = <0x0 0xc 0x0 0x4>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+   interrupt-names = "gp", "gpmmu", "pp", "pmu",
+   "pp0", "ppmmu0", "pp1", "ppmmu1",
+   "pp2", "ppmmu2";
+   clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_MALI>;
+   clock-names = "bus", "core";
+
+   /*
+* Mali clocking is provided by two identical clock paths
+* MALI_0 and MALI_1 muxed to a single clock by a glitch
+* free mux to safely change frequency while running.
+*/
+   assigned-clocks = <&clkc CLKID_MALI_0_SEL>,
+ <&clkc CLKID_MALI_0>,
+ <&clkc CLKID_MALI>; /* Glitch free mux */
+   assigned-clock-parents = <&clkc CLKID_FCLK_DIV3>,
+<0>, /* Do Nothing */
+<&clkc CLKID_MALI_0>;
+   assigned-clock-rates = <0>, /* Do Nothing */
+  <6>,
+  <0>; /* Do Nothing */
+   };
+};
+
 &i2c_A {
clocks = <&clkc CLKID_I2C>;
 };
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
new file mode 100644
index 000..f06cc234
--- /dev/null
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-mali.dtsi
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2017 BayLibre SAS
+ * Author: Neil Armstrong 
+ *
+ * SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+ */
+
+&apb {
+   mali: gpu@c {
+   compatible = "amlogic,meson-gxbb-mali", "arm,mali-450";
+   reg = <0x0 0xc 0x0 0x4>;
+   interrupts = ,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+   interrupt-names = "gp", "gpmmu", "pp", "pmu",
+   "pp0", "ppmmu0", "pp1", "ppmmu1",
+   "pp2", "ppmmu2";
+   clocks = <&clkc CLKID_CLK81>, <&clkc CLKID_MALI>;
+   clock-names = "bus", "core";
+
+   /*
+* Mali clocking is provided by two identical clock paths
+* MALI_0 and MALI_1 muxed to a single clock by a glitch
+* free mux to safely change frequency while running.
+*/
+   assigned-clocks = <&clkc CLKID_MALI_0_SEL>,
+ <&clkc CLKID_MALI_0>,
+ <&clkc CLKID_MALI>; /* Glitch free mux */
+   assigned-clock-parents = <&clkc CLKID_FCLK_DIV3>,
+<0>, /* Do Nothing */
+<&clkc CLKID_MALI_0>;
+   assigned-clock-rates = <0>, /* Do Nothing */
+  <6>,
+  <0>; /* Do Nothing */
+   };
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi 
b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
index 615308e..5a90e30 100644
--- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905d.dtsi
@@ -42,6 +42,7 @@
  */
 
 #include "meson-gxl.dtsi"
+#include "meson-gxl-mali.dtsi"
 
 / {
compatible = "amlogic,s905d", "amlogic,meson-gxl";
diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x.dtsi 
b/arch/arm64/boo

Re: [PATCH v2 1/7] pinctrl: Renesas RZ/A1 pin and gpio controller

[Geert Uytterhoeven]

Hi Jacopo,

On Mon, Mar 20, 2017 at 5:14 PM, Jacopo Mondi  wrote:
> Add combined gpio and pin controller driver for Renesas RZ/A1
> r7s72100 SoC.
>
> Signed-off-by: Jacopo Mondi 

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-rza1.c

> +/*
> + * This pincontroller/gpio combined driver support Renesas devices of RZ/A1
> + * family.
> + * This includes SoCs which are sub- or super- sets of this particular line,
> + * as RZ/A1H (r7s721000), RZ/A1M (r7s721001) and RZ/A1L (r7s721002) are.

RZ/A1M = r7s721010, RZ/A1L = r7s721020

> +#define RZA1_ADDR(mem, reg, port)  ((mem) + (reg) + ((port) * 4))


> +
> +#define RZA1_NPORTS12
> +#define RZA1_PINS_PER_PORT 16
> +#define RZA1_NPINS (RZA1_PINS_PER_PORT * RZA1_NPORTS)
> +#define RZA1_PIN_TO_PORT(pin)  ((pin) / RZA1_PINS_PER_PORT)
> +#define RZA1_PIN_TO_OFFSET(pin)((pin) % RZA1_PINS_PER_PORT)
> +
> +/*
> + * Be careful here: the pin configuration subnodes in device tree enumerates

enumerate

> + * alternate functions from 1 to 8; subtract 1 before using macros so to 
> match
> + * registers configuration which expects numbers from 0 to 7 instead.

register configuration

> + */
> +#define MUX_FUNC_OFFS  3
> +#define MUX_FUNC_MASK  (BIT(MUX_FUNC_OFFS) - 1)
> +#define MUX_FUNC_PFC_MASK  BIT(0)
> +#define MUX_FUNC_PFCE_MASK BIT(1)
> +#define MUX_FUNC_PFCEA_MASKBIT(2)
> +#define MUX_CONF_BIDIR BIT(0)
> +#define MUX_CONF_SWIO_INPUTBIT(1)
> +#define MUX_CONF_SWIO_OUTPUT   BIT(2)
> +
> +/**
> + * rza1_pin_conf - describes a pin position, id, mux config and output value
> + *
> + * Use uint32_t to match types used in of_device nodes argument lists.
> + *
> + * @id: the pin identifier from 0 to RZA1_NPINS
> + * @port: the port where pin sits on
> + * @offset: pin offset in the port
> + * @mux: alternate function configuration settings
> + * @value: output value to set the pin to
> + */
> +struct rza1_pin_conf {
> +   uint32_t id;
> +   uint32_t port;
> +   uint32_t offset;
> +   uint32_t mux_conf;
> +   uint32_t value;

In the kernel, we tend to use u32 instead of uint32_t.
But many of these fields can be smaller than 32 bits, right, saving some
memory (important when running with built-in RAM only).

> +/**
> + * rza1_pinctrl - RZ pincontroller device
> + *
> + * @dev: parent device structure
> + * @mutex: protect [pinctrl|pinmux]_generic functions
> + * @base: logical address base
> + * @nports: number of pin controller ports
> + * @ports: pin controller banks
> + * @ngpiochips: number of gpio chips
> + * @gpio_ranges: gpio ranges for pinctrl core
> + * @pins: pin array for pinctrl core
> + * @desc: pincontroller desc for pinctrl core
> + * @pctl: pinctrl device
> + */
> +struct rza1_pinctrl {
> +   struct device *dev;
> +
> +   struct mutex mutex;
> +
> +   void __iomem *base;
> +
> +   unsigned int nport;
> +   struct rza1_port *ports;
> +
> +   unsigned int ngpiochips;
> +
> +   struct pinctrl_gpio_range *gpio_ranges;
> +   struct pinctrl_pin_desc *pins;
> +   struct pinctrl_desc desc;
> +   struct pinctrl_dev *pctl;

It's a good idea not to mix pointers and integers, as the pointers will
be 64-bit on 64-bit platforms, leading to gaps due to alignment rules.
Not super-important here, as (for now) this driver is meant for 32-bit SoCs.

> +/**
> + * rza1_set_bit() - un-locked set/clear a single bit in pin configuration
> + * registers
> + */
> +static inline void rza1_set_bit(const struct rza1_port *port,
> +   unsigned int reg, unsigned int offset,
> +   bool set)

I think "reg" and "set" still fit on the previous lines (many more cases
in other functions).

I'd call "offset" "bit" (and "reg" "offset"?)

> +{
> +   void __iomem *mem = RZA1_ADDR(port->base, reg, port->id);

I think this would be easier to read without using the RZA1_ADDR() macro.

> +   u16 val = ioread16(mem);
> +
> +   if (set)
> +   val |= BIT(offset);
> +   else
> +   val &= ~BIT(offset);
> +
> +   iowrite16(val, mem);
> +}
> +
> +static inline int rza1_get_bit(struct rza1_port *port,
> +  unsigned int reg, unsigned int offset)
> +{
> +   void __iomem *mem = RZA1_ADDR(port->base, reg, port->id);
> +
> +   return ioread16(mem) & BIT(offset);

Same comments as for rza1_set_bit().

> +}
> +
> +/**
> + * rza1_pin_reset() - reset a pin to default initial state
> + *
> + * Reset pin state disabling input buffer and bi-directional control,
> + * and configure it as input port.
> + * Note that pin is now configured with direction as input but with input
> + * buffer disabled. This implies the pin value cannot be read in this state.
> + *
> + * @port: port where pin sits on
> + * @offset: pin o

[PATCH v4 2/3] clk: meson-gxbb: Add MALI clocks

[Neil Armstrong]

The Mali is clocked by two identical clock paths behind a glitch free mux
to safely change frequency while running.

The two "mali_0" and "mali_1" clocks are composed of a mux, divider and gate.
Expose these two clocks trees using generic clocks.
Finally the glitch free mux is added as "mali" clock.

Signed-off-by: Neil Armstrong 
---
 drivers/clk/meson/gxbb.c | 139 +++
 1 file changed, 139 insertions(+)

diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index 5059c7b..d07dc22 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -634,6 +634,131 @@
},
 };
 
+/*
+ * The MALI IP is clocked by two identical clocks (mali_0 and mali_1)
+ * muxed by a glitch-free switch.
+ */
+
+static u32 mux_table_mali_0_1[] = {0, 1, 2, 3, 4, 5, 6, 7};
+static const char *gxbb_mali_0_1_parent_names[] = {
+   "xtal", "gp0_pll", "mpll2", "mpll1", "fclk_div7",
+   "fclk_div4", "fclk_div3", "fclk_div5"
+};
+
+static struct clk_mux gxbb_mali_0_sel = {
+   .reg = (void *)HHI_MALI_CLK_CNTL,
+   .mask = 0x7,
+   .shift = 9,
+   .table = mux_table_mali_0_1,
+   .lock = &clk_lock,
+   .hw.init = &(struct clk_init_data){
+   .name = "mali_0_sel",
+   .ops = &clk_mux_ops,
+   /*
+* bits 10:9 selects from 8 possible parents:
+* xtal, gp0_pll, mpll2, mpll1, fclk_div7,
+* fclk_div4, fclk_div3, fclk_div5
+*/
+   .parent_names = gxbb_mali_0_1_parent_names,
+   .num_parents = 8,
+   .flags = CLK_SET_RATE_NO_REPARENT,
+   },
+};
+
+static struct clk_divider gxbb_mali_0_div = {
+   .reg = (void *)HHI_MALI_CLK_CNTL,
+   .shift = 0,
+   .width = 7,
+   .lock = &clk_lock,
+   .hw.init = &(struct clk_init_data){
+   .name = "mali_0_div",
+   .ops = &clk_divider_ops,
+   .parent_names = (const char *[]){ "mali_0_sel" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_NO_REPARENT,
+   },
+};
+
+static struct clk_gate gxbb_mali_0 = {
+   .reg = (void *)HHI_MALI_CLK_CNTL,
+   .bit_idx = 8,
+   .lock = &clk_lock,
+   .hw.init = &(struct clk_init_data){
+   .name = "mali_0",
+   .ops = &clk_gate_ops,
+   .parent_names = (const char *[]){ "mali_0_div" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static struct clk_mux gxbb_mali_1_sel = {
+   .reg = (void *)HHI_MALI_CLK_CNTL,
+   .mask = 0x7,
+   .shift = 25,
+   .table = mux_table_mali_0_1,
+   .lock = &clk_lock,
+   .hw.init = &(struct clk_init_data){
+   .name = "mali_1_sel",
+   .ops = &clk_mux_ops,
+   /*
+* bits 10:9 selects from 8 possible parents:
+* xtal, gp0_pll, mpll2, mpll1, fclk_div7,
+* fclk_div4, fclk_div3, fclk_div5
+*/
+   .parent_names = gxbb_mali_0_1_parent_names,
+   .num_parents = 8,
+   .flags = CLK_SET_RATE_NO_REPARENT,
+   },
+};
+
+static struct clk_divider gxbb_mali_1_div = {
+   .reg = (void *)HHI_MALI_CLK_CNTL,
+   .shift = 16,
+   .width = 7,
+   .lock = &clk_lock,
+   .hw.init = &(struct clk_init_data){
+   .name = "mali_1_div",
+   .ops = &clk_divider_ops,
+   .parent_names = (const char *[]){ "mali_1_sel" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_NO_REPARENT,
+   },
+};
+
+static struct clk_gate gxbb_mali_1 = {
+   .reg = (void *)HHI_MALI_CLK_CNTL,
+   .bit_idx = 24,
+   .lock = &clk_lock,
+   .hw.init = &(struct clk_init_data){
+   .name = "mali_1",
+   .ops = &clk_gate_ops,
+   .parent_names = (const char *[]){ "mali_1_div" },
+   .num_parents = 1,
+   .flags = CLK_SET_RATE_PARENT,
+   },
+};
+
+static u32 mux_table_mali[] = {0, 1};
+static const char *gxbb_mali_parent_names[] = {
+   "mali_0", "mali_1"
+};
+
+static struct clk_mux gxbb_mali = {
+   .reg = (void *)HHI_MALI_CLK_CNTL,
+   .mask = 1,
+   .shift = 31,
+   .table = mux_table_mali,
+   .lock = &clk_lock,
+   .hw.init = &(struct clk_init_data){
+   .name = "mali",
+   .ops = &clk_mux_ops,
+   .parent_names = gxbb_mali_parent_names,
+   .num_parents = 2,
+   .flags = CLK_SET_RATE_NO_REPARENT,
+   },
+};
+
 /* Everything Else (EE) domain gates */
 static MESON_GATE(gxbb_ddr, HHI_GCLK_MPEG0, 0);
 static MESON_GATE(gxbb_dos, HHI_GCLK_MPEG0, 1);
@@ -827,6 +952,13 @@
[CLKID_SAR_ADC_CLK] = &gxbb_sar_adc_clk.hw,
[CLKID_SAR_ADC_SEL] = &gxbb_sar_adc_clk_sel.hw,
[CLKID_SAR_ADC_DIV] = &gxbb_sar_a