| Message ID | 1513061911-30076-13-git-send-email-jagan@amarulasolutions.com |
|---|---|
| State | Deferred |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | sun50i: a64: add support for axp803, musb | expand |
Hi, On Tue, Dec 12, 2017 at 12:28:27PM +0530, Jagan Teki wrote: > This patch enable ums through CMD_USB_MASS_STORAGE. > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > --- > configs/bananapi_m64_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configs/bananapi_m64_defconfig b/configs/bananapi_m64_defconfig > index 55feafe..d4aade5 100644 > --- a/configs/bananapi_m64_defconfig > +++ b/configs/bananapi_m64_defconfig > @@ -11,6 +11,7 @@ CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-bananapi-m64" > CONFIG_SPL=y > # CONFIG_CMD_FLASH is not set > # CONFIG_CMD_FPGA is not set > +CONFIG_CMD_USB_MASS_STORAGE=y How does that work with the current over-size issue we have on the A64? And I'd also like to keep the way we did things for several years now, which is to *not* have board-specific options selected besides the hardware-related ones. If you want to enable a general feature, do it for all the boards so that our users will have a consistent experience across boards, and we will not have to always chase all the defconfigs to provide it. And I guess the next question would be: what is the targetted use case and why should we enable it for all the boards? We're having binary size issues on the A64, so we really want to provide something meaningful for the majority of our users. We won't enable something used by only a small fraction, especially when it's so easy to enable it. Maxime
On Tue, Dec 12, 2017 at 1:42 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Tue, Dec 12, 2017 at 12:28:27PM +0530, Jagan Teki wrote: >> This patch enable ums through CMD_USB_MASS_STORAGE. >> >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >> --- >> configs/bananapi_m64_defconfig | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/configs/bananapi_m64_defconfig b/configs/bananapi_m64_defconfig >> index 55feafe..d4aade5 100644 >> --- a/configs/bananapi_m64_defconfig >> +++ b/configs/bananapi_m64_defconfig >> @@ -11,6 +11,7 @@ CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-bananapi-m64" >> CONFIG_SPL=y >> # CONFIG_CMD_FLASH is not set >> # CONFIG_CMD_FPGA is not set >> +CONFIG_CMD_USB_MASS_STORAGE=y > > How does that work with the current over-size issue we have on the > A64? > > And I'd also like to keep the way we did things for several years now, > which is to *not* have board-specific options selected besides the > hardware-related ones. > > If you want to enable a general feature, do it for all the boards so > that our users will have a consistent experience across boards, and we > will not have to always chase all the defconfigs to provide it. I always believe test-proven even it is general feature there may be changes it may(not) work on all boards. Once the relevant boards have needed it and then it will anyway added. > > And I guess the next question would be: what is the targetted use case > and why should we enable it for all the boards? We're having binary > size issues on the A64, so we really want to provide something > meaningful for the majority of our users. > > We won't enable something used by only a small fraction, especially > when it's so easy to enable it. Yes, this was tested on this board and it is working. the use-case I've seen to upgrade the target partitioned-disk directly from host instead of plug-and-play every time and especially with eMMC. thanks!
在 2017年12月12日星期二 CST 下午4:12:13，Maxime Ripard 写道： > Hi, > > On Tue, Dec 12, 2017 at 12:28:27PM +0530, Jagan Teki wrote: > > This patch enable ums through CMD_USB_MASS_STORAGE. > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > --- > > > > configs/bananapi_m64_defconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/configs/bananapi_m64_defconfig > > b/configs/bananapi_m64_defconfig index 55feafe..d4aade5 100644 > > --- a/configs/bananapi_m64_defconfig > > +++ b/configs/bananapi_m64_defconfig > > @@ -11,6 +11,7 @@ CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-bananapi-m64" > > > > CONFIG_SPL=y > > # CONFIG_CMD_FLASH is not set > > # CONFIG_CMD_FPGA is not set > > > > +CONFIG_CMD_USB_MASS_STORAGE=y > > How does that work with the current over-size issue we have on the > A64? > > And I'd also like to keep the way we did things for several years now, > which is to *not* have board-specific options selected besides the > hardware-related ones. > > If you want to enable a general feature, do it for all the boards so > that our users will have a consistent experience across boards, and we > will not have to always chase all the defconfigs to provide it. I think there's a problem on A64 -- the Pine series are all designed to be host-only, and it's the most popular A64 board series. > > And I guess the next question would be: what is the targetted use case > and why should we enable it for all the boards? We're having binary > size issues on the A64, so we really want to provide something > meaningful for the majority of our users. > > We won't enable something used by only a small fraction, especially > when it's so easy to enable it. > > Maxime
On Wednesday, December 13, 2017 2:36:26 AM CET Icenowy Zheng wrote: > 在 2017年12月12日星期二 CST 下午4:12:13，Maxime Ripard 写道： > > > Hi, > > > > On Tue, Dec 12, 2017 at 12:28:27PM +0530, Jagan Teki wrote: > > > This patch enable ums through CMD_USB_MASS_STORAGE. > > > > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > > > --- > > > > > > configs/bananapi_m64_defconfig | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/configs/bananapi_m64_defconfig > > > b/configs/bananapi_m64_defconfig index 55feafe..d4aade5 100644 > > > --- a/configs/bananapi_m64_defconfig > > > +++ b/configs/bananapi_m64_defconfig > > > @@ -11,6 +11,7 @@ CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-bananapi-m64" > > > > > > CONFIG_SPL=y > > > # CONFIG_CMD_FLASH is not set > > > # CONFIG_CMD_FPGA is not set > > > > > > +CONFIG_CMD_USB_MASS_STORAGE=y > > > > How does that work with the current over-size issue we have on the > > A64? > > > > And I'd also like to keep the way we did things for several years now, > > which is to *not* have board-specific options selected besides the > > hardware-related ones. > > > > If you want to enable a general feature, do it for all the boards so > > that our users will have a consistent experience across boards, and we > > will not have to always chase all the defconfigs to provide it. > > I think there's a problem on A64 -- the Pine series are all designed to be > host-only, and it's the most popular A64 board series. No, it just means the *initial* role of the Pine will be Host, but the roles can be swapped using HNP (Host Negotiation Protocol) afterwards, which is pure software. Regards, Stefan
On Wed, Dec 13, 2017 at 7:31 AM, Stefan Brüns <stefan.bruens@rwth-aachen.de> wrote: > On Wednesday, December 13, 2017 2:36:26 AM CET Icenowy Zheng wrote: >> 在 2017年12月12日星期二 CST 下午4:12:13，Maxime Ripard 写道： >> >> > Hi, >> > >> > On Tue, Dec 12, 2017 at 12:28:27PM +0530, Jagan Teki wrote: >> > > This patch enable ums through CMD_USB_MASS_STORAGE. >> > > >> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> >> > > --- >> > > >> > > configs/bananapi_m64_defconfig | 1 + >> > > 1 file changed, 1 insertion(+) >> > > >> > > diff --git a/configs/bananapi_m64_defconfig >> > > b/configs/bananapi_m64_defconfig index 55feafe..d4aade5 100644 >> > > --- a/configs/bananapi_m64_defconfig >> > > +++ b/configs/bananapi_m64_defconfig >> > > @@ -11,6 +11,7 @@ CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-bananapi-m64" >> > > >> > > CONFIG_SPL=y >> > > # CONFIG_CMD_FLASH is not set >> > > # CONFIG_CMD_FPGA is not set >> > > >> > > +CONFIG_CMD_USB_MASS_STORAGE=y >> > >> > How does that work with the current over-size issue we have on the >> > A64? >> > >> > And I'd also like to keep the way we did things for several years now, >> > which is to *not* have board-specific options selected besides the >> > hardware-related ones. >> > >> > If you want to enable a general feature, do it for all the boards so >> > that our users will have a consistent experience across boards, and we >> > will not have to always chase all the defconfigs to provide it. >> >> I think there's a problem on A64 -- the Pine series are all designed to be >> host-only, and it's the most popular A64 board series. > > No, it just means the *initial* role of the Pine will be Host, but the roles > can be swapped using HNP (Host Negotiation Protocol) afterwards, which is pure > software. I don't think we can configure otg in peripheral mode, atleast I couldn't find any info in schematics and it's not working at software level(based on my recent test) thanks!
Hi, On Tue, Dec 12, 2017 at 02:39:59PM +0530, Jagan Teki wrote: > On Tue, Dec 12, 2017 at 1:42 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > Hi, > > > > On Tue, Dec 12, 2017 at 12:28:27PM +0530, Jagan Teki wrote: > >> This patch enable ums through CMD_USB_MASS_STORAGE. > >> > >> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> > >> --- > >> configs/bananapi_m64_defconfig | 1 + > >> 1 file changed, 1 insertion(+) > >> > >> diff --git a/configs/bananapi_m64_defconfig b/configs/bananapi_m64_defconfig > >> index 55feafe..d4aade5 100644 > >> --- a/configs/bananapi_m64_defconfig > >> +++ b/configs/bananapi_m64_defconfig > >> @@ -11,6 +11,7 @@ CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-bananapi-m64" > >> CONFIG_SPL=y > >> # CONFIG_CMD_FLASH is not set > >> # CONFIG_CMD_FPGA is not set > >> +CONFIG_CMD_USB_MASS_STORAGE=y > > > > How does that work with the current over-size issue we have on the > > A64? > > > > And I'd also like to keep the way we did things for several years now, > > which is to *not* have board-specific options selected besides the > > hardware-related ones. > > > > If you want to enable a general feature, do it for all the boards so > > that our users will have a consistent experience across boards, and we > > will not have to always chase all the defconfigs to provide it. > > I always believe test-proven even it is general feature there may be > changes it may(not) work on all boards. Once the relevant boards have > needed it and then it will anyway added. And this leads to inconsistencies across boards in the features they support, which in turn lead to downstream users being unable to rely on one particular feature being enabled. And really, this is what we've doing all along. > > And I guess the next question would be: what is the targetted use case > > and why should we enable it for all the boards? We're having binary > > size issues on the A64, so we really want to provide something > > meaningful for the majority of our users. > > > > We won't enable something used by only a small fraction, especially > > when it's so easy to enable it. > > Yes, this was tested on this board and it is working. the use-case > I've seen to upgrade the target partitioned-disk directly from host > instead of plug-and-play every time and especially with eMMC. We already have DFU and fastboot to cover that. Do we really need to have a third option when we already have size issues in our binary? Really, if someone uses it, then yeah, we should make it as convenient as possible to enable it. But since it's just one option to check, what's the point? That's also why we have Kconfig after all. Maxime
diff --git a/configs/bananapi_m64_defconfig b/configs/bananapi_m64_defconfig index 55feafe..d4aade5 100644 --- a/configs/bananapi_m64_defconfig +++ b/configs/bananapi_m64_defconfig @@ -11,6 +11,7 @@ CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-bananapi-m64" CONFIG_SPL=y # CONFIG_CMD_FLASH is not set # CONFIG_CMD_FPGA is not set +CONFIG_CMD_USB_MASS_STORAGE=y # CONFIG_SPL_DOS_PARTITION is not set # CONFIG_SPL_ISO_PARTITION is not set # CONFIG_SPL_EFI_PARTITION is not set
This patch enable ums through CMD_USB_MASS_STORAGE. Signed-off-by: Jagan Teki <jagan@amarulasolutions.com> --- configs/bananapi_m64_defconfig | 1 + 1 file changed, 1 insertion(+)