Re: [PATCH 09/15] drm/i915/bios: interpret the i2c element

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Dec 21, 2015 at 03:11:00PM +0200, Jani Nikula wrote:
> Add parsing of the i2c element, defined in MIPI sequence block v2. Drop
> the status operation byte while at it, that does not exist.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_bios.c | 5 +++++
>  drivers/gpu/drm/i915/intel_bios.h | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index d6eaf32f33e5..45a7a2bc96c6 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -812,6 +812,11 @@ static int goto_next_sequence(const u8 *data, int index, int total)
>  		case MIPI_SEQ_ELEM_GPIO:
>  			len = 2;

Somewhat off topic, but I wonder if this is correct. The "structure"
diagram shows it as 2 bytes for v1 and v2, but I'm not sure that section
isn't there just as an example. Later the describing the GPIO block it
seems to say it's 2 bytes for v1, and three bytes for v2.

>  			break;
> +		case MIPI_SEQ_ELEM_I2C:
> +			if (index + 7 > total)
> +				return 0;
> +			len = *(data + index + 6) + 7;
> +			break;

This one isn't show in the structure diagrams at all, so I guess we get
to trust the other section. It says this was introduced in v2. Should be
add a paranoia check for that?

Should we also check that the payload length is below the specified max
of 240 bytes?

>  		default:
>  			DRM_ERROR("Unknown operation byte\n");
>  			return 0;
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 4e87df16e7b3..411b33794536 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -968,7 +968,7 @@ enum mipi_seq_element {
>  	MIPI_SEQ_ELEM_SEND_PKT,
>  	MIPI_SEQ_ELEM_DELAY,
>  	MIPI_SEQ_ELEM_GPIO,
> -	MIPI_SEQ_ELEM_STATUS,
> +	MIPI_SEQ_ELEM_I2C,		/* sequence block v2+ */
>  	MIPI_SEQ_ELEM_MAX
>  };
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx