Re: [PATCH 3/8] drm/i915: Adding the parsing logic for the i2c element

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 04 Feb 2016, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> On Thu, Feb 04, 2016 at 12:50:51PM +0200, Jani Nikula wrote:
>> From: vkorjani <vikas.korjani@xxxxxxxxx>
>> 
>> New sequence element for i2c is been added in the
>> mipi sequence block of the VBT. This patch parses
>> and executes the i2c sequence.
>> 
>> v2: Add i2c_put_adapter call(Jani), rebase
>> 
>> v3: corrected the retry loop(Jani), rebase
>> 
>> v4 by Jani:
>>  - don't put the adapter if get fails
>>  - print an error message if all retries exhausted
>>  - use a for loop
>>  - fix warnings for unused variables
>> 
>> v5 by Jani:
>>  - rebase on the skip i2c element patch
>> 
>> v6: by Jani:
>>  - ignore the gmbus i2c elements (Ville)
>> 
>> Signed-off-by: vkorjani <vikas.korjani@xxxxxxxxx>
>> Signed-off-by: Deepak M <m.deepak@xxxxxxxxx>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>>  drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 64 ++++++++++++++++++++++++++++--
>>  1 file changed, 61 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> index 6f013efba45b..f4d303ee538b 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c
>> @@ -31,6 +31,7 @@
>>  #include <drm/drm_panel.h>
>>  #include <linux/slab.h>
>>  #include <video/mipi_display.h>
>> +#include <linux/i2c.h>
>>  #include <asm/intel-mid.h>
>>  #include <video/mipi_display.h>
>>  #include "i915_drv.h"
>> @@ -235,9 +236,66 @@ out:
>>  	return data;
>>  }
>>  
>> -static const u8 *mipi_exec_i2c_skip(struct intel_dsi *intel_dsi, const u8 *data)
>> +static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 *data)
>>  {
>> -	return data + *(data + 6) + 7;
>> +	struct i2c_adapter *adapter;
>> +	int ret, i;
>> +	u8 reg_offset, payload_size;
>> +	struct i2c_msg msg;
>> +	u8 *transmit_buffer;
>> +	u8 flag, resource_id, bus_number;
>> +	u16 slave_add;
>> +
>> +	flag = *data++;
>> +	resource_id = *data++;
>> +	bus_number = *data++;
>> +	slave_add = *(u16 *)(data);
>> +	data += 2;
>> +	reg_offset = *data++;
>> +	payload_size = *data++;
>> +
>> +	if (resource_id == 0xff || bus_number == 0xff) {
>> +		DRM_DEBUG_KMS("ignoring gmbus (resource id %02x, bus %02x)\n",
>> +			      resource_id, bus_number);
>> +		goto out;
>> +	}
>> +
>
> Still missing the check for __i2c_first_dynamic_bus_num which I think
> would at least provide some kind of half arsed protection against
> something not registering these magic i2c busses.

I meant to reply to that part of the review but forgot.

Problem is, we'd have to include drivers/i2c/i2c-core.h directly, which
also has this warning,

/* These symbols are exported ONLY FOR the i2c core.
 * No other users will be supported.
 */

and there are no users outside of drivers/i2c. I'm quite reluctant to
add that.


BR,
Jani.


>
>> +	adapter = i2c_get_adapter(bus_number);
>> +	if (!adapter) {
>> +		DRM_ERROR("i2c_get_adapter(%u)\n", bus_number);
>> +		goto out;
>> +	}
>> +
>> +	transmit_buffer = kmalloc(1 + payload_size, GFP_TEMPORARY);
>> +	if (!transmit_buffer)
>> +		goto out_put;
>> +
>> +	transmit_buffer[0] = reg_offset;
>> +	memcpy(&transmit_buffer[1], data, payload_size);
>> +
>> +	msg.addr = slave_add;
>> +	msg.flags = 0;
>> +	msg.len = payload_size + 1;
>> +	msg.buf = &transmit_buffer[0];
>> +
>> +	for (i = 0; i < 6; i++) {
>> +		ret = i2c_transfer(adapter, &msg, 1);
>> +		if (ret == 1) {
>> +			goto out_free;
>> +		} else if (ret == -EAGAIN) {
>> +			usleep_range(1000, 2500);
>> +		} else {
>> +			break;
>> +		}
>> +	}
>> +
>> +	DRM_ERROR("i2c transfer failed: %d\n", ret);
>> +out_free:
>> +	kfree(transmit_buffer);
>> +out_put:
>> +	i2c_put_adapter(adapter);
>> +out:
>> +	return data + payload_size;
>>  }
>>  
>>  typedef const u8 * (*fn_mipi_elem_exec)(struct intel_dsi *intel_dsi,
>> @@ -246,7 +304,7 @@ static const fn_mipi_elem_exec exec_elem[] = {
>>  	[MIPI_SEQ_ELEM_SEND_PKT] = mipi_exec_send_packet,
>>  	[MIPI_SEQ_ELEM_DELAY] = mipi_exec_delay,
>>  	[MIPI_SEQ_ELEM_GPIO] = mipi_exec_gpio,
>> -	[MIPI_SEQ_ELEM_I2C] = mipi_exec_i2c_skip,
>> +	[MIPI_SEQ_ELEM_I2C] = mipi_exec_i2c,
>>  };
>>  
>>  /*
>> -- 
>> 2.1.4
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux