diff mbox

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

Message ID 1348169867-2917-4-git-send-email-morpheus.ibis@gmail.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Pavel Herrmann Sept. 20, 2012, 7:37 p.m. UTC
This core provides unified access to different block controllers (SATA, SCSI).

Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
---
 Makefile                   |   1 +
 drivers/blockctrl/Makefile |  42 ++++++
 drivers/blockctrl/core.c   | 349 +++++++++++++++++++++++++++++++++++++++++++++
 include/dm/blockctrl.h     |  75 ++++++++++
 4 files changed, 467 insertions(+)
 create mode 100644 drivers/blockctrl/Makefile
 create mode 100644 drivers/blockctrl/core.c
 create mode 100644 include/dm/blockctrl.h

Comments

Marek Vasut Sept. 20, 2012, 8:05 p.m. UTC | #1
Dear Pavel Herrmann,

> This core provides unified access to different block controllers (SATA,
> SCSI).

Description of the patch missing or is sub-par. You should work on this skill.

> Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> ---
>  Makefile                   |   1 +
>  drivers/blockctrl/Makefile |  42 ++++++
>  drivers/blockctrl/core.c   | 349
> +++++++++++++++++++++++++++++++++++++++++++++ include/dm/blockctrl.h     |
>  75 ++++++++++
>  4 files changed, 467 insertions(+)
>  create mode 100644 drivers/blockctrl/Makefile
>  create mode 100644 drivers/blockctrl/core.c
>  create mode 100644 include/dm/blockctrl.h
> 
> diff --git a/Makefile b/Makefile
> index e43fd9d..4420484 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
>  LIBS-$(CONFIG_DM) += common/dm/libdm.o
>  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
>  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o

${} ? What is this ?

[..]

This handles SCSI? Sata ? what ?

Should this not be called scsi_core ? sata_core ? What did the previous core do? 
sata?  scsi? block? I'm lost.

I stop here, I don't know what this is all about, sorry.

Best regards,
Marek Vasut
Pavel Herrmann Sept. 21, 2012, 7:21 a.m. UTC | #2
On Thursday 20 of September 2012 22:05:36 Marek Vasut wrote:
> Dear Pavel Herrmann,
> 
> > This core provides unified access to different block controllers (SATA,
> > SCSI).
> 
> Description of the patch missing or is sub-par. You should work on this
> skill.
> > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > ---
> > 
> >  Makefile                   |   1 +
> >  drivers/blockctrl/Makefile |  42 ++++++
> >  drivers/blockctrl/core.c   | 349
> > 
> > +++++++++++++++++++++++++++++++++++++++++++++ include/dm/blockctrl.h     |
> > 
> >  75 ++++++++++
> >  4 files changed, 467 insertions(+)
> >  create mode 100644 drivers/blockctrl/Makefile
> >  create mode 100644 drivers/blockctrl/core.c
> >  create mode 100644 include/dm/blockctrl.h
> > 
> > diff --git a/Makefile b/Makefile
> > index e43fd9d..4420484 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
> > 
> >  LIBS-$(CONFIG_DM) += common/dm/libdm.o
> >  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
> >  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> > 
> > +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
> 
> ${} ? What is this ?
> 
> [..]
> 
> This handles SCSI? Sata ? what ?
> 
> Should this not be called scsi_core ? sata_core ? What did the previous core
> do? sata?  scsi? block? I'm lost.

the previous core handled disks (and cards and stuff) and partitions (think 
/dev/sdxy), and was largely a replacement of /disk
this core handles any interface those disks are connected to (SATA, PATA, 
SCSI), and should replace /drivers/block

> I stop here, I don't know what this is all about, sorry.
> 
> Best regards,
> Marek Vasut
Marek Vasut Sept. 21, 2012, 12:51 p.m. UTC | #3
Dear Pavel Herrmann,

> On Thursday 20 of September 2012 22:05:36 Marek Vasut wrote:
> > Dear Pavel Herrmann,
> > 
> > > This core provides unified access to different block controllers (SATA,
> > > SCSI).
> > 
> > Description of the patch missing or is sub-par. You should work on this
> > skill.
> > 
> > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > > ---
> > > 
> > >  Makefile                   |   1 +
> > >  drivers/blockctrl/Makefile |  42 ++++++
> > >  drivers/blockctrl/core.c   | 349
> > > 
> > > +++++++++++++++++++++++++++++++++++++++++++++ include/dm/blockctrl.h   
> > >  |
> > > 
> > >  75 ++++++++++
> > >  4 files changed, 467 insertions(+)
> > >  create mode 100644 drivers/blockctrl/Makefile
> > >  create mode 100644 drivers/blockctrl/core.c
> > >  create mode 100644 include/dm/blockctrl.h
> > > 
> > > diff --git a/Makefile b/Makefile
> > > index e43fd9d..4420484 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
> > > 
> > >  LIBS-$(CONFIG_DM) += common/dm/libdm.o
> > >  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
> > >  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> > > 
> > > +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
> > 
> > ${} ? What is this ?

