linux-kernel - Re: [PATCH v2 2/2] drm/bridge/synopsys: dsi: handle endianness correctly in dw_mipi_dsi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e4d9d6d6-76c4-4914-d800-c007a6e8938b@codeaurora.org>
Date:   Tue, 16 Jan 2018 12:22:52 +0530
From:   Archit Taneja <architt@...eaurora.org>
To:     Andrzej Hajda <a.hajda@...sung.com>,
        Brian Norris <briannorris@...omium.org>
Cc:     Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        David Airlie <airlied@...ux.ie>,
        Yannick Fertre <yannick.fertre@...com>,
        Philippe Cornu <philippe.cornu@...com>,
        Vincent Abriou <vincent.abriou@...com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Sean Paul <seanpaul@...omium.org>,
        Nickey Yang <nickey.yang@...k-chips.com>, hl@...k-chips.com,
        linux-rockchip@...ts.infradead.org, mka@...omium.org,
        hoegsberg@...il.com, zyw@...k-chips.com, xbl@...k-chips.com
Subject: Re: [PATCH v2 2/2] drm/bridge/synopsys: dsi: handle endianness
 correctly in dw_mipi_dsi_write()



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 <briannorris@...omium.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@...sung.com>

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