Hello Shinya,
> Detlev Zundel wrote:
>> To be honest, I did not expect such problems, as I saw no hints from
>> comments on why this code was needed. Thinking afresh, it now makes at
>> least some sense why the code was. It nevertheless was inconsistent, as
>> the word access was only done in an asymmetric way regarding the
>> REG_SIZES parameter.
>
> I used to define locally "long + -16" variants.
As I said, I understand now why there were different data-types involved
although this was kind of non-obvious. So I take it, you had a working
configuration with REG_SIZE = 4, correct?
Can you enlighten me, why exactly the 8-bit accesses do not work on your
hardware? Is this because of a "too simplistic" address decoding logic?
What endianness is your CPU using?
You see, I still do not understand the problem completely. There is a
board named ppmc7xx which uses REG_SIZE -8. The comment says "64-bit
accesses to 8-bit port" and the definition is just like in the new
version with uchars and padding. Why did that work? Very likely the
comment is wrong, but still
>>> How do I supposed to configure UART in my board config file?
>>
>> I'm not sure at all. I believe you tested with 4 and -4 and it doesn't
>> work, right?
>
> Right.
>
>> Now we have the problem that we have byte registers (after all, there
>> are only 8 bits significant even for your platform) which need to be
>> accessed as 32-bit entities (or 16 bit for other platforms maybe).
>
> This is why Linux 8250 driver supports not only UPIO_MEM but also
> UPIO_MEM32.
I see. Actually I was looking a lot at the Linux driver but was hoping
that we could away without introducing serial_{in,out}...
>> I don't see any way out here than to probably re-introduce different
>> data-types again - which I certainly do not like too much as the
>> registers stay 8 bit wide.
>
> If there's no good alternatives, I think reverting is a good idea
> because there must be other platforms affected by this change.
Reverting is not a long-term option if somebody wants to maintain the
source at all. Did you see all the different layout structures with
fields being defined only in one of the 5 alternatives (of the possible
8)? Bah...
>> Does anyone else have a good idea here?
>
> Hm, how about introducing serial_{in,out} concepts from Linux?
Maybe, but maybe we can also do some more cheat^B^B^B^B^B creative
coding. I was think about something along those lines:
diff --git a/include/ns16550.h b/include/ns16550.h
index ce606b5..7924396 100644
--- a/include/ns16550.h
+++ b/include/ns16550.h
@@ -21,16 +21,20 @@
* will not allocate storage for arrays of size 0
*/
+#if !defined(CONFIG_SYS_NS16550_REG_TYPE)
+#define UART_REG_TYPE unsigned char
+#endif
+
#if !defined(CONFIG_SYS_NS16550_REG_SIZE) || (CONFIG_SYS_NS16550_REG_SIZE == 0)
#error "Please define NS16550 registers size."
#elif (CONFIG_SYS_NS16550_REG_SIZE > 0)
-#define UART_REG(x) \
- unsigned char prepad_##x[CONFIG_SYS_NS16550_REG_SIZE - 1]; \
- unsigned char x;
+#define UART_REG(x) \
+ UART_REG_TYPE prepad_##x[CONFIG_SYS_NS16550_REG_SIZE -
sizeof(UART_REG_TYPE)]; \
+ UART_REG_TYPE x;
#elif (CONFIG_SYS_NS16550_REG_SIZE < 0)
#define UART_REG(x) \
- unsigned char x; \
- unsigned char postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE - 1];
+ UART_REG_TYPE x; \
+ UART_REG_TYPE postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE -
sizeof(UART_REG_TYPE)];
#endif
struct NS16550 {
Then you could do a
#define CONFIG_SYS_NS16550_REG_SIZE 4
#define CONFIG_SYS_NS16550_REG_TYPE unsigned long
This of course needs to be documented once it works ;)
What do you think?
Cheers
Detlev
--
config LGUEST
If unsure, say N. If curious, say M. If masochistic, say Y.
-- linux/drivers/lguest/Kconfig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: d...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