Why not just reuse drivers/block and in drivers/block compile in the libblock.o 
so you don't polute the top-level makefile ? Easy as that.

> > [..]
> > 
> > This handles SCSI? Sata ? what ?
> > 
> > Should this not be called scsi_core ? sata_core ? What did the previous
> > core do? sata?  scsi? block? I'm lost.
> 
> the previous core handled disks (and cards and stuff) and partitions (think
> /dev/sdxy), and was largely a replacement of /disk
> this core handles any interface those disks are connected to (SATA, PATA,
> SCSI), and should replace /drivers/block

Why is this not in the commit message then ? I have a proposal, before you 
submit a patchset, prepare it, work on something else for a bit, then read again 
the commit message only and see if you still understand what it means.

Am I correct that this will look as such:
user -> [ 01/11 ] -> [ 03/11 or something else ] -> [ if 03/11, then disc ]

> > I stop here, I don't know what this is all about, sorry.
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut
Pavel Herrmann Sept. 21, 2012, 1:14 p.m. UTC | #4
On Friday 21 of September 2012 14:51:33 Marek Vasut wrote:
> Dear Pavel Herrmann,
> 
> > On Thursday 20 of September 2012 22:05:36 Marek Vasut wrote:
> > > Dear Pavel Herrmann,
> > > 
> > > > This core provides unified access to different block controllers
> > > > (SATA,
> > > > SCSI).
> > > 
> > > Description of the patch missing or is sub-par. You should work on this
> > > skill.
> > > 
> > > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > > > ---
> > > > 
> > > >  Makefile                   |   1 +
> > > >  drivers/blockctrl/Makefile |  42 ++++++
> > > >  drivers/blockctrl/core.c   | 349
> > > > 
> > > > +++++++++++++++++++++++++++++++++++++++++++++ include/dm/blockctrl.h
> > > > 
> > > >  75 ++++++++++
> > > >  4 files changed, 467 insertions(+)
> > > >  create mode 100644 drivers/blockctrl/Makefile
> > > >  create mode 100644 drivers/blockctrl/core.c
> > > >  create mode 100644 include/dm/blockctrl.h
> > > > 
> > > > diff --git a/Makefile b/Makefile
> > > > index e43fd9d..4420484 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
> > > > 
> > > >  LIBS-$(CONFIG_DM) += common/dm/libdm.o
> > > >  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
> > > >  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> > > > 
> > > > +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
> > > 
> > > ${} ? What is this ?
> 
> Why not just reuse drivers/block and in drivers/block compile in the
> libblock.o so you don't polute the top-level makefile ? Easy as that.
> 
> > > [..]
> > > 
> > > This handles SCSI? Sata ? what ?
> > > 
> > > Should this not be called scsi_core ? sata_core ? What did the previous
> > > core do? sata?  scsi? block? I'm lost.
> > 
> > the previous core handled disks (and cards and stuff) and partitions
> > (think
> > /dev/sdxy), and was largely a replacement of /disk
> > this core handles any interface those disks are connected to (SATA, PATA,
> > SCSI), and should replace /drivers/block
> 
> Why is this not in the commit message then ? I have a proposal, before you
> submit a patchset, prepare it, work on something else for a bit, then read
> again the commit message only and see if you still understand what it
> means.

I actually did. the "something else" was splitting it into smaller patches, so 
the original text information got distributed into the other patches. if i put 
it all here you would surely complain about it not being there, or it being 
duplicated

> Am I correct that this will look as such:
> user -> [ 01/11 ] -> [ 03/11 or something else ] -> [ if 03/11, then disc ]

no idea what this means, sorry

Pavel Herrmann
Pavel Herrmann Sept. 21, 2012, 1:33 p.m. UTC | #5
On Friday 21 of September 2012 14:51:33 Marek Vasut wrote:
> Dear Pavel Herrmann,
> 
> > On Thursday 20 of September 2012 22:05:36 Marek Vasut wrote:
> > > Dear Pavel Herrmann,
> > > 
> > > > This core provides unified access to different block controllers
> > > > (SATA,
> > > > SCSI).
> > > 
> > > Description of the patch missing or is sub-par. You should work on this
> > > skill.
> > > 
> > > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > > > ---
> > > > 
> > > >  Makefile                   |   1 +
> > > >  drivers/blockctrl/Makefile |  42 ++++++
> > > >  drivers/blockctrl/core.c   | 349
> > > > 
> > > > +++++++++++++++++++++++++++++++++++++++++++++ include/dm/blockctrl.h
> > > > 
> > > >  75 ++++++++++
> > > >  4 files changed, 467 insertions(+)
> > > >  create mode 100644 drivers/blockctrl/Makefile
> > > >  create mode 100644 drivers/blockctrl/core.c
> > > >  create mode 100644 include/dm/blockctrl.h
> > > > 
> > > > diff --git a/Makefile b/Makefile
> > > > index e43fd9d..4420484 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
> > > > 
> > > >  LIBS-$(CONFIG_DM) += common/dm/libdm.o
> > > >  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
> > > >  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> > > > 
> > > > +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
> > > 
> > > ${} ? What is this ?
> 
> Why not just reuse drivers/block and in drivers/block compile in the
> libblock.o so you don't polute the top-level makefile ? Easy as that.

to make a distinction between drivers that are converted to new API and those 
that are not, and to enable drivers to have the same filename for original and 
converted versions.

Pavel Herrmann
Marek Vasut Sept. 21, 2012, 1:56 p.m. UTC | #6
Dear Pavel Herrmann,

> On Friday 21 of September 2012 14:51:33 Marek Vasut wrote:
> > Dear Pavel Herrmann,
> > 
> > > On Thursday 20 of September 2012 22:05:36 Marek Vasut wrote:
> > > > Dear Pavel Herrmann,
> > > > 
> > > > > This core provides unified access to different block controllers
> > > > > (SATA,
> > > > > SCSI).
> > > > 
> > > > Description of the patch missing or is sub-par. You should work on
> > > > this skill.
> > > > 
> > > > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > > > > ---
> > > > > 
> > > > >  Makefile                   |   1 +
> > > > >  drivers/blockctrl/Makefile |  42 ++++++
> > > > >  drivers/blockctrl/core.c   | 349
> > > > > 
> > > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > > > include/dm/blockctrl.h
> > > > > 
> > > > >  75 ++++++++++
> > > > >  4 files changed, 467 insertions(+)
> > > > >  create mode 100644 drivers/blockctrl/Makefile
> > > > >  create mode 100644 drivers/blockctrl/core.c
> > > > >  create mode 100644 include/dm/blockctrl.h
> > > > > 
> > > > > diff --git a/Makefile b/Makefile
> > > > > index e43fd9d..4420484 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
> > > > > 
> > > > >  LIBS-$(CONFIG_DM) += common/dm/libdm.o
> > > > >  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
> > > > >  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> > > > > 
> > > > > +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
> > > > 
> > > > ${} ? What is this ?
> > 
> > Why not just reuse drivers/block and in drivers/block compile in the
> > libblock.o so you don't polute the top-level makefile ? Easy as that.
> > 
> > > > [..]
> > > > 
> > > > This handles SCSI? Sata ? what ?
> > > > 
> > > > Should this not be called scsi_core ? sata_core ? What did the
> > > > previous core do? sata?  scsi? block? I'm lost.
> > > 
> > > the previous core handled disks (and cards and stuff) and partitions
> > > (think
> > > /dev/sdxy), and was largely a replacement of /disk
> > > this core handles any interface those disks are connected to (SATA,
> > > PATA, SCSI), and should replace /drivers/block
> > 
> > Why is this not in the commit message then ? I have a proposal, before
> > you submit a patchset, prepare it, work on something else for a bit,
> > then read again the commit message only and see if you still understand
> > what it means.
> 
> I actually did. the "something else" was splitting it into smaller patches,

I mean something totally different, so you won't have the code in front of you. 
You DO understand the code because you wrote it, you need to work on the part 
where you explain others properly what your change does. Even if it mean writing 
essay-esque commit message.

> so the original text information got distributed into the other patches.
> if i put it all here you would surely complain about it not being there,
> or it being duplicated

Not really ...

> > Am I correct that this will look as such:
> > user -> [ 01/11 ] -> [ 03/11 or something else ] -> [ if 03/11, then disc
> > ]
> 
> no idea what this means, sorry

Patch numbers, how the code added in them connect into each other.
> 
> Pavel Herrmann

Best regards,
Marek Vasut
Marek Vasut Sept. 21, 2012, 1:58 p.m. UTC | #7
Dear Pavel Herrmann,

> On Friday 21 of September 2012 14:51:33 Marek Vasut wrote:
> > Dear Pavel Herrmann,
> > 
> > > On Thursday 20 of September 2012 22:05:36 Marek Vasut wrote:
> > > > Dear Pavel Herrmann,
> > > > 
> > > > > This core provides unified access to different block controllers
> > > > > (SATA,
> > > > > SCSI).
> > > > 
> > > > Description of the patch missing or is sub-par. You should work on
> > > > this skill.
> > > > 
> > > > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > > > > ---
> > > > > 
> > > > >  Makefile                   |   1 +
> > > > >  drivers/blockctrl/Makefile |  42 ++++++
> > > > >  drivers/blockctrl/core.c   | 349
> > > > > 
> > > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > > > include/dm/blockctrl.h
> > > > > 
> > > > >  75 ++++++++++
> > > > >  4 files changed, 467 insertions(+)
> > > > >  create mode 100644 drivers/blockctrl/Makefile
> > > > >  create mode 100644 drivers/blockctrl/core.c
> > > > >  create mode 100644 include/dm/blockctrl.h
> > > > > 
> > > > > diff --git a/Makefile b/Makefile
> > > > > index e43fd9d..4420484 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
> > > > > 
> > > > >  LIBS-$(CONFIG_DM) += common/dm/libdm.o
> > > > >  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
> > > > >  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> > > > > 
> > > > > +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
> > > > 
> > > > ${} ? What is this ?
> > 
> > Why not just reuse drivers/block and in drivers/block compile in the
> > libblock.o so you don't polute the top-level makefile ? Easy as that.
> 
> to make a distinction between drivers that are converted to new API and
> those that are not, and to enable drivers to have the same filename for
> original and converted versions.

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.

How does that work? It's much cleaner.

> Pavel Herrmann

Best regards,
Marek Vasut
Pavel Herrmann Sept. 21, 2012, 3:04 p.m. UTC | #8
On Friday 21 of September 2012 15:56:38 Marek Vasut wrote:
> Dear Pavel Herrmann,
> 
> > On Friday 21 of September 2012 14:51:33 Marek Vasut wrote:
> > > Dear Pavel Herrmann,
> > > 
> > > > On Thursday 20 of September 2012 22:05:36 Marek Vasut wrote:
> > > > > Dear Pavel Herrmann,
> > > > > 
> > > > > > This core provides unified access to different block controllers
> > > > > > (SATA,
> > > > > > SCSI).
> > > > > 
> > > > > Description of the patch missing or is sub-par. You should work on
> > > > > this skill.
> > > > > 
> > > > > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > > > > > ---
> > > > > > 
> > > > > >  Makefile                   |   1 +
> > > > > >  drivers/blockctrl/Makefile |  42 ++++++
> > > > > >  drivers/blockctrl/core.c   | 349
> > > > > > 
> > > > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > include/dm/blockctrl.h
> > > > > > 
> > > > > >  75 ++++++++++
> > > > > >  4 files changed, 467 insertions(+)
> > > > > >  create mode 100644 drivers/blockctrl/Makefile
> > > > > >  create mode 100644 drivers/blockctrl/core.c
> > > > > >  create mode 100644 include/dm/blockctrl.h
> > > > > > 
> > > > > > diff --git a/Makefile b/Makefile
> > > > > > index e43fd9d..4420484 100644
> > > > > > --- a/Makefile
> > > > > > +++ b/Makefile
> > > > > > @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
> > > > > > 
> > > > > >  LIBS-$(CONFIG_DM) += common/dm/libdm.o
> > > > > >  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
> > > > > >  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> > > > > > 
> > > > > > +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
> > > > > 
> > > > > ${} ? What is this ?
> > > 
> > > Why not just reuse drivers/block and in drivers/block compile in the
> > > libblock.o so you don't polute the top-level makefile ? Easy as that.
> > > 
> > > > > [..]
> > > > > 
> > > > > This handles SCSI? Sata ? what ?
> > > > > 
> > > > > Should this not be called scsi_core ? sata_core ? What did the
> > > > > previous core do? sata?  scsi? block? I'm lost.
> > > > 
> > > > the previous core handled disks (and cards and stuff) and partitions
> > > > (think
> > > > /dev/sdxy), and was largely a replacement of /disk
> > > > this core handles any interface those disks are connected to (SATA,
> > > > PATA, SCSI), and should replace /drivers/block
> > > 
> > > Why is this not in the commit message then ? I have a proposal, before
> > > you submit a patchset, prepare it, work on something else for a bit,
> > > then read again the commit message only and see if you still understand
> > > what it means.
> > 
> > I actually did. the "something else" was splitting it into smaller
> > patches,
> 
> I mean something totally different, so you won't have the code in front of
> you. You DO understand the code because you wrote it, you need to work on
> the part where you explain others properly what your change does. Even if
> it mean writing essay-esque commit message.

ok, next time

> > so the original text information got distributed into the other patches.
> > if i put it all here you would surely complain about it not being there,
> > or it being duplicated
> 
> Not really ...
> 
> > > Am I correct that this will look as such:
> > > user -> [ 01/11 ] -> [ 03/11 or something else ] -> [ if 03/11, then
> > > disc
> > > ]
> > 
> > no idea what this means, sorry
> 
> Patch numbers, how the code added in them connect into each other.

