| Message ID | 20210330115200.26006-1-dafna.hirschfeld@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: bridge: rename the function drm_bridge_hpd_notify to drm_bridge_hpd_cb | expand |
Hey Dafna, Thanks for submitting a cleanup patch, it is much appreciated. It looks good to me, feel free to add my r-b. I'm not going to merge this right away, but will let this patch soak for a while to let other people have a look at it. On Tue, 30 Mar 2021 at 13:52, Dafna Hirschfeld <dafna.hirschfeld@collabora.com> wrote: > > drm_bridge_funcs has a function called 'hpd_notify'. > The function drm_bridge_hpd_notify does not call > 'hpd_notify' but it calls 'hpd_cb'. This is rather > confusing. Rename the function to fix this confusion. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- > drivers/gpu/drm/bridge/display-connector.c | 2 +- > drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- > drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- > drivers/gpu/drm/drm_bridge.c | 8 ++++---- > include/drm/drm_bridge.h | 8 ++++---- > 8 files changed, 18 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 76555ae64e9c..748f82910f4f 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) > cec_phys_addr_invalidate(adv7511->cec_adap); > drm_kms_helper_hotplug_event(adv7511->connector.dev); > } else { > - drm_bridge_hpd_notify(&adv7511->bridge, status); > + drm_bridge_hpd_cb(&adv7511->bridge, status); > } > } > } > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > index d0c65610ebb5..682da288ff6d 100644 > --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) > if (mhdp->connector.dev) > drm_kms_helper_hotplug_event(mhdp->bridge.dev); > else > - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > } > } > > @@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) > else > drm_kms_helper_hotplug_event(mhdp->bridge.dev); > } else { > - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > } > } > > diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c > index 05eb759da6fc..8ccd69d7fe34 100644 > --- a/drivers/gpu/drm/bridge/display-connector.c > +++ b/drivers/gpu/drm/bridge/display-connector.c > @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) > struct display_connector *conn = arg; > struct drm_bridge *bridge = &conn->bridge; > > - drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); > + drm_bridge_hpd_cb(bridge, display_connector_detect(bridge)); > > return IRQ_HANDLED; > } > diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > index fee27952ec6d..58f61b5da605 100644 > --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) > connected = lt9611uxc->hdmi_connected; > mutex_unlock(<9611uxc->ocm_lock); > > - drm_bridge_hpd_notify(<9611uxc->bridge, > - connected ? > - connector_status_connected : > - connector_status_disconnected); > + drm_bridge_hpd_cb(<9611uxc->bridge, > + connected ? > + connector_status_connected : > + connector_status_disconnected); > } > } > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index dda4fa9a1a08..984ab5c4bc71 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) > > if (hdmi->bridge.dev) { > drm_helper_hpd_irq_event(hdmi->bridge.dev); > - drm_bridge_hpd_notify(&hdmi->bridge, status); > + drm_bridge_hpd_cb(&hdmi->bridge, status); > } > } > > diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c > index e0e015243a60..2f079b6f51bc 100644 > --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c > +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c > @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) > struct tpd12s015_device *tpd = data; > struct drm_bridge *bridge = &tpd->bridge; > > - drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge)); > + drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge)); > > return IRQ_HANDLED; > } > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index 64f0effb52ac..653761a0d5f9 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) > EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); > > /** > - * drm_bridge_hpd_notify - notify hot plug detection events > + * drm_bridge_hpd_cb - notify hot plug detection events > * @bridge: bridge control structure > * @status: output connection status > * > @@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); > * > * This function shall be called in a context that can sleep. > */ > -void drm_bridge_hpd_notify(struct drm_bridge *bridge, > - enum drm_connector_status status) > +void drm_bridge_hpd_cb(struct drm_bridge *bridge, > + enum drm_connector_status status) > { > mutex_lock(&bridge->hpd_mutex); > if (bridge->hpd_cb) > bridge->hpd_cb(bridge->hpd_data, status); > mutex_unlock(&bridge->hpd_mutex); > } > -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); > +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb); > > #ifdef CONFIG_OF > /** > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 2195daa289d2..ab54715eda8b 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -605,7 +605,7 @@ struct drm_bridge_funcs { > * @hpd_enable: > * > * Enable hot plug detection. From now on the bridge shall call > - * drm_bridge_hpd_notify() each time a change is detected in the output > + * drm_bridge_hpd_cb() each time a change is detected in the output > * connection status, until hot plug detection gets disabled with > * @hpd_disable. > * > @@ -620,7 +620,7 @@ struct drm_bridge_funcs { > * @hpd_disable: > * > * Disable hot plug detection. Once this function returns the bridge > - * shall not call drm_bridge_hpd_notify() when a change in the output > + * shall not call drm_bridge_hpd_cb() when a change in the output > * connection status occurs. > * > * This callback is optional and shall only be implemented by bridges > @@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, > enum drm_connector_status status), > void *data); > void drm_bridge_hpd_disable(struct drm_bridge *bridge); > -void drm_bridge_hpd_notify(struct drm_bridge *bridge, > - enum drm_connector_status status); > +void drm_bridge_hpd_cb(struct drm_bridge *bridge, > + enum drm_connector_status status); > > #ifdef CONFIG_DRM_PANEL_BRIDGE > struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel); > -- > 2.17.1 >
Reviewed-by: Robert Foss <robert.foss@linaro.org> On Tue, 30 Mar 2021 at 14:28, Robert Foss <robert.foss@linaro.org> wrote: > > Hey Dafna, > > Thanks for submitting a cleanup patch, it is much appreciated. It > looks good to me, feel free to add my r-b. > > I'm not going to merge this right away, but will let this patch soak > for a while to let other people have a look at it. > > On Tue, 30 Mar 2021 at 13:52, Dafna Hirschfeld > <dafna.hirschfeld@collabora.com> wrote: > > > > drm_bridge_funcs has a function called 'hpd_notify'. > > The function drm_bridge_hpd_notify does not call > > 'hpd_notify' but it calls 'hpd_cb'. This is rather > > confusing. Rename the function to fix this confusion. > > > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > > --- > > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > > drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- > > drivers/gpu/drm/bridge/display-connector.c | 2 +- > > drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- > > drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- > > drivers/gpu/drm/drm_bridge.c | 8 ++++---- > > include/drm/drm_bridge.h | 8 ++++---- > > 8 files changed, 18 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > index 76555ae64e9c..748f82910f4f 100644 > > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > > @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) > > cec_phys_addr_invalidate(adv7511->cec_adap); > > drm_kms_helper_hotplug_event(adv7511->connector.dev); > > } else { > > - drm_bridge_hpd_notify(&adv7511->bridge, status); > > + drm_bridge_hpd_cb(&adv7511->bridge, status); > > } > > } > > } > > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > > index d0c65610ebb5..682da288ff6d 100644 > > --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > > @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) > > if (mhdp->connector.dev) > > drm_kms_helper_hotplug_event(mhdp->bridge.dev); > > else > > - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > > + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > > } > > } > > > > @@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) > > else > > drm_kms_helper_hotplug_event(mhdp->bridge.dev); > > } else { > > - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > > + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > > } > > } > > > > diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c > > index 05eb759da6fc..8ccd69d7fe34 100644 > > --- a/drivers/gpu/drm/bridge/display-connector.c > > +++ b/drivers/gpu/drm/bridge/display-connector.c > > @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) > > struct display_connector *conn = arg; > > struct drm_bridge *bridge = &conn->bridge; > > > > - drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); > > + drm_bridge_hpd_cb(bridge, display_connector_detect(bridge)); > > > > return IRQ_HANDLED; > > } > > diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > > index fee27952ec6d..58f61b5da605 100644 > > --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > > +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > > @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) > > connected = lt9611uxc->hdmi_connected; > > mutex_unlock(<9611uxc->ocm_lock); > > > > - drm_bridge_hpd_notify(<9611uxc->bridge, > > - connected ? > > - connector_status_connected : > > - connector_status_disconnected); > > + drm_bridge_hpd_cb(<9611uxc->bridge, > > + connected ? > > + connector_status_connected : > > + connector_status_disconnected); > > } > > } > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > index dda4fa9a1a08..984ab5c4bc71 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) > > > > if (hdmi->bridge.dev) { > > drm_helper_hpd_irq_event(hdmi->bridge.dev); > > - drm_bridge_hpd_notify(&hdmi->bridge, status); > > + drm_bridge_hpd_cb(&hdmi->bridge, status); > > } > > } > > > > diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c > > index e0e015243a60..2f079b6f51bc 100644 > > --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c > > +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c > > @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) > > struct tpd12s015_device *tpd = data; > > struct drm_bridge *bridge = &tpd->bridge; > > > > - drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge)); > > + drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge)); > > > > return IRQ_HANDLED; > > } > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > > index 64f0effb52ac..653761a0d5f9 100644 > > --- a/drivers/gpu/drm/drm_bridge.c > > +++ b/drivers/gpu/drm/drm_bridge.c > > @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) > > EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); > > > > /** > > - * drm_bridge_hpd_notify - notify hot plug detection events > > + * drm_bridge_hpd_cb - notify hot plug detection events > > * @bridge: bridge control structure > > * @status: output connection status > > * > > @@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); > > * > > * This function shall be called in a context that can sleep. > > */ > > -void drm_bridge_hpd_notify(struct drm_bridge *bridge, > > - enum drm_connector_status status) > > +void drm_bridge_hpd_cb(struct drm_bridge *bridge, > > + enum drm_connector_status status) > > { > > mutex_lock(&bridge->hpd_mutex); > > if (bridge->hpd_cb) > > bridge->hpd_cb(bridge->hpd_data, status); > > mutex_unlock(&bridge->hpd_mutex); > > } > > -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); > > +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb); > > > > #ifdef CONFIG_OF > > /** > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > > index 2195daa289d2..ab54715eda8b 100644 > > --- a/include/drm/drm_bridge.h > > +++ b/include/drm/drm_bridge.h > > @@ -605,7 +605,7 @@ struct drm_bridge_funcs { > > * @hpd_enable: > > * > > * Enable hot plug detection. From now on the bridge shall call > > - * drm_bridge_hpd_notify() each time a change is detected in the output > > + * drm_bridge_hpd_cb() each time a change is detected in the output > > * connection status, until hot plug detection gets disabled with > > * @hpd_disable. > > * > > @@ -620,7 +620,7 @@ struct drm_bridge_funcs { > > * @hpd_disable: > > * > > * Disable hot plug detection. Once this function returns the bridge > > - * shall not call drm_bridge_hpd_notify() when a change in the output > > + * shall not call drm_bridge_hpd_cb() when a change in the output > > * connection status occurs. > > * > > * This callback is optional and shall only be implemented by bridges > > @@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, > > enum drm_connector_status status), > > void *data); > > void drm_bridge_hpd_disable(struct drm_bridge *bridge); > > -void drm_bridge_hpd_notify(struct drm_bridge *bridge, > > - enum drm_connector_status status); > > +void drm_bridge_hpd_cb(struct drm_bridge *bridge, > > + enum drm_connector_status status); > > > > #ifdef CONFIG_DRM_PANEL_BRIDGE > > struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel); > > -- > > 2.17.1 > >
Hi Dafna, Thank you for the patch. On Tue, Mar 30, 2021 at 01:52:00PM +0200, Dafna Hirschfeld wrote: > drm_bridge_funcs has a function called 'hpd_notify'. > The function drm_bridge_hpd_notify does not call > 'hpd_notify' but it calls 'hpd_cb'. This is rather > confusing. Rename the function to fix this confusion. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> > --- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- > drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- > drivers/gpu/drm/bridge/display-connector.c | 2 +- > drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- > drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- > drivers/gpu/drm/drm_bridge.c | 8 ++++---- > include/drm/drm_bridge.h | 8 ++++---- > 8 files changed, 18 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > index 76555ae64e9c..748f82910f4f 100644 > --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c > @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) > cec_phys_addr_invalidate(adv7511->cec_adap); > drm_kms_helper_hotplug_event(adv7511->connector.dev); > } else { > - drm_bridge_hpd_notify(&adv7511->bridge, status); > + drm_bridge_hpd_cb(&adv7511->bridge, status); > } > } > } > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > index d0c65610ebb5..682da288ff6d 100644 > --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) > if (mhdp->connector.dev) > drm_kms_helper_hotplug_event(mhdp->bridge.dev); > else > - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > } > } > > @@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) > else > drm_kms_helper_hotplug_event(mhdp->bridge.dev); > } else { > - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); > } > } > > diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c > index 05eb759da6fc..8ccd69d7fe34 100644 > --- a/drivers/gpu/drm/bridge/display-connector.c > +++ b/drivers/gpu/drm/bridge/display-connector.c > @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) > struct display_connector *conn = arg; > struct drm_bridge *bridge = &conn->bridge; > > - drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); > + drm_bridge_hpd_cb(bridge, display_connector_detect(bridge)); > > return IRQ_HANDLED; > } > diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > index fee27952ec6d..58f61b5da605 100644 > --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c > @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) > connected = lt9611uxc->hdmi_connected; > mutex_unlock(<9611uxc->ocm_lock); > > - drm_bridge_hpd_notify(<9611uxc->bridge, > - connected ? > - connector_status_connected : > - connector_status_disconnected); > + drm_bridge_hpd_cb(<9611uxc->bridge, > + connected ? > + connector_status_connected : > + connector_status_disconnected); > } > } > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index dda4fa9a1a08..984ab5c4bc71 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) > > if (hdmi->bridge.dev) { > drm_helper_hpd_irq_event(hdmi->bridge.dev); > - drm_bridge_hpd_notify(&hdmi->bridge, status); > + drm_bridge_hpd_cb(&hdmi->bridge, status); > } > } > > diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c > index e0e015243a60..2f079b6f51bc 100644 > --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c > +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c > @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) > struct tpd12s015_device *tpd = data; > struct drm_bridge *bridge = &tpd->bridge; > > - drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge)); > + drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge)); > > return IRQ_HANDLED; > } > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index 64f0effb52ac..653761a0d5f9 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) > EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); > > /** > - * drm_bridge_hpd_notify - notify hot plug detection events > + * drm_bridge_hpd_cb - notify hot plug detection events This function is still documented as notifying hot plug detection events, so drm_bridge_hpd_cb() isn't a great name :-S I do agree there's confusion with the current naming scheme though. bridge->hpd_cb() is an internal callback, not part of bridge ops, so I'd rather not expose its name in the public drm_bridge_hpd_notify() API. Could we find a better naming scheme ? > * @bridge: bridge control structure > * @status: output connection status > * > @@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); > * > * This function shall be called in a context that can sleep. > */ > -void drm_bridge_hpd_notify(struct drm_bridge *bridge, > - enum drm_connector_status status) > +void drm_bridge_hpd_cb(struct drm_bridge *bridge, > + enum drm_connector_status status) > { > mutex_lock(&bridge->hpd_mutex); > if (bridge->hpd_cb) > bridge->hpd_cb(bridge->hpd_data, status); > mutex_unlock(&bridge->hpd_mutex); > } > -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); > +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb); > > #ifdef CONFIG_OF > /** > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 2195daa289d2..ab54715eda8b 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -605,7 +605,7 @@ struct drm_bridge_funcs { > * @hpd_enable: > * > * Enable hot plug detection. From now on the bridge shall call > - * drm_bridge_hpd_notify() each time a change is detected in the output > + * drm_bridge_hpd_cb() each time a change is detected in the output > * connection status, until hot plug detection gets disabled with > * @hpd_disable. > * > @@ -620,7 +620,7 @@ struct drm_bridge_funcs { > * @hpd_disable: > * > * Disable hot plug detection. Once this function returns the bridge > - * shall not call drm_bridge_hpd_notify() when a change in the output > + * shall not call drm_bridge_hpd_cb() when a change in the output > * connection status occurs. > * > * This callback is optional and shall only be implemented by bridges > @@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, > enum drm_connector_status status), > void *data); > void drm_bridge_hpd_disable(struct drm_bridge *bridge); > -void drm_bridge_hpd_notify(struct drm_bridge *bridge, > - enum drm_connector_status status); > +void drm_bridge_hpd_cb(struct drm_bridge *bridge, > + enum drm_connector_status status); > > #ifdef CONFIG_DRM_PANEL_BRIDGE > struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
Hi On 06.04.21 14:40, Laurent Pinchart wrote: > Hi Dafna, > > Thank you for the patch. > > On Tue, Mar 30, 2021 at 01:52:00PM +0200, Dafna Hirschfeld wrote: >> drm_bridge_funcs has a function called 'hpd_notify'. >> The function drm_bridge_hpd_notify does not call >> 'hpd_notify' but it calls 'hpd_cb'. This is rather >> confusing. Rename the function to fix this confusion. >> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> >> --- >> drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- >> drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- >> drivers/gpu/drm/bridge/display-connector.c | 2 +- >> drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- >> drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- >> drivers/gpu/drm/drm_bridge.c | 8 ++++---- >> include/drm/drm_bridge.h | 8 ++++---- >> 8 files changed, 18 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> index 76555ae64e9c..748f82910f4f 100644 >> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c >> @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) >> cec_phys_addr_invalidate(adv7511->cec_adap); >> drm_kms_helper_hotplug_event(adv7511->connector.dev); >> } else { >> - drm_bridge_hpd_notify(&adv7511->bridge, status); >> + drm_bridge_hpd_cb(&adv7511->bridge, status); >> } >> } >> } >> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >> index d0c65610ebb5..682da288ff6d 100644 >> --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >> +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c >> @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) >> if (mhdp->connector.dev) >> drm_kms_helper_hotplug_event(mhdp->bridge.dev); >> else >> - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); >> + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); >> } >> } >> >> @@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) >> else >> drm_kms_helper_hotplug_event(mhdp->bridge.dev); >> } else { >> - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); >> + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); >> } >> } >> >> diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c >> index 05eb759da6fc..8ccd69d7fe34 100644 >> --- a/drivers/gpu/drm/bridge/display-connector.c >> +++ b/drivers/gpu/drm/bridge/display-connector.c >> @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) >> struct display_connector *conn = arg; >> struct drm_bridge *bridge = &conn->bridge; >> >> - drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); >> + drm_bridge_hpd_cb(bridge, display_connector_detect(bridge)); >> >> return IRQ_HANDLED; >> } >> diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c >> index fee27952ec6d..58f61b5da605 100644 >> --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c >> +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c >> @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) >> connected = lt9611uxc->hdmi_connected; >> mutex_unlock(<9611uxc->ocm_lock); >> >> - drm_bridge_hpd_notify(<9611uxc->bridge, >> - connected ? >> - connector_status_connected : >> - connector_status_disconnected); >> + drm_bridge_hpd_cb(<9611uxc->bridge, >> + connected ? >> + connector_status_connected : >> + connector_status_disconnected); >> } >> } >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> index dda4fa9a1a08..984ab5c4bc71 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c >> @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) >> >> if (hdmi->bridge.dev) { >> drm_helper_hpd_irq_event(hdmi->bridge.dev); >> - drm_bridge_hpd_notify(&hdmi->bridge, status); >> + drm_bridge_hpd_cb(&hdmi->bridge, status); >> } >> } >> >> diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c >> index e0e015243a60..2f079b6f51bc 100644 >> --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c >> +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c >> @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) >> struct tpd12s015_device *tpd = data; >> struct drm_bridge *bridge = &tpd->bridge; >> >> - drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge)); >> + drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge)); >> >> return IRQ_HANDLED; >> } >> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c >> index 64f0effb52ac..653761a0d5f9 100644 >> --- a/drivers/gpu/drm/drm_bridge.c >> +++ b/drivers/gpu/drm/drm_bridge.c >> @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) >> EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); >> >> /** >> - * drm_bridge_hpd_notify - notify hot plug detection events >> + * drm_bridge_hpd_cb - notify hot plug detection events > > This function is still documented as notifying hot plug detection > events, so drm_bridge_hpd_cb() isn't a great name :-S I do agree there's > confusion with the current naming scheme though. > > bridge->hpd_cb() is an internal callback, not part of bridge ops, so I'd > rather not expose its name in the public drm_bridge_hpd_notify() API. > Could we find a better naming scheme ? I that bridge->funcs->hpd_notify is called only from drm_bridge_connector_hpd_notify which is called from both the hpd cb and from the 'detect' of the bridge connector. But 'detect' is not necesseraly by hpd right? I mean there is other ways apart of hpd to detect a connctor right? If that is the case then bridge->funcs->hpd_notify is also not a good name. Also, I see that in some places the function drm_bridge_connector_init is called without later calling the function drm_bridge_connector_enable_hpd. Is there a use case in which we want to create a bridge connector without enabeling the hpd_cb ? Regarding the naming, maybe we can replace 'drm_bridge_hpd_notify' with 'drm_bridge_hpd' ? Thanks, Dafna > >> * @bridge: bridge control structure >> * @status: output connection status >> * >> @@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); >> * >> * This function shall be called in a context that can sleep. >> */ >> -void drm_bridge_hpd_notify(struct drm_bridge *bridge, >> - enum drm_connector_status status) >> +void drm_bridge_hpd_cb(struct drm_bridge *bridge, >> + enum drm_connector_status status) >> { >> mutex_lock(&bridge->hpd_mutex); >> if (bridge->hpd_cb) >> bridge->hpd_cb(bridge->hpd_data, status); >> mutex_unlock(&bridge->hpd_mutex); >> } >> -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); >> +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb); >> >> #ifdef CONFIG_OF >> /** >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h >> index 2195daa289d2..ab54715eda8b 100644 >> --- a/include/drm/drm_bridge.h >> +++ b/include/drm/drm_bridge.h >> @@ -605,7 +605,7 @@ struct drm_bridge_funcs { >> * @hpd_enable: >> * >> * Enable hot plug detection. From now on the bridge shall call >> - * drm_bridge_hpd_notify() each time a change is detected in the output >> + * drm_bridge_hpd_cb() each time a change is detected in the output >> * connection status, until hot plug detection gets disabled with >> * @hpd_disable. >> * >> @@ -620,7 +620,7 @@ struct drm_bridge_funcs { >> * @hpd_disable: >> * >> * Disable hot plug detection. Once this function returns the bridge >> - * shall not call drm_bridge_hpd_notify() when a change in the output >> + * shall not call drm_bridge_hpd_cb() when a change in the output >> * connection status occurs. >> * >> * This callback is optional and shall only be implemented by bridges >> @@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, >> enum drm_connector_status status), >> void *data); >> void drm_bridge_hpd_disable(struct drm_bridge *bridge); >> -void drm_bridge_hpd_notify(struct drm_bridge *bridge, >> - enum drm_connector_status status); >> +void drm_bridge_hpd_cb(struct drm_bridge *bridge, >> + enum drm_connector_status status); >> >> #ifdef CONFIG_DRM_PANEL_BRIDGE >> struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel); >
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index 76555ae64e9c..748f82910f4f 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -449,7 +449,7 @@ static void adv7511_hpd_work(struct work_struct *work) cec_phys_addr_invalidate(adv7511->cec_adap); drm_kms_helper_hotplug_event(adv7511->connector.dev); } else { - drm_bridge_hpd_notify(&adv7511->bridge, status); + drm_bridge_hpd_cb(&adv7511->bridge, status); } } } diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c index d0c65610ebb5..682da288ff6d 100644 --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c @@ -794,7 +794,7 @@ static void cdns_mhdp_fw_cb(const struct firmware *fw, void *context) if (mhdp->connector.dev) drm_kms_helper_hotplug_event(mhdp->bridge.dev); else - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); } } @@ -2314,7 +2314,7 @@ static irqreturn_t cdns_mhdp_irq_handler(int irq, void *data) else drm_kms_helper_hotplug_event(mhdp->bridge.dev); } else { - drm_bridge_hpd_notify(&mhdp->bridge, cdns_mhdp_detect(mhdp)); + drm_bridge_hpd_cb(&mhdp->bridge, cdns_mhdp_detect(mhdp)); } } diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c index 05eb759da6fc..8ccd69d7fe34 100644 --- a/drivers/gpu/drm/bridge/display-connector.c +++ b/drivers/gpu/drm/bridge/display-connector.c @@ -98,7 +98,7 @@ static irqreturn_t display_connector_hpd_irq(int irq, void *arg) struct display_connector *conn = arg; struct drm_bridge *bridge = &conn->bridge; - drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); + drm_bridge_hpd_cb(bridge, display_connector_detect(bridge)); return IRQ_HANDLED; } diff --git a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c index fee27952ec6d..58f61b5da605 100644 --- a/drivers/gpu/drm/bridge/lontium-lt9611uxc.c +++ b/drivers/gpu/drm/bridge/lontium-lt9611uxc.c @@ -175,10 +175,10 @@ static void lt9611uxc_hpd_work(struct work_struct *work) connected = lt9611uxc->hdmi_connected; mutex_unlock(<9611uxc->ocm_lock); - drm_bridge_hpd_notify(<9611uxc->bridge, - connected ? - connector_status_connected : - connector_status_disconnected); + drm_bridge_hpd_cb(<9611uxc->bridge, + connected ? + connector_status_connected : + connector_status_disconnected); } } diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..984ab5c4bc71 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -3026,7 +3026,7 @@ static irqreturn_t dw_hdmi_irq(int irq, void *dev_id) if (hdmi->bridge.dev) { drm_helper_hpd_irq_event(hdmi->bridge.dev); - drm_bridge_hpd_notify(&hdmi->bridge, status); + drm_bridge_hpd_cb(&hdmi->bridge, status); } } diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c index e0e015243a60..2f079b6f51bc 100644 --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c @@ -103,7 +103,7 @@ static irqreturn_t tpd12s015_hpd_isr(int irq, void *data) struct tpd12s015_device *tpd = data; struct drm_bridge *bridge = &tpd->bridge; - drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge)); + drm_bridge_hpd_cb(bridge, tpd12s015_detect(bridge)); return IRQ_HANDLED; } diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 64f0effb52ac..653761a0d5f9 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -1173,7 +1173,7 @@ void drm_bridge_hpd_disable(struct drm_bridge *bridge) EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); /** - * drm_bridge_hpd_notify - notify hot plug detection events + * drm_bridge_hpd_cb - notify hot plug detection events * @bridge: bridge control structure * @status: output connection status * @@ -1183,15 +1183,15 @@ EXPORT_SYMBOL_GPL(drm_bridge_hpd_disable); * * This function shall be called in a context that can sleep. */ -void drm_bridge_hpd_notify(struct drm_bridge *bridge, - enum drm_connector_status status) +void drm_bridge_hpd_cb(struct drm_bridge *bridge, + enum drm_connector_status status) { mutex_lock(&bridge->hpd_mutex); if (bridge->hpd_cb) bridge->hpd_cb(bridge->hpd_data, status); mutex_unlock(&bridge->hpd_mutex); } -EXPORT_SYMBOL_GPL(drm_bridge_hpd_notify); +EXPORT_SYMBOL_GPL(drm_bridge_hpd_cb); #ifdef CONFIG_OF /** diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index 2195daa289d2..ab54715eda8b 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -605,7 +605,7 @@ struct drm_bridge_funcs { * @hpd_enable: * * Enable hot plug detection. From now on the bridge shall call - * drm_bridge_hpd_notify() each time a change is detected in the output + * drm_bridge_hpd_cb() each time a change is detected in the output * connection status, until hot plug detection gets disabled with * @hpd_disable. * @@ -620,7 +620,7 @@ struct drm_bridge_funcs { * @hpd_disable: * * Disable hot plug detection. Once this function returns the bridge - * shall not call drm_bridge_hpd_notify() when a change in the output + * shall not call drm_bridge_hpd_cb() when a change in the output * connection status occurs. * * This callback is optional and shall only be implemented by bridges @@ -878,8 +878,8 @@ void drm_bridge_hpd_enable(struct drm_bridge *bridge, enum drm_connector_status status), void *data); void drm_bridge_hpd_disable(struct drm_bridge *bridge); -void drm_bridge_hpd_notify(struct drm_bridge *bridge, - enum drm_connector_status status); +void drm_bridge_hpd_cb(struct drm_bridge *bridge, + enum drm_connector_status status); #ifdef CONFIG_DRM_PANEL_BRIDGE struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel);
drm_bridge_funcs has a function called 'hpd_notify'. The function drm_bridge_hpd_notify does not call 'hpd_notify' but it calls 'hpd_cb'. This is rather confusing. Rename the function to fix this confusion. Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@collabora.com> --- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 2 +- drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c | 4 ++-- drivers/gpu/drm/bridge/display-connector.c | 2 +- drivers/gpu/drm/bridge/lontium-lt9611uxc.c | 8 ++++---- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/ti-tpd12s015.c | 2 +- drivers/gpu/drm/drm_bridge.c | 8 ++++---- include/drm/drm_bridge.h | 8 ++++---- 8 files changed, 18 insertions(+), 18 deletions(-)