linux-kernel - Re: [PATCH v9 6/7] mfd: cros_ec: differentiate SCP from EC by feature bit.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6a6819ab-f6f6-616c-7d07-a94dddbe1262@collabora.com>
Date:   Fri, 31 May 2019 10:54:45 +0200
From:   Enric Balletbo i Serra <enric.balletbo@...labora.com>
To:     Pi-Hsun Shih <pihsun@...omium.org>
Cc:     Lee Jones <lee.jones@...aro.org>,
        Benson Leung <bleung@...omium.org>,
        Guenter Roeck <groeck@...omium.org>,
        open list <linux-kernel@...r.kernel.org>,
        Gwendal Grignou <gwendal@...omium.org>
Subject: Re: [PATCH v9 6/7] mfd: cros_ec: differentiate SCP from EC by feature
 bit.

Hi,

On 31/5/19 9:38, Pi-Hsun Shih wrote:
> System Companion Processor (SCP) is Cortex M4 co-processor on some
> MediaTek platform that can run EC-style firmware. Since a SCP and EC
> would both exist on a system, and use the cros_ec_dev driver, we need to
> differentiate between them for the userspace, or they would both be
> registered at /dev/cros_ec, causing a conflict.
> 
> Signed-off-by: Pi-Hsun Shih <pihsun@...omium.org>

I think I already did, but anyway,

Acked-by: Enric Balletbo i Serra <enric.balletbo@...labora.com>


> 
> ---
> Changes from v8:
>  - No change.
> 
> Changes from v7:
>  - Address comments in v7.
>  - Rebase the series onto https://lore.kernel.org/patchwork/patch/1059196/.
> 
> Changes from v6, v5, v4, v3, v2:
>  - No change.
> 
> Changes from v1:
>  - New patch extracted from Patch 5.
> ---
>  drivers/mfd/cros_ec_dev.c            | 10 ++++++++++
>  include/linux/mfd/cros_ec.h          |  1 +
>  include/linux/mfd/cros_ec_commands.h |  2 +-
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index a5391f96eafd..66107de3dbce 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -440,6 +440,16 @@ static int ec_device_probe(struct platform_device *pdev)
>  		ec_platform->ec_name = CROS_EC_DEV_TP_NAME;
>  	}
>  
> +	/* Check whether this is actually a SCP rather than an EC. */
> +	if (cros_ec_check_features(ec, EC_FEATURE_SCP)) {
> +		dev_info(dev, "CrOS SCP MCU detected.\n");
> +		/*
> +		 * Help userspace differentiating ECs from SCP,
> +		 * regardless of the probing order.
> +		 */
> +		ec_platform->ec_name = CROS_EC_DEV_SCP_NAME;
> +	}
> +
>  	/*
>  	 * Add the class device
>  	 * Link to the character device for creating the /dev entry
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index cfa78bb4990f..751cb3756d49 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -27,6 +27,7 @@
>  #define CROS_EC_DEV_PD_NAME "cros_pd"
>  #define CROS_EC_DEV_TP_NAME "cros_tp"
>  #define CROS_EC_DEV_ISH_NAME "cros_ish"
> +#define CROS_EC_DEV_SCP_NAME "cros_scp"
>  
>  /*
>   * The EC is unresponsive for a time after a reboot command.  Add a
> diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
> index dcec96f01879..8b578b4c1ec7 100644
> --- a/include/linux/mfd/cros_ec_commands.h
> +++ b/include/linux/mfd/cros_ec_commands.h
> @@ -884,7 +884,7 @@ enum ec_feature_code {
>  	EC_FEATURE_REFINED_TABLET_MODE_HYSTERESIS = 37,
>  	/* EC supports audio codec. */
>  	EC_FEATURE_AUDIO_CODEC = 38,
> -	/* EC Supports SCP. */
> +	/* The MCU is a System Companion Processor (SCP). */

This change is already done on the Gwendal's efforts to sync the commands file,
to avoid more mess here, please remove that change and resend the patchset
without touching the cros_ec_commands.h file.

[1] https://lore.kernel.org/lkml/20190518063949.GY4319@dell/T/

Thanks,
 Enric

>  	EC_FEATURE_SCP = 39,
>  	/* The MCU is an Integrated Sensor Hub */
>  	EC_FEATURE_ISH = 40,
>