Hi Masahiro, On 4 October 2014 06:57, Masahiro YAMADA <yamad...@jp.panasonic.com> wrote: > > Hi Simon, > > > > 2014-10-03 22:51 GMT+09:00 Simon Glass <s...@chromium.org>: > > Hi Masahiro, > > > > On 3 October 2014 07:04, Masahiro Yamada <yamad...@jp.panasonic.com> wrote: > >> Simon, > >> > >> > >> > >> I am totally being confused. > >> > >> > >> > >> As far as I looked at the dm code, > >> the private data is calloc'ed in device_probe() function > >> > >> if (drv->priv_auto_alloc_size) { > >> dev->priv = calloc(1, drv->priv_auto_alloc_size); > >> if (!dev->priv) { > >> ret = -ENOMEM; > >> goto fail; > >> } > >> } > >> > >> > >> > >> So, dev->priv is storing the address of the allocated memory. > >> > >> Am I understanding correctly? > > > > Yes, it always does in driver model. This driver is no different. > > > > BTW the ns16550 driver is probably the oddest in U-Boot - it looks > > like UniPhier has its own UART driver so it would be better to convert > > that I think. See below for ideas on other UART drivers to look at > > which are much more normal. > > > >> > >> > >> > >> If so, I can't understand the following code: > >> > >> > >> static int ns16550_serial_getc(struct udevice *dev) > >> { > >> struct NS16550 *const com_port = dev_get_priv(dev); > >> > >> if (!serial_in(&com_port->lsr) & UART_LSR_DR) > >> return -EAGAIN; > >> > >> > >> > >> "com_port" is dev->priv, so it is pointing to the allocated area on RAM, > >> I guess. > >> > >> > >> It looks like > >> serial_in(&com_port->lsr) is trying to read from the hardware register ? > >> > >> Or reading from malloc area RAM ?? > > > > If you go one level deeper you will see that serial_in() is defined at > > the top in about 5 ways, one of which is used for driver model: > > > > #define serial_in(addr) \ > > ns16550_readb(com_port, addr - (unsigned char *)com_port) > > > > So it used com_port which is not an parameter. The parameter addr is > > only used to specify the register. This is actually the same as the > > non-DM code except in that case the parameter specifies the register > > and hardware address at the same time. > > Without your help, I never could have understood this code. > Now it is clearer, thanks! (although this code is really unfortunate...)
It felt like laparoscopic surgery - there is a nicely-healed wound but inside is another story. > > > > > This is done since the internal NS16550_t type is unfortunately > > exported all over U-Boot. This is annoying because it is actually an > > internal register format for the UART. Even worse is the fact that the > > structure changes depending on CONFIG_SYS_NS16550_REG_SIZE. We can't > > have this sort of thing in driver model - we need to be able to cope > > with the device tree specifying all the information that the UART > > needs. So for driver model: > > Agreed. > We should pass "reg-shift" from a device tree or a board-file. > > If all the board switched to the driver model, > the driver code would become much clean. Yes, I'm looking forward to that day. > > > > > /* > > * For driver model we always use one byte per register, and sort out the > > * differences in the driver > > */ > > #define CONFIG_SYS_NS16550_REG_SIZE (-1) > > > > (see ns16550.h header for where this is used) > > > > I would like to avoid having that type exported and use a different > > method to pass the UART information around. But it is used in about 30 > > places . So this approach allows us to move forward bit by bit, > > without duplicating the UART driver and creating a maintenance > > headache. > > > > In terns of implementing a new UART driver, are you using device tree? > > UniPhier SoCs have not supported the device tree control yet. > > I am planning to do the dm conversion with a board file. > > > > If so then you should just be able to follow along with Tegra - it's > > really easy - just copy that serial_tegra.c and adjust the device tree > > compat string and input clock. Other examples are serial_omap and > > serial_dw. (See u-boot-dm/working) > > These are unfortunate. > They are borrowing ns16550 code. Tegra and many other SOCs have used this code for a while, just with some defines to make it work. I didn't want to copy it. > > > > If you are using platform data, then the examples are serial_mxc and > > serial_pl01x. > > I am doing this way. > > Anyway, the register map of UniPhier is not 8250-compatible. > It cannot borrow ns16550 code. That's probably for the best. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot