public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] include/ns16550.h: Unify structure declaration for	registers
Date: Mon, 04 May 2009 17:40:37 +0200	[thread overview]
Message-ID: <m2zldsanbe.fsf@ohwell.denx.de> (raw)
In-Reply-To: <49FA5790.3070200@pobox.com> (Shinya Kuribayashi's message of "Fri, 01 May 2009 10:59:44 +0900")

Hi Shinya,

>>>> I see.  Actually I was looking a lot at the Linux driver but was hoping
>>>> that we could away without introducing serial_{in,out}...
>>> In my horrible opinion, the combinations of base addres + reg_shift
>>> + iotype (char, long, or whatever), are simpler, more configurable,
>>> more slid, easy to use, than what we used to have or what you
>>> consolidated this time.
>>
>> You lost me here.
>>
>> You truly consider
>>
>> static unsigned int serial_in(struct uart_8250_port *up, int offset)
> [snip]
>> }
>>
>> to be "simpler and more solid" readb(struct->field) (which is
>> effectively what we have in the current implementation)?  You consider
>> "more configurable" to be a good in its own?
>
> Yes.

Wow.  As a rhetorical question - where do you actually draw the line if
you consider configurable to be a good in its own?  Shouldn't we then
have configuration options for UARTs who are attached bit-reversed on
the databus also?  And an option for a bit-shift in the data itself?

>> If your answers to these questions are yes, then we have different ideas
>> of writing code.
>
> Please make sure we don't need full serial_{in,out} port from Linux
> as-is.  As suggested, the combinations of base addres + reg_shift +
> iotype, are rather reasonable to support various kind of hardwares.
>
> I mean we need something like this:
>
> static unsigned int serial_in(struct uart_8250_port *up, int offset)
> {
>        unsigned int tmp;
>        int ret;
>        offset = map_8250_in_reg(up, offset) << up->port.regshift;
>
>        switch (up->port.iotype) {
>        case UPIO_MEM:
>                ret = readb(up->port.membase + offset);
>                break;
>
>        case UPIO_MEM32:
>                ret = readl(up->port.membase + offset);
>                break;
>
>        default:
>                ret = inb(up->port.iobase + offset);
>                break;
>        }
>        return ret;
> }
>
> Its implementation must be differed in U-Boot code, of course.

[...]
> I admit the address decoder in my UART hardware is weird and needs
> special configuration,  but this is not just for my case, it's not
> unusual.
>
> There're various kind of hardwares in the world, and there're many
> U-Boot ports which can not be pushed to upstream for various reasons.
> We can easily ignore such boards of course, but it would be very nice
> for U-Boot if it could provide easy configurable drivers and could
> support as many hardwares as possible.

Currently it seems that all in-tree boards can be accomodated with the
construct that I suggested.  I am not at all sure that we want code
which is only used by out-of-tree ports.

Post the port and we can rediscuss new code.

Cheers
  Detlev

-- 
[Linux] USB consoles was a  bad hack written on a drunken dare.   I'm still
constantly amazed that the thing even works at all, let alone the fact that
people are actually using it :) 
                            -- Greg KH <20090420225358.GC28697@kroah.com>
--
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: dzu at denx.de

  reply	other threads:[~2009-05-04 15:40 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-03 14:45 [U-Boot] [PATCH] include/ns16550.h: Unify structure declaration for registers Detlev Zundel
2009-04-03 14:55 ` Detlev Zundel
2009-04-03 23:24 ` Wolfgang Denk
2009-04-25  1:21 ` Shinya Kuribayashi
2009-04-27 13:41   ` Detlev Zundel
2009-04-27 14:26     ` Shinya Kuribayashi
2009-04-27 15:36       ` Detlev Zundel
2009-04-27 16:09         ` Detlev Zundel
2009-04-29 18:51         ` Shinya Kuribayashi
2009-04-29 19:12           ` Shinya Kuribayashi
2009-04-30 13:30             ` Detlev Zundel
2009-04-30 14:10               ` Detlev Zundel
2009-05-01  0:56               ` Shinya Kuribayashi
2009-05-01  5:29                 ` Shinya Kuribayashi
2009-04-30 12:26           ` Detlev Zundel
2009-04-30 12:52             ` Jerry Van Baren
2009-04-30 14:08               ` Detlev Zundel
2009-04-30 14:38                 ` Detlev Zundel
2009-04-30 17:06                 ` Jerry Van Baren
2009-05-01  2:21               ` Shinya Kuribayashi
2009-05-01  1:59             ` Shinya Kuribayashi
2009-05-04 15:40               ` Detlev Zundel [this message]
2009-05-04 21:21                 ` Scott Wood
2009-05-04 21:57                 ` Wolfgang Denk
2009-05-05  1:36                   ` Shinya Kuribayashi
2009-05-05  9:09                     ` Detlev Zundel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m2zldsanbe.fsf@ohwell.denx.de \
    --to=dzu@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox