[PATCH v10 1/7] ACPI / processor_idle: introduce ACPI_PROCESSOR_CSTATE
ACPI 6.0 adds a new method to specify the CPU idle states(C-states) called Low Power Idle(LPI) states. Since new architectures like ARM64 use only LPIs, introduce ACPI_PROCESSOR_CSTATE to encapsulate all the code supporting the old style C-states(_CST). This patch will help to extend the processor_idle module to support LPI. Cc: "Rafael J. Wysocki" Signed-off-by: Sudeep Holla --- drivers/acpi/Kconfig | 4 +++ drivers/acpi/processor_idle.c | 80 --- include/acpi/processor.h | 2 +- 3 files changed, 58 insertions(+), 28 deletions(-) diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index b7e2e776397d..1358fb7d7a68 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -213,6 +213,10 @@ config ACPI_CPU_FREQ_PSS bool select THERMAL +config ACPI_PROCESSOR_CSTATE + def_bool y + depends on IA64 || X86 + config ACPI_PROCESSOR_IDLE bool select CPU_IDLE diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 444e3745c8b3..ca0de35d1c3a 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -59,6 +59,12 @@ module_param(latency_factor, uint, 0644); static DEFINE_PER_CPU(struct cpuidle_device *, acpi_cpuidle_device); +struct cpuidle_driver acpi_idle_driver = { + .name = "acpi_idle", + .owner =THIS_MODULE, +}; + +#ifdef CONFIG_ACPI_PROCESSOR_CSTATE static DEFINE_PER_CPU(struct acpi_processor_cx * [CPUIDLE_STATE_MAX], acpi_cstate); @@ -804,11 +810,6 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev, acpi_idle_do_entry(cx); } -struct cpuidle_driver acpi_idle_driver = { - .name = "acpi_idle", - .owner =THIS_MODULE, -}; - /** * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE * device i.e. per-cpu data @@ -925,6 +926,50 @@ static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr) return 0; } +static inline void acpi_processor_cstate_first_run_checks(void) +{ + acpi_status status; + static int first_run; + + if (first_run) + return; + dmi_check_system(processor_power_dmi_table); + max_cstate = acpi_processor_cstate_check(max_cstate); + if (max_cstate < ACPI_C_STATES_MAX) + pr_notice("ACPI: processor limited to max C-state %d\n", + max_cstate); + first_run++; + + if (acpi_gbl_FADT.cst_control && !nocst) { + status = acpi_os_write_port(acpi_gbl_FADT.smi_command, + acpi_gbl_FADT.cst_control, 8); + if (ACPI_FAILURE(status)) + ACPI_EXCEPTION((AE_INFO, status, + "Notifying BIOS of _CST ability failed")); + } +} +#else + +static inline int disabled_by_idle_boot_param(void) { return 0; } +static inline void acpi_processor_cstate_first_run_checks(void) { } +static int acpi_processor_get_power_info(struct acpi_processor *pr) +{ + return -ENODEV; +} + +static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr, + struct cpuidle_device *dev) +{ + return -EINVAL; +} + +static int acpi_processor_setup_cpuidle_states(struct acpi_processor *pr) +{ + return -EINVAL; +} + +#endif /* CONFIG_ACPI_PROCESSOR_CSTATE */ + int acpi_processor_hotplug(struct acpi_processor *pr) { int ret = 0; @@ -1015,35 +1060,16 @@ static int acpi_processor_registered; int acpi_processor_power_init(struct acpi_processor *pr) { - acpi_status status; int retval; struct cpuidle_device *dev; - static int first_run; if (disabled_by_idle_boot_param()) return 0; - if (!first_run) { - dmi_check_system(processor_power_dmi_table); - max_cstate = acpi_processor_cstate_check(max_cstate); - if (max_cstate < ACPI_C_STATES_MAX) - printk(KERN_NOTICE - "ACPI: processor limited to max C-state %d\n", - max_cstate); - first_run++; - } + acpi_processor_cstate_first_run_checks(); - if (acpi_gbl_FADT.cst_control && !nocst) { - status = - acpi_os_write_port(acpi_gbl_FADT.smi_command, acpi_gbl_FADT.cst_control, 8); - if (ACPI_FAILURE(status)) { - ACPI_EXCEPTION((AE_INFO, status, - "Notifying BIOS of _CST ability failed")); - } - } - - acpi_processor_get_power_info(pr); - pr->flags.power_setup_done = 1; + if (!acpi_processor_get_power_info(pr)) + pr->flags.power_setup_done = 1; /*
[PATCH v10 5/7] drivers: firmware: psci: initialise idle states using ACPI LPI
This patch adds support for initialisation of PSCI CPUIdle states from Low Power Idle(_LPI) entries in the ACPI tables when acpi is enabled. Cc: Mark Rutland Cc: "Rafael J. Wysocki" Acked-by: Lorenzo Pieralisi Signed-off-by: Sudeep Holla --- drivers/firmware/psci.c | 66 +++-- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index 03e04582791c..8263429e21b8 100644 --- a/drivers/firmware/psci.c +++ b/drivers/firmware/psci.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) "psci: " fmt +#include #include #include #include @@ -256,13 +257,6 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu) u32 *psci_states; struct device_node *state_node; - /* -* If the PSCI cpu_suspend function hook has not been initialized -* idle states must not be enabled, so bail out -*/ - if (!psci_ops.cpu_suspend) - return -EOPNOTSUPP; - /* Count idle states */ while ((state_node = of_parse_phandle(cpu_node, "cpu-idle-states", count))) { @@ -310,11 +304,69 @@ static int psci_dt_cpu_init_idle(struct device_node *cpu_node, int cpu) return ret; } +#ifdef CONFIG_ACPI +#include + +static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu) +{ + int i, count; + u32 *psci_states; + struct acpi_lpi_state *lpi; + struct acpi_processor *pr = per_cpu(processors, cpu); + + if (unlikely(!pr || !pr->flags.has_lpi)) + return -EINVAL; + + count = pr->power.count - 1; + if (count <= 0) + return -ENODEV; + + psci_states = kcalloc(count, sizeof(*psci_states), GFP_KERNEL); + if (!psci_states) + return -ENOMEM; + + for (i = 0; i < count; i++) { + u32 state; + + lpi = &pr->power.lpi_states[i + 1]; + /* +* Only bits[31:0] represent a PSCI power_state while +* bits[63:32] must be 0x0 as per ARM ACPI FFH Specification +*/ + state = lpi->address; + if (!psci_power_state_is_valid(state)) { + pr_warn("Invalid PSCI power state %#x\n", state); + kfree(psci_states); + return -EINVAL; + } + psci_states[i] = state; + } + /* Idle states parsed correctly, initialize per-cpu pointer */ + per_cpu(psci_power_state, cpu) = psci_states; + return 0; +} +#else +static int __maybe_unused psci_acpi_cpu_init_idle(unsigned int cpu) +{ + return -EINVAL; +} +#endif + int psci_cpu_init_idle(unsigned int cpu) { struct device_node *cpu_node; int ret; + /* +* If the PSCI cpu_suspend function hook has not been initialized +* idle states must not be enabled, so bail out +*/ + if (!psci_ops.cpu_suspend) + return -EOPNOTSUPP; + + if (!acpi_disabled) + return psci_acpi_cpu_init_idle(cpu); + cpu_node = of_get_cpu_node(cpu, NULL); if (!cpu_node) return -ENODEV; -- 2.7.4
[PATCH v10 4/7] cpuidle: introduce CPU_PM_CPU_IDLE_ENTER macro for ARM{32,64}
The function arm_enter_idle_state is exactly the same in both generic ARM{32,64} CPUIdle driver and will be the same even on ARM64 backend for ACPI processor idle driver. So we can unify it and move it to a common place by introducing CPU_PM_CPU_IDLE_ENTER macro that can be used in all places avoiding duplication. This is in preparation of reuse of the generic cpuidle entry function for ACPI LPI support on ARM64. Cc: "Rafael J. Wysocki" Cc: Daniel Lezcano Cc: Lorenzo Pieralisi Suggested-by: "Rafael J. Wysocki" Signed-off-by: Sudeep Holla --- drivers/cpuidle/cpuidle-arm.c | 26 ++ include/linux/cpuidle.h | 18 ++ 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c index e342565e8715..4ba3d3fe142f 100644 --- a/drivers/cpuidle/cpuidle-arm.c +++ b/drivers/cpuidle/cpuidle-arm.c @@ -36,26 +36,12 @@ static int arm_enter_idle_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, int idx) { - int ret; - - if (!idx) { - cpu_do_idle(); - return idx; - } - - ret = cpu_pm_enter(); - if (!ret) { - /* -* Pass idle state index to cpu_suspend which in turn will -* call the CPU ops suspend protocol with idle index as a -* parameter. -*/ - ret = arm_cpuidle_suspend(idx); - - cpu_pm_exit(); - } - - return ret ? -1 : idx; + /* +* Pass idle state index to arm_cpuidle_suspend which in turn +* will call the CPU ops suspend protocol with idle index as a +* parameter. +*/ + return CPU_PM_CPU_IDLE_ENTER(arm_cpuidle_suspend, idx); } static struct cpuidle_driver arm_idle_driver = { diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index 07b83d32f66c..bb31373c3478 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -252,4 +252,22 @@ static inline int cpuidle_register_governor(struct cpuidle_governor *gov) #define CPUIDLE_DRIVER_STATE_START 0 #endif +#define CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, idx) \ +({ \ + int __ret; \ + \ + if (!idx) { \ + cpu_do_idle(); \ + return idx; \ + } \ + \ + __ret = cpu_pm_enter(); \ + if (!__ret) { \ + __ret = low_level_idle_enter(idx); \ + cpu_pm_exit(); \ + } \ + \ + __ret ? -1 : idx; \ +}) + #endif /* _LINUX_CPUIDLE_H */ -- 2.7.4
[PATCH v10 2/7] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
ACPI 6.0 introduced an optional object _LPI that provides an alternate method to describe Low Power Idle states. It defines the local power states for each node in a hierarchical processor topology. The OSPM can use _LPI object to select a local power state for each level of processor hierarchy in the system. They used to produce a composite power state request that is presented to the platform by the OSPM. Since multiple processors affect the idle state for any non-leaf hierarchy node, coordination of idle state requests between the processors is required. ACPI supports two different coordination schemes: Platform coordinated and OS initiated. This patch adds initial support for Platform coordination scheme of LPI. Cc: "Rafael J. Wysocki" Signed-off-by: Sudeep Holla --- drivers/acpi/bus.c | 14 +- drivers/acpi/processor_driver.c | 2 +- drivers/acpi/processor_idle.c | 462 +++- include/acpi/processor.h| 24 ++- include/linux/acpi.h| 4 + 5 files changed, 446 insertions(+), 60 deletions(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 262ca31b86d9..80ebb05e387c 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -302,6 +302,14 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context) EXPORT_SYMBOL(acpi_run_osc); bool osc_sb_apei_support_acked; + +/* + * ACPI 6.0 Section 8.4.4.2 Idle State Coordination + * OSPM supports platform coordinated low power idle(LPI) states + */ +bool osc_pc_lpi_support_confirmed; +EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed); + static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48"; static void acpi_bus_osc_support(void) { @@ -322,6 +330,7 @@ static void acpi_bus_osc_support(void) capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PPC_OST_SUPPORT; capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT; + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT; if (!ghes_disable) capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT; @@ -329,9 +338,12 @@ static void acpi_bus_osc_support(void) return; if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) { u32 *capbuf_ret = context.ret.pointer; - if (context.ret.length > OSC_SUPPORT_DWORD) + if (context.ret.length > OSC_SUPPORT_DWORD) { osc_sb_apei_support_acked = capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; + osc_pc_lpi_support_confirmed = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; + } kfree(context.ret.pointer); } /* do we need to check other returned cap? Sounds no */ diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c index d2fa8cb82d2b..0ca14ac7bb28 100644 --- a/drivers/acpi/processor_driver.c +++ b/drivers/acpi/processor_driver.c @@ -90,7 +90,7 @@ static void acpi_processor_notify(acpi_handle handle, u32 event, void *data) pr->performance_platform_limit); break; case ACPI_PROCESSOR_NOTIFY_POWER: - acpi_processor_cst_has_changed(pr); + acpi_processor_power_state_has_changed(pr); acpi_bus_generate_netlink_event(device->pnp.device_class, dev_name(&device->dev), event, 0); break; diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index ca0de35d1c3a..fced1df535bd 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -303,7 +303,6 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr) struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; union acpi_object *cst; - if (nocst) return -ENODEV; @@ -576,7 +575,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr) return (working); } -static int acpi_processor_get_power_info(struct acpi_processor *pr) +static int acpi_processor_get_cstate_info(struct acpi_processor *pr) { unsigned int i; int result; @@ -810,31 +809,12 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev, acpi_idle_do_entry(cx); } -/** - * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE - * device i.e. per-cpu data - * - * @pr: the ACPI processor - * @dev : the cpuidle device - */ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr, struct cpuidle_device *dev) { int i, count = CPUIDLE_DRIVER_STATE_START; struct acpi_processor_cx *cx; - if (!pr->flags.power_setup_done) - return -EINVAL; - - if (pr->flags.power == 0) { -
Re: [PATCH v10 2/7] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
On 21/07/16 14:34, Rafael J. Wysocki wrote: On Tuesday, July 19, 2016 06:52:54 PM Sudeep Holla wrote: ACPI 6.0 introduced an optional object _LPI that provides an alternate method to describe Low Power Idle states. It defines the local power states for each node in a hierarchical processor topology. The OSPM can use _LPI object to select a local power state for each level of processor hierarchy in the system. They used to produce a composite power state request that is presented to the platform by the OSPM. Since multiple processors affect the idle state for any non-leaf hierarchy node, coordination of idle state requests between the processors is required. ACPI supports two different coordination schemes: Platform coordinated and OS initiated. This patch adds initial support for Platform coordination scheme of LPI. Cc: "Rafael J. Wysocki" Signed-off-by: Sudeep Holla --- drivers/acpi/bus.c | 14 +- drivers/acpi/processor_driver.c | 2 +- drivers/acpi/processor_idle.c | 462 +++- include/acpi/processor.h| 24 ++- include/linux/acpi.h| 4 + 5 files changed, 446 insertions(+), 60 deletions(-) [cut] +static int acpi_processor_get_lpi_info(struct acpi_processor *pr) +{ + int ret, i; + acpi_status status; + acpi_handle handle = pr->handle, pr_ahandle; + struct acpi_device *d = NULL; + struct acpi_lpi_states_array info[2], *tmp, *prev, *curr; + + if (!osc_pc_lpi_support_confirmed) + return -EOPNOTSUPP; + + if (!acpi_has_method(handle, "_LPI")) + return -EINVAL; + + flat_state_cnt = 0; + prev = &info[0]; + curr = &info[1]; + handle = pr->handle; + ret = acpi_processor_evaluate_lpi(handle, prev); + if (ret) + return ret; + flatten_lpi_states(pr, prev, NULL); + + while (ACPI_SUCCESS(status = acpi_get_parent(handle, &pr_ahandle))) { I should have mentioned that earlier, but forgot, sorry about that. Assignments under while () etc are generally discouraged as (a) error-prone and (b) confusing to static analysis tools. So I'd do status = acpi_get_parent(handle, &pr_ahandle); while (ACPI_SUCCESS(status)) { Sure, will update accordingly. + acpi_bus_get_device(pr_ahandle, &d); + handle = pr_ahandle; + + if (strcmp(acpi_device_hid(d), ACPI_PROCESSOR_CONTAINER_HID)) + break; + + /* can be optional ? */ + if (!acpi_has_method(handle, "_LPI")) + break; + + ret = acpi_processor_evaluate_lpi(handle, curr); + if (ret) + break; + + /* flatten all the LPI states in this level of hierarchy */ + flatten_lpi_states(pr, curr, prev); + + tmp = prev, prev = curr, curr = tmp; status = acpi_get_parent(handle, &pr_ahandle); + } + OK Apart from this the patch looks OK to me, so please only update this one and I'll queue up the series. Thanks, will do it shortly. Also I found a bug in my testing creating some fake tables to test this non-recursive logic. I have missed a pointer update in the inner loop. I will include the below one liner in the update. -->8 diff --git i/drivers/acpi/processor_idle.c w/drivers/acpi/processor_idle.c index fced1df535bd..c8800b55268d 100644 --- i/drivers/acpi/processor_idle.c +++ w/drivers/acpi/processor_idle.c @@ -1142,6 +1142,7 @@ static int flatten_lpi_states(struct acpi_processor *pr, combine_lpi_states(p, t, flpi)) { stash_composite_state(curr_level, flpi); flat_state_cnt++; + flpi++; } } } -- Regards, Sudeep
[PATCH v10 2/7] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
ACPI 6.0 introduced an optional object _LPI that provides an alternate method to describe Low Power Idle states. It defines the local power states for each node in a hierarchical processor topology. The OSPM can use _LPI object to select a local power state for each level of processor hierarchy in the system. They used to produce a composite power state request that is presented to the platform by the OSPM. Since multiple processors affect the idle state for any non-leaf hierarchy node, coordination of idle state requests between the processors is required. ACPI supports two different coordination schemes: Platform coordinated and OS initiated. This patch adds initial support for Platform coordination scheme of LPI. Cc: "Rafael J. Wysocki" Signed-off-by: Sudeep Holla --- drivers/acpi/bus.c | 14 +- drivers/acpi/processor_driver.c | 2 +- drivers/acpi/processor_idle.c | 466 +++- include/acpi/processor.h| 24 ++- include/linux/acpi.h| 4 + 5 files changed, 450 insertions(+), 60 deletions(-) Updates: - Fixed the missing pointer update in the inner loop. - Removed assignments under while () as suggested. diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 262ca31b86d9..80ebb05e387c 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -302,6 +302,14 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context) EXPORT_SYMBOL(acpi_run_osc); bool osc_sb_apei_support_acked; + +/* + * ACPI 6.0 Section 8.4.4.2 Idle State Coordination + * OSPM supports platform coordinated low power idle(LPI) states + */ +bool osc_pc_lpi_support_confirmed; +EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed); + static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48"; static void acpi_bus_osc_support(void) { @@ -322,6 +330,7 @@ static void acpi_bus_osc_support(void) capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PPC_OST_SUPPORT; capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_HOTPLUG_OST_SUPPORT; + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_PCLPI_SUPPORT; if (!ghes_disable) capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_APEI_SUPPORT; @@ -329,9 +338,12 @@ static void acpi_bus_osc_support(void) return; if (ACPI_SUCCESS(acpi_run_osc(handle, &context))) { u32 *capbuf_ret = context.ret.pointer; - if (context.ret.length > OSC_SUPPORT_DWORD) + if (context.ret.length > OSC_SUPPORT_DWORD) { osc_sb_apei_support_acked = capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; + osc_pc_lpi_support_confirmed = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; + } kfree(context.ret.pointer); } /* do we need to check other returned cap? Sounds no */ diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c index d2fa8cb82d2b..0ca14ac7bb28 100644 --- a/drivers/acpi/processor_driver.c +++ b/drivers/acpi/processor_driver.c @@ -90,7 +90,7 @@ static void acpi_processor_notify(acpi_handle handle, u32 event, void *data) pr->performance_platform_limit); break; case ACPI_PROCESSOR_NOTIFY_POWER: - acpi_processor_cst_has_changed(pr); + acpi_processor_power_state_has_changed(pr); acpi_bus_generate_netlink_event(device->pnp.device_class, dev_name(&device->dev), event, 0); break; diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index ca0de35d1c3a..cea52528aa18 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -303,7 +303,6 @@ static int acpi_processor_get_power_info_cst(struct acpi_processor *pr) struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; union acpi_object *cst; - if (nocst) return -ENODEV; @@ -576,7 +575,7 @@ static int acpi_processor_power_verify(struct acpi_processor *pr) return (working); } -static int acpi_processor_get_power_info(struct acpi_processor *pr) +static int acpi_processor_get_cstate_info(struct acpi_processor *pr) { unsigned int i; int result; @@ -810,31 +809,12 @@ static void acpi_idle_enter_freeze(struct cpuidle_device *dev, acpi_idle_do_entry(cx); } -/** - * acpi_processor_setup_cpuidle_cx - prepares and configures CPUIDLE - * device i.e. per-cpu data - * - * @pr: the ACPI processor - * @dev : the cpuidle device - */ static int acpi_processor_setup_cpuidle_cx(struct acpi_processor *pr, struct cpuidle_device *dev) { int i, count = CPUIDLE_DRIVER_STATE_START; struct acpi_processo
Re: [PATCH v3 1/8] scpi: Add cmd indirection table to prepare for legacy commands
Hi Neil, On 07/09/16 16:34, Neil Armstrong wrote: Add indirection table to permit multiple command values for legacy support. I wrote the most of the patch and you changed the author too ;) Signed-off-by: Neil Armstrong --- drivers/firmware/arm_scpi.c | 145 ++-- 1 file changed, 127 insertions(+), 18 deletions(-) diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c index 4388937..9a87687 100644 --- a/drivers/firmware/arm_scpi.c +++ b/drivers/firmware/arm_scpi.c [..] @@ -161,6 +194,7 @@ struct scpi_drvinfo { u32 protocol_version; u32 firmware_version; int num_chans; + int *scpi_cmds; atomic_t next_chan; struct scpi_ops *scpi_ops; struct scpi_chan *channels; @@ -390,6 +424,19 @@ static u32 scpi_get_version(void) return scpi_info->protocol_version; } +static inline int check_cmd(unsigned int offset) +{ + if (offset >= CMD_MAX_COUNT || If we call scpi_send_message internally(as it's static) why is this check needed ? + !scpi_info || + !scpi_info->scpi_cmds) Will be even reach to this point if above is true ? + return -EINVAL; + + if (scpi_info->scpi_cmds[offset] < 0) + return -EOPNOTSUPP; IMO just above couple of lines in the beginning of scpi_send_message will suffice. You can just add this to my original patch. static int scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max) { @@ -397,8 +444,13 @@ scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max) struct clk_get_info clk; __le16 le_clk_id = cpu_to_le16(clk_id); - ret = scpi_send_message(SCPI_CMD_GET_CLOCK_INFO, &le_clk_id, - sizeof(le_clk_id), &clk, sizeof(clk)); + ret = check_cmd(CMD_GET_CLOCK_INFO); + if (ret) + return ret; + It's totally unnecessary to add check in each and every function calling scpi_send_message, why not add it to scpi_send_message instead. -- Regards, Sudeep
Re: [PATCH v3 2/8] scpi: Add alternative legacy structures, functions and macros
On 07/09/16 16:34, Neil Armstrong wrote: In order to support the legacy SCPI protocol variant, add back the structures and macros that varies against the final specification. Add indirection table for legacy commands. Add bitmap field for channel selection Add support for legacy in scpi_send_message. Signed-off-by: Neil Armstrong --- drivers/firmware/arm_scpi.c | 218 ++-- 1 file changed, 211 insertions(+), 7 deletions(-) diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c index 9a87687..9ba1020 100644 --- a/drivers/firmware/arm_scpi.c +++ b/drivers/firmware/arm_scpi.c [..] @@ -336,6 +424,39 @@ static void scpi_handle_remote_msg(struct mbox_client *c, void *msg) scpi_process_cmd(ch, cmd); } +static void legacy_scpi_process_cmd(struct scpi_chan *ch) +{ + unsigned long flags; + struct scpi_xfer *t; + + spin_lock_irqsave(&ch->rx_lock, flags); + if (list_empty(&ch->rx_pending)) { + spin_unlock_irqrestore(&ch->rx_lock, flags); + return; + } + + t = list_first_entry(&ch->rx_pending, struct scpi_xfer, node); + list_del(&t->node); + This is a bad assumption that it will be always first. The legacy SCPI did support multiple commands at a time and they can be reordered when SCP responds to them. Except this it's almost same scpi_process_cmd. You should be able to use it as is if you pass the command. + /* check if wait_for_completion is in progress or timed-out */ + if (t && !completion_done(&t->done)) { + struct legacy_scpi_shared_mem *mem = ch->rx_payload; + unsigned int len = t->rx_len; + + t->status = le32_to_cpu(mem->status); + memcpy_fromio(t->rx_buf, mem->payload, len); + complete(&t->done); + } + spin_unlock_irqrestore(&ch->rx_lock, flags); +} + +static void legacy_scpi_handle_remote_msg(struct mbox_client *c, void *_msg) +{ + struct scpi_chan *ch = container_of(c, struct scpi_chan, cl); + + legacy_scpi_process_cmd(ch); You will get the command in *_msg IIRC. So you can just pass that to scpi_process_cmd. You can even reuse scpi_handle_remote_msg diff --git i/drivers/firmware/arm_scpi.c w/drivers/firmware/arm_scpi.c index edf1a3327041..165f2fc3b627 100644 --- i/drivers/firmware/arm_scpi.c +++ w/drivers/firmware/arm_scpi.c @@ -419,7 +419,12 @@ static void scpi_handle_remote_msg(struct mbox_client *c, void *msg) { struct scpi_chan *ch = container_of(c, struct scpi_chan, cl); struct scpi_shared_mem *mem = ch->rx_payload; - u32 cmd = le32_to_cpu(mem->command); + u32 cmd; + + if (ch->is_legacy) + cmd = *(u32 *)msg; + else + cmd = le32_to_cpu(mem->command); scpi_process_cmd(ch, cmd); } +} + static void scpi_tx_prepare(struct mbox_client *c, void *msg) { unsigned long flags; @@ -356,6 +477,21 @@ static void scpi_tx_prepare(struct mbox_client *c, void *msg) mem->command = cpu_to_le32(t->cmd); } +static void legacy_scpi_tx_prepare(struct mbox_client *c, void *msg) +{ + unsigned long flags; + struct scpi_xfer *t = msg; + struct scpi_chan *ch = container_of(c, struct scpi_chan, cl); + + if (t->tx_buf) + memcpy_toio(ch->tx_payload, t->tx_buf, t->tx_len); + if (t->rx_buf) { + spin_lock_irqsave(&ch->rx_lock, flags); + list_add_tail(&t->node, &ch->rx_pending); + spin_unlock_irqrestore(&ch->rx_lock, flags); + } +} Again here the only difference is token addition. I think we should retain that as it's helpful in debugging and I don't think it will have any issues. Worst case we can make it conditional but let's check if we can retain it first. @@ -386,15 +522,25 @@ static int scpi_send_message(u8 cmd, void *tx_buf, unsigned int tx_len, struct scpi_xfer *msg; struct scpi_chan *scpi_chan; - chan = atomic_inc_return(&scpi_info->next_chan) % scpi_info->num_chans; + if (scpi_info->is_legacy) + chan = test_bit(cmd, scpi_info->cmd_priority) ? 1 : 0; + else + chan = atomic_inc_return(&scpi_info->next_chan) % + scpi_info->num_chans; scpi_chan = scpi_info->channels + chan; msg = get_scpi_xfer(scpi_chan); if (!msg) return -ENOMEM; - msg->slot = BIT(SCPI_SLOT); - msg->cmd = PACK_SCPI_CMD(cmd, tx_len); + if (scpi_info->is_legacy) { + mutex_lock(&scpi_chan->xfers_lock); Why does legacy need a different locking scheme ? [...] @@ -635,6 +804,24 @@ static int scpi_sensor_get_value(u16 sensor, u64 *val) return ret; } +static int legacy_scpi_sensor_get_value(u16 sensor, u64 *val) +{ + __le16 id = cpu_to_le16(sensor); + struct sensor_value buf; + int ret; + + ret = check_cmd(CMD_SENSOR_VALUE);
Re: [PATCH v3 4/8] scpi: Add support for Legacy match table for Amlogic GXBB SoC
On 07/09/16 16:34, Neil Armstrong wrote: Add new DT match table to setup the is_legacy boolean value across the scpi functions. Add the Amlogic GXBB SoC compatible for platform and as legacy match entry. Signed-off-by: Neil Armstrong --- drivers/firmware/arm_scpi.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c index 6a16100..60a76e63 100644 --- a/drivers/firmware/arm_scpi.c +++ b/drivers/firmware/arm_scpi.c @@ -979,6 +979,11 @@ static int scpi_alloc_xfer_list(struct device *dev, struct scpi_chan *ch) return 0; } +static const struct of_device_id legacy_scpi_of_match[] = { + {.compatible = "amlogic,meson-gxbb-scpi"}, + {}, +}; This needs to be documented. + static int scpi_probe(struct platform_device *pdev) { int count, idx, ret; @@ -991,6 +996,9 @@ static int scpi_probe(struct platform_device *pdev) if (!scpi_info) return -ENOMEM; + if (of_match_device(legacy_scpi_of_match, &pdev->dev)) + scpi_info->is_legacy = true; + count = of_count_phandle_with_args(np, "mboxes", "#mbox-cells"); if (count < 0) { dev_err(dev, "no mboxes property in '%s'\n", np->full_name); @@ -1097,6 +1105,7 @@ err: static const struct of_device_id scpi_of_match[] = { {.compatible = "arm,scpi"}, + {.compatible = "amlogic,meson-gxbb-scpi"}, I also prefer adding "arm,legacy-scpi". So far AMLogic has followed the legacy specification except the capabilities. You could use "amlogic,meson-gxbb-scpi" for that and "arm,legacy-scpi" for the probe part or we can add the use of "amlogic,meson-gxbb-scpi" later but just document it now. -- Regards, Sudeep
Re: [PATCH v3 4/8] scpi: Add support for Legacy match table for Amlogic GXBB SoC
On 19/09/16 16:50, Sudeep Holla wrote: On 07/09/16 16:34, Neil Armstrong wrote: Add new DT match table to setup the is_legacy boolean value across the scpi functions. Add the Amlogic GXBB SoC compatible for platform and as legacy match entry. Signed-off-by: Neil Armstrong --- drivers/firmware/arm_scpi.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c index 6a16100..60a76e63 100644 --- a/drivers/firmware/arm_scpi.c +++ b/drivers/firmware/arm_scpi.c @@ -979,6 +979,11 @@ static int scpi_alloc_xfer_list(struct device *dev, struct scpi_chan *ch) return 0; } +static const struct of_device_id legacy_scpi_of_match[] = { +{.compatible = "amlogic,meson-gxbb-scpi"}, +{}, +}; This needs to be documented. Ignore that I see you have done that in the later patch. -- Regards, Sudeep
[PATCH] coresight: always use stashed trace id value in etm4_trace_id
etm4_trace_id is not guaranteed to be executed on the CPU whose ETM is being accessed. This leads to exception similar to below one if the CPU whose ETM is being accessed is in deeper idle states. So it must be executed on the CPU whose ETM is being accessed. Unhandled fault: synchronous external abort (0x96000210) at 0x08db4040 Internal error: : 96000210 [#1] PREEMPT SMP Modules linked in: CPU: 5 PID: 5979 Comm: etm.sh Not tainted 4.7.0-rc3 #159 Hardware name: ARM Juno development board (r2) (DT) task: 80096dd34b00 ti: 80096dfe4000 task.ti: 80096dfe4000 PC is at etm4_trace_id+0x5c/0x90 LR is at etm4_trace_id+0x3c/0x90 Call trace: etm4_trace_id+0x5c/0x90 coresight_id_match+0x78/0xa8 bus_for_each_dev+0x60/0xa0 coresight_enable+0xc0/0x1b8 enable_source_store+0x3c/0x70 dev_attr_store+0x18/0x28 sysfs_kf_write+0x48/0x58 kernfs_fop_write+0x14c/0x1e0 __vfs_write+0x1c/0x100 vfs_write+0xa0/0x1b8 SyS_write+0x44/0xa0 el0_svc_naked+0x24/0x28 However, TRCTRACEIDR is not guaranteed to hold the previous programmed trace id if it enters deeper idle states. Further, the trace id that is computed in etm4_init_trace_id is programmed into TRCTRACEIDR only in etm4_enable_hw which happens much later in the sequence after coresight_id_match is executed from enable_source_store. This patch simplifies etm4_trace_id by returning the stashed trace id value similar to etm4_cpu_id. Cc: Mathieu Poirier Signed-off-by: Sudeep Holla --- drivers/hwtracing/coresight/coresight-etm4x.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) Hi Mathieu, While trying to support ETM with CPUIdle active, I faced this issue. Regards, Sudeep diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 43fa3beaa0df..d6f1d6d874eb 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -77,22 +77,8 @@ static int etm4_cpu_id(struct coresight_device *csdev) static int etm4_trace_id(struct coresight_device *csdev) { struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - unsigned long flags; - int trace_id = -1; - if (!local_read(&drvdata->mode)) - return drvdata->trcid; - - spin_lock_irqsave(&drvdata->spinlock, flags); - - CS_UNLOCK(drvdata->base); - trace_id = readl_relaxed(drvdata->base + TRCTRACEIDR); - trace_id &= ETM_TRACEID_MASK; - CS_LOCK(drvdata->base); - - spin_unlock_irqrestore(&drvdata->spinlock, flags); - - return trace_id; + return drvdata->trcid; } static void etm4_enable_hw(void *info) -- 2.7.4
Re: [RFC PATCH 0/2] scpi: Add SCPI framework to handle vendors variants
On 20/06/16 11:25, Neil Armstrong wrote: On 06/06/2016 07:10 PM, Sudeep Holla wrote: [...] Though this initial version of SCPI is not published, I am sure it is almost same as v1.0 except that the CMD is not part of payload like v1.0. In v1.0 it's part of payload and the mailbox is used just for doorbell mechanism. I hoped it would be this simple, but it touches core defines and structure that will over complicate the current driver. (i.e. the CMD indexes that differs, the CMD size shift, the high and low priority redirection or struct ordering) Ah ok, understood. IMO, we can add some compatible to indicate the pre v1.0 protocol and add the support to the existing driver itself. Let me know your thoughts. My proposal is : - add a registry layer but with only a single scpi instance (no mode OF involved, remove drivers changes) - add a scpi_legacy.c driver that only supports the old SCPI like Amlogic and Rockchip, and add a disclaimer for new developed SoCs - add your extension capability to handle Amlogic's and Rockchip's extended commands If you agree, I'll post a new patchset for review with these changes. Yes that sounds better. Also post along with the users to make it easy to review even if they are not completely ready for upstream. -- Regards, Sudeep
Re: [PATCH v2 3/3] firmware: scpi: add device power domain support using genpd
On 20/06/16 18:50, Kevin Hilman wrote: "Jon Medhurst (Tixy)" writes: On Thu, 2016-06-16 at 18:59 +0100, Sudeep Holla wrote: On 16/06/16 18:47, Jon Medhurst (Tixy) wrote: On Thu, 2016-06-16 at 11:38 +0100, Sudeep Holla wrote: [...] +enum scpi_power_domain_state { + SCPI_PD_STATE_ON = 0, + SCPI_PD_STATE_OFF = 3, +}; The SCPI doc defines the meaning of these numbers (0 and 3) in the 'Juno specifics' chapter. So does these values need to come from device-tree to allow for other hardware or SCP implementations? Ah unfortunately true :(. I had not noticed that. But I would like to check if this can be made as part of the standard protocol. Adding such details to DT seems overkill and defeat of the whole purpose of the standard protocol. Well. it seems to me the 'standard protocol' is whatever the current implementation of ARM's closed source SCP firmware is. It also seems to me that people are making things up as they go along, without a clue as to how to make things generic, robust and future proof. Basically, Status Normal ARM Fucked Up. Fully agree here. Just because ARM calls it a "standard" does not make it so. As we've already seen[1], vendors are using initial/older/whatever versions of SCPI, so pushing the Juno version as standard just becuase it's in ARM's closed firmware is not the right way forward either. I am not disagreeing on that. There's an on going effort to make it as standard as PSCI. But that's still work in progress. We need to deal with the existing variety of SCPI until then. No argument on that. The only argument I had on the other thread is not to make the changes too flexible that enabled the vendors to make up their own deviation of SCPI even for their future platforms. All I am asking is to keep is less flexible just to support existing platforms out there and not to help the vendors to deviate further. -- Regards, Sudeep
Re: [PATCH v2 0/3] firmware: scpi: add device power domain support
On 20/06/16 18:56, Kevin Hilman wrote: Sudeep Holla writes: This series add support for SCPI based device device power state management using genpd. Regards, Sudeep v1[1]->v2: - Fixed the endianness handling in scpi_device_get_power_state as spotted by Tixy - Renamed scpi_pd.c to scpi_pm_domain.c as suggested by Ulf - Added Mark's ack on DT binding [1] https://marc.info/?l=linux-kernel&m=146522850003483&w=2 Sudeep Holla (3): firmware: arm_scpi: add support for device power state management Documentation: add DT bindings for ARM SCPI power domains firmware: scpi: add device power domain support using genpd Other than the couple nits on specific patches, Reviewed-by: Kevin Hilman And just a[nother] reminder to be sure to keep things that are Juno specific well described since we know other vendors will be (and are being) "creative" in their implementations of SCPI. Agreed as mentioned in the other thread. I just sent a PR to arm-soc this afternoon. Do you think it's better to recall it and incorporate this ? Extremely sorry for rushing, I thought it would get too late. Ulf reviewed them so I rushed a bit. -- Regards, Sudeep
Re: [PATCH v2 3/3] firmware: scpi: add device power domain support using genpd
On 20/06/16 18:53, Kevin Hilman wrote: Sudeep Holla writes: This patch hooks up the support for device power domain provided by SCPI using the Linux generic power domain infrastructure. Cc: "Rafael J. Wysocki" Cc: Kevin Hilman Cc: Ulf Hansson Cc: linux...@vger.kernel.org Signed-off-by: Sudeep Holla [...] +static int scpi_pd_power(struct scpi_pm_domain *pd, bool power_on) +{ + int ret; + enum scpi_power_domain_state state; + + if (power_on) + state = SCPI_PD_STATE_ON; + else + state = SCPI_PD_STATE_OFF; + + ret = pd->ops->device_set_power_state(pd->domain, state); There should probably be some sanity checks here that these function pointers are non-NULL. Yes I agree. Since with the current scpi driver, if scpi_ops is populated, it's all non-NULL I skipped the sanity check as the probe will check for non-NULL scpi_ops. However, with extension work by Neil I agree, we may have to revisit all such callers. -- Regards, Sudeep
Re: [PATCH] coresight: always use stashed trace id value in etm4_trace_id
On 21/06/16 18:05, Mathieu Poirier wrote: On 20 June 2016 at 08:25, Sudeep Holla wrote: etm4_trace_id is not guaranteed to be executed on the CPU whose ETM is being accessed. This leads to exception similar to below one if the CPU whose ETM is being accessed is in deeper idle states. So it must be executed on the CPU whose ETM is being accessed. Unhandled fault: synchronous external abort (0x96000210) at 0x08db4040 Internal error: : 96000210 [#1] PREEMPT SMP Modules linked in: CPU: 5 PID: 5979 Comm: etm.sh Not tainted 4.7.0-rc3 #159 Hardware name: ARM Juno development board (r2) (DT) task: 80096dd34b00 ti: 80096dfe4000 task.ti: 80096dfe4000 PC is at etm4_trace_id+0x5c/0x90 LR is at etm4_trace_id+0x3c/0x90 Call trace: etm4_trace_id+0x5c/0x90 coresight_id_match+0x78/0xa8 bus_for_each_dev+0x60/0xa0 coresight_enable+0xc0/0x1b8 enable_source_store+0x3c/0x70 dev_attr_store+0x18/0x28 sysfs_kf_write+0x48/0x58 kernfs_fop_write+0x14c/0x1e0 __vfs_write+0x1c/0x100 vfs_write+0xa0/0x1b8 SyS_write+0x44/0xa0 el0_svc_naked+0x24/0x28 However, TRCTRACEIDR is not guaranteed to hold the previous programmed trace id if it enters deeper idle states. Further, the trace id that is computed in etm4_init_trace_id is programmed into TRCTRACEIDR only in etm4_enable_hw which happens much later in the sequence after coresight_id_match is executed from enable_source_store. This patch simplifies etm4_trace_id by returning the stashed trace id value similar to etm4_cpu_id. Cc: Mathieu Poirier Signed-off-by: Sudeep Holla --- drivers/hwtracing/coresight/coresight-etm4x.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) Hi Mathieu, While trying to support ETM with CPUIdle active, I faced this issue. Regards, Sudeep diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c index 43fa3beaa0df..d6f1d6d874eb 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.c +++ b/drivers/hwtracing/coresight/coresight-etm4x.c @@ -77,22 +77,8 @@ static int etm4_cpu_id(struct coresight_device *csdev) static int etm4_trace_id(struct coresight_device *csdev) { struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent); - unsigned long flags; - int trace_id = -1; - if (!local_read(&drvdata->mode)) - return drvdata->trcid; - - spin_lock_irqsave(&drvdata->spinlock, flags); - - CS_UNLOCK(drvdata->base); - trace_id = readl_relaxed(drvdata->base + TRCTRACEIDR); - trace_id &= ETM_TRACEID_MASK; - CS_LOCK(drvdata->base); - - spin_unlock_irqrestore(&drvdata->spinlock, flags); - - return trace_id; + return drvdata->trcid; } This code was written prior to the integration with the Perf core. To make sure the correct active value was return to users it goes to the HW if the IP is in use. With the integration with Perf the code was moved around and the traceID is no longer called on the CPU (as you noticed) and has lost the required PM runtime operation. Is the perf integration already queued ? If not, it good to have this until then. I see crashes if I disable idle on one core and enable ETM source on that. With this I don't see any crashes even if we have CPUIdle enabled. However the trace collection halts once the core suspends and resumes back. I have a simple solution for handling CPUIdle too, will post it soon. -- Regards, Sudeep
Re: [PATCH 1/2] arm64: defconfig: enable SENSORS_ARM_SCPI
On 13/06/16 16:15, Javi Merino wrote: ARM SCPI Sensors were merged for v4.4 and they are defined in the Juno dts. Enable it in the defconfig to get them registered automatically in Juno by default. Applied, thanks. -- Regards, Sudeep
Re: [PATCH 1/2] armv8: aarch32: Execute 32-bit Linux for LayerScape platforms
On 23/09/16 14:18, Robin Murphy wrote: On 23/09/16 14:13, Stuart Yoder wrote: [...] Which arch/arm/mach-* platform are you using for Juno? I don't even know! :) I just start with a multi_v7_defconfig plus a few extra bits (LPAE, KVM, sil24, sky2, etc.) and it works. I guess it's the combination of mach-vexpress and mach-virt. It should be matching "arm,vexpress" and just using mach-vexpress/v2m.c as it's present as compatible in Juno DT but nothing else there. mach-virt was deleted and we just have "Generic DT based system" in arch/arm/kernel/devtree.c which IIUC doesn't require any compatible in the DT. So dropping "arm,vexpress" will make it use "Generic DT based system" I believe. -- Regards, Sudeep
Re: [PATCH] i2c: qup: skip qup_i2c_suspend if the device is already runtime suspended
On Thu, Aug 25, 2016 at 12:23 PM, Sudeep Holla wrote: > If the i2c device is already runtime suspended, if qup_i2c_suspend is > executed during suspend-to-idle or suspend-to-ram it will result in the > following splat: > > WARNING: CPU: 3 PID: 1593 at drivers/clk/clk.c:476 > clk_core_unprepare+0x80/0x90 > Modules linked in: > > CPU: 3 PID: 1593 Comm: bash Tainted: GW 4.8.0-rc3 #14 > Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) > PC is at clk_core_unprepare+0x80/0x90 > LR is at clk_unprepare+0x28/0x40 > pc : [] lr : [] pstate: 6145 > Call trace: > clk_core_unprepare+0x80/0x90 > qup_i2c_disable_clocks+0x2c/0x68 > qup_i2c_suspend+0x10/0x20 > platform_pm_suspend+0x24/0x68 > dpm_run_callback.isra.7+0x1c/0x70 > __device_suspend+0xf4/0x298 > dpm_suspend+0x10c/0x228 > dpm_suspend_start+0x68/0x78 > suspend_devices_and_enter+0xb8/0x460 > pm_suspend+0x1ec/0x240 > state_store+0x84/0xf8 > kobj_attr_store+0x14/0x28 > sysfs_kf_write+0x48/0x58 > kernfs_fop_write+0x15c/0x1f8 > __vfs_write+0x1c/0x100 > vfs_write+0x9c/0x1b8 > SyS_write+0x44/0xa0 > el0_svc_naked+0x24/0x28 > > This patch fixes the issue by executing qup_i2c_pm_suspend_runtime > conditionally in qup_i2c_suspend. > Gentle ping ! Regards, Sudeep
Re: [PATCH] spi: qup: skip clk_disable_unprepare if the device is already runtime suspended
Hi, On 01/09/16 21:29, Mark Brown wrote: On Thu, Aug 25, 2016 at 01:33:28PM +0100, Sudeep Holla wrote: If the spi device is already runtime suspended, if spi_qup_suspend is executed during suspend-to-idle or suspend-to-ram it will result in the following splat: WARNING: CPU: 3 PID: 1593 at drivers/clk/clk.c:476 clk_core_unprepare+0x80/0x90 Modules linked in: CPU: 3 PID: 1593 Comm: bash Tainted: GW 4.8.0-rc3 #14 Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) PC is at clk_core_unprepare+0x80/0x90 LR is at clk_unprepare+0x28/0x40 pc : [] lr : [] pstate: 6145 Please think hard before including complete backtraces in upstream reports, they are very large and contain almost no useful information relative to their size so often obscure the relevant content in your message. If part of the backtrace is usefully illustrative then it's usually better to pull out the relevant sections. I removed most of the addresses and just retained the symbols(somehow the last line with pc and lr was left unintentionally). While you may have the above opinion, other maintainers may differ. In future, I will try to add it as a note just to describe the issue. -- Regards, Sudeep
Re: [PATCH] spi: qup: skip clk_disable_unprepare if the device is already runtime suspended
On 02/09/16 10:38, Mark Brown wrote: On Fri, Sep 02, 2016 at 09:42:04AM +0100, Sudeep Holla wrote: On 01/09/16 21:29, Mark Brown wrote: On Thu, Aug 25, 2016 at 01:33:28PM +0100, Sudeep Holla wrote: CPU: 3 PID: 1593 Comm: bash Tainted: GW 4.8.0-rc3 #14 Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) PC is at clk_core_unprepare+0x80/0x90 LR is at clk_unprepare+0x28/0x40 pc : [] lr : [] pstate: 6145 Please think hard before including complete backtraces in upstream reports, they are very large and contain almost no useful information relative to their size so often obscure the relevant content in your message. If part of the backtrace is usefully illustrative then it's usually better to pull out the relevant sections. I removed most of the addresses and just retained the symbols(somehow the last line with pc and lr was left unintentionally). While you may have the above opinion, other maintainers may differ. In future, I will try to add it as a note just to describe the issue. Oh, *that's* why it looked so weird. Removing the addresses doesn't help here, the issue isn't that the addresses are confusing it's that you had a tiny commit message dwarfed by the backtrace preamble then a screenful of call stack which conveyed no meaningful information, including not just the entire callback path for a suspend (which doesn't tell us anything really, especially beyond the first frame) and going on to show the entire call stack from the sysfs write you used to trigger suspend which is even less relevant. This gives us 30 lines or so of splat (more than a screenful) for five lines of actual content with the important bit which describes what the change is supposed to be doing buried at the bottom. That's a really bad signal to noise ratio. What would've been better would be explaining why the change you are making fixes the problem. Agreed. -- Regards, Sudeep
Re: [PATCH ] drivers/base: cacheinfo: remove warning in resume
On 02/09/16 13:58, Sumit Gupta wrote: Hi Sudeep, Thank you for your comments. I understand the warning we get but the patch is completely wrong. One it removes the feature of adding/removing the cache devices on cpu hotplug events. Have you tested your patch with simple cpu hotplug and seen no change before and after this change ? I referred other node "cpufreq" under "cpu" node and that was also not getting deleted when core goes down. So, thought the behavior should be similar as entries are only used to read data and it won't change after boot. I agree, but for caches it has been like this and in a way make sense. On 29/08/16 08:20, Sumit Gupta wrote: CPU notifier is present for creating device entries for child node "cache" under parent node "cpu" as per DT. Again DT is only on few architectures not on all(e.g. x86) During resume from suspend, while booting all non-boot CPU's, this notifier for adding cache device gets called before cpu device is added by device_resume. Because of this warning message of "parent should not be sleeping" comes during resume. Yes that's correct and needs to be fixed. I have seen this but haven't spent much time to check in detail. It's harmless warning IMO. See the comment in the code too:"This is a fib. But we'll allow new children to be added below a resumed device, even if the device hasn't been completed yet" CPU devices are special and they have separate hotplug paths. So we need to consider that for cpu devices and set is_prepared quite early. Removing the notifier to explicitly add/remove cache device as CPU and cache device get added/removed anyway as part of normal suspend resume sequence. dpm_resume_end - > dpm_resume -> device_resume Yes, but: 1. the caches objects are visible even when the cpu is offline 2. how is this handled for normal cpu-hotplug events ? This patch breaks the existing feature. Compared with x86 now and there even cpufreq is getting deleted. As per the comments, right behavior seems to be where cache entries should be created during core on and removed during core off in hotplug. I will try to find other way of doing it. Also will see why cpufreq is not getting deleted as in x86. IIUC you are saying cpufreq sysfs entries are deleted on x86 while not on ARM{32,64} ? If so are you running same kernel version on both x86 and ARM ? I remember some recent change around cpufreq sysfs entries, it could be result of that. -- Regards, Sudeep -- Regards, Sudeep
Re: [PATCH 2/2] coresight: fix handling of ETM trace register access via sysfs
On 04/08/16 16:46, Mathieu Poirier wrote: On 3 August 2016 at 10:12, Sudeep Holla wrote: The ETM registers are classified into 2 categories: trace and management. The core power domain contains most of the trace unit logic including all(except TRCOSLAR and TRCOSLSR) the trace registers. The debug power domain contains the external debugger interface including all management registers. This patch adds coresight unit specific function coresight_simple_func which can be used for ETM trace registers by providing a ETM specific read function which does smp cross call to ensure the trace core is powered up before the register is accessed. Cc: Mathieu Poirier Signed-off-by: Sudeep Holla Hey Sudeep, I'm good with this patch - just a few things to amend below. Many thanks, Mathieu --- drivers/hwtracing/coresight/coresight-etb10.c | 2 +- .../hwtracing/coresight/coresight-etm3x-sysfs.c| 2 +- .../hwtracing/coresight/coresight-etm4x-sysfs.c| 58 -- drivers/hwtracing/coresight/coresight-etm4x.h | 1 + drivers/hwtracing/coresight/coresight-priv.h | 9 +++- drivers/hwtracing/coresight/coresight-stm.c| 2 +- drivers/hwtracing/coresight/coresight-tmc.c| 2 +- 7 files changed, 54 insertions(+), 22 deletions(-) Hi Mathieu, I think the latest release of the firmware(inparticular SCP v1.16.0) for Is this public? If so please give me the link so that we test with the same environment. Yes I believe so. You should be able to grab latest @[1] I agree with all the other comments, will repost v2 soon. -- Regards, Sudeep [1] https://snapshots.linaro.org/member-builds/armlt-platforms-release/28/juno-uefi.zip
Re: [PATCH 2/2] coresight: fix handling of ETM trace register access via sysfs
On 04/08/16 17:02, Mathieu Poirier wrote: On 3 August 2016 at 10:12, Sudeep Holla wrote: The ETM registers are classified into 2 categories: trace and management. The core power domain contains most of the trace unit logic including all(except TRCOSLAR and TRCOSLSR) the trace registers. The debug power domain contains the external debugger interface including all management registers. This patch adds coresight unit specific function coresight_simple_func which can be used for ETM trace registers by providing a ETM specific read function which does smp cross call to ensure the trace core is powered up before the register is accessed. Cc: Mathieu Poirier Signed-off-by: Sudeep Holla --- drivers/hwtracing/coresight/coresight-etb10.c | 2 +- .../hwtracing/coresight/coresight-etm3x-sysfs.c| 2 +- .../hwtracing/coresight/coresight-etm4x-sysfs.c| 58 -- drivers/hwtracing/coresight/coresight-etm4x.h | 1 + drivers/hwtracing/coresight/coresight-priv.h | 9 +++- drivers/hwtracing/coresight/coresight-stm.c| 2 +- drivers/hwtracing/coresight/coresight-tmc.c| 2 +- 7 files changed, 54 insertions(+), 22 deletions(-) Hi Mathieu, I think the latest release of the firmware(inparticular SCP v1.16.0) for Juno fixes the issue you had previously encountered. However there's a pending issue with A53 ETM management register access when it's powered down. I don't think Juno platform should block these changes as ETMv4 specification is clear on the power management and register access perspective. Regards, Sudeep diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 8a4927ca9181..d7325c6534ad 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -559,7 +559,7 @@ static const struct file_operations etb_fops = { }; #define coresight_etb10_simple_func(name, offset) \ - coresight_simple_func(struct etb_drvdata, name, offset) + coresight_simple_func(struct etb_drvdata, NULL, name, offset) coresight_etb10_simple_func(rdp, ETB_RAM_DEPTH_REG); coresight_etb10_simple_func(sts, ETB_STATUS_REG); diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c index 02d4b629891f..4856c8098416 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c @@ -1222,7 +1222,7 @@ static struct attribute *coresight_etm_attrs[] = { }; #define coresight_etm3x_simple_func(name, offset) \ - coresight_simple_func(struct etm_drvdata, name, offset) + coresight_simple_func(struct etm_drvdata, NULL, name, offset) coresight_etm3x_simple_func(etmccr, ETMCCR); coresight_etm3x_simple_func(etmccer, ETMCCER); diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 7c84308c5564..2390ee43e3d9 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2039,15 +2039,38 @@ static struct attribute *coresight_etmv4_attrs[] = { NULL, }; +struct etm_reg { + void __iomem *addr; + u32 data; +}; + And this is just declaration, so it's static implicitly unless someone includes this .c file :) [...] static struct attribute *coresight_etmv4_trcidr_attrs[] = { &dev_attr_trcidr0.attr, diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h b/drivers/hwtracing/coresight/coresight-etm4x.h index 2629954429a1..ba4dd2e820ea 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x.h +++ b/drivers/hwtracing/coresight/coresight-etm4x.h @@ -185,6 +185,7 @@ /* PowerDown Control Register bits */ #define TRCPDCR_PU BIT(3) +#define TRCPDSR_POWER BIT(0) I forgot - this isn't referenced anywhere. Right will remove it, added it while I was experimenting things out. -- Regards, Sudeep
[PATCH v2 2/2] coresight: fix handling of ETM trace register access via sysfs
The ETM registers are classified into 2 categories: trace and management. The core power domain contains most of the trace unit logic including all(except TRCOSLAR and TRCOSLSR) the trace registers. The debug power domain contains the external debugger interface including all management registers. This patch adds coresight unit specific function coresight_simple_func which can be used for ETM trace registers by providing a ETM specific read function which does smp cross call to ensure the trace core is powered up before the register is accessed. Cc: Mathieu Poirier Signed-off-by: Sudeep Holla --- drivers/hwtracing/coresight/coresight-etb10.c | 2 +- .../hwtracing/coresight/coresight-etm3x-sysfs.c| 2 +- .../hwtracing/coresight/coresight-etm4x-sysfs.c| 62 -- drivers/hwtracing/coresight/coresight-priv.h | 9 +++- drivers/hwtracing/coresight/coresight-stm.c| 2 +- drivers/hwtracing/coresight/coresight-tmc.c| 2 +- 6 files changed, 57 insertions(+), 22 deletions(-) Changes v1->v2: - s/etmv4_reg/etmv4_reg/ - s/etmv4_chk_trace_reg_access/etmv4_cross_read/ - s/coresight_etm4x_trace_reg_func/coresight_etm4x_cross_read/ - Removed stale macro defination - Added a comment mentioning that the smp cross reading of the trace registers guarantees the core power domain is enabled. diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c index 8a4927ca9181..d7325c6534ad 100644 --- a/drivers/hwtracing/coresight/coresight-etb10.c +++ b/drivers/hwtracing/coresight/coresight-etb10.c @@ -559,7 +559,7 @@ static const struct file_operations etb_fops = { }; #define coresight_etb10_simple_func(name, offset) \ - coresight_simple_func(struct etb_drvdata, name, offset) + coresight_simple_func(struct etb_drvdata, NULL, name, offset) coresight_etb10_simple_func(rdp, ETB_RAM_DEPTH_REG); coresight_etb10_simple_func(sts, ETB_STATUS_REG); diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c index 02d4b629891f..4856c8098416 100644 --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c @@ -1222,7 +1222,7 @@ static struct attribute *coresight_etm_attrs[] = { }; #define coresight_etm3x_simple_func(name, offset) \ - coresight_simple_func(struct etm_drvdata, name, offset) + coresight_simple_func(struct etm_drvdata, NULL, name, offset) coresight_etm3x_simple_func(etmccr, ETMCCR); coresight_etm3x_simple_func(etmccer, ETMCCER); diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c index 7c84308c5564..fd7ff613db17 100644 --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c @@ -2039,15 +2039,42 @@ static struct attribute *coresight_etmv4_attrs[] = { NULL, }; +struct etmv4_reg { + void __iomem *addr; + u32 data; +}; + +static void do_smp_cross_read(void *data) +{ + struct etmv4_reg *reg = data; + + reg->data = readl_relaxed(reg->addr); +} + +static u32 etmv4_cross_read(const struct device *dev, u32 offset) +{ + struct etmv4_drvdata *drvdata = dev_get_drvdata(dev); + struct etmv4_reg reg; + + reg.addr = drvdata->base + offset; + /* +* smp cross call ensures the CPU will be powered up before +* accessing the ETMv4 trace core registers +*/ + smp_call_function_single(drvdata->cpu, do_smp_cross_read, ®, 1); + return reg.data; +} + #define coresight_etm4x_simple_func(name, offset) \ - coresight_simple_func(struct etmv4_drvdata, name, offset) + coresight_simple_func(struct etmv4_drvdata, NULL, name, offset) + +#define coresight_etm4x_cross_read(name, offset) \ + coresight_simple_func(struct etmv4_drvdata, etmv4_cross_read, \ + name, offset) -coresight_etm4x_simple_func(trcoslsr, TRCOSLSR); coresight_etm4x_simple_func(trcpdcr, TRCPDCR); coresight_etm4x_simple_func(trcpdsr, TRCPDSR); coresight_etm4x_simple_func(trclsr, TRCLSR); -coresight_etm4x_simple_func(trcconfig, TRCCONFIGR); -coresight_etm4x_simple_func(trctraceid, TRCTRACEIDR); coresight_etm4x_simple_func(trcauthstatus, TRCAUTHSTATUS); coresight_etm4x_simple_func(trcdevid, TRCDEVID); coresight_etm4x_simple_func(trcdevtype, TRCDEVTYPE); @@ -2055,6 +2082,9 @@ coresight_etm4x_simple_func(trcpidr0, TRCPIDR0); coresight_etm4x_simple_func(trcpidr1, TRCPIDR1); coresight_etm4x_simple_func(trcpidr2, TRCPIDR2); coresight_etm4x_simple_func(trcpidr3, TRCPIDR3); +coresight_etm4x_cross_read(trcoslsr, TRCOSLSR); +coresight_etm4x_cross_read(trcconfig, TRCCONFIGR); +coresight_etm4x_cross_read(trctraceid, TRCTRACEIDR);
[PATCH] i2c: qup: skip qup_i2c_suspend if the device is already runtime suspended
If the i2c device is already runtime suspended, if qup_i2c_suspend is executed during suspend-to-idle or suspend-to-ram it will result in the following splat: WARNING: CPU: 3 PID: 1593 at drivers/clk/clk.c:476 clk_core_unprepare+0x80/0x90 Modules linked in: CPU: 3 PID: 1593 Comm: bash Tainted: GW 4.8.0-rc3 #14 Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) PC is at clk_core_unprepare+0x80/0x90 LR is at clk_unprepare+0x28/0x40 pc : [] lr : [] pstate: 6145 Call trace: clk_core_unprepare+0x80/0x90 qup_i2c_disable_clocks+0x2c/0x68 qup_i2c_suspend+0x10/0x20 platform_pm_suspend+0x24/0x68 dpm_run_callback.isra.7+0x1c/0x70 __device_suspend+0xf4/0x298 dpm_suspend+0x10c/0x228 dpm_suspend_start+0x68/0x78 suspend_devices_and_enter+0xb8/0x460 pm_suspend+0x1ec/0x240 state_store+0x84/0xf8 kobj_attr_store+0x14/0x28 sysfs_kf_write+0x48/0x58 kernfs_fop_write+0x15c/0x1f8 __vfs_write+0x1c/0x100 vfs_write+0x9c/0x1b8 SyS_write+0x44/0xa0 el0_svc_naked+0x24/0x28 This patch fixes the issue by executing qup_i2c_pm_suspend_runtime conditionally in qup_i2c_suspend. Cc: Andy Gross Cc: David Brown Cc: Wolfram Sang Signed-off-by: Sudeep Holla --- drivers/i2c/busses/i2c-qup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c index 501bd15cb78e..a8497cfdae6f 100644 --- a/drivers/i2c/busses/i2c-qup.c +++ b/drivers/i2c/busses/i2c-qup.c @@ -1599,7 +1599,8 @@ static int qup_i2c_pm_resume_runtime(struct device *device) #ifdef CONFIG_PM_SLEEP static int qup_i2c_suspend(struct device *device) { - qup_i2c_pm_suspend_runtime(device); + if (!pm_runtime_suspended(device)) + return qup_i2c_pm_suspend_runtime(device); return 0; } -- 2.7.4
[PATCH] spi: qup: skip clk_disable_unprepare if the device is already runtime suspended
If the spi device is already runtime suspended, if spi_qup_suspend is executed during suspend-to-idle or suspend-to-ram it will result in the following splat: WARNING: CPU: 3 PID: 1593 at drivers/clk/clk.c:476 clk_core_unprepare+0x80/0x90 Modules linked in: CPU: 3 PID: 1593 Comm: bash Tainted: GW 4.8.0-rc3 #14 Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) PC is at clk_core_unprepare+0x80/0x90 LR is at clk_unprepare+0x28/0x40 pc : [] lr : [] pstate: 6145 Call trace: clk_core_unprepare+0x80/0x90 spi_qup_suspend+0x68/0x90 platform_pm_suspend+0x24/0x68 dpm_run_callback.isra.7+0x1c/0x70 __device_suspend+0xf4/0x298 dpm_suspend+0x10c/0x228 dpm_suspend_start+0x68/0x78 suspend_devices_and_enter+0xb8/0x460 pm_suspend+0x1ec/0x240 state_store+0x84/0xf8 kobj_attr_store+0x14/0x28 sysfs_kf_write+0x48/0x58 kernfs_fop_write+0x15c/0x1f8 __vfs_write+0x1c/0x100 vfs_write+0x9c/0x1b8 SyS_write+0x44/0xa0 el0_svc_naked+0x24/0x28 This patch fixes the issue by executing clk_disable_unprepare conditionally in spi_qup_suspend. Cc: Andy Gross Cc: David Brown Cc: Mark Brown Signed-off-by: Sudeep Holla --- drivers/spi/spi-qup.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c index c338ef1136f6..a047e9882da8 100644 --- a/drivers/spi/spi-qup.c +++ b/drivers/spi/spi-qup.c @@ -982,8 +982,10 @@ static int spi_qup_suspend(struct device *device) if (ret) return ret; - clk_disable_unprepare(controller->cclk); - clk_disable_unprepare(controller->iclk); + if (!pm_runtime_suspended(device)) { + clk_disable_unprepare(controller->cclk); + clk_disable_unprepare(controller->iclk); + } return 0; } -- 2.7.4
Re: [PATCH v2 0/7] scpi: Add support for legacy SCPI protocol
On 25/08/16 14:18, Neil Armstrong wrote: On 08/23/2016 01:46 PM, Neil Armstrong wrote: This patchset aims to support the legacy SCPI firmware implementation that was delivered as early technology preview for the JUNO platform. Finally a stable, maintained and public implementation for the SCPI protocol has been upstreamed part of the JUNO support and it is the recommended way of implementing SCP communication on ARMv8 platforms. The Amlogic GXBB platform is using this legacy protocol, as the RK3368 & RK3399 platforms. This patchset will only add support for Amlogic GXBB SoC. This patchset add support for the legacy protocol in the arm_scpi.c file, avoiding code duplication. Last RFC discution tread can be found at : https://lkml.org/lkml/2016/8/9/210 The last patch depends on the "Platform MHU" dtsi patch. Changes since v1 at : http://lkml.kernel.org/r/1471515066-3626-1-git-send-email-narmstr...@baylibre.com - Dropped vendor_send_message and rockchip vendor mechanism patches - Merged alternate functions into main functions using is_legacy boolean - Added DT match table to set is_legacy to true - Kept alternate scpi_ops structure for legacy Neil Armstrong (7): scpi: Add alternative legacy structures, functions and macros scpi: Use legacy variants command index calling scpi_send_message scpi: Add support for Legacy match table for Amlogic GXBB SoC scpi: grow MAX_DVFS_OPPS to 16 entries dt-bindings: Add support for Amlogic GXBB SCPI Interface ARM64: dts: meson-gxbb: Add SRAM node ARM64: dts: meson-gxbb: Add SCPI with cpufreq & sensors Nodes Documentation/devicetree/bindings/arm/arm,scpi.txt | 8 +- arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi| 45 drivers/firmware/arm_scpi.c| 279 +++-- 3 files changed, 302 insertions(+), 30 deletions(-) Hi Sudeep, Sorry but I posted this V2 before you had time to look at my previous v1 replies... That's fine. In this series, I merged the scpi_send_message, but I must still evaluate how it's possible to use the list to queue commands. Ah OK. Here I used if(is_legacy) to stop duplicating functions, is this ok for you ? I am still thinking if it can be abstracted well, some kind of mapping but haven't thought too much about that yet. Also I was thinking about bitmap for high priority commands. I remember doing something before but seem to have lost that copy. I will try to dig it out.. -- Regards, Sudeep
Re: [PATCH] spi: qup: skip clk_disable_unprepare if the device is already runtime suspended
Hi Andy, On 25/08/16 14:26, Andy Gross wrote: On Thu, Aug 25, 2016 at 01:33:28PM +0100, Sudeep Holla wrote: If the spi device is already runtime suspended, if spi_qup_suspend is executed during suspend-to-idle or suspend-to-ram it will result in the following splat: WARNING: CPU: 3 PID: 1593 at drivers/clk/clk.c:476 clk_core_unprepare+0x80/0x90 Modules linked in: Thanks for fixing this. I had noticed this yesterday when I was testing your freeze patch but hadn't had time to dig in. Yes, even I tested the same once I got PSCI firmware :) Tested-by: Andy Gross Thanks. -- Regards, Sudeep
Re: [PATCH v2 0/7] scpi: Add support for legacy SCPI protocol
On 25/08/16 14:45, Sudeep Holla wrote: On 25/08/16 14:18, Neil Armstrong wrote: [...] Here I used if(is_legacy) to stop duplicating functions, is this ok for you ? I am still thinking if it can be abstracted well, some kind of mapping but haven't thought too much about that yet. Also I was thinking about bitmap for high priority commands. I remember doing something before but seem to have lost that copy. I will try to dig it out.. OK how about something like: 1. in struct scpi_drvinfo DECLARE_BITMAP(cmd_priority, SCPI_CMD_COUNT); 2. scpi_send_message scpi_chan = test_bit(cmd, scpi_info->cmd_priority) ? scpi_info->channels + 1 : scpi_info->channels; 3. probe for (idx = 0; idx < ARRAY_SIZE(hpriority_cmds); idx++) set_bit(hpriority_cmds[idx], scpi_info->cmd_priority); For commands, I am thinking some kind of indirection like the below patch. See if that helps, I will add the description later, but you can build your patches on top of it if you think that works and keeps code simple. Regards, Sudeep -->8 From afde0d2f1d3381d443445301ab5ec111276934e5 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Thu, 25 Aug 2016 17:21:49 +0100 Subject: [PATCH] firmware: arm_scpi: create command indirection to support legacy commands Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scpi.c | 68 + 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c index 438893762076..c2063cb76b08 100644 --- a/drivers/firmware/arm_scpi.c +++ b/drivers/firmware/arm_scpi.c @@ -129,7 +129,39 @@ enum scpi_std_cmd { SCPI_CMD_SENSOR_ASYNC_VALUE = 0x1a, SCPI_CMD_SET_DEVICE_PWR_STATE = 0x1b, SCPI_CMD_GET_DEVICE_PWR_STATE = 0x1c, - SCPI_CMD_COUNT +}; + +enum scpi_drv_cmd { + SCPI_CAPABILITIES = 0, + GET_DVFS_INFO = 1, + SET_DVFS= 2, + GET_DVFS= 3, + GET_CLOCK_INFO = 4, + SET_CLOCK_VALUE = 5, + GET_CLOCK_VALUE = 6, + PSU_CAPABILITIES= 7, + SENSOR_CAPABILITIES = 8, + SENSOR_INFO = 9, + SENSOR_VALUE= 10, + SET_DEV_PWR_STATE = 11, + GET_DEV_PWR_STATE = 12, + CMD_MAX_COUNT +}; + +static int scpi_std_commands[CMD_MAX_COUNT] = { + SCPI_CMD_SCPI_CAPABILITIES, + SCPI_CMD_GET_DVFS_INFO, + SCPI_CMD_SET_DVFS, + SCPI_CMD_GET_DVFS, + SCPI_CMD_GET_CLOCK_INFO, + SCPI_CMD_SET_CLOCK_VALUE, + SCPI_CMD_GET_CLOCK_VALUE, + SCPI_CMD_PSU_CAPABILITIES, + SCPI_CMD_SENSOR_CAPABILITIES, + SCPI_CMD_SENSOR_INFO, + SCPI_CMD_SENSOR_VALUE, + SCPI_CMD_SET_DEVICE_PWR_STATE, + SCPI_CMD_GET_DEVICE_PWR_STATE, }; struct scpi_xfer { @@ -161,6 +193,7 @@ struct scpi_drvinfo { u32 protocol_version; u32 firmware_version; int num_chans; + int *cmds; atomic_t next_chan; struct scpi_ops *scpi_ops; struct scpi_chan *channels; @@ -397,7 +430,7 @@ scpi_clk_get_range(u16 clk_id, unsigned long *min, unsigned long *max) struct clk_get_info clk; __le16 le_clk_id = cpu_to_le16(clk_id); - ret = scpi_send_message(SCPI_CMD_GET_CLOCK_INFO, &le_clk_id, + ret = scpi_send_message(scpi_info->cmds[GET_CLOCK_INFO], &le_clk_id, sizeof(le_clk_id), &clk, sizeof(clk)); if (!ret) { *min = le32_to_cpu(clk.min_rate); @@ -412,7 +445,7 @@ static unsigned long scpi_clk_get_val(u16 clk_id) struct clk_get_value clk; __le16 le_clk_id = cpu_to_le16(clk_id); - ret = scpi_send_message(SCPI_CMD_GET_CLOCK_VALUE, &le_clk_id, + ret = scpi_send_message(scpi_info->cmds[GET_CLOCK_VALUE], &le_clk_id, sizeof(le_clk_id), &clk, sizeof(clk)); return ret ? ret : le32_to_cpu(clk.rate); } @@ -425,8 +458,8 @@ static int scpi_clk_set_val(u16 clk_id, unsigned long rate) .rate = cpu_to_le32(rate) }; - return scpi_send_message(SCPI_CMD_SET_CLOCK_VALUE, &clk, sizeof(clk), -&stat, sizeof(stat)); + return scpi_send_message(scpi_info->cmds[SET_CLOCK_VALUE], &clk, +sizeof(clk), &stat, sizeof(stat)); } static int scpi_dvfs_get_idx(u8 domain) @@ -434,8 +467,8 @@ static int scpi_dvfs_get_idx(u8 domain) int ret; u8 dvfs_idx; - ret = scpi_send_message(SCPI_CMD_GET_DVFS, &domain, sizeof(domain), - &dvfs_idx, sizeof(dvfs_idx)); + ret = scpi_send_message(scpi_info->cmds[GET_DVFS], &domain, + sizeof(domain), &dvfs_idx, sizeof(dvfs_idx)); return ret
Re: [PATCHv2 1/3] dt: xilinx: zynqmp: Add bindings for PM firmware
On 16/08/17 13:24, Michal Simek wrote: > From: Soren Brinkmann > > Document the DT bindings for the Zynq UltraScale+ PM Firmware. > > Signed-off-by: Soren Brinkmann > Signed-off-by: Michal Simek > --- > > Changes in v2: > - Move to bindings/firmware and also add it to firmware node > Reported-by Rob > > .../bindings/firmware/xlnx,zynqmp-pm.txt | 22 > ++ > 1 file changed, 22 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt > > diff --git a/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt > b/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt > new file mode 100644 > index ..7de0c82758b3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/firmware/xlnx,zynqmp-pm.txt > @@ -0,0 +1,22 @@ > +Xilinx Zynq MPSoC Firmware Device Tree Bindings > + > +The zynqmp-pm node describes the interface to platform firmware. > + > +Required properties: > + - compatible: Must contain: "xlnx,zynqmp-pm" > + - method: The method of calling the PM-API firmware layer. > + Permitted values are: > + - "smc" : To be used in configurations without a hypervisor > + - "hvc" : To be used when hypervisor is present > + - interrupts: Interrupt specifier What exactly is this interrupt ? I see it's not SGI, so if it's SPI, where does this originate from ? How is that dealt ? -- Regards, Sudeep
Re: [PATCHv2 0/3] arm64 xilinx zynqmp firmware interface
On 17/08/17 09:42, Michal Simek wrote: > On 17.8.2017 09:52, Marc Zyngier wrote: >> On 17/08/17 07:10, Michal Simek wrote: >>> On 16.8.2017 17:39, Marc Zyngier wrote: On 16/08/17 13:24, Michal Simek wrote: > Hi, > > xilinx is using this interface for very long time and we can't merge our > driver changes to Linux because of missing communication layer with > firmware. This interface was developed before scpi and scmi was > available. In connection to power management scpi and scmi are missing > pieces which we already use. There is a separate discussion how to > extend scmi to support all our use cases. So maybe we should wait and see where this discussion is going before we merge yet another firmware interface? >>> >>> It will take a lot of time when this discussion ends and I can't see any >>> benefit to hold all >> >> Well, so far, the benefit of this series is exactly nil, as the code it >> brings is *unused*. It is impossible to review in isolation. >> > > Ok. I will add others drivers to this series that's not a problem. > >> In the meantime, you can continue finding out how *not* to have to merge >> this code, and instead focus on using the infrastructure we already >> have, or at least influence the infrastructure that is being designed. >> It will be much better than dumping yet another slab of "I'm so >> different" code that is going to ultimately bitrot. > > I am happy to look the better proposed way. SCPI is ancient and SCMI is > replacement and not merged yet. We already had a call with arm and > Sudeep was on it too where outcome from that was that we can't use that > because it doesn't support what we need to support now. > OK, none of the specifics were discussed in the meeting to conclude that SCMI can't be used. My takeaway from the meeting was Xilinx has this interface for long and being deployed in various systems. I would like to get into specifics before discarding SCMI as unusable. What bothers me more is that why was that not raised during the specification review which was quite a long period IMO ? I tend to think Xilinx never bothered to look/review the specification as this f/w interface was already there. However I still can't see why this was posted once we started pushing out SCMI patches especially given that this f/w interface was there for long and no attempts were made in past to upstream this. Also I am not dismissing the series yet, but if I find that SCMI can be used(after getting specifics from this series myself), I will at-least argue against the "SCMI can't be used" argument. -- Regards, Sudeep
Re: [PATCHv2 0/3] arm64 xilinx zynqmp firmware interface
On 17/08/17 11:32, Michal Simek wrote: > On 17.8.2017 11:12, Sudeep Holla wrote: >> >> >> On 17/08/17 09:42, Michal Simek wrote: >>> On 17.8.2017 09:52, Marc Zyngier wrote: >>>> On 17/08/17 07:10, Michal Simek wrote: >>>>> On 16.8.2017 17:39, Marc Zyngier wrote: >>>>>> On 16/08/17 13:24, Michal Simek wrote: >>>>>>> Hi, >>>>>>> >>>>>>> xilinx is using this interface for very long time and we >>>>>>> can't merge our driver changes to Linux because of >>>>>>> missing communication layer with firmware. This interface >>>>>>> was developed before scpi and scmi was available. In >>>>>>> connection to power management scpi and scmi are missing >>>>>>> pieces which we already use. There is a separate >>>>>>> discussion how to extend scmi to support all our use >>>>>>> cases. >>>>>> >>>>>> So maybe we should wait and see where this discussion is >>>>>> going before we merge yet another firmware interface? >>>>> >>>>> It will take a lot of time when this discussion ends and I >>>>> can't see any benefit to hold all >>>> >>>> Well, so far, the benefit of this series is exactly nil, as the >>>> code it brings is *unused*. It is impossible to review in >>>> isolation. >>>> >>> >>> Ok. I will add others drivers to this series that's not a >>> problem. >>> >>>> In the meantime, you can continue finding out how *not* to have >>>> to merge this code, and instead focus on using the >>>> infrastructure we already have, or at least influence the >>>> infrastructure that is being designed. It will be much better >>>> than dumping yet another slab of "I'm so different" code that >>>> is going to ultimately bitrot. >>> >>> I am happy to look the better proposed way. SCPI is ancient and >>> SCMI is replacement and not merged yet. We already had a call >>> with arm and Sudeep was on it too where outcome from that was >>> that we can't use that because it doesn't support what we need to >>> support now. >>> >> >> OK, none of the specifics were discussed in the meeting to conclude >> that SCMI can't be used. My takeaway from the meeting was Xilinx >> has this interface for long and being deployed in various systems. >> I would like to get into specifics before discarding SCMI as >> unusable. What bothers me more is that why was that not raised >> during the specification review which was quite a long period IMO ? >> I tend to think Xilinx never bothered to look/review the >> specification as this f/w interface was already there. > > Xilinx is using this interface from Aug 2015. I am not aware about > any invitation to spec review. And not sure who was there from > xilinx. > Sure, I can understand and that's not a problem but Xilinx was involved. >> >> However I still can't see why this was posted once we started >> pushing out SCMI patches especially given that this f/w interface >> was there for long and no attempts were made in past to upstream >> this. > > The reason is simple which is upstream our code which depends on > this communication layer. I don't think there is quick path to move > to different interface than this one. > Do you mean "smc" when you refer communication layer ? If so, that's fine. You can use "smc" as transport with SCMI if you want, specification doesn't prevent that. >> >> Also I am not dismissing the series yet, but if I find that SCMI >> can be used(after getting specifics from this series myself), I >> will at-least argue against the "SCMI can't be used" argument. > > This is not my argument that we can't use SCMI. This is what was my > understanding from that meeting we had. And definitely there is no > quick path for us to switch to SCMI and breaks all current existing > customers. > I understand the latter and I mentioned the same earlier, but I disagree with the former. That meeting was mostly introduction(and informal IMO) and didn't involve anything at the technical level. > And this interface is just in the same position as current SCPI. It > means you have SCPI already merged and you are adding new one. SCMI > could be maybe also just SCPIv2. Agreed, but it was posted as soon as the specification is out and so is the SCMI. I am not arguing it as a point, but just mentioning that this post was simply bad timing :) -- Regards, Sudeep
Re: [PATCH][RESEND] of: return of_get_cpu_node from of_cpu_device_node_get if CPUs are not registered
Hi Rob, On 14/08/17 17:18, Sudeep Holla wrote: > Instead of the callsites choosing between of_cpu_device_node_get if the > CPUs are registered as of_node is populated by then and of_get_cpu_node > when the CPUs are not yet registered as CPU of_nodes are not yet stashed > thereby needing to parse the device tree, we can call of_get_cpu_node > in case the CPUs are not yet registered in of_cpu_device_node_get. > > This will allow to use of_cpu_device_node_get anywhere hiding the > details from the caller. > > Cc: Rob Herring > Cc: Frank Rowand > Signed-off-by: Sudeep Holla > --- > include/linux/of_device.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Hi Rob, > > You have already acked this patch, but I wanted you to pick up this via > your tree and hence I am resending it. I tried pinging on that thread > but I can understand that it's lost in your mailbox :) > Gentle ping! -- Regards, Sudeep
Re: [PATCH v3 0/4] Generalize fncpy availability
On 20/06/17 17:20, Florian Fainelli wrote: > On 06/20/2017 02:10 AM, Lorenzo Pieralisi wrote: >> [+Sudeep] >> >> On Mon, Jun 19, 2017 at 10:32:38AM -0700, Florian Fainelli wrote: >>> On 06/19/2017 05:24 AM, Mark Rutland wrote: On Fri, Jun 16, 2017 at 05:07:40PM -0700, Florian Fainelli wrote: > Hi all, Hi Florian, > This patch series makes ARM's fncpy() implementation more generic > (dropping the > Thumb-specifics) and available in an asm-generic header file. > > Tested on a Broadcom ARM64 STB platform with code that is written to SRAM. > > Changes in v3 (thanks Doug!): > - correct include guard names in asm-generic/fncpy.h to __ASM_FNCPY_H > - utilize Kbuild to provide the fncpy.h header on ARM64 > > Changes in v2: > - leave the ARM implementation where it is > - make the generic truly generic (no) > > This is helpful in making SoC-specific power management code become true > drivers > that can be shared between different architectures. > Could you elaborate on what this is needed for? >>> >>> Several uses cases come to mind: >>> >>> - it could be used as a trampoline code prior to entering S2 for systems >>> that do not support PSCI 1.0 >> >> I think S2 here means PM_SUSPEND_MEM. It is very wrong to manage power >> states through platform specific hooks on PSCI based systems, consider >> upgrading to PSCI 1.0 please (or implement PSCI CPU_SUSPEND power >> states that allow to achieve same power savings as PM_SUSPEND_MEM >> by just entering suspend-to-idle). > > S2 is PM_SUSPEND_STANDBY and S3 is PM_SUSPEND_MEM, at least that how I > read it. I would rather we update to PSCI 1.0 (at least) to properly > support SYSTEM_SUSPEND rather than retrofitting a system-wide suspend > state into CPU_SUSPEND since that seems wrong. > This has been discussed multiple times in the past. No one has come back with strong reason to add that to the PSCI SYSTEM_SUSPEND API. Care to explain the difference between PM_SUSPEND_STANDBY and S3 is PM_SUSPEND_MEM on your platform. And why it can't be achieved with suspend-to-idle ? You can always report any issue with PSCI specification at err...@arm.com as mentioned in the document. -- Regards, Sudeep
[PATCH] mailbox: add support for doorbell/signal mode controllers
Some mailbox controllers are lack FIFOs or memory to transmit data. They typically contains single doorbell registers to just signal the remote. The actually data is transmitted/shared using some shared memory which is not part of the mailbox. Such controllers don't need to transmit any data, they just transmit the signal. In such controllers the data pointer passed to mbox_send_message is passed to client via it's tx_prepare callback. Controller doesn't need any data to be passed from the client. This patch introduce the new API send_signal to support such doorbell/ signal mode in mailbox controllers. This is useful to avoid another layer of abstraction as typically multiple channels can be multiplexied into single register. Cc: Jassi Brar Cc: Arnd Bergmann Cc: Bjorn Andersson Signed-off-by: Sudeep Holla --- drivers/mailbox/mailbox.c | 11 ++- include/linux/mailbox_controller.h | 11 +++ 2 files changed, 21 insertions(+), 1 deletion(-) Hi Jassi, Arnd, This is rough idea I have on extending mailbox interface to support the doorbell requirements. The new API send_signal will eliminate the issue Jassi has explained in earlier discussion with respect to generic message format using Rockchip example. Regards, Sudeep diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c index 674b35f402f5..495b4574b954 100644 --- a/drivers/mailbox/mailbox.c +++ b/drivers/mailbox/mailbox.c @@ -77,7 +77,10 @@ static void msg_submit(struct mbox_chan *chan) if (chan->cl->tx_prepare) chan->cl->tx_prepare(chan->cl, data); /* Try to submit a message to the MBOX controller */ - err = chan->mbox->ops->send_data(chan, data); + if (chan->mbox->ops->send_data) + err = chan->mbox->ops->send_data(chan, data); + else + err = chan->mbox->ops->send_signal(chan); if (!err) { chan->active_req = data; chan->msg_count--; @@ -451,6 +454,12 @@ int mbox_controller_register(struct mbox_controller *mbox) /* Sanity check */ if (!mbox || !mbox->dev || !mbox->ops || !mbox->num_chans) return -EINVAL; + /* +* A controller can support either doorbell mode or normal message +* transmission mode but not both +*/ + if (mbox->ops->send_data && mbox->ops->send_signal) + return -EINVAL; if (mbox->txdone_irq) txdone = TXDONE_BY_IRQ; diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h index 74deadb42d76..bdbc5b74097e 100644 --- a/include/linux/mailbox_controller.h +++ b/include/linux/mailbox_controller.h @@ -24,6 +24,16 @@ struct mbox_chan; * transmission of data is reported by the controller via * mbox_chan_txdone (if it has some TX ACK irq). It must not * sleep. + * @send_signal: The API asks the MBOX controller driver, in atomic + * context try to transmit a signal on the bus. Returns 0 if + * data is accepted for transmission, -EBUSY while rejecting + * if the remote hasn't yet absorbed the last signal sent. Actual + * transmission of data must be handled by the client and is + * reported by the controller via mbox_chan_txdone (if it has + * some TX ACK irq). It must not sleep. Unlike send_data, + * send_signal doesn't handle any messages/data. It just sends + * notification signal(doorbell) and client needs to prepare all + * the data. * @startup: Called when a client requests the chan. The controller * could ask clients for additional parameters of communication * to be provided via client's chan_data. This call may @@ -46,6 +56,7 @@ struct mbox_chan; */ struct mbox_chan_ops { int (*send_data)(struct mbox_chan *chan, void *data); + int (*send_signal)(struct mbox_chan *chan); int (*startup)(struct mbox_chan *chan); void (*shutdown)(struct mbox_chan *chan); bool (*last_tx_done)(struct mbox_chan *chan); -- 2.7.4
Re: [PATCH] mailbox: add support for doorbell/signal mode controllers
On 01/11/17 18:03, Jassi Brar wrote: > On Wed, Nov 1, 2017 at 10:02 PM, Sudeep Holla wrote: > >> >> Such controllers don't need to transmit any data, they just transmit >> the signal. In such controllers the data pointer passed to >> mbox_send_message is passed to client via it's tx_prepare callback. >> Controller doesn't need any data to be passed from the client. >> > Some controllers need a non-zero value written to a register in order > to trigger the signal. You are right, just right non-zero or whatever controller value to trigger the interrupt to remote. > That register is visible to the remote. While the data/packet is setup > during tx_prepare() callback. Agreed. > You are overlooking this class of doorbell controllers. > Not sure what do you mean by that ? >> >> This is rough idea I have on extending mailbox interface to support >> the doorbell requirements. >> > What doorbell requirements does the api not support? > QComm's APCS IPC is what you call a "doorbell" controller and is > already supported by the API. It could run SCMI even easier than MHU > (your controller). > Again agreed. But see below for reason to create this API. >> The new API send_signal will eliminate the >> issue Jassi has explained in earlier discussion with respect to generic >> message format using Rockchip example. >> > Sorry I don't see how. > Please explain how can send_signal() api be used by, say, rockchip to > support SCMI? > 80 writel_relaxed(msg->cmd, mb->mbox_base + MAILBOX_A2B_CMD(chans->idx)); 81 writel_relaxed(msg->rx_size, mb->mbox_base + 82MAILBOX_A2B_DAT(chans->idx)); 83 will be replaced with writel(whatever_value_to trigger_signal, MAILBOX_A2B_CMD(chans->idx)); in its send_signal function. > I am not convinced we should clone an api just so that a client driver > becomes simpler. Esp when it shifts, and not avoid, the additional > code (to support the client) onto the provider side. > It doesn't tie the data format with particular mailbox controller. send_data has void *data and the interpretation is controller specific. send_signal on the other handle can implemented by the controllers which knows how and can trigger the specific signal to the remote. -- Regards, Sudeep
Re: [PATCH v3 18/22] clk: add support for clocks provided by SCMI
On 02/11/17 07:23, Stephen Boyd wrote: > On 09/28, Sudeep Holla wrote: >> On some ARM based systems, a separate Cortex-M based System Control >> Processor(SCP) provides the overall power, clock, reset and system >> control. System Control and Management Interface(SCMI) Message Protocol >> is defined for the communication between the Application Cores(AP) >> and the SCP. >> >> This patch adds support for the clocks provided by SCP using SCMI >> protocol. >> >> Cc: Michael Turquette >> Cc: Stephen Boyd >> Cc: linux-...@vger.kernel.org >> Signed-off-by: Sudeep Holla >> --- > > Acked-by: Stephen Boyd > Thanks >> MAINTAINERS| 2 +- >> drivers/clk/Kconfig| 10 +++ >> drivers/clk/Makefile | 1 + >> drivers/clk/clk-scmi.c | 210 >> + >> 4 files changed, 222 insertions(+), 1 deletion(-) >> create mode 100644 drivers/clk/clk-scmi.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 23ec3471f542..32c184391aee 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -12941,7 +12941,7 @@ M: Sudeep Holla >> L: linux-arm-ker...@lists.infradead.org >> S: Maintained >> F: Documentation/devicetree/bindings/arm/arm,sc[mp]i.txt >> -F: drivers/clk/clk-scpi.c >> +F: drivers/clk/clk-sc[mp]i.c > > Is there a lot of copy/paste going on from clk-scpi.c? Maybe it > could be consolidated? > Not much apart from the usual driver skeleton. Also SCPI specification will not be enhanced any further while SCMI will be. So they will deviated in the feature set going further. Even I was thinking of merging then together initially but based on some WIP changes to the specification, I thought it may not be good idea. But if we think it can be merged in future , I will do that for sure(for easy maintenance) -- Regards, Sudeep
Re: [PATCH] mailbox: add support for doorbell/signal mode controllers
On 02/11/17 02:39, Jassi Brar wrote: > On Wed, Nov 1, 2017 at 11:45 PM, Sudeep Holla wrote: >> >> >> On 01/11/17 18:03, Jassi Brar wrote: >>> On Wed, Nov 1, 2017 at 10:02 PM, Sudeep Holla wrote: >>> >>>> >>>> Such controllers don't need to transmit any data, they just transmit >>>> the signal. In such controllers the data pointer passed to >>>> mbox_send_message is passed to client via it's tx_prepare callback. >>>> Controller doesn't need any data to be passed from the client. >>>> >>> Some controllers need a non-zero value written to a register in order >>> to trigger the signal. >> >> You are right, just right non-zero or whatever controller value to >> trigger the interrupt to remote. >> >>> That register is visible to the remote. While the data/packet is setup >>> during tx_prepare() callback. >> >> Agreed. >> >>> You are overlooking this class of doorbell controllers. >>> >> >> Not sure what do you mean by that ? >> > Such doorbell controllers can't use send_signal(chan) because they > need that non-zero value from client to send over the shared register. > You are assuming every protocol implements just one command. > No that non-zero value is not client specific, it's entirely controller specific. Not sure why do you think I am assuming every protocol implements just one command. >>>> >>>> This is rough idea I have on extending mailbox interface to support >>>> the doorbell requirements. >>>> >>> What doorbell requirements does the api not support? >>> QComm's APCS IPC is what you call a "doorbell" controller and is >>> already supported by the API. After looking at this, you will see that doorbell has not data specific to client in the above case. unsigned long idx = (unsigned long)chan->con_priv; writel(BIT(idx), apcs->reg); So it's channel specific, same in mailbox-sti >> Again agreed. But see below for reason to create this API. >> >>>> The new API send_signal will eliminate the >>>> issue Jassi has explained in earlier discussion with respect to generic >>>> message format using Rockchip example. >>>> >>> Sorry I don't see how. >>> Please explain how can send_signal() api be used by, say, rockchip to >>> support SCMI? >>> >> >> 80 writel_relaxed(msg->cmd, mb->mbox_base + >> MAILBOX_A2B_CMD(chans->idx)); >> 81 writel_relaxed(msg->rx_size, mb->mbox_base + >> >> 82MAILBOX_A2B_DAT(chans->idx)); >> >> 83 >> >> will be replaced with >> >> writel(whatever_value_to trigger_signal, MAILBOX_A2B_CMD(chans->idx)); >> >> in its send_signal function. >> > 1) Where does the "whatever_value_to_trigger_signal" come from? Controller specific. > That has to come from client. No. > You can not dictate the channel transfers a fixed u32 value over its >lifetime. SCMI may use one command code but other protocols use more. Yes if it's just a doorbell, see the above 2 cases I have pointed out. > > 2) Using 'rx_size' is not a software choice made in the driver. The > _hardware_ has two registers shared with remote side - a CMD and a > DATA register. So the driver (written agnostic to any particular > client) would naturally expect the command+data from the client to be > programmed in to CMD and DAT registers. > OK, if this controller needs to be used in doorbell mode for SCMI, we can send one fixed cmd and fixed rx_size() or 1 based on inclusive or exclusive). > >>> I am not convinced we should clone an api just so that a client driver >>> becomes simpler. Esp when it shifts, and not avoid, the additional >>> code (to support the client) onto the provider side. >>> >> >> It doesn't tie the data format with particular mailbox controller. >> send_data has void *data and the interpretation is controller specific. >> send_signal on the other handle can implemented by the controllers which >> knows how and can trigger the specific signal to the remote. >> > Yeah that's what I said - you want to make a client simpler by pushing > the code requirement onto the provider side. > No, I want to support generic case of mailbox doorbell instead of creating another unnecessary abstraction layer > For example, you mean we modify the provider rockchip-mailbox.c by > implementing > > rockchip_send_signal(chan) > { > struct rockchip_mbox_msg msg; > > msg.cmd = chan->idx; //only one command supported by the channel !!! Yes, it's just a doorbell. That actual data is transmitted or shared elsewhere. This doorbell is a signal to the remote to examine that, > msg.rx_size = 0; > > rockchip_send_data(chan, (void*) &msg); > } > > whereas I suggest this SCMI specific code should be part of > transport/mapping shim layer of SCMI. > Yes that's what I did with abstraction and few think including me that it's unnecessary abstraction for such a generic use. -- Regards, Sudeep
Re: [PATCH] mailbox: add support for doorbell/signal mode controllers
On 01/11/17 22:12, Bjorn Andersson wrote: > On Wed 01 Nov 11:03 PDT 2017, Jassi Brar wrote: >> On Wed, Nov 1, 2017 at 10:02 PM, Sudeep Holla wrote: > [..] >>> >>> This is rough idea I have on extending mailbox interface to support >>> the doorbell requirements. >>> >> What doorbell requirements does the api not support? >> QComm's APCS IPC is what you call a "doorbell" controller and is >> already supported by the API. It could run SCMI even easier than MHU >> (your controller). >> > > I agree; from a mbox consumer perspective a doorbell should be a mailbox > channel that when signalled will ring the bell, i.e. the message is not > significant and should not be provided by the client. > Exactly. > If the message is significant and is not derived from the mailbox > channel (e.g. channel id -> bit in register) it is not a mailbox > doorbell, it's s regular mailbox used as a doorbell. > Agreed, in my case(ARM MHU) it's indeed a register bit. > > The potential improvement I see in the Qualcomm case is to wrap the > mbox_send_message(chan, NULL); mbox_client_txdone(chan, 0); calls in one > simple "mbox_ring_door_bell(chan)" - but I haven't investigated the > validity of this as a generic function. > Yes that's exactly what I want to do as we make progress with this patch. For that we find need to add send_signal(chan), instead of send_data(chan, data). -- Regards, Sudeep
Re: [PATCH] mailbox: add support for doorbell/signal mode controllers
On 02/11/17 11:26, Jassi Brar wrote: > On Thu, Nov 2, 2017 at 4:17 PM, Sudeep Holla wrote: [...] >> >> No that non-zero value is not client specific, it's entirely controller >> specific. >> > ?? > For example BCM2835 has such a controller. Have a look at > bcm2835_send_data() and let me know what is that controller specific > value. > You can keep finding one or the other platform that has a deviation. Come on there are generic infrastructure support in many subsystem that are just used in one or two platforms. I hope you agree for this enhancement to the mailbox framework as it's more commonly used mode. I am not saying this patch is final, but I just want an agreement to add such a support. [...] >>> 1) Where does the "whatever_value_to_trigger_signal" come from? >> >> Controller specific. >> >>> That has to come from client. >> >> No. >> > Again, let me know what does the controller expect 'val' to be > > writel(val, MAILBOX_A2B_CMD(chans->idx)) > It depends on the controller. Whatever value that can generate a signal to remote. > > Your entire post is based on your assertion that the controller > expects a particular non-zero value to trigger a signal, which is > wrong. Why do you think that ? There are lots of example in the mailbox today. Please have a look at few example which don't use data passed from the client: 1. pcc_send_data (drivers/mailbox/pcc.c) 2. sti_mbox_send_data (drivers/mailbox/mailbox-sti.c) 3. qcom_apcs_ipc_send_data (drivers/mailbox/qcom-apcs-ipc-mailbox.c) 4. tegra_hsp_doorbell_send_data (drivers/mailbox/tegra-hsp.c) And SCMI fits the above case. Also you keep saying I am making this change to get SCMI with ARM MHU. Honestly I don't care much about that, I need better support from mailbox framework if possible for any platforms running SCMI. So please stop assuming my changes are motivated by that. SCMI is designed to solve more generic consolidation issues, so I am more focused on that than getting it run on some development platform I have with ARM MHU. Believe me that's least of my concern. -- Regards, Sudeep
Re: [tip:x86/mm] mm/sparsemem: Allocate mem_section at runtime for CONFIG_SPARSEMEM_EXTREME=y
(+Will, Catalin) On Fri, Oct 20, 2017 at 1:27 PM, tip-bot for Kirill A. Shutemov wrote: > Commit-ID: 83e3c48729d9ebb7af5a31a504f3fd6aff0348c4 > Gitweb: > https://git.kernel.org/tip/83e3c48729d9ebb7af5a31a504f3fd6aff0348c4 > Author: Kirill A. Shutemov > AuthorDate: Fri, 29 Sep 2017 17:08:16 +0300 > Committer: Ingo Molnar > CommitDate: Fri, 20 Oct 2017 13:07:09 +0200 > > mm/sparsemem: Allocate mem_section at runtime for CONFIG_SPARSEMEM_EXTREME=y > > Size of the mem_section[] array depends on the size of the physical address > space. > > In preparation for boot-time switching between paging modes on x86-64 > we need to make the allocation of mem_section[] dynamic, because otherwise > we waste a lot of RAM: with CONFIG_NODE_SHIFT=10, mem_section[] size is 32kB > for 4-level paging and 2MB for 5-level paging mode. > > The patch allocates the array on the first call to > sparse_memory_present_with_active_regions(). > I am seeing a boot failure with this patch in linux-next today(20171102) Unable to handle kernel NULL pointer dereference at virtual address Mem abort info: ESR = 0x9604 Exception class = DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 Data abort info: ISV = 0, ISS = 0x0004 CM = 0, WnR = 0 [] user address but active_mm is swapper Internal error: Oops: 9604 [#1] PREEMPT SMP Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 4.14.0-rc7-next-20171102 #133 Hardware name: ARM Juno development board (r2) (DT) task: 08f82a80 task.stack: 08f7 pstate: 20c5 (nzCv daIF -PAN -UAO) pc : memory_present+0x5c/0xe8 lr : memory_present+0x34/0xe8 sp : 08f73e90 x29: 08f73e90 x28: 80e60018 x27: fd9b8d18 x26: 0105 x25: x24: 090c4000 x23: x22: 090c4000 x21: 0008 x20: 0004 x19: x18: 0010 x17: 0001 x16: x15: x14: 8909a3af x13: 0909a3bd x12: 08f79df0 x11: 08590de8 x10: 08f9c7f0 x9 : x8 : 80097ffccc80 x7 : x6 : 003f x5 : 08f79fc0 x4 : 0001 x3 : 0010 x2 : 000e x1 : 0008 x0 : Process swapper (pid: 0, stack limit = 0x08f7) Call trace: memory_present+0x5c/0xe8 bootmem_init+0x90/0x114 setup_arch+0x190/0x4a0 start_kernel+0x64/0x3a8 Code: 54000449 d35afeb3 f94032c0 d37df273 (f8736800) random: get_random_bytes called from print_oops_end_marker+0x4c/0x68 with crng_init=0 ---[ end trace ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task!
Re: [PATCH] mailbox: add support for doorbell/signal mode controllers
On 02/11/17 12:21, Jassi Brar wrote: > On Thu, Nov 2, 2017 at 5:19 PM, Sudeep Holla wrote: >> On 02/11/17 11:26, Jassi Brar wrote: > >>>>> 1) Where does the "whatever_value_to_trigger_signal" come from? >>>> >>>> Controller specific. >>>> >>>>> That has to come from client. >>>> >>>> No. >>>> >>> Again, let me know what does the controller expect 'val' to be >>> >>> writel(val, MAILBOX_A2B_CMD(chans->idx)) >>> >> >> It depends on the controller. Whatever value that can generate a signal >> to remote. >> > As you _know_, the controller expects any non-zero value. Now what > value would you write in there? > I just said its *non-zero value* to give example. What action needs to be done to trigger the doorbell is *entirely* controller specific and typically it's a bit in the register. >> >> 1. pcc_send_data (drivers/mailbox/pcc.c) >> 2. sti_mbox_send_data (drivers/mailbox/mailbox-sti.c) >> 3. qcom_apcs_ipc_send_data (drivers/mailbox/qcom-apcs-ipc-mailbox.c) >> 4. tegra_hsp_doorbell_send_data (drivers/mailbox/tegra-hsp.c) >> >> And SCMI fits the above case. >> > These are only 4 out of 14. Can we overlook that your implementation > rules out 70% controllers. > I am *not* saying we will break other 10 controllers. All I am says there are 4 controllers that can make use of this new feature. 4 is good number IMO to generalize something. > BTW these 4 don't even need any send_signal() api, they would remain > unchanged. What's the new api for? > Sure, it's working fine doesn't mean it can't be used at all. That's not a right argument TBH. -- Regards, Sudeep
Re: [tip:x86/mm] mm/sparsemem: Allocate mem_section at runtime for CONFIG_SPARSEMEM_EXTREME=y
On 02/11/17 13:34, Kirill A. Shutemov wrote: > On Thu, Nov 02, 2017 at 12:31:54PM +0000, Sudeep Holla wrote: >> (+Will, Catalin) >> >> On Fri, Oct 20, 2017 at 1:27 PM, tip-bot for Kirill A. Shutemov >> wrote: >>> Commit-ID: 83e3c48729d9ebb7af5a31a504f3fd6aff0348c4 >>> Gitweb: >>> https://git.kernel.org/tip/83e3c48729d9ebb7af5a31a504f3fd6aff0348c4 >>> Author: Kirill A. Shutemov >>> AuthorDate: Fri, 29 Sep 2017 17:08:16 +0300 >>> Committer: Ingo Molnar >>> CommitDate: Fri, 20 Oct 2017 13:07:09 +0200 >>> >>> mm/sparsemem: Allocate mem_section at runtime for CONFIG_SPARSEMEM_EXTREME=y >>> >>> Size of the mem_section[] array depends on the size of the physical address >>> space. >>> >>> In preparation for boot-time switching between paging modes on x86-64 >>> we need to make the allocation of mem_section[] dynamic, because otherwise >>> we waste a lot of RAM: with CONFIG_NODE_SHIFT=10, mem_section[] size is 32kB >>> for 4-level paging and 2MB for 5-level paging mode. >>> >>> The patch allocates the array on the first call to >>> sparse_memory_present_with_active_regions(). >>> >> >> I am seeing a boot failure with this patch in linux-next today(20171102) > > Could you share the kernel config? > It's the default config on arm64. Generated file is almost 160kB, I will send it to you off-list. > Have you bisected the failure to the commit? > I just reverted this commit as I suspected that and it boots fine after the revert. -- Regards, Sudeep
Re: [tip:x86/mm] mm/sparsemem: Allocate mem_section at runtime for CONFIG_SPARSEMEM_EXTREME=y
On 02/11/17 14:12, Kirill A. Shutemov wrote: > On Thu, Nov 02, 2017 at 01:42:42PM +0000, Sudeep Holla wrote: >> >> >> On 02/11/17 13:34, Kirill A. Shutemov wrote: >>> On Thu, Nov 02, 2017 at 12:31:54PM +, Sudeep Holla wrote: >>>> (+Will, Catalin) >>>> >>>> On Fri, Oct 20, 2017 at 1:27 PM, tip-bot for Kirill A. Shutemov >>>> wrote: >>>>> Commit-ID: 83e3c48729d9ebb7af5a31a504f3fd6aff0348c4 >>>>> Gitweb: >>>>> https://git.kernel.org/tip/83e3c48729d9ebb7af5a31a504f3fd6aff0348c4 >>>>> Author: Kirill A. Shutemov >>>>> AuthorDate: Fri, 29 Sep 2017 17:08:16 +0300 >>>>> Committer: Ingo Molnar >>>>> CommitDate: Fri, 20 Oct 2017 13:07:09 +0200 >>>>> >>>>> mm/sparsemem: Allocate mem_section at runtime for >>>>> CONFIG_SPARSEMEM_EXTREME=y >>>>> >>>>> Size of the mem_section[] array depends on the size of the physical >>>>> address space. >>>>> >>>>> In preparation for boot-time switching between paging modes on x86-64 >>>>> we need to make the allocation of mem_section[] dynamic, because otherwise >>>>> we waste a lot of RAM: with CONFIG_NODE_SHIFT=10, mem_section[] size is >>>>> 32kB >>>>> for 4-level paging and 2MB for 5-level paging mode. >>>>> >>>>> The patch allocates the array on the first call to >>>>> sparse_memory_present_with_active_regions(). >>>>> >>>> >>>> I am seeing a boot failure with this patch in linux-next today(20171102) >>> >>> Could you share the kernel config? >>> >> >> It's the default config on arm64. Generated file is almost 160kB, I will >> send it to you off-list. >> >>> Have you bisected the failure to the commit? >>> >> I just reverted this commit as I suspected that and it boots fine after >> the revert. > > Could you try the patch below instead? > > From 4a9d843f9d939d958612b0079ebe5743f265e1e0 Mon Sep 17 00:00:00 2001 > From: "Kirill A. Shutemov" > Date: Thu, 2 Nov 2017 17:02:29 +0300 > Subject: [PATCH] mm, sparse: Fix boot on arm64 > > Since 83e3c48729d9 ("mm/sparsemem: Allocate mem_section at runtime for > CONFIG_SPARSEMEM_EXTREME=y") we allocate mem_section dynamically in > sparse_memory_present_with_active_regions(). But some architectures, like > arm64, don't use the routine to initialize sparsemem. > > Let's move the initialization into memory_present() it should cover all > architectures. > Thanks for the quick fix. It boots fine with this patch. Reported-and-tested-by: Sudeep Holla -- Regards, Sudeep
Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init
On Sun, Nov 12, 2017 at 11:24:56PM +0100, Arnd Bergmann wrote: > On Sun, Nov 12, 2017 at 10:16 PM, Thomas Gleixner wrote: > > On Mon, 6 Nov 2017, Arnd Bergmann wrote: > >> The newly added function triggers a harmless Kbuild warning because > >> of a missing annotation: > >> > >> WARNING: vmlinux.o(.text+0x448098): Section mismatch in reference from the > >> function timer_of_exit() to the function .init.text:timer_clk_exit() > >> The function timer_of_exit() references > >> the function __init timer_clk_exit(). > >> This is often because timer_of_exit lacks a __init > >> annotation or the annotation of timer_clk_exit is wrong. > >> > >> The function is only called from other __init functions, so it > >> can safely be marked as __init as well. > > > > Hmm. I don't see any caller at all. From the intention of the patch I > > assume this isn't designed for using from init functions, so we rather have > > to remove the __init annotations from the called functions. > > > > Sudeep posted a patch which does that: > > > > > > https://lkml.kernel.org/r/1509979716-10646-1-git-send-email-sudeep.ho...@arm.com > > > > Though I rather would know whether this function is going to be used at > > all and what the intention of this patch was. > > > > Benjamin > > My interpretation was that timer drivers are still supposed to be unregistered > at module unload time, but that you might use the new timer_of_exit() > in the failure path of whatever function calls timer_of_init() successfully > when something fails in the next step. > > Sudeep's interpretation also makes sense, I had not thought of that, but > I now found the patch that adds a user in an init function: > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1519644.html > > It seems I guessed right and Sudeep guessed wrong (both by pure chance > I admit). Ah OK, I just went by name that it will be called by some exit/remove function. > Both patches solve the problem, Sudeep's version is a little > more robust in case we ever add a caller in an __exit function (which I > think is currently not allowed), while mine saves a little bit of memory > and matches the current usage better. > Agreed, may be if we add users which is called from init functions, the warning should disappear. Also as tglx suggested, we could rename if it's just used from init function error/exit paths. -- Regards, Sudeep
Re: [PATCH] clocksource/drivers/timer-of: mark timer_of_exit as __init
On 13/11/17 14:58, Daniel Lezcano wrote: > On 13/11/2017 11:11, Sudeep Holla wrote: [..] >> >> Agreed, may be if we add users which is called from init functions, the >> warning should disappear. Also as tglx suggested, we could rename if it's >> just used from init function error/exit paths. > > The drivers are not compiled as module AFAICT, the function will be > called in the init error path. > Understood, IMO better to rename the functions as cleanup or something similar. exit made it sound differently and since there were no users, I assumed it to be used in remove/exit functions. Sorry for the noise. -- Regards, Sudeep
Re: next-20171102: ARM64 dies on boot
On 03/11/17 04:18, Yury Norov wrote: > Hi all, > > I reproduce it with qemu. The exact reason of panic is the NULL-dereference > in memory_present: > (gdb) bt > #0 0x08dd8c6c in sparse_index_init (nid=, > section_nr=) > at mm/sparse.c:80 > #1 memory_present (nid=0, start=18446462598881083392, end=0) at > mm/sparse.c:215 > #2 0x08dc518c in arm64_memory_present () at arch/arm64/mm/init.c:307 > #3 bootmem_init () at arch/arm64/mm/init.c:500 > #4 0x08dc28fc in setup_arch (cmdline_p=) at > arch/arm64/kernel/setup.c:287 > #5 0x08dc083c in start_kernel () at init/main.c:530 > #6 0x in ?? () > [...] > This is very early stage, so there's no messages in console. > Config is attached. If no ideas, I can bisect it later. > Reported and fixed[1], may be not in -next yet. -- Regards, Sudeep [1] https://marc.info/?l=linux-kernel&m=150962592016250&w=2
[PATCH v4 02/20] dt-bindings: arm: add support for ARM System Control and Management Interface(SCMI) protocol
This patch adds devicetree binding for System Control and Management Interface (SCMI) Message Protocol used between the Application Cores(AP) and the System Control Processor(SCP). The MHU peripheral provides a mechanism for inter-processor communication between SCP's M3 processor and AP. SCP offers control and management of the core/cluster power states, various power domain DVFS including the core/cluster, certain system clocks configuration, thermal sensors and many others. SCMI protocol is developed as better replacement to the existing SCPI which is not flexible and easily extensible. Cc: Rob Herring Cc: Mark Rutland Signed-off-by: Sudeep Holla --- Documentation/devicetree/bindings/arm/arm,scmi.txt | 179 + MAINTAINERS| 4 +- 2 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/arm/arm,scmi.txt diff --git a/Documentation/devicetree/bindings/arm/arm,scmi.txt b/Documentation/devicetree/bindings/arm/arm,scmi.txt new file mode 100644 index ..5f3719ab7075 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/arm,scmi.txt @@ -0,0 +1,179 @@ +System Control and Management Interface (SCMI) Message Protocol +-- + +The SCMI is intended to allow agents such as OSPM to manage various functions +that are provided by the hardware platform it is running on, including power +and performance functions. + +This binding is intended to define the interface the firmware implementing +the SCMI as described in ARM document number ARM DUI 0922B ("ARM System Control +and Management Interface Platform Design Document")[0] provide for OSPM in +the device tree. + +Required properties: + +The scmi node with the following properties shall be under the /firmware/ node. + +- compatible : shall be "arm,scmi" +- mboxes: List of phandle and mailbox channel specifiers. It should contain + exactly one or two mailboxes, one for transmitting messages("tx") + and another optional for receiving the notifications("rx") if + supported. +- shmem : List of phandle pointing to the shared memory(SHM) area as per + generic mailbox client binding. +- #address-cells : should be '1' if the device has sub-nodes, maps to + protocol identifier for a given sub-node. +- #size-cells : should be '0' as 'reg' property doesn't have any size + associated with it. + +Optional properties: + +- mbox-names: shall be "tx" or "rx" depending on mboxes entries. + +See Documentation/devicetree/bindings/mailbox/mailbox.txt for more details +about the generic mailbox controller and client driver bindings. + +The mailbox is the only permitted method of calling the SCMI firmware. +Mailbox doorbell is used as a mechanism to alert the presence of a +messages and/or notification. + +Each protocol supported shall have a sub-node with corresponding compatible +as described in the following sections. If the platform supports dedicated +communication channel for a particular protocol, the 3 properties namely: +mboxes, mbox-names and shmem shall be present in the sub-node corresponding +to that protocol. + +Clock/Performance bindings for the clocks/OPPs based on SCMI Message Protocol + + +This binding uses the common clock binding[1]. + +Required properties: +- #clock-cells : Should be 1. Contains the Clock ID value used by SCMI commands. + +Power domain bindings for the power domains based on SCMI Message Protocol + + +This binding for the SCMI power domain providers uses the generic power +domain binding[2]. + +Required properties: + - #power-domain-cells : Should be 1. Contains the device or the power +domain ID value used by SCMI commands. + +Sensor bindings for the sensors based on SCMI Message Protocol +-- +SCMI provides an API to access the various sensors on the SoC. + +Required properties: +- #thermal-sensor-cells: should be set to 1. This property follows the +thermal device tree bindings[3]. + +Valid cell values are raw identifiers (Sensor ID) +as used by the firmware. Refer to platform details +for your implementation for the IDs to use. + +SRAM and Shared Memory for SCMI +--- + +A small area of SRAM is reserved for SCMI communication between application +processors and SCP. + +The properties should follow the generic mmio-sram description found in [4] + +Each sub-node represents the reserved area for SCMI. + +Required sub-node properties: +- reg : The base offset and size of the reserved area with the SR
[PATCH v4 10/20] firmware: arm_scmi: probe and initialise all the supported protocols
Now that we have basic support for all the protocols in the specification, let's probe them individually and initialise them. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 51 +- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 20d083be474c..11c18ac9816f 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -494,6 +494,21 @@ void scmi_setup_protocol_implemented(const struct scmi_handle *handle, info->protocols_imp = prot_imp; } +static bool +scmi_is_protocol_implemented(const struct scmi_handle *handle, u8 prot_id) +{ + int i; + struct scmi_info *info = handle_to_scmi_info(handle); + + if (!info->protocols_imp) + return false; + + for (i = 0; i < MAX_PROTOCOLS_IMP; i++) + if (info->protocols_imp[i] == prot_id) + return true; + return false; +} + /** * scmi_handle_get() - Get the SCMI handle for a device * @@ -691,6 +706,23 @@ static inline int scmi_mbox_chan_setup(struct scmi_info *info) return 0; } +static inline void +scmi_create_protocol_device(struct device_node *np, struct scmi_info *info, + int prot_id) +{ + struct scmi_device *sdev; + + sdev = scmi_device_create(np, info->dev, prot_id); + if (!sdev) { + dev_err(info->dev, "failed to create %d protocol device\n", + prot_id); + return; + } + + /* setup handle now as the transport is ready */ + scmi_set_handle(sdev); +} + static int scmi_probe(struct platform_device *pdev) { int ret; @@ -698,7 +730,7 @@ static int scmi_probe(struct platform_device *pdev) const struct scmi_desc *desc; struct scmi_info *info; struct device *dev = &pdev->dev; - struct device_node *np = dev->of_node; + struct device_node *child, *np = dev->of_node; /* Only mailbox method supported, check for the presence of one */ if (scmi_mailbox_check(np)) { @@ -741,6 +773,23 @@ static int scmi_probe(struct platform_device *pdev) list_add_tail(&info->node, &scmi_list); mutex_unlock(&scmi_list_mutex); + for_each_available_child_of_node(np, child) { + u32 prot_id; + + if (of_property_read_u32(child, "reg", &prot_id)) + continue; + + prot_id &= MSG_PROTOCOL_ID_MASK; + + if (!scmi_is_protocol_implemented(handle, prot_id)) { + dev_err(dev, "SCMI protocol %d not implemented\n", + prot_id); + continue; + } + + scmi_create_protocol_device(child, info, prot_id); + } + return 0; } -- 2.7.4
[PATCH v4 15/20] firmware: arm_scmi: add device power domain support using genpd
This patch hooks up the support for device power domain provided by SCMI using the Linux generic power domain infrastructure. Cc: Kevin Hilman Cc: Ulf Hansson Signed-off-by: Sudeep Holla --- drivers/firmware/Kconfig | 13 +++ drivers/firmware/arm_scmi/Makefile | 1 + drivers/firmware/arm_scmi/scmi_pm_domain.c | 140 + 3 files changed, 154 insertions(+) create mode 100644 drivers/firmware/arm_scmi/scmi_pm_domain.c diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 86ea9d531587..86d2901ad87a 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -40,6 +40,19 @@ config ARM_SCMI_PROTOCOL This protocol library provides interface for all the client drivers making use of the features offered by the SCMI. +config ARM_SCMI_POWER_DOMAIN + tristate "SCMI power domain driver" + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) + default y + select PM_GENERIC_DOMAINS if PM + help + This enables support for the SCMI power domains which can be + enabled or disabled via the SCP firmware + + This driver can also be built as a module. If so, the module + will be called scmi_pm_domain. Note this may needed early in boot + before rootfs may be available. + config ARM_SCPI_PROTOCOL tristate "ARM System Control and Power Interface (SCPI) Message Protocol" depends on ARM || ARM64 || COMPILE_TEST diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index 3236890905b9..99e36c580fbc 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -2,3 +2,4 @@ obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o scmi-bus-y = bus.o scmi-driver-y = driver.o scmi-protocols-y = base.o clock.o perf.o power.o sensors.o +obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o diff --git a/drivers/firmware/arm_scmi/scmi_pm_domain.c b/drivers/firmware/arm_scmi/scmi_pm_domain.c new file mode 100644 index ..d8c9aa6960cf --- /dev/null +++ b/drivers/firmware/arm_scmi/scmi_pm_domain.c @@ -0,0 +1,140 @@ +/* + * SCMI Generic power domain support. + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include +#include +#include +#include +#include + +struct scmi_pm_domain { + struct generic_pm_domain genpd; + const struct scmi_handle *handle; + const char *name; + u32 domain; +}; + +#define to_scmi_pd(gpd) container_of(gpd, struct scmi_pm_domain, genpd) + +static int scmi_pd_power(struct generic_pm_domain *domain, bool power_on) +{ + int ret; + u32 state, ret_state; + struct scmi_pm_domain *pd = to_scmi_pd(domain); + const struct scmi_power_ops *ops = pd->handle->power_ops; + + if (power_on) + state = SCMI_POWER_STATE_GENERIC_ON; + else + state = SCMI_POWER_STATE_GENERIC_OFF; + + ret = ops->state_set(pd->handle, pd->domain, state); + if (!ret) + ret = ops->state_get(pd->handle, pd->domain, &ret_state); + if (!ret && state != ret_state) + return -EIO; + + return ret; +} + +static int scmi_pd_power_on(struct generic_pm_domain *domain) +{ + return scmi_pd_power(domain, true); +} + +static int scmi_pd_power_off(struct generic_pm_domain *domain) +{ + return scmi_pd_power(domain, false); +} + +static int scmi_pm_domain_probe(struct scmi_device *sdev) +{ + int num_domains, i; + struct device *dev = &sdev->dev; + struct device_node *np = dev->of_node; + struct scmi_pm_domain *scmi_pd; + struct genpd_onecell_data *scmi_pd_data; + struct generic_pm_domain **domains; + const struct scmi_handle *handle = sdev->handle; + + if (!handle || !handle->power_ops) + return -ENODEV; + + num_domains = handle->power_ops->num_domains_get(handle); + if (num_domains < 0) { + dev_err(dev, "number of domains not found\n"); + return num_domains; + } + + scmi_pd = devm_kcalloc(dev, num_domains, sizeof(*scmi_pd), GFP_KERNEL); + if (!scmi_pd) + return -ENOMEM; + + scmi_pd_data = devm_kzalloc(dev, sizeof(
[PATCH v4 00/20] firmware: ARM System Control and Management Interface(SCMI) support
Hi all, ARM System Control and Management Interface(SCMI) is more flexible and easily extensible than any of the existing interfaces. Many vendors were involved in the making of this formal specification and is now published[1]. There is a strong trend in the industry to provide micro-controllers in systems to abstract various power, or other system management tasks. These controllers usually have similar interfaces, both in terms of the functions that are provided by them, and in terms of how requests are communicated to them. This specification is to standardise and avoid (any further) fragmentation in the design of such interface by various vendors. This patch set is intended to get feedback on the design and structure of the code. This is not complete and not fully tested due to non-availability of firmware with full feature set at this time. It currently doesn't support notification, asynchronous/delayed response, perf/power statistics region and sensor register region to name a few. I have borrowed some of the ideas of message allocation/management from TI SCI. Changes: v3[5]->v4: - Added SCMI protocol bus to enumerate supported protocols as suggested by Arnd - Dropped the abstraction to mailbox as it may be optional v2[4]->v3: - Addressed various comments recieved so far(clock, hwmon and cpufreq drivers along with scmi drivers) - Hwmon driver now uses core layer to create and manage sysfs attributes - Added a shim layer to abstract the mailbox interface to support any custom adaptation required by the controller driver - Simple ARM MHU shim layer using newly added abstraction v1[3]->v2[4]: - Additional support for polling based DVFS and per protocol channels - Dependent drivers(clock, hwmon, cpufreq and power domains) - Various other review comments and issued found during testing addressed - Explicit binding for method dropped as even SMC based method are adviertised as mailbox RFC[2]->v1[3]: - Add generic mailbox binding for shared memory(Rob H) - Dropped compatibles per protocol(Suggested by Matt S) - Dropped lot of unnecessary pointer casting(Arnd B) - Dropped packing of structures(Arnd B) - Few other changes/additions based initial testing with firmware providing SCMI interface to OSPM -- Regards, Sudeep [1] http://infocenter.arm.com/help/topic/com.arm.doc.den0056a/index.html [2] https://marc.info/?l=linux-kernel&m=149685193627620&w=2 [3] https://marc.info/?l=linux-arm-kernel&m=149849482623492&w=2 [4] https://marc.info/?l=devicetree&m=150185763105926&w=2 [5] https://marc.info/?l=devicetree&m=150660452015351&w=2 Sudeep Holla (20): dt-bindings: mailbox: add support for mailbox client shared memory dt-bindings: arm: add support for ARM System Control and Management Interface(SCMI) protocol firmware: arm_scmi: add basic driver infrastructure for SCMI firmware: arm_scmi: add common infrastructure and support for base protocol firmware: arm_scmi: add scmi protocol bus to enumerate protocol devices firmware: arm_scmi: add initial support for performance protocol firmware: arm_scmi: add initial support for clock protocol firmware: arm_scmi: add initial support for power protocol firmware: arm_scmi: add initial support for sensor protocol firmware: arm_scmi: probe and initialise all the supported protocols firmware: arm_scmi: add support for polling based SCMI transfers firmware: arm_scmi: add option for polling based performance domain operations firmware: arm_scmi: refactor in preparation to support per-protocol channels firmware: arm_scmi: add per-protocol channels support using idr objects firmware: arm_scmi: add device power domain support using genpd clk: add support for clocks provided by SCMI hwmon: (core) Add hwmon_max to hwmon_sensor_types enumeration hwmon: add support for sensors exported via ARM SCMI cpufreq: add support for CPU DVFS based on SCMI message protocol cpufreq: scmi: add support for fast frequency switching Documentation/devicetree/bindings/arm/arm,scmi.txt | 179 + .../devicetree/bindings/mailbox/mailbox.txt| 28 + MAINTAINERS| 11 +- drivers/clk/Kconfig| 10 + drivers/clk/Makefile | 1 + drivers/clk/clk-scmi.c | 213 + drivers/cpufreq/Kconfig.arm| 11 + drivers/cpufreq/Makefile | 1 + drivers/cpufreq/scmi-cpufreq.c | 290 +++ drivers/firmware/Kconfig | 34 + drivers/firmware/Makefile | 1 + drivers/firmware/arm_scmi/Makefile | 5 + drivers/firmware/arm_scm
[PATCH v4 17/20] hwmon: (core) Add hwmon_max to hwmon_sensor_types enumeration
It's useful to know the maximum types of sensor supported by hwmon framework. It can be used to allocate some data structures when sorting the monitors based on their type. This will be used by scmi hwmon support. Cc: linux-hw...@vger.kernel.org Acked-by: Guenter Roeck Signed-off-by: Sudeep Holla --- include/linux/hwmon.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h index ceb751987c40..e5fd2707b6df 100644 --- a/include/linux/hwmon.h +++ b/include/linux/hwmon.h @@ -29,6 +29,7 @@ enum hwmon_sensor_types { hwmon_humidity, hwmon_fan, hwmon_pwm, + hwmon_max, }; enum hwmon_chip_attributes { -- 2.7.4
[PATCH v4 19/20] cpufreq: add support for CPU DVFS based on SCMI message protocol
On some ARM based systems, a separate Cortex-M based System Control Processor(SCP) provides the overall power, clock, reset and system control including CPU DVFS. SCMI Message Protocol is used to communicate with the SCP. This patch adds a cpufreq driver for such systems using SCMI interface to drive CPU DVFS. Cc: "Rafael J. Wysocki" Cc: Viresh Kumar Cc: linux...@vger.kernel.org Signed-off-by: Sudeep Holla --- MAINTAINERS| 2 +- drivers/cpufreq/Kconfig.arm| 11 ++ drivers/cpufreq/Makefile | 1 + drivers/cpufreq/scmi-cpufreq.c | 274 + 4 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 drivers/cpufreq/scmi-cpufreq.c diff --git a/MAINTAINERS b/MAINTAINERS index 1f9004d68ad4..f0a4251461a7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12955,7 +12955,7 @@ L: linux-arm-ker...@lists.infradead.org S: Maintained F: Documentation/devicetree/bindings/arm/arm,sc[mp]i.txt F: drivers/clk/clk-sc[mp]i.c -F: drivers/cpufreq/scpi-cpufreq.c +F: drivers/cpufreq/sc[mp]i-cpufreq.c F: drivers/firmware/arm_scpi.c F: drivers/firmware/arm_scmi/ F: include/linux/sc[mp]i_protocol.h diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm index bdce4488ded1..e21f84cbd9b4 100644 --- a/drivers/cpufreq/Kconfig.arm +++ b/drivers/cpufreq/Kconfig.arm @@ -205,6 +205,17 @@ config ARM_SA1100_CPUFREQ config ARM_SA1110_CPUFREQ bool +config ARM_SCMI_CPUFREQ + tristate "SCMI based CPUfreq driver" + depends on ARM_SCMI_PROTOCOL || COMPILE_TEST + select PM_OPP + help + This adds the CPUfreq driver support for ARM platforms using SCMI + protocol for CPU power management. + + This driver uses SCMI Message Protocol driver to interact with the + firmware providing the CPU DVFS functionality. + config ARM_SCPI_CPUFREQ tristate "SCPI based CPUfreq driver" depends on ARM_BIG_LITTLE_CPUFREQ && ARM_SCPI_PROTOCOL && COMMON_CLK_SCPI diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile index c7af9b2a255e..1206207f9b62 100644 --- a/drivers/cpufreq/Makefile +++ b/drivers/cpufreq/Makefile @@ -71,6 +71,7 @@ obj-$(CONFIG_ARM_S3C64XX_CPUFREQ) += s3c64xx-cpufreq.o obj-$(CONFIG_ARM_S5PV210_CPUFREQ) += s5pv210-cpufreq.o obj-$(CONFIG_ARM_SA1100_CPUFREQ) += sa1100-cpufreq.o obj-$(CONFIG_ARM_SA1110_CPUFREQ) += sa1110-cpufreq.o +obj-$(CONFIG_ARM_SCMI_CPUFREQ) += scmi-cpufreq.o obj-$(CONFIG_ARM_SCPI_CPUFREQ) += scpi-cpufreq.o obj-$(CONFIG_ARM_SPEAR_CPUFREQ)+= spear-cpufreq.o obj-$(CONFIG_ARM_STI_CPUFREQ) += sti-cpufreq.o diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c new file mode 100644 index ..b1057a13e6a7 --- /dev/null +++ b/drivers/cpufreq/scmi-cpufreq.c @@ -0,0 +1,274 @@ +/* + * System Control and Power Interface (SCMI) based CPUFreq Interface driver + * + * Copyright (C) 2017 ARM Ltd. + * Sudeep Holla + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct scmi_data { + int domain_id; + struct device *cpu_dev; + struct thermal_cooling_device *cdev; +}; + +static const struct scmi_handle *handle; + +unsigned int scmi_cpufreq_get_rate(unsigned int cpu) +{ + struct cpufreq_policy *policy = cpufreq_cpu_get_raw(cpu); + struct scmi_perf_ops *perf_ops = handle->perf_ops; + struct scmi_data *priv = policy->driver_data; + unsigned long rate; + int ret; + + ret = perf_ops->freq_get(handle, priv->domain_id, &rate, false); + if (ret) + return 0; + return rate / 1000; +} + +/* + * perf_ops->freq_set is not a synchronous, the actual OPP change will + * happen asynchronously and can get notified if the events are + * subscribed for by the SCMI firmware + */ +static int +scmi_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) +{ + struct scmi_data *priv = policy->driver_data; + struct scmi_perf_ops *perf_ops = handle->perf_ops; + u64 freq = policy->freq_table[index].frequency * 1000; + + return perf_ops->freq_set(handle, priv->domain_id, freq, false); +} + +static int +scmi_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpum
[PATCH v4 20/20] cpufreq: scmi: add support for fast frequency switching
The cpufreq core provides option for drivers to implement fast_switch callback which is invoked for frequency switching from interrupt context. This patch adds support for fast_switch callback in SCMI cpufreq driver by making use of polling based SCMI transfer. It also sets the flag fast_switch_possible. Cc: "Rafael J. Wysocki" Cc: Viresh Kumar Cc: linux...@vger.kernel.org Signed-off-by: Sudeep Holla --- drivers/cpufreq/scmi-cpufreq.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c index b1057a13e6a7..91df5013f7b2 100644 --- a/drivers/cpufreq/scmi-cpufreq.c +++ b/drivers/cpufreq/scmi-cpufreq.c @@ -64,6 +64,19 @@ scmi_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) return perf_ops->freq_set(handle, priv->domain_id, freq, false); } +static unsigned int scmi_cpufreq_fast_switch(struct cpufreq_policy *policy, +unsigned int target_freq) +{ + struct scmi_data *priv = policy->driver_data; + struct scmi_perf_ops *perf_ops = handle->perf_ops; + + if (!perf_ops->freq_set(handle, priv->domain_id, + target_freq * 1000, true)) + return target_freq; + + return 0; +} + static int scmi_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) { @@ -167,6 +180,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy) policy->cpuinfo.transition_latency = latency; + policy->fast_switch_possible = true; return 0; out_free_cpufreq_table: @@ -183,6 +197,7 @@ static int scmi_cpufreq_exit(struct cpufreq_policy *policy) { struct scmi_data *priv = policy->driver_data; + policy->fast_switch_possible = false; cpufreq_cooling_unregister(priv->cdev); dev_pm_opp_free_cpufreq_table(priv->cpu_dev, &policy->freq_table); kfree(priv); @@ -226,6 +241,7 @@ static struct cpufreq_driver scmi_cpufreq_driver = { .verify = cpufreq_generic_frequency_table_verify, .attr = cpufreq_generic_attr, .target_index = scmi_cpufreq_set_target, + .fast_switch= scmi_cpufreq_fast_switch, .get= scmi_cpufreq_get_rate, .init = scmi_cpufreq_init, .exit = scmi_cpufreq_exit, -- 2.7.4
[PATCH v4 18/20] hwmon: add support for sensors exported via ARM SCMI
Create a driver to add support for SoC sensors exported by the System Control Processor (SCP) via the System Control and Management Interface (SCMI). The supported sensor types is one of voltage, temperature, current, and power. The sensor labels and values provided by the SCP are exported via the hwmon sysfs interface. Cc: linux-hw...@vger.kernel.org Acked-by: Guenter Roeck Signed-off-by: Sudeep Holla --- drivers/hwmon/Kconfig | 12 +++ drivers/hwmon/Makefile | 1 + drivers/hwmon/scmi-hwmon.c | 233 + 3 files changed, 246 insertions(+) create mode 100644 drivers/hwmon/scmi-hwmon.c diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index d65431417b17..0b75e9a89463 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -321,6 +321,18 @@ config SENSORS_APPLESMC Say Y here if you have an applicable laptop and want to experience the awesome power of applesmc. +config SENSORS_ARM_SCMI + tristate "ARM SCMI Sensors" + depends on ARM_SCMI_PROTOCOL + depends on THERMAL || !THERMAL_OF + help + This driver provides support for temperature, voltage, current + and power sensors available on SCMI based platforms. The actual + number and type of sensors exported depend on the platform. + + This driver can also be built as a module. If so, the module + will be called scmi-hwmon. + config SENSORS_ARM_SCPI tristate "ARM SCPI Sensors" depends on ARM_SCPI_PROTOCOL diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index c84d9784be98..a51c2dcef11c 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -44,6 +44,7 @@ obj-$(CONFIG_SENSORS_ADT7462) += adt7462.o obj-$(CONFIG_SENSORS_ADT7470) += adt7470.o obj-$(CONFIG_SENSORS_ADT7475) += adt7475.o obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o +obj-$(CONFIG_SENSORS_ARM_SCMI) += scmi-hwmon.o obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o obj-$(CONFIG_SENSORS_ASPEED) += aspeed-pwm-tacho.o diff --git a/drivers/hwmon/scmi-hwmon.c b/drivers/hwmon/scmi-hwmon.c new file mode 100644 index ..7a0ab768d4b5 --- /dev/null +++ b/drivers/hwmon/scmi-hwmon.c @@ -0,0 +1,233 @@ +/* + * System Control and Management Interface(SCMI) based hwmon sensor driver + * + * Copyright (C) 2017 ARM Ltd. + * Sudeep Holla + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed "as is" WITHOUT ANY WARRANTY of any + * kind, whether express or implied; without even the implied warranty + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include + +struct scmi_sensors { + const struct scmi_handle *handle; + const struct scmi_sensor_info **info[hwmon_max]; +}; + +static int scmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + int ret; + u64 value; + const struct scmi_sensor_info *sensor; + struct scmi_sensors *scmi_sensors = dev_get_drvdata(dev); + const struct scmi_handle *h = scmi_sensors->handle; + + sensor = *(scmi_sensors->info[type] + channel); + ret = h->sensor_ops->reading_get(h, sensor->id, false, &value); + if (!ret) + *val = value; + + return ret; +} + +static int +scmi_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, const char **str) +{ + const struct scmi_sensor_info *sensor; + struct scmi_sensors *scmi_sensors = dev_get_drvdata(dev); + + sensor = *(scmi_sensors->info[type] + channel); + *str = sensor->name; + + return 0; +} + +static umode_t +scmi_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + const struct scmi_sensor_info *sensor; + const struct scmi_sensors *scmi_sensors = drvdata; + + sensor = *(scmi_sensors->info[type] + channel); + if (sensor && sensor->name) + return S_IRUGO; + + return 0; +} + +static const struct hwmon_ops scmi_hwmon_ops = { + .is_visible = scmi_hwmon_is_visible, + .read = scmi_hwmon_read, + .read_string = scmi_hwmon_read_string, +}; + +static struct hwmon_chip_info scmi_chip_info = { + .ops = &scmi_hwmon_ops, + .info = NULL, +}; + +static int scmi_hwmon_add_chan_info(struct hwmon_channel_info *scmi_hwmon_chan, + struct device *dev, int num, + enum hwmon_sensor_types type, u32 confi
[PATCH v4 16/20] clk: add support for clocks provided by SCMI
On some ARM based systems, a separate Cortex-M based System Control Processor(SCP) provides the overall power, clock, reset and system control. System Control and Management Interface(SCMI) Message Protocol is defined for the communication between the Application Cores(AP) and the SCP. This patch adds support for the clocks provided by SCP using SCMI protocol. Cc: linux-...@vger.kernel.org Cc: Michael Turquette Acked-by: Stephen Boyd Signed-off-by: Sudeep Holla --- MAINTAINERS| 2 +- drivers/clk/Kconfig| 10 +++ drivers/clk/Makefile | 1 + drivers/clk/clk-scmi.c | 213 + 4 files changed, 225 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/clk-scmi.c diff --git a/MAINTAINERS b/MAINTAINERS index 8ee63627f1ac..1f9004d68ad4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12954,7 +12954,7 @@ M: Sudeep Holla L: linux-arm-ker...@lists.infradead.org S: Maintained F: Documentation/devicetree/bindings/arm/arm,sc[mp]i.txt -F: drivers/clk/clk-scpi.c +F: drivers/clk/clk-sc[mp]i.c F: drivers/cpufreq/scpi-cpufreq.c F: drivers/firmware/arm_scpi.c F: drivers/firmware/arm_scmi/ diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 1c4e1aa6767e..57c66b22eab8 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -62,6 +62,16 @@ config COMMON_CLK_HI655X multi-function device has one fixed-rate oscillator, clocked at 32KHz. +config COMMON_CLK_SCMI + tristate "Clock driver controlled via SCMI interface" + depends on ARM_SCMI_PROTOCOL || COMPILE_TEST + ---help--- + This driver provides support for clocks that are controlled + by firmware that implements the SCMI interface. + + This driver uses SCMI Message Protocol to interact with the + firmware providing all the clock controls. + config COMMON_CLK_SCPI tristate "Clock driver controlled via SCPI interface" depends on ARM_SCPI_PROTOCOL || COMPILE_TEST diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index c99f363826f0..46ad2f2b686a 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -39,6 +39,7 @@ obj-$(CONFIG_CLK_QORIQ) += clk-qoriq.o obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o obj-$(CONFIG_COMMON_CLK_HI655X)+= clk-hi655x.o obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o +obj-$(CONFIG_COMMON_CLK_SCMI) += clk-scmi.o obj-$(CONFIG_COMMON_CLK_SCPI) += clk-scpi.o obj-$(CONFIG_COMMON_CLK_SI5351)+= clk-si5351.o obj-$(CONFIG_COMMON_CLK_SI514) += clk-si514.o diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c new file mode 100644 index ..1e4d7a57779b --- /dev/null +++ b/drivers/clk/clk-scmi.c @@ -0,0 +1,213 @@ +/* + * System Control and Power Interface (SCMI) Protocol based clock driver + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include +#include +#include +#include +#include +#include +#include + +struct scmi_clk { + u32 id; + struct clk_hw hw; + const struct scmi_clock_info *info; + const struct scmi_handle *handle; +}; + +#define to_scmi_clk(clk) container_of(clk, struct scmi_clk, hw) + +static unsigned long scmi_clk_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + int ret; + u64 rate; + struct scmi_clk *clk = to_scmi_clk(hw); + + ret = clk->handle->clk_ops->rate_get(clk->handle, clk->id, &rate); + if (ret) + return 0; + return rate; +} + +static long scmi_clk_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + int step; + u64 fmin, fmax, ftmp; + struct scmi_clk *clk = to_scmi_clk(hw); + + /* +* We can't figure out what rate it will be, so just return the +* rate back to the caller. scmi_clk_recalc_rate() will be called +* after the rate is set and we'll know what rate the clock is +* running at then. +*/ + if (clk->info->rate_discrete) + return rate; + + fmin = clk->info->range.min_rate; + fmax = clk->info->range.max_rate; + if
[PATCH v4 14/20] firmware: arm_scmi: add per-protocol channels support using idr objects
In order to maintain the channel information per protocol, we need some sort of list or hashtable to hold all this information. IDR provides sparse array mapping of small integer ID numbers onto arbitrary pointers. In this case the arbitrary pointers can be pointers to the channel information. This patch adds support for per-protocol channels using those idr objects. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 54 +- 1 file changed, 42 insertions(+), 12 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 24acb421208c..6734a035bcc6 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -118,6 +118,7 @@ struct scmi_chan_info { struct mbox_chan *chan; void __iomem *payload; struct device *dev; + struct scmi_handle *handle; }; /** @@ -129,7 +130,7 @@ struct scmi_chan_info { * @version: SCMI revision information containing protocol version, * implementation version and (sub-)vendor identification. * @minfo: Message info - * @tx_cinfo: Reference to SCMI channel information + * @tx_idr: IDR object to map protocol id to channel info pointer * @protocols_imp: list of protocols implemented, currently maximum of * MAX_PROTOCOLS_IMP elements allocated by the base protocol * @node: list head @@ -141,7 +142,7 @@ struct scmi_info { struct scmi_revision_info version; struct scmi_handle handle; struct scmi_xfers_info minfo; - struct scmi_chan_info *tx_cinfo; + struct idr tx_idr; u8 *protocols_imp; struct list_head node; int users; @@ -232,7 +233,7 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m) struct scmi_xfer *xfer; struct scmi_chan_info *cinfo = client_to_scmi_chan_info(cl); struct device *dev = cinfo->dev; - struct scmi_info *info = dev_get_drvdata(dev); + struct scmi_info *info = handle_to_scmi_info(cinfo->handle); struct scmi_xfers_info *minfo = &info->minfo; struct scmi_shared_mem __iomem *mem = cinfo->payload; @@ -409,7 +410,11 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) int timeout; struct scmi_info *info = handle_to_scmi_info(handle); struct device *dev = info->dev; - struct scmi_chan_info *cinfo = info->tx_cinfo; + struct scmi_chan_info *cinfo; + + cinfo = idr_find(&info->tx_idr, xfer->hdr.protocol_id); + if (unlikely(!cinfo)) + return -EINVAL; ret = mbox_send_message(cinfo->chan, xfer); if (ret < 0) { @@ -681,13 +686,18 @@ static int scmi_mailbox_check(struct device_node *np) return of_parse_phandle_with_args(np, "mboxes", "#mbox-cells", 0, &arg); } -static int scmi_mbox_free_channel(struct scmi_chan_info *cinfo) +static int scmi_mbox_free_channel(int id, void *p, void *data) { + struct scmi_chan_info *cinfo = p; + struct idr *idr = data; + if (!IS_ERR_OR_NULL(cinfo->chan)) { mbox_free_channel(cinfo->chan); cinfo->chan = NULL; } + idr_remove(idr, id); + return 0; } @@ -695,6 +705,7 @@ static int scmi_remove(struct platform_device *pdev) { int ret = 0; struct scmi_info *info = platform_get_drvdata(pdev); + struct idr *idr = &info->tx_idr; mutex_lock(&scmi_list_mutex); if (info->users) @@ -703,28 +714,34 @@ static int scmi_remove(struct platform_device *pdev) list_del(&info->node); mutex_unlock(&scmi_list_mutex); - if (!ret) + if (!ret) { /* Safe to free channels since no more users */ - return scmi_mbox_free_channel(info->tx_cinfo); + ret = idr_for_each(idr, scmi_mbox_free_channel, idr); + idr_destroy(&info->tx_idr); + } return ret; } -static inline int scmi_mbox_chan_setup(struct scmi_info *info) +static inline int +scmi_mbox_chan_setup(struct scmi_info *info, struct device *dev, int prot_id) { int ret; struct resource res; resource_size_t size; - struct device *dev = info->dev; struct device_node *shmem, *np = dev->of_node; struct scmi_chan_info *cinfo; struct mbox_client *cl; + if (scmi_mailbox_check(np)) { + cinfo = idr_find(&info->tx_idr, SCMI_PROTOCOL_BASE); + goto idr_alloc; + } + cinfo = devm_kzalloc(info->dev, sizeof(*cinfo), GFP_KERNEL); if (!cinfo) return -ENOMEM; - info->tx_cinfo = cinfo; cinfo->dev = dev; cl = &cinfo->cl; @@ -758,6 +775,14 @@ static inline int scmi_mbox_chan_setup(struct scmi_info *i
[PATCH v4 11/20] firmware: arm_scmi: add support for polling based SCMI transfers
It would be useful to have options to perform some SCMI transfers atomically by polling for the completion flag instead of interrupt driven. The SCMI specification has option to disable the interrupt and poll for the completion flag in the shared memory. This patch adds support for polling based SCMI transfers using that option. This might be used for uninterrupted/atomic DVFS operations from the scheduler context. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 49 +++--- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 11c18ac9816f..0c9dda72f10c 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -26,9 +26,11 @@ */ #include +#include #include #include #include +#include #include #include #include @@ -363,6 +365,21 @@ void scmi_one_xfer_put(const struct scmi_handle *handle, struct scmi_xfer *xfer) up(&minfo->sem_xfer_count); } +static bool +scmi_xfer_poll_done(const struct scmi_info *info, struct scmi_xfer *xfer) +{ + struct scmi_shared_mem *mem = info->tx_payload; + u16 xfer_id = MSG_XTRACT_TOKEN(le32_to_cpu(mem->msg_header)); + + if (xfer->hdr.seq != xfer_id) + return false; + + return le32_to_cpu(mem->channel_status) & + (SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR | + SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE); +} + +#define SCMI_MAX_POLLING_TIMEOUT_NS(100 * NSEC_PER_USEC) /** * scmi_do_xfer() - Do one transfer * @@ -389,14 +406,30 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) /* mbox_send_message returns non-negative value on success, so reset */ ret = 0; - /* And we wait for the response. */ - timeout = msecs_to_jiffies(info->desc->max_rx_timeout_ms); - if (!wait_for_completion_timeout(&xfer->done, timeout)) { - dev_err(dev, "mbox timed out in resp(caller: %pF)\n", - (void *)_RET_IP_); - ret = -ETIMEDOUT; - } else if (xfer->hdr.status) { - ret = scmi_to_linux_errno(xfer->hdr.status); + if (xfer->hdr.poll_completion) { + ktime_t stop, cur; + + stop = ktime_add_ns(ktime_get(), SCMI_MAX_POLLING_TIMEOUT_NS); + do { + udelay(5); + cur = ktime_get(); + } while (!scmi_xfer_poll_done(info, xfer) && +ktime_before(cur, stop)); + + if (ktime_before(cur, stop)) + scmi_fetch_response(xfer, info->tx_payload); + else + ret = -ETIMEDOUT; + } else { + /* And we wait for the response. */ + timeout = msecs_to_jiffies(info->desc->max_rx_timeout_ms); + if (!wait_for_completion_timeout(&xfer->done, timeout)) { + dev_err(dev, "mbox timed out in resp(caller: %pF)\n", + (void *)_RET_IP_); + ret = -ETIMEDOUT; + } else if (xfer->hdr.status) { + ret = scmi_to_linux_errno(xfer->hdr.status); + } } /* * NOTE: we might prefer not to need the mailbox ticker to manage the -- 2.7.4
[PATCH v4 12/20] firmware: arm_scmi: add option for polling based performance domain operations
In order to implement fast CPU DVFS switching, we need to perform all DVFS operations atomically. Since SCMI transfer already provide option to choose between pooling vs interrupt driven(default), we can opt for polling based transfers for set,get performance domain operations. This patch adds option to choose between polling vs interrupt driven SCMI transfers for set,get performance level operations. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/perf.c | 19 +++ include/linux/scmi_protocol.h| 8 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index a1f5cf136748..ad73ef6b8d7d 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -303,8 +303,8 @@ static int scmi_perf_limits_get(const struct scmi_handle *handle, u32 domain, return ret; } -static int -scmi_perf_level_set(const struct scmi_handle *handle, u32 domain, u32 level) +static int scmi_perf_level_set(const struct scmi_handle *handle, u32 domain, + u32 level, bool poll) { int ret; struct scmi_xfer *t; @@ -315,6 +315,7 @@ scmi_perf_level_set(const struct scmi_handle *handle, u32 domain, u32 level) if (ret) return ret; + t->hdr.poll_completion = poll; lvl = t->tx.buf; lvl->domain = cpu_to_le32(domain); lvl->level = cpu_to_le32(level); @@ -325,8 +326,8 @@ scmi_perf_level_set(const struct scmi_handle *handle, u32 domain, u32 level) return ret; } -static int -scmi_perf_level_get(const struct scmi_handle *handle, u32 domain, u32 *level) +static int scmi_perf_level_get(const struct scmi_handle *handle, u32 domain, + u32 *level, bool poll) { int ret; struct scmi_xfer *t; @@ -336,6 +337,7 @@ scmi_perf_level_get(const struct scmi_handle *handle, u32 domain, u32 *level) if (ret) return ret; + t->hdr.poll_completion = poll; *(__le32 *)t->tx.buf = cpu_to_le32(domain); ret = scmi_do_xfer(handle, t); @@ -447,23 +449,24 @@ static int scmi_dvfs_get_transition_latency(const struct scmi_handle *handle, } static int scmi_dvfs_freq_set(const struct scmi_handle *handle, u32 domain, - unsigned long freq) + unsigned long freq, bool poll) { struct scmi_perf_info *pi = handle->perf_priv; struct perf_dom_info *dom = pi->dom_info + domain; - return scmi_perf_level_set(handle, domain, freq / dom->mult_factor); + return scmi_perf_level_set(handle, domain, freq / dom->mult_factor, + poll); } static int scmi_dvfs_freq_get(const struct scmi_handle *handle, u32 domain, - unsigned long *freq) + unsigned long *freq, bool poll) { int ret; u32 level; struct scmi_perf_info *pi = handle->perf_priv; struct perf_dom_info *dom = pi->dom_info + domain; - ret = scmi_perf_level_get(handle, domain, &level); + ret = scmi_perf_level_get(handle, domain, &level, poll); if (!ret) *freq = level * dom->mult_factor; diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index 70e2993f6d48..3b378af1badd 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -100,16 +100,16 @@ struct scmi_clk_ops { struct scmi_perf_ops { int (*limits_set)(const struct scmi_handle *, u32, u32, u32); int (*limits_get)(const struct scmi_handle *, u32, u32 *, u32 *); - int (*level_set)(const struct scmi_handle *, u32, u32); - int (*level_get)(const struct scmi_handle *, u32, u32 *); + int (*level_set)(const struct scmi_handle *, u32, u32, bool); + int (*level_get)(const struct scmi_handle *, u32, u32 *, bool); int (*limits_notify_enable)(const struct scmi_handle *, u32, bool); int (*level_notify_enable)(const struct scmi_handle *, u32, bool); int (*device_domain_id)(struct device *); int (*get_transition_latency)(const struct scmi_handle *, struct device *); int (*add_opps_to_device)(const struct scmi_handle *, struct device *); - int (*freq_set)(const struct scmi_handle *, u32, unsigned long); - int (*freq_get)(const struct scmi_handle *, u32, unsigned long *); + int (*freq_set)(const struct scmi_handle *, u32, unsigned long, bool); + int (*freq_get)(const struct scmi_handle *, u32, unsigned long *, bool); }; /** -- 2.7.4
[PATCH v4 13/20] firmware: arm_scmi: refactor in preparation to support per-protocol channels
In order to support per-protocol channels if available, we need to factor out all the mailbox channel information(Tx/Rx payload and channel handle) out of the main SCMI instance information structure. This patch refactors the existing channel information into a separate chan_info structure. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 86 -- 1 file changed, 54 insertions(+), 32 deletions(-) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 0c9dda72f10c..24acb421208c 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -105,6 +105,22 @@ struct scmi_desc { }; /** + * struct scmi_chan_info - Structure representing a SCMI channel informfation + * + * @cl: Mailbox Client + * @chan: Transmit/Receive mailbox channel + * @payload: Transmit/Receive mailbox channel payload area + * @dev: Reference to device in the SCMI hierarchy corresponding to this + * channel + */ +struct scmi_chan_info { + struct mbox_client cl; + struct mbox_chan *chan; + void __iomem *payload; + struct device *dev; +}; + +/** * struct scmi_info - Structure representing a SCMI instance * * @dev: Device pointer @@ -112,10 +128,8 @@ struct scmi_desc { * @handle: Instance of SCMI handle to send to clients * @version: SCMI revision information containing protocol version, * implementation version and (sub-)vendor identification. - * @cl: Mailbox Client - * @tx_chan: Transmit mailbox channel - * @tx_payload: Transmit mailbox channel payload area * @minfo: Message info + * @tx_cinfo: Reference to SCMI channel information * @protocols_imp: list of protocols implemented, currently maximum of * MAX_PROTOCOLS_IMP elements allocated by the base protocol * @node: list head @@ -126,16 +140,14 @@ struct scmi_info { const struct scmi_desc *desc; struct scmi_revision_info version; struct scmi_handle handle; - struct mbox_client cl; - struct mbox_chan *tx_chan; - void __iomem *tx_payload; struct scmi_xfers_info minfo; + struct scmi_chan_info *tx_cinfo; u8 *protocols_imp; struct list_head node; int users; }; -#define client_to_scmi_info(c) container_of(c, struct scmi_info, cl) +#define client_to_scmi_chan_info(c) container_of(c, struct scmi_chan_info, cl) #define handle_to_scmi_info(h) container_of(h, struct scmi_info, handle) /* @@ -218,10 +230,11 @@ static void scmi_rx_callback(struct mbox_client *cl, void *m) { u16 xfer_id; struct scmi_xfer *xfer; - struct scmi_info *info = client_to_scmi_info(cl); + struct scmi_chan_info *cinfo = client_to_scmi_chan_info(cl); + struct device *dev = cinfo->dev; + struct scmi_info *info = dev_get_drvdata(dev); struct scmi_xfers_info *minfo = &info->minfo; - struct device *dev = info->dev; - struct scmi_shared_mem __iomem *mem = info->tx_payload; + struct scmi_shared_mem __iomem *mem = cinfo->payload; xfer_id = MSG_XTRACT_TOKEN(ioread32(&mem->msg_header)); @@ -272,8 +285,8 @@ static inline u32 pack_scmi_header(struct scmi_msg_hdr *hdr) static void scmi_tx_prepare(struct mbox_client *cl, void *m) { struct scmi_xfer *t = m; - struct scmi_info *info = client_to_scmi_info(cl); - struct scmi_shared_mem __iomem *mem = info->tx_payload; + struct scmi_chan_info *cinfo = client_to_scmi_chan_info(cl); + struct scmi_shared_mem __iomem *mem = cinfo->payload; /* Mark channel busy + clear error */ iowrite32(0x0, &mem->channel_status); @@ -366,15 +379,15 @@ void scmi_one_xfer_put(const struct scmi_handle *handle, struct scmi_xfer *xfer) } static bool -scmi_xfer_poll_done(const struct scmi_info *info, struct scmi_xfer *xfer) +scmi_xfer_poll_done(const struct scmi_chan_info *cinfo, struct scmi_xfer *xfer) { - struct scmi_shared_mem *mem = info->tx_payload; - u16 xfer_id = MSG_XTRACT_TOKEN(le32_to_cpu(mem->msg_header)); + struct scmi_shared_mem __iomem *mem = cinfo->payload; + u16 xfer_id = MSG_XTRACT_TOKEN(ioread32(&mem->msg_header)); if (xfer->hdr.seq != xfer_id) return false; - return le32_to_cpu(mem->channel_status) & + return ioread32(&mem->channel_status) & (SCMI_SHMEM_CHAN_STAT_CHANNEL_ERROR | SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE); } @@ -396,8 +409,9 @@ int scmi_do_xfer(const struct scmi_handle *handle, struct scmi_xfer *xfer) int timeout; struct scmi_info *info = handle_to_scmi_info(handle); struct device *dev = info->dev; + struct scmi_chan_info *cinfo = info->tx_cinfo; - ret = mbox_send_message(info->tx_chan, xfer); + ret = mbox_send_message(cinfo->cha
[PATCH v4 08/20] firmware: arm_scmi: add initial support for power protocol
The power protocol is intended for management of power states of various power domains. The power domain management protocol provides commands to describe the protocol version, discover the implementation specific attributes, set and get the power state of a domain. This patch adds support for the above mention features of the protocol. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla -- drivers/firmware/arm_scmi/Makefile | 2 +- drivers/firmware/arm_scmi/power.c | 242 + include/linux/scmi_protocol.h | 28 + 3 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/power.c --- drivers/firmware/arm_scmi/Makefile | 2 +- drivers/firmware/arm_scmi/power.c | 255 + include/linux/scmi_protocol.h | 29 + 3 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/power.c diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index 2130ee9ac825..420c761ced94 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -1,4 +1,4 @@ obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o scmi-bus-y = bus.o scmi-driver-y = driver.o -scmi-protocols-y = base.o clock.o perf.o +scmi-protocols-y = base.o clock.o perf.o power.o diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c new file mode 100644 index ..50195bd6bbd2 --- /dev/null +++ b/drivers/firmware/arm_scmi/power.c @@ -0,0 +1,255 @@ +/* + * System Control and Management Interface (SCMI) Power Protocol + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "common.h" + +enum scmi_power_protocol_cmd { + POWER_DOMAIN_ATTRIBUTES = 0x3, + POWER_STATE_SET = 0x4, + POWER_STATE_GET = 0x5, + POWER_STATE_NOTIFY = 0x6, +}; + +struct scmi_msg_resp_power_attributes { + __le16 num_domains; + __le16 reserved; + __le32 stats_addr_low; + __le32 stats_addr_high; + __le32 stats_size; +}; + +struct scmi_msg_resp_power_domain_attributes { + __le32 flags; +#define SUPPORTS_STATE_SET_NOTIFY(x) ((x) & BIT(31)) +#define SUPPORTS_STATE_SET_ASYNC(x)((x) & BIT(30)) +#define SUPPORTS_STATE_SET_SYNC(x) ((x) & BIT(29)) + u8 name[SCMI_MAX_STR_SIZE]; +}; + +struct scmi_power_set_state { + __le32 flags; +#define STATE_SET_ASYNCBIT(0) + __le32 domain; + __le32 state; +}; + +struct scmi_power_state_notify { + __le32 domain; + __le32 notify_enable; +}; + +struct power_dom_info { + bool state_set_sync; + bool state_set_async; + bool state_set_notify; + char name[SCMI_MAX_STR_SIZE]; +}; + +struct scmi_power_info { + int num_domains; + u64 stats_addr; + u32 stats_size; + struct power_dom_info *dom_info; +}; + +static int scmi_power_attributes_get(const struct scmi_handle *handle, +struct scmi_power_info *pi) +{ + int ret; + struct scmi_xfer *t; + struct scmi_msg_resp_power_attributes *attr; + + ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, +SCMI_PROTOCOL_POWER, 0, sizeof(*attr), &t); + if (ret) + return ret; + + attr = t->rx.buf; + + ret = scmi_do_xfer(handle, t); + if (!ret) { + pi->num_domains = le16_to_cpu(attr->num_domains); + pi->stats_addr = le32_to_cpu(attr->stats_addr_low) | + (u64)le32_to_cpu(attr->stats_addr_high) << 32; + pi->stats_size = le32_to_cpu(attr->stats_size); + } + + scmi_one_xfer_put(handle, t); + return ret; +} + +static int +scmi_power_domain_attributes_get(const struct scmi_handle *handle, u32 domain, +struct power_dom_info *dom_info) +{ + int ret; + struct scmi_xfer *t; + struct scmi_msg_resp_power_domain_attributes *attr; + + ret = scmi_one_xfer_init(handle, POWER_DOMAIN_ATTRIBUTES, +SCMI_PROTOCOL_POWER, sizeof(domain), +sizeof(*attr), &t); + if (ret) + return ret; + + *(__le32 *)t->tx.buf = cpu_to
[PATCH v4 09/20] firmware: arm_scmi: add initial support for sensor protocol
The sensor protocol provides functions to manage platform sensors, and provides the commands to describe the protocol version and the various attribute flags. It also provides commands to discover various sensors implemented and managed by the platform, read any sensor synchronously or asynchronously as allowed by the platform, program sensor attributes and/or configurations, if applicable. This patch adds support for most of the above features. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/Makefile | 2 +- drivers/firmware/arm_scmi/sensors.c | 302 include/linux/scmi_protocol.h | 42 + 3 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/sensors.c diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index 420c761ced94..3236890905b9 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -1,4 +1,4 @@ obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o scmi-bus-y = bus.o scmi-driver-y = driver.o -scmi-protocols-y = base.o clock.o perf.o power.o +scmi-protocols-y = base.o clock.o perf.o power.o sensors.o diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c new file mode 100644 index ..aee38d3d1c2d --- /dev/null +++ b/drivers/firmware/arm_scmi/sensors.c @@ -0,0 +1,302 @@ +/* + * System Control and Management Interface (SCMI) Sensor Protocol + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "common.h" + +enum scmi_sensor_protocol_cmd { + SENSOR_DESCRIPTION_GET = 0x3, + SENSOR_CONFIG_SET = 0x4, + SENSOR_TRIP_POINT_SET = 0x5, + SENSOR_READING_GET = 0x6, +}; + +struct scmi_msg_resp_sensor_attributes { + __le16 num_sensors; + u8 max_requests; + u8 reserved; + __le32 reg_addr_low; + __le32 reg_addr_high; + __le32 reg_size; +}; + +struct scmi_msg_resp_sensor_description { + __le16 num_returned; + __le16 num_remaining; + struct { + __le32 id; + __le32 attributes_low; +#define SUPPORTS_ASYNC_READ(x) ((x) & BIT(31)) +#define NUM_TRIP_POINTS(x) (((x) >> 4) & 0xff) + __le32 attributes_high; +#define SENSOR_TYPE(x) ((x) & 0xff) +#define SENSOR_SCALE(x)(((x) >> 11) & 0x3f) +#define SENSOR_UPDATE_SCALE(x) (((x) >> 22) & 0x1f) +#define SENSOR_UPDATE_BASE(x) (((x) >> 27) & 0x1f) + u8 name[SCMI_MAX_STR_SIZE]; + } desc[0]; +}; + +struct scmi_msg_set_sensor_config { + __le32 id; + __le32 event_control; +}; + +struct scmi_msg_set_sensor_trip_point { + __le32 id; + __le32 event_control; +#define SENSOR_TP_EVENT_MASK (0x3) +#define SENSOR_TP_DISABLED 0x0 +#define SENSOR_TP_POSITIVE 0x1 +#define SENSOR_TP_NEGATIVE 0x2 +#define SENSOR_TP_BOTH 0x3 +#define SENSOR_TP_ID(x)(((x) & 0xff) << 4) + __le32 value_low; + __le32 value_high; +}; + +struct scmi_msg_sensor_reading_get { + __le32 id; + __le32 flags; +#define SENSOR_READ_ASYNC BIT(0) +}; + +struct sensors_info { + int num_sensors; + int max_requests; + u64 reg_addr; + u32 reg_size; + struct scmi_sensor_info *sensors; +}; + +static int scmi_sensor_attributes_get(const struct scmi_handle *handle, + struct sensors_info *si) +{ + int ret; + struct scmi_xfer *t; + struct scmi_msg_resp_sensor_attributes *attr; + + ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, +SCMI_PROTOCOL_SENSOR, 0, sizeof(*attr), &t); + if (ret) + return ret; + + attr = t->rx.buf; + + ret = scmi_do_xfer(handle, t); + if (!ret) { + si->num_sensors = le16_to_cpu(attr->num_sensors); + si->max_requests = attr->max_requests; + si->reg_addr = le32_to_cpu(attr->reg_addr_low) | + (u64)le32_to_cpu(attr->reg_addr_high) << 32; + si->reg_size = le32_to_cpu(attr->reg_size); + } + + scmi_one_xfer_put(handle, t); + return ret; +} + +stat
[PATCH v4 06/20] firmware: arm_scmi: add initial support for performance protocol
The performance protocol is intended for the performance management of group(s) of device(s) that run in the same performance domain. It includes even the CPUs. A performance domain is defined by a set of devices that always have to run at the same performance level. For example, a set of CPUs that share a voltage domain, and have a common frequency control, is said to be in the same performance domain. The commands in this protocol provide functionality to describe the protocol version, describe various attribute flags, set and get the performance level of a domain. It also supports discovery of the list of performance levels supported by a performance domain, and the properties of each performance level. This patch adds basic support for the performance protocol. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/Makefile | 2 +- drivers/firmware/arm_scmi/common.h | 1 + drivers/firmware/arm_scmi/perf.c | 527 + include/linux/scmi_protocol.h | 34 +++ 4 files changed, 563 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/perf.c diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index 5f4ec2613db6..687cbbfb3af6 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -1,4 +1,4 @@ obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o scmi-bus-y = bus.o scmi-driver-y = driver.o -scmi-protocols-y = base.o +scmi-protocols-y = base.o perf.o diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index f1eeacaef57b..b045a3f268e7 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -30,6 +30,7 @@ #define PROTOCOL_REV_MAJOR(x) ((x) >> PROTOCOL_REV_MINOR_BITS) #define PROTOCOL_REV_MINOR(x) ((x) & PROTOCOL_REV_MINOR_MASK) #define MAX_PROTOCOLS_IMP 16 +#define MAX_OPPS 16 enum scmi_common_cmd { PROTOCOL_VERSION = 0x0, diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c new file mode 100644 index ..a1f5cf136748 --- /dev/null +++ b/drivers/firmware/arm_scmi/perf.c @@ -0,0 +1,527 @@ +/* + * System Control and Management Interface (SCMI) Performance Protocol + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include +#include +#include +#include + +#include "common.h" + +enum scmi_performance_protocol_cmd { + PERF_DOMAIN_ATTRIBUTES = 0x3, + PERF_DESCRIBE_LEVELS = 0x4, + PERF_LIMITS_SET = 0x5, + PERF_LIMITS_GET = 0x6, + PERF_LEVEL_SET = 0x7, + PERF_LEVEL_GET = 0x8, + PERF_NOTIFY_LIMITS = 0x9, + PERF_NOTIFY_LEVEL = 0xa, +}; + +struct scmi_opp { + u32 perf; + u32 power; + u32 trans_latency_us; +}; + +struct scmi_msg_resp_perf_attributes { + __le16 num_domains; + __le16 flags; +#define POWER_SCALE_IN_MILLIWATT(x)((x) & BIT(0)) + __le32 stats_addr_low; + __le32 stats_addr_high; + __le32 stats_size; +}; + +struct scmi_msg_resp_perf_domain_attributes { + __le32 flags; +#define SUPPORTS_SET_LIMITS(x) ((x) & BIT(31)) +#define SUPPORTS_SET_PERF_LVL(x) ((x) & BIT(30)) +#define SUPPORTS_PERF_LIMIT_NOTIFY(x) ((x) & BIT(29)) +#define SUPPORTS_PERF_LEVEL_NOTIFY(x) ((x) & BIT(28)) + __le32 rate_limit_us; + __le32 sustained_freq_khz; + __le32 sustained_perf_level; + u8 name[SCMI_MAX_STR_SIZE]; +}; + +struct scmi_msg_perf_describe_levels { + __le32 domain; + __le32 level_index; +}; + +struct scmi_perf_set_limits { + __le32 domain; + __le32 max_level; + __le32 min_level; +}; + +struct scmi_perf_get_limits { + __le32 max_level; + __le32 min_level; +}; + +struct scmi_perf_set_level { + __le32 domain; + __le32 level; +}; + +struct scmi_perf_notify_level_or_limits { + __le32 domain; + __le32 notify_enable; +}; + +struct scmi_msg_resp_perf_describe_levels { + __le16 num_returned; + __le16 num_remaining; + struct { + __le32 perf_val; + __le32 power; + __le16 transition_latency_us; + __le16 reserved; + } opp[0]; +}; + +struct perf_dom_info { + bool set_limits; + bool set_perf
[PATCH v4 07/20] firmware: arm_scmi: add initial support for clock protocol
The clock protocol is intended for management of clocks. It is used to enable or disable clocks, and to set and get the clock rates. This protocol provides commands to describe the protocol version, discover various implementation specific attributes, describe a clock, enable and disable a clock and get/set the rate of the clock synchronously or asynchronously. This patch adds initial support for the clock protocol. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/Makefile | 2 +- drivers/firmware/arm_scmi/clock.c | 353 + include/linux/scmi_protocol.h | 41 + 3 files changed, 395 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/clock.c diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index 687cbbfb3af6..2130ee9ac825 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -1,4 +1,4 @@ obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o scmi-bus-y = bus.o scmi-driver-y = driver.o -scmi-protocols-y = base.o perf.o +scmi-protocols-y = base.o clock.o perf.o diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c new file mode 100644 index ..0fac1d4b377b --- /dev/null +++ b/drivers/firmware/arm_scmi/clock.c @@ -0,0 +1,353 @@ +/* + * System Control and Management Interface (SCMI) Clock Protocol + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "common.h" + +enum scmi_clock_protocol_cmd { + CLOCK_ATTRIBUTES = 0x3, + CLOCK_DESCRIBE_RATES = 0x4, + CLOCK_RATE_SET = 0x5, + CLOCK_RATE_GET = 0x6, + CLOCK_CONFIG_SET = 0x7, +}; + +struct scmi_msg_resp_clock_protocol_attributes { + __le16 num_clocks; + u8 max_async_req; + u8 reserved; +}; + +struct scmi_msg_resp_clock_attributes { + __le32 attributes; +#defineCLOCK_ENABLEBIT(0) + u8 name[SCMI_MAX_STR_SIZE]; +}; + +struct scmi_clock_set_config { + __le32 id; + __le32 attributes; +}; + +struct scmi_msg_clock_describe_rates { + __le32 id; + __le32 rate_index; +}; + +struct scmi_msg_resp_clock_describe_rates { + __le32 num_rates_flags; +#define NUM_RETURNED(x)((x) & 0xfff) +#define RATE_DISCRETE(x) !((x) & BIT(12)) +#define NUM_REMAINING(x) ((x) >> 16) + struct { + __le32 value_low; + __le32 value_high; + } rate[0]; +#define RATE_TO_U64(X) \ +({ \ + typeof(X) x = (X); \ + le32_to_cpu((x).value_low) | (u64)le32_to_cpu((x).value_high) << 32; \ +}) +}; + +struct scmi_clock_set_rate { + __le32 flags; +#define CLOCK_SET_ASYNCBIT(0) +#define CLOCK_SET_DELAYED BIT(1) +#define CLOCK_SET_ROUND_UP BIT(2) +#define CLOCK_SET_ROUND_AUTO BIT(3) + __le32 id; + __le32 value_low; + __le32 value_high; +}; + +struct clock_info { + int num_clocks; + int max_async_req; + struct scmi_clock_info *clk; +}; + +static int scmi_clock_protocol_attributes_get(const struct scmi_handle *handle, + struct clock_info *ci) +{ + int ret; + struct scmi_xfer *t; + struct scmi_msg_resp_clock_protocol_attributes *attr; + + ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, +SCMI_PROTOCOL_CLOCK, 0, sizeof(*attr), &t); + if (ret) + return ret; + + attr = t->rx.buf; + + ret = scmi_do_xfer(handle, t); + if (!ret) { + ci->num_clocks = le16_to_cpu(attr->num_clocks); + ci->max_async_req = attr->max_async_req; + } + + scmi_one_xfer_put(handle, t); + return ret; +} + +static int scmi_clock_attributes_get(const struct scmi_handle *handle, +u32 clk_id, struct scmi_clock_info *clk) +{ + int ret; + struct scmi_xfer *t; + struct scmi_msg_resp_clock_attributes *attr; + + ret = scmi_one_xfer_init(handle, CLOCK_ATTRIBUTES, SCMI_PROTOCOL_CLOCK, +sizeof(clk_id), sizeof(*attr), &t); + if (ret) + return ret; + + *(__le32 *)t->tx.buf = cpu_to_le32(clk_id); +
[PATCH v4 04/20] firmware: arm_scmi: add common infrastructure and support for base protocol
The base protocol describes the properties of the implementation and provide generic error management. The base protocol provides commands to describe protocol version, discover implementation specific attributes and vendor/sub-vendor identification, list of protocols implemented and the various agents are in the system including OSPM and the platform. It also supports registering for notifications of platform errors. This protocol is mandatory. This patch adds support for the same along with some basic infrastructure to add support for other protocols. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/Makefile | 3 +- drivers/firmware/arm_scmi/base.c | 293 + drivers/firmware/arm_scmi/common.h | 37 + drivers/firmware/arm_scmi/driver.c | 53 +++ include/linux/scmi_protocol.h | 37 + 5 files changed, 422 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/base.c diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index b2a24ba2b636..5d9c7ef35f0f 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -1,2 +1,3 @@ -obj-y = scmi-driver.o +obj-y = scmi-driver.o scmi-protocols.o scmi-driver-y = driver.o +scmi-protocols-y = base.o diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c new file mode 100644 index ..2b5fbb724899 --- /dev/null +++ b/drivers/firmware/arm_scmi/base.c @@ -0,0 +1,293 @@ +/* + * System Control and Management Interface (SCMI) Base Protocol + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include "common.h" + +enum scmi_base_protocol_cmd { + BASE_DISCOVER_VENDOR = 0x3, + BASE_DISCOVER_SUB_VENDOR = 0x4, + BASE_DISCOVER_IMPLEMENT_VERSION = 0x5, + BASE_DISCOVER_LIST_PROTOCOLS = 0x6, + BASE_DISCOVER_AGENT = 0x7, + BASE_NOTIFY_ERRORS = 0x8, +}; + +struct scmi_msg_resp_base_attributes { + u8 num_protocols; + u8 num_agents; + __le16 reserved; +}; + +/** + * scmi_base_attributes_get() - gets the implementation details + * that are associated with the base protocol. + * + * @handle - SCMI entity handle + * + * Return: 0 on success, else appropriate SCMI error. + */ +static int scmi_base_attributes_get(const struct scmi_handle *handle) +{ + int ret; + struct scmi_xfer *t; + struct scmi_msg_resp_base_attributes *attr_info; + struct scmi_revision_info *rev = handle->version; + + ret = scmi_one_xfer_init(handle, PROTOCOL_ATTRIBUTES, +SCMI_PROTOCOL_BASE, 0, sizeof(*attr_info), &t); + if (ret) + return ret; + + ret = scmi_do_xfer(handle, t); + if (!ret) { + attr_info = t->rx.buf; + rev->num_protocols = attr_info->num_protocols; + rev->num_agents = attr_info->num_agents; + } + + scmi_one_xfer_put(handle, t); + return ret; +} + +/** + * scmi_base_vendor_id_get() - gets vendor/subvendor identifier ASCII string. + * + * @handle - SCMI entity handle + * @sub_vendor - specify true if sub-vendor ID is needed + * + * Return: 0 on success, else appropriate SCMI error. + */ +static int +scmi_base_vendor_id_get(const struct scmi_handle *handle, bool sub_vendor) +{ + u8 cmd; + int ret, size; + char *vendor_id; + struct scmi_xfer *t; + struct scmi_revision_info *rev = handle->version; + + if (sub_vendor) { + cmd = BASE_DISCOVER_SUB_VENDOR; + vendor_id = rev->sub_vendor_id; + size = ARRAY_SIZE(rev->sub_vendor_id); + } else { + cmd = BASE_DISCOVER_VENDOR; + vendor_id = rev->vendor_id; + size = ARRAY_SIZE(rev->vendor_id); + } + + ret = scmi_one_xfer_init(handle, cmd, SCMI_PROTOCOL_BASE, 0, size, &t); + if (ret) + return ret; + + ret = scmi_do_xfer(handle, t); + if (!ret) + memcpy(vendor_id, t->rx.buf, size); + + scmi_one_xfer_put(handle, t); + return ret; +} + +/** + * scmi_base_implementation_version_get() - gets a vendor-specific + * implementation 32-bit version. The format of the version number is + * vendor-specific + * +
[PATCH v4 05/20] firmware: arm_scmi: add scmi protocol bus to enumerate protocol devices
The SCMI specification encompasses various protocols. However, not every protocol has to be present on a given platform/implementation as not every protocol is relevant for it. Furthermore, the platform chooses which protocols it exposes to a given agent. The only protocol that must be implemented is the base protocol. The base protocol is used by an agent to discover which protocols are available to it. In order to enumerate the discovered implemented protocols, this patch adds support for a separate scmi protocol bus. It also adds mechanism to register support for different protocols. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/Makefile | 3 +- drivers/firmware/arm_scmi/bus.c| 232 + drivers/firmware/arm_scmi/common.h | 1 + include/linux/scmi_protocol.h | 62 ++ 4 files changed, 297 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/bus.c diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile index 5d9c7ef35f0f..5f4ec2613db6 100644 --- a/drivers/firmware/arm_scmi/Makefile +++ b/drivers/firmware/arm_scmi/Makefile @@ -1,3 +1,4 @@ -obj-y = scmi-driver.o scmi-protocols.o +obj-y = scmi-bus.o scmi-driver.o scmi-protocols.o +scmi-bus-y = bus.o scmi-driver-y = driver.o scmi-protocols-y = base.o diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c new file mode 100644 index ..40c72a35fc99 --- /dev/null +++ b/drivers/firmware/arm_scmi/bus.c @@ -0,0 +1,232 @@ +/* + * System Control and Management Interface (SCMI) Message Protocol bus layer + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include +#include +#include +#include +#include + +#include "common.h" + +static DEFINE_IDA(scmi_bus_id); +static DEFINE_IDR(scmi_protocols); +static DEFINE_SPINLOCK(protocol_lock); + +static const struct scmi_device_id * +scmi_dev_match_id(struct scmi_device *scmi_dev, struct scmi_driver *scmi_drv) +{ + const struct scmi_device_id *id = scmi_drv->id_table; + + if (!id) + return NULL; + + for (; id->protocol_id; id++) + if (id->protocol_id == scmi_dev->protocol_id) + return id; + + return NULL; +} + +static int scmi_dev_match(struct device *dev, struct device_driver *drv) +{ + struct scmi_driver *scmi_drv = to_scmi_driver(drv); + struct scmi_device *scmi_dev = to_scmi_dev(dev); + const struct scmi_device_id *id; + + id = scmi_dev_match_id(scmi_dev, scmi_drv); + if (id) + return 1; + + return 0; +} + +static int scmi_protocol_init(int protocol_id, struct scmi_handle *handle) +{ + scmi_prot_init_fn_t fn = idr_find(&scmi_protocols, protocol_id); + + if (unlikely(!fn)) + return -EINVAL; + return fn(handle); +} + +static int scmi_dev_probe(struct device *dev) +{ + struct scmi_driver *scmi_drv = to_scmi_driver(dev->driver); + struct scmi_device *scmi_dev = to_scmi_dev(dev); + const struct scmi_device_id *id; + int ret; + + id = scmi_dev_match_id(scmi_dev, scmi_drv); + if (!id) + return -ENODEV; + + if (!scmi_dev->handle) + return -EPROBE_DEFER; + + ret = scmi_protocol_init(scmi_dev->protocol_id, scmi_dev->handle); + if (ret) + return ret; + + return scmi_drv->probe(scmi_dev); +} + +static int scmi_dev_remove(struct device *dev) +{ + struct scmi_driver *scmi_drv = to_scmi_driver(dev->driver); + struct scmi_device *scmi_dev = to_scmi_dev(dev); + + if (scmi_drv->remove) + scmi_drv->remove(scmi_dev); + + return 0; +} + +static struct bus_type scmi_bus_type = { + .name = "scmi_protocol", + .match = scmi_dev_match, + .probe = scmi_dev_probe, + .remove = scmi_dev_remove, +}; + +int scmi_driver_register(struct scmi_driver *driver, struct module *owner, +const char *mod_name) +{ + int retval; + + driver->driver.bus = &scmi_bus_type; + driver->driver.name = driver->name; + driver->driver.owner = owner; + driver->driver.mod_name = mod_name; + +
[PATCH v4 01/20] dt-bindings: mailbox: add support for mailbox client shared memory
Many users of the mailbox controllers depend on the shared memory between the two end points to exchange the main data while using simple doorbell mechanism to alert the end points of the presence of a message. This patch defines device tree bindings to represent such shared memory in a generic way. Cc: Rob Herring Cc: Mark Rutland Acked-by: Rob Herring Signed-off-by: Sudeep Holla --- .../devicetree/bindings/mailbox/mailbox.txt| 28 ++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/mailbox/mailbox.txt b/Documentation/devicetree/bindings/mailbox/mailbox.txt index be05b9746c69..af8ecee2ac68 100644 --- a/Documentation/devicetree/bindings/mailbox/mailbox.txt +++ b/Documentation/devicetree/bindings/mailbox/mailbox.txt @@ -23,6 +23,11 @@ assign appropriate mailbox channel to client drivers. Optional property: - mbox-names: List of identifier strings for each mailbox channel. +- shmem : List of phandle pointing to the shared memory(SHM) area between the + users of these mailboxes for IPC, one for each mailbox. This shared + memory can be part of any memory reserved for the purpose of this + communication between the mailbox client and the remote. + Example: pwr_cntrl: power { @@ -30,3 +35,26 @@ assign appropriate mailbox channel to client drivers. mbox-names = "pwr-ctrl", "rpc"; mboxes = <&mailbox 0 &mailbox 1>; }; + +Example with shared memory(shmem): + + sram: sram@5000 { + compatible = "mmio-sram"; + reg = <0x5000 0x1>; + + #address-cells = <1>; + #size-cells = <1>; + ranges = <0 0x5000 0x1>; + + cl_shmem: shmem@0 { + compatible = "client-shmem"; + reg = <0x0 0x200>; + }; + }; + + client@2e00 { + ... + mboxes = <&mailbox 0>; + shmem = <&cl_shmem>; + .. + }; -- 2.7.4
[PATCH v4 03/20] firmware: arm_scmi: add basic driver infrastructure for SCMI
The SCMI is intended to allow OSPM to manage various functions that are provided by the hardware platform it is running on, including power and performance functions. SCMI provides two levels of abstraction, protocols and transports. Protocols define individual groups of system control and management messages. A protocol specification describes the messages that it supports. Transports describe the method by which protocol messages are communicated between agents and the platform. This patch adds basic infrastructure to manage the message allocation, initialisation, packing/unpacking and shared memory management. Cc: Arnd Bergmann Signed-off-by: Sudeep Holla --- MAINTAINERS| 3 +- drivers/firmware/Kconfig | 21 ++ drivers/firmware/Makefile | 1 + drivers/firmware/arm_scmi/Makefile | 2 + drivers/firmware/arm_scmi/common.h | 77 drivers/firmware/arm_scmi/driver.c | 708 + include/linux/scmi_protocol.h | 27 ++ 7 files changed, 838 insertions(+), 1 deletion(-) create mode 100644 drivers/firmware/arm_scmi/Makefile create mode 100644 drivers/firmware/arm_scmi/common.h create mode 100644 drivers/firmware/arm_scmi/driver.c create mode 100644 include/linux/scmi_protocol.h diff --git a/MAINTAINERS b/MAINTAINERS index 25fbf29331c0..8ee63627f1ac 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12957,7 +12957,8 @@ F: Documentation/devicetree/bindings/arm/arm,sc[mp]i.txt F: drivers/clk/clk-scpi.c F: drivers/cpufreq/scpi-cpufreq.c F: drivers/firmware/arm_scpi.c -F: include/linux/scpi_protocol.h +F: drivers/firmware/arm_scmi/ +F: include/linux/sc[mp]i_protocol.h SYSTEM RESET/SHUTDOWN DRIVERS M: Sebastian Reichel diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 6e4ed5a9c6fd..86ea9d531587 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -19,6 +19,27 @@ config ARM_PSCI_CHECKER on and off through hotplug, so for now torture tests and PSCI checker are mutually exclusive. +config ARM_SCMI_PROTOCOL + bool "ARM System Control and Management Interface (SCMI) Message Protocol" + depends on ARM || ARM64 || COMPILE_TEST + depends on MAILBOX + help + ARM System Control and Management Interface (SCMI) protocol is a + set of operating system-independent software interfaces that are + used in system management. SCMI is extensible and currently provides + interfaces for: Discovery and self-description of the interfaces + it supports, Power domain management which is the ability to place + a given device or domain into the various power-saving states that + it supports, Performance management which is the ability to control + the performance of a domain that is composed of compute engines + such as application processors and other accelerators, Clock + management which is the ability to set and inquire rates on platform + managed clocks and Sensor management which is the ability to read + sensor data, and be notified of sensor value. + + This protocol library provides interface for all the client drivers + making use of the features offered by the SCMI. + config ARM_SCPI_PROTOCOL tristate "ARM System Control and Power Interface (SCPI) Message Protocol" depends on ARM || ARM64 || COMPILE_TEST diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile index a37f12e8d137..91d3ff62c653 100644 --- a/drivers/firmware/Makefile +++ b/drivers/firmware/Makefile @@ -23,6 +23,7 @@ obj-$(CONFIG_QCOM_SCM_32) += qcom_scm-32.o CFLAGS_qcom_scm-32.o :=$(call as-instr,.arch armv7-a\n.arch_extension sec,-DREQUIRES_SEC=1) -march=armv7-a obj-$(CONFIG_TI_SCI_PROTOCOL) += ti_sci.o +obj-$(CONFIG_ARM_SCMI_PROTOCOL)+= arm_scmi/ obj-y += broadcom/ obj-y += meson/ obj-$(CONFIG_GOOGLE_FIRMWARE) += google/ diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile new file mode 100644 index ..b2a24ba2b636 --- /dev/null +++ b/drivers/firmware/arm_scmi/Makefile @@ -0,0 +1,2 @@ +obj-y = scmi-driver.o +scmi-driver-y = driver.o diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h new file mode 100644 index ..cdfc43427c3c --- /dev/null +++ b/drivers/firmware/arm_scmi/common.h @@ -0,0 +1,77 @@ +/* + * System Control and Management Interface (SCMI) Message Protocol + * driver common header file containing some definitions, structures + * and function prototypes used in all the different SCMI protocols. + * + * Copyright (C) 2017 ARM Ltd. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free
Re: [PATCH v4 15/20] firmware: arm_scmi: add device power domain support using genpd
Hi Ulf, On 03/11/17 14:47, Sudeep Holla wrote: > This patch hooks up the support for device power domain provided by > SCMI using the Linux generic power domain infrastructure. > > Cc: Kevin Hilman > Cc: Ulf Hansson Sorry for not adding you Reviewed-by tag along with dev_warn suggestion you gave. I seem to have lost that change locally somehow. I have fixed it now. -- Regards, Sudeep
[PATCH] clocksource: timer-of: drop __init annotations in timer_*_exit functions
timer_{base,clk,irq}_exit functions have __init annotations. Commit f48729a999ee ("clocksource/drivers/timer-of: Add timer_of_exit function") added timer_of_exit to undo what has been done by the timer_of_init() function, which means we need to drop __init to use timer_*_exit functions. It also generates the build time warning: "WARNING: vmlinux.o: Section mismatch in reference from the function timer_of_exit() to the variable .init.text:$x The function timer_of_exit() references the variable __init $x. This is often because timer_of_exit lacks a __init annotation or the annotation of $x is wrong." This patch fixes the above build warning. Fixes: f48729a999ee ("clocksource/drivers/timer-of: Add timer_of_exit function") Cc: Daniel Lezcano Cc: Thomas Gleixner Signed-off-by: Sudeep Holla --- drivers/clocksource/timer-of.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c index 7c64a5c1bfc1..701b476ccc7c 100644 --- a/drivers/clocksource/timer-of.c +++ b/drivers/clocksource/timer-of.c @@ -24,7 +24,7 @@ #include "timer-of.h" -static __init void timer_irq_exit(struct of_timer_irq *of_irq) +static void timer_irq_exit(struct of_timer_irq *of_irq) { struct timer_of *to = container_of(of_irq, struct timer_of, of_irq); @@ -72,7 +72,7 @@ static __init int timer_irq_init(struct device_node *np, return 0; } -static __init void timer_clk_exit(struct of_timer_clk *of_clk) +static void timer_clk_exit(struct of_timer_clk *of_clk) { of_clk->rate = 0; clk_disable_unprepare(of_clk->clk); @@ -116,7 +116,7 @@ static __init int timer_clk_init(struct device_node *np, goto out; } -static __init void timer_base_exit(struct of_timer_base *of_base) +static void timer_base_exit(struct of_timer_base *of_base) { iounmap(of_base->base); } -- 2.7.4
Re: [Linaro-acpi] [PATCH v5 0/5] Provide better MADT subtable sanity checks
On 12/10/15 08:04, Hanjun Guo wrote: On 10/12/2015 11:58 AM, Pat Erley wrote: On 10/11/2015 08:49 PM, Hanjun Guo wrote: On 10/12/2015 11:08 AM, Pat Erley wrote: On 10/05/2015 10:12 AM, Al Stone wrote: On 10/05/2015 07:39 AM, Rafael J. Wysocki wrote: On Wednesday, September 30, 2015 10:10:16 AM Al Stone wrote: On 09/30/2015 03:00 AM, Hanjun Guo wrote: On 2015/9/30 7:45, Al Stone wrote: NB: this patch set is for use against the linux-pm bleeding edge branch. [snip...] For this patch set, Reviewed-by: Hanjun Guo Thanks Hanjun Thanks, Hanjun! Series applied, thanks! Rafael Thanks, Rafael! Just decided to test out linux-next (to see the new nouveau cleanups). This change set prevents my Lenovo W510 from booting properly. Reverting: 7494b0 "ACPI: add in a bad_madt_entry() function to eventually replace the macro" Gets the system booting again. I'm attaching my dmesg from the failed boot, who wants the acpidump? [0.00] ACPI: undefined version for either FADT 4.0 or MADT 1 [0.00] ACPI: Error parsing LAPIC address override entry [0.00] ACPI: Invalid BIOS MADT, disabling ACPI Seems the MADT revision is not right, could you dump the ACPI MADT (APIC) table and send it out? I will take a look :) Thanks Hanjun Here ya go, enjoy. Feel free to CC me on any patches that might fix it. Thanks! I think I had the right guess, the MADT revision is not right for ACPI 4.0: [000h 4]Signature : "APIC"[Multiple APIC Description Table (MADT)] [004h 0004 4] Table Length : 00BC [008h 0008 1] *Revision : 01* I encountered such problem before because the table was just copied from previous version, and without the update for table revision. I think we may need to ignore the table revision for x86, but restrict it for ARM64, I'd like Al and Rafael's suggestion before I send out a patch. Instead of just removing the check completely on x86, IMO restrict it to some newer/later version of ACPI so you can still force vendors to fix their ACPI tables at-least in future. It would be good to get such sanity check in the tools used to build those tables, but yes since such static tables can be built in many ways, its difficult to deal it in all those tools. -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpufreq: arm_big_little: fix frequency check when bL switcher is active
On 08/10/15 10:23, Jon Medhurst (Tixy) wrote: [...] diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c index f1e42f8..59115a4 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -149,6 +149,18 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) __func__, cpu, old_cluster, new_cluster, new_rate); ret = clk_set_rate(clk[new_cluster], new_rate * 1000); + if (!ret) { + /* +* FIXME: clk_set_rate has to handle the case where clk_change_rate +* can fail due to hardware or firmware issues. Until the clk core +* layer is fixed, we can check here. In most of the cases we will +* be reading only the cached value anyway. This needs to be removed +* once clk core is fixed. +*/ + if (clk_get_rate(clk[new_cluster]) != new_rate * 1000) + ret = -EIO; + } + if (WARN_ON(ret)) { pr_err("clk_set_rate failed: %d, new cluster: %d\n", ret, new_cluster); @@ -189,15 +201,6 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) mutex_unlock(&cluster_lock[old_cluster]); } - /* -* FIXME: clk_set_rate has to handle the case where clk_change_rate -* can fail due to hardware or firmware issues. Until the clk core -* layer is fixed, we can check here. In most of the cases we will -* be reading only the cached value anyway. This needs to be removed -* once clk core is fixed. -*/ - if (bL_cpufreq_get_rate(cpu) != new_rate) - return -EIO; return 0; } The above change looks good to me but with minor nit. You can get rid of if(!ret) check if you move the hunk after if (WARN_ON(ret)) -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Linaro-acpi] [PATCH v5 0/5] Provide better MADT subtable sanity checks
On 12/10/15 20:25, Rafael J. Wysocki wrote: On Monday, October 12, 2015 10:44:52 AM Sudeep Holla wrote: [...] Instead of just removing the check completely on x86, IMO restrict it to some newer/later version of ACPI so you can still force vendors to fix their ACPI tables at-least in future. No, we can't force vendors to fix their ACPI tables. This is completely unrealistic. No, I was referring to the future platforms *only* We simly need to deal with the bugs in the ACPI tables in the kernel. Yes sadly true for existing systems, but if we now place a check for ACPIv6.0 and above, we might avoid seeing such broken tables sometime in future once the kernel with this check in place is used for validation. It would be good to get such sanity check in the tools used to build those tables, but yes since such static tables can be built in many ways, its difficult to deal it in all those tools. As I said to Al, we need those checks in firmware test suites. Having them in the kernel is OK too, but they should cause warnings to be printed to the kernel log instead of causing the kernel to panic. Agreed -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpufreq: arm_big_little: fix frequency check when bL switcher is active
On 13/10/15 08:19, Jon Medhurst (Tixy) wrote: On Mon, 2015-10-12 at 14:20 +0100, Sudeep Holla wrote: On 08/10/15 10:23, Jon Medhurst (Tixy) wrote: [...] diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c index f1e42f8..59115a4 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -149,6 +149,18 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) __func__, cpu, old_cluster, new_cluster, new_rate); ret = clk_set_rate(clk[new_cluster], new_rate * 1000); + if (!ret) { + /* +* FIXME: clk_set_rate has to handle the case where clk_change_rate +* can fail due to hardware or firmware issues. Until the clk core +* layer is fixed, we can check here. In most of the cases we will +* be reading only the cached value anyway. This needs to be removed +* once clk core is fixed. +*/ + if (clk_get_rate(clk[new_cluster]) != new_rate * 1000) + ret = -EIO; + } + if (WARN_ON(ret)) { pr_err("clk_set_rate failed: %d, new cluster: %d\n", ret, new_cluster); @@ -189,15 +201,6 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) mutex_unlock(&cluster_lock[old_cluster]); } - /* -* FIXME: clk_set_rate has to handle the case where clk_change_rate -* can fail due to hardware or firmware issues. Until the clk core -* layer is fixed, we can check here. In most of the cases we will -* be reading only the cached value anyway. This needs to be removed -* once clk core is fixed. -*/ - if (bL_cpufreq_get_rate(cpu) != new_rate) - return -EIO; return 0; } The above change looks good to me but with minor nit. You can get rid of if(!ret) check if you move the hunk after if (WARN_ON(ret)) But then we wouldn't get the WARN_ON and pr_err triggered when we detect the clock rate isn't set, which surely is half the reason for the check in the first place? Not sure if I understand what you mean or may be I was not clear, so thought I will put the delta here. Let me know if and how its still a problem. Regards, Sudeep -->8 diff --git i/drivers/cpufreq/arm_big_little.c w/drivers/cpufreq/arm_big_little.c index f1e42f8ce0fc..05e850f80f39 100644 --- i/drivers/cpufreq/arm_big_little.c +++ w/drivers/cpufreq/arm_big_little.c @@ -164,6 +164,16 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) mutex_unlock(&cluster_lock[new_cluster]); + /* +* FIXME: clk_set_rate has to handle the case where clk_change_rate +* can fail due to hardware or firmware issues. Until the clk core +* layer is fixed, we can check here. In most of the cases we will +* be reading only the cached value anyway. This needs to be removed +* once clk core is fixed. +*/ + if (bL_cpufreq_get_rate(cpu) != new_rate) + return -EIO; + /* Recalc freq for old cluster when switching clusters */ if (old_cluster != new_cluster) { pr_debug("%s: cpu: %d, old cluster: %d, new cluster: %d\n", @@ -189,15 +199,6 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) mutex_unlock(&cluster_lock[old_cluster]); } - /* -* FIXME: clk_set_rate has to handle the case where clk_change_rate -* can fail due to hardware or firmware issues. Until the clk core -* layer is fixed, we can check here. In most of the cases we will -* be reading only the cached value anyway. This needs to be removed -* once clk core is fixed. -*/ - if (bL_cpufreq_get_rate(cpu) != new_rate) - return -EIO; return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 12/17] ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag
On 12/10/15 21:28, Tony Lindgren wrote: * Tony Lindgren [151012 13:27]: * Sudeep Holla [150921 08:52]: The IRQF_NO_SUSPEND flag is used to identify the interrupts that should be left enabled so as to allow them to work as expected during the suspend-resume cycle, but doesn't guarantee that it will wake the system from a suspended state, enable_irq_wake is recommended to be used for the wakeup. This patch removes the use of IRQF_NO_SUSPEND flags replacing it with enable_irq_wake instead. Applying into omap-for-v4.4/cleanup thanks. Actually I don't think this does the right thing. The interrupts in the $subject patch are in the always on powerdomain, and we really Agreed want them to be excluded from the suspend. OK but what's wrong with this patch. At-least the name suggest it's a wakeup interrupt. And using IRQF_NO_SUSPEND for the wakeup interrupt is simply wrong. So not applying without further explanations. But I don't understand the real need for IRQF_NO_SUSPEND over wakeup APIs ? -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] ARM: dts: change gpio-key,wakeup property to boolean
Keyboard driver for GPIO buttons(gpio-keys) checks for the legacy "gpio-key,wakeup" boolean property to enable gpio buttons as wakeup source. Few dts files assign value "1" to gpio-key,wakeup and in one instance a value "0" is assigned probably assuming it won't be enabled as a wakeup source. Since the presence of the boolean property indicates it is enabled, value of "0" have no value. This patch removes the property where value "0" is assigned and removes the value "1" in most of the other cases. Cc: Heiko Stuebner Cc: linux-rockc...@lists.infradead.org Cc: Viresh Kumar Cc: spear-de...@list.st.com Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/rk3066a-bqcurie2.dts | 3 +-- arch/arm/boot/dts/rk3066a-rayeager.dts | 2 +- arch/arm/boot/dts/rk3188-radxarock.dts | 2 +- arch/arm/boot/dts/rk3288-evb.dtsi | 2 +- arch/arm/boot/dts/rk3288-firefly.dtsi | 2 +- arch/arm/boot/dts/rk3288-popmetal.dts | 2 +- arch/arm/boot/dts/rk3288-r89.dts| 2 +- arch/arm/boot/dts/spear1310-evb.dts | 2 +- arch/arm/boot/dts/spear1340-evb.dts | 2 +- arch/arm/boot/dts/spear320-hmi.dts | 4 ++-- arch/arm64/boot/dts/rockchip/rk3368-r88.dts | 2 +- 11 files changed, 12 insertions(+), 13 deletions(-) diff --git a/arch/arm/boot/dts/rk3066a-bqcurie2.dts b/arch/arm/boot/dts/rk3066a-bqcurie2.dts index c0273755431a..71a15772cda6 100644 --- a/arch/arm/boot/dts/rk3066a-bqcurie2.dts +++ b/arch/arm/boot/dts/rk3066a-bqcurie2.dts @@ -74,7 +74,7 @@ linux,code = <116>; label = "GPIO Key Power"; linux,input-type = <1>; - gpio-key,wakeup = <1>; + gpio-key,wakeup; debounce-interval = <100>; }; button@1 { @@ -82,7 +82,6 @@ linux,code = <104>; label = "GPIO Key Vol-"; linux,input-type = <1>; - gpio-key,wakeup = <0>; debounce-interval = <100>; }; /* VOL+ comes somehow thru the ADC */ diff --git a/arch/arm/boot/dts/rk3066a-rayeager.dts b/arch/arm/boot/dts/rk3066a-rayeager.dts index e36383c701dc..72bf52a04506 100644 --- a/arch/arm/boot/dts/rk3066a-rayeager.dts +++ b/arch/arm/boot/dts/rk3066a-rayeager.dts @@ -65,7 +65,7 @@ #size-cells = <0>; button@0 { - gpio-key,wakeup = <1>; + gpio-key,wakeup; gpios = <&gpio6 2 GPIO_ACTIVE_LOW>; label = "GPIO Power"; linux,code = <116>; diff --git a/arch/arm/boot/dts/rk3188-radxarock.dts b/arch/arm/boot/dts/rk3188-radxarock.dts index d2180e5d2b05..ba6b174777e7 100644 --- a/arch/arm/boot/dts/rk3188-radxarock.dts +++ b/arch/arm/boot/dts/rk3188-radxarock.dts @@ -63,7 +63,7 @@ linux,code = <116>; label = "GPIO Key Power"; linux,input-type = <1>; - gpio-key,wakeup = <1>; + gpio-key,wakeup; debounce-interval = <100>; }; }; diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi b/arch/arm/boot/dts/rk3288-evb.dtsi index f6d2e7894b05..ab013155f04f 100644 --- a/arch/arm/boot/dts/rk3288-evb.dtsi +++ b/arch/arm/boot/dts/rk3288-evb.dtsi @@ -103,7 +103,7 @@ linux,code = <116>; label = "GPIO Key Power"; linux,input-type = <1>; - gpio-key,wakeup = <1>; + gpio-key,wakeup; debounce-interval = <100>; }; }; diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi b/arch/arm/boot/dts/rk3288-firefly.dtsi index 20fa0ef0b96b..4fa2dedd0c7a 100644 --- a/arch/arm/boot/dts/rk3288-firefly.dtsi +++ b/arch/arm/boot/dts/rk3288-firefly.dtsi @@ -67,7 +67,7 @@ #size-cells = <0>; button@0 { - gpio-key,wakeup = <1>; + gpio-key,wakeup; gpios = <&gpio0 5 GPIO_ACTIVE_LOW>; label = "GPIO Power"; linux,code = <116>; diff --git a/arch/arm/boot/dts/rk3288-popmetal.dts b/arch/arm/boot/dts/rk3288-popmetal.dts index f82b956ebf17..9b7f389c38e5 100644 --- a/arch/arm/boot/dts/rk3288-popmetal.dts +++ b/arch/arm/boot/dts/rk3288-popmetal.dts @@ -74,7 +74,7 @@ linux,code = <116>; label = "GPIO Key Power";
[PATCH 1/2] ARM: dts: fix gpio-keys wakeup-source property
The keyboard driver for GPIO buttons(gpio-keys) checks for one of the two boolean properties to enable gpio buttons as wakeup source: 1. "wakeup-source" or 2. the legacy "gpio-key,wakeup" However juno, ste-snowball and emev2-kzm9d dts file have a undetected "wakeup" property to indictate the wakeup source. This patch fixes it by making use of "wakeup-source" property. Cc: Simon Horman Cc: Magnus Damm Cc: Linus Walleij Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/emev2-kzm9d.dts | 8 arch/arm/boot/dts/ste-snowball.dts| 10 +- arch/arm64/boot/dts/arm/juno-motherboard.dtsi | 12 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/arch/arm/boot/dts/emev2-kzm9d.dts b/arch/arm/boot/dts/emev2-kzm9d.dts index 955c24ee4a8c..8c24975e8f9d 100644 --- a/arch/arm/boot/dts/emev2-kzm9d.dts +++ b/arch/arm/boot/dts/emev2-kzm9d.dts @@ -35,28 +35,28 @@ button@1 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; label = "DSW2-1"; linux,code = ; gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>; }; button@2 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; label = "DSW2-2"; linux,code = ; gpios = <&gpio0 15 GPIO_ACTIVE_HIGH>; }; button@3 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; label = "DSW2-3"; linux,code = ; gpios = <&gpio0 16 GPIO_ACTIVE_HIGH>; }; button@4 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; label = "DSW2-4"; linux,code = ; gpios = <&gpio0 17 GPIO_ACTIVE_HIGH>; diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts index 32a5ccb14e7e..e80e42163883 100644 --- a/arch/arm/boot/dts/ste-snowball.dts +++ b/arch/arm/boot/dts/ste-snowball.dts @@ -47,35 +47,35 @@ button@1 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <2>; label = "userpb"; gpios = <&gpio1 0 0x4>; }; button@2 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <3>; label = "extkb1"; gpios = <&gpio4 23 0x4>; }; button@3 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <4>; label = "extkb2"; gpios = <&gpio4 24 0x4>; }; button@4 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <5>; label = "extkb3"; gpios = <&gpio5 1 0x4>; }; button@5 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <6>; label = "extkb4"; gpios = <&gpio5 2 0x4>; diff --git a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi index 05e461dc6c1f..a15e1fb4d192 100644 --- a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi +++ b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi @@ -61,42 +61,42 @@ button@1 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <116>; label = "POWER"; gpios = <
Re: [PATCH 2/2] ARM: dts: change gpio-key,wakeup property to boolean
On 13/10/15 15:07, Rob Herring wrote: On Tue, Oct 13, 2015 at 8:32 AM, Sudeep Holla wrote: Keyboard driver for GPIO buttons(gpio-keys) checks for the legacy "gpio-key,wakeup" boolean property to enable gpio buttons as wakeup source. Few dts files assign value "1" to gpio-key,wakeup and in one instance a value "0" is assigned probably assuming it won't be enabled as a wakeup source. Since the presence of the boolean property indicates it is enabled, value of "0" have no value. This patch removes the property where value "0" is assigned and removes the value "1" in most of the other cases. Why don't you just change everything to wakeup-source. Agreed, I wanted to do that but was not sure if that's acceptable. I will update it, now I know that you are fine with it. I also plan to find all the variety of bindings we have and fix them retaining only those which are handled in the current kernel code as legacy support. -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2][UPDATE] ARM: dts: replace gpio-key,wakeup with wakeup-source property
Keyboard driver for GPIO buttons(gpio-keys) checks for the legacy "gpio-key,wakeup" boolean property to enable gpio buttons as wakeup source. Few dts files assign value "1" to gpio-key,wakeup and in one instance a value "0" is assigned probably assuming it won't be enabled as a wakeup source. Since the presence of the boolean property indicates it is enabled, value of "0" have no value. This patch replaces the legacy "gpio-key,wakeup" with the unified "wakeup-source" property which inturn fixes the above mentioned issue. Cc: Heiko Stuebner Cc: linux-rockc...@lists.infradead.org Cc: Viresh Kumar Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/rk3066a-bqcurie2.dts | 3 +-- arch/arm/boot/dts/rk3066a-rayeager.dts | 2 +- arch/arm/boot/dts/rk3188-radxarock.dts | 2 +- arch/arm/boot/dts/rk3288-evb.dtsi | 2 +- arch/arm/boot/dts/rk3288-firefly.dtsi | 2 +- arch/arm/boot/dts/rk3288-popmetal.dts | 2 +- arch/arm/boot/dts/rk3288-r89.dts| 2 +- arch/arm/boot/dts/spear1310-evb.dts | 2 +- arch/arm/boot/dts/spear1340-evb.dts | 2 +- arch/arm/boot/dts/spear320-hmi.dts | 4 ++-- arch/arm64/boot/dts/rockchip/rk3368-r88.dts | 2 +- 11 files changed, 12 insertions(+), 13 deletions(-) diff --git a/arch/arm/boot/dts/rk3066a-bqcurie2.dts b/arch/arm/boot/dts/rk3066a-bqcurie2.dts index c0273755431a..93be8f066e6d 100644 --- a/arch/arm/boot/dts/rk3066a-bqcurie2.dts +++ b/arch/arm/boot/dts/rk3066a-bqcurie2.dts @@ -74,7 +74,7 @@ linux,code = <116>; label = "GPIO Key Power"; linux,input-type = <1>; - gpio-key,wakeup = <1>; + wakeup-source; debounce-interval = <100>; }; button@1 { @@ -82,7 +82,6 @@ linux,code = <104>; label = "GPIO Key Vol-"; linux,input-type = <1>; - gpio-key,wakeup = <0>; debounce-interval = <100>; }; /* VOL+ comes somehow thru the ADC */ diff --git a/arch/arm/boot/dts/rk3066a-rayeager.dts b/arch/arm/boot/dts/rk3066a-rayeager.dts index e36383c701dc..da1c37a721c0 100644 --- a/arch/arm/boot/dts/rk3066a-rayeager.dts +++ b/arch/arm/boot/dts/rk3066a-rayeager.dts @@ -65,7 +65,7 @@ #size-cells = <0>; button@0 { - gpio-key,wakeup = <1>; + wakeup-source; gpios = <&gpio6 2 GPIO_ACTIVE_LOW>; label = "GPIO Power"; linux,code = <116>; diff --git a/arch/arm/boot/dts/rk3188-radxarock.dts b/arch/arm/boot/dts/rk3188-radxarock.dts index d2180e5d2b05..acd3bb940e9a 100644 --- a/arch/arm/boot/dts/rk3188-radxarock.dts +++ b/arch/arm/boot/dts/rk3188-radxarock.dts @@ -63,7 +63,7 @@ linux,code = <116>; label = "GPIO Key Power"; linux,input-type = <1>; - gpio-key,wakeup = <1>; + wakeup-source; debounce-interval = <100>; }; }; diff --git a/arch/arm/boot/dts/rk3288-evb.dtsi b/arch/arm/boot/dts/rk3288-evb.dtsi index f6d2e7894b05..e35cebc69e87 100644 --- a/arch/arm/boot/dts/rk3288-evb.dtsi +++ b/arch/arm/boot/dts/rk3288-evb.dtsi @@ -103,7 +103,7 @@ linux,code = <116>; label = "GPIO Key Power"; linux,input-type = <1>; - gpio-key,wakeup = <1>; + wakeup-source; debounce-interval = <100>; }; }; diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi b/arch/arm/boot/dts/rk3288-firefly.dtsi index 20fa0ef0b96b..c5dd26139b63 100644 --- a/arch/arm/boot/dts/rk3288-firefly.dtsi +++ b/arch/arm/boot/dts/rk3288-firefly.dtsi @@ -67,7 +67,7 @@ #size-cells = <0>; button@0 { - gpio-key,wakeup = <1>; + wakeup-source; gpios = <&gpio0 5 GPIO_ACTIVE_LOW>; label = "GPIO Power"; linux,code = <116>; diff --git a/arch/arm/boot/dts/rk3288-popmetal.dts b/arch/arm/boot/dts/rk3288-popmetal.dts index f82b956ebf17..34a0b063b3ec 100644 --- a/arch/arm/boot/dts/rk3288-popmetal.dts +++ b/arch/arm/boot/dts/rk3288-popmetal.dts @@ -74,7 +74,7 @@ linux,code = <116>; label = "GPIO Key Power"; linux,input-type =
Re: [PATCH 12/17] ARM: OMAP2+: remove misuse of IRQF_NO_SUSPEND flag
On 13/10/15 15:53, Tony Lindgren wrote: * Sudeep Holla [151013 03:46]: On 12/10/15 21:28, Tony Lindgren wrote: * Tony Lindgren [151012 13:27]: * Sudeep Holla [150921 08:52]: The IRQF_NO_SUSPEND flag is used to identify the interrupts that should be left enabled so as to allow them to work as expected during the suspend-resume cycle, but doesn't guarantee that it will wake the system >from a suspended state, enable_irq_wake is recommended to be used for the wakeup. This patch removes the use of IRQF_NO_SUSPEND flags replacing it with enable_irq_wake instead. Applying into omap-for-v4.4/cleanup thanks. Actually I don't think this does the right thing. The interrupts in the $subject patch are in the always on powerdomain, and we really Agreed want them to be excluded from the suspend. OK but what's wrong with this patch. At-least the name suggest it's a wakeup interrupt. And using IRQF_NO_SUSPEND for the wakeup interrupt is simply wrong. Hmm so if we have a separate always on irq controller for the wake-up events and we want to keep it always on and exclude it from any suspend related things.. Why would we not use IRQF_NO_SUSPEND on it? Above you say "The IRQF_NO_SUSPEND flag is used to identify the interrupts that should be left enabled so as to allow them to work as expected during the suspend-resume cycle..." and that's exactly what we want to do here :) OK if these interrupts meet that criteria to use IRQF_NO_SUSPEND, then it should be fine, my earlier argument was based on the assumption that it's just another wakeup interrupt. For the dedicated wake-up interrupts, we have separate registers to enable and disable them. The $subject irq is the shared interrupt that allows making use of the pin specific wake-up interrupts, and for those yes we are using enable_irq_wake(). If it's already take care, then fine. I am just hunting all the misuse of IRQF_NO_SUSPEND flag especially as wakeup source and fixing them So not applying without further explanations. But I don't understand the real need for IRQF_NO_SUSPEND over wakeup APIs ? Because in the $subject case we just want to always keep it on and never suspend it. It's unrelated to the wakeup APIs at least for the omap related SoCs. OK, understood now. Thanks -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cpufreq: arm_big_little: fix frequency check when bL switcher is active
On 14/10/15 08:12, Jon Medhurst (Tixy) wrote: On Tue, 2015-10-13 at 11:36 +0100, Sudeep Holla wrote: On 13/10/15 08:19, Jon Medhurst (Tixy) wrote: [...] But then we wouldn't get the WARN_ON and pr_err triggered when we detect the clock rate isn't set, which surely is half the reason for the check in the first place? Not sure if I understand what you mean or may be I was not clear, so thought I will put the delta here. Let me know if and how its still a problem. diff --git i/drivers/cpufreq/arm_big_little.c w/drivers/cpufreq/arm_big_little.c index f1e42f8ce0fc..05e850f80f39 100644 --- i/drivers/cpufreq/arm_big_little.c +++ w/drivers/cpufreq/arm_big_little.c @@ -164,6 +164,16 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) mutex_unlock(&cluster_lock[new_cluster]); + /* +* FIXME: clk_set_rate has to handle the case where clk_change_rate +* can fail due to hardware or firmware issues. Until the clk core +* layer is fixed, we can check here. In most of the cases we will +* be reading only the cached value anyway. This needs to be removed +* once clk core is fixed. +*/ + if (bL_cpufreq_get_rate(cpu) != new_rate) + return -EIO; + /* Recalc freq for old cluster when switching clusters */ if (old_cluster != new_cluster) { pr_debug("%s: cpu: %d, old cluster: %d, new cluster: %d\n", That's what I though you meant, and I can't see why you would want to do that and bypass the error reporting for clk_get_rate failing. After all, the code we're moving around is explicitly there to workaround the fact that clk_set_rate doesn't actually pass through all errors, so it's doing additional error checking. (At least, that's what the comment says). So this looks more logical to me. OK, I understand what you mean now. I don't have a strong opinion, but here is the reason why I prefer the approach I said earlier: clk_set_rate doesn't return error if the h/w or f/w return error which is usually the last step. So calling clk_get_rate when clk_set_rate return error quite early makes no sense to me. -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] ARM: dts: fix gpio-keys wakeup-source property
On 14/10/15 01:13, Simon Horman wrote: [cc linux-sh] On Tue, Oct 13, 2015 at 02:32:43PM +0100, Sudeep Holla wrote: The keyboard driver for GPIO buttons(gpio-keys) checks for one of the two boolean properties to enable gpio buttons as wakeup source: 1. "wakeup-source" or 2. the legacy "gpio-key,wakeup" However juno, ste-snowball and emev2-kzm9d dts file have a undetected "wakeup" property to indictate the wakeup source. This patch fixes it by making use of "wakeup-source" property. Cc: Simon Horman Cc: Magnus Damm Cc: Linus Walleij Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/emev2-kzm9d.dts | 8 arch/arm/boot/dts/ste-snowball.dts| 10 +- arch/arm64/boot/dts/arm/juno-motherboard.dtsi | 12 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) emev2-kzm9d portion: Acked-by: Simon Horman My preferred course of action would be to take that portion through the renesas tree if it was broken out into a separate patch. But I won't object if someone wants to take the whole patch/series. I can split this patch, but it is more like a bug fix IMO, I can't use these GPIO as wakeup on Juno. So I hope arm-soc guys take it directly. If not, I can split and sent it separately. -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] ARM: dts: fix gpio-keys wakeup-source property
Hi Linus, On 13/10/15 14:32, Sudeep Holla wrote: The keyboard driver for GPIO buttons(gpio-keys) checks for one of the two boolean properties to enable gpio buttons as wakeup source: 1. "wakeup-source" or 2. the legacy "gpio-key,wakeup" However juno, ste-snowball and emev2-kzm9d dts file have a undetected "wakeup" property to indictate the wakeup source. This patch fixes it by making use of "wakeup-source" property. Cc: Simon Horman Cc: Magnus Damm Cc: Linus Walleij Please review this and provide ack if you are fine with the change for ste-snowball. I can then ask arm-soc guys to pick this up as fix for v4.3 -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/5] ACPI: add in a bad_madt_entry() function to eventually replace the macro
Hi Al, On 19/08/15 23:07, Al Stone wrote: I finally got a chance to try this series on Juno. Well it exposed a firmware bug in MADT table :) [..] acpi_tbl_entry_handler handler, @@ -245,6 +484,8 @@ acpi_parse_entries(char *id, unsigned long table_size, table_end) { if (entry->type == entry_id && (!max_entries || count < max_entries)) { + if (bad_madt_entry(table_header, entry)) + return -EINVAL; Not sure if we can have the above check here unconditionally. Currently I can see there are 2 other users of acpi_parse_entries i.e. PCC and NUMA. So may be it can be made conditional or return success for non-MADT tables from bad_madt_entry ? Other than that, you can add for ARM64 specific parts: Reviewed-and-tested-by: Sudeep Holla Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] pinctrl: mediatek: Implement wake handler and suspend resume
On 06/09/15 11:39, maoguang meng wrote: On Wed, 2015-09-02 at 14:02 +0800, Daniel Kurtz wrote: Hi maoguang, On Tue, Aug 25, 2015 at 12:27 AM, Sudeep Holla wrote: On 14/08/15 09:38, maoguang.m...@mediatek.com wrote: From: Maoguang Meng This patch implement irq_set_wake to get who is wakeup source and setup on suspend resume. Signed-off-by: Maoguang Meng [snip] Can you please respond to Sudeep's questions: Does this pinmux controller: 1. Support wake-up configuration ? If not, you need to use IRQCHIP_SKIP_SET_WAKE. I don't see any value in writing the mask_{set,clear} if the same registers are used for {en,dis}able YES. we can call enable_irq_wake(irq) to config this irq as a wake-up source. No that doesn't answer my question. Yes you can always call enable_irq_wake(irq) as along as you have irq_set_wake implemented. But my question was does this pinmux controller support wake-up configuration. IMHO, by looking at the implementation I can confirm *NO*, it doesn't. So please stop copy-pasting the implementation from other drivers. The reason is you operate on the same mask_{set,clear} which you use to {en,dis}able the interrupts which means you don't have to do anything to configure an interrupt as a wakeup source other than just keeping them enabled. Hopefully this clarifies. 2. Is in always on domain ? If not, you need save/restore only to resume back the functionality. Generally we can set IRQCHIP_MASK_ON_SUSPEND to ensure non-wake-up interrupts are disabled during suspend and re-enabled in resume path. You just save/restore raw values without tracking the wake-up source. YES. Again *YES* for what part of my question. If it's always-on, then you don't need this suspend/resume callbacks at all, so I assume the answer is *NO*. Also I see that no care is taken to set the port irq as wake enable source. It may work with current mainline, but won't with -next. Please ensure the port irq to the parent interrupt controller remains enabled(i.e set as wake). As you know, we do not care the port irq as wake enbale source, when enter suspend we think no matter what the port irq is enabled/disabled which are not affected as a wake-up source, Because eint has a line to receive SPM. I understand that, but for proper wakeup functionality you need to do configure the parent if you want to identify and handle that wakeup interrupt. If you don't care then it should be fine. For example,after suspend,press power key(use eint5) will generate an electrical signal which is send to spm by eint. and then spm wake cpu. Yes I got that, as I mentioned above, I think you don't need to identify and handle the wakeup interrupt. E.g. if it's a keyboard key press, you may need to identify the key pressed, so it needs to be handled properly. In your case, I assume it's just power button whose main function is to wake/boot and that's already achieved when you are woken up. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4] pinctrl: mediatek: Implement wake handler and suspend resume
On 08/09/15 10:28, Sudeep Holla wrote: On 06/09/15 11:39, maoguang meng wrote: On Wed, 2015-09-02 at 14:02 +0800, Daniel Kurtz wrote: Hi maoguang, On Tue, Aug 25, 2015 at 12:27 AM, Sudeep Holla wrote: On 14/08/15 09:38, maoguang.m...@mediatek.com wrote: From: Maoguang Meng This patch implement irq_set_wake to get who is wakeup source and setup on suspend resume. Signed-off-by: Maoguang Meng [snip] Can you please respond to Sudeep's questions: Does this pinmux controller: 1. Support wake-up configuration ? If not, you need to use IRQCHIP_SKIP_SET_WAKE. I don't see any value in writing the mask_{set,clear} if the same registers are used for {en,dis}able YES. we can call enable_irq_wake(irq) to config this irq as a wake-up source. No that doesn't answer my question. Yes you can always call enable_irq_wake(irq) as along as you have irq_set_wake implemented. But my question was does this pinmux controller support wake-up configuration. IMHO, by looking at the implementation I can confirm *NO*, it doesn't. So please stop copy-pasting the implementation from other drivers. The reason is you operate on the same mask_{set,clear} which you use to {en,dis}able the interrupts which means you don't have to do anything to configure an interrupt as a wakeup source other than just keeping them enabled. Hopefully this clarifies. 2. Is in always on domain ? If not, you need save/restore only to resume back the functionality. Generally we can set IRQCHIP_MASK_ON_SUSPEND to ensure non-wake-up interrupts are disabled during suspend and re-enabled in resume path. You just save/restore raw values without tracking the wake-up source. YES. Again *YES* for what part of my question. If it's always-on, then you don't need this suspend/resume callbacks at all, so I assume the answer is *NO*. IIUC this pinmux controller, something like below should work. >8 From 7c26992bce047efb66a6ba8d2ffec2b272499df7 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Tue, 8 Sep 2015 17:35:38 +0100 Subject: [PATCH] pinctrl: mediatek: Enable SKIP_SET_WAKE and MASK_ON_SUSPEND The mediatek pinmux controller doesn't provides any facility to configure the wakeup sources. So instead of providing redundant irq_set_wake functionality, SKIP_SET_WAKE could be set to it. Also there's no need to maintain 2 sets of masks. This patch enables IRQCHIP_SKIP_SET_WAKE and IRQCHIP_MASK_ON_SUSPEND and also removes wake_mask. Cc: Linus Walleij Cc: Matthias Brugger Cc: Hongzhou Yang Cc: Yingjoe Chen Cc: Maoguang Meng Cc: Chaotian Jing Cc: linux-g...@vger.kernel.org Cc: linux-media...@lists.infradead.org Signed-off-by: Sudeep Holla --- drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 24 +--- drivers/pinctrl/mediatek/pinctrl-mtk-common.h | 1 - 2 files changed, 1 insertion(+), 24 deletions(-) diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c index 7726c6caaf83..d8e194a5bb31 100644 --- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c +++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c @@ -1063,20 +1063,6 @@ static int mtk_eint_set_type(struct irq_data *d, return 0; } -static int mtk_eint_irq_set_wake(struct irq_data *d, unsigned int on) -{ - struct mtk_pinctrl *pctl = irq_data_get_irq_chip_data(d); - int shift = d->hwirq & 0x1f; - int reg = d->hwirq >> 5; - - if (on) - pctl->wake_mask[reg] |= BIT(shift); - else - pctl->wake_mask[reg] &= ~BIT(shift); - - return 0; -} - static void mtk_eint_chip_write_mask(const struct mtk_eint_offsets *chip, void __iomem *eint_reg_base, u32 *buf) { @@ -1112,7 +1098,6 @@ static int mtk_eint_suspend(struct device *device) reg = pctl->eint_reg_base; mtk_eint_chip_read_mask(eint_offsets, reg, pctl->cur_mask); - mtk_eint_chip_write_mask(eint_offsets, reg, pctl->wake_mask); return 0; } @@ -1153,9 +1138,9 @@ static struct irq_chip mtk_pinctrl_irq_chip = { .irq_unmask = mtk_eint_unmask, .irq_ack = mtk_eint_ack, .irq_set_type = mtk_eint_set_type, - .irq_set_wake = mtk_eint_irq_set_wake, .irq_request_resources = mtk_pinctrl_irq_request_resources, .irq_release_resources = mtk_pinctrl_irq_release_resources, + .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MASK_ON_SUSPEND, }; static unsigned int mtk_eint_init(struct mtk_pinctrl *pctl) @@ -1393,13 +1378,6 @@ int mtk_pctrl_init(struct platform_device *pdev, } ports_buf = pctl->devdata->eint_offsets.ports; - pctl->wake_mask = devm_kcalloc(&pdev->dev, ports_buf, - sizeof(*pctl->wake_mask), GFP_KERNEL); - if (!pctl->wake_mask) { - ret = -ENOMEM;
Re: [PATCH 0/2] irqchip: renesas: Propagate wake-up settings to parent
On 08/09/15 18:00, Geert Uytterhoeven wrote: Hi Thomas, Jason, Marc, The renesas-intc-irqpin and renesas-irqc interrupt controllers are cascaded to GICs, but their drivers don't propagate wake-up settings to their parent interrupt controllers. Since commit aec89ef72ba6c944 ("irqchip/gic: Enable SKIP_SET_WAKE and MASK_ON_SUSPEND"), the GIC driver masks interrupts during suspend, and wake-up through gpio-keys now fails on r8a73a4/ape6evm, r8a7740/armadillo and sh73a0/kzm9g. Fix this by propagating wake-up settings to the parent interrupt controllers. There's no need to handle irq_set_irq_wake() failures, as the renesas-intc-irqpin and renesas-irqc interrupt controllers are always cascaded to GICs, and the GIC driver always sets SKIP_SET_WAKE since the aforementioned commit. Ah so, we have the first culprit. I expect many more to come, as I had seen many drivers didn't bother to enable wakeup source correctly. These fixes are meant for v4.3, as commit aec89ef72ba6c944 is already upstream. Looks fine to me, so for the series: Acked-by: Sudeep Holla Note that masking GIC interrupts during suspend has another side effect: before, if no_console_suspend was enabled, the system woke up on console activity. This is no longer the case, but that's not a bug. It's very much similar to IRQF_NO_SUSPEND flag which keeps the IRQ enabled but doesn't guarantee wakeup. no_console_suspend doesn't suspend console but doesn't mean it can wake up. I expected this to come up :) IIUC, it was added to help in debugging the suspend path. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/2] ARM: dts: fix gpio-keys wakeup-source property
Hi Arnd, On 15/10/15 15:57, Linus Walleij wrote: On Tue, Oct 13, 2015 at 3:32 PM, Sudeep Holla wrote: The keyboard driver for GPIO buttons(gpio-keys) checks for one of the two boolean properties to enable gpio buttons as wakeup source: 1. "wakeup-source" or 2. the legacy "gpio-key,wakeup" However juno, ste-snowball and emev2-kzm9d dts file have a undetected "wakeup" property to indicate the wakeup source. This patch fixes it by making use of "wakeup-source" property. Cc: Simon Horman Cc: Magnus Damm Cc: Linus Walleij Signed-off-by: Sudeep Holla OK nice, Reviewed-by: Linus Walleij Can you pick this up as fix for v4.3 with Ack from Simon and Linus Walleij ? I will repost 2/2 as a part of bigger cleanup for v4.4 -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 1/2] ACPI / tables: simplify acpi_parse_entries
Hi Rafael, On 01/10/15 16:11, Sudeep Holla wrote: acpi_parse_entries passes the table end pointer to the sub-table entry handler. acpi_parse_entries itself could validate the end of an entry against the table end using the length in the sub-table entry. This patch adds the validation of the sub-table entry end using the length field.This will help to eliminate the need to pass the table end to the handlers. It also moves the check for zero length entry early so that execution of the handler can be avoided. Cc: "Rafael J. Wysocki" Now that Al's MADT is queued, can you consider applying these ? -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2] ARM: dts: fix gpio-keys wakeup-source property
The keyboard driver for GPIO buttons(gpio-keys) checks for one of the two boolean properties to enable gpio buttons as wakeup source: 1. "wakeup-source" or 2. the legacy "gpio-key,wakeup" However juno, ste-snowball and emev2-kzm9d dts file have a undetected "wakeup" property to indictate the wakeup source. This patch fixes it by making use of "wakeup-source" property. Cc: Magnus Damm Acked-by: Simon Horman Reviewed-by: Linus Walleij Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/emev2-kzm9d.dts | 8 arch/arm/boot/dts/ste-snowball.dts| 10 +- arch/arm64/boot/dts/arm/juno-motherboard.dtsi | 12 ++-- 3 files changed, 15 insertions(+), 15 deletions(-) Hi ARM-SoC group, Please consider this as a fix for v4.3 if possible. v2->v1[1] : - included all the reviewed/ack tags, also posting this fix independently Regards, Sudeep [1] https://lkml.org/lkml/2015/10/13/413 diff --git a/arch/arm/boot/dts/emev2-kzm9d.dts b/arch/arm/boot/dts/emev2-kzm9d.dts index 955c24ee4a8c..8c24975e8f9d 100644 --- a/arch/arm/boot/dts/emev2-kzm9d.dts +++ b/arch/arm/boot/dts/emev2-kzm9d.dts @@ -35,28 +35,28 @@ button@1 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; label = "DSW2-1"; linux,code = ; gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>; }; button@2 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; label = "DSW2-2"; linux,code = ; gpios = <&gpio0 15 GPIO_ACTIVE_HIGH>; }; button@3 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; label = "DSW2-3"; linux,code = ; gpios = <&gpio0 16 GPIO_ACTIVE_HIGH>; }; button@4 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; label = "DSW2-4"; linux,code = ; gpios = <&gpio0 17 GPIO_ACTIVE_HIGH>; diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts index 32a5ccb14e7e..e80e42163883 100644 --- a/arch/arm/boot/dts/ste-snowball.dts +++ b/arch/arm/boot/dts/ste-snowball.dts @@ -47,35 +47,35 @@ button@1 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <2>; label = "userpb"; gpios = <&gpio1 0 0x4>; }; button@2 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <3>; label = "extkb1"; gpios = <&gpio4 23 0x4>; }; button@3 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <4>; label = "extkb2"; gpios = <&gpio4 24 0x4>; }; button@4 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <5>; label = "extkb3"; gpios = <&gpio5 1 0x4>; }; button@5 { debounce_interval = <50>; - wakeup = <1>; + wakeup-source; linux,code = <6>; label = "extkb4"; gpios = <&gpio5 2 0x4>; diff --git a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi index 902f14a8e85b..42153860b41f 100644 --- a/arch/arm64/boot/dts/arm/juno-motherboard.dtsi +++ b/arch/arm64/boot/dts/arm/juno-motherboard.dtsi @@ -61,42 +61,42 @@ button@1 { debounce_interval = <50>; - wakeup = <1>; +
Re: [PATCH v3 2/2] ARM: dts: bcm5301x: Add BCM SVK DT files
On 15/10/15 23:24, Jon Mason wrote: Add device tree files for Broadcom Northstar based SVKs. Since the bcm5301x.dtsi already exists, all that is necessary is the dts files to enable the UARTs. With these files, the SVKs are able to boot to shell. Signed-off-by: Jon Mason [..] diff --git a/arch/arm/boot/dts/bcm94708.dts b/arch/arm/boot/dts/bcm94708.dts new file mode 100644 index 000..49682d6 --- /dev/null +++ b/arch/arm/boot/dts/bcm94708.dts @@ -0,0 +1,56 @@ [..] +/dts-v1/; + +#include "bcm4708.dtsi" + +/ { + model = "NorthStar SVK (BCM94708)"; + compatible = "brcm,bcm94708", "brcm,bcm4708"; + + aliases { + serial0 = &uart0; + }; + + chosen { + bootargs = "console=ttyS0,115200"; You can set stdout-path instead which even works for earlycon -- Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 09/19] ARM: dts: am335x: replace gpio-key,wakeup with wakeup-source property
Though the keyboard driver for GPIO buttons(gpio-keys) will continue to check for/support the legacy "gpio-key,wakeup" boolean property to enable gpio buttons as wakeup source, "wakeup-source" is the new standard binding. This patch replaces the legacy "gpio-key,wakeup" with the unified "wakeup-source" property in order to avoid any futher copy-paste duplication. Cc: "Benoît Cousson" Cc: Tony Lindgren Cc: linux-o...@vger.kernel.org Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/am335x-evm.dts| 4 ++-- arch/arm/boot/dts/am335x-evmsk.dts | 2 +- arch/arm/boot/dts/am335x-pepper.dts | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts index 1942a5c8132d..a6e398a213ec 100644 --- a/arch/arm/boot/dts/am335x-evm.dts +++ b/arch/arm/boot/dts/am335x-evm.dts @@ -83,14 +83,14 @@ label = "volume-up"; linux,code = <115>; gpios = <&gpio0 2 GPIO_ACTIVE_LOW>; - gpio-key,wakeup; + wakeup-source; }; switch@10 { label = "volume-down"; linux,code = <114>; gpios = <&gpio0 3 GPIO_ACTIVE_LOW>; - gpio-key,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/am335x-evmsk.dts b/arch/arm/boot/dts/am335x-evmsk.dts index 315bb02c9920..6b57baaa2bb0 100644 --- a/arch/arm/boot/dts/am335x-evmsk.dts +++ b/arch/arm/boot/dts/am335x-evmsk.dts @@ -123,7 +123,7 @@ label = "button2"; linux,code = <0x102>; gpios = <&gpio0 30 GPIO_ACTIVE_HIGH>; - gpio-key,wakeup; + wakeup-source; }; switch@4 { diff --git a/arch/arm/boot/dts/am335x-pepper.dts b/arch/arm/boot/dts/am335x-pepper.dts index 7106114c7464..5107dbb07a7d 100644 --- a/arch/arm/boot/dts/am335x-pepper.dts +++ b/arch/arm/boot/dts/am335x-pepper.dts @@ -627,21 +627,21 @@ label = "home"; linux,code = ; gpios = <&gpio1 22 GPIO_ACTIVE_LOW>; - gpio-key,wakeup; + wakeup-source; }; button@1 { label = "menu"; linux,code = ; gpios = <&gpio1 23 GPIO_ACTIVE_LOW>; - gpio-key,wakeup; + wakeup-source; }; buttons@2 { label = "power"; linux,code = ; gpios = <&gpio0 7 GPIO_ACTIVE_LOW>; - gpio-key,wakeup; + wakeup-source; }; }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 18/19] ARM: dts: ste: replace legacy *,wakeup property with wakeup-source
Though the keyboard and other driver will continue to support the legacy "gpio-key,wakeup", "linux,wakeup" boolean property to enable the wakeup source, "wakeup-source" is the new standard binding. This patch replaces all the legacy wakeup properties with the unified "wakeup-source" property in order to avoid any futher copy-paste duplication. Cc: Linus Walleij Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/ste-href-tvk1281618.dtsi | 2 +- arch/arm/boot/dts/ste-nomadik-s8815.dts| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/ste-href-tvk1281618.dtsi b/arch/arm/boot/dts/ste-href-tvk1281618.dtsi index 0e1c96943d47..3228119f91d1 100644 --- a/arch/arm/boot/dts/ste-href-tvk1281618.dtsi +++ b/arch/arm/boot/dts/ste-href-tvk1281618.dtsi @@ -66,7 +66,7 @@ keypad,num-columns = <8>; keypad,num-rows = <8>; linux,no-autorepeat; - linux,wakeup; + wakeup-source; linux,keymap = <0x0301006b 0x04010066 0x06040072 diff --git a/arch/arm/boot/dts/ste-nomadik-s8815.dts b/arch/arm/boot/dts/ste-nomadik-s8815.dts index 35282c0105c6..789329030658 100644 --- a/arch/arm/boot/dts/ste-nomadik-s8815.dts +++ b/arch/arm/boot/dts/ste-nomadik-s8815.dts @@ -163,7 +163,7 @@ label = "user_button"; gpios = <&gpio0 3 0x1>; linux,code = <1>; /* KEY_ESC */ - gpio-key,wakeup; + wakeup-source; pinctrl-names = "default"; pinctrl-0 = <&user_button_default_mode>; }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 19/19] ARM: dts: zynq: replace gpio-key,wakeup with wakeup-source property
Though the keyboard driver for GPIO buttons(gpio-keys) will continue to check for/support the legacy "gpio-key,wakeup" boolean property to enable gpio buttons as wakeup source, "wakeup-source" is the new standard binding. This patch replaces the legacy "gpio-key,wakeup" with the unified "wakeup-source" property in order to avoid any futher copy-paste duplication. Cc: Michal Simek Cc: "Sören Brinkmann" Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/zynq-zc702.dts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/zynq-zc702.dts b/arch/arm/boot/dts/zynq-zc702.dts index 5df8f81f4217..cb64209bca08 100644 --- a/arch/arm/boot/dts/zynq-zc702.dts +++ b/arch/arm/boot/dts/zynq-zc702.dts @@ -43,14 +43,14 @@ label = "sw14"; gpios = <&gpio0 12 0>; linux,code = <108>; /* down */ - gpio-key,wakeup; + wakeup-source; autorepeat; }; sw13 { label = "sw13"; gpios = <&gpio0 14 0>; linux,code = <103>; /* up */ - gpio-key,wakeup; + wakeup-source; autorepeat; }; }; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 13/19] ARM: dts: tegra: replace legacy *,wakeup property with wakeup-source
Though the keyboard and other driver will continue to support the legacy "gpio-key,wakeup", "nvidia,wakeup-source" boolean property to enable the wakeup source, "wakeup-source" is the new standard binding. This patch replaces all the legacy wakeup properties with the unified "wakeup-source" property in order to avoid any futher copy-paste duplication. Cc: Stephen Warren Cc: Thierry Reding Cc: Alexandre Courbot Cc: linux-te...@vger.kernel.org Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/tegra114-dalmore.dts| 2 +- arch/arm/boot/dts/tegra114-roth.dts | 2 +- arch/arm/boot/dts/tegra114-tn7.dts| 2 +- arch/arm/boot/dts/tegra124-jetson-tk1.dts | 2 +- arch/arm/boot/dts/tegra124-nyan.dtsi | 4 ++-- arch/arm/boot/dts/tegra124-venice2.dts| 2 +- arch/arm/boot/dts/tegra20-harmony.dts | 2 +- arch/arm/boot/dts/tegra20-paz00.dts | 2 +- arch/arm/boot/dts/tegra20-seaboard.dts| 4 ++-- arch/arm/boot/dts/tegra20-trimslice.dts | 2 +- arch/arm/boot/dts/tegra20-ventana.dts | 2 +- arch/arm/boot/dts/tegra20-whistler.dts| 2 +- arch/arm/boot/dts/tegra30-apalis-eval.dts | 2 +- arch/arm/boot/dts/tegra30-cardhu.dtsi | 2 +- arch/arm/boot/dts/tegra30-colibri-eval-v3.dts | 2 +- 15 files changed, 17 insertions(+), 17 deletions(-) diff --git a/arch/arm/boot/dts/tegra114-dalmore.dts b/arch/arm/boot/dts/tegra114-dalmore.dts index 8b7aa0dcdc6e..40b88051ea3e 100644 --- a/arch/arm/boot/dts/tegra114-dalmore.dts +++ b/arch/arm/boot/dts/tegra114-dalmore.dts @@ -1164,7 +1164,7 @@ label = "Power"; gpios = <&gpio TEGRA_GPIO(Q, 0) GPIO_ACTIVE_LOW>; linux,code = ; - gpio-key,wakeup; + wakeup-source; }; volume_down { diff --git a/arch/arm/boot/dts/tegra114-roth.dts b/arch/arm/boot/dts/tegra114-roth.dts index 38acf78d7815..9d868af97b8e 100644 --- a/arch/arm/boot/dts/tegra114-roth.dts +++ b/arch/arm/boot/dts/tegra114-roth.dts @@ -1047,7 +1047,7 @@ label = "Power"; gpios = <&gpio TEGRA_GPIO(Q, 0) GPIO_ACTIVE_LOW>; linux,code = ; - gpio-key,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/tegra114-tn7.dts b/arch/arm/boot/dts/tegra114-tn7.dts index f91c2c9b2f94..89047edb5c5f 100644 --- a/arch/arm/boot/dts/tegra114-tn7.dts +++ b/arch/arm/boot/dts/tegra114-tn7.dts @@ -292,7 +292,7 @@ label = "Power"; gpios = <&gpio TEGRA_GPIO(Q, 0) GPIO_ACTIVE_LOW>; linux,code = ; - gpio-key,wakeup; + wakeup-source; }; volume_down { diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts index 66b4451eb2ca..067190532f78 100644 --- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts +++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts @@ -1761,7 +1761,7 @@ gpios = <&gpio TEGRA_GPIO(Q, 0) GPIO_ACTIVE_LOW>; linux,code = ; debounce-interval = <10>; - gpio-key,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi index a9aec23e06f2..c7af943478b1 100644 --- a/arch/arm/boot/dts/tegra124-nyan.dtsi +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi @@ -496,7 +496,7 @@ linux,input-type = <5>; linux,code = ; debounce-interval = <1>; - gpio-key,wakeup; + wakeup-source; }; power { @@ -504,7 +504,7 @@ gpios = <&gpio TEGRA_GPIO(Q, 0) GPIO_ACTIVE_LOW>; linux,code = ; debounce-interval = <30>; - gpio-key,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/tegra124-venice2.dts b/arch/arm/boot/dts/tegra124-venice2.dts index cfbdf429b45d..4a5f23c5d893 100644 --- a/arch/arm/boot/dts/tegra124-venice2.dts +++ b/arch/arm/boot/dts/tegra124-venice2.dts @@ -975,7 +975,7 @@ gpios = <&gpio TEGRA_GPIO(Q, 0) GPIO_ACTIVE_LOW>; linux,code = ; debounce-interval = <10>; - gpio-key,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/tegra20-harmony.dts b/arch/arm/boot/dts/tegr
[PATCH 16/19] ARM: dts: s5pv210: replace legacy *,wakeup property with wakeup-source
Though the keyboard and other driver will continue to support the legacy "gpio-key,wakeup", "linux,input-wakeup" boolean property to enable the wakeup source, "wakeup-source" is the new standard binding. This patch replaces all the legacy wakeup properties with the unified "wakeup-source" property in order to avoid any futher copy-paste duplication. Cc: Marek Szyprowski Cc: Kukjin Kim Cc: linux-samsung-...@vger.kernel.org Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/s5pv210-aquila.dts | 4 ++-- arch/arm/boot/dts/s5pv210-goni.dts | 4 ++-- arch/arm/boot/dts/s5pv210-smdkv210.dts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/arm/boot/dts/s5pv210-aquila.dts b/arch/arm/boot/dts/s5pv210-aquila.dts index f00cea7aca2f..72f162e7040b 100644 --- a/arch/arm/boot/dts/s5pv210-aquila.dts +++ b/arch/arm/boot/dts/s5pv210-aquila.dts @@ -257,7 +257,7 @@ linux,code = ; label = "power"; debounce-interval = <1>; - gpio-key,wakeup; + wakeup-source; }; }; }; @@ -268,7 +268,7 @@ &keypad { linux,input-no-autorepeat; - linux,input-wakeup; + wakeup-source; samsung,keypad-num-rows = <3>; samsung,keypad-num-columns = <3>; pinctrl-names = "default"; diff --git a/arch/arm/boot/dts/s5pv210-goni.dts b/arch/arm/boot/dts/s5pv210-goni.dts index a3d4643b202e..f4d0a023c31c 100644 --- a/arch/arm/boot/dts/s5pv210-goni.dts +++ b/arch/arm/boot/dts/s5pv210-goni.dts @@ -239,7 +239,7 @@ linux,code = ; label = "power"; debounce-interval = <1>; - gpio-key,wakeup; + wakeup-source; }; }; }; @@ -250,7 +250,7 @@ &keypad { linux,input-no-autorepeat; - linux,input-wakeup; + wakeup-source; samsung,keypad-num-rows = <3>; samsung,keypad-num-columns = <3>; pinctrl-names = "default"; diff --git a/arch/arm/boot/dts/s5pv210-smdkv210.dts b/arch/arm/boot/dts/s5pv210-smdkv210.dts index da7d210df670..54fcc3fc82e2 100644 --- a/arch/arm/boot/dts/s5pv210-smdkv210.dts +++ b/arch/arm/boot/dts/s5pv210-smdkv210.dts @@ -59,7 +59,7 @@ &keypad { linux,input-no-autorepeat; - linux,input-wakeup; + wakeup-source; samsung,keypad-num-rows = <8>; samsung,keypad-num-columns = <8>; pinctrl-names = "default"; -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 12/19] ARM: dts: exynos: replace legacy *,wakeup property with wakeup-source
Though the keyboard and other driver will continue to support the legacy "gpio-key,wakeup", "linux-keypad,wakeup" boolean property to enable the wakeup source, "wakeup-source" is the new standard binding. This patch replaces all the legacy wakeup properties with the unified "wakeup-source" property in order to avoid any futher copy-paste duplication. Cc: Kukjin Kim Cc: Krzysztof Kozlowski Cc: linux-samsung-...@vger.kernel.org Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/exynos3250-monk.dts | 6 +++--- arch/arm/boot/dts/exynos3250-rinato.dts | 6 +++--- arch/arm/boot/dts/exynos4210-origen.dts | 10 +- arch/arm/boot/dts/exynos4210-smdkv310.dts | 2 +- arch/arm/boot/dts/exynos4210-trats.dts | 2 +- arch/arm/boot/dts/exynos4210-universal_c210.dts | 4 ++-- arch/arm/boot/dts/exynos4412-odroid-common.dtsi | 2 +- arch/arm/boot/dts/exynos4412-odroidx.dts| 2 +- arch/arm/boot/dts/exynos4412-origen.dts | 2 +- arch/arm/boot/dts/exynos4412-smdk4412.dts | 2 +- arch/arm/boot/dts/exynos4412-trats2.dts | 4 ++-- arch/arm/boot/dts/exynos5250-arndale.dts| 12 ++-- arch/arm/boot/dts/exynos5250-snow.dts | 4 ++-- arch/arm/boot/dts/exynos5250-spring.dts | 4 ++-- arch/arm/boot/dts/exynos5420-arndale-octa.dts | 2 +- arch/arm/boot/dts/exynos5420-peach-pit.dts | 4 ++-- arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 ++-- 17 files changed, 36 insertions(+), 36 deletions(-) diff --git a/arch/arm/boot/dts/exynos3250-monk.dts b/arch/arm/boot/dts/exynos3250-monk.dts index 540a0adf2be6..b77ea2163a0a 100644 --- a/arch/arm/boot/dts/exynos3250-monk.dts +++ b/arch/arm/boot/dts/exynos3250-monk.dts @@ -43,7 +43,7 @@ linux,code = ; label = "power key"; debounce-interval = <10>; - gpio-key,wakeup; + wakeup-source; }; }; @@ -67,7 +67,7 @@ interrupt-parent = <&gpx1>; interrupts = <5 0>; reg = <0x25>; - wakeup; + wakeup-source; muic: max77836-muic { compatible = "maxim,max77836-muic"; @@ -184,7 +184,7 @@ interrupt-parent = <&gpx0>; interrupts = <7 0>; reg = <0x66>; - wakeup; + wakeup-source; s2mps14_osc: clocks { compatible = "samsung,s2mps14-clk"; diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts b/arch/arm/boot/dts/exynos3250-rinato.dts index 41a5fafb9aa9..7acd11d2d957 100644 --- a/arch/arm/boot/dts/exynos3250-rinato.dts +++ b/arch/arm/boot/dts/exynos3250-rinato.dts @@ -43,7 +43,7 @@ linux,code = ; label = "power key"; debounce-interval = <10>; - gpio-key,wakeup; + wakeup-source; }; }; @@ -58,7 +58,7 @@ interrupt-parent = <&gpx1>; interrupts = <5 0>; reg = <0x25>; - wakeup; + wakeup-source; muic: max77836-muic { compatible = "maxim,max77836-muic"; @@ -245,7 +245,7 @@ interrupt-parent = <&gpx0>; interrupts = <7 0>; reg = <0x66>; - wakeup; + wakeup-source; s2mps14_osc: clocks { compatible = "samsung,s2mps14-clk"; diff --git a/arch/arm/boot/dts/exynos4210-origen.dts b/arch/arm/boot/dts/exynos4210-origen.dts index e050d85cdacd..9e97f6065493 100644 --- a/arch/arm/boot/dts/exynos4210-origen.dts +++ b/arch/arm/boot/dts/exynos4210-origen.dts @@ -59,35 +59,35 @@ label = "Up"; gpios = <&gpx2 0 1>; linux,code = ; - gpio-key,wakeup; + wakeup-source; }; down { label = "Down"; gpios = <&gpx2 1 1>; linux,code = ; - gpio-key,wakeup; + wakeup-source; }; back { label = "Back"; gpios = <&gpx1 7 1>; linux,code = ; - gpio-key,wakeup; + wakeup-source; };
[PATCH 17/19] ARM: dts: armada: replace isil,irq2-can-wakeup-machine with wakeup-source property
Though the driver will continue to check for and support the legacy "isil,irq2-can-wakeup-machine" boolean property to wakeup source, "wakeup-source" is the new standard binding. This patch replaces the legacy "isil,irq2-can-wakeup-machine" with the unified "wakeup-source" property in order to avoid any futher copy-paste duplication. Cc: Jason Cooper Cc: Andrew Lunn Cc: Gregory Clement Cc: Sebastian Hesselbarth Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/armada-370-netgear-rn102.dts | 2 +- arch/arm/boot/dts/armada-370-netgear-rn104.dts | 2 +- arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/arm/boot/dts/armada-370-netgear-rn102.dts b/arch/arm/boot/dts/armada-370-netgear-rn102.dts index a31207860f34..6012c8ff778e 100644 --- a/arch/arm/boot/dts/armada-370-netgear-rn102.dts +++ b/arch/arm/boot/dts/armada-370-netgear-rn102.dts @@ -120,7 +120,7 @@ isl12057: isl12057@68 { compatible = "isil,isl12057"; reg = <0x68>; - isil,irq2-can-wakeup-machine; + wakeup-source; }; g762: g762@3e { diff --git a/arch/arm/boot/dts/armada-370-netgear-rn104.dts b/arch/arm/boot/dts/armada-370-netgear-rn104.dts index 00540f292979..7c5e8041cf48 100644 --- a/arch/arm/boot/dts/armada-370-netgear-rn104.dts +++ b/arch/arm/boot/dts/armada-370-netgear-rn104.dts @@ -126,7 +126,7 @@ isl12057: isl12057@68 { compatible = "isil,isl12057"; reg = <0x68>; - isil,irq2-can-wakeup-machine; + wakeup-source; }; g762: g762@3e { diff --git a/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts b/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts index 1516fc2627f9..daf88ede1192 100644 --- a/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts +++ b/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts @@ -133,7 +133,7 @@ isl12057: isl12057@68 { compatible = "isil,isl12057"; reg = <0x68>; - isil,irq2-can-wakeup-machine; + wakeup-source; }; /* Controller for rear fan #1 of 3 (Protechnic -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 15/19] ARM: dts: omap: replace legacy *,wakeup property with wakeup-source
Though the keyboard and other driver will continue to support the legacy "gpio-key,wakeup", "linux,wakeup" boolean property to enable the wakeup source, "wakeup-source" is the new standard binding. This patch replaces all the legacy wakeup properties with the unified "wakeup-source" property in order to avoid any futher copy-paste duplication. Cc: "Benoît Cousson" Cc: Tony Lindgren Cc: linux-o...@vger.kernel.org Signed-off-by: Sudeep Holla --- arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts | 8 ++--- arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +- arch/arm/boot/dts/omap3-beagle.dts | 2 +- arch/arm/boot/dts/omap3-cm-t3x.dtsi| 2 +- arch/arm/boot/dts/omap3-devkit8000-common.dtsi | 2 +- arch/arm/boot/dts/omap3-devkit8000-lcd-common.dtsi | 2 +- arch/arm/boot/dts/omap3-gta04.dtsi | 2 +- arch/arm/boot/dts/omap3-ldp.dts| 18 ++-- arch/arm/boot/dts/omap3-lilly-a83x.dtsi| 2 +- arch/arm/boot/dts/omap3-n900.dts | 10 +++ arch/arm/boot/dts/omap3-overo-alto35-common.dtsi | 2 +- .../boot/dts/omap3-overo-chestnut43-common.dtsi| 4 +-- arch/arm/boot/dts/omap3-overo-common-lcd35.dtsi| 2 +- arch/arm/boot/dts/omap3-overo-common-lcd43.dtsi| 2 +- arch/arm/boot/dts/omap3-overo-gallop43-common.dtsi | 4 +-- arch/arm/boot/dts/omap3-overo-palo35-common.dtsi | 4 +-- arch/arm/boot/dts/omap3-overo-palo43-common.dtsi | 4 +-- arch/arm/boot/dts/omap3-pandora-common.dtsi| 34 +++--- .../boot/dts/omap3-panel-sharp-ls037v7dw01.dtsi| 2 +- arch/arm/boot/dts/omap4-duovero-parlor.dts | 2 +- arch/arm/boot/dts/omap4-var-om44customboard.dtsi | 2 +- arch/arm/boot/dts/omap5-cm-t54.dts | 2 +- 22 files changed, 57 insertions(+), 57 deletions(-) diff --git a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts index 91146c318798..caeb3b076042 100644 --- a/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts +++ b/arch/arm/boot/dts/logicpd-torpedo-37xx-devkit.dts @@ -23,28 +23,28 @@ label = "sysboot2"; gpios = <&gpio1 2 GPIO_ACTIVE_LOW>; /* gpio2 */ linux,code = ; - gpio-key,wakeup; + wakeup-source; }; sysboot5 { label = "sysboot5"; gpios = <&gpio1 7 GPIO_ACTIVE_LOW>; /* gpio7 */ linux,code = ; - gpio-key,wakeup; + wakeup-source; }; gpio1 { label = "gpio1"; gpios = <&gpio6 21 GPIO_ACTIVE_LOW>;/* gpio181 */ linux,code = ; - gpio-key,wakeup; + wakeup-source; }; gpio2 { label = "gpio2"; gpios = <&gpio6 18 GPIO_ACTIVE_LOW>;/* gpio178 */ linux,code = ; - gpio-key,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts index 7c4dca122a91..f0e2cf3c9e89 100644 --- a/arch/arm/boot/dts/omap3-beagle-xm.dts +++ b/arch/arm/boot/dts/omap3-beagle-xm.dts @@ -69,7 +69,7 @@ label = "user"; gpios = <&gpio1 4 GPIO_ACTIVE_HIGH>; linux,code = <0x114>; - gpio-key,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts index 67659a0ed13e..de5138e61ddc 100644 --- a/arch/arm/boot/dts/omap3-beagle.dts +++ b/arch/arm/boot/dts/omap3-beagle.dts @@ -80,7 +80,7 @@ label = "user"; gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>; linux,code = <0x114>; - gpio-key,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/omap3-cm-t3x.dtsi b/arch/arm/boot/dts/omap3-cm-t3x.dtsi index 4d091ca43e25..5ec6491d7933 100644 --- a/arch/arm/boot/dts/omap3-cm-t3x.dtsi +++ b/arch/arm/boot/dts/omap3-cm-t3x.dtsi @@ -238,7 +238,7 @@ ti,debounce-tol = /bits/ 16 <10>; ti,debounce-rep = /bits/ 16 <1>; - linux,wakeup; + wakeup-source; }; }; diff --git a/arch/arm/boot/dts/omap3-devkit8000-common.dtsi b/arch/arm/boot/dts/omap3-devkit8000-common.dtsi index 9