[U-Boot] [PATCH 03/11] DM: add block controller core

[Marek Vasut <marex@denx.de>]

Dear Pavel Herrmann,

> On Friday 21 of September 2012 20:01:27 Marek Vasut wrote:
> > Dear Pavel Herrmann,
> > 
> > > On Friday 21 of September 2012 18:08:13 Marek Vasut wrote:
> > > > Dear Pavel Herrmann,
> > > > 
> > > > > On Friday 21 of September 2012 17:39:21 Marek Vasut wrote:
> > > > > > Dear Pavel Herrmann,
> > > > > > 
> > > > > > [...]
> > > > > > 
> > > > > > > > Can't the old driver just have a compat section in them? Like
> > > > > > > > I did with serial stuff:
> > > > > > > > 
> > > > > > > > 1) rename the internal functions to
> > > > > > > > ${driver}_${function_name} from pure ${function_name} and
> > > > > > > > introduce section which behaves as a wrapper (implement
> > > > > > > > ${function_name} calling
> > > > > > > > ${driver}_${function_name} ). 2) Add your DM goo, implement
> > > > > > > > #ifdef around it so either the compat section or DM section
> > > > > > > > is enabled.
> > > > > > > 
> > > > > > > I actually did something of this sort, see [4/11], with less
> > > > > > > touching.
> > > > > > > 
> > > > > > > the problem is that while SATA drivers are easy to convert, IDE
> > > > > > > ones are not. I would actually propose to do a ide_legacy
> > > > > > > driver (mostly out of the code currently in common/cmd_ide.c),
> > > > > > > and keep it as the only option until IDE dies completely.
> > > > > > 
> > > > > > IDE will be around for a LONG time.
> > > > > > 
> > > > > > You introduce that CONFIG_SYS_SATA_LEGACY for no reason, if you
> > > > > > did it as
> > > > > > said above, simple CONFIG_DM would suffice as the drivers would
> > > > > > be intacts with DM disabled. Note the compiler will opt-out
> > > > > > these proxy calls.
> > > > > > 
> > > > > > Besides, with this approach of yours, you need to enable
> > > > > > SATA_LEGACY for every single board now, introducing a lot of
> > > > > > churn into the patches and if it's not defined, every board
> > > > > > using SATA is broken, right?
> > > > > 
> > > > > No, if you dont define CONFIG_DM, you get the old way of
> > > > > interacting with disks. only if you define CONFIG_DM you only need
> > > > > CONFIG_SATA_LEGACY to plug old SATA drivers into DM codepaths
> > > > 
> > > > So if you enable DM for a board, you also need to enable this compat
> > > > layer? My opinion is, that you either enable DM on the board and the
> > > > board is ready for it or don't enable it.
> > > 
> > > if you enable DM on a board, you can either use the compat layer and
> > > the old driver, or use a ported driver and get rid of the compat layer
> > > 
> > > > Would you need the compat layer at all were you to follow my advice?
> > > 
> > > if i understand what you meant, i would build this compat layer into
> > > every one of the drivers currently in tree (with some cleanup). in
> > > that case, i would not need it
> > 
> > That's correct. You would prepare every driver to be DM-ish and the final
> > deployment of DM would be mechanical. So would be the removal of the
> > non-DM part later on.
> 
> that would lead to massive code duplication.

How?

> im not talking about the SATA
> drivers now, there its just about ready for DM already (static for all
> functions, remove dependency on static block_dev_desc array, include driver
> registration functions, done), but about IDE drivers, where the drivers
> have ~200 lines, whereas cmd_ide.c has ~2000 lines.

IDE implements some nasty hooks into cmd_ide.c, right ? I think if you managed 
to clean up and analyze cmd_ide.c, it'd be possible to figure out a path to fix 
the IDE drivers.

> as for the SCSI
> drivers, ahci.c is not really SCSI, more like a SATA driver with SCSI
> wrapper built in, the other one is a bit more complex, and would require
> some wrapper code addition (cmd_scsi is ~600 lines)

Might need some thinking here. Can the SCSI handling be abstracted out?

[...]

Best regards,
Marek Vasut