Re: [RFCv2 PATCH 4/5] drm/bridge: add dw-hdmi cec driver using Hans Verkuil's CEC code

Hi,

2016-11-14 16:22 GMT+01:00 Hans Verkuil hverkuil@xs4all.nl:

From: Russell King rmk+kernel@arm.linux.org.uk

Add a CEC driver for the dw-hdmi hardware using Hans Verkuil's CEC implementation.

Signed-off-by: Russell King rmk+kernel@armlinux.org.uk

I've seen that the patchset concerning CEC/HDMI notifier after this one dropped dw-hdmi support. Is this only temporary, or does this driver need someone to take care of it?

diff --git a/drivers/gpu/drm/bridge/dw-hdmi-cec.c b/drivers/gpu/drm/bridge/dw-hdmi-cec.c new file mode 100644 index 0000000..e7e12b5 --- /dev/null +++ b/drivers/gpu/drm/bridge/dw-hdmi-cec.c @@ -0,0 +1,346 @@ +/* http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/

• • tree/drivers/mxc/hdmi-cec/mxc_hdmi-cec.c?h=imx_3.0.35_4.1.0 */

It is perhaps mandatory to have GPL header?

+#include <linux/hdmi-notifier.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/notifier.h> +#include <linux/platform_data/dw_hdmi-cec.h> +#include <linux/platform_device.h> +#include <linux/sched.h> +#include <linux/slab.h>

+#include <drm/drm_edid.h>

+#include <media/cec.h> +#include <media/cec-edid.h>

+#define DEV_NAME "mxc_hdmi_cec"

I think that to respect the convention it should be dw-hdmi-cec?

•   writeb_relaxed(addresses & 255, cec->base + HDMI_CEC_ADDR_L);
  

•   writeb_relaxed(addresses >> 8, cec->base + HDMI_CEC_ADDR_H);
  

Some platforms (at least rockchip) discuss with dw-hdmi with longs instead of bytes dw-hdmi-i2s-audio.c uses hdmi_read/hdmi_write for that

Is it ok to add write and read functions to dw_hdmi_cec_ops ?

+static unsigned int parse_hdmi_addr(const struct edid *edid) +{

•   if (!edid || edid->extensions == 0)
  

•           return (u16)~0;
  

•   return cec_get_edid_phys_addr((u8 *)edid,
  

•                           EDID_LENGTH * (edid->extensions + 1), NULL);
  

+}

+static int dw_hdmi_cec_notify(struct notifier_block *nb, unsigned long event,

•                         void *data)
  

+{

•   struct dw_hdmi_cec *cec = container_of(nb, struct dw_hdmi_cec, nb);
  

•   struct hdmi_notifier *n = data;
  

•   unsigned int phys;
  

•   dev_info(cec->adap->devnode.parent, "event %lu\n", event);
  

•   switch (event) {
  

•   case HDMI_CONNECTED:
  

•           break;
  

•   case HDMI_DISCONNECTED:
  

•           cec_s_phys_addr(cec->adap, CEC_PHYS_ADDR_INVALID, false);
  

•           break;
  

•   case HDMI_NEW_EDID:
  

•           phys = parse_hdmi_addr(n->edid);
  

•           cec_s_phys_addr(cec->adap, phys, false);
  

•           break;
  

•   }
  

•   return NOTIFY_OK;
  

+}

Thanks to "cec: integrate HDMI notifier support" this code can be dropped