diff mbox series

[1/2] drm/tegra: dpaux: Populate AUX bus

Message ID 20211220104855.428290-2-thierry.reding@gmail.com
State Accepted
Headers show
Series drm/tegra: Fix panel support on Venice 2 and Nyan | expand

Commit Message

Thierry Reding Dec. 20, 2021, 10:48 a.m. UTC
From: Thierry Reding <treding@nvidia.com>

The DPAUX hardware block exposes an DP AUX interface that provides
access to an AUX bus and the devices on that bus. Use the DP AUX bus
infrastructure that was recently introduced to probe devices on this
bus from DT.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/Kconfig | 1 +
 drivers/gpu/drm/tegra/dpaux.c | 7 +++++++
 2 files changed, 8 insertions(+)

Comments

Dmitry Osipenko Dec. 22, 2021, 7:48 p.m. UTC | #1
20.12.2021 13:48, Thierry Reding пишет:
> From: Thierry Reding <treding@nvidia.com>
> 
> The DPAUX hardware block exposes an DP AUX interface that provides
> access to an AUX bus and the devices on that bus. Use the DP AUX bus
> infrastructure that was recently introduced to probe devices on this
> bus from DT.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/Kconfig | 1 +
>  drivers/gpu/drm/tegra/dpaux.c | 7 +++++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
> index 8cf5aeb9db6c..201f5175ecfe 100644
> --- a/drivers/gpu/drm/tegra/Kconfig
> +++ b/drivers/gpu/drm/tegra/Kconfig
> @@ -5,6 +5,7 @@ config DRM_TEGRA
>  	depends on COMMON_CLK
>  	depends on DRM
>  	depends on OF
> +	select DRM_DP_AUX_BUS
>  	select DRM_KMS_HELPER
>  	select DRM_MIPI_DSI
>  	select DRM_PANEL
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index 1f96e416fa08..9da1edcdc835 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -18,6 +18,7 @@
>  #include <linux/reset.h>
>  #include <linux/workqueue.h>
>  
> +#include <drm/drm_dp_aux_bus.h>
>  #include <drm/drm_dp_helper.h>
>  #include <drm/drm_panel.h>
>  
> @@ -570,6 +571,12 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>  	list_add_tail(&dpaux->list, &dpaux_list);
>  	mutex_unlock(&dpaux_lock);
>  
> +	err = devm_of_dp_aux_populate_ep_devices(&dpaux->aux);
> +	if (err < 0) {
> +		dev_err(dpaux->dev, "failed to populate AUX bus: %d\n", err);
> +		return err;
> +	}
> +
>  	return 0;
>  }

Needs stable tag for 5.15+.
Doug Anderson Jan. 6, 2022, 1:02 a.m. UTC | #2
Hi,

On Mon, Dec 20, 2021 at 2:49 AM Thierry Reding <thierry.reding@gmail.com> wrote:
>
> From: Thierry Reding <treding@nvidia.com>
>
> The DPAUX hardware block exposes an DP AUX interface that provides
> access to an AUX bus and the devices on that bus. Use the DP AUX bus
> infrastructure that was recently introduced to probe devices on this
> bus from DT.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/Kconfig | 1 +
>  drivers/gpu/drm/tegra/dpaux.c | 7 +++++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
> index 8cf5aeb9db6c..201f5175ecfe 100644
> --- a/drivers/gpu/drm/tegra/Kconfig
> +++ b/drivers/gpu/drm/tegra/Kconfig
> @@ -5,6 +5,7 @@ config DRM_TEGRA
>         depends on COMMON_CLK
>         depends on DRM
>         depends on OF
> +       select DRM_DP_AUX_BUS
>         select DRM_KMS_HELPER
>         select DRM_MIPI_DSI
>         select DRM_PANEL
> diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
> index 1f96e416fa08..9da1edcdc835 100644
> --- a/drivers/gpu/drm/tegra/dpaux.c
> +++ b/drivers/gpu/drm/tegra/dpaux.c
> @@ -18,6 +18,7 @@
>  #include <linux/reset.h>
>  #include <linux/workqueue.h>
>
> +#include <drm/drm_dp_aux_bus.h>
>  #include <drm/drm_dp_helper.h>
>  #include <drm/drm_panel.h>
>
> @@ -570,6 +571,12 @@ static int tegra_dpaux_probe(struct platform_device *pdev)
>         list_add_tail(&dpaux->list, &dpaux_list);
>         mutex_unlock(&dpaux_lock);
>
> +       err = devm_of_dp_aux_populate_ep_devices(&dpaux->aux);
> +       if (err < 0) {
> +               dev_err(dpaux->dev, "failed to populate AUX bus: %d\n", err);
> +               return err;

nit:

if (err < 0)
  return dev_err_probe(dpaux->dev, err, "failed to populate AUX bus\n");

That's more concise and gives the pretty formatting for the error code
(not that devm_of_dp_aux_populate_ep_devices() currently returns
anything other than 0).

> +       }

I think you need to use of_dp_aux_populate_ep_devices() and then
depopulate in your remove, right? Otherwise the ordering won't be
right. Either that or fully transition your probe to devm (and get rid
of your remove) by adding some devm_add_action_or_reset() helpers...

Hrm, I guess looking closer it's also a little concerning that if
devm_of_dp_aux_populate_ep_devices() returns an error then you'll
totally skip all the things that tegra_dpaux_remove() is supposed to
handle. I guess that's a pre-existing bug (earlier error cases in the
probe don't, for instance, undo the pm_runtime state) but this makes
it worse.


-Doug
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 8cf5aeb9db6c..201f5175ecfe 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -5,6 +5,7 @@  config DRM_TEGRA
 	depends on COMMON_CLK
 	depends on DRM
 	depends on OF
+	select DRM_DP_AUX_BUS
 	select DRM_KMS_HELPER
 	select DRM_MIPI_DSI
 	select DRM_PANEL
diff --git a/drivers/gpu/drm/tegra/dpaux.c b/drivers/gpu/drm/tegra/dpaux.c
index 1f96e416fa08..9da1edcdc835 100644
--- a/drivers/gpu/drm/tegra/dpaux.c
+++ b/drivers/gpu/drm/tegra/dpaux.c
@@ -18,6 +18,7 @@ 
 #include <linux/reset.h>
 #include <linux/workqueue.h>
 
+#include <drm/drm_dp_aux_bus.h>
 #include <drm/drm_dp_helper.h>
 #include <drm/drm_panel.h>
 
@@ -570,6 +571,12 @@  static int tegra_dpaux_probe(struct platform_device *pdev)
 	list_add_tail(&dpaux->list, &dpaux_list);
 	mutex_unlock(&dpaux_lock);
 
+	err = devm_of_dp_aux_populate_ep_devices(&dpaux->aux);
+	if (err < 0) {
+		dev_err(dpaux->dev, "failed to populate AUX bus: %d\n", err);
+		return err;
+	}
+
 	return 0;
 }