| Message ID | 20220311191005.364540-2-sjg@chromium.org |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | Reduce size of a few boards | expand |
On Fri, Mar 11, 2022 at 12:10:01PM -0700, Simon Glass wrote: > In some cases we do not want to enable partition support in SPL. Add an > option to allow this. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > disk/Kconfig | 24 ++++++++++++++++++++---- > disk/Makefile | 6 +++--- > drivers/block/blk-uclass.c | 2 +- > 3 files changed, 24 insertions(+), 8 deletions(-) I'm not sure this makes sense? I thought when I looked in to this last the only place where we had partition code being linked and not discarded in the case of SPL and no devices that would have partitions on them was one xilinx platform. How do we get to bringing in partition code and not having something that uses it?
Hi Tom, On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote: > > On Fri, Mar 11, 2022 at 12:10:01PM -0700, Simon Glass wrote: > > > In some cases we do not want to enable partition support in SPL. Add an > > option to allow this. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > disk/Kconfig | 24 ++++++++++++++++++++---- > > disk/Makefile | 6 +++--- > > drivers/block/blk-uclass.c | 2 +- > > 3 files changed, 24 insertions(+), 8 deletions(-) > > I'm not sure this makes sense? I thought when I looked in to this last > the only place where we had partition code being linked and not > discarded in the case of SPL and no devices that would have partitions > on them was one xilinx platform. How do we get to bringing in partition > code and not having something that uses it? The problem is that drivers are not discarded and Takahiro's series adds a driver for partitions. So yes, we were able to get away with this before, but cannot now. Regards, Simon
On Mon, Mar 14, 2022 at 12:24:41PM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote: > > > > On Fri, Mar 11, 2022 at 12:10:01PM -0700, Simon Glass wrote: > > > > > In some cases we do not want to enable partition support in SPL. Add an > > > option to allow this. > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > --- > > > > > > disk/Kconfig | 24 ++++++++++++++++++++---- > > > disk/Makefile | 6 +++--- > > > drivers/block/blk-uclass.c | 2 +- > > > 3 files changed, 24 insertions(+), 8 deletions(-) > > > > I'm not sure this makes sense? I thought when I looked in to this last > > the only place where we had partition code being linked and not > > discarded in the case of SPL and no devices that would have partitions > > on them was one xilinx platform. How do we get to bringing in partition > > code and not having something that uses it? > > The problem is that drivers are not discarded and Takahiro's series > adds a driver for partitions. > > So yes, we were able to get away with this before, but cannot now. Things which aren't needed / used need to be discarded. So we might have use cases for this, yes (I was thinking after I sent that OK, yes, imx probably tends to not need partition support since we read at raw offsets, outside from maybe falcon mode using targets). But we shouldn't generally be now pulling in drivers that aren't functionally used.
Hi Tom, On Mon, 14 Mar 2022 at 12:31, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Mar 14, 2022 at 12:24:41PM -0600, Simon Glass wrote: > > Hi Tom, > > > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Fri, Mar 11, 2022 at 12:10:01PM -0700, Simon Glass wrote: > > > > > > > In some cases we do not want to enable partition support in SPL. Add an > > > > option to allow this. > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > --- > > > > > > > > disk/Kconfig | 24 ++++++++++++++++++++---- > > > > disk/Makefile | 6 +++--- > > > > drivers/block/blk-uclass.c | 2 +- > > > > 3 files changed, 24 insertions(+), 8 deletions(-) > > > > > > I'm not sure this makes sense? I thought when I looked in to this last > > > the only place where we had partition code being linked and not > > > discarded in the case of SPL and no devices that would have partitions > > > on them was one xilinx platform. How do we get to bringing in partition > > > code and not having something that uses it? > > > > The problem is that drivers are not discarded and Takahiro's series > > adds a driver for partitions. > > > > So yes, we were able to get away with this before, but cannot now. > > Things which aren't needed / used need to be discarded. So we might > have use cases for this, yes (I was thinking after I sent that OK, yes, > imx probably tends to not need partition support since we read at raw > offsets, outside from maybe falcon mode using targets). But we > shouldn't generally be now pulling in drivers that aren't functionally > used. We don't have a way of detecting whether a driver is used, with driver model, so the only option is Kconfig. Does that make sense? Please let me know if there is any disconnect here. Regards, Simon
On Mon, Mar 14, 2022 at 01:21:07PM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 14 Mar 2022 at 12:31, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Mar 14, 2022 at 12:24:41PM -0600, Simon Glass wrote: > > > Hi Tom, > > > > > > On Mon, 14 Mar 2022 at 06:49, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Fri, Mar 11, 2022 at 12:10:01PM -0700, Simon Glass wrote: > > > > > > > > > In some cases we do not want to enable partition support in SPL. Add an > > > > > option to allow this. > > > > > > > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > > > > --- > > > > > > > > > > disk/Kconfig | 24 ++++++++++++++++++++---- > > > > > disk/Makefile | 6 +++--- > > > > > drivers/block/blk-uclass.c | 2 +- > > > > > 3 files changed, 24 insertions(+), 8 deletions(-) > > > > > > > > I'm not sure this makes sense? I thought when I looked in to this last > > > > the only place where we had partition code being linked and not > > > > discarded in the case of SPL and no devices that would have partitions > > > > on them was one xilinx platform. How do we get to bringing in partition > > > > code and not having something that uses it? > > > > > > The problem is that drivers are not discarded and Takahiro's series > > > adds a driver for partitions. > > > > > > So yes, we were able to get away with this before, but cannot now. > > > > Things which aren't needed / used need to be discarded. So we might > > have use cases for this, yes (I was thinking after I sent that OK, yes, > > imx probably tends to not need partition support since we read at raw > > offsets, outside from maybe falcon mode using targets). But we > > shouldn't generally be now pulling in drivers that aren't functionally > > used. > > We don't have a way of detecting whether a driver is used, with driver > model, so the only option is Kconfig. Does that make sense? Please let > me know if there is any disconnect here. Yes, it's possible that we have platforms enabling functionality they don't _need_, but unlikely (aside from the we don't need partition reading, we do raw offsets from block device only platforms). So there shouldn't be a whole bunch of stuff now being pulled in to SPL that wasn't before and doesn't need investigation / explanation.
On Fri, Mar 11, 2022 at 12:10:01PM -0700, Simon Glass wrote: > In some cases we do not want to enable partition support in SPL. Add an > option to allow this. > > Signed-off-by: Simon Glass <sjg@chromium.org> With a slight tweak (as disk/disk-uclass.c isn't in yet), applied to u-boot/next, thanks!
Hi Simon, On Fri, Mar 11, 2022 at 12:10:01PM -0700, Simon Glass wrote: > In some cases we do not want to enable partition support in SPL. Add an > option to allow this. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > disk/Kconfig | 24 ++++++++++++++++++++---- > disk/Makefile | 6 +++--- > drivers/block/blk-uclass.c | 2 +- > 3 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/disk/Kconfig b/disk/Kconfig > index cee16a80bc2..13700322e97 100644 > --- a/disk/Kconfig > +++ b/disk/Kconfig > @@ -4,10 +4,6 @@ menu "Partition Types" > config PARTITIONS > bool "Enable Partition Labels (disklabels) support" > default y > - select SPL_SPRINTF if SPL > - select TPL_SPRINTF if TPL > - select SPL_STRTO if SPL > - select TPL_STRTO if TPL > help > Partition Labels (disklabels) Supported: > Zero or more of the following: > @@ -23,6 +19,26 @@ config PARTITIONS > you must configure support for at least one non-MTD partition type > as well. > > +config SPL_PARTITIONS > + bool "Enable Partition Labels (disklabels) support in SPL" > + default y if PARTITIONS > + select SPL_SPRINTF > + select SPL_STRTO > + help > + Enable this for base partition support in SPL. The required > + partition table types shold be enabled separately. This add a > + small amount of size to SPL, typically 500 bytes. Despite of you aim here, this option doesn't work. As I mentioned in https://lists.denx.de/pipermail/u-boot/2022-April/481258.html phycore-rk3288_defconfig, for instance, compiles disk/part.o (and disk/disk-uclass.o) in SPL image. Basically, I think that CONFIG_(SPL_)PARTITIONS should not be user-selectable. Instead, it should be implicitly selected by any of CONFIG_(SPL_)XXX_PARTITION's. -Takahiro Akashi > + > +config TPL_PARTITIONS > + bool "Enable Partition Labels (disklabels) support in TPL" > + default y if PARTITIONS > + select TPL_SPRINTF > + select TPL_STRTO > + help > + Enable this for base partition support in SPL. The required > + partition table types shold be enabled separately. This add a > + small amount of size to SPL, typically 500 bytes. > + > config MAC_PARTITION > bool "Enable Apple's MacOS partition table" > depends on PARTITIONS > diff --git a/disk/Makefile b/disk/Makefile > index ec37b74f5f4..ffd7b07f867 100644 > --- a/disk/Makefile > +++ b/disk/Makefile > @@ -5,9 +5,9 @@ > > #ccflags-y += -DET_DEBUG -DDEBUG > > -obj-$(CONFIG_PARTITIONS) += part.o > -ifdef CONFIG_$(SPL_)BLK > -obj-$(CONFIG_PARTITIONS) += disk-uclass.o > +obj-$(CONFIG_$(SPL_TPL_)PARTITIONS) += part.o > +ifdef CONFIG_$(SPL_TPL_)BLK > +obj-$(CONFIG_$(SPL_TPL_)PARTITIONS) += disk-uclass.o > endif > obj-$(CONFIG_$(SPL_)MAC_PARTITION) += part_mac.o > obj-$(CONFIG_$(SPL_)DOS_PARTITION) += part_dos.o > diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c > index 58dc74e71f1..bcd18ed38b2 100644 > --- a/drivers/block/blk-uclass.c > +++ b/drivers/block/blk-uclass.c > @@ -712,7 +712,7 @@ int blk_unbind_all(int if_type) > > static int blk_post_probe(struct udevice *dev) > { > - if (IS_ENABLED(CONFIG_PARTITIONS) && > + if (CONFIG_IS_ENABLED(PARTITIONS) && > IS_ENABLED(CONFIG_HAVE_BLOCK_DEVICE)) { > struct blk_desc *desc = dev_get_uclass_plat(dev); > > -- > 2.35.1.723.g4982287a31-goog >
diff --git a/disk/Kconfig b/disk/Kconfig index cee16a80bc2..13700322e97 100644 --- a/disk/Kconfig +++ b/disk/Kconfig @@ -4,10 +4,6 @@ menu "Partition Types" config PARTITIONS bool "Enable Partition Labels (disklabels) support" default y - select SPL_SPRINTF if SPL - select TPL_SPRINTF if TPL - select SPL_STRTO if SPL - select TPL_STRTO if TPL help Partition Labels (disklabels) Supported: Zero or more of the following: @@ -23,6 +19,26 @@ config PARTITIONS you must configure support for at least one non-MTD partition type as well. +config SPL_PARTITIONS + bool "Enable Partition Labels (disklabels) support in SPL" + default y if PARTITIONS + select SPL_SPRINTF + select SPL_STRTO + help + Enable this for base partition support in SPL. The required + partition table types shold be enabled separately. This add a + small amount of size to SPL, typically 500 bytes. + +config TPL_PARTITIONS + bool "Enable Partition Labels (disklabels) support in TPL" + default y if PARTITIONS + select TPL_SPRINTF + select TPL_STRTO + help + Enable this for base partition support in SPL. The required + partition table types shold be enabled separately. This add a + small amount of size to SPL, typically 500 bytes. + config MAC_PARTITION bool "Enable Apple's MacOS partition table" depends on PARTITIONS diff --git a/disk/Makefile b/disk/Makefile index ec37b74f5f4..ffd7b07f867 100644 --- a/disk/Makefile +++ b/disk/Makefile @@ -5,9 +5,9 @@ #ccflags-y += -DET_DEBUG -DDEBUG -obj-$(CONFIG_PARTITIONS) += part.o -ifdef CONFIG_$(SPL_)BLK -obj-$(CONFIG_PARTITIONS) += disk-uclass.o +obj-$(CONFIG_$(SPL_TPL_)PARTITIONS) += part.o +ifdef CONFIG_$(SPL_TPL_)BLK +obj-$(CONFIG_$(SPL_TPL_)PARTITIONS) += disk-uclass.o endif obj-$(CONFIG_$(SPL_)MAC_PARTITION) += part_mac.o obj-$(CONFIG_$(SPL_)DOS_PARTITION) += part_dos.o diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index 58dc74e71f1..bcd18ed38b2 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -712,7 +712,7 @@ int blk_unbind_all(int if_type) static int blk_post_probe(struct udevice *dev) { - if (IS_ENABLED(CONFIG_PARTITIONS) && + if (CONFIG_IS_ENABLED(PARTITIONS) && IS_ENABLED(CONFIG_HAVE_BLOCK_DEVICE)) { struct blk_desc *desc = dev_get_uclass_plat(dev);
In some cases we do not want to enable partition support in SPL. Add an option to allow this. Signed-off-by: Simon Glass <sjg@chromium.org> --- disk/Kconfig | 24 ++++++++++++++++++++---- disk/Makefile | 6 +++--- drivers/block/blk-uclass.c | 2 +- 3 files changed, 24 insertions(+), 8 deletions(-)