Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750981AbeAPGxC (ORCPT + 1 other); Tue, 16 Jan 2018 01:53:02 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:56546 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750725AbeAPGxB (ORCPT ); Tue, 16 Jan 2018 01:53:01 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 199676025D Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=architt@codeaurora.org Subject: Re: [PATCH v2 2/2] drm/bridge/synopsys: dsi: handle endianness correctly in dw_mipi_dsi_write() To: Andrzej Hajda , Brian Norris Cc: Laurent Pinchart , David Airlie , Yannick Fertre , Philippe Cornu , Vincent Abriou , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Sean Paul , Nickey Yang , hl@rock-chips.com, linux-rockchip@lists.infradead.org, mka@chromium.org, hoegsberg@gmail.com, zyw@rock-chips.com, xbl@rock-chips.com References: <20180109203248.139249-1-briannorris@chromium.org> <20180109203248.139249-2-briannorris@chromium.org> <199f9097-736e-afe8-2093-bb28fba8b308@samsung.com> From: Archit Taneja Message-ID: Date: Tue, 16 Jan 2018 12:22:52 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <199f9097-736e-afe8-2093-bb28fba8b308@samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On 01/10/2018 08:03 PM, Andrzej Hajda wrote: > On 09.01.2018 21:32, Brian Norris wrote: >> We're filling the "remainder" word with little-endian data, then writing >> it out to IO registers with endian-correcting writel(). That probably >> won't work on big-endian systems. >> >> Let's mark the "remainder" variable as LE32 (since we fill it with >> memcpy()) and do the swapping explicitly. >> >> Some of this function could be done more easily without memcpy(), but >> the unaligned "remainder" case is a little hard to do without >> potentially overrunning 'tx_buf', so I just applied the same solution in >> all cases (memcpy() + le32_to_cpu()). >> >> Tested only on a little-endian system. >> >> Signed-off-by: Brian Norris >> --- >> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> index ed91e32ee43a..90f13df6f106 100644 >> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c >> @@ -360,18 +360,18 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi, >> { >> const u8 *tx_buf = packet->payload; >> int len = packet->payload_length, pld_data_bytes = sizeof(u32), ret; >> - u32 remainder; >> + __le32 word; >> u32 val; >> >> while (len) { >> if (len < pld_data_bytes) { >> - remainder = 0; >> - memcpy(&remainder, tx_buf, len); >> - dsi_write(dsi, DSI_GEN_PLD_DATA, remainder); >> + word = 0; >> + memcpy(&word, tx_buf, len); >> + dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word)); >> len = 0; >> } else { >> - memcpy(&remainder, tx_buf, pld_data_bytes); >> - dsi_write(dsi, DSI_GEN_PLD_DATA, remainder); >> + memcpy(&word, tx_buf, pld_data_bytes); >> + dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word)); >> tx_buf += pld_data_bytes; >> len -= pld_data_bytes; >> } >> @@ -386,9 +386,9 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi, >> } >> } >> >> - remainder = 0; >> - memcpy(&remainder, packet->header, sizeof(packet->header)); >> - return dw_mipi_dsi_gen_pkt_hdr_write(dsi, remainder); >> + word = 0; >> + memcpy(&word, packet->header, sizeof(packet->header)); >> + return dw_mipi_dsi_gen_pkt_hdr_write(dsi, le32_to_cpu(word)); > > You could create and use appropriate helper, lets say: > > u32 le_to_cpup(const u8 *p, int count) > { >     __le32 r = 0; > >     memcpy(&r, p, count); >     return le32_to_cpu(r); > } > > With or without this change: > Reviewed-by: Andrzej Hajda Queued to drm-misc-next as is. Thanks, Archit > >  -- > Regards > Andrzej > > >> } >> >> static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host, > > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project