ok then

user -> [7/11] -> [1/11] -> [5/11] (partition) -> [1/11] -> [5/11] (ata) -> 
[3/11] -> [4/11] (or another driver)

if you have FS on a whole disk and not just a partition, you omit the first 
[5/11] -> [1/11] bit, if your filesystem is on a card or a flash driver you 
replace the [5/11] -> [3/11] bit

Pavel Herrmann
Pavel Herrmann Sept. 21, 2012, 3:09 p.m. UTC | #9
On Friday 21 of September 2012 15:58:55 Marek Vasut wrote:
> Dear Pavel Herrmann,
> 
> > On Friday 21 of September 2012 14:51:33 Marek Vasut wrote:
> > > Dear Pavel Herrmann,
> > > 
> > > > On Thursday 20 of September 2012 22:05:36 Marek Vasut wrote:
> > > > > Dear Pavel Herrmann,
> > > > > 
> > > > > > This core provides unified access to different block controllers
> > > > > > (SATA,
> > > > > > SCSI).
> > > > > 
> > > > > Description of the patch missing or is sub-par. You should work on
> > > > > this skill.
> > > > > 
> > > > > > Signed-off-by: Pavel Herrmann <morpheus.ibis@gmail.com>
> > > > > > ---
> > > > > > 
> > > > > >  Makefile                   |   1 +
> > > > > >  drivers/blockctrl/Makefile |  42 ++++++
> > > > > >  drivers/blockctrl/core.c   | 349
> > > > > > 
> > > > > > +++++++++++++++++++++++++++++++++++++++++++++
> > > > > > include/dm/blockctrl.h
> > > > > > 
> > > > > >  75 ++++++++++
> > > > > >  4 files changed, 467 insertions(+)
> > > > > >  create mode 100644 drivers/blockctrl/Makefile
> > > > > >  create mode 100644 drivers/blockctrl/core.c
> > > > > >  create mode 100644 include/dm/blockctrl.h
> > > > > > 
> > > > > > diff --git a/Makefile b/Makefile
> > > > > > index e43fd9d..4420484 100644
> > > > > > --- a/Makefile
> > > > > > +++ b/Makefile
> > > > > > @@ -304,6 +304,7 @@ LIBS-y += test/libtest.o
> > > > > > 
> > > > > >  LIBS-$(CONFIG_DM) += common/dm/libdm.o
> > > > > >  LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
> > > > > >  LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
> > > > > > 
> > > > > > +LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
> > > > > 
> > > > > ${} ? What is this ?
> > > 
> > > Why not just reuse drivers/block and in drivers/block compile in the
> > > libblock.o so you don't polute the top-level makefile ? Easy as that.
> > 
> > to make a distinction between drivers that are converted to new API and
> > those that are not, and to enable drivers to have the same filename for
> > original and converted versions.
> 
> 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.

> How does that work? It's much cleaner.
> 
> > Pavel Herrmann
> 
> Best regards,
> Marek Vasut
Marek Vasut Sept. 21, 2012, 3:39 p.m. UTC | #10
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?

> > How does that work? It's much cleaner.
> > 
> > > Pavel Herrmann
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut
Pavel Herrmann Sept. 21, 2012, 3:46 p.m. UTC | #11
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

> > > How does that work? It's much cleaner.
> > > 
> > > > Pavel Herrmann
> > > 
> > > Best regards,
> > > Marek Vasut
> 
> Best regards,
> Marek Vasut
Marek Vasut Sept. 21, 2012, 4:08 p.m. UTC | #12
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.

Would you need the compat layer at all were you to follow my advice?

The whole idea goes deeper, see if you prepended this patchset with such a 
conversion as above, you'd already have a readily defined structure of blockdev 
operations in each driver to use in this patchset. This patchset would then 
really only be the DM stuff. The DM-part addition to the drivers would be 
mechanical.

> > > > How does that work? It's much cleaner.
> > > > 
> > > > > Pavel Herrmann
> > > > 
> > > > Best regards,
> > > > Marek Vasut
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut
Pavel Herrmann Sept. 21, 2012, 5:22 p.m. UTC | #13
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

> The whole idea goes deeper, see if you prepended this patchset with such a
> conversion as above, you'd already have a readily defined structure of
> blockdev operations in each driver to use in this patchset. This patchset
> would then really only be the DM stuff. The DM-part addition to the drivers
> would be mechanical.
> 
> > > > > How does that work? It's much cleaner.
> > > > > 
> > > > > > Pavel Herrmann
> > > > > 
> > > > > Best regards,
> > > > > Marek Vasut
> > > 
> > > Best regards,
> > > Marek Vasut
> 
> Best regards,
> Marek Vasut
Marek Vasut Sept. 21, 2012, 6:01 p.m. UTC | #14
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.

