[1/3] drm/msm/dp: Simplify aux irq handling code

Message ID 20210507212505.1224111-2-swboyd@chromium.org
State New
Headers show
Series
  • drm/msm/dp: Simplify aux code
Related show

Commit Message

Stephen Boyd May 7, 2021, 9:25 p.m.
We don't need to stash away 'isr' in the aux structure to pass to two
functions. Let's use a local variable instead. And we can complete the
completion variable in one place instead of two to simplify the code.

Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Abhinav Kumar <abhinavk@codeaurora.org>
Cc: Kuogee Hsieh <khsieh@codeaurora.org>
Cc: aravindh@codeaurora.org
Cc: Sean Paul <sean@poorly.run>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/gpu/drm/msm/dp/dp_aux.c     | 22 ++++++++--------------
 drivers/gpu/drm/msm/dp/dp_catalog.c |  2 +-
 drivers/gpu/drm/msm/dp/dp_catalog.h |  2 +-
 3 files changed, 10 insertions(+), 16 deletions(-)

Comments

Kuogee Hsieh May 24, 2021, 4:22 p.m. | #1
On 2021-05-07 14:25, Stephen Boyd wrote:
> We don't need to stash away 'isr' in the aux structure to pass to two

> functions. Let's use a local variable instead. And we can complete the

> completion variable in one place instead of two to simplify the code.

> 

> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> Cc: Abhinav Kumar <abhinavk@codeaurora.org>

> Cc: Kuogee Hsieh <khsieh@codeaurora.org>

> Cc: aravindh@codeaurora.org

> Cc: Sean Paul <sean@poorly.run>

> Signed-off-by: Stephen Boyd <swboyd@chromium.org>


Reviewed-by: Kuogee Hsieh <khsieh@codeaurora.org>

> ---

>  drivers/gpu/drm/msm/dp/dp_aux.c     | 22 ++++++++--------------

>  drivers/gpu/drm/msm/dp/dp_catalog.c |  2 +-

>  drivers/gpu/drm/msm/dp/dp_catalog.h |  2 +-

>  3 files changed, 10 insertions(+), 16 deletions(-)

> 

> diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c 

> b/drivers/gpu/drm/msm/dp/dp_aux.c

> index 7c22bfe0fc7d..91188466cece 100644

> --- a/drivers/gpu/drm/msm/dp/dp_aux.c

> +++ b/drivers/gpu/drm/msm/dp/dp_aux.c

> @@ -27,7 +27,6 @@ struct dp_aux_private {

>  	bool no_send_stop;

>  	u32 offset;

>  	u32 segment;

> -	u32 isr;

> 

>  	struct drm_dp_aux dp_aux;

>  };

> @@ -181,10 +180,8 @@ static void dp_aux_cmd_fifo_rx(struct 

> dp_aux_private *aux,

>  	}

>  }

> 

> -static void dp_aux_native_handler(struct dp_aux_private *aux)

> +static void dp_aux_native_handler(struct dp_aux_private *aux, u32 isr)

>  {

> -	u32 isr = aux->isr;

> -

>  	if (isr & DP_INTR_AUX_I2C_DONE)

>  		aux->aux_error_num = DP_AUX_ERR_NONE;

>  	else if (isr & DP_INTR_WRONG_ADDR)

> @@ -197,14 +194,10 @@ static void dp_aux_native_handler(struct

> dp_aux_private *aux)

>  		aux->aux_error_num = DP_AUX_ERR_PHY;

>  		dp_catalog_aux_clear_hw_interrupts(aux->catalog);

>  	}

> -

> -	complete(&aux->comp);

>  }

> 

> -static void dp_aux_i2c_handler(struct dp_aux_private *aux)

> +static void dp_aux_i2c_handler(struct dp_aux_private *aux, u32 isr)

>  {

> -	u32 isr = aux->isr;

> -

>  	if (isr & DP_INTR_AUX_I2C_DONE) {

>  		if (isr & (DP_INTR_I2C_NACK | DP_INTR_I2C_DEFER))

>  			aux->aux_error_num = DP_AUX_ERR_NACK;

> @@ -226,8 +219,6 @@ static void dp_aux_i2c_handler(struct 

> dp_aux_private *aux)

>  			dp_catalog_aux_clear_hw_interrupts(aux->catalog);

>  		}

>  	}

> -

> -	complete(&aux->comp);

>  }

> 

>  static void dp_aux_update_offset_and_segment(struct dp_aux_private 

> *aux,

> @@ -412,6 +403,7 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux 

> *dp_aux,

> 

>  void dp_aux_isr(struct drm_dp_aux *dp_aux)

>  {

> +	u32 isr;

>  	struct dp_aux_private *aux;

> 

>  	if (!dp_aux) {

> @@ -421,15 +413,17 @@ void dp_aux_isr(struct drm_dp_aux *dp_aux)

> 

>  	aux = container_of(dp_aux, struct dp_aux_private, dp_aux);

> 

> -	aux->isr = dp_catalog_aux_get_irq(aux->catalog);

> +	isr = dp_catalog_aux_get_irq(aux->catalog);

> 

>  	if (!aux->cmd_busy)

>  		return;

> 

>  	if (aux->native)

> -		dp_aux_native_handler(aux);

> +		dp_aux_native_handler(aux, isr);

>  	else

> -		dp_aux_i2c_handler(aux);

> +		dp_aux_i2c_handler(aux, isr);

> +

> +	complete(&aux->comp);

>  }

> 

>  void dp_aux_reconfig(struct drm_dp_aux *dp_aux)

> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c

> b/drivers/gpu/drm/msm/dp/dp_catalog.c

> index b1a9b1b98f5f..a70c238f34b0 100644

> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c

> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c

> @@ -292,7 +292,7 @@ void dp_catalog_dump_regs(struct dp_catalog 

> *dp_catalog)

>  	dump_regs(catalog->io->dp_controller.base + offset, len);

>  }

> 

> -int dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog)

> +u32 dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog)

>  {

>  	struct dp_catalog_private *catalog = container_of(dp_catalog,

>  				struct dp_catalog_private, dp_catalog);

> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h

> b/drivers/gpu/drm/msm/dp/dp_catalog.h

> index 176a9020a520..502bc0dc7787 100644

> --- a/drivers/gpu/drm/msm/dp/dp_catalog.h

> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h

> @@ -80,7 +80,7 @@ int dp_catalog_aux_clear_hw_interrupts(struct

> dp_catalog *dp_catalog);

>  void dp_catalog_aux_reset(struct dp_catalog *dp_catalog);

>  void dp_catalog_aux_enable(struct dp_catalog *dp_catalog, bool 

> enable);

>  void dp_catalog_aux_update_cfg(struct dp_catalog *dp_catalog);

> -int dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog);

> +u32 dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog);

> 

>  /* DP Controller APIs */

>  void dp_catalog_ctrl_state_ctrl(struct dp_catalog *dp_catalog, u32 

> state);

Patch

diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c
index 7c22bfe0fc7d..91188466cece 100644
--- a/drivers/gpu/drm/msm/dp/dp_aux.c
+++ b/drivers/gpu/drm/msm/dp/dp_aux.c
@@ -27,7 +27,6 @@  struct dp_aux_private {
 	bool no_send_stop;
 	u32 offset;
 	u32 segment;
-	u32 isr;
 
 	struct drm_dp_aux dp_aux;
 };
@@ -181,10 +180,8 @@  static void dp_aux_cmd_fifo_rx(struct dp_aux_private *aux,
 	}
 }
 
-static void dp_aux_native_handler(struct dp_aux_private *aux)
+static void dp_aux_native_handler(struct dp_aux_private *aux, u32 isr)
 {
-	u32 isr = aux->isr;
-
 	if (isr & DP_INTR_AUX_I2C_DONE)
 		aux->aux_error_num = DP_AUX_ERR_NONE;
 	else if (isr & DP_INTR_WRONG_ADDR)
@@ -197,14 +194,10 @@  static void dp_aux_native_handler(struct dp_aux_private *aux)
 		aux->aux_error_num = DP_AUX_ERR_PHY;
 		dp_catalog_aux_clear_hw_interrupts(aux->catalog);
 	}
-
-	complete(&aux->comp);
 }
 
-static void dp_aux_i2c_handler(struct dp_aux_private *aux)
+static void dp_aux_i2c_handler(struct dp_aux_private *aux, u32 isr)
 {
-	u32 isr = aux->isr;
-
 	if (isr & DP_INTR_AUX_I2C_DONE) {
 		if (isr & (DP_INTR_I2C_NACK | DP_INTR_I2C_DEFER))
 			aux->aux_error_num = DP_AUX_ERR_NACK;
@@ -226,8 +219,6 @@  static void dp_aux_i2c_handler(struct dp_aux_private *aux)
 			dp_catalog_aux_clear_hw_interrupts(aux->catalog);
 		}
 	}
-
-	complete(&aux->comp);
 }
 
 static void dp_aux_update_offset_and_segment(struct dp_aux_private *aux,
@@ -412,6 +403,7 @@  static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux,
 
 void dp_aux_isr(struct drm_dp_aux *dp_aux)
 {
+	u32 isr;
 	struct dp_aux_private *aux;
 
 	if (!dp_aux) {
@@ -421,15 +413,17 @@  void dp_aux_isr(struct drm_dp_aux *dp_aux)
 
 	aux = container_of(dp_aux, struct dp_aux_private, dp_aux);
 
-	aux->isr = dp_catalog_aux_get_irq(aux->catalog);
+	isr = dp_catalog_aux_get_irq(aux->catalog);
 
 	if (!aux->cmd_busy)
 		return;
 
 	if (aux->native)
-		dp_aux_native_handler(aux);
+		dp_aux_native_handler(aux, isr);
 	else
-		dp_aux_i2c_handler(aux);
+		dp_aux_i2c_handler(aux, isr);
+
+	complete(&aux->comp);
 }
 
 void dp_aux_reconfig(struct drm_dp_aux *dp_aux)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c
index b1a9b1b98f5f..a70c238f34b0 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -292,7 +292,7 @@  void dp_catalog_dump_regs(struct dp_catalog *dp_catalog)
 	dump_regs(catalog->io->dp_controller.base + offset, len);
 }
 
-int dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog)
+u32 dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog)
 {
 	struct dp_catalog_private *catalog = container_of(dp_catalog,
 				struct dp_catalog_private, dp_catalog);
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h
index 176a9020a520..502bc0dc7787 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.h
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.h
@@ -80,7 +80,7 @@  int dp_catalog_aux_clear_hw_interrupts(struct dp_catalog *dp_catalog);
 void dp_catalog_aux_reset(struct dp_catalog *dp_catalog);
 void dp_catalog_aux_enable(struct dp_catalog *dp_catalog, bool enable);
 void dp_catalog_aux_update_cfg(struct dp_catalog *dp_catalog);
-int dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog);
+u32 dp_catalog_aux_get_irq(struct dp_catalog *dp_catalog);
 
 /* DP Controller APIs */
 void dp_catalog_ctrl_state_ctrl(struct dp_catalog *dp_catalog, u32 state);