diff mbox series

[U-Boot,V2] imx: mx7dsabresd: include BLK for MMC/eMMC support of driver model

Message ID 1507758325-22467-1-git-send-email-eric@nelint.com
State Changes Requested
Delegated to: Stefano Babic
Headers show
Series [U-Boot,V2] imx: mx7dsabresd: include BLK for MMC/eMMC support of driver model | expand

Commit Message

Eric Nelson Oct. 11, 2017, 9:45 p.m. UTC 
Commit 6fbbcfd introduced device-tree support for MMC devices on
the mx7sabresd boards and didn't include BLK, which requires BLK.

Commit 8ae5bb3 did the same for secure boot.

Fix both by allowing blk-uclass (BLK) support.

Tested-by: Fabio Estevam <festevam@gmail.com>
Signed-off-by: Eric Nelson <eric@nelint.com>
---
V2 includes the updated to mx7dsabresd_secure_defconfig
 configs/mx7dsabresd_defconfig        | 1 -
 configs/mx7dsabresd_secure_defconfig | 1 -
 2 files changed, 2 deletions(-)

Comments

Tom Rini Oct. 11, 2017, 9:49 p.m. UTC | #1 
On Wed, Oct 11, 2017 at 02:45:25PM -0700, Eric Nelson wrote:

> Commit 6fbbcfd introduced device-tree support for MMC devices on
> the mx7sabresd boards and didn't include BLK, which requires BLK.
> 
> Commit 8ae5bb3 did the same for secure boot.
> 
> Fix both by allowing blk-uclass (BLK) support.
> 
> Tested-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Eric Nelson <eric@nelint.com>
> ---
> V2 includes the updated to mx7dsabresd_secure_defconfig
>  configs/mx7dsabresd_defconfig        | 1 -
>  configs/mx7dsabresd_secure_defconfig | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/configs/mx7dsabresd_defconfig b/configs/mx7dsabresd_defconfig
> index 795c4f2..144fb50 100644
> --- a/configs/mx7dsabresd_defconfig
> +++ b/configs/mx7dsabresd_defconfig
> @@ -38,7 +38,6 @@ CONFIG_CMD_EXT4=y
>  CONFIG_CMD_EXT4_WRITE=y
>  CONFIG_CMD_FAT=y
>  CONFIG_OF_CONTROL=y
> -# CONFIG_BLK is not set
>  CONFIG_DFU_MMC=y
>  CONFIG_DFU_RAM=y
>  CONFIG_DM_GPIO=y
> diff --git a/configs/mx7dsabresd_secure_defconfig b/configs/mx7dsabresd_secure_defconfig
> index bd68831..d1af138 100644
> --- a/configs/mx7dsabresd_secure_defconfig
> +++ b/configs/mx7dsabresd_secure_defconfig
> @@ -40,7 +40,6 @@ CONFIG_CMD_EXT4=y
>  CONFIG_CMD_EXT4_WRITE=y
>  CONFIG_CMD_FAT=y
>  CONFIG_OF_CONTROL=y
> -# CONFIG_BLK is not set
>  CONFIG_DFU_MMC=y
>  CONFIG_DFU_RAM=y
>  CONFIG_DM_GPIO=y

It sounds like BLK shouldn't be default y if DM_MMC but rather selected
by DM_MMC.  Yes?
Fabio Estevam Oct. 11, 2017, 9:53 p.m. UTC | #2 
On Wed, Oct 11, 2017 at 6:49 PM, Tom Rini <trini@konsulko.com> wrote:

> It sounds like BLK shouldn't be default y if DM_MMC but rather selected
> by DM_MMC.  Yes?

What about this?

--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -13,6 +13,7 @@ config MMC
 config DM_MMC
        bool "Enable MMC controllers using Driver Model"
        depends on DM
+       select BLK
        help
          This enables the MultiMediaCard (MMC) uclass which supports MMC and
          Secure Digital I/O (SDIO) cards. Both removable (SD, micro-SD, etc.)
Tom Rini Oct. 11, 2017, 9:55 p.m. UTC | #3 
On Wed, Oct 11, 2017 at 06:53:13PM -0300, Fabio Estevam wrote:
> On Wed, Oct 11, 2017 at 6:49 PM, Tom Rini <trini@konsulko.com> wrote:
> 
> > It sounds like BLK shouldn't be default y if DM_MMC but rather selected
> > by DM_MMC.  Yes?
> 
> What about this?
> 
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -13,6 +13,7 @@ config MMC
>  config DM_MMC
>         bool "Enable MMC controllers using Driver Model"
>         depends on DM
> +       select BLK
>         help
>           This enables the MultiMediaCard (MMC) uclass which supports MMC and
>           Secure Digital I/O (SDIO) cards. Both removable (SD, micro-SD, etc.)

