From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752075AbeAYMQT (ORCPT ); Thu, 25 Jan 2018 07:16:19 -0500 Received: from mailout2.w1.samsung.com ([210.118.77.12]:53604 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751964AbeAYMQQ (ORCPT ); Thu, 25 Jan 2018 07:16:16 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20180125121614euoutp024f34b96648ffb0bb90b4d64bc2dd2409~NDKoeFRYW2079420794euoutp02- X-AuditID: cbfec7f1-f793a6d00000326b-af-5a69ca8cbae9 Subject: Re: [PATCH v1 1/2] drm/bridge/synopsys: dsi: Fix dsi_host_transfer() return value To: Brian Norris , Philippe CORNU Cc: Bhumika Goyal , Alexandre TORGUE , David Airlie , Linux Kernel , "dri-devel@lists.freedesktop.org" , Yannick FERTRE , "open list:ARM/Rockchip SoC..." , Laurent Pinchart , Maxime Coquelin , Ludovic BARRE , Mickael REULIER , Vincent ABRIOU , "linux-arm-kernel@lists.infradead.org" , Thierry Reding From: Andrzej Hajda Message-id: <61a3e22a-811b-53e5-f735-534dfd9f67da@samsung.com> Date: Thu, 25 Jan 2018 13:16:10 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-version: 1.0 In-reply-to: <20180124183748.l5iqoiyqxy4o2pyk@ban.mtv.corp.google.com> Content-type: text/plain; charset="utf-8" Content-transfer-encoding: 7bit Content-language: en-US X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrKKsWRmVeSWpSXmKPExsWy7djP87q9pzKjDJYvYbToPXeSyWLjk9OM Fp/u9bNZbPr4ntXiytf3bBadE5ewW2x6fI3V4vKuOWwWnx78Z7Y4csDRYt7ftawWpw4eYrJ4 0PKC1eLnrnksFmeuHmCxaO9sZXMQ8JjdcJHFY+esu+wesztmsnps//aA1eN+93Emj81L6j2e /tjL7PF5k1wARxSXTUpqTmZZapG+XQJXxtbjuQXX5SquHldpYDws0cXIySEhYCIx48MyJghb TOLCvfVsILaQwFJGidcPayHsz4wSL3/EdDFygNXfOqnUxcgFFF7GKHH3/GNGCOcZo0TT9Y2M IEXCAjESH57EgPSKCIRKfFh1nh2khllgD6vE5A+3mUESbAKaEn833wRbxitgJ9GxcCPYESwC qhJLls9lBbFFBSIkup7tYoWoEZT4MfkeC4jNKeAm8e7ZfLB6ZqA5L75MYoGw5SU2r3nLDGGL SzS33mQBWSwhsI5dYv3XL2wQX7pIzN/9jxHCFpZ4dXwLO4QtI9HZcZAJoqGbUeJT/wl2CGcK o8S/DzOYIaqsJQ4fv8gKsYJPYtK26cyQcOGV6GgTgijxkHjW9ghqgaPE0/cPoUH0iUliy91t zBMY5Wch+WgWki9mIfliFpIvFjCyrGIUSS0tzk1PLTbSK07MLS7NS9dLzs/dxAhMbKf/Hf+4 g/H9CatDjAIcjEo8vBz9GVFCrIllxZW5hxglOJiVRHgvtwOFeFMSK6tSi/Lji0pzUosPMUpz sCiJ89pGtUUKCaQnlqRmp6YWpBbBZJk4OKUaGNN+/9VsYxTTiBZLiBLwnn/n0d9Lym0/uvOK r53qTXj2T6HpxiSRyfNlf6oanlzX2Fz8y3nCVvcti0+oiihZm1qVuQrd+vU5TC1gX+y5ri0T /mQ+ZD1y4YDzhf1nj0SbSS34t9X7j2VXhaG7Vq2UtX3YNK2e7d2rVZ+8/Vce4s87qTgrv2/n BiWW4oxEQy3mouJEAFc7hRxoAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrFIsWRmVeSWpSXmKPExsVy+t/xa7o9pzKjDO6tFLXoPXeSyWLjk9OM Fp/u9bNZbPr4ntXiytf3bBadE5ewW2x6fI3V4vKuOWwWnx78Z7Y4csDRYt7ftawWpw4eYrJ4 0PKC1eLnrnksFmeuHmCxaO9sZXMQ8JjdcJHFY+esu+wesztmsnps//aA1eN+93Emj81L6j2e /tjL7PF5k1wARxSXTUpqTmZZapG+XQJXxtbjuQXX5SquHldpYDws0cXIwSEhYCJx66RSFyMn kCkmceHeerYuRi4OIYEljBK9s9qgnGeMEh/PNLKDVAkLxEg8P36MDcQWEQiV6GnrZwQpYhY4 wCpxdXI/K0THJyaJyd8uglWxCWhK/N18E8zmFbCT6Fi4kQnEZhFQlViyfC4riC0qECHRNBPC 5hUQlPgx+R4LiM0p4Cbx7tl8JpBTmQXUJaZMyQUJMwvIS2xe85YZwhaXaG69yTKBUXAWku5Z CB2zkHTMQtKxgJFlFaNIamlxbnpusaFecWJucWleul5yfu4mRmAMbjv2c/MOxksbgw8xCnAw KvHwcvRnRAmxJpYVV+YeYpTgYFYS4b3cDhTiTUmsrEotyo8vKs1JLT7EKM3BoiTO27tndaSQ QHpiSWp2ampBahFMlomDU6qBUWbmhnqeyEj1SQbnEk52TFyQaLx6K7f57VNHGJc47RXVvyg2 59GNXS+36nW81H9mpNVbtWW59bt891htiVl/v26//3Pli0M7n15JlLzRMnlFso35R80/b+zY NBfvip/XtfBq/gauiTXZ/lYX7PTOHpZQ2zK9dcazvQbRXYFznp6/3rRYe+HphAYlluKMREMt 5qLiRABOUlolvQIAAA== X-CMS-MailID: 20180125121612eucas1p1a59214264289832f1ab955238a2a7cf2 X-Msg-Generator: CA CMS-TYPE: 201P X-CMS-RootMailID: 20180124183757epcas5p2e8296b5712fc0f2004eb81111693e166 X-RootMTR: 20180124183757epcas5p2e8296b5712fc0f2004eb81111693e166 References: <20180123142618.28384-1-philippe.cornu@st.com> <20180123142618.28384-2-philippe.cornu@st.com> <4a1b0208-3187-2f08-69fe-ca3b77ee88a8@st.com> <20180124183748.l5iqoiyqxy4o2pyk@ban.mtv.corp.google.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 24.01.2018 19:37, Brian Norris wrote: > Hi Philippe, > > On Wed, Jan 24, 2018 at 01:33:54PM +0000, Philippe CORNU wrote: >> On 01/23/2018 10:38 PM, Brian Norris wrote: >>> Hi Philippe, >>> >>> On Tue, Jan 23, 2018 at 6:26 AM, Philippe Cornu wrote: >>>> The dw_mipi_dsi_host_transfer() must return the number of >>>> bytes transmitted/received on success instead of 0. >>> I'm a little confused. As of the latest drm-misc-next I'm looking at, >>> this still has conflicting documentation. >>> >>> For ->transfer(): >>> >>> On success it shall return the number of bytes >>> * transmitted for write packets or the number of bytes received for read >>> * packets. >>> >>> While mipi_dsi_generic_read() says: >>> >>> * Return: The number of bytes successfully read or a negative error code on >>> * failure. >>> >>> But it just returns the value that ->transfer() returns. >>> >> Not sure to follow you here: mipi_dsi_generic_read() will trig a dsi >> generic read so it has to return "the number of bytes received for read >> packets" as explained for the ->transfer() function... so it looks >> "coherent"... >> >> But maybe you want to point out something different? > Actually, reading back what I wrote, I'm not sure it made sense. I think > *I* was confusing "supporting TX only" with "supporting TX and RX". I > believe the documentation isn't conflicting, but your current patch is a > little misleading. > > With your current patch, you're returning the 'mipi_dsi_packet::size', > which is the sum of both TX and RX. I did not found docs saying mipi_dsi_packet::size is a sum of tx and rx. tx and rx packets are two different packets, so they do not sum up. But thanks for bringing it up, it shows docs are incomplete/misleading. > Since we only support TX right now, > I suppose that actually is fine (because 'rx_len == 0'). But if we start > supporting RX too, then this field is not the right one to return. > > Anyway, maybe this patch was fine as it was. But when you get RX > support, this will have to be something like: > > if (msg->rx_len) > return msg->rx_len; > else > return packet.size; > > BTW, does anyone actually care about seeing the number of TX bytes > returned? That seems weird, because I wouldn't expect you'd get a good > result from a partial TX (dunno about partial RX). And I also see that > there are other drivers that get this all wrong too. See > mtk_dsi_host_transfer(), which only returns 0 for TX and 'recv_cnt' for > RX. As far as I remember MIPI DSI standard does not allow partial TX, it is all-or-nothing operation. > > So all-in-all, maybe my problem isn't that the documentation is > conflicting, exactly, but that the requirements are somewhat odd, such > that either implementations get it wrong (2 of 3 that I've looked at!), > or they have to write somewhat odd special-casing. mipi_dsi_host_ops::transfer in case of write sends only tx packet, in case of read it sends tx packets and receives rx packet, so it can be confusing what it should return in case of read. IMO changing mipi_dsi_host_ops::transfer to always return number of bytes RECEIVED or error should make it clearer and simpler. +CC Thierry Regards Andrzej > >>> So I'm not sure whether the documentation is still wrong, or if the >>> implementation is. >>> >>> Anyway, I guess maybe that isn't super-critical to *this* patch, since >>> we don't have RX support yet... >>> >> The main reason why I want to "fix" this is because I do not want to >> explain to our customers (writing dsi panel drivers) why we have a >> different returned value compare to other platforms : ) > Brian > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel