>-----Original Message----- >From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] >Sent: Thursday, September 17, 2015 2:48 PM >To: Deepak, M; intel-gfx@xxxxxxxxxxxxxxxxxxxxx >Cc: Deepak, M >Subject: Re: [MIPI SEQ PARSING v2 PATCH 01/11] drm/i915: Adding >the parsing logic for the i2c element > >On Thu, 10 Sep 2015, Deepak M <m.deepak@xxxxxxxxx> 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 >> >> Signed-off-by: vkorjani <vikas.korjani@xxxxxxxxx> >> Signed-off-by: Deepak M <m.deepak@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_bios.c | 6 +++ >> drivers/gpu/drm/i915/intel_bios.h | 1 + >> drivers/gpu/drm/i915/intel_dsi_panel_vbt.c | 61 >++++++++++++++++++++++++++++ >> 3 files changed, 68 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/intel_bios.c >> b/drivers/gpu/drm/i915/intel_bios.c >> index c8acc29..0bf0942 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.c >> +++ b/drivers/gpu/drm/i915/intel_bios.c >> @@ -714,6 +714,12 @@ static u8 *goto_next_sequence(u8 *data, int >> *size) >> >> data += 3; >> break; >> + case MIPI_SEQ_ELEM_I2C: >> + /* skip by this element payload size */ >> + data += 7; >> + len = *data; >> + data += len + 1; >> + break; >> default: >> DRM_ERROR("Unknown element\n"); >> return NULL; >> diff --git a/drivers/gpu/drm/i915/intel_bios.h >> b/drivers/gpu/drm/i915/intel_bios.h >> index 1b7417e..21a7f3f 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.h >> +++ b/drivers/gpu/drm/i915/intel_bios.h >> @@ -956,6 +956,7 @@ enum mipi_seq_element { >> MIPI_SEQ_ELEM_SEND_PKT, >> MIPI_SEQ_ELEM_DELAY, >> MIPI_SEQ_ELEM_GPIO, >> + MIPI_SEQ_ELEM_I2C, >> MIPI_SEQ_ELEM_STATUS, > >Side note, MIPI_SEQ_ELEM_STATUS doesn't seem to be in spec. > > >> MIPI_SEQ_ELEM_MAX >> }; >> diff --git a/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> b/drivers/gpu/drm/i915/intel_dsi_panel_vbt.c >> index a5e99ac..9989f61 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" >> @@ -104,6 +105,65 @@ static struct gpio_table gtable[] = { >> { GPIO_NC_11_PCONF0, GPIO_NC_11_PAD, 0} }; >> >> +static const u8 *mipi_exec_i2c(struct intel_dsi *intel_dsi, const u8 >> +*data) { >> + struct i2c_adapter *adapter; >> + int ret; >> + u8 reg_offset, payload_size, retries = 5; >> + struct i2c_msg msg; >> + u8 *transmit_buffer = NULL; >> + u8 flag = *data++; >> + u8 index = *data++; >> + u8 bus_number = *data++; >> + u16 slave_add = *(u16 *)(data); >> + >> + data = data + 2; >> + reg_offset = *data++; >> + payload_size = *data++; >> + >> + adapter = i2c_get_adapter(bus_number); >> + >> + if (!adapter) { >> + DRM_ERROR("i2c_get_adapter(%u) failed, index:%u flag: >%u\n", >> + (bus_number + 1), index, flag); > >Why do you log bus_number + 1 instead of what was actually tried? [Deepak M] Will fix this. > >I don't see why the flag/index are useful for in the message, as they're not >relevant to the i2c_get_adapter failing. > [Deepak M] Flag field is reserved and then the index is used for windows, for Linux these two can be skipped. Will not try to declare flag and index variable. >> + goto out; >> + } >> + >> + transmit_buffer = kmalloc(1 + payload_size, GFP_TEMPORARY); >> + >> + if (!transmit_buffer) >> + goto out; >> + >> + transmit_buffer[0] = reg_offset; >> + memcpy(&transmit_buffer[1], data, (size_t)payload_size); > >Why do you need the cast? > >> + >> + msg.addr = slave_add; >> + msg.flags = 0; >> + msg.len = payload_size + 1; >> + msg.buf = &transmit_buffer[0]; >> + >> + do { >> + ret = i2c_transfer(adapter, &msg, 1); >> + if (ret == 1) >> + break; >> + else if (ret == -EAGAIN) >> + usleep_range(1000, 2500); >> + else { >> + DRM_ERROR("i2c transfer failed, error code:%d", ret); > >\n missing. > >> + break; >> + } >> + } while (retries--); >> + >> + if (retries == 0) > >Due to the way you post-decrement retries, you may end up here even if the >transfer succeeds on the last try. > >You may also end up printing two error messages, if i2c_transfer fails on the >last try. [Deepak M] okay, Will handle this. > >> + DRM_ERROR("i2c transfer failed, error code:%d", ret); > >\n missing. > >> +out: >> + kfree(transmit_buffer); >> + i2c_put_adapter(adapter); >> + >> + data = data + payload_size; >> + return data; >> +} >> + >> static inline enum port intel_dsi_seq_port_to_port(u8 port) { >> return port ? PORT_C : PORT_A; >> @@ -236,6 +296,7 @@ static const fn_mipi_elem_exec exec_elem[] = { >> mipi_exec_send_packet, >> mipi_exec_delay, >> mipi_exec_gpio, >> + mipi_exec_i2c, >> NULL, /* status read; later */ >> }; >> >> -- >> 1.7.9.5 >> >> _______________________________________________ >> 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
