| 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
| ||
|
Date: Mon, 15 Apr 2019 17:06:53 +0200
From: Enric Balletbo i Serra <enric.balletbo@...labora.com>
To: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Rushikesh S Kadam <rushikesh.s.kadam@...el.com>,
Jiri Kosina <jikos@...nel.org>
Cc: Jett Rink <jettrink@...omium.org>,
Benson Leung <bleung@...omium.org>,
Guenter Roeck <groeck@...omium.org>,
Nick Crews <ncrews@...omium.org>,
Gwendal Grignou <gwendal@...gle.com>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] platform: chrome: Add ChromeOS EC ISHTP driver
Hi,
On 12/4/19 17:22, Srinivas Pandruvada wrote:
> On Thu, 2019-04-11 at 15:54 +0200, Enric Balletbo i Serra wrote:
>> Hi,
>>
>> On 11/4/19 13:10, Rushikesh S Kadam wrote:
>>> Hi Enric, Srinivas
>>>
>>> On Thu, Apr 11, 2019 at 12:55:13PM +0200, Enric Balletbo i Serra
>>> wrote:
>>>> Hi,
>>>>
>>>> On 10/4/19 17:31, Jett Rink wrote:
>>>>> Reviewed-by: Jett Rink <jettrink@...omium.org>
>>>>> Tested-by: Jett Rink <jettrink@...omium.org>
>>>>>
>>>>>
>>>>> On Sun, Apr 7, 2019 at 6:10 AM Rushikesh S Kadam
>>>>> <rushikesh.s.kadam@...el.com> wrote:
>>>>>>
>>>>>> This driver implements a slim layer to enable the ChromeOS
>>>>>> EC kernel stack (cros_ec) to communicate with ChromeOS EC
>>>>>> firmware running on the Intel Integrated Sensor Hub (ISH).
>>>>>>
>>>>>> The driver registers a ChromeOS EC MFD device to connect
>>>>>> with cros_ec kernel stack (upper layer), and it registers a
>>>>>> client with the ISH Transport Protocol bus (lower layer) to
>>>>>> talk with the ISH firwmare. See description of the ISHTP
>>>>>> protocol at Documentation/hid/intel-ish-hid.txt
>>>>>>
>>>>>> Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@...el.com
>>>>>>>
>>>>>> ---
>>>>>>
>>>>>> v3
>>>>>> - Made several changes to improve code readability. Replaced
>>>>>> multiple cl_data_to_dev(client_data) with dev variable.
>>>>>> Use
>>>>>> reverse Xmas tree for variable defintion where it made
>>>>>> sense.
>>>>>> Dropped few debug prints. Add docstring for function
>>>>>> prepare_cros_ec_rx().
>>>>>> - Fix code in function prepare_cros_ec_rx() under label
>>>>>> end_cros_ec_dev_init_error.
>>>>>> - Recycle buffer in process_recv() on failing to obtain the
>>>>>> semaphore.
>>>>>> - Increase ISHTP TX/RX ring buffer size to 8.
>>>>>> - Alphabetically ordered CROS_EC_ISHTP entries in Makefile
>>>>>> and
>>>>>> Kconfig.
>>>>>> - Updated commit message.
>>>>>>
>>>>>> v2
>>>>>> - Dropped unused "reset" parameter in function
>>>>>> cros_ec_init()
>>>>>> - Change driver name to cros_ec_ishtp to be consistent with
>>>>>> other
>>>>>> references in the code.
>>>>>> - Fixed a few typos.
>>>>>>
>>>>>> v1
>>>>>> - Initial version
>>>>>>
>>>>>> drivers/platform/chrome/Kconfig | 13 +
>>>>>> drivers/platform/chrome/Makefile | 1 +
>>>>>> drivers/platform/chrome/cros_ec_ishtp.c | 765
>>>>>> ++++++++++++++++++++++++++++++++
>>>>>> 3 files changed, 779 insertions(+)
>>>>>> create mode 100644 drivers/platform/chrome/cros_ec_ishtp.c
>>>>>>
>>>>>> diff --git a/drivers/platform/chrome/Kconfig
>>>>>> b/drivers/platform/chrome/Kconfig
>>>>>> index 16b1615..5848179 100644
>>>>>> --- a/drivers/platform/chrome/Kconfig
>>>>>> +++ b/drivers/platform/chrome/Kconfig
>>>>>> @@ -62,6 +62,19 @@ config CROS_EC_I2C
>>>>>> a checksum. Failing accesses will be retried three
>>>>>> times to
>>>>>> improve reliability.
>>>>>>
>>>>>> +config CROS_EC_ISHTP
>>>>>> + tristate "ChromeOS Embedded Controller (ISHTP)"
>>>>>> + depends on MFD_CROS_EC
>>>>>> + depends on INTEL_ISH_HID
>>>>>> + help
>>>>>> + If you say Y here, you get support for talking to
>>>>>> the ChromeOS EC
>>>>>> + firmware running on Intel Integrated Sensor Hub
>>>>>> (ISH), using the
>>>>>> + ISH Transport protocol (ISH-TP). This uses a simple
>>>>>> byte-level
>>>>>> + protocol with a checksum.
>>>>>> +
>>>>>> + To compile this driver as a module, choose M here:
>>>>>> the
>>>>>> + module will be called cros_ec_ishtp.
>>>>>> +
>>>>>> config CROS_EC_SPI
>>>>>> tristate "ChromeOS Embedded Controller (SPI)"
>>>>>> depends on MFD_CROS_EC && SPI
>>>>>> diff --git a/drivers/platform/chrome/Makefile
>>>>>> b/drivers/platform/chrome/Makefile
>>>>>> index cd591bf..4efe102 100644
>>>>>> --- a/drivers/platform/chrome/Makefile
>>>>>> +++ b/drivers/platform/chrome/Makefile
>>>>>> @@ -7,6 +7,7 @@ cros_ec_ctl-objs :=
>>>>>> cros_ec_sysfs.o cros_ec_lightbar.o \
>>>>>> cros_ec_vbc.o
>>>>>> cros_ec_debugfs.o
>>>>>> obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
>>>>>> obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
>>>>>> +obj-$(CONFIG_CROS_EC_ISHTP) += cros_ec_ishtp.o
>>>>>> obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
>>>>>> cros_ec_lpcs-objs := cros_ec_lpc.o
>>>>>> cros_ec_lpc_reg.o
>>>>>> cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
>>>>>> diff --git a/drivers/platform/chrome/cros_ec_ishtp.c
>>>>>> b/drivers/platform/chrome/cros_ec_ishtp.c
>>>>>> new file mode 100644
>>>>>> index 0000000..b1d19c4
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/platform/chrome/cros_ec_ishtp.c
>>>>>> @@ -0,0 +1,765 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>>> +/*
>>>>>> + * ISHTP client driver for talking to the Chrome OS EC
>>>>>> firmware running
>>>>>> + * on Intel Integrated Sensor Hub (ISH) using the ISH
>>>>>> Transport protocol
>>>>>> + * (ISH-TP).
>>>>>> + *
>>>>>> + * Copyright (c) 2019, Intel Corporation.
>>>>>> + */
>>>>>> +
>>>>>> +#include <linux/delay.h>
>>>>>> +#include <linux/mfd/core.h>
>>>>>> +#include <linux/mfd/cros_ec.h>
>>>>>> +#include <linux/mfd/cros_ec_commands.h>
>>>>>> +#include <linux/module.h>
>>>>>> +#include <linux/pci.h>
>>>>>> +#include <linux/intel-ish-client-if.h>
>>>>>> +
>>>>
>>>> I think that this patch depends on another patchset that's in
>>>> linux-next but
>>>> diddn't land yet to mainline. Do you know if the dependencies are
>>>> queued for
>>>> next merge window? Can you provide the exact patches that this
>>>> patch depends on?
>>>
>>> Enric,
>>> Sorry I missed mentioning this.
>>>
>>> The patch have dependency on intel-ish-hid stack on hid git tree,
>>> branch for-5.2/ish
>>>
> https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-5.2/ish
>>>
>>> Srinivas,
>>> Could you tell if the patches are queued for next merge window?
>>>
> Yes they are queued up for the next merge window. They are already in
> linux-next. One option is to check if Jiri pick this patch as part of
> his pull request. I am assuming that this patch doesn't depend on any
> other ChromeOS EC patches.
>
Ok, in such case a v4 will be needed, just few style changes that was thinking
to change myself if had the patch had gone through the platform tree.
Thanks,
Enric
Powered by blists - more mailing lists