MIME-version: 1.0
Content-type: text/plain; charset="utf-8"
Subject: Re: [PATCH v2 2/2] drm/bridge/synopsys: dsi: handle endianness
 correctly in dw_mipi_dsi_write()
To: Brian Norris <briannorris@chromium.org>,
        Archit Taneja <architt@codeaurora.org>,
        Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: David Airlie <airlied@linux.ie>,
        Yannick Fertre <yannick.fertre@st.com>,
        Philippe Cornu <philippe.cornu@st.com>,
        Vincent Abriou <vincent.abriou@st.com>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        Sean Paul <seanpaul@chromium.org>,
        Nickey Yang <nickey.yang@rock-chips.com>, hl@rock-chips.com,
        linux-rockchip@lists.infradead.org, mka@chromium.org,
        hoegsberg@gmail.com, zyw@rock-chips.com, xbl@rock-chips.com
From: Andrzej Hajda <a.hajda@samsung.com>
Message-id: <199f9097-736e-afe8-2093-bb28fba8b308@samsung.com>
Date: Wed, 10 Jan 2018 15:33:41 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
        Thunderbird/52.5.0
In-reply-to: <20180109203248.139249-2-briannorris@chromium.org>
Content-transfer-encoding: 8bit
Content-language: en-US
CMS-TYPE: 201P
References: <20180109203248.139249-1-briannorris@chromium.org>
        <CGME20180109203308epcas1p17226c8b3b3a0474adcd51b87b70084bf@epcas1p1.samsung.com>
        <20180109203248.139249-2-briannorris@chromium.org>
Sender: linux-kernel-owner@vger.kernel.org

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 <briannorris@chromium.org>
> ---
>  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 <a.hajda@samsung.com>

 --
Regards
Andrzej


>  }
>  
>  static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,