On 4 August 2011 04:37, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@xxxxxxxxxxxx> wrote: > On 22:37 Wed 03 Aug , Antony Pavlov wrote: >> ns16550_read() and ns16550_write() functions are private functions >> of the driver so there is no need to pass them 'struct console_device *' >> pointer to get private device data. >> >> Signed-off-by: Antony Pavlov <antonynpavlov@xxxxxxxxx> >> --- > Does not simplify the code > > does not see the need Just the opposite! Without this patch we do the operation 'dev = cdev->dev' at _EVERY SINGLE CALL_ of ns16550_read/ns16550_write. In the driver, only in ns16550_tstc() we have single call of ns16550_read(). In all other places we have multiple ns16550_read/ns16550_write. So we can move 'dev = cdev->dev' to caller without any damage. Moreover, in ns16550_putc() we have cycle: ---- while ((ns16550_read(dev, lsr) & LSR_THRE) == 0) ; ---- On every cycle loop we do 'dev = cdev->dev' ! Disassemble the compiled barebox and you will see that you have many memory memory accesses at the entrance to the ns16550_read/ns16550_write functions. So, removing this 'dev = cdev->dev' we remove one memory access. This is a real optimization! Also, confirmation of this patch comes from simple logic. Simple question. Have we need of 'console_device *cdev' pointer in ns16550_read/ns16550_write at all? My answer is 'No'. Console_device is very high-level abstraction structure, but ns16550_read/ns16550_write functions work at the low-level. They work with the private driver data, but cdev can't directly supply this private data. -- Best regards, Antony Pavlov _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox
