Hi Paul, Am 18.05.2016 um 16:52 schrieb Simon Glass: >>>>>>> >>>>>>> Are you sure systems rely on using I/O ports with map_physmem? The only >>>>>>> other systems that define CONFIG_SYS_NS16550_PORT_MAPPED are x86 ones, >>>>>>> in include/configs/x86-common.h, and so far as I can tell they don't use >>>>>>> device model which suggests this code has simply been untested before. I >>>>>>> don't see why you would use map_physmem on an I/O port address that is >>>>>>> then going to be passed to inb/outb & I think the code here is simply >>>>>>> wrong to do so. >>>>>> >>>>>> the current code looks wrong. serial_in_shift() is expanded to inb() >>>>>> in case of CONFIG_SYS_NS16550_PORT_MAPPED and to >>>>>> in_le32()/in_be32()/readl()/readb() otherwise. Only in the latter case >>>>>> a map_physmem() is required and should be done in serial_in_shift() >>>>>> itself or preferrably only once in >>>>>> ns16550_serial_ofdata_to_platdata(). >>>>>> >>>>>> I think the correct approach would be the following: >>>>> >>>>> This is better I think. But how about adding a device tree binding to >>>>> select I/O access? In principle each device might have its own >>>>> settings. >>>> >>>> Note that's what I worked towards last time I had a crack at this, but >>>> it just expanded into an attempt to tackle the mess that is ns16550.c & >>>> rather lost sight of the original goal of making Malta work. >>>> >>>> https://patchwork.ozlabs.org/patch/575643/ >>>> https://patchwork.ozlabs.org/patch/577194/ >>> >>> Yes it is tricky. What do you think about the suggestions above? >> >> for now we should only fix the broken port-based I/O. Additional DT >> bindings could be added later. I'd like to merge the DM support on MIPS >> Malta in this merge window ;) > > In that case your patch of moving it to ofdata_to_platdata() looks OK to me.
are you going to send a v4 of this patch? I can also do this if you like. Then I could apply all other v3 patches of this series. -- - Daniel
signature.asc
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
