Re: [PATCH v2 2/2] drm/bridge/synopsys: dsi: handle endianness correctly in dw_mipi_dsi_write()
From: Andrzej Hajda
Date: Wed Jan 10 2018 - 09:33:52 EST
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@xxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxx>
Â--
Regards
Andrzej
> }
>
> static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,