| Message ID | 1502849350-40252-1-git-send-email-anoo@linux.vnet.ibm.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, Aug 16, 2017 at 11:39 AM, Adriana Kobylak <anoo@linux.vnet.ibm.com> wrote: > Add MTD and UBI support to the default g5 include, conditioned to > having MTDPARTS_DEFAULT defined. Can you explain what we're planning on using mtdparts for? As I understand it this is for specifying the mtd partitions on the command line. Are you proposing to move away from the device tree description? > This allows platforms to enable this support if desired without > adding it by default since it increases the size of u-boot. Do you have numbers? It's good to keep track in case we need to go back and save space in the future. > Signed-off-by: Adriana Kobylak <anoo@linux.vnet.ibm.com> > --- > include/configs/ast-g5-ncsi.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/configs/ast-g5-ncsi.h b/include/configs/ast-g5-ncsi.h > index 12d6684..b126e6e 100644 > --- a/include/configs/ast-g5-ncsi.h > +++ b/include/configs/ast-g5-ncsi.h > @@ -28,4 +28,14 @@ > > #define CONFIG_HW_WATCHDOG > > +/* MTD and UBI */ > +#ifdef MTDPARTS_DEFAULT > +#define CONFIG_FLASH_CFI_MTD > +#define CONFIG_CMD_UBI I thought Milton mentioned today that we didn't need to enable the UBI command in u-boot, but I may have misunderstood. > +#define CONFIG_RBTREE > +#define CONFIG_MTD_DEVICE > +#define CONFIG_MTD_PARTITIONS > +#define CONFIG_CMD_MTDPARTS > +#endif > + > #endif /* __AST_G5_NCSI_CONFIG_H */ > -- > 1.8.2.2 >
Thanks Joel, answers inline below. On 2017-08-21 06:03, Joel Stanley wrote: > On Wed, Aug 16, 2017 at 11:39 AM, Adriana Kobylak > <anoo@linux.vnet.ibm.com> wrote: >> Add MTD and UBI support to the default g5 include, conditioned to >> having MTDPARTS_DEFAULT defined. > > Can you explain what we're planning on using mtdparts for? As I > understand it this is for specifying the mtd partitions on the command > line. > > Are you proposing to move away from the device tree description? mtdparts is needed by u-boot to find the kernel in a ubi volume, for example the u-boot command "ubi part" requires it. Implementing the device tree in u-boot could be a future possibility but the current version of u-boot in openbmc can't read from any other chips besides the bmc, so we need to duplicate the device tree layout of the bmc chip in u-boot for now. > >> This allows platforms to enable this support if desired without >> adding it by default since it increases the size of u-boot. > > Do you have numbers? It's good to keep track in case we need to go > back and save space in the future. After you mentioned that you saw u-boot increasing by 122kB with an earlier patch, I removed UBIFS since it wasn't needed, bringing the increase down by 50kB. The new size is 302kB (increase of 72kB over current size). > >> Signed-off-by: Adriana Kobylak <anoo@linux.vnet.ibm.com> >> --- >> include/configs/ast-g5-ncsi.h | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/include/configs/ast-g5-ncsi.h >> b/include/configs/ast-g5-ncsi.h >> index 12d6684..b126e6e 100644 >> --- a/include/configs/ast-g5-ncsi.h >> +++ b/include/configs/ast-g5-ncsi.h >> @@ -28,4 +28,14 @@ >> >> #define CONFIG_HW_WATCHDOG >> >> +/* MTD and UBI */ >> +#ifdef MTDPARTS_DEFAULT >> +#define CONFIG_FLASH_CFI_MTD >> +#define CONFIG_CMD_UBI > > I thought Milton mentioned today that we didn't need to enable the UBI > command in u-boot, but I may have misunderstood. It is needed since the kernel is in ubi volume. > >> +#define CONFIG_RBTREE >> +#define CONFIG_MTD_DEVICE >> +#define CONFIG_MTD_PARTITIONS >> +#define CONFIG_CMD_MTDPARTS >> +#endif >> + >> #endif /* __AST_G5_NCSI_CONFIG_H */ >> -- >> 1.8.2.2 >>
On Tue, 2017-08-15 at 21:09 -0500, Adriana Kobylak wrote: > Add MTD and UBI support to the default g5 include, conditioned to > having MTDPARTS_DEFAULT defined. > > This allows platforms to enable this support if desired without > adding it by default since it increases the size of u-boot. > > Signed-off-by: Adriana Kobylak <anoo@linux.vnet.ibm.com> > --- > include/configs/ast-g5-ncsi.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/configs/ast-g5-ncsi.h b/include/configs/ast-g5- > ncsi.h > index 12d6684..b126e6e 100644 > --- a/include/configs/ast-g5-ncsi.h > +++ b/include/configs/ast-g5-ncsi.h I suggest we make this common to all platforms to reduce the configuration headache. Putting the #defines in ast-g5-ncsi doesn't isolate the change to Witherspoon, and I think we should avoid further fragmentation e.g. by introducing a Witherspoon-specific configuration. If other platforms need to recover the extra space required we can deal with that on an as-needed basis in the future. Cheers, Andrew > @@ -28,4 +28,14 @@ > > #define CONFIG_HW_WATCHDOG > > +/* MTD and UBI */ > +#ifdef MTDPARTS_DEFAULT > +#define CONFIG_FLASH_CFI_MTD > +#define CONFIG_CMD_UBI > +#define CONFIG_RBTREE > +#define CONFIG_MTD_DEVICE > +#define CONFIG_MTD_PARTITIONS > +#define CONFIG_CMD_MTDPARTS > +#endif > + > #endif /* __AST_G5_NCSI_CONFIG_H */
diff --git a/include/configs/ast-g5-ncsi.h b/include/configs/ast-g5-ncsi.h index 12d6684..b126e6e 100644 --- a/include/configs/ast-g5-ncsi.h +++ b/include/configs/ast-g5-ncsi.h @@ -28,4 +28,14 @@ #define CONFIG_HW_WATCHDOG +/* MTD and UBI */ +#ifdef MTDPARTS_DEFAULT +#define CONFIG_FLASH_CFI_MTD +#define CONFIG_CMD_UBI +#define CONFIG_RBTREE +#define CONFIG_MTD_DEVICE +#define CONFIG_MTD_PARTITIONS +#define CONFIG_CMD_MTDPARTS +#endif + #endif /* __AST_G5_NCSI_CONFIG_H */
Add MTD and UBI support to the default g5 include, conditioned to having MTDPARTS_DEFAULT defined. This allows platforms to enable this support if desired without adding it by default since it increases the size of u-boot. Signed-off-by: Adriana Kobylak <anoo@linux.vnet.ibm.com> --- include/configs/ast-g5-ncsi.h | 10 ++++++++++ 1 file changed, 10 insertions(+)