> > The whole idea goes deeper, see if you prepended this patchset with such
> > a conversion as above, you'd already have a readily defined structure of
> > blockdev operations in each driver to use in this patchset. This
> > patchset would then really only be the DM stuff. The DM-part addition to
> > the drivers would be mechanical.
> > 
> > > > > > How does that work? It's much cleaner.
> > > > > > 
> > > > > > > Pavel Herrmann
> > > > > > 
> > > > > > Best regards,
> > > > > > Marek Vasut
> > > > 
> > > > Best regards,
> > > > Marek Vasut
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut
Pavel Herrmann Sept. 21, 2012, 7:15 p.m. UTC | #15
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. 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. 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)

> > > The whole idea goes deeper, see if you prepended this patchset with such
> > > a conversion as above, you'd already have a readily defined structure of
> > > blockdev operations in each driver to use in this patchset. This
> > > patchset would then really only be the DM stuff. The DM-part addition to
> > > the drivers would be mechanical.
> > > 
> > > > > > > How does that work? It's much cleaner.
> > > > > > > 
> > > > > > > > Pavel Herrmann
> > > > > > > 
> > > > > > > Best regards,
> > > > > > > Marek Vasut
> > > > > 
> > > > > Best regards,
> > > > > Marek Vasut
> > > 
> > > Best regards,
> > > Marek Vasut
> 
> Best regards,
> Marek Vasut
Marek Vasut Sept. 21, 2012, 7:22 p.m. UTC | #16
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
diff mbox

Patch

diff --git a/Makefile b/Makefile
index e43fd9d..4420484 100644
--- a/Makefile
+++ b/Makefile
@@ -304,6 +304,7 @@  LIBS-y += test/libtest.o
 LIBS-$(CONFIG_DM) += common/dm/libdm.o
 LIBS-$(CONFIG_DM) += drivers/demo/libdemo.o
 LIBS-${CONFIG_DM_BLOCK} += drivers/blockdev/libblockdev.o
+LIBS-${CONFIG_DM_BLOCK} += drivers/blockctrl/libblockctrl.o
 
 ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP34XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX),)
 LIBS-y += $(CPUDIR)/omap-common/libomap-common.o
