Re: [PATCH 3/3] drm/ast: Define struct ast_ddc in ast_ddc.c

[Jocelyn Falempe <jfalempe@xxxxxxxxxx>]

Hi,

Thanks for the patch, it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@xxxxxxxxxx>

--

Jocelyn

On 03/04/2024 12:31, Thomas Zimmermann wrote:
> Move the definition of struct ast_ddc to ast_ddc.c and return the i2c
> adapter from ast_ddc_create(). Update callers accordingly. Avoids
> including Linux i2c header files, except where required. No functional
> changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
>   drivers/gpu/drm/ast/ast_ddc.c  | 14 ++++++++++++--
>   drivers/gpu/drm/ast/ast_ddc.h  | 13 ++-----------
>   drivers/gpu/drm/ast/ast_mode.c |  8 ++++----
>   3 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_ddc.c b/drivers/gpu/drm/ast/ast_ddc.c
> index 4df52aeba4f7e..29cf5d157f344 100644
> --- a/drivers/gpu/drm/ast/ast_ddc.c
> +++ b/drivers/gpu/drm/ast/ast_ddc.c
> @@ -21,12 +21,22 @@
>    * of the Software.
>    */
>   
> +#include <linux/i2c-algo-bit.h>
> +#include <linux/i2c.h>
> +
>   #include <drm/drm_managed.h>
>   #include <drm/drm_print.h>
>   
>   #include "ast_ddc.h"
>   #include "ast_drv.h"
>   
> +struct ast_ddc {
> +	struct ast_device *ast;
> +
> +	struct i2c_algo_bit_data bit;
> +	struct i2c_adapter adapter;
> +};
> +
>   static void ast_ddc_algo_bit_data_setsda(void *data, int state)
>   {
>   	struct ast_ddc *ddc = data;
> @@ -132,7 +142,7 @@ static void ast_ddc_release(struct drm_device *dev, void *res)
>   	i2c_del_adapter(&ddc->adapter);
>   }
>   
> -struct ast_ddc *ast_ddc_create(struct ast_device *ast)
> +struct i2c_adapter *ast_ddc_create(struct ast_device *ast)
>   {
>   	struct drm_device *dev = &ast->base;
>   	struct ast_ddc *ddc;
> @@ -173,5 +183,5 @@ struct ast_ddc *ast_ddc_create(struct ast_device *ast)
>   	if (ret)
>   		return ERR_PTR(ret);
>   
> -	return ddc;
> +	return &ddc->adapter;
>   }
> diff --git a/drivers/gpu/drm/ast/ast_ddc.h b/drivers/gpu/drm/ast/ast_ddc.h
> index 08f3994e09ccd..85c93edc9ae12 100644
> --- a/drivers/gpu/drm/ast/ast_ddc.h
> +++ b/drivers/gpu/drm/ast/ast_ddc.h
> @@ -3,18 +3,9 @@
>   #ifndef __AST_DDC_H__
>   #define __AST_DDC_H__
>   
> -#include <linux/i2c.h>
> -#include <linux/i2c-algo-bit.h>
> -
>   struct ast_device;
> +struct i2c_adapter;
>   
> -struct ast_ddc {
> -	struct ast_device *ast;
> -
> -	struct i2c_adapter adapter;
> -	struct i2c_algo_bit_data bit;
> -};
> -
> -struct ast_ddc *ast_ddc_create(struct ast_device *ast);
> +struct i2c_adapter *ast_ddc_create(struct ast_device *ast);
>   
>   #endif
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index bb9b66aba9ee9..ae21d89b899c8 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1360,7 +1360,7 @@ static const struct drm_connector_funcs ast_vga_connector_funcs = {
>   static int ast_vga_connector_init(struct drm_device *dev, struct drm_connector *connector)
>   {
>   	struct ast_device *ast = to_ast_device(dev);
> -	struct ast_ddc *ddc;
> +	struct i2c_adapter *ddc;
>   	int ret;
>   
>   	ddc = ast_ddc_create(ast);
> @@ -1371,7 +1371,7 @@ static int ast_vga_connector_init(struct drm_device *dev, struct drm_connector *
>   	}
>   
>   	ret = drm_connector_init_with_ddc(dev, connector, &ast_vga_connector_funcs,
> -					  DRM_MODE_CONNECTOR_VGA, &ddc->adapter);
> +					  DRM_MODE_CONNECTOR_VGA, ddc);
>   	if (ret)
>   		return ret;
>   
> @@ -1429,7 +1429,7 @@ static const struct drm_connector_funcs ast_sil164_connector_funcs = {
>   static int ast_sil164_connector_init(struct drm_device *dev, struct drm_connector *connector)
>   {
>   	struct ast_device *ast = to_ast_device(dev);
> -	struct ast_ddc *ddc;
> +	struct i2c_adapter *ddc;
>   	int ret;
>   
>   	ddc = ast_ddc_create(ast);
> @@ -1440,7 +1440,7 @@ static int ast_sil164_connector_init(struct drm_device *dev, struct drm_connecto
>   	}
>   
>   	ret = drm_connector_init_with_ddc(dev, connector, &ast_sil164_connector_funcs,
> -					  DRM_MODE_CONNECTOR_DVII, &ddc->adapter);
> +					  DRM_MODE_CONNECTOR_DVII, ddc);
>   	if (ret)
>   		return ret;
>