[U-Boot] [PATCH v2 3/5] dm: ns16550: Don't map_physmem for I/O ports

[Daniel Schwierzeck <daniel.schwierzeck@gmail.com>]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160521/c85be92f/attachment.sig>