Yes, I think that's it, along with removing the default y if DM_MMC from
the BLK entry.  Thanks!
Fabio Estevam Oct. 11, 2017, 10:07 p.m. UTC | #4 
On Wed, Oct 11, 2017 at 6:55 PM, Tom Rini <trini@konsulko.com> wrote:

> Yes, I think that's it, along with removing the default y if DM_MMC from
> the BLK entry.  Thanks!

Ok, if I do as suggested:

--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -1,7 +1,6 @@
 config BLK
        bool "Support block devices"
        depends on DM
-       default y if DM_MMC
        help
          Enable support for block devices, such as SCSI, MMC and USB
          flash sticks. These provide a block-level interface which permits
diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index 94050836..f4c953c 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -13,6 +13,7 @@ config MMC
 config DM_MMC
        bool "Enable MMC controllers using Driver Model"
        depends on DM
+       select BLK
        help
          This enables the MultiMediaCard (MMC) uclass which supports MMC and
          Secure Digital I/O (SDIO) cards. Both removable (SD, micro-SD, etc.)

Then mx7dsabresd_defconfig build and boots fine.

However, mx6slevk_defconfig fails to build like this:

  CC      common/usb_storage.o
common/usb_storage.c: In function ‘usb_stor_probe_device’:
common/usb_storage.c:208:30: error: ‘struct usb_device’ has no member
named ‘dev’
  data = dev_get_platdata(udev->dev);
                              ^
