On Thu, Feb 22, 2018 at 04:07:39PM +0000, Alexey Brodkin wrote:
Hi Tom,
On Thu, 2018-02-22 at 10:43 -0500, Tom Rini wrote:
On Wed, Feb 21, 2018 at 03:26:05PM +0300, Alexey Brodkin wrote:
[snip]
static inline void serial_out_shift(void *addr, int shift, int value) { -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED +#ifdef CONFIG_ARCH_DFSS
- write_aux_reg((int)addr, value);
+#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED) outb(value, (ulong)addr); #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN) out_le32(addr, value); @@ -70,7 +75,9 @@ static inline void serial_out_shift(void *addr, int shift, int value)
static inline int serial_in_shift(void *addr, int shift) { -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED +#ifdef CONFIG_ARCH_DFSS
- return read_aux_reg((int)addr);
+#elif defined(CONFIG_SYS_NS16550_PORT_MAPPED) return inb((ulong)addr); #elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN) return in_le32(addr);
As always, thanks for the detailed explanation. Yes, I think that of the options, putting the details in read/write_aux_reg (and please make sure read/write_aux_reg have a good function comment too) is the best choice. Thanks!
Frankly I didn't understand your comment well enough :) Do you suggest to keep proposed implementation (i.e. modification of serial_{in|out}_shift()) but add comments on what do we do and why or you really meant something completely different?
I'm fine with what you have above, in the driver. I missed that read_aux_reg already exists, sorry. So I guess a detailed commit message will have to be enough documentation for future reviewers of the code. Or, we'll see what comes out of Simon's comments.