diff --git a/drivers/blockctrl/Makefile b/drivers/blockctrl/Makefile
new file mode 100644
index 0000000..21a9094
--- /dev/null
+++ b/drivers/blockctrl/Makefile
@@ -0,0 +1,42 @@ 
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	:= $(obj)libblockctrl.o
+
+COBJS-${CONFIG_DM_BLOCK} := core.o
+
+COBJS	:= $(COBJS-y)
+SRCS	:= $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+
+all:	$(LIB)
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(call cmd_link_o_target, $(OBJS))
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/drivers/blockctrl/core.c b/drivers/blockctrl/core.c
new file mode 100644
index 0000000..1f1f834
--- /dev/null
+++ b/drivers/blockctrl/core.c
@@ -0,0 +1,349 @@ 
+/*
+ * (C) Copyright 2012
+ * Pavel Herrmann <morpheus.ibis@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <dm/blockctrl.h>
+#include <dm/manager.h>
+#include <malloc.h>
+
+struct blockctrl_core_entry {
+	struct list_head	list;
+	struct instance		*instance;
+	struct blockctrl_ops	*ops;
+};
+
+static struct blockctrl_core_entry *get_entry_by_instance(struct instance *dev)
+{
+	struct blockctrl_core_entry *entry;
+	struct core_instance *core = get_core_instance(CORE_BLOCKCTRL);
+
+	if (!core)
+		return NULL;
+
+	list_for_each_entry(entry, &core->succ, list) {
+		if (entry->instance == dev)
+			return entry;
+	}
+
+	return NULL;
+}
+
+/* Core API functions */
+static int get_count(struct core_instance *core)
+{
+	int count = 0;
+	struct blockctrl_core_entry *entry;
+
+	list_for_each_entry(entry, &core->succ, list) {
+		count++;
+	}
+
+	return count;
+}
+
+static struct instance *get_child(struct core_instance *core, int index)
+{
+	struct blockctrl_core_entry *entry = NULL;
+
+	list_for_each_entry(entry, &core->succ, list) {
+		if (!index)
+			return entry->instance;
+		index--;
+	}
+
+	return NULL;
+}
+
+static int bind(struct core_instance *core, struct instance *dev, void *ops,
+	void *data)
+{
+	struct blockctrl_core_entry *entry;
+
+	if (ops == NULL)
+		return -EINVAL;
+
+	entry = malloc(sizeof(*entry));
+	if (entry == NULL)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&entry->list);
+	entry->instance = dev;
+	entry->ops = ops;
+	list_add_tail(&entry->list, &core->succ);
+
+	return 0;
+}
+
+static int unbind(struct core_instance *core, struct instance *dev)
+{
+	struct blockctrl_core_entry *entry, *n;
+
+	list_for_each_entry_safe(entry, n, &core->succ, list) {
+		if (entry->instance == dev) {
+			list_del(&entry->list);
+			free(entry);
+		}
+	}
+
+	return 0;
+}
+
+static int replace(struct core_instance *core, struct instance *new,
+	struct instance *old)
+{
+	struct blockctrl_core_entry *entry = get_entry_by_instance(old);
+
+	if (!entry)
+		return -ENOENT;
+
+	entry->instance = new;
+
+	return 0;
+}
+
+static int init(struct core_instance *core)
+{
+	INIT_LIST_HEAD(&core->succ);
+	core->private_data = NULL;
+
+	return 0;
+}
+
+static int reloc(struct core_instance *core, struct core_instance *old)
+{
+	struct blockctrl_core_entry *entry, *new;
+
+	/* no private_data to copy, yet */
+
+	/* fixup links in old list and prepare new list head */
+	/* FIXME */
+	/* list_fix_reloc(&old->succ); */
+	INIT_LIST_HEAD(&core->succ);
+	core->private_data = NULL;
+
+	/* copy list entries to new memory */
+	list_for_each_entry(entry, &old->succ, list) {
+		new = malloc(sizeof(*new));
+		if (!new)
+			return -ENOMEM;
+
+		INIT_LIST_HEAD(&new->list);
+		new->instance = entry->instance;
+		new->ops = entry->ops;
+		list_add_tail(&new->list, &core->succ);
+		/*no free at this point, old memory should not be freed*/
+	}
+
+	return 0;
+}
+
+static int destroy(struct core_instance *core)
+{
+	struct blockctrl_core_entry *entry, *n;
+
+	/* destroy private data */
+	free(core->private_data);
+	core->private_data = NULL;
+
+	/* destroy successor list */
+	list_for_each_entry_safe(entry, n, &core->succ, list) {
+		list_del(&entry->list);
+		free(entry);
+	}
+
+	return 0;
+}
+
+U_BOOT_CORE(CORE_BLOCKCTRL,
+	init,
+	reloc,
+	destroy,
+	get_count,
+	get_child,
+	bind,
+	unbind,
+	replace);
+
+lbaint_t blockctrl_read(struct instance *i, int port, lbaint_t start,
+	lbaint_t length, void *buffer)
+{
+	struct blockctrl_core_entry *entry;
+	struct blockctrl_ops *driver_ops;
+	int error;
+
+	entry = get_entry_by_instance(i);
+	if (!entry)
+		return -ENOENT;
+
+	error = driver_activate(i);
+	if (error)
+		return error;
+
+	driver_ops = entry->ops;
+	if (!driver_ops || !driver_ops->read)
+		return -EINVAL;
+
+	return driver_ops->read(i, port, start, length, buffer);
+}
+
+lbaint_t blockctrl_write(struct instance *i, int port, lbaint_t start,
+	lbaint_t length, void *buffer)
+{
+	struct blockctrl_core_entry *entry;
+	struct blockctrl_ops *driver_ops;
+	int error;
+
+	entry = get_entry_by_instance(i);
+	if (!entry)
+		return -ENOENT;
+
+	error = driver_activate(i);
+	if (error)
+		return error;
+
+	driver_ops = entry->ops;
+	if (!driver_ops || !driver_ops->write)
+		return -EINVAL;
+
+	return driver_ops->write(i, port, start, length, buffer);
+}
+
+int blockctrl_scan(struct instance *i, int port)
+{
+	struct blockctrl_core_entry *entry;
+	struct blockctrl_ops *driver_ops;
+	int error;
+
+	entry = get_entry_by_instance(i);
+	if (!entry)
+		return -ENOENT;
+
+	error = driver_activate(i);
+	if (error)
+		return error;
+
+	driver_ops = entry->ops;
+	if (!driver_ops || !driver_ops->scan)
+		return -EINVAL;
+
+	return driver_ops->scan(i, port);
+}
+
+int blockctrl_get_port_count(struct instance *i)
+{
+	struct blockctrl_core_entry *entry;
+	struct blockctrl_ops *driver_ops;
+	int error;
+
+	entry = get_entry_by_instance(i);
+	if (!entry)
+		return -ENOENT;
+
+	error = driver_activate(i);
+	if (error)
+		return error;
+
+	driver_ops = entry->ops;
+	if (!driver_ops || !driver_ops->get_port_count)
+		return -EINVAL;
+
+	return driver_ops->get_port_count(i);
+}
+
+int blockctrl_get_port_option(struct instance *i, int port,
+	enum blockctrl_port_option_code op, struct option *result)
+{
+	struct blockctrl_core_entry *entry;
+	struct blockctrl_ops *driver_ops;
+	int error;
+
+	entry = get_entry_by_instance(i);
+	if (!entry)
+		return -ENOENT;
+
+	error = driver_activate(i);
+	if (error)
+		return error;
+
+	driver_ops = entry->ops;
+	if (!driver_ops || !driver_ops->get_port_option)
+		return -EINVAL;
+
+	return driver_ops->get_port_option(i, port, op, result);
+}
+
+struct instance *blockctrl_rescan_port(struct instance *i, int port)
+{
+	/* we assume that all children of blockctrl are blockdev_ata */
+	struct instance *child = NULL;
+	struct blockctrl_core_entry *entry;
+	struct driver_instance *di;
+	struct blockdev_ata_platform_data *pdata;
+	struct driver_info *info;
+	int nfound;
+
+	entry = get_entry_by_instance(i);
+	if (!entry)
+		return NULL;
+
+	list_for_each_entry(di, &i->succ, list) {
+		if (di->i.info)
+			pdata = di->i.info->platform_data;
+		else
+			pdata = NULL;
+
+		if (pdata && (pdata->port_number == port))
+			child = &di->i;
+	}
+
+	/*
+	 * If we have an active link, we check whether this is a new device,
+	 * in which case we simply bind a new instance, or an old device,
+	 * in which case we reactivate the device to force partition rescan.
+	 * If we dont have an active link, we remove any device attached.
+	 */
+	nfound = blockctrl_scan(i, port);
+	if (!nfound) {
+		if (child) {
+			/* rescan the disk size and partitions, just in case */
+			driver_remove(child);
+			scan_partitions(child);
+		} else {
+			pdata = malloc(sizeof(*pdata));
+			pdata->port_number = port;
+			info = malloc(sizeof(*info));
+			info->platform_data = pdata;
+			info->name = "blockdev_ata";
+			child = driver_bind(i, info);
+			scan_partitions(child);
+			return child;
+		}
+	} else {
+		/* link is not active, remove and unbind the child device */
+		if (child) {
+			driver_remove(child);
+			driver_unbind(child);
+		}
+	}
+
+	return NULL;
+}
diff --git a/include/dm/blockctrl.h b/include/dm/blockctrl.h
new file mode 100644
index 0000000..4b6d582
--- /dev/null
+++ b/include/dm/blockctrl.h
@@ -0,0 +1,75 @@ 
+/*
+ * (C) Copyright 2012
+ * Pavel Herrmann <morpheus.ibis@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _DM_BLOCKCTRL_H_
+#define _DM_BLOCKCTRL_H 1
+
+#include <dm/structures.h>
+#include <dm/blockdev.h>
+
+enum blockctrl_port_option_code {
+	BLKP_OPT_IFTYPE = 0,
+	BLKP_OPT_IFSPEED,
+	BLKP_OPT_BLOCKSIZE,
+	BLKP_OPT_BLOCKCOUNT,
+	BLKP_OPT_REMOVABLE,
+	BLKP_OPT_LBA48,
+	BLKP_OPT_VENDOR,
+	BLKP_OPT_PRODUCT,
+	BLKP_OPT_REVISION,
+};
+
+enum blockctrl_iftype {
+	BLKP_IFTYPE_SATA,
+	BLKP_IFTYPE_PATA,
+	BLKP_IFTYPE_SCSI,
+	BLKP_IFTYPE_VIRTUAL,
+};
+
+
+struct blockctrl_ops {
+	lbaint_t	(*read)(struct instance *i, int port, lbaint_t start,
+				lbaint_t length, void *buffer);
+	lbaint_t	(*write)(struct instance *i, int port, lbaint_t start,
+				lbaint_t length, void *buffer);
+	int		(*scan)(struct instance *i, int port);
+	int		(*get_port_count)(struct instance *i);
+	int		(*get_port_option)(struct instance *i, int port,
+				enum blockctrl_port_option_code op,
+				struct option *result);
+};
+
+/* driver wrappers */
+lbaint_t blockctrl_read(struct instance *i, int port, lbaint_t start,
+	lbaint_t length, void *buffer);
+lbaint_t blockctrl_write(struct instance *i, int port, lbaint_t start,
+	lbaint_t length, void *buffer);
+int blockctrl_scan(struct instance *i, int port);
+int blockctrl_get_port_count(struct instance *i);
+int blockctrl_get_port_option(struct instance *i, int port,
+	enum blockctrl_port_option_code op, struct option *result);
+
+/* command helpers */
+struct instance *blockctrl_rescan_port(struct instance *i, int port);
+
+#endif