From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2C9F0C433DB for ; Thu, 7 Jan 2021 16:03:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id ED4B623403 for ; Thu, 7 Jan 2021 16:03:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728455AbhAGQDd (ORCPT ); Thu, 7 Jan 2021 11:03:33 -0500 Received: from mail.kernel.org ([198.145.29.99]:38472 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728372AbhAGQDc (ORCPT ); Thu, 7 Jan 2021 11:03:32 -0500 Received: by mail.kernel.org (Postfix) with ESMTPSA id 4ACD523118; Thu, 7 Jan 2021 16:02:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1610035371; bh=sq1hZodw++8sOiYwpRpFRWnOK/gGWirm92MZoS+MlSc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=q8T3ZkD0iERIVRyeAFdIrqI+Gjhoy31JlG63lJ7goxQwX8pkZedu7zB4RBgByen3Z 38o/dnRgTpztmtPu7K9O7o8p88AMcQZwiXWOk+K+DEcuHEJsU0A+n3bqMwKeQYRpk/ hfXv+RATx5uYyMlTEHbwhLBKqSD0lvbqZ8f+1M04= Date: Thu, 7 Jan 2021 17:04:11 +0100 From: Greg KH To: Cristian Marussi Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, sudeep.holla@arm.com, lukasz.luba@arm.com, Jonathan.Cameron@huawei.com, arnd@arndb.de, robh@kernel.org Subject: Re: [PATCH v4 2/2] firmware: arm_scmi: Add SCMI System Power Control driver Message-ID: References: <20210105130945.8192-1-cristian.marussi@arm.com> <20210105130945.8192-2-cristian.marussi@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210105130945.8192-2-cristian.marussi@arm.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 05, 2021 at 01:09:45PM +0000, Cristian Marussi wrote: > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig > index 3f14dffb9669..2b39453e9dd1 100644 > --- a/drivers/firmware/Kconfig > +++ b/drivers/firmware/Kconfig > @@ -40,6 +40,18 @@ config ARM_SCMI_POWER_DOMAIN > will be called scmi_pm_domain. Note this may needed early in boot > before rootfs may be available. > > +config ARM_SCMI_POWER_CONTROL > + bool "SCMI system power control driver" > + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) > + default n n is always the default, no need to list it here. > + help > + This enables System Power control logic which binds system shutdown or > + reboot actions to SCMI System Power notifications generated by SCP > + firmware. > + > + Graceful requests' methods and timeout and can be configured using > + a few available module parameters. > + > 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 6a2ef63306d6..ddddb4636ebd 100644 > --- a/drivers/firmware/arm_scmi/Makefile > +++ b/drivers/firmware/arm_scmi/Makefile > @@ -9,3 +9,4 @@ scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \ > $(scmi-transport-y) > obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o > obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o > +obj-$(CONFIG_ARM_SCMI_POWER_CONTROL) += scmi_power_control.o > diff --git a/drivers/firmware/arm_scmi/scmi_power_control.c b/drivers/firmware/arm_scmi/scmi_power_control.c > new file mode 100644 > index 000000000000..216c2f031111 > --- /dev/null > +++ b/drivers/firmware/arm_scmi/scmi_power_control.c > @@ -0,0 +1,359 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SCMI Generic SystemPower Control driver. > + * > + * Copyright (C) 2020-2021 ARM Ltd. > + */ > +/** > + * DOC: Theory of operation What does "DOC:" provide? Did you tie this into the kernel doc build system? If not, then what is this tag for? > + * > + * In order to handle platform originated SCMI SystemPower requests (like > + * shutdowns or cold/warm resets) we register an SCMI Notification notifier > + * block to react when such SCMI SystemPower events are emitted. > + * > + * Once such a notification is received we act accordingly to perform the > + * required system transition depending on the kind of request. > + * > + * By default graceful requests are routed to userspace through the same > + * API methods (orderly_poweroff/reboot()) used by ACPI when handling ACPI > + * Shutdown bus events: alternatively, by properly setting a couple of module > + * parameters (scmi_graceful_*_signum), it is possible to choose to use signals > + * to CAD pid as a mean to communicate such graceful requests to userspace. > + * > + * Forceful requests, instead, will simply cause an immediate emergency_sync() > + * and subsequent Kernel-only shutdown/reboot. > + * > + * Additionally, setting scmi_graceful_request_tmo_ms to a non-zero value, it > + * is possible to convert a timed-out graceful request to a forceful one. > + * > + * The assumption here is that even graceful requests can be upper-bound by a > + * maximum final timeout strictly enforced by the platform itself which can > + * ultimately cut the power off at will anytime: in order to avoid such extreme > + * scenario, when scmi_graceful_request_tmo_ms is non-zero, we track progress of > + * graceful requests through the means of a reboot notifier converting timed-out > + * graceful requests to forceful ones: in such a way at least we perform a > + * clean sync and shutdown/restart before the power is cut. > + * > + * Given that such platform hard-timeout, if present, is anyway highly platform > + * and event specific and not exposed at run-time by the SCMI SytemPower > + * protocol, the parameter scmi_graceful_request_tmo_ms defaults to zero. > + */ > + > +#define pr_fmt(fmt) "SCMI SystemPower - " fmt drivers should not need this, and even if they do, that's a HUGE prefix, be more terse please. Always use dev_*() calls instead of pr_() as you have access to a device at all times, right? If not, then this isn't a driver :) > +enum scmi_syspower_state { > + SCMI_SYSPOWER_IDLE, > + SCMI_SYSPOWER_IN_PROGRESS, > + SCMI_SYSPOWER_REBOOTING > +}; > + > +static enum scmi_syspower_state scmi_state; > +/* Protect access to scmi_state */ > +static DEFINE_MUTEX(scmi_state_mtx); Why does this only handle "one" state and device? Shouldn't this all be per-device? > + > +static enum scmi_system_events scmi_required_transition = SCMI_SYSTEM_MAX; > + > +static unsigned int scmi_graceful_poweroff_signum; > +static unsigned int scmi_graceful_reboot_signum; > +static unsigned int scmi_graceful_request_tmo_ms; why "unsigned int"? > + > +static struct timer_list scmi_request_timer; > +static struct work_struct scmi_forceful_work; > + > +/** > + * scmi_reboot_notifier - A reboot notifier to catch an ongoing successful > + * system transition > + * @nb: Reference to the related notifier block > + * @reason: The reason for the ongoing reboot > + * @__unused: The cmd being executed on a restart request (unused) > + * > + * When an ongoing system transition is detected, compatible with the one > + * requested by SCMI, cancel the timer work. > + * This is registered only when a valid SCMI SystemPower transition has been > + * received and scmi_graceful_request_tmo_ms was non-zero. > + * > + * Return: NOTIFY_OK in any case > + */ > +static int scmi_reboot_notifier(struct notifier_block *nb, > + unsigned long reason, void *__unused) > +{ > + mutex_lock(&scmi_state_mtx); > + switch (reason) { > + case SYS_HALT: > + case SYS_POWER_OFF: > + if (scmi_required_transition == SCMI_SYSTEM_SHUTDOWN) > + scmi_state = SCMI_SYSPOWER_REBOOTING; > + break; > + case SYS_RESTART: > + if (scmi_required_transition == SCMI_SYSTEM_COLDRESET || > + scmi_required_transition == SCMI_SYSTEM_WARMRESET) > + scmi_state = SCMI_SYSPOWER_REBOOTING; > + break; > + default: > + break; > + } > + > + if (scmi_state == SCMI_SYSPOWER_REBOOTING) { > + pr_debug("Reboot in progress...cancel timer.\n"); use dev_dbg() and pass it your device pointer please. Same for all usages of pr_*() in the driver. > +static int scmi_syspower_probe(struct scmi_device *sdev) > +{ > + int ret; > + struct scmi_handle *handle = sdev->handle; > + struct notifier_block *userspace_nb; > + > + if (!handle) > + return -ENODEV; > + > + userspace_nb = devm_kzalloc(&sdev->dev, sizeof(*userspace_nb), > + GFP_KERNEL); > + if (!userspace_nb) > + return -ENOMEM; > + > + userspace_nb->notifier_call = &scmi_userspace_notifier; > + ret = handle->notify_ops->register_event_notifier(handle, > + SCMI_PROTOCOL_SYSTEM, > + SCMI_EVENT_SYSTEM_POWER_STATE_NOTIFIER, > + NULL, userspace_nb); Crazy indentation, checkpatch was ok with this? > +module_param(scmi_graceful_request_tmo_ms, uint, 0644); > +MODULE_PARM_DESC(scmi_graceful_request_tmo_ms, > + "Maximum time(ms) allowed to userspace for complying to the request. Unlimited if zero."); > + > +module_param(scmi_graceful_poweroff_signum, uint, 0644); > +MODULE_PARM_DESC(scmi_graceful_poweroff_signum, > + "Signal to request graceful poweroff to CAD process. Ignored if zero."); > + > +module_param(scmi_graceful_reboot_signum, uint, 0644); > +MODULE_PARM_DESC(scmi_graceful_reboot_signum, > + "Signal to request graceful reboot to CAD process. Ignored if zero."); This is not the 1990's, please do not add module parameters. Make this "just work", or worst case, sysfs attributes. thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B0068C433E0 for ; Thu, 7 Jan 2021 16:05:22 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7312820857 for ; Thu, 7 Jan 2021 16:05:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7312820857 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=G3C2Cb1LAM3TOpeoXEF4oBXdfJA+WwNeS0K3eIpUEk8=; b=atgIZ1TVzMfaGmodmvpIMcCyB DCTAhxDzpUxej2ftuCRJVFyrNk1VCtI0T5NQJ5t0y3dwOIUF6qf1pgsIqH5azTY7ZElJ5v1BIX0RX lcxNu16CEUSf265T0ztgd0CgccBUzuUZ+DIPWh0Z4p9F1T2/0iZdcjeoGF3BrcS8PHUxwfE2asPyb 96hQlhBetmTFJBPeQHR9o5u5QVs02IVeNk0PJ9tdkNlnPIpQ1/9sH8Cz/QBbWFkVJRh2J41/JfhdK DT5tmWRoY4lzVrYx7LNpjgJEsSI6+fWFvHqIiu31SqjW93pNxx6UiyqP3FLm3gKKiGp06wGYB8Xw5 K6uucp1KQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kxXkX-0004G8-0d; Thu, 07 Jan 2021 16:02:57 +0000 Received: from mail.kernel.org ([198.145.29.99]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kxXkS-0004Eo-ON for linux-arm-kernel@lists.infradead.org; Thu, 07 Jan 2021 16:02:53 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 4ACD523118; Thu, 7 Jan 2021 16:02:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1610035371; bh=sq1hZodw++8sOiYwpRpFRWnOK/gGWirm92MZoS+MlSc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=q8T3ZkD0iERIVRyeAFdIrqI+Gjhoy31JlG63lJ7goxQwX8pkZedu7zB4RBgByen3Z 38o/dnRgTpztmtPu7K9O7o8p88AMcQZwiXWOk+K+DEcuHEJsU0A+n3bqMwKeQYRpk/ hfXv+RATx5uYyMlTEHbwhLBKqSD0lvbqZ8f+1M04= Date: Thu, 7 Jan 2021 17:04:11 +0100 From: Greg KH To: Cristian Marussi Subject: Re: [PATCH v4 2/2] firmware: arm_scmi: Add SCMI System Power Control driver Message-ID: References: <20210105130945.8192-1-cristian.marussi@arm.com> <20210105130945.8192-2-cristian.marussi@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210105130945.8192-2-cristian.marussi@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210107_110252_955148_AD0D6B33 X-CRM114-Status: GOOD ( 33.28 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: arnd@arndb.de, sudeep.holla@arm.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Jonathan.Cameron@huawei.com, lukasz.luba@arm.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Jan 05, 2021 at 01:09:45PM +0000, Cristian Marussi wrote: > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig > index 3f14dffb9669..2b39453e9dd1 100644 > --- a/drivers/firmware/Kconfig > +++ b/drivers/firmware/Kconfig > @@ -40,6 +40,18 @@ config ARM_SCMI_POWER_DOMAIN > will be called scmi_pm_domain. Note this may needed early in boot > before rootfs may be available. > > +config ARM_SCMI_POWER_CONTROL > + bool "SCMI system power control driver" > + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF) > + default n n is always the default, no need to list it here. > + help > + This enables System Power control logic which binds system shutdown or > + reboot actions to SCMI System Power notifications generated by SCP > + firmware. > + > + Graceful requests' methods and timeout and can be configured using > + a few available module parameters. > + > 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 6a2ef63306d6..ddddb4636ebd 100644 > --- a/drivers/firmware/arm_scmi/Makefile > +++ b/drivers/firmware/arm_scmi/Makefile > @@ -9,3 +9,4 @@ scmi-module-objs := $(scmi-bus-y) $(scmi-driver-y) $(scmi-protocols-y) \ > $(scmi-transport-y) > obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-module.o > obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o > +obj-$(CONFIG_ARM_SCMI_POWER_CONTROL) += scmi_power_control.o > diff --git a/drivers/firmware/arm_scmi/scmi_power_control.c b/drivers/firmware/arm_scmi/scmi_power_control.c > new file mode 100644 > index 000000000000..216c2f031111 > --- /dev/null > +++ b/drivers/firmware/arm_scmi/scmi_power_control.c > @@ -0,0 +1,359 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * SCMI Generic SystemPower Control driver. > + * > + * Copyright (C) 2020-2021 ARM Ltd. > + */ > +/** > + * DOC: Theory of operation What does "DOC:" provide? Did you tie this into the kernel doc build system? If not, then what is this tag for? > + * > + * In order to handle platform originated SCMI SystemPower requests (like > + * shutdowns or cold/warm resets) we register an SCMI Notification notifier > + * block to react when such SCMI SystemPower events are emitted. > + * > + * Once such a notification is received we act accordingly to perform the > + * required system transition depending on the kind of request. > + * > + * By default graceful requests are routed to userspace through the same > + * API methods (orderly_poweroff/reboot()) used by ACPI when handling ACPI > + * Shutdown bus events: alternatively, by properly setting a couple of module > + * parameters (scmi_graceful_*_signum), it is possible to choose to use signals > + * to CAD pid as a mean to communicate such graceful requests to userspace. > + * > + * Forceful requests, instead, will simply cause an immediate emergency_sync() > + * and subsequent Kernel-only shutdown/reboot. > + * > + * Additionally, setting scmi_graceful_request_tmo_ms to a non-zero value, it > + * is possible to convert a timed-out graceful request to a forceful one. > + * > + * The assumption here is that even graceful requests can be upper-bound by a > + * maximum final timeout strictly enforced by the platform itself which can > + * ultimately cut the power off at will anytime: in order to avoid such extreme > + * scenario, when scmi_graceful_request_tmo_ms is non-zero, we track progress of > + * graceful requests through the means of a reboot notifier converting timed-out > + * graceful requests to forceful ones: in such a way at least we perform a > + * clean sync and shutdown/restart before the power is cut. > + * > + * Given that such platform hard-timeout, if present, is anyway highly platform > + * and event specific and not exposed at run-time by the SCMI SytemPower > + * protocol, the parameter scmi_graceful_request_tmo_ms defaults to zero. > + */ > + > +#define pr_fmt(fmt) "SCMI SystemPower - " fmt drivers should not need this, and even if they do, that's a HUGE prefix, be more terse please. Always use dev_*() calls instead of pr_() as you have access to a device at all times, right? If not, then this isn't a driver :) > +enum scmi_syspower_state { > + SCMI_SYSPOWER_IDLE, > + SCMI_SYSPOWER_IN_PROGRESS, > + SCMI_SYSPOWER_REBOOTING > +}; > + > +static enum scmi_syspower_state scmi_state; > +/* Protect access to scmi_state */ > +static DEFINE_MUTEX(scmi_state_mtx); Why does this only handle "one" state and device? Shouldn't this all be per-device? > + > +static enum scmi_system_events scmi_required_transition = SCMI_SYSTEM_MAX; > + > +static unsigned int scmi_graceful_poweroff_signum; > +static unsigned int scmi_graceful_reboot_signum; > +static unsigned int scmi_graceful_request_tmo_ms; why "unsigned int"? > + > +static struct timer_list scmi_request_timer; > +static struct work_struct scmi_forceful_work; > + > +/** > + * scmi_reboot_notifier - A reboot notifier to catch an ongoing successful > + * system transition > + * @nb: Reference to the related notifier block > + * @reason: The reason for the ongoing reboot > + * @__unused: The cmd being executed on a restart request (unused) > + * > + * When an ongoing system transition is detected, compatible with the one > + * requested by SCMI, cancel the timer work. > + * This is registered only when a valid SCMI SystemPower transition has been > + * received and scmi_graceful_request_tmo_ms was non-zero. > + * > + * Return: NOTIFY_OK in any case > + */ > +static int scmi_reboot_notifier(struct notifier_block *nb, > + unsigned long reason, void *__unused) > +{ > + mutex_lock(&scmi_state_mtx); > + switch (reason) { > + case SYS_HALT: > + case SYS_POWER_OFF: > + if (scmi_required_transition == SCMI_SYSTEM_SHUTDOWN) > + scmi_state = SCMI_SYSPOWER_REBOOTING; > + break; > + case SYS_RESTART: > + if (scmi_required_transition == SCMI_SYSTEM_COLDRESET || > + scmi_required_transition == SCMI_SYSTEM_WARMRESET) > + scmi_state = SCMI_SYSPOWER_REBOOTING; > + break; > + default: > + break; > + } > + > + if (scmi_state == SCMI_SYSPOWER_REBOOTING) { > + pr_debug("Reboot in progress...cancel timer.\n"); use dev_dbg() and pass it your device pointer please. Same for all usages of pr_*() in the driver. > +static int scmi_syspower_probe(struct scmi_device *sdev) > +{ > + int ret; > + struct scmi_handle *handle = sdev->handle; > + struct notifier_block *userspace_nb; > + > + if (!handle) > + return -ENODEV; > + > + userspace_nb = devm_kzalloc(&sdev->dev, sizeof(*userspace_nb), > + GFP_KERNEL); > + if (!userspace_nb) > + return -ENOMEM; > + > + userspace_nb->notifier_call = &scmi_userspace_notifier; > + ret = handle->notify_ops->register_event_notifier(handle, > + SCMI_PROTOCOL_SYSTEM, > + SCMI_EVENT_SYSTEM_POWER_STATE_NOTIFIER, > + NULL, userspace_nb); Crazy indentation, checkpatch was ok with this? > +module_param(scmi_graceful_request_tmo_ms, uint, 0644); > +MODULE_PARM_DESC(scmi_graceful_request_tmo_ms, > + "Maximum time(ms) allowed to userspace for complying to the request. Unlimited if zero."); > + > +module_param(scmi_graceful_poweroff_signum, uint, 0644); > +MODULE_PARM_DESC(scmi_graceful_poweroff_signum, > + "Signal to request graceful poweroff to CAD process. Ignored if zero."); > + > +module_param(scmi_graceful_reboot_signum, uint, 0644); > +MODULE_PARM_DESC(scmi_graceful_reboot_signum, > + "Signal to request graceful reboot to CAD process. Ignored if zero."); This is not the 1990's, please do not add module parameters. Make this "just work", or worst case, sysfs attributes. thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel