Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 03/11] DM: add block controller core
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.i...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
