[PATCH v2] net: ec_bhf: remove excessive debug messages
This cuts down the number of debug information spit out by
the driver.
Signed-off-by: Dariusz Marcinkiewicz
---
Changes since v1:
* as suggested by Dave Miller, I am simply ditching all
debugfs stuff
drivers/net/ethernet/ec_bhf.c | 101 -
1 file changed, 10 insertions(+), 91 deletions(-)
diff --git a/drivers/net/ethernet/ec_bhf.c b/drivers/net/ethernet/ec_bhf.c
index 056b44b..d101750 100644
--- a/drivers/net/ethernet/ec_bhf.c
+++ b/drivers/net/ethernet/ec_bhf.c
@@ -1,5 +1,5 @@
/*
- * drivers/net/ethernet/beckhoff/ec_bhf.c
+ * drivers/net/ethernet/ec_bhf.c
*
* Copyright (C) 2014 Darek Marcinkiewicz
*
@@ -18,9 +18,6 @@
* Those can be found on Bechhoff CX50xx industrial PCs.
*/
-#if 0
-#define DEBUG
-#endif
#include
#include
#include
@@ -74,6 +71,8 @@
#define DMA_WINDOW_SIZE_MASK 0xfffc
+#define ETHERCAT_MASTER_ID 0x14
+
static struct pci_device_id ids[] = {
{ PCI_DEVICE(0x15ec, 0x5000), },
{ 0, }
@@ -131,7 +130,6 @@ struct bhf_dma {
struct ec_bhf_priv {
struct net_device *net_dev;
-
struct pci_dev *dev;
void __iomem *io;
@@ -162,32 +160,6 @@ struct ec_bhf_priv {
#define PRIV_TO_DEV(priv) (&(priv)->dev->dev)
-#define ETHERCAT_MASTER_ID 0x14
-
-static void ec_bhf_print_status(struct ec_bhf_priv *priv)
-{
- struct device *dev = PRIV_TO_DEV(priv);
-
- dev_dbg(dev, "Frame error counter: %d\n",
- ioread8(priv->mac_io + MAC_FRAME_ERR_CNT));
- dev_dbg(dev, "RX error counter: %d\n",
- ioread8(priv->mac_io + MAC_RX_ERR_CNT));
- dev_dbg(dev, "CRC error counter: %d\n",
- ioread8(priv->mac_io + MAC_CRC_ERR_CNT));
- dev_dbg(dev, "TX frame counter: %d\n",
- ioread32(priv->mac_io + MAC_TX_FRAME_CNT));
- dev_dbg(dev, "RX frame counter: %d\n",
- ioread32(priv->mac_io + MAC_RX_FRAME_CNT));
- dev_dbg(dev, "TX fifo level: %d\n",
- ioread8(priv->mac_io + MAC_TX_FIFO_LVL));
- dev_dbg(dev, "Dropped frames: %d\n",
- ioread8(priv->mac_io + MAC_DROPPED_FRMS));
- dev_dbg(dev, "Connected with CCAT slot: %d\n",
- ioread8(priv->mac_io + MAC_CONNECTED_CCAT_FLAG));
- dev_dbg(dev, "Link status: %d\n",
- ioread8(priv->mii_io + MII_LINK_STATUS));
-}
-
static void ec_bhf_reset(struct ec_bhf_priv *priv)
{
iowrite8(0, priv->mac_io + MAC_FRAME_ERR_CNT);
@@ -210,8 +182,6 @@ static void ec_bhf_send_packet(struct ec_bhf_priv *priv,
struct tx_desc *desc)
u32 addr = (u8 *)desc - priv->tx_buf.buf;
iowrite32((ALIGN(len, 8) << 24) | addr, priv->fifo_io + FIFO_TX_REG);
-
- dev_dbg(PRIV_TO_DEV(priv), "Done sending packet\n");
}
static int ec_bhf_desc_sent(struct tx_desc *desc)
@@ -244,7 +214,6 @@ static void ec_bhf_add_rx_desc(struct ec_bhf_priv *priv,
struct rx_desc *desc)
static void ec_bhf_process_rx(struct ec_bhf_priv *priv)
{
struct rx_desc *desc = &priv->rx_descs[priv->rx_dnext];
- struct device *dev = PRIV_TO_DEV(priv);
while (ec_bhf_pkt_received(desc)) {
int pkt_size = (le16_to_cpu(desc->header.len) &
@@ -253,20 +222,16 @@ static void ec_bhf_process_rx(struct ec_bhf_priv *priv)
struct sk_buff *skb;
skb = netdev_alloc_skb_ip_align(priv->net_dev, pkt_size);
- dev_dbg(dev, "Received packet, size: %d\n", pkt_size);
-
if (skb) {
memcpy(skb_put(skb, pkt_size), data, pkt_size);
skb->protocol = eth_type_trans(skb, priv->net_dev);
- dev_dbg(dev, "Protocol type: %x\n", skb->protocol);
-
priv->stat_rx_bytes += pkt_size;
netif_rx(skb);
} else {
- dev_err_ratelimited(dev,
- "Couldn't allocate a skb_buff for a packet of
size %u\n",
- pkt_size);
+ dev_err_ratelimited(PRIV_TO_DEV(priv),
+ "Couldn't allocate a skb_buff for a
packet of size %u\n",
+ pkt_size);
}
desc->header.recv = 0;
@@ -276,7 +241,6 @@ static void ec_bhf_process_rx(struct ec_bhf_priv *priv)
priv->rx_dnext = (priv->rx_dnext + 1) % priv->rx_dcount;
desc = &priv->rx_descs[priv->rx_dnext];
}
-
}
static enum hrtimer_restart ec_bhf_timer_fun(struct hrtimer *timer)
@@ -299,14 +263,7 @@ static int ec_bhf_setup_offsets(struct ec_bhf_priv *priv)
unsigned bloc
[PATCH 1/2] net: ec_bhf: remove excessive debug messages
This cuts down on the number of debug information spit out by
the driver. Some of the potentially useful debug info gets exposed
by debugfs.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/net/ethernet/ec_bhf.c | 185 ++---
1 file changed, 99 insertions(+), 86 deletions(-)
diff --git a/drivers/net/ethernet/ec_bhf.c b/drivers/net/ethernet/ec_bhf.c
index 056b44b..935db19 100644
--- a/drivers/net/ethernet/ec_bhf.c
+++ b/drivers/net/ethernet/ec_bhf.c
@@ -1,5 +1,5 @@
/*
- * drivers/net/ethernet/beckhoff/ec_bhf.c
+ * drivers/net/ethernet/ec_bhf.c
*
* Copyright (C) 2014 Darek Marcinkiewicz
*
@@ -18,14 +18,12 @@
* Those can be found on Bechhoff CX50xx industrial PCs.
*/
-#if 0
-#define DEBUG
-#endif
#include
#include
#include
#include
#include
+#include
#include
#include
@@ -131,9 +129,10 @@ struct bhf_dma {
struct ec_bhf_priv {
struct net_device *net_dev;
-
struct pci_dev *dev;
+ struct dentry *root;
+
void __iomem *io;
void __iomem *dma_io;
@@ -164,29 +163,45 @@ struct ec_bhf_priv {
#define ETHERCAT_MASTER_ID 0x14
-static void ec_bhf_print_status(struct ec_bhf_priv *priv)
-{
- struct device *dev = PRIV_TO_DEV(priv);
+static struct debugfs_reg32 ec_bhf_debugfs_mii_regs[] = {
+ {
+ .name = "link-status",
+ .offset = MII_LINK_STATUS
+ }
+};
- dev_dbg(dev, "Frame error counter: %d\n",
- ioread8(priv->mac_io + MAC_FRAME_ERR_CNT));
- dev_dbg(dev, "RX error counter: %d\n",
- ioread8(priv->mac_io + MAC_RX_ERR_CNT));
- dev_dbg(dev, "CRC error counter: %d\n",
- ioread8(priv->mac_io + MAC_CRC_ERR_CNT));
- dev_dbg(dev, "TX frame counter: %d\n",
- ioread32(priv->mac_io + MAC_TX_FRAME_CNT));
- dev_dbg(dev, "RX frame counter: %d\n",
- ioread32(priv->mac_io + MAC_RX_FRAME_CNT));
- dev_dbg(dev, "TX fifo level: %d\n",
- ioread8(priv->mac_io + MAC_TX_FIFO_LVL));
- dev_dbg(dev, "Dropped frames: %d\n",
- ioread8(priv->mac_io + MAC_DROPPED_FRMS));
- dev_dbg(dev, "Connected with CCAT slot: %d\n",
- ioread8(priv->mac_io + MAC_CONNECTED_CCAT_FLAG));
- dev_dbg(dev, "Link status: %d\n",
- ioread8(priv->mii_io + MII_LINK_STATUS));
-}
+static struct debugfs_regset32 ec_bhf_debugfs_mii_regset = {
+ .regs = ec_bhf_debugfs_mii_regs,
+ .nregs = ARRAY_SIZE(ec_bhf_debugfs_mii_regs)
+};
+
+static struct debugfs_reg32 ec_bhf_debugfs_fifo_regs[] = {
+ {
+ .name = "fifo-tx",
+ .offset = FIFO_TX_REG
+ },
+ {
+ .name = "fifo-rx",
+ .offset = FIFO_RX_REG
+ }
+};
+
+static struct debugfs_regset32 ec_bhf_debugfs_fifo_regset = {
+ .regs = ec_bhf_debugfs_fifo_regs,
+ .nregs = ARRAY_SIZE(ec_bhf_debugfs_fifo_regs)
+};
+
+static struct debugfs_reg32 ec_bhf_debugfs_mac_regs[] = {
+ {
+ .name = "fifo-level",
+ .offset = MAC_TX_FIFO_LVL
+ },
+};
+
+static struct debugfs_regset32 ec_bhf_debugfs_mac_regset = {
+ .regs = ec_bhf_debugfs_mac_regs,
+ .nregs = ARRAY_SIZE(ec_bhf_debugfs_mac_regs)
+};
static void ec_bhf_reset(struct ec_bhf_priv *priv)
{
@@ -210,8 +225,6 @@ static void ec_bhf_send_packet(struct ec_bhf_priv *priv,
struct tx_desc *desc)
u32 addr = (u8 *)desc - priv->tx_buf.buf;
iowrite32((ALIGN(len, 8) << 24) | addr, priv->fifo_io + FIFO_TX_REG);
-
- dev_dbg(PRIV_TO_DEV(priv), "Done sending packet\n");
}
static int ec_bhf_desc_sent(struct tx_desc *desc)
@@ -244,7 +257,6 @@ static void ec_bhf_add_rx_desc(struct ec_bhf_priv *priv,
struct rx_desc *desc)
static void ec_bhf_process_rx(struct ec_bhf_priv *priv)
{
struct rx_desc *desc = &priv->rx_descs[priv->rx_dnext];
- struct device *dev = PRIV_TO_DEV(priv);
while (ec_bhf_pkt_received(desc)) {
int pkt_size = (le16_to_cpu(desc->header.len) &
@@ -253,20 +265,16 @@ static void ec_bhf_process_rx(struct ec_bhf_priv *priv)
struct sk_buff *skb;
skb = netdev_alloc_skb_ip_align(priv->net_dev, pkt_size);
- dev_dbg(dev, "Received packet, size: %d\n", pkt_size);
-
if (skb) {
memcpy(skb_put(skb, pkt_size), data, pkt_size);
skb->protocol = eth_type_trans(skb, priv->net_dev);
- dev_dbg(dev, "Protocol type: %x\n", skb->protocol);
-
priv->stat_rx_bytes += pkt_size;
netif_rx(skb);
} else {
-
[PATCH 2/2] MAINTAINERS: add entry for ec_bhf driver
Added entry for ec_bhf driver. Signed-off-by: Dariusz Marcinkiewicz --- MAINTAINERS |6 ++ 1 file changed, 6 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 7e2eb4c..b5416f8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1835,6 +1835,12 @@ S: Orphan F: Documentation/filesystems/befs.txt F: fs/befs/ +BECKHOFF CX5020 ETHERCAT MASTER DRIVER +M: Dariusz Marcinkiewicz +L: net...@vger.kernel.org +S: Maintained +F: drivers/net/ethernet/ec_bhf.c + BFS FILE SYSTEM M: "Tigran A. Aivazian" S: Maintained -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v7.1 5/9] drm: tda998x: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill
in the cec_connector_info.
Changes since v7:
- typo fix
Changes since v6:
- move cec_notifier_conn_unregister to tda998x_bridge_detach,
- add a mutex protecting accesses to a CEC notifier.
Changes since v2:
- cec_notifier_phys_addr_invalidate where appropriate,
- don't check for NULL notifier before calling
cec_notifier_conn_unregister.
Changes since v1:
Add memory barrier to make sure that the notifier
becomes visible to the irq thread once it is
fully constructed.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/i2c/tda998x_drv.c | 36 +--
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 61e042918a7fc..c6e922cd3c0b5 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -82,6 +82,8 @@ struct tda998x_priv {
u8 audio_port_enable[AUDIO_ROUTE_NUM];
struct tda9950_glue cec_glue;
struct gpio_desc *calib;
+
+ struct mutex cec_notify_mutex;
struct cec_notifier *cec_notify;
};
@@ -805,8 +807,11 @@ static irqreturn_t tda998x_irq_thread(int irq, void *data)
tda998x_edid_delay_start(priv);
} else {
schedule_work(&priv->detect_work);
- cec_notifier_set_phys_addr(priv->cec_notify,
- CEC_PHYS_ADDR_INVALID);
+
+ mutex_lock(&priv->cec_notify_mutex);
+ cec_notifier_phys_addr_invalidate(
+ priv->cec_notify);
+ mutex_unlock(&priv->cec_notify_mutex);
}
handled = true;
@@ -1331,6 +1336,8 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
struct drm_device *drm)
{
struct drm_connector *connector = &priv->connector;
+ struct cec_connector_info conn_info;
+ struct cec_notifier *notifier;
int ret;
connector->interlace_allowed = 1;
@@ -1347,6 +1354,16 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
if (ret)
return ret;
+ cec_fill_conn_info_from_drm(&conn_info, connector);
+
+ notifier = cec_notifier_conn_register(priv->cec_glue.parent,
+ NULL, &conn_info);
+ return -ENOMEM;
+
+ mutex_lock(&priv->cec_notify_mutex);
+ priv->cec_notify = notifier;
+ mutex_unlock(&priv->cec_notify_mutex);
+
drm_connector_attach_encoder(&priv->connector,
priv->bridge.encoder);
@@ -1366,6 +1383,11 @@ static void tda998x_bridge_detach(struct drm_bridge
*bridge)
{
struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge);
+ mutex_lock(&priv->cec_notify_mutex);
+ cec_notifier_conn_unregister(priv->cec_notify);
+ priv->cec_notify = NULL;
+ mutex_unlock(&priv->cec_notify_mutex);
+
drm_connector_cleanup(&priv->connector);
}
@@ -1789,9 +1811,6 @@ static void tda998x_destroy(struct device *dev)
cancel_work_sync(&priv->detect_work);
i2c_unregister_device(priv->cec);
-
- if (priv->cec_notify)
- cec_notifier_put(priv->cec_notify);
}
static int tda998x_create(struct device *dev)
@@ -1812,6 +1831,7 @@ static int tda998x_create(struct device *dev)
mutex_init(&priv->mutex); /* protect the page access */
mutex_init(&priv->audio_mutex); /* protect access from audio thread */
mutex_init(&priv->edid_mutex);
+ mutex_init(&priv->cec_notify_mutex);
INIT_LIST_HEAD(&priv->bridge.list);
init_waitqueue_head(&priv->edid_delay_waitq);
timer_setup(&priv->edid_delay_timer, tda998x_edid_delay_done, 0);
@@ -1916,12 +1936,6 @@ static int tda998x_create(struct device *dev)
cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
}
- priv->cec_notify = cec_notifier_get(dev);
- if (!priv->cec_notify) {
- ret = -ENOMEM;
- goto fail;
- }
-
priv->cec_glue.parent = dev;
priv->cec_glue.data = priv;
priv->cec_glue.init = tda998x_cec_hook_init;
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH v7 1/3] media: cec: expose HDMI connector to CEC dev mapping
This patch proposes to expose explicit mapping between HDMI connectors and /dev/cecX adapters to userland. New structure with connector info (card number and connector id in case of DRM connectors) is added to cec_adapter. That connector info is expected to be provided when an adapter is created. CEC notifier is extended so that it can be used to communicate the connector's info to CEC adapters' creators. New ioctl, exposing connector info to userland, is added to /dev/cec. Changes since v6: - updated remaining cec_allocate_adapter calls Changes since v5: - make the patch apply against the latest changes in the affected code Changes since v4: - small tweaks + added documentation Changes since v3: - cec_get_connter_conn takes connector_info as argument Changes since v2: - cec_s_connector_info removed, the connector info is now passed to cec_allocate_adapter - updated commit message Changes since v1: - removed the unnecessary event, - extended cec_connctor_info to allow for various types of connectors. Signed-off-by: Dariusz Marcinkiewicz --- Documentation/media/kapi/cec-core.rst | 7 +- Documentation/media/uapi/cec/cec-funcs.rst| 1 + .../uapi/cec/cec-ioc-adap-g-conn-info.rst | 109 ++ .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 3 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 2 +- drivers/gpu/drm/drm_dp_cec.c | 22 ++-- drivers/gpu/drm/i2c/tda9950.c | 3 +- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_hdmi.c | 6 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +- drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 2 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 8 +- drivers/media/cec/cec-adap.c | 13 +++ drivers/media/cec/cec-api.c | 12 ++ drivers/media/cec/cec-core.c | 8 +- drivers/media/cec/cec-notifier.c | 20 +++- drivers/media/cec/cec-pin.c | 2 +- drivers/media/i2c/adv7511.c | 2 +- drivers/media/i2c/adv7604.c | 2 +- drivers/media/i2c/adv7842.c | 2 +- drivers/media/i2c/tc358743.c | 3 +- .../media/platform/cros-ec-cec/cros-ec-cec.c | 7 +- drivers/media/platform/meson/ao-cec-g12a.c| 3 +- drivers/media/platform/meson/ao-cec.c | 6 +- drivers/media/platform/s5p-cec/s5p_cec.c | 6 +- drivers/media/platform/seco-cec/seco-cec.c| 8 +- drivers/media/platform/sti/cec/stih-cec.c | 6 +- drivers/media/platform/stm32/stm32-cec.c | 2 +- drivers/media/platform/tegra-cec/tegra_cec.c | 5 +- drivers/media/platform/vivid/vivid-cec.c | 2 +- drivers/media/usb/pulse8-cec/pulse8-cec.c | 3 +- .../media/usb/rainshadow-cec/rainshadow-cec.c | 3 +- include/drm/drm_dp_helper.h | 14 +-- include/media/cec-notifier.h | 34 -- include/media/cec.h | 16 ++- include/uapi/linux/cec.h | 24 37 files changed, 316 insertions(+), 59 deletions(-) create mode 100644 Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst diff --git a/Documentation/media/kapi/cec-core.rst b/Documentation/media/kapi/cec-core.rst index 3ce26b7c2b2b6..3678a0a75104e 100644 --- a/Documentation/media/kapi/cec-core.rst +++ b/Documentation/media/kapi/cec-core.rst @@ -37,7 +37,8 @@ calling cec_allocate_adapter() and deleted by calling cec_delete_adapter(): .. c:function:: struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops, void *priv, - const char *name, u32 caps, u8 available_las); +const char *name, u32 caps, u8 available_las, +const struct cec_connector_info *connector_info); .. c:function:: void cec_delete_adapter(struct cec_adapter *adap); @@ -65,6 +66,10 @@ available_las: the number of simultaneous logical addresses that this adapter can handle. Must be 1 <= available_las <= CEC_MAX_LOG_ADDRS. +connector_info: +pointer to a struct describing connector this adapter is associated with, +can be NULL. + To obtain the priv pointer use this helper function: .. c:function:: diff --git a/Documentation/media/uapi/cec/cec-funcs.rst b/Documentation/media/uapi/cec/cec-funcs.rst index 620590b168c9e..dc6da9c639a85 100644 --- a/Documentation/media/uapi/cec/cec-funcs.rst +++ b/Documentation/media/uapi/cec/cec-funcs.rst @@ -24,6 +24,7 @@ Function Reference cec-ioc-adap-g-caps cec-ioc-adap-g-log-addrs cec-ioc-adap-g-phys-addr +cec-ioc-adap-g-conn-info cec-ioc-dqevent cec-ioc-g-mode cec-ioc-receive diff --git a/Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst b/Documentation/media/uapi/cec/cec
[PATCH v7 3/3] drm/i2c: tda9950: pass HDMI connector info to CEC adapter
With that change tda998x provides a connector info to the CEC
adapter. In order to be able to that it delays creation of
respective CEC device until the DRM connector is initialized.
Requires testing.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/i2c/tda9950.c | 13 +---
drivers/gpu/drm/i2c/tda998x_drv.c | 55 ---
2 files changed, 36 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
index b944dd9df85e1..61ec50a2ca275 100644
--- a/drivers/gpu/drm/i2c/tda9950.c
+++ b/drivers/gpu/drm/i2c/tda9950.c
@@ -382,6 +382,7 @@ static int tda9950_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct tda9950_glue *glue = client->dev.platform_data;
+ const struct cec_connector_info *conn_info;
struct device *dev = &client->dev;
struct tda9950_priv *priv;
unsigned long irqflags;
@@ -422,10 +423,16 @@ static int tda9950_probe(struct i2c_client *client,
if (glue && glue->parent)
priv->hdmi = glue->parent;
+ priv->notify = cec_notifier_get(priv->hdmi);
+ if (!priv->notify)
+ return -ENOMEM;
+
+ conn_info = cec_notifier_get_conn_info(priv->notify);
+
priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
CEC_CAP_DEFAULTS,
CEC_MAX_LOG_ADDRS,
- NULL);
+ conn_info);
if (IS_ERR(priv->adap))
return PTR_ERR(priv->adap);
@@ -461,10 +468,6 @@ static int tda9950_probe(struct i2c_client *client,
if (ret < 0)
return ret;
- priv->notify = cec_notifier_get(priv->hdmi);
- if (!priv->notify)
- return -ENOMEM;
-
ret = cec_register_adapter(priv->adap, priv->hdmi);
if (ret < 0) {
cec_notifier_put(priv->notify);
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb5155..b1d76e6561e30 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1253,6 +1253,8 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
struct drm_device *drm)
{
struct drm_connector *connector = &priv->connector;
+ struct cec_connector_info conn_info;
+ struct i2c_board_info cec_info;
int ret;
connector->interlace_allowed = 1;
@@ -1269,6 +1271,32 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
if (ret)
return ret;
+ /*
+* Some TDA998x are actually two I2C devices merged onto one piece
+* of silicon: TDA9989 and TDA19989 combine the HDMI transmitter
+* with a slightly modified TDA9950 CEC device. The CEC device
+* is at the TDA9950 address, with the address pins strapped across
+* to the TDA998x address pins. Hence, it always has the same
+* offset.
+*/
+ memset(&cec_info, 0, sizeof(cec_info));
+ strlcpy(cec_info.type, "tda9950", sizeof(cec_info.type));
+ cec_info.addr = priv->cec_addr;
+ cec_info.platform_data = &priv->cec_glue;
+ cec_info.irq = priv->hdmi->irq;
+
+ priv->cec = i2c_new_device(priv->hdmi->adapter, &cec_info);
+ if (!priv->cec)
+ return -ENODEV;
+
+ cec_fill_connector_info(&conn_info, connector);
+
+ priv->cec_notify = cec_notifier_get_conn(priv->cec_glue.parent,
+NULL, &conn_info);
+ if (!priv->cec_notify)
+ return -ENOMEM;
+
+
drm_connector_attach_encoder(&priv->connector,
priv->bridge.encoder);
@@ -1658,7 +1686,6 @@ static int tda998x_create(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct device_node *np = client->dev.of_node;
- struct i2c_board_info cec_info;
struct tda998x_priv *priv;
u32 video;
int rev_lo, rev_hi, ret;
@@ -1776,12 +1803,6 @@ static int tda998x_create(struct device *dev)
cec_write(priv, REG_CEC_RXSHPDINTENA, CEC_RXSHPDLEV_HPD);
}
- priv->cec_notify = cec_notifier_get(dev);
- if (!priv->cec_notify) {
- ret = -ENOMEM;
- goto fail;
- }
-
priv->cec_glue.parent = dev;
priv->cec_glue.data = priv;
priv->cec_glue.init = tda998x_cec_hook_init;
@@ -1789,26 +1810,6 @@ static int tda998x_create(struct device *dev)
priv->cec_glue.open = tda998x_cec_hook_open;
priv->cec_glue.release = tda998x_cec
[PATCH v7 2/3] drm/bridge: dw-hdmi: pass connector info to the CEC adapter
This patch makes dw-hdmi pass DRM connector info to a respective
CEC adapter. In order to be able to do that it delays creation of
the dw-hdmi-cec platform device until DRM connector is initialized.
Requires testing.
Changes since version v5:
- connector info is passed trough notifier, fixed CEC platform
device initialization
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 13 ++-
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 103 ++
2 files changed, 66 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
index 84fb7b6a0a5e0..64cc278f5a754 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
@@ -232,6 +232,7 @@ static void dw_hdmi_cec_del(void *data)
static int dw_hdmi_cec_probe(struct platform_device *pdev)
{
struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev);
+ const struct cec_connector_info *conn_info;
struct dw_hdmi_cec *cec;
int ret;
@@ -258,10 +259,16 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
dw_hdmi_write(cec, ~0, HDMI_IH_MUTE_CEC_STAT0);
dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY);
+ cec->notify = cec_notifier_get(pdev->dev.parent);
+ if (!cec->notify)
+ return -ENOMEM;
+
+ conn_info = cec_notifier_get_conn_info(cec->notify);
+
cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi",
CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
CEC_CAP_RC | CEC_CAP_PASSTHROUGH,
-CEC_MAX_LOG_ADDRS, NULL);
+CEC_MAX_LOG_ADDRS, conn_info);
if (IS_ERR(cec->adap))
return PTR_ERR(cec->adap);
@@ -281,10 +288,6 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
if (ret < 0)
return ret;
- cec->notify = cec_notifier_get(pdev->dev.parent);
- if (!cec->notify)
- return -ENOMEM;
-
ret = cec_register_adapter(cec->adap, pdev->dev.parent);
if (ret < 0) {
cec_notifier_put(cec->notify);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ab7968c8f6a29..ed2f3e7e9c1cf 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -138,6 +138,8 @@ struct dw_hdmi {
struct drm_connector connector;
struct drm_bridge bridge;
+ int irq;
+
unsigned int version;
struct platform_device *audio;
@@ -189,6 +191,7 @@ struct dw_hdmi {
void (*enable_audio)(struct dw_hdmi *hdmi);
void (*disable_audio)(struct dw_hdmi *hdmi);
+ bool cec_configured;
struct cec_notifier *cec_notifier;
};
@@ -2113,11 +2116,35 @@ static const struct drm_connector_helper_funcs
dw_hdmi_connector_helper_funcs =
.get_modes = dw_hdmi_connector_get_modes,
};
+static void dw_hdmi_cec_enable(struct dw_hdmi *hdmi)
+{
+ mutex_lock(&hdmi->mutex);
+ hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_CECCLK_DISABLE;
+ hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
+ mutex_unlock(&hdmi->mutex);
+}
+
+static void dw_hdmi_cec_disable(struct dw_hdmi *hdmi)
+{
+ mutex_lock(&hdmi->mutex);
+ hdmi->mc_clkdis |= HDMI_MC_CLKDIS_CECCLK_DISABLE;
+ hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
+ mutex_unlock(&hdmi->mutex);
+}
+
+static const struct dw_hdmi_cec_ops dw_hdmi_cec_ops = {
+ .write = hdmi_writeb,
+ .read = hdmi_readb,
+ .enable = dw_hdmi_cec_enable,
+ .disable = dw_hdmi_cec_disable,
+};
+
static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
{
struct dw_hdmi *hdmi = bridge->driver_private;
struct drm_encoder *encoder = bridge->encoder;
struct drm_connector *connector = &hdmi->connector;
+ struct cec_connector_info conn_info;
connector->interlace_allowed = 1;
connector->polled = DRM_CONNECTOR_POLL_HPD;
@@ -2129,6 +2156,34 @@ static int dw_hdmi_bridge_attach(struct drm_bridge
*bridge)
drm_connector_attach_encoder(connector, encoder);
+ cec_fill_connector_info(&conn_info, connector);
+
+ hdmi->cec_notifier = cec_notifier_get_conn(hdmi->dev, NULL,
+ &conn_info);
+ if (!hdmi->cec_notifier)
+ return -ENOMEM;
+
+ if (hdmi->cec_configured) {
+ struct platform_device_info pdevinfo;
+ struct dw_hdmi_cec_data cec;
+
+ memset(&pdevinfo, 0, sizeof(pdevinfo));
+ pdev
Re: [PATCH v6 2/3] drm/bridge: dw-hdmi: pass connector info to the CEC adapter
On Mon, May 20, 2019 at 1:30 PM Hans Verkuil wrote: > > On 5/17/19 5:42 PM, Dariusz Marcinkiewicz wrote: > > This patch makes dw-hdmi pass DRM connector info to a respective > > CEC adapter. In order to be able to do that it delays creation of > > the dw-hdmi-cec platform device until DRM connector is initialized. > > > > Requires testing. > > Testing this patch with the Khadas VIM2 board gives this kernel warning: > Thank you for testing! This was probably because the platform device info was not fully initialized. Hopefully it is better in v7. Regards.
Re: [PATCH v6 1/3] media: cec: expose HDMI connector to CEC dev mapping
On Mon, May 20, 2019 at 12:25 PM Hans Verkuil wrote: > > It's a bit unusual since it uses the Synopsys bridge, but not the Synopsys > CEC driver (it has its own meson cec driver). > > The first thing I noticed is that I did not get any connector info. > I think that the root cause of that is that you forgot that there are > several drm drivers that call cec_notifier_get() instead of > cec_notifier_get_conn(). > > I think all those calls to cec_notifier_get() in drm drivers should be > replaced > by cec_notifier_get_conn() where the second argument is NULL, but the third > argument > should contain valid connector info. > > A quick grep gives me the following drivers that need work: > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > drivers/gpu/drm/exynos/exynos_hdmi.c > drivers/gpu/drm/i2c/tda998x_drv.c > drivers/gpu/drm/sti/sti_hdmi.c > drivers/gpu/drm/tegra/output.c > I am afraid that just replacing cec_notifier_get with cec_notifier_get_conn won't necessarily make this information available, as, e.g. looking at tegra, those would also require some reshuffling of the code so that notifier is created once connector is initialized. Btw. I've updated dw-hdmi.c and tda998x_drv.c to use cec_notifier_get_conn, so at least those 2 will be covered. > The second thing I noticed is that patch 2 gave me a new kernel warning, but > I'll do some more testing for that and reply to patch 2/3 once I know more. > I hope this is fixed in v7. Thank you for your testing and best regards!
[RFC PATCH] media: cec: DRM connector to CEC dev mapping
This patch proposes to introduce explicit mapping between DRM connectors
and /dev/cecX adapters.
This is achieved here by adding a new structure with connector info
(DRM card number and connector id) to cec_adapter. That connector info
is expected to be set either by a code creating an adapter or trough
a CEC notifier (which is extended for that purpose).
A new ioctl, exposing connector info to userland, is added to /dev/cec.
New CEC event type is also added. That event is to notify an application
that the info about mapping between an adapter and a DRM connector was
just set.
Signed-off-by: Dariusz Marcinkiewicz
---
Notes:
Sending the patch in its current form the get feedback if that is
considered a right approach or if some other way of learning
the mapping in userland is preferred.
.../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +-
drivers/gpu/drm/drm_dp_cec.c | 20
drivers/gpu/drm/i915/intel_dp.c | 4 +-
drivers/gpu/drm/i915/intel_hdmi.c | 7 ++-
drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +-
drivers/media/cec/cec-adap.c | 21 -
drivers/media/cec/cec-api.c | 17 +++
drivers/media/cec/cec-core.c | 12 -
drivers/media/cec/cec-notifier.c | 47 ---
include/drm/drm_dp_helper.h | 14 +++---
include/media/cec-notifier.h | 31 ++--
include/media/cec.h | 13 -
include/uapi/linux/cec.h | 18 +++
13 files changed, 172 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index c4ea3a91f17aa..41678df654cdc 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -379,7 +379,7 @@ void amdgpu_dm_initialize_dp_connector(struct
amdgpu_display_manager *dm,
drm_dp_aux_register(&aconnector->dm_dp_aux.aux);
drm_dp_cec_register_connector(&aconnector->dm_dp_aux.aux,
- aconnector->base.name, dm->adev->dev);
+ &aconnector->base);
aconnector->mst_mgr.cbs = &dm_mst_cbs;
drm_dp_mst_topology_mgr_init(
&aconnector->mst_mgr,
diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
index b15cee85b702b..cb09bb2197a85 100644
--- a/drivers/gpu/drm/drm_dp_cec.c
+++ b/drivers/gpu/drm/drm_dp_cec.c
@@ -8,7 +8,9 @@
#include
#include
#include
+#include
#include
+#include
#include
/*
@@ -295,6 +297,7 @@ static void drm_dp_cec_unregister_work(struct work_struct
*work)
*/
void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
{
+ struct drm_connector *connector = aux->cec.connector;
u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
unsigned int num_las = 1;
u8 cap;
@@ -344,16 +347,19 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const
struct edid *edid)
/* Create a new adapter */
aux->cec.adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
-aux, aux->cec.name, cec_caps,
+aux, connector->name, cec_caps,
num_las);
if (IS_ERR(aux->cec.adap)) {
aux->cec.adap = NULL;
goto unlock;
}
- if (cec_register_adapter(aux->cec.adap, aux->cec.parent)) {
+ if (cec_register_adapter(aux->cec.adap, connector->dev->dev)) {
cec_delete_adapter(aux->cec.adap);
aux->cec.adap = NULL;
} else {
+ cec_s_connector_info(aux->cec.adap,
+connector->dev->primary->index,
+connector->base.id);
/*
* Update the phys addr for the new CEC adapter. When called
* from drm_dp_cec_register_connector() edid == NULL, so in
@@ -406,22 +412,20 @@ EXPORT_SYMBOL(drm_dp_cec_unset_edid);
/**
* drm_dp_cec_register_connector() - register a new connector
* @aux: DisplayPort AUX channel
- * @name: name of the CEC device
- * @parent: parent device
+ * @connector: drm connector
*
* A new connector was registered with associated CEC adapter name and
* CEC adapter parent device. After registering the name and parent
* drm_dp_cec_set_edid() is called to check if the connector supports
* CEC and to register a CEC adapter if that is the case.
*/
-void drm_dp_cec_register_connector(struct drm_dp_aux *aux, const char *name,
- struct device *parent)
+void drm_dp_cec_register_connector(struct drm_d
[PATCH v7 2/9] drm/i915/intel_hdmi: use cec_notifier_conn_(un)register
Use the new cec_notifier_conn_(un)register() functions to
(un)register the notifier for the HDMI connector, and fill in
the cec_connector_info.
Signed-off-by: Dariusz Marcinkiewicz
Signed-off-by: Hans Verkuil
Tested-by: Hans Verkuil
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 13 +
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index b1ca8e5bdb56d..9fcf2c58c29c5 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2752,8 +2752,9 @@ intel_hdmi_connector_register(struct drm_connector
*connector)
static void intel_hdmi_destroy(struct drm_connector *connector)
{
- if (intel_attached_hdmi(connector)->cec_notifier)
- cec_notifier_put(intel_attached_hdmi(connector)->cec_notifier);
+ struct cec_notifier *n = intel_attached_hdmi(connector)->cec_notifier;
+
+ cec_notifier_conn_unregister(n);
intel_connector_destroy(connector);
}
@@ -3068,6 +3069,7 @@ void intel_hdmi_init_connector(struct intel_digital_port
*intel_dig_port,
struct drm_device *dev = intel_encoder->base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
enum port port = intel_encoder->port;
+ struct cec_connector_info conn_info;
DRM_DEBUG_KMS("Adding HDMI connector on port %c\n",
port_name(port));
@@ -3120,8 +3122,11 @@ void intel_hdmi_init_connector(struct intel_digital_port
*intel_dig_port,
I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
}
- intel_hdmi->cec_notifier = cec_notifier_get_conn(dev->dev,
-port_identifier(port));
+ cec_fill_conn_info_from_drm(&conn_info, connector);
+
+ intel_hdmi->cec_notifier =
+ cec_notifier_conn_register(dev->dev, port_identifier(port),
+ &conn_info);
if (!intel_hdmi->cec_notifier)
DRM_DEBUG_KMS("CEC notifier get failed\n");
}
--
2.23.0.rc1.153.gdeed80330f-goog
[PATCH v7 0/9] drm: cec: convert DRM drivers to the new notifier API
This series updates DRM drivers to use new CEC notifier API. Changes since v6: Made CEC notifiers' registration and de-registration symmetric in tda998x and dw-hdmi drivers. Also, accidentally dropped one patch in v6 (change to drm_dp_cec), brought it back now. Changes since v5: Fixed a warning about a missing comment for a new member of drm_dp_aux_cec struct. Sending to a wider audience, including maintainers of respective drivers. Changes since v4: Addressing review comments. Changes since v3: Updated adapter flags in dw-hdmi-cec. Changes since v2: Include all DRM patches from "cec: improve notifier support, add connector info connector info" series. Changes since v1: Those patches delay creation of notifiers until respective connectors are constructed. It seems that those patches, for a couple of drivers, by adding the delay, introduce a race between notifiers' creation and the IRQs handling threads - at least I don't see anything obvious in there that would explicitly forbid such races to occur. v2 adds a write barrier to make sure IRQ threads see the notifier once it is created (replacing the WRITE_ONCE I put in v1). The best thing to do here, I believe, would be not to have any synchronization and make sure that an IRQ only gets enabled after the notifier is created. Dariusz Marcinkiewicz (9): drm_dp_cec: add connector info support. drm/i915/intel_hdmi: use cec_notifier_conn_(un)register dw-hdmi-cec: use cec_notifier_cec_adap_(un)register tda9950: use cec_notifier_cec_adap_(un)register drm: tda998x: use cec_notifier_conn_(un)register drm: sti: use cec_notifier_conn_(un)register drm: tegra: use cec_notifier_conn_(un)register drm: dw-hdmi: use cec_notifier_conn_(un)register drm: exynos: exynos_hdmi: use cec_notifier_conn_(un)register .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 13 +++--- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 46 +-- drivers/gpu/drm/drm_dp_cec.c | 25 ++ drivers/gpu/drm/exynos/exynos_hdmi.c | 31 +++-- drivers/gpu/drm/i2c/tda9950.c | 12 ++--- drivers/gpu/drm/i2c/tda998x_drv.c | 36 ++- drivers/gpu/drm/i915/display/intel_dp.c | 4 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 13 -- drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +- drivers/gpu/drm/sti/sti_hdmi.c| 19 +--- drivers/gpu/drm/tegra/output.c| 28 --- include/drm/drm_dp_helper.h | 17 --- 13 files changed, 155 insertions(+), 94 deletions(-) -- 2.23.0.rc1.153.gdeed80330f-goog
Re: [PATCH v6 4/8] drm: tda998x: use cec_notifier_conn_(un)register
Hello. On Tue, Aug 13, 2019 at 1:20 PM Russell King - ARM Linux admin wrote: > > This also doesn't make sense: tda998x_destroy() is the opposite of > tda998x_create(). However, tda998x_connector_destroy() is the > opposite of tda998x_connector_create(). > > By moving the CEC creation code into tda998x_connector_create(), you > are creating the possibility for the following sequence to mess up > CEC and leak: > > tda998x_create() > tda998x_connector_create() > tda998x_connector_destroy() > tda998x_connector_create() > tda998x_connector_destroy() > tda998x_destroy() > > Anything you create in tda998x_connector_create() must be cleaned up > by tda998x_connector_destroy(). > Thank you. I've just sent out another revision of the patch, where registration and deregistration is symmetric. Please take a look. Best regards.
Re: [PATCH v7 1/3] media: cec: expose HDMI connector to CEC dev mapping
Hi Hans. I would like to come back to this thread. On Fri, May 24, 2019 at 11:21 AM Hans Verkuil wrote: > > Hi Dariusz, > > I did some more testing with the Khadas VIM2 and found another problem, > something that will, unfortunately, require some redesign. > > See my comments below... > ... > > Another issue here is that when the HDMI driver removes the notifier, > then it should also zero the connector info. Remember that both the > HDMI and the CEC drivers can be loaded and unloaded independently from > one another. > I took a peek at the changes in https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=cec-conn. Do I understand it correctly, that the above problem is addressed there by unregistering an adapter in cec_notifier_conn_unregister (which will result in /dev/cecX node going away)? I wonder to what degree this solves the problem of HDMI and CEC drivers being loaded and unloaded independently. It seems that in cases where HDMI driver is unloaded and then loaded again, counterintuitively, the /dev/cecX might not come back again, is this right, or am I missing something? Also, is it guaranteed that adapter drivers won't try to access an adapter once it gets removed by cec_notifier_conn_unregister? Thank you.
Re: [PATCH RESEND] media: cros-ec-cec: do not bail on device_init_wakeup failure
Hi. On Wed, Jun 24, 2020 at 4:45 PM Guenter Roeck wrote: > > On Mon, Jun 22, 2020 at 12:23 PM Hans Verkuil > > wrote: > > > > > > > Signed-off-by: Dariusz Marcinkiewicz > > > > > > This can be CCed to stable, I guess? > > > > > > > That issue is not a recent regression but has been in there since the > > very beginning. So it might be argued that is it not severe enough to > > warrant cc'ing stable. Happy to do that anyways if you think > > otherwise. > > > > Confused. Internally you would like to have this patch applied to > chromeos-4.4. Here you suggest that it may not be important enough to > apply to stable releases. Which one is it ? > It affects us on 4.4, hence the backport to 4.4. I have nothing against this being merged into the mainline stable. I simply wasn't sure if that should be considered severe enough to be backported there. As said before, I am happy to CC this to stable. Thank you and best regards. (apologies for double post again)
[PATCH v3] media: cros-ec-cec: do not bail on device_init_wakeup failure
Do not fail probing when device_init_wakeup fails. Also clear wakeup
on remove.
device_init_wakeup fails when the device is already enabled as wakeup
device. Hence, the driver fails to probe the device if:
- The device has already been enabled for wakeup (via /proc/acpi/wakeup)
- The driver has been unloaded and is being loaded again.
This goal of the patch is to fix the above cases.
Overwhelming majority of the drivers do not consider device_init_wakeup
failure as a fatal error and proceed regardless of whether it succeeds
or not.
Changes since v2:
- disabled wakeup in remove
- CC'ing stable
- description fixed
Changes since v1:
- added Fixes tag
Fixes: cd70de2d356ee ("media: platform: Add ChromeOS EC CEC driver")
Cc: sta...@vger.kernel.org
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
index 0e7e2772f08f..3881ed7bc3d9 100644
--- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
+++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
@@ -277,12 +277,6 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, cros_ec_cec);
cros_ec_cec->cros_ec = cros_ec;
- ret = device_init_wakeup(&pdev->dev, 1);
- if (ret) {
- dev_err(&pdev->dev, "failed to initialize wakeup\n");
- return ret;
- }
-
cros_ec_cec->adap = cec_allocate_adapter(&cros_ec_cec_ops, cros_ec_cec,
DRV_NAME,
CEC_CAP_DEFAULTS |
@@ -310,6 +304,8 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
if (ret < 0)
goto out_probe_notify;
+ device_init_wakeup(&pdev->dev, 1);
+
return 0;
out_probe_notify:
@@ -339,6 +335,8 @@ static int cros_ec_cec_remove(struct platform_device *pdev)
cros_ec_cec->adap);
cec_unregister_adapter(cros_ec_cec->adap);
+ device_init_wakeup(&pdev->dev, 0);
+
return 0;
}
--
2.27.0.212.ge8ba1cc988-goog
[PATCH v2] media: cros-ec-cec: do not bail on device_init_wakeup failure
Do not fail probing when device_init_wakeup fails.
device_init_wakeup fails when the device is already enabled as wakeup
device. Hence, the driver fails to probe the device if:
- The device has already been enabled for wakeup (by e.g. sysfs)
- The driver has been unloaded and is being loaded again.
This goal of the patch is to fix the above cases.
Overwhelming majority of the drivers do not check device_init_wakeup
return code.
v2: add Fixes tags
Fixes: cd70de2d356ee ("media: platform: Add ChromeOS EC CEC driver")
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
index 0e7e2772f08f..2d95e16cd248 100644
--- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
+++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
@@ -277,11 +277,7 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, cros_ec_cec);
cros_ec_cec->cros_ec = cros_ec;
- ret = device_init_wakeup(&pdev->dev, 1);
- if (ret) {
- dev_err(&pdev->dev, "failed to initialize wakeup\n");
- return ret;
- }
+ device_init_wakeup(&pdev->dev, 1);
cros_ec_cec->adap = cec_allocate_adapter(&cros_ec_cec_ops, cros_ec_cec,
DRV_NAME,
--
2.27.0.111.gc72c7da667-goog
[PATCH] media: cros-ec-cec: disable the device wakeup on remove
The device is enabled for wake up while the driver is loaded. For symmetry, disable wakeup when it is removed. Signed-off-by: Dariusz Marcinkiewicz --- drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c index 2d95e16cd248..3881ed7bc3d9 100644 --- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c +++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c @@ -277,8 +277,6 @@ static int cros_ec_cec_probe(struct platform_device *pdev) platform_set_drvdata(pdev, cros_ec_cec); cros_ec_cec->cros_ec = cros_ec; - device_init_wakeup(&pdev->dev, 1); - cros_ec_cec->adap = cec_allocate_adapter(&cros_ec_cec_ops, cros_ec_cec, DRV_NAME, CEC_CAP_DEFAULTS | @@ -306,6 +304,8 @@ static int cros_ec_cec_probe(struct platform_device *pdev) if (ret < 0) goto out_probe_notify; + device_init_wakeup(&pdev->dev, 1); + return 0; out_probe_notify: @@ -335,6 +335,8 @@ static int cros_ec_cec_remove(struct platform_device *pdev) cros_ec_cec->adap); cec_unregister_adapter(cros_ec_cec->adap); + device_init_wakeup(&pdev->dev, 0); + return 0; } -- 2.27.0.212.ge8ba1cc988-goog
Re: [PATCH] media: cros-ec-cec: disable the device wakeup on remove
On Mon, Jul 6, 2020 at 11:00 AM Alexandre Belloni wrote: > > On 06/07/2020 10:52:38+0200, Dariusz Marcinkiewicz wrote: > > The device is enabled for wake up while the driver is loaded. > > For symmetry, disable wakeup when it is removed. > > > > This is not necessary as it is handled by the core properly. There are > currently 73 calls to device_init_wakeup that are not necessary, this > would add one more. > Thank you. To make sure - your comment applies even if the device node in question is not actually removed, as it is the case here? This is a platform dev, which won't be freed when the driver is unloaded. Regards. > > Signed-off-by: Dariusz Marcinkiewicz > > --- > > drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 6 -- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > > b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > > index 2d95e16cd248..3881ed7bc3d9 100644 > > --- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > > +++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c > > @@ -277,8 +277,6 @@ static int cros_ec_cec_probe(struct platform_device > > *pdev) > > platform_set_drvdata(pdev, cros_ec_cec); > > cros_ec_cec->cros_ec = cros_ec; > > > > - device_init_wakeup(&pdev->dev, 1); > > - > > cros_ec_cec->adap = cec_allocate_adapter(&cros_ec_cec_ops, > > cros_ec_cec, > >DRV_NAME, > >CEC_CAP_DEFAULTS | > > @@ -306,6 +304,8 @@ static int cros_ec_cec_probe(struct platform_device > > *pdev) > > if (ret < 0) > > goto out_probe_notify; > > > > + device_init_wakeup(&pdev->dev, 1); > > + > > return 0; > > > > out_probe_notify: > > @@ -335,6 +335,8 @@ static int cros_ec_cec_remove(struct platform_device > > *pdev) > >cros_ec_cec->adap); > > cec_unregister_adapter(cros_ec_cec->adap); > > > > + device_init_wakeup(&pdev->dev, 0); > > + > > return 0; > > } > > > > -- > > 2.27.0.212.ge8ba1cc988-goog > > > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Re: [PATCH v7 1/3] media: cec: expose HDMI connector to CEC dev mapping
On Fri, May 24, 2019 at 11:21 AM Hans Verkuil wrote: > > Hi Dariusz, > > I did some more testing with the Khadas VIM2 and found another problem, > something that will, unfortunately, require some redesign. > ... > > The other problem is in the CEC driver: it creates the CEC device as > soon as the HDMI device is found (cec_notifier_parse_hdmi_phandle). > > But that doesn't mean that the HDMI device also had registered itself > as a CEC notifier. > > Until now that never mattered: as long as the HDMI device was found > the CEC adapter would function fine, it would just have no physical > address until so notified by the HDMI device once it registered its > CEC notifier. > > But if we want to have valid connector info during the lifetime of > the CEC adapter, then this no longer works. > > I'm not entirely sure how to handle this. > > Another issue here is that when the HDMI driver removes the notifier, > then it should also zero the connector info. Remember that both the > HDMI and the CEC drivers can be loaded and unloaded independently from > one another. > Given all of the above, what do you think about coming back to the v1 of the patch, where a connector info could be set on an adapter at any time and an event was used to notify userland when that happened? That approach seems to cover all the scenarios mentioned above. Thank you for testing the patches! Best regards.
[PATCH v5 2/3] drm/bridge: dw-hdmi: pass connector info to the CEC adapter
This patch makes dw-hdmi pass DRM connector info to a respective
CEC adapter. In order to be able to do that it delays creation of
the dw-hdmi-cec platform device until DRM connector is initialized.
Requires testing.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 5 +-
drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 2 +
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 82 +++
3 files changed, 52 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
index 84fb7b6a0a5e0..cf879629e0726 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
@@ -232,6 +232,7 @@ static void dw_hdmi_cec_del(void *data)
static int dw_hdmi_cec_probe(struct platform_device *pdev)
{
struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev);
+ struct cec_connector_info conn_info;
struct dw_hdmi_cec *cec;
int ret;
@@ -258,10 +259,12 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
dw_hdmi_write(cec, ~0, HDMI_IH_MUTE_CEC_STAT0);
dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY);
+ cec_fill_connector_info(&conn_info, data->connector);
+
cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi",
CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
CEC_CAP_RC | CEC_CAP_PASSTHROUGH,
-CEC_MAX_LOG_ADDRS, NULL);
+CEC_MAX_LOG_ADDRS, &conn_info);
if (IS_ERR(cec->adap))
return PTR_ERR(cec->adap);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
index cf4dc121a2c43..a2ac91ec845ed 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
@@ -1,6 +1,7 @@
#ifndef DW_HDMI_CEC_H
#define DW_HDMI_CEC_H
+struct drm_connector;
struct dw_hdmi;
struct dw_hdmi_cec_ops {
@@ -13,6 +14,7 @@ struct dw_hdmi_cec_ops {
struct dw_hdmi_cec_data {
struct dw_hdmi *hdmi;
const struct dw_hdmi_cec_ops *ops;
+ const struct drm_connector *connector;
int irq;
};
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index db761329a1e3e..6aaf0a7db9818 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -138,6 +138,8 @@ struct dw_hdmi {
struct drm_connector connector;
struct drm_bridge bridge;
+ int irq;
+
unsigned int version;
struct platform_device *audio;
@@ -189,6 +191,7 @@ struct dw_hdmi {
void (*enable_audio)(struct dw_hdmi *hdmi);
void (*disable_audio)(struct dw_hdmi *hdmi);
+ bool cec_configured;
struct cec_notifier *cec_notifier;
};
@@ -2105,6 +2108,29 @@ static const struct drm_connector_helper_funcs
dw_hdmi_connector_helper_funcs =
.get_modes = dw_hdmi_connector_get_modes,
};
+static void dw_hdmi_cec_enable(struct dw_hdmi *hdmi)
+{
+ mutex_lock(&hdmi->mutex);
+ hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_CECCLK_DISABLE;
+ hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
+ mutex_unlock(&hdmi->mutex);
+}
+
+static void dw_hdmi_cec_disable(struct dw_hdmi *hdmi)
+{
+ mutex_lock(&hdmi->mutex);
+ hdmi->mc_clkdis |= HDMI_MC_CLKDIS_CECCLK_DISABLE;
+ hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
+ mutex_unlock(&hdmi->mutex);
+}
+
+static const struct dw_hdmi_cec_ops dw_hdmi_cec_ops = {
+ .write = hdmi_writeb,
+ .read = hdmi_readb,
+ .enable = dw_hdmi_cec_enable,
+ .disable = dw_hdmi_cec_disable,
+};
+
static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
{
struct dw_hdmi *hdmi = bridge->driver_private;
@@ -2121,6 +2147,23 @@ static int dw_hdmi_bridge_attach(struct drm_bridge
*bridge)
drm_connector_attach_encoder(connector, encoder);
+ if (hdmi->cec_configured) {
+ struct platform_device_info pdevinfo;
+ struct dw_hdmi_cec_data cec;
+
+ cec.hdmi = hdmi;
+ cec.ops = &dw_hdmi_cec_ops;
+ cec.irq = hdmi->irq;
+ cec.connector = connector;
+
+ pdevinfo.name = "dw-hdmi-cec";
+ pdevinfo.data = &cec;
+ pdevinfo.size_data = sizeof(cec);
+ pdevinfo.dma_mask = 0;
+
+ hdmi->cec = platform_device_register_full(&pdevinfo);
+ }
+
return 0;
}
@@ -2390,29 +2433,6 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
return -ENODEV;
}
-static void dw_hdmi_cec_enable(struct dw_hdmi *hdmi)
-{
- mutex_l
[PATCH v5 1/3] media: cec: expose HDMI connector to CEC dev mapping
This patch proposes to expose explicit mapping between HDMI connectors and /dev/cecX adapters to userland. New structure with connector info (card number and connector id in case of DRM connectors) is added to cec_adapter. That connector info is expected to be provided when an adapter is created. CEC notifier is extended so that it can be used to communicate the connector's info to CEC adapters' creators. New ioctl, exposing connector info to userland, is added to /dev/cec. Changes since v4: - small tweaks + added documentation Changes since v3: - cec_get_connter_conn takes connector_info as argument Changes since v2: - cec_s_connector_info removed, the connector info is now passed to cec_allocate_adapter - updated commit message Changes since v1: - removed the unnecessary event, - extended cec_connctor_info to allow for various types of connectors. Signed-off-by: Dariusz Marcinkiewicz --- Documentation/media/kapi/cec-core.rst | 7 +- Documentation/media/uapi/cec/cec-funcs.rst| 1 + .../uapi/cec/cec-ioc-adap-g-conn-info.rst | 109 ++ .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 3 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 2 +- drivers/gpu/drm/drm_dp_cec.c | 22 ++-- drivers/gpu/drm/i2c/tda9950.c | 3 +- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_hdmi.c | 6 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 8 +- drivers/media/cec/cec-adap.c | 13 +++ drivers/media/cec/cec-api.c | 12 ++ drivers/media/cec/cec-core.c | 8 +- drivers/media/cec/cec-notifier.c | 20 +++- drivers/media/cec/cec-pin.c | 2 +- drivers/media/i2c/tc358743.c | 3 +- .../media/platform/cros-ec-cec/cros-ec-cec.c | 7 +- drivers/media/platform/meson/ao-cec.c | 6 +- drivers/media/platform/s5p-cec/s5p_cec.c | 6 +- drivers/media/platform/seco-cec/seco-cec.c| 8 +- drivers/media/platform/sti/cec/stih-cec.c | 6 +- drivers/media/platform/stm32/stm32-cec.c | 2 +- drivers/media/platform/tegra-cec/tegra_cec.c | 5 +- drivers/media/platform/vivid/vivid-cec.c | 2 +- drivers/media/usb/pulse8-cec/pulse8-cec.c | 3 +- .../media/usb/rainshadow-cec/rainshadow-cec.c | 3 +- include/drm/drm_dp_helper.h | 14 +-- include/media/cec-notifier.h | 34 -- include/media/cec.h | 16 ++- include/uapi/linux/cec.h | 24 32 files changed, 310 insertions(+), 54 deletions(-) create mode 100644 Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst diff --git a/Documentation/media/kapi/cec-core.rst b/Documentation/media/kapi/cec-core.rst index 3ce26b7c2b2b6..3678a0a75104e 100644 --- a/Documentation/media/kapi/cec-core.rst +++ b/Documentation/media/kapi/cec-core.rst @@ -37,7 +37,8 @@ calling cec_allocate_adapter() and deleted by calling cec_delete_adapter(): .. c:function:: struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops, void *priv, - const char *name, u32 caps, u8 available_las); +const char *name, u32 caps, u8 available_las, +const struct cec_connector_info *connector_info); .. c:function:: void cec_delete_adapter(struct cec_adapter *adap); @@ -65,6 +66,10 @@ available_las: the number of simultaneous logical addresses that this adapter can handle. Must be 1 <= available_las <= CEC_MAX_LOG_ADDRS. +connector_info: +pointer to a struct describing connector this adapter is associated with, +can be NULL. + To obtain the priv pointer use this helper function: .. c:function:: diff --git a/Documentation/media/uapi/cec/cec-funcs.rst b/Documentation/media/uapi/cec/cec-funcs.rst index 620590b168c9e..dc6da9c639a85 100644 --- a/Documentation/media/uapi/cec/cec-funcs.rst +++ b/Documentation/media/uapi/cec/cec-funcs.rst @@ -24,6 +24,7 @@ Function Reference cec-ioc-adap-g-caps cec-ioc-adap-g-log-addrs cec-ioc-adap-g-phys-addr +cec-ioc-adap-g-conn-info cec-ioc-dqevent cec-ioc-g-mode cec-ioc-receive diff --git a/Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst b/Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst new file mode 100644 index 0..87f475d7dfed4 --- /dev/null +++ b/Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst @@ -0,0 +1,109 @@ +.. SPDX-License-Identifier: GPL-2.0 +.. +.. Copyright 2019 Google LLC +.. +.. This documentation is free software; you can redistribute it and/or +.. modify it under the terms of the GNU General Public License +.. version 2 as published by the Free Sof
[PATCH v5 3/3] drm/i2c: tda9950: pass HDMI connector info to CEC adapter
With that change tda998x provides a connector info to the CEC
adapter. In order to be able to that it delays creation of
respective CEC device until the DRM connector is initialized.
Requires testing.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/i2c/tda9950.c | 7 -
drivers/gpu/drm/i2c/tda998x_drv.c | 41 +--
include/linux/platform_data/tda9950.h | 2 ++
3 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
index b944dd9df85e1..2778a0015cc31 100644
--- a/drivers/gpu/drm/i2c/tda9950.c
+++ b/drivers/gpu/drm/i2c/tda9950.c
@@ -382,6 +382,7 @@ static int tda9950_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct tda9950_glue *glue = client->dev.platform_data;
+ struct cec_connector_info conn_info;
struct device *dev = &client->dev;
struct tda9950_priv *priv;
unsigned long irqflags;
@@ -422,10 +423,14 @@ static int tda9950_probe(struct i2c_client *client,
if (glue && glue->parent)
priv->hdmi = glue->parent;
+ memset(&conn_info, 0, sizeof(conn_info));
+ if (glue)
+ cec_fill_connector_info(&conn_info, glue->connector);
+
priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
CEC_CAP_DEFAULTS,
CEC_MAX_LOG_ADDRS,
- NULL);
+ &conn_info);
if (IS_ERR(priv->adap))
return PTR_ERR(priv->adap);
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb5155..ef2aa3134b387 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1253,6 +1253,7 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
struct drm_device *drm)
{
struct drm_connector *connector = &priv->connector;
+ struct i2c_board_info cec_info;
int ret;
connector->interlace_allowed = 1;
@@ -1269,6 +1270,24 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
if (ret)
return ret;
+ /*
+* Some TDA998x are actually two I2C devices merged onto one piece
+* of silicon: TDA9989 and TDA19989 combine the HDMI transmitter
+* with a slightly modified TDA9950 CEC device. The CEC device
+* is at the TDA9950 address, with the address pins strapped across
+* to the TDA998x address pins. Hence, it always has the same
+* offset.
+*/
+ memset(&cec_info, 0, sizeof(cec_info));
+ strlcpy(cec_info.type, "tda9950", sizeof(cec_info.type));
+ cec_info.addr = priv->cec_addr;
+ cec_info.platform_data = &priv->cec_glue;
+ cec_info.irq = priv->hdmi->irq;
+
+ priv->cec = i2c_new_device(priv->hdmi->adapter, &cec_info);
+ if (!priv->cec)
+ return -ENODEV;
+
drm_connector_attach_encoder(&priv->connector,
priv->bridge.encoder);
@@ -1658,7 +1677,6 @@ static int tda998x_create(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct device_node *np = client->dev.of_node;
- struct i2c_board_info cec_info;
struct tda998x_priv *priv;
u32 video;
int rev_lo, rev_hi, ret;
@@ -1783,32 +1801,13 @@ static int tda998x_create(struct device *dev)
}
priv->cec_glue.parent = dev;
+ priv->cec_glue.connector = &priv->connector;
priv->cec_glue.data = priv;
priv->cec_glue.init = tda998x_cec_hook_init;
priv->cec_glue.exit = tda998x_cec_hook_exit;
priv->cec_glue.open = tda998x_cec_hook_open;
priv->cec_glue.release = tda998x_cec_hook_release;
- /*
-* Some TDA998x are actually two I2C devices merged onto one piece
-* of silicon: TDA9989 and TDA19989 combine the HDMI transmitter
-* with a slightly modified TDA9950 CEC device. The CEC device
-* is at the TDA9950 address, with the address pins strapped across
-* to the TDA998x address pins. Hence, it always has the same
-* offset.
-*/
- memset(&cec_info, 0, sizeof(cec_info));
- strlcpy(cec_info.type, "tda9950", sizeof(cec_info.type));
- cec_info.addr = priv->cec_addr;
- cec_info.platform_data = &priv->cec_glue;
- cec_info.irq = client->irq;
-
- priv->cec = i2c_new_device(client->adapter, &cec_info);
- if (!priv->cec) {
- ret = -ENODEV;
- goto fail;
- }
-
/* enable EDID read
Re: [RFC PATCH v2] media: cec: expose HDMI connector to CEC dev mapping
Hi Hans. From: Hans Verkuil Date: Thu, May 9, 2019 at 11:31 AM To: Dariusz Marcinkiewicz Cc: , , > On 5/9/19 9:52 AM, Dariusz Marcinkiewicz wrote: > > Hi Hans. > > > > On Wed, Apr 24, 2019 at 2:09 PM Hans Verkuil wrote: > >> > >> Hi Dariusz, > >> > >> This is getting close, so I think for the next version you can drop > >> the RFC tag. > >> > >> Some comments: > >> > > ... > >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c > >>> @@ -261,7 +261,7 @@ static int dw_hdmi_cec_probe(struct platform_device > >>> *pdev) > >>> cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi", > >>>CEC_CAP_LOG_ADDRS | > >>> CEC_CAP_TRANSMIT | > >>>CEC_CAP_RC | CEC_CAP_PASSTHROUGH, > >>> - CEC_MAX_LOG_ADDRS); > >>> + CEC_MAX_LOG_ADDRS, NULL); > >> > >> Hmm, the connector information is actually available through cec->hdmi. > >> > >> I think it would make sense to create a helper function that fills in > >> struct cec_connector_info based on a struct drm_connector pointer. > >> And add a function to drivers/gpu/drm/bridge/synopsys/dw-hdmi.c that > >> dw-hdmi-cec.c can call that does the same. > > > > Looking at the code here, is the connector info guaranteed to be > > available at the time cec_allocate_adapter is called here? > > drm_connector won't be initialized until dw_hdmi_bridge_attach is > > called, which happens after the cec platform device is created. > > Good point. The creation of the cec platform device should probably > be moved to dw_hdmi_bridge_attach. > > > ... > >>> priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950", > >>> CEC_CAP_DEFAULTS, > >>> - CEC_MAX_LOG_ADDRS); > >>> + CEC_MAX_LOG_ADDRS, > >>> + NULL); > >> > >> Here too the drm_connector can be found via struct tda9950_glue. > >> So it is easy to provide proper connector information. > > > > The same concern as with the comment before. > > Same solution: this has to be moved. > > I have hardware to test patches for both drivers. It might take 2-3 weeks > before I can test as I don't always has access to the hardware, but at > least I can verify that moving this code won't break anything. > > It's best to first move the code in separate patches before applying the > "expose HDMI connector to CEC dev mapping" patch on top of them. > I've submitted another revision of the changes, with those 2 patches added on top. Hope that is ok. Please take a look. It would be great if you could give those 2 patches a go on an actual hardware. Thank you and best regards.
[PATCH v6 2/3] drm/bridge: dw-hdmi: pass connector info to the CEC adapter
This patch makes dw-hdmi pass DRM connector info to a respective
CEC adapter. In order to be able to do that it delays creation of
the dw-hdmi-cec platform device until DRM connector is initialized.
Requires testing.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 5 +-
drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h | 2 +
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 82 +++
3 files changed, 52 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
index 84fb7b6a0a5e0..cf879629e0726 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
@@ -232,6 +232,7 @@ static void dw_hdmi_cec_del(void *data)
static int dw_hdmi_cec_probe(struct platform_device *pdev)
{
struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev);
+ struct cec_connector_info conn_info;
struct dw_hdmi_cec *cec;
int ret;
@@ -258,10 +259,12 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
dw_hdmi_write(cec, ~0, HDMI_IH_MUTE_CEC_STAT0);
dw_hdmi_write(cec, 0, HDMI_CEC_POLARITY);
+ cec_fill_connector_info(&conn_info, data->connector);
+
cec->adap = cec_allocate_adapter(&dw_hdmi_cec_ops, cec, "dw_hdmi",
CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
CEC_CAP_RC | CEC_CAP_PASSTHROUGH,
-CEC_MAX_LOG_ADDRS, NULL);
+CEC_MAX_LOG_ADDRS, &conn_info);
if (IS_ERR(cec->adap))
return PTR_ERR(cec->adap);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
index cf4dc121a2c43..a2ac91ec845ed 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.h
@@ -1,6 +1,7 @@
#ifndef DW_HDMI_CEC_H
#define DW_HDMI_CEC_H
+struct drm_connector;
struct dw_hdmi;
struct dw_hdmi_cec_ops {
@@ -13,6 +14,7 @@ struct dw_hdmi_cec_ops {
struct dw_hdmi_cec_data {
struct dw_hdmi *hdmi;
const struct dw_hdmi_cec_ops *ops;
+ const struct drm_connector *connector;
int irq;
};
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index ab7968c8f6a29..1275cb74299a8 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -138,6 +138,8 @@ struct dw_hdmi {
struct drm_connector connector;
struct drm_bridge bridge;
+ int irq;
+
unsigned int version;
struct platform_device *audio;
@@ -189,6 +191,7 @@ struct dw_hdmi {
void (*enable_audio)(struct dw_hdmi *hdmi);
void (*disable_audio)(struct dw_hdmi *hdmi);
+ bool cec_configured;
struct cec_notifier *cec_notifier;
};
@@ -2113,6 +2116,29 @@ static const struct drm_connector_helper_funcs
dw_hdmi_connector_helper_funcs =
.get_modes = dw_hdmi_connector_get_modes,
};
+static void dw_hdmi_cec_enable(struct dw_hdmi *hdmi)
+{
+ mutex_lock(&hdmi->mutex);
+ hdmi->mc_clkdis &= ~HDMI_MC_CLKDIS_CECCLK_DISABLE;
+ hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
+ mutex_unlock(&hdmi->mutex);
+}
+
+static void dw_hdmi_cec_disable(struct dw_hdmi *hdmi)
+{
+ mutex_lock(&hdmi->mutex);
+ hdmi->mc_clkdis |= HDMI_MC_CLKDIS_CECCLK_DISABLE;
+ hdmi_writeb(hdmi, hdmi->mc_clkdis, HDMI_MC_CLKDIS);
+ mutex_unlock(&hdmi->mutex);
+}
+
+static const struct dw_hdmi_cec_ops dw_hdmi_cec_ops = {
+ .write = hdmi_writeb,
+ .read = hdmi_readb,
+ .enable = dw_hdmi_cec_enable,
+ .disable = dw_hdmi_cec_disable,
+};
+
static int dw_hdmi_bridge_attach(struct drm_bridge *bridge)
{
struct dw_hdmi *hdmi = bridge->driver_private;
@@ -2129,6 +2155,23 @@ static int dw_hdmi_bridge_attach(struct drm_bridge
*bridge)
drm_connector_attach_encoder(connector, encoder);
+ if (hdmi->cec_configured) {
+ struct platform_device_info pdevinfo;
+ struct dw_hdmi_cec_data cec;
+
+ cec.hdmi = hdmi;
+ cec.ops = &dw_hdmi_cec_ops;
+ cec.irq = hdmi->irq;
+ cec.connector = connector;
+
+ pdevinfo.name = "dw-hdmi-cec";
+ pdevinfo.data = &cec;
+ pdevinfo.size_data = sizeof(cec);
+ pdevinfo.dma_mask = 0;
+
+ hdmi->cec = platform_device_register_full(&pdevinfo);
+ }
+
return 0;
}
@@ -2398,29 +2441,6 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
return -ENODEV;
}
-static void dw_hdmi_cec_enable(struct dw_hdmi *hdmi)
-{
- mutex_l
[PATCH v6 1/3] media: cec: expose HDMI connector to CEC dev mapping
This patch proposes to expose explicit mapping between HDMI connectors and /dev/cecX adapters to userland. New structure with connector info (card number and connector id in case of DRM connectors) is added to cec_adapter. That connector info is expected to be provided when an adapter is created. CEC notifier is extended so that it can be used to communicate the connector's info to CEC adapters' creators. New ioctl, exposing connector info to userland, is added to /dev/cec. Changes since v5: - make the patch apply against the latest changes in the affected code Changes since v4: - small tweaks + added documentation Changes since v3: - cec_get_connter_conn takes connector_info as argument Changes since v2: - cec_s_connector_info removed, the connector info is now passed to cec_allocate_adapter - updated commit message Changes since v1: - removed the unnecessary event, - extended cec_connctor_info to allow for various types of connectors. Signed-off-by: Dariusz Marcinkiewicz --- Documentation/media/kapi/cec-core.rst | 7 +- Documentation/media/uapi/cec/cec-funcs.rst| 1 + .../uapi/cec/cec-ioc-adap-g-conn-info.rst | 109 ++ .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 3 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c | 2 +- drivers/gpu/drm/drm_dp_cec.c | 22 ++-- drivers/gpu/drm/i2c/tda9950.c | 3 +- drivers/gpu/drm/i915/intel_dp.c | 4 +- drivers/gpu/drm/i915/intel_hdmi.c | 6 +- drivers/gpu/drm/nouveau/nouveau_connector.c | 3 +- drivers/gpu/drm/vc4/vc4_hdmi.c| 8 +- drivers/media/cec/cec-adap.c | 13 +++ drivers/media/cec/cec-api.c | 12 ++ drivers/media/cec/cec-core.c | 8 +- drivers/media/cec/cec-notifier.c | 20 +++- drivers/media/cec/cec-pin.c | 2 +- drivers/media/i2c/tc358743.c | 3 +- .../media/platform/cros-ec-cec/cros-ec-cec.c | 7 +- drivers/media/platform/meson/ao-cec.c | 6 +- drivers/media/platform/s5p-cec/s5p_cec.c | 6 +- drivers/media/platform/seco-cec/seco-cec.c| 8 +- drivers/media/platform/sti/cec/stih-cec.c | 6 +- drivers/media/platform/stm32/stm32-cec.c | 2 +- drivers/media/platform/tegra-cec/tegra_cec.c | 5 +- drivers/media/platform/vivid/vivid-cec.c | 2 +- drivers/media/usb/pulse8-cec/pulse8-cec.c | 3 +- .../media/usb/rainshadow-cec/rainshadow-cec.c | 3 +- include/drm/drm_dp_helper.h | 14 +-- include/media/cec-notifier.h | 34 -- include/media/cec.h | 16 ++- include/uapi/linux/cec.h | 24 32 files changed, 310 insertions(+), 54 deletions(-) create mode 100644 Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst diff --git a/Documentation/media/kapi/cec-core.rst b/Documentation/media/kapi/cec-core.rst index 3ce26b7c2b2b6..3678a0a75104e 100644 --- a/Documentation/media/kapi/cec-core.rst +++ b/Documentation/media/kapi/cec-core.rst @@ -37,7 +37,8 @@ calling cec_allocate_adapter() and deleted by calling cec_delete_adapter(): .. c:function:: struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops, void *priv, - const char *name, u32 caps, u8 available_las); +const char *name, u32 caps, u8 available_las, +const struct cec_connector_info *connector_info); .. c:function:: void cec_delete_adapter(struct cec_adapter *adap); @@ -65,6 +66,10 @@ available_las: the number of simultaneous logical addresses that this adapter can handle. Must be 1 <= available_las <= CEC_MAX_LOG_ADDRS. +connector_info: +pointer to a struct describing connector this adapter is associated with, +can be NULL. + To obtain the priv pointer use this helper function: .. c:function:: diff --git a/Documentation/media/uapi/cec/cec-funcs.rst b/Documentation/media/uapi/cec/cec-funcs.rst index 620590b168c9e..dc6da9c639a85 100644 --- a/Documentation/media/uapi/cec/cec-funcs.rst +++ b/Documentation/media/uapi/cec/cec-funcs.rst @@ -24,6 +24,7 @@ Function Reference cec-ioc-adap-g-caps cec-ioc-adap-g-log-addrs cec-ioc-adap-g-phys-addr +cec-ioc-adap-g-conn-info cec-ioc-dqevent cec-ioc-g-mode cec-ioc-receive diff --git a/Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst b/Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst new file mode 100644 index 0..87f475d7dfed4 --- /dev/null +++ b/Documentation/media/uapi/cec/cec-ioc-adap-g-conn-info.rst @@ -0,0 +1,109 @@ +.. SPDX-License-Identifier: GPL-2.0 +.. +.. Copyright 2019 Google LLC +.. +.. This documentation is free software; you can redistribute it and/or +.. modify it under
[PATCH v6 3/3] drm/i2c: tda9950: pass HDMI connector info to CEC adapter
With that change tda998x provides a connector info to the CEC
adapter. In order to be able to that it delays creation of
respective CEC device until the DRM connector is initialized.
Requires testing.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/gpu/drm/i2c/tda9950.c | 7 -
drivers/gpu/drm/i2c/tda998x_drv.c | 41 +--
include/linux/platform_data/tda9950.h | 2 ++
3 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
index b944dd9df85e1..2778a0015cc31 100644
--- a/drivers/gpu/drm/i2c/tda9950.c
+++ b/drivers/gpu/drm/i2c/tda9950.c
@@ -382,6 +382,7 @@ static int tda9950_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct tda9950_glue *glue = client->dev.platform_data;
+ struct cec_connector_info conn_info;
struct device *dev = &client->dev;
struct tda9950_priv *priv;
unsigned long irqflags;
@@ -422,10 +423,14 @@ static int tda9950_probe(struct i2c_client *client,
if (glue && glue->parent)
priv->hdmi = glue->parent;
+ memset(&conn_info, 0, sizeof(conn_info));
+ if (glue)
+ cec_fill_connector_info(&conn_info, glue->connector);
+
priv->adap = cec_allocate_adapter(&tda9950_cec_ops, priv, "tda9950",
CEC_CAP_DEFAULTS,
CEC_MAX_LOG_ADDRS,
- NULL);
+ &conn_info);
if (IS_ERR(priv->adap))
return PTR_ERR(priv->adap);
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
b/drivers/gpu/drm/i2c/tda998x_drv.c
index 7f34601bb5155..ef2aa3134b387 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -1253,6 +1253,7 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
struct drm_device *drm)
{
struct drm_connector *connector = &priv->connector;
+ struct i2c_board_info cec_info;
int ret;
connector->interlace_allowed = 1;
@@ -1269,6 +1270,24 @@ static int tda998x_connector_init(struct tda998x_priv
*priv,
if (ret)
return ret;
+ /*
+* Some TDA998x are actually two I2C devices merged onto one piece
+* of silicon: TDA9989 and TDA19989 combine the HDMI transmitter
+* with a slightly modified TDA9950 CEC device. The CEC device
+* is at the TDA9950 address, with the address pins strapped across
+* to the TDA998x address pins. Hence, it always has the same
+* offset.
+*/
+ memset(&cec_info, 0, sizeof(cec_info));
+ strlcpy(cec_info.type, "tda9950", sizeof(cec_info.type));
+ cec_info.addr = priv->cec_addr;
+ cec_info.platform_data = &priv->cec_glue;
+ cec_info.irq = priv->hdmi->irq;
+
+ priv->cec = i2c_new_device(priv->hdmi->adapter, &cec_info);
+ if (!priv->cec)
+ return -ENODEV;
+
drm_connector_attach_encoder(&priv->connector,
priv->bridge.encoder);
@@ -1658,7 +1677,6 @@ static int tda998x_create(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
struct device_node *np = client->dev.of_node;
- struct i2c_board_info cec_info;
struct tda998x_priv *priv;
u32 video;
int rev_lo, rev_hi, ret;
@@ -1783,32 +1801,13 @@ static int tda998x_create(struct device *dev)
}
priv->cec_glue.parent = dev;
+ priv->cec_glue.connector = &priv->connector;
priv->cec_glue.data = priv;
priv->cec_glue.init = tda998x_cec_hook_init;
priv->cec_glue.exit = tda998x_cec_hook_exit;
priv->cec_glue.open = tda998x_cec_hook_open;
priv->cec_glue.release = tda998x_cec_hook_release;
- /*
-* Some TDA998x are actually two I2C devices merged onto one piece
-* of silicon: TDA9989 and TDA19989 combine the HDMI transmitter
-* with a slightly modified TDA9950 CEC device. The CEC device
-* is at the TDA9950 address, with the address pins strapped across
-* to the TDA998x address pins. Hence, it always has the same
-* offset.
-*/
- memset(&cec_info, 0, sizeof(cec_info));
- strlcpy(cec_info.type, "tda9950", sizeof(cec_info.type));
- cec_info.addr = priv->cec_addr;
- cec_info.platform_data = &priv->cec_glue;
- cec_info.irq = client->irq;
-
- priv->cec = i2c_new_device(client->adapter, &cec_info);
- if (!priv->cec) {
- ret = -ENODEV;
- goto fail;
- }
-
/* enable EDID read
[PATCH RESEND] media: cros-ec-cec: do not bail on device_init_wakeup failure
Do not fail probing when device_init_wakeup fails.
device_init_wakeup fails when the device is already enabled as wakeup
device. Hence, the driver fails to probe the device if:
- The device has already been enabled for wakeup (via e.g. sysfs)
- The driver has been unloaded and is being loaded again.
This goal of the patch is to fix the above cases.
Overwhelming majority of the drivers do not check device_init_wakeup
return value.
Signed-off-by: Dariusz Marcinkiewicz
---
drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
index 0e7e2772f08f..2d95e16cd248 100644
--- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
+++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
@@ -277,11 +277,7 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, cros_ec_cec);
cros_ec_cec->cros_ec = cros_ec;
- ret = device_init_wakeup(&pdev->dev, 1);
- if (ret) {
- dev_err(&pdev->dev, "failed to initialize wakeup\n");
- return ret;
- }
+ device_init_wakeup(&pdev->dev, 1);
cros_ec_cec->adap = cec_allocate_adapter(&cros_ec_cec_ops, cros_ec_cec,
DRV_NAME,
--
2.27.0.290.gba653c62da-goog
Re: [PATCH RESEND] media: cros-ec-cec: do not bail on device_init_wakeup failure
Hello. I realized that I sent the previous answer using HTML, and as a consequence it was blocked from mailing lists. Sending it again (apologies for double posting). On Mon, Jun 22, 2020 at 12:23 PM Hans Verkuil wrote: > > On 22/06/2020 12:05, Dariusz Marcinkiewicz wrote: > > Do not fail probing when device_init_wakeup fails. > > > > device_init_wakeup fails when the device is already enabled as wakeup > > device. Hence, the driver fails to probe the device if: > > - The device has already been enabled for wakeup (via e.g. sysfs) > > - The driver has been unloaded and is being loaded again. > > > > This goal of the patch is to fix the above cases. > > > > Overwhelming majority of the drivers do not check device_init_wakeup > > return value. > > > > Signed-off-by: Dariusz Marcinkiewicz > > This can be CCed to stable, I guess? > That issue is not a recent regression but has been in there since the very beginning. So it might be argued that is it not severe enough to warrant cc'ing stable. Happy to do that anyways if you think otherwise. > Can you provide a Fixes: tag as well? > Done, submitted v2 with that a couple of days ago. Thank you and best regards.
