On Tue, Nov 04, 2014 at 03:21:14PM +0100, Andrzej Hajda wrote:
On 11/04/2014 02:58 PM, Thierry Reding wrote:
On Tue, Nov 04, 2014 at 12:43:21PM +0100, Andrzej Hajda wrote:
On 11/03/2014 10:13 AM, Thierry Reding wrote:
[...]
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
[...]
packet->header[1] = (msg->tx_len >> 0) & 0xff;packet->header[2] = (msg->tx_len >> 8) & 0xff;packet->payload_length = msg->tx_len;packet->payload = tx;- } else {
packet->header[1] = (msg->tx_len > 0) ? tx[0] : 0;packet->header[2] = (msg->tx_len > 1) ? tx[1] : 0;- }
- packet->size = sizeof(packet->header) + packet->payload_length;
size seems to be completely useless.
It's not. Tegra has two FIFOs that can be selected depending on the size of a transfer. I use this field to detect which FIFO needs to be selected.
But size is always equal payload_length + 4, so it does not carry any additional information.
Right, but it's out of convenience to prevent every driver from doing this again. It's part of the help that the helper provides. =)
- return 0;
+} +EXPORT_SYMBOL(mipi_dsi_create_packet);
+/**
- mipi_dsi_dcs_write - send DCS write command
- @dsi: DSI device
- @data: pointer to the command followed by parameters
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h index 8569dc5a1026..663aa68826f4 100644 --- a/include/drm/drm_mipi_dsi.h +++ b/include/drm/drm_mipi_dsi.h @@ -44,6 +44,24 @@ struct mipi_dsi_msg { };
/**
- struct mipi_dsi_packet - represents a MIPI DSI packet in protocol format
- @size: size (in bytes) of the packet
- @header: the four bytes that make up the header (Data ID, Word Count or
Packet Data, and ECC)
- @payload_length: number of bytes in the payload
- @payload: a pointer to a buffer containing the payload, if any
- */
+struct mipi_dsi_packet {
- size_t size;
- u8 header[4];
I wonder if it wouldn't be good to make it u32 or at least anonymous union: union { u8 header[4]; u32 header32; };
I'm not sure this is very useful. It's pretty trivial how you concatenate the individual bytes and it actually remove any ambiguity about the endianness.
This looks better:
val = le16_to_cpu(pkt->header32); writel(val, REG);
than this:
val = pkt->header[2] << 16 | pkt->header[1] << 8 | pkt->header[0]; writel(val, REG);
I disagree. =) Having the individual bytes makes their ordering very explicit.
Thierry