Re: [Intel-gfx] [PATCH v2 5/5] media: platform: Add Chrome OS EC CEC driver

2018-05-21 Thread Neil Armstrong
Hi Enric,

On 18/05/2018 17:02, Enric Balletbo Serra wrote:
> Hi Neil,
> 
> 2018-05-18 15:05 GMT+02:00 Neil Armstrong :
>> The Chrome OS Embedded Controller can expose a CEC bus, this patch add the
> 
> A minor nit, there is a "consensus" on tell cros-ec as "ChromeOS
> Embedded Controller" or "ChromeOS EC". Yes, I know that you can see in
> the kernel many other ways to refer to the ChromeOS EC, but to
> standardize a little bit, could you replace all occurrences s/Chrome
> OS/ChromeOS/. Thanks.

Ok, I'll do a cleanup.

> 
>> driver for such feature of the Embedded Controller.
>>
>> This driver is part of the cros-ec MFD and will be add as a sub-device when
>> the feature bit is exposed by the EC.
>>
>> The controller will only handle a single logical address and handles
>> all the messages retries and will only expose Success or Error.
>>
>> The controller will be tied to the HDMI CEC notifier by using the platform
>> DMI Data and the i915 device name and connector name.
>>
>> Signed-off-by: Neil Armstrong 
>> ---
>>  drivers/media/platform/Kconfig   |  11 +
>>  drivers/media/platform/Makefile  |   2 +
>>  drivers/media/platform/cros-ec-cec/Makefile  |   1 +
>>  drivers/media/platform/cros-ec-cec/cros-ec-cec.c | 347 
>> +++
>>  4 files changed, 361 insertions(+)
>>  create mode 100644 drivers/media/platform/cros-ec-cec/Makefile
>>  create mode 100644 drivers/media/platform/cros-ec-cec/cros-ec-cec.c
>>
>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
>> index c7a1cf8..e55a8ed2 100644
>> --- a/drivers/media/platform/Kconfig
>> +++ b/drivers/media/platform/Kconfig
>> @@ -546,6 +546,17 @@ menuconfig CEC_PLATFORM_DRIVERS
>>
>>  if CEC_PLATFORM_DRIVERS
>>
>> +config VIDEO_CROS_EC_CEC
>> +   tristate "Chrome OS EC CEC driver"
> 
> here
> 
>> +   depends on MFD_CROS_EC || COMPILE_TEST
>> +   select CEC_CORE
>> +   select CEC_NOTIFIER
>> +   ---help---
>> + If you say yes here you will get support for the
>> + Chrome OS Embedded Controller's CEC.
> 
> here
> 
>> + The CEC bus is present in the HDMI connector and enables 
>> communication
>> + between compatible devices.
>> +
>>  config VIDEO_MESON_AO_CEC
>> tristate "Amlogic Meson AO CEC driver"
>> depends on ARCH_MESON || COMPILE_TEST
>> diff --git a/drivers/media/platform/Makefile 
>> b/drivers/media/platform/Makefile
>> index 932515d..830696f 100644
>> --- a/drivers/media/platform/Makefile
>> +++ b/drivers/media/platform/Makefile
>> @@ -92,3 +92,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)+= 
>> qcom/camss-8x16/
>>  obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
>>
>>  obj-y  += meson/
>> +
>> +obj-y  += cros-ec-cec/
>> diff --git a/drivers/media/platform/cros-ec-cec/Makefile 
>> b/drivers/media/platform/cros-ec-cec/Makefile
>> new file mode 100644
>> index 000..9ce97f9
>> --- /dev/null
>> +++ b/drivers/media/platform/cros-ec-cec/Makefile
>> @@ -0,0 +1 @@
>> +obj-$(CONFIG_VIDEO_CROS_EC_CEC) += cros-ec-cec.o
>> diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c 
>> b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
>> new file mode 100644
>> index 000..7e1e275
>> --- /dev/null
>> +++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
>> @@ -0,0 +1,347 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * CEC driver for Chrome OS Embedded Controller
> 
> and here
> 
>> + *
>> + * Copyright (c) 2018 BayLibre, SAS
>> + * Author: Neil Armstrong 
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define DRV_NAME   "cros-ec-cec"
>> +
>> +/**
>> + * struct cros_ec_cec - Driver data for EC CEC
>> + *
>> + * @cros_ec: Pointer to EC device
>> + * @notifier: Notifier info for responding to EC events
>> + * @adap: CEC adapter
>> + * @notify: CEC notifier pointer
>> + * @rx_msg: storage for a received message
>> + */
>> +struct cros_ec_cec {
>> +   struct cros_ec_device *cros_ec;
>> +   struct notifier_block notifier;
>> +   struct cec_adapter *adap;
>> +   struct cec_notifier *notify;
>> +   struct cec_msg rx_msg;
>> +};
>> +
>> +static void handle_cec_message(struct cros_ec_cec *cros_ec_cec)
>> +{
>> +   struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
>> +   uint8_t *cec_message = cros_ec->event_data.data.cec_message;
>> +   unsigned int len = cros_ec->event_size;
>> +
>> +   cros_ec_cec->rx_msg.len = len;
>> +   memcpy(cros_ec_cec->rx_msg.msg, cec_message, len);
>> +
>> +   cec_received_msg(cros_ec_cec->adap, _ec_cec->rx_msg);
>> +}
>> +
>> +static void handle_cec_event(struct cros_ec_cec *cros_ec_cec)
>> +{
>> +   struct cros_ec_device *cros_ec = 

Re: [Intel-gfx] [PATCH v2 5/5] media: platform: Add Chrome OS EC CEC driver

2018-05-18 Thread Enric Balletbo Serra
Hi Neil,

2018-05-18 15:05 GMT+02:00 Neil Armstrong :
> The Chrome OS Embedded Controller can expose a CEC bus, this patch add the

A minor nit, there is a "consensus" on tell cros-ec as "ChromeOS
Embedded Controller" or "ChromeOS EC". Yes, I know that you can see in
the kernel many other ways to refer to the ChromeOS EC, but to
standardize a little bit, could you replace all occurrences s/Chrome
OS/ChromeOS/. Thanks.

> driver for such feature of the Embedded Controller.
>
> This driver is part of the cros-ec MFD and will be add as a sub-device when
> the feature bit is exposed by the EC.
>
> The controller will only handle a single logical address and handles
> all the messages retries and will only expose Success or Error.
>
> The controller will be tied to the HDMI CEC notifier by using the platform
> DMI Data and the i915 device name and connector name.
>
> Signed-off-by: Neil Armstrong 
> ---
>  drivers/media/platform/Kconfig   |  11 +
>  drivers/media/platform/Makefile  |   2 +
>  drivers/media/platform/cros-ec-cec/Makefile  |   1 +
>  drivers/media/platform/cros-ec-cec/cros-ec-cec.c | 347 
> +++
>  4 files changed, 361 insertions(+)
>  create mode 100644 drivers/media/platform/cros-ec-cec/Makefile
>  create mode 100644 drivers/media/platform/cros-ec-cec/cros-ec-cec.c
>
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index c7a1cf8..e55a8ed2 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -546,6 +546,17 @@ menuconfig CEC_PLATFORM_DRIVERS
>
>  if CEC_PLATFORM_DRIVERS
>
> +config VIDEO_CROS_EC_CEC
> +   tristate "Chrome OS EC CEC driver"

here

> +   depends on MFD_CROS_EC || COMPILE_TEST
> +   select CEC_CORE
> +   select CEC_NOTIFIER
> +   ---help---
> + If you say yes here you will get support for the
> + Chrome OS Embedded Controller's CEC.

here

> + The CEC bus is present in the HDMI connector and enables 
> communication
> + between compatible devices.
> +
>  config VIDEO_MESON_AO_CEC
> tristate "Amlogic Meson AO CEC driver"
> depends on ARCH_MESON || COMPILE_TEST
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 932515d..830696f 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -92,3 +92,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS)+= 
> qcom/camss-8x16/
>  obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/
>
>  obj-y  += meson/
> +
> +obj-y  += cros-ec-cec/
> diff --git a/drivers/media/platform/cros-ec-cec/Makefile 
> b/drivers/media/platform/cros-ec-cec/Makefile
> new file mode 100644
> index 000..9ce97f9
> --- /dev/null
> +++ b/drivers/media/platform/cros-ec-cec/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_VIDEO_CROS_EC_CEC) += cros-ec-cec.o
> diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c 
> b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> new file mode 100644
> index 000..7e1e275
> --- /dev/null
> +++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * CEC driver for Chrome OS Embedded Controller