common/usb_storage.c:218:32: error: ‘struct usb_device’ has no member
named ‘dev’
   ret = blk_create_devicef(udev->dev, "usb_storage_blk", str,
                                ^
I can fix it by doing:

--- a/configs/mx6slevk_defconfig
+++ b/configs/mx6slevk_defconfig
@@ -44,6 +44,7 @@ CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_DM_THERMAL=y
 CONFIG_USB=y
+CONFIG_DM_USB=y
 CONFIG_USB_STORAGE=y
 CONFIG_USB_HOST_ETHER=y
 CONFIG_USB_ETHER_ASIX=y

Is there a better fix for this breakage?
Tom Rini Oct. 11, 2017, 10:17 p.m. UTC | #5 
On Wed, Oct 11, 2017 at 07:07:04PM -0300, Fabio Estevam wrote:
> On Wed, Oct 11, 2017 at 6:55 PM, Tom Rini <trini@konsulko.com> wrote:
> 
> > Yes, I think that's it, along with removing the default y if DM_MMC from
> > the BLK entry.  Thanks!
> 
> Ok, if I do as suggested:
> 
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -1,7 +1,6 @@
>  config BLK
>         bool "Support block devices"
>         depends on DM
> -       default y if DM_MMC
>         help
>           Enable support for block devices, such as SCSI, MMC and USB
>           flash sticks. These provide a block-level interface which permits
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 94050836..f4c953c 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -13,6 +13,7 @@ config MMC
>  config DM_MMC
>         bool "Enable MMC controllers using Driver Model"
>         depends on DM
> +       select BLK
>         help
>           This enables the MultiMediaCard (MMC) uclass which supports MMC and
>           Secure Digital I/O (SDIO) cards. Both removable (SD, micro-SD, etc.)
> 
> Then mx7dsabresd_defconfig build and boots fine.
> 
> However, mx6slevk_defconfig fails to build like this:
> 
>   CC      common/usb_storage.o
> common/usb_storage.c: In function ‘usb_stor_probe_device’:
> common/usb_storage.c:208:30: error: ‘struct usb_device’ has no member
> named ‘dev’
>   data = dev_get_platdata(udev->dev);
>                               ^
> common/usb_storage.c:218:32: error: ‘struct usb_device’ has no member
> named ‘dev’
>    ret = blk_create_devicef(udev->dev, "usb_storage_blk", str,
>                                 ^
> I can fix it by doing:
> 
> --- a/configs/mx6slevk_defconfig
> +++ b/configs/mx6slevk_defconfig
> @@ -44,6 +44,7 @@ CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
>  CONFIG_DM_THERMAL=y
>  CONFIG_USB=y
> +CONFIG_DM_USB=y
>  CONFIG_USB_STORAGE=y
>  CONFIG_USB_HOST_ETHER=y
>  CONFIG_USB_ETHER_ASIX=y
> 
> Is there a better fix for this breakage?

Hmmm.  So, if you have DM and DM_MMC, you need BLK.  If you have DM and
BLK and USB, you also need DM_USB (and vice-versa, if you have DM and DM_USB and
not DM_MMC, things will break too, on another platform I bet).  So,
DM_USB should also select BLK I think.  The case like mx6slkevk where
you have DM_USB supported by not enabled are a bug of sorts where it
needs to also be enabling DM_USB.  I think I'll need to do a world build
to see what fails in this case, unless you want to do it?  Thanks!
Fabio Estevam Oct. 11, 2017, 10:28 p.m. UTC | #6 
On Wed, Oct 11, 2017 at 7:17 PM, Tom Rini <trini@konsulko.com> wrote:

> Hmmm.  So, if you have DM and DM_MMC, you need BLK.  If you have DM and
> BLK and USB, you also need DM_USB (and vice-versa, if you have DM and DM_USB and
> not DM_MMC, things will break too, on another platform I bet).  So,
> DM_USB should also select BLK I think.  The case like mx6slkevk where
> you have DM_USB supported by not enabled are a bug of sorts where it
> needs to also be enabling DM_USB.  I think I'll need to do a world build
> to see what fails in this case, unless you want to do it?  Thanks!

Please go ahead with the build. I am about to leave for holidays.

Thanks
Tom Rini Oct. 12, 2017, 4:33 p.m. UTC | #7 
On Wed, Oct 11, 2017 at 06:17:12PM -0400, Tom Rini wrote:
> On Wed, Oct 11, 2017 at 07:07:04PM -0300, Fabio Estevam wrote:
> > On Wed, Oct 11, 2017 at 6:55 PM, Tom Rini <trini@konsulko.com> wrote:
> > 
> > > Yes, I think that's it, along with removing the default y if DM_MMC from
> > > the BLK entry.  Thanks!
> > 
> > Ok, if I do as suggested:
> > 
> > --- a/drivers/block/Kconfig
> > +++ b/drivers/block/Kconfig
> > @@ -1,7 +1,6 @@
> >  config BLK
> >         bool "Support block devices"
> >         depends on DM
> > -       default y if DM_MMC
> >         help
> >           Enable support for block devices, such as SCSI, MMC and USB
> >           flash sticks. These provide a block-level interface which permits
> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> > index 94050836..f4c953c 100644
> > --- a/drivers/mmc/Kconfig
> > +++ b/drivers/mmc/Kconfig
> > @@ -13,6 +13,7 @@ config MMC
> >  config DM_MMC
> >         bool "Enable MMC controllers using Driver Model"
> >         depends on DM
> > +       select BLK
> >         help
> >           This enables the MultiMediaCard (MMC) uclass which supports MMC and
> >           Secure Digital I/O (SDIO) cards. Both removable (SD, micro-SD, etc.)
> > 
> > Then mx7dsabresd_defconfig build and boots fine.
> > 
> > However, mx6slevk_defconfig fails to build like this:
> > 
> >   CC      common/usb_storage.o
> > common/usb_storage.c: In function ‘usb_stor_probe_device’:
> > common/usb_storage.c:208:30: error: ‘struct usb_device’ has no member
> > named ‘dev’
> >   data = dev_get_platdata(udev->dev);
> >                               ^
> > common/usb_storage.c:218:32: error: ‘struct usb_device’ has no member
> > named ‘dev’
> >    ret = blk_create_devicef(udev->dev, "usb_storage_blk", str,
> >                                 ^
> > I can fix it by doing:
> > 
> > --- a/configs/mx6slevk_defconfig
> > +++ b/configs/mx6slevk_defconfig
> > @@ -44,6 +44,7 @@ CONFIG_DM_REGULATOR_FIXED=y
> >  CONFIG_DM_REGULATOR_GPIO=y
> >  CONFIG_DM_THERMAL=y
> >  CONFIG_USB=y
> > +CONFIG_DM_USB=y
> >  CONFIG_USB_STORAGE=y
> >  CONFIG_USB_HOST_ETHER=y
> >  CONFIG_USB_ETHER_ASIX=y
> > 
> > Is there a better fix for this breakage?
> 
> Hmmm.  So, if you have DM and DM_MMC, you need BLK.  If you have DM and
> BLK and USB, you also need DM_USB (and vice-versa, if you have DM and DM_USB and
> not DM_MMC, things will break too, on another platform I bet).  So,
> DM_USB should also select BLK I think.  The case like mx6slkevk where
> you have DM_USB supported by not enabled are a bug of sorts where it
> needs to also be enabling DM_USB.  I think I'll need to do a world build
> to see what fails in this case, unless you want to do it?  Thanks!

OK, disregard what I had been saying.  At this point, it's a matter of
correcting and testing boards to have either DM_MMC (and DM_USB and BLK)
on, or having DM_MMC off (because they want USB gadget support).
Fabio Estevam Oct. 12, 2017, 6:35 p.m. UTC | #8 
Tom,

On Thu, Oct 12, 2017 at 1:33 PM, Tom Rini <trini@konsulko.com> wrote:

> OK, disregard what I had been saying.  At this point, it's a matter of
> correcting and testing boards to have either DM_MMC (and DM_USB and BLK)
> on, or having DM_MMC off (because they want USB gadget support).

Here is what I tried:
https://pastebin.com/yYEdC5fA

but then am335x_hs_evm_defconfig fails like this:

  LD      spl/drivers/built-in.o
  LD      spl/u-boot-spl
/usr/bin/arm-linux-gnueabi-ld.bfd: u-boot-spl section `.rodata' will
not fit in region `.sram'
/usr/bin/arm-linux-gnueabi-ld.bfd: region `.sram' overflowed by 7212 bytes
scripts/Makefile.spl:358: recipe for target 'spl/u-boot-spl' failed
make[1]: *** [spl/u-boot-spl] Error 1
Makefile:1407: recipe for target 'spl/u-boot-spl' failed
make: *** [spl/u-boot-spl] Error

Suggestions?
Tom Rini Oct. 12, 2017, 6:38 p.m. UTC | #9 
On Thu, Oct 12, 2017 at 03:35:38PM -0300, Fabio Estevam wrote:
> Tom,
> 
> On Thu, Oct 12, 2017 at 1:33 PM, Tom Rini <trini@konsulko.com> wrote:
> 
> > OK, disregard what I had been saying.  At this point, it's a matter of
> > correcting and testing boards to have either DM_MMC (and DM_USB and BLK)
> > on, or having DM_MMC off (because they want USB gadget support).
> 
> Here is what I tried:
> https://pastebin.com/yYEdC5fA
> 
> but then am335x_hs_evm_defconfig fails like this:
> 
>   LD      spl/drivers/built-in.o
>   LD      spl/u-boot-spl
> /usr/bin/arm-linux-gnueabi-ld.bfd: u-boot-spl section `.rodata' will
> not fit in region `.sram'
> /usr/bin/arm-linux-gnueabi-ld.bfd: region `.sram' overflowed by 7212 bytes
> scripts/Makefile.spl:358: recipe for target 'spl/u-boot-spl' failed
> make[1]: *** [spl/u-boot-spl] Error 1
> Makefile:1407: recipe for target 'spl/u-boot-spl' failed
> make: *** [spl/u-boot-spl] Error
> 
> Suggestions?

Yes, just work on the imx* targets.  For am33xx/am43xx/am57xx we don't
want to swap things around as we then get rid of USB gadget support.
It's a bit tricky atm to get all cases covered as, iirc, there's
something outstanding for the USB gadget case for DM.
Fabio Estevam Oct. 12, 2017, 6:39 p.m. UTC | #10 
On Thu, Oct 12, 2017 at 3:38 PM, Tom Rini <trini@konsulko.com> wrote:

> Yes, just work on the imx* targets.  For am33xx/am43xx/am57xx we don't
> want to swap things around as we then get rid of USB gadget support.
> It's a bit tricky atm to get all cases covered as, iirc, there's
> something outstanding for the USB gadget case for DM.

Ok, so in this case, should I fix this by touching only the imx6 defconfigs?
Tom Rini Oct. 12, 2017, 6:49 p.m. UTC | #11 
On Thu, Oct 12, 2017 at 03:39:56PM -0300, Fabio Estevam wrote:
> On Thu, Oct 12, 2017 at 3:38 PM, Tom Rini <trini@konsulko.com> wrote:
> 
> > Yes, just work on the imx* targets.  For am33xx/am43xx/am57xx we don't
> > want to swap things around as we then get rid of USB gadget support.
> > It's a bit tricky atm to get all cases covered as, iirc, there's
> > something outstanding for the USB gadget case for DM.
> 
> Ok, so in this case, should I fix this by touching only the imx6 defconfigs?

Yes, thanks.
diff mbox series

Patch

diff --git a/configs/mx7dsabresd_defconfig b/configs/mx7dsabresd_defconfig
index 795c4f2..144fb50 100644
--- a/configs/mx7dsabresd_defconfig
+++ b/configs/mx7dsabresd_defconfig
@@ -38,7 +38,6 @@  CONFIG_CMD_EXT4=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_FAT=y
 CONFIG_OF_CONTROL=y
-# CONFIG_BLK is not set
 CONFIG_DFU_MMC=y
 CONFIG_DFU_RAM=y
 CONFIG_DM_GPIO=y
diff --git a/configs/mx7dsabresd_secure_defconfig b/configs/mx7dsabresd_secure_defconfig
index bd68831..d1af138 100644
--- a/configs/mx7dsabresd_secure_defconfig
+++ b/configs/mx7dsabresd_secure_defconfig
@@ -40,7 +40,6 @@  CONFIG_CMD_EXT4=y
 CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_FAT=y
 CONFIG_OF_CONTROL=y
-# CONFIG_BLK is not set
 CONFIG_DFU_MMC=y
 CONFIG_DFU_RAM=y
 CONFIG_DM_GPIO=y