Re: [PATCH 1/4] drm/dp: add flags field to drm_dp_aux_msg struct

[Jani Nikula <jani.nikula@linux.intel.com>]

On Sat, 22 Mar 2014, Alex Deucher <alexdeucher@gmail.com> wrote:
> This adds a flags field and a new flag, BARE_ADDRESS,
> which drivers can use for special handling when they
> want to set just the aux address.  This is needed
> to properly reset the connection between i2c transactions.

Sorry it took me so long to get to this.

The changes in patches 1-3 look sensible in general, but I think I'd
prefer you dropped the flags field and used size == 0 to mean bare
address. It feels silly to have to set size = 1 and have a dummy one
byte buffer that doesn't get transfered. Without the payload I think it
feels natural only the address is transfered.

BR,
Jani.


>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  include/drm/drm_dp_helper.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index b7488c9..a006e96 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -403,6 +403,8 @@ drm_dp_enhanced_frame_cap(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
>   * DisplayPort AUX channel
>   */
>  
> +#define DRM_DP_AUX_MSG_FLAGS_BARE_ADDRESS (1 << 0)
> +
>  /**
>   * struct drm_dp_aux_msg - DisplayPort AUX channel transaction
>   * @address: address of the (first) register to access
> @@ -417,6 +419,7 @@ struct drm_dp_aux_msg {
>  	u8 reply;
>  	void *buffer;
>  	size_t size;
> +	u32 flags;
>  };
>  
>  /**
> -- 
> 1.8.3.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center