Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1642998

Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support

From Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Newsgroups linux.kernel
Subject Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support
Date 2017-05-17 08:00 +0200
Message-ID <tI0a6-7yt-3@gated-at.bofh.it> (permalink)
References <tHoo9-lZ-13@gated-at.bofh.it> <tHIGf-4Ub-17@gated-at.bofh.it> <tHXYC-6cB-5@gated-at.bofh.it> <tHZQJ-7s4-3@gated-at.bofh.it> <tI00p-7v7-23@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


>>>>>  static u32 lpuart32_read(void __iomem *addr)
>>>>>  {
>>>>> -	return ioread32be(addr);
>>>>> +	return lpuart_is_be ? ioread32be(addr) : readl(addr);
>>>>>  }
>>>>>  
>>>>>  static void lpuart32_write(u32 val, void __iomem *addr)
>>>>>  {
>>>>> -	iowrite32be(val, addr);
>>>>> +	if (lpuart_is_be)
>>>>> +		iowrite32be(val, addr);
>>>>> +	else
>>>>> +		writel(val, addr);
>>>>>  }
>>>>
>>>> What if this is ever executed on big endian system?
>>>>
>>>
>>> Sorry, not catching the point...
>>>
>>> What issues will meet?
>>
>> Isn't writel() in host endian?
> 
> On big endian systems, it is supposed to run iowrite32be.

Your code states, "force BE if lpuart_is_be, don't care otherwise".
This semantics looks questionable for code reviewer.
If driver handles endian, should't it be explicit in both cases?
And if indeed driver means handling BE explicitly, but don't caring
otherwise, maybe variable name should suggest that (i.e. "force_be")?

Although driver maintainer could think differently. I won't insist on this.

Nikita

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register  support Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2017-05-16 13:20 +0200
  Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register  support Dong Aisheng <dongas86@gmail.com> - 2017-05-17 05:40 +0200
    Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register  support Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2017-05-17 07:40 +0200
      Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register  support Dong Aisheng <dongas86@gmail.com> - 2017-05-17 07:50 +0200
        Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register  support Nikita Yushchenko <nikita.yoush@cogentembedded.com> - 2017-05-17 08:00 +0200
          Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register  support Dong Aisheng <dongas86@gmail.com> - 2017-05-17 08:20 +0200
        Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-17 12:00 +0200
  Re: [V2, 2/6] tty: serial: lpuart: add little endian 32 bit register support Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-17 12:00 +0200

csiph-web