On Tue, Nov 24, 2015 at 1:33 AM, Thierry Reding thierry.reding@gmail.com wrote:
On Fri, Oct 30, 2015 at 05:38:26PM -0700, bjorn@kryo.se wrote:
From: Werner Johansson werner.johansson@sonymobile.com
The MIPI_DSI_TURN_ON_PERIPHERAL and MIPI_DSI_SHUTDOWN_PERIPHERAL packets are required for some panels, one example being the Panasonic VVX10F034N00 panel.
Signed-off-by: Werner Johansson werner.johansson@sonymobile.com Signed-off-by: Bjorn Andersson bjorn.andersson@sonymobile.com
drivers/gpu/drm/drm_mipi_dsi.c | 47 ++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_mipi_dsi.h | 3 +++ 2 files changed, 50 insertions(+)
This being a dependency on 3/3 it would've been good to send this To: me as well. I hadn't seen this in my inbox and had simply discarded it as being independent. Of course if I had properly checked the build I would've noticed...
Sorry for not including you as a recipient, I relied too heavily on get_maintainer. Thanks for fixing my misstake.
Anyway, I've applied this with slight modifications. I removed the raw short write helper. I don't think it buys us much and we already have an open-coded variant in mipi_dsi_set_maximum_return_packet_size(). I've also moved these functions closer to the Set Maximum Return Packet Size command implementation because they are part of the same command set. I also made the functions return int instead of ssize_t to avoid potential confusion about their return value (ssize_t implies "number of bytes transferred, or a negative error code on failure).
Sounds good, thanks.
One minor comment below...
diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c index 2d5ca8ee..13b4a9c 100644 --- a/drivers/gpu/drm/drm_mipi_dsi.c +++ b/drivers/gpu/drm/drm_mipi_dsi.c @@ -862,6 +862,53 @@ int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format) } EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
+/**
- mipi_dsi_raw_short_write() - Sends a data-less short DSI packet
- @dsi: DSI peripheral device
- @type: Data Type of packet to send
- Return: 0 on success or a negative error code on failure.
- */
+static ssize_t mipi_dsi_raw_short_write(struct mipi_dsi_device *dsi, u8 type) +{
u8 dummy[2] = { 0, 0 };struct mipi_dsi_msg msg = {.channel = dsi->channel,.tx_buf = dummy,.tx_len = sizeof(dummy),.type = type};if (mipi_dsi_packet_format_is_short(type))return mipi_dsi_device_transfer(dsi, &msg);elsereturn -1;+}
When the kerneldoc comment says "negative error code", developers will expect one of the -E* constants. -1 maps to -EPERM (operation not permitted), which isn't a good error code for this case. This is also an internal function, so these errors really should never happen.
Anyway, since I removed this helper this isn't an issue anymore, just wanted to mention it.
Right. Thanks for fixing it!
Regards, Bjorn