From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753777AbbKXJeE (ORCPT ); Tue, 24 Nov 2015 04:34:04 -0500 Received: from mail-wm0-f53.google.com ([74.125.82.53]:37246 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753756AbbKXJd7 (ORCPT ); Tue, 24 Nov 2015 04:33:59 -0500 Date: Tue, 24 Nov 2015 10:33:56 +0100 From: Thierry Reding To: bjorn@kryo.se Cc: David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH 1/3] drm/dsi: Add support for Turn on/Shutdown peripheral packets Message-ID: <20151124093356.GA6149@ulmo> References: <1446251908-2603-1-git-send-email-bjorn@kryo.se> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fdj2RfSjLxBAspz7" Content-Disposition: inline In-Reply-To: <1446251908-2603-1-git-send-email-bjorn@kryo.se> User-Agent: Mutt/1.5.23+102 (2ca89bed6448) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --fdj2RfSjLxBAspz7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Oct 30, 2015 at 05:38:26PM -0700, bjorn@kryo.se wrote: > From: Werner Johansson >=20 > The MIPI_DSI_TURN_ON_PERIPHERAL and MIPI_DSI_SHUTDOWN_PERIPHERAL > packets are required for some panels, one example being the > Panasonic VVX10F034N00 panel. >=20 > Signed-off-by: Werner Johansson > Signed-off-by: Bjorn Andersson > --- > 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... 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). One minor comment below... > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_ds= i.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_de= vice *dsi, u8 format) > } > EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format); > =20 > +/** > + * 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] =3D { 0, 0 }; > + struct mipi_dsi_msg msg =3D { > + .channel =3D dsi->channel, > + .tx_buf =3D dummy, > + .tx_len =3D sizeof(dummy), > + .type =3D type > + }; > + > + if (mipi_dsi_packet_format_is_short(type)) > + return mipi_dsi_device_transfer(dsi, &msg); > + else > + return -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. Thanks, Thierry --fdj2RfSjLxBAspz7 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWVC8BAAoJEN0jrNd/PrOh3dUP/3aQ0iQge6AqHOARmThEJKmw kIYcuxwH95TSV3ib2xLl0MXiG2SCTJmm+UaUpk1pT+TucUYcx6ELpuSfVJuqagw6 XE7g9Qkt1jN7RxCj8jJ72E6KeBOFzuxsSSMxGtoPF93na3rd+vzgpH0I30EUUx43 8a9OZx+8EGhwSxyarSDDbwhyQMLp2lOKTLFrOCZIgWm6/FVzgO3kj33o/jR/jIKb WUH+bOW6aiH1jvWn9BZVMr1xErke8Gjtr2Daulmle3JU91jKRl79yBhgnyYU6Z4e +c0MhCyJ+Csfgr2LCrqS6/m5kpDQW8gkSy8fbGl5Fj/4U+0KlVbYWnjNjurmuevk y1bc1nL/yMc2uXA1ykYrms3uEmsVO83jW08bow2EyElSQndXhHqUrE+uQCSgnDop 1MYkDbMfuShukBgLonscriuALRc3jXGix6YhMCvaasBPz3fDuOLZCTOpySp+gnpU VQ7ccKMMZg2ByBfG0EmIE+HflLbGglEnIkkkeDX9w7XLjmy8qtvAFZgKP0aEZv20 2vPcciYh5Ot1V6aytaHO2ylNrgnIX9vcF5XBBReqBums9MJ1tD/Xk4UCAL+C5XyN 0t+wlXtC1DC7J5yMtiTz341iF/ANN8Gf1FsXxqi4vUhQ1R6wRaDG/xxFZH9V6zGN Q6CrGLQg+x4BnKXYL6Qe =Ikp8 -----END PGP SIGNATURE----- --fdj2RfSjLxBAspz7--