and here

> + *
> + * Copyright (c) 2018 BayLibre, SAS
> + * Author: Neil Armstrong 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRV_NAME   "cros-ec-cec"
> +
> +/**
> + * struct cros_ec_cec - Driver data for EC CEC
> + *
> + * @cros_ec: Pointer to EC device
> + * @notifier: Notifier info for responding to EC events
> + * @adap: CEC adapter
> + * @notify: CEC notifier pointer
> + * @rx_msg: storage for a received message
> + */
> +struct cros_ec_cec {
> +   struct cros_ec_device *cros_ec;
> +   struct notifier_block notifier;
> +   struct cec_adapter *adap;
> +   struct cec_notifier *notify;
> +   struct cec_msg rx_msg;
> +};
> +
> +static void handle_cec_message(struct cros_ec_cec *cros_ec_cec)
> +{
> +   struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
> +   uint8_t *cec_message = cros_ec->event_data.data.cec_message;
> +   unsigned int len = cros_ec->event_size;
> +
> +   cros_ec_cec->rx_msg.len = len;
> +   memcpy(cros_ec_cec->rx_msg.msg, cec_message, len);
> +
> +   cec_received_msg(cros_ec_cec->adap, _ec_cec->rx_msg);
> +}
> +
> +static void handle_cec_event(struct cros_ec_cec *cros_ec_cec)
> +{
> +   struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
> +   uint32_t events = cros_ec->event_data.data.cec_events;
> +
> +   if (events & EC_MKBP_CEC_SEND_OK)
> +   cec_transmit_attempt_done(cros_ec_cec->adap,
> + 

Re: [Intel-gfx] [PATCH v2 5/5] media: platform: Add Chrome OS EC CEC driver

2018-05-17 Thread kbuild test robot
Hi Neil,

I love your patch! Yet something to improve:

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.17-rc5 next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Neil-Armstrong/Add-ChromeOS-EC-CEC-Support/20180516-180519
base:   git://linuxtv.org/media_tree.git master
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   drivers/media//platform/cros-ec-cec/cros-ec-cec.c: In function 
'cros_ec_cec_get_notifier':
>> drivers/media//platform/cros-ec-cec/cros-ec-cec.c:231:33: error: 
>> 'pci_bus_type' undeclared (first use in this function); did you mean 
>> 'pci_pcie_type'?
   d = bus_find_device_by_name(_bus_type, NULL,
^~~~
pci_pcie_type
   drivers/media//platform/cros-ec-cec/cros-ec-cec.c:231:33: note: each 
undeclared identifier is reported only once for each function it appears in

vim +231 drivers/media//platform/cros-ec-cec/cros-ec-cec.c

   217  
   218  static int cros_ec_cec_get_notifier(struct device *dev,
   219  struct cec_notifier **notify)
   220  {
   221  int i;
   222  
   223  for (i = 0 ; i < ARRAY_SIZE(cec_dmi_match_table) ; ++i) {
   224  const struct cec_dmi_match *m = _dmi_match_table[i];
   225  
   226  if (dmi_match(DMI_SYS_VENDOR, m->sys_vendor) &&
   227  dmi_match(DMI_PRODUCT_NAME, m->product_name)) {
   228  struct device *d;
   229  
   230  /* Find the device, bail out if not yet 
registered */
 > 231  d = bus_find_device_by_name(_bus_type, NULL,
   232  m->devname);
   233  if (!d)
   234  return -EPROBE_DEFER;
   235  
   236  *notify = cec_notifier_get_conn(d, m->conn);
   237  return 0;
   238  }
   239  }
   240  
   241  /* Hardware support must be added in the cec_dmi_match_table */
   242  dev_warn(dev, "CEC notifier not configured for this 
hardware\n");
   243  
   244  return -ENODEV;
   245  }
   246  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v2 5/5] media: platform: Add Chrome OS EC CEC driver

2018-05-15 Thread Hans Verkuil
On 05/15/2018 04:42 PM, Neil Armstrong wrote:
> The Chrome OS Embedded Controller can expose a CEC bus, this patch add the
> driver for such feature of the Embedded Controller.
> 
> This driver is part of the cros-ec MFD and will be add as a sub-device when
> the feature bit is exposed by the EC.
> 
> The controller will only handle a single logical address and handles
> all the messages retries and will only expose Success or Error.
> 
> The controller will be tied to the HDMI CEC notifier by using the platform
> DMI Data and the i915 device name and connector name.
> 
> Signed-off-by: Neil Armstrong 
> ---



> +static SIMPLE_DEV_PM_OPS(cros_ec_cec_pm_ops,
> + cros_ec_cec_suspend, cros_ec_cec_resume);
> +
> +/*
> + * The Firmware only handles single CEC interface tied to a single HDMI

single CEC -> a single CEC

> + * connector we specify along the DRM device name handling the HDMI output

along -> along with

> + */
> +
> +struct cec_dmi_match {
> + char *sys_vendor;
> + char *product_name;
> + char *devname;
> + char *conn;
> +};
> +

Looks good!

Regards,

Hans
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx