Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Jocelyn Bohr]

Great, I will check out v2! And yes free to add a "Signed-off-by" for me on
any of the original patchsets.

On Mon, Apr 30, 2018 at 1:37 AM Alex Kiernan  wrote:

> On Fri, Apr 27, 2018 at 1:20 PM, Alex Kiernan 
> wrote:
> > On Fri, Apr 27, 2018 at 9:04 AM, Lukasz Majewski  wrote:
> >> Hi Kever,
> >>
> >>> Hi Jocelyn and Alex,
> >>>
> >>>
> >>> It's great to see you are getting this feature from google code
> >>> to be mainline code.
> >>>
> >>> My opinion is that we should have the same fastboot cmd handling
> >>> in both UDP and USB in a common file, but not have separate code in
> >>> net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to
> >>> merge these two in one?
> >>
> >> I've already proposed to put common code (the protocol handling)
> >> into /drivers/fastboot and have only small glue code for USB and NET.
> >>
> >
> > I'm working towards that. I'm starting with the command
> > implementations, then I'll look at the protocol.
> >
> > Hopefully I'll get an RFCv2 out sometime over the weekend with the
> > first bits done.
> >
>
> Just posted a v2, still needs more work, but getting feedback on the
> direction would be useful.
>
> --
> Alex Kiernan
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Kiernan]

On Fri, Apr 27, 2018 at 1:20 PM, Alex Kiernan  wrote:
> On Fri, Apr 27, 2018 at 9:04 AM, Lukasz Majewski  wrote:
>> Hi Kever,
>>
>>> Hi Jocelyn and Alex,
>>>
>>>
>>> It's great to see you are getting this feature from google code
>>> to be mainline code.
>>>
>>> My opinion is that we should have the same fastboot cmd handling
>>> in both UDP and USB in a common file, but not have separate code in
>>> net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to
>>> merge these two in one?
>>
>> I've already proposed to put common code (the protocol handling)
>> into /drivers/fastboot and have only small glue code for USB and NET.
>>
>
> I'm working towards that. I'm starting with the command
> implementations, then I'll look at the protocol.
>
> Hopefully I'll get an RFCv2 out sometime over the weekend with the
> first bits done.
>

Just posted a v2, still needs more work, but getting feedback on the
direction would be useful.

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Kiernan]

Hi Sam,

On Fri, Apr 27, 2018 at 8:10 PM, Sam Protsenko
 wrote:
> On 24 April 2018 at 12:37, Alex Kiernan  wrote:
>>
>> This series merges the fastboot UDP support from AOSP into mainline
>> U-Boot.
>>
>
> Can you please point me out from which AOSP code exactly this code was
> ported? Links to code, maybe some docs. Thanks.
>

Sure:

https://android.googlesource.com/platform/external/u-boot/+/android-o-mr1-iot-preview-8

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Sam Protsenko]

On 24 April 2018 at 12:37, Alex Kiernan  wrote:
>
> This series merges the fastboot UDP support from AOSP into mainline
> U-Boot.
>

Can you please point me out from which AOSP code exactly this code was
ported? Links to code, maybe some docs. Thanks.

> Open questions:
>
> - what's the correct way of attributing the original authors? I've added
>   Co-authored-by, is that right? checkpatch doesn't seem to know any
>   of the co- tags
> - currently there's no NAND support and I've no way of testing that, so
>   my inclination is towards leaving it like that until someone with that
>   particular itch to scratch can look at it
> - you can select both USB and UDP fastboot, but the comments in the
>   AOSP code suggest that needs fixing - again, I've no board I can test
>   USB fastboot on
> - the USB and UDP code want consolidating, with this series there would
>   then be two separate implementations of the same protocol
> - the syntax for the USB fastboot command changes from `fastboot `
>   to `fastboot usb `, that feels unacceptable and we probably
>   want something like `fastboot ` or `fastboot udp`?
> - unrelated to this series, but a show-stopper for me, there's no FIT image
>   support, but that doesn't feel unsolveable - something like add an option
>   to pass in the load address and/or use loadaddr, then something else to
>   let you override the bootm invocation on the server side
>
> I've not tested all the code paths yet, but the obvious stuff works
> (basic interaction, download, boot) - every interaction elicits a
> `WARNING: unknown variable: slot-count` on the console; I'm guessing that
> my local end is sending a getvar for that, but I've not investigated.
>
> Without any way of testing any of the USB/NAND code I'm nervous of wading
> into that kind of refactoring, would that be a pre-requisite for merging?
>
>
> Alex Kiernan (5):
>   dfu: Refactor fastboot_okay/fail to take response
>   dfu: Extract fastboot_okay/fail to fb_common.c
>   net: dfu: Merge AOSP UDP fastboot
>   dfu: Resolve Kconfig dependency loops
>   net: dfu: Support building without MMC
>
>  cmd/fastboot.c  |  32 ++-
>  cmd/fastboot/Kconfig|  21 +-
>  cmd/net.c   |   6 +
>  common/Makefile |   4 +
>  common/fb_common.c  |  44 
>  common/fb_mmc.c | 114 ++---
>  common/fb_nand.c|  31 +--
>  common/image-sparse.c   |  41 ++-
>  drivers/usb/gadget/f_fastboot.c |  36 +--
>  include/fastboot.h  |  17 +-
>  include/fb_mmc.h|   4 +-
>  include/fb_nand.h   |   4 +-
>  include/image-sparse.h  |   2 +-
>  include/net.h   |   6 +-
>  include/net/fastboot.h  |  27 ++
>  net/Makefile|   1 +
>  net/fastboot.c  | 548 
> 
>  net/net.c   |   9 +
>  18 files changed, 824 insertions(+), 123 deletions(-)
>  create mode 100644 common/fb_common.c
>  create mode 100644 include/net/fastboot.h
>  create mode 100644 net/fastboot.c
>
> --
> 2.7.4
>
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Kiernan]

On Fri, Apr 27, 2018 at 9:04 AM, Lukasz Majewski  wrote:
> Hi Kever,
>
>> Hi Jocelyn and Alex,
>>
>>
>> It's great to see you are getting this feature from google code
>> to be mainline code.
>>
>> My opinion is that we should have the same fastboot cmd handling
>> in both UDP and USB in a common file, but not have separate code in
>> net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to
>> merge these two in one?
>
> I've already proposed to put common code (the protocol handling)
> into /drivers/fastboot and have only small glue code for USB and NET.
>

I'm working towards that. I'm starting with the command
implementations, then I'll look at the protocol.

Hopefully I'll get an RFCv2 out sometime over the weekend with the
first bits done.

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Lukasz Majewski]

Hi Kever,

> Hi Jocelyn and Alex,
> 
> 
>     It's great to see you are getting this feature from google code
> to be mainline code.
> 
>     My opinion is that we should have the same fastboot cmd handling
> in both UDP and USB in a common file, but not have separate code in
> net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to
> merge these two in one?

I've already proposed to put common code (the protocol handling)
into /drivers/fastboot and have only small glue code for USB and NET.

> 
> Thanks,
> - Kever
> 
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgp_eBRiCUAPs.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Kever Yang]

Hi Jocelyn and Alex,


    It's great to see you are getting this feature from google code to be
mainline code.

    My opinion is that we should have the same fastboot cmd handling
in both UDP and USB in a common file, but not have separate code in
net/fastboot.c and drivers/usb/gadget/f_fastboot.c, could you try to merge
these two in one?

Thanks,
- Kever

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Lukasz Majewski]

On Wed, 25 Apr 2018 13:50:50 +0100
Alex Kiernan  wrote:

> On Wed, Apr 25, 2018 at 8:53 AM, Alex Kiernan
>  wrote:
> > On Tue, Apr 24, 2018 at 6:10 PM, Jocelyn Bohr 
> > wrote:  
> >> Thanks so much for porting this, Alex!
> >>  
> 
> ...
> 
> >>
> >> You can select both network and USB fastboot to be enabled with
> >> the Kconfig, but at runtime you have to select to wait on either
> >> USB or network by running
> >> "fastboot udp" or "fastboot usb ". When the Android
> >> bootloader
> >> gets the command to reboot back to fastboot, it will read the
> >> "fastbootcmd" env
> >> variable and run that as a command
> >> (common/android_bootloader.c:151). 
> >
> > Thanks for that (especially the detail on android_bootloader which
> > I'd not read through). The bit that concerns me is in
> > timed_send_info:
> >
> >   +/**
> >   + * Send an INFO packet during long commands based on timer. If
> >   + * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is
> > sent
> >   + * if the time is 30 seconds after start. Else, noop.
> >   + *
> >   + * TODO: Handle the situation where both UDP and USB fastboot are
> >   + *   enabled.
> >   + *
> >   + * @param start:  Time since last INFO packet was sent.
> >   + * @param msg:String describing the reason for waiting
> >   + */
> >   +void timed_send_info(ulong *start, const char *msg);
> >
> > The code in timed_send_info is guarded with an ifdef, rather than
> > based on the transport that's been selected at runtime. Do we need
> > to make timed_send_info work based on the runtime state, rather
> > than the compile time, or can we drop the ifdef guard and remove
> > the TODO? I guess I'm assuming the former, but with no way to test
> > USB I don't want head off down the wrong road!
> >  
> 
> Having started digging through how we'd merge the two code paths, it's
> clear if you had UDP and USB both enabled at compile time and then try
> and run the USB path it'll try and do UDP things in timed_send_info,
> which can't be good!
> 

I think that we can assume operation of only one medium in a time - i.e.

when you issue fastboot usb then no UDP support (and opposite).


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgprr61J8cEhD.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Kiernan]

On Wed, Apr 25, 2018 at 8:53 AM, Alex Kiernan  wrote:
> On Tue, Apr 24, 2018 at 6:10 PM, Jocelyn Bohr  wrote:
>> Thanks so much for porting this, Alex!
>>

...

>>
>> You can select both network and USB fastboot to be enabled with the Kconfig,
>> but at runtime you have to select to wait on either USB or network by
>> running
>> "fastboot udp" or "fastboot usb ". When the Android
>> bootloader
>> gets the command to reboot back to fastboot, it will read the "fastbootcmd"
>> env
>> variable and run that as a command (common/android_bootloader.c:151).
>>
>
> Thanks for that (especially the detail on android_bootloader which I'd
> not read through). The bit that concerns me is in timed_send_info:
>
>   +/**
>   + * Send an INFO packet during long commands based on timer. If
>   + * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is sent
>   + * if the time is 30 seconds after start. Else, noop.
>   + *
>   + * TODO: Handle the situation where both UDP and USB fastboot are
>   + *   enabled.
>   + *
>   + * @param start:  Time since last INFO packet was sent.
>   + * @param msg:String describing the reason for waiting
>   + */
>   +void timed_send_info(ulong *start, const char *msg);
>
> The code in timed_send_info is guarded with an ifdef, rather than
> based on the transport that's been selected at runtime. Do we need to
> make timed_send_info work based on the runtime state, rather than the
> compile time, or can we drop the ifdef guard and remove the TODO? I
> guess I'm assuming the former, but with no way to test USB I don't
> want head off down the wrong road!
>

Having started digging through how we'd merge the two code paths, it's
clear if you had UDP and USB both enabled at compile time and then try
and run the USB path it'll try and do UDP things in timed_send_info,
which can't be good!

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Deymo]

2018-04-25 10:43 GMT+02:00 Alex Kiernan :

> >> - what's the correct way of attributing the original authors? I've
> >> added Co-authored-by, is that right? checkpatch doesn't seem to know
> >> any of the co- tags
> >
> > I think that two authors (Alex and Jocelyn) have replied to your e-mail.
> >
> > Maybe it would be doable to have S-o-B or Acked-by from them?
> >
>
> Alex, Jocelyn, would you be happy for me to add a Signed-off-by from you?
>

Sure!
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Kiernan]

On Wed, Apr 25, 2018 at 8:52 AM, Lukasz Majewski  wrote:
> Hi Alex,
>
>> Open questions:
>>
>> - what's the correct way of attributing the original authors? I've
>> added Co-authored-by, is that right? checkpatch doesn't seem to know
>> any of the co- tags
>
> I think that two authors (Alex and Jocelyn) have replied to your e-mail.
>
> Maybe it would be doable to have S-o-B or Acked-by from them?
>

Alex, Jocelyn, would you be happy for me to add a Signed-off-by from you?

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Lukasz Majewski]

Hi Alex,

> > - the USB and UDP code want consolidating, with this series there
> > would then be two separate implementations of the same protocol  
> 
> Yes - this is an issue. 
> 
> For USB the fastboot protocol (at least some its logic) is implemented
> in drivers/usb/gadget/f_fastboot.c
> 
> I think that some medium agnostic code (like protocol itself) can be
> unified.
> 
> I also think that it would be a good idea to have ./drivers/fastboot
> directory introduced to move common code there - and leave only USB
> related code in f_fastboot.c

Especially that we do have fb_nand.c and fb_mmc.c in ./common.

In that way we could move them to ./drivers/fastboot.

> 
> Please as a reference, look on how ./drivers/dfu is organised.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpnm7RJYNm55.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Kiernan]

On Tue, Apr 24, 2018 at 6:10 PM, Jocelyn Bohr  wrote:
> Thanks so much for porting this, Alex!
>
> On Tue, Apr 24, 2018 at 4:58 AM Alex Kiernan  wrote:
>>
>> On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo  wrote:
>> > +Jocelyn.
>> >
>> > Thanks Alex for taking the time to port this!!
>>
>> It turned out to be a great opportunity to play with coccinelle on
>> something trivial, which I'd been meaning to do for ages... the
>> refactor to add response into the fastboot_okay/fail call chain was a
>> breeze with it.
>>
>> > 2018-04-24 11:37 GMT+02:00 Alex Kiernan :
>> >>
>> >>
>> >> This series merges the fastboot UDP support from AOSP into mainline
>> >> U-Boot.
>> >>
>> >> Open questions:
>> >>
>> >> - what's the correct way of attributing the original authors? I've
>> >> added
>> >>   Co-authored-by, is that right? checkpatch doesn't seem to know any
>> >>   of the co- tags
>> >> - currently there's no NAND support and I've no way of testing that, so
>> >>   my inclination is towards leaving it like that until someone with
>> >> that
>> >>   particular itch to scratch can look at it
>> >
>> >
>> > Fastboot uses partition names, like "system" and "boot" which have a
>> > meaning
>> > in the Android partition scheme. For GPT, we just use the partition
>> > names as
>> > the android names; but if you are booting from some other storage like
>> > NAND
>> > where you don't have that then you need some mapping glue ("system" -->
>> > device and partition number). I've seen U-Boot modifications for several
>> > devices where they just stick a table hard-coded in the U-Boot code;
>> > which
>> > works great for that device but it isn't really a generic approach.
>> > Other
>> > than handling the names issue, I don't see any problem with supporting
>> > NAND
>> > here, but a generic way to set global names / alias would be needed for
>> > this.
>> >
>>
>> With the fastboot NAND support in mainline it looks like we end up in
>> mtdparts to deliver that mapping through environment variables. No
>> actual idea what that looks like when you're driving it.
>>
>> >>
>> >> - you can select both USB and UDP fastboot, but the comments in the
>> >>   AOSP code suggest that needs fixing - again, I've no board I can test
>> >>   USB fastboot on
>> >
>> >
>> > I thought we checked in the Kconfig that you couldn't enable both. I
>> > don't
>> > remember the details now but yeah you can't wait for network or USB
>> > traffic
>> > on the current code.
>> >
>>
>> The version I picked from (o-mr1-iot-preview-8) has them as
>> independent symbols, but making them a choice would resolve the issue
>> for now.
>
>
> You can select both network and USB fastboot to be enabled with the Kconfig,
> but at runtime you have to select to wait on either USB or network by
> running
> "fastboot udp" or "fastboot usb ". When the Android
> bootloader
> gets the command to reboot back to fastboot, it will read the "fastbootcmd"
> env
> variable and run that as a command (common/android_bootloader.c:151).
>

Thanks for that (especially the detail on android_bootloader which I'd
not read through). The bit that concerns me is in timed_send_info:

  +/**
  + * Send an INFO packet during long commands based on timer. If
  + * CONFIG_UDP_FUNCTION_FASTBOOT is defined, an INFO packet is sent
  + * if the time is 30 seconds after start. Else, noop.
  + *
  + * TODO: Handle the situation where both UDP and USB fastboot are
  + *   enabled.
  + *
  + * @param start:  Time since last INFO packet was sent.
  + * @param msg:String describing the reason for waiting
  + */
  +void timed_send_info(ulong *start, const char *msg);

The code in timed_send_info is guarded with an ifdef, rather than
based on the transport that's been selected at runtime. Do we need to
make timed_send_info work based on the runtime state, rather than the
compile time, or can we drop the ifdef guard and remove the TODO? I
guess I'm assuming the former, but with no way to test USB I don't
want head off down the wrong road!

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Lukasz Majewski]

Hi Alex,

> Open questions:
> 
> - what's the correct way of attributing the original authors? I've
> added Co-authored-by, is that right? checkpatch doesn't seem to know
> any of the co- tags

I think that two authors (Alex and Jocelyn) have replied to your e-mail.

Maybe it would be doable to have S-o-B or Acked-by from them?

> - currently there's no NAND support and I've no way of testing that,
> so my inclination is towards leaving it like that until someone with
> that particular itch to scratch can look at it

As I've written in the other mail - please look into "mtdparts".

> - you can select both USB and UDP fastboot, but the comments in the
>   AOSP code suggest that needs fixing - again, I've no board I can
> test USB fastboot on

I need to check if I do posses a bort with fastboot support (as I
mostly test DFU).

> - the USB and UDP code want consolidating, with this series there
> would then be two separate implementations of the same protocol

Yes - this is an issue. 

For USB the fastboot protocol (at least some its logic) is implemented
in drivers/usb/gadget/f_fastboot.c

I think that some medium agnostic code (like protocol itself) can be
unified.

I also think that it would be a good idea to have ./drivers/fastboot
directory introduced to move common code there - and leave only USB
related code in f_fastboot.c

Please as a reference, look on how ./drivers/dfu is organised.

> - the syntax for the USB fastboot command changes from `fastboot
> ` to `fastboot usb `, that feels unacceptable
> and we probably want something like `fastboot ` or
> `fastboot udp`?

Similar issue was with "thor"/"ums"/"dfu" command previously.

For backward compatibility we can make an alias:

fastboot  -> fastboot usb 

IMHO, it would be great to have

fastboot usb 
fastboot udp ip (etc)

(It would be great to have the same syntax as it is in Android).

> - unrelated to this series, but a show-stopper for me, there's no FIT
> image support, but that doesn't feel unsolveable - something like add
> an option to pass in the load address and/or use loadaddr, then
> something else to let you override the bootm invocation on the server
> side




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpOZJY7XTxES.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Lukasz Majewski]

Hi Alex,

> On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo 
> wrote:
> > +Jocelyn.
> >
> > Thanks Alex for taking the time to port this!!  
> 
> It turned out to be a great opportunity to play with coccinelle on
> something trivial, which I'd been meaning to do for ages... the
> refactor to add response into the fastboot_okay/fail call chain was a
> breeze with it.
> 
> > 2018-04-24 11:37 GMT+02:00 Alex Kiernan :  
> >>
> >>
> >> This series merges the fastboot UDP support from AOSP into mainline
> >> U-Boot.
> >>
> >> Open questions:
> >>
> >> - what's the correct way of attributing the original authors? I've
> >> added Co-authored-by, is that right? checkpatch doesn't seem to
> >> know any of the co- tags
> >> - currently there's no NAND support and I've no way of testing
> >> that, so my inclination is towards leaving it like that until
> >> someone with that particular itch to scratch can look at it  
> >
> >
> > Fastboot uses partition names, like "system" and "boot" which have
> > a meaning in the Android partition scheme. For GPT, we just use the
> > partition names as the android names; but if you are booting from
> > some other storage like NAND where you don't have that then you
> > need some mapping glue ("system" --> device and partition number).
> > I've seen U-Boot modifications for several devices where they just
> > stick a table hard-coded in the U-Boot code; which works great for
> > that device but it isn't really a generic approach. Other than
> > handling the names issue, I don't see any problem with supporting
> > NAND here, but a generic way to set global names / alias would be
> > needed for this. 
> 
> With the fastboot NAND support in mainline it looks like we end up in
> mtdparts to deliver that mapping through environment variables. 

When you mentioned about NAND partitions - I also have thought about
mtdparts. They are well supported in mainline.

> No
> actual idea what that looks like when you're driving it.

Many boards use them - so there should not be any issue with finding
examples.

> 
> >>
> >> - you can select both USB and UDP fastboot, but the comments in the
> >>   AOSP code suggest that needs fixing - again, I've no board I can
> >> test USB fastboot on  
> >
> >
> > I thought we checked in the Kconfig that you couldn't enable both.
> > I don't remember the details now but yeah you can't wait for
> > network or USB traffic on the current code.
> >  
> 
> The version I picked from (o-mr1-iot-preview-8) has them as
> independent symbols, but making them a choice would resolve the issue
> for now.
> 
> >> I've not tested all the code paths yet, but the obvious stuff works
> >> (basic interaction, download, boot) - every interaction elicits a
> >> `WARNING: unknown variable: slot-count` on the console; I'm
> >> guessing that my local end is sending a getvar for that, but I've
> >> not investigated.  
> >
> >
> > Yes, there's a bit of different handling of partitions for A/B
> > android devices. Instead of "system" you have two partitions:
> > "system_a" and "system_b" (potentially more although 2 is the most
> > common case) so to know whether you have an old device or a newer
> > device the fastboot client checks the "slot-count" variable.
> > Undefined means of course that you have an old-style device, but if
> > you return something like "2" you would be able to flash "system"
> > on the "slot A" which is translated (again in the fastboot client)
> > to flash "system_a" partition. This is useful when using the higher
> > level operations like flashall/update and you want to specify to
> > flash only "slot A", "slot B" or even both of them. There are other
> > fastboot variables that require some plumbing, such as
> > "has-slot:" to tell whether "system" is a partition that
> > indeed has the two versions _a and _b. Typically some partitions
> > are twice (like "system" and "boot") and some other are not (like
> > "misc" or "userdata"). Anyway, this as is should work for either
> > old-style partition schemes or by force flashing "system_a" with
> > the system.img. 
> 
> Cool, thanks... that saves me a bunch of investigation!
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpW4xNbVQiMM.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Jocelyn Bohr]

Thanks so much for porting this, Alex!

On Tue, Apr 24, 2018 at 4:58 AM Alex Kiernan  wrote:

> On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo  wrote:
> > +Jocelyn.
> >
> > Thanks Alex for taking the time to port this!!
>
> It turned out to be a great opportunity to play with coccinelle on
> something trivial, which I'd been meaning to do for ages... the
> refactor to add response into the fastboot_okay/fail call chain was a
> breeze with it.
>
> > 2018-04-24 11:37 GMT+02:00 Alex Kiernan :
> >>
> >>
> >> This series merges the fastboot UDP support from AOSP into mainline
> >> U-Boot.
> >>
> >> Open questions:
> >>
> >> - what's the correct way of attributing the original authors? I've added
> >>   Co-authored-by, is that right? checkpatch doesn't seem to know any
> >>   of the co- tags
> >> - currently there's no NAND support and I've no way of testing that, so
> >>   my inclination is towards leaving it like that until someone with that
> >>   particular itch to scratch can look at it
> >
> >
> > Fastboot uses partition names, like "system" and "boot" which have a
> meaning
> > in the Android partition scheme. For GPT, we just use the partition
> names as
> > the android names; but if you are booting from some other storage like
> NAND
> > where you don't have that then you need some mapping glue ("system" -->
> > device and partition number). I've seen U-Boot modifications for several
> > devices where they just stick a table hard-coded in the U-Boot code;
> which
> > works great for that device but it isn't really a generic approach. Other
> > than handling the names issue, I don't see any problem with supporting
> NAND
> > here, but a generic way to set global names / alias would be needed for
> > this.
> >
>
> With the fastboot NAND support in mainline it looks like we end up in
> mtdparts to deliver that mapping through environment variables. No
> actual idea what that looks like when you're driving it.
>
> >>
> >> - you can select both USB and UDP fastboot, but the comments in the
> >>   AOSP code suggest that needs fixing - again, I've no board I can test
> >>   USB fastboot on
> >
> >
> > I thought we checked in the Kconfig that you couldn't enable both. I
> don't
> > remember the details now but yeah you can't wait for network or USB
> traffic
> > on the current code.
> >
>
> The version I picked from (o-mr1-iot-preview-8) has them as
> independent symbols, but making them a choice would resolve the issue
> for now.
>

You can select both network and USB fastboot to be enabled with the Kconfig,
but at runtime you have to select to wait on either USB or network by
running
"fastboot udp" or "fastboot usb ". When the Android
bootloader
gets the command to reboot back to fastboot, it will read the "fastbootcmd"
env
variable and run that as a command (common/android_bootloader.c:151).


>
> >> I've not tested all the code paths yet, but the obvious stuff works
> >> (basic interaction, download, boot) - every interaction elicits a
> >> `WARNING: unknown variable: slot-count` on the console; I'm guessing
> that
> >> my local end is sending a getvar for that, but I've not investigated.
> >
> >
> > Yes, there's a bit of different handling of partitions for A/B android
> > devices. Instead of "system" you have two partitions: "system_a" and
> > "system_b" (potentially more although 2 is the most common case) so to
> know
> > whether you have an old device or a newer device the fastboot client
> checks
> > the "slot-count" variable. Undefined means of course that you have an
> > old-style device, but if you return something like "2" you would be able
> to
> > flash "system" on the "slot A" which is translated (again in the fastboot
> > client) to flash "system_a" partition. This is useful when using the
> higher
> > level operations like flashall/update and you want to specify to flash
> only
> > "slot A", "slot B" or even both of them. There are other fastboot
> variables
> > that require some plumbing, such as "has-slot:" to tell
> whether
> > "system" is a partition that indeed has the two versions _a and _b.
> > Typically some partitions are twice (like "system" and "boot") and some
> > other are not (like "misc" or "userdata"). Anyway, this as is should work
> > for either old-style partition schemes or by force flashing "system_a"
> with
> > the system.img.
> >
>
> Cool, thanks... that saves me a bunch of investigation!
>
> --
> Alex Kiernan
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Kiernan]

On Tue, Apr 24, 2018 at 11:24 AM, Alex Deymo  wrote:
> +Jocelyn.
>
> Thanks Alex for taking the time to port this!!

It turned out to be a great opportunity to play with coccinelle on
something trivial, which I'd been meaning to do for ages... the
refactor to add response into the fastboot_okay/fail call chain was a
breeze with it.

> 2018-04-24 11:37 GMT+02:00 Alex Kiernan :
>>
>>
>> This series merges the fastboot UDP support from AOSP into mainline
>> U-Boot.
>>
>> Open questions:
>>
>> - what's the correct way of attributing the original authors? I've added
>>   Co-authored-by, is that right? checkpatch doesn't seem to know any
>>   of the co- tags
>> - currently there's no NAND support and I've no way of testing that, so
>>   my inclination is towards leaving it like that until someone with that
>>   particular itch to scratch can look at it
>
>
> Fastboot uses partition names, like "system" and "boot" which have a meaning
> in the Android partition scheme. For GPT, we just use the partition names as
> the android names; but if you are booting from some other storage like NAND
> where you don't have that then you need some mapping glue ("system" -->
> device and partition number). I've seen U-Boot modifications for several
> devices where they just stick a table hard-coded in the U-Boot code; which
> works great for that device but it isn't really a generic approach. Other
> than handling the names issue, I don't see any problem with supporting NAND
> here, but a generic way to set global names / alias would be needed for
> this.
>

With the fastboot NAND support in mainline it looks like we end up in
mtdparts to deliver that mapping through environment variables. No
actual idea what that looks like when you're driving it.

>>
>> - you can select both USB and UDP fastboot, but the comments in the
>>   AOSP code suggest that needs fixing - again, I've no board I can test
>>   USB fastboot on
>
>
> I thought we checked in the Kconfig that you couldn't enable both. I don't
> remember the details now but yeah you can't wait for network or USB traffic
> on the current code.
>

The version I picked from (o-mr1-iot-preview-8) has them as
independent symbols, but making them a choice would resolve the issue
for now.

>> I've not tested all the code paths yet, but the obvious stuff works
>> (basic interaction, download, boot) - every interaction elicits a
>> `WARNING: unknown variable: slot-count` on the console; I'm guessing that
>> my local end is sending a getvar for that, but I've not investigated.
>
>
> Yes, there's a bit of different handling of partitions for A/B android
> devices. Instead of "system" you have two partitions: "system_a" and
> "system_b" (potentially more although 2 is the most common case) so to know
> whether you have an old device or a newer device the fastboot client checks
> the "slot-count" variable. Undefined means of course that you have an
> old-style device, but if you return something like "2" you would be able to
> flash "system" on the "slot A" which is translated (again in the fastboot
> client) to flash "system_a" partition. This is useful when using the higher
> level operations like flashall/update and you want to specify to flash only
> "slot A", "slot B" or even both of them. There are other fastboot variables
> that require some plumbing, such as "has-slot:" to tell whether
> "system" is a partition that indeed has the two versions _a and _b.
> Typically some partitions are twice (like "system" and "boot") and some
> other are not (like "misc" or "userdata"). Anyway, this as is should work
> for either old-style partition schemes or by force flashing "system_a" with
> the system.img.
>

Cool, thanks... that saves me a bunch of investigation!

-- 
Alex Kiernan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [RFC PATCH v1 0/5] Add fastboot UDP support

[Alex Deymo]

+Jocelyn.

Thanks Alex for taking the time to port this!! Some comments inline here
about your questions.

2018-04-24 11:37 GMT+02:00 Alex Kiernan :

>
> This series merges the fastboot UDP support from AOSP into mainline
> U-Boot.
>
> Open questions:
>
> - what's the correct way of attributing the original authors? I've added
>   Co-authored-by, is that right? checkpatch doesn't seem to know any
>   of the co- tags
> - currently there's no NAND support and I've no way of testing that, so
>   my inclination is towards leaving it like that until someone with that
>   particular itch to scratch can look at it
>

Fastboot uses partition names, like "system" and "boot" which have a
meaning in the Android partition scheme. For GPT, we just use the partition
names as the android names; but if you are booting from some other storage
like NAND where you don't have that then you need some mapping glue
("system" --> device and partition number). I've seen U-Boot modifications
for several devices where they just stick a table hard-coded in the U-Boot
code; which works great for that device but it isn't really a generic
approach. Other than handling the names issue, I don't see any problem with
supporting NAND here, but a generic way to set global names / alias would
be needed for this.



> - you can select both USB and UDP fastboot, but the comments in the
>   AOSP code suggest that needs fixing - again, I've no board I can test
>   USB fastboot on
>

I thought we checked in the Kconfig that you couldn't enable both. I don't
remember the details now but yeah you can't wait for network or USB traffic
on the current code.

- the USB and UDP code want consolidating, with this series there would
>   then be two separate implementations of the same protocol
> - the syntax for the USB fastboot command changes from `fastboot
> `
>   to `fastboot usb `, that feels unacceptable and we probably
>   want something like `fastboot ` or `fastboot udp`?
> - unrelated to this series, but a show-stopper for me, there's no FIT image
>   support, but that doesn't feel unsolveable - something like add an option
>   to pass in the load address and/or use loadaddr, then something else to
>   let you override the bootm invocation on the server side
>
> I've not tested all the code paths yet, but the obvious stuff works
> (basic interaction, download, boot) - every interaction elicits a
> `WARNING: unknown variable: slot-count` on the console; I'm guessing that
> my local end is sending a getvar for that, but I've not investigated.
>

Yes, there's a bit of different handling of partitions for A/B android
devices. Instead of "system" you have two partitions: "system_a" and
"system_b" (potentially more although 2 is the most common case) so to know
whether you have an old device or a newer device the fastboot client checks
the "slot-count" variable. Undefined means of course that you have an
old-style device, but if you return something like "2" you would be able to
flash "system" on the "slot A" which is translated (again in the fastboot
client) to flash "system_a" partition. This is useful when using the higher
level operations like flashall/update and you want to specify to flash only
"slot A", "slot B" or even both of them. There are other fastboot variables
that require some plumbing, such as "has-slot:" to tell whether
"system" is a partition that indeed has the two versions _a and _b.
Typically some partitions are twice (like "system" and "boot") and some
other are not (like "misc" or "userdata"). Anyway, this as is should work
for either old-style partition schemes or by force flashing "system_a" with
the system.img.



> Without any way of testing any of the USB/NAND code I'm nervous of wading
> into that kind of refactoring, would that be a pre-requisite for merging?
>
>
> Alex Kiernan (5):
>   dfu: Refactor fastboot_okay/fail to take response
>   dfu: Extract fastboot_okay/fail to fb_common.c
>   net: dfu: Merge AOSP UDP fastboot
>   dfu: Resolve Kconfig dependency loops
>   net: dfu: Support building without MMC
>
>  cmd/fastboot.c  |  32 ++-
>  cmd/fastboot/Kconfig|  21 +-
>  cmd/net.c   |   6 +
>  common/Makefile |   4 +
>  common/fb_common.c  |  44 
>  common/fb_mmc.c | 114 ++---
>  common/fb_nand.c|  31 +--
>  common/image-sparse.c   |  41 ++-
>  drivers/usb/gadget/f_fastboot.c |  36 +--
>  include/fastboot.h  |  17 +-
>  include/fb_mmc.h|   4 +-
>  include/fb_nand.h   |   4 +-
>  include/image-sparse.h  |   2 +-
>  include/net.h   |   6 +-
>  include/net/fastboot.h  |  27 ++
>  net/Makefile|   1 +
>  net/fastboot.c  | 548 ++
> ++
>  net/net.c   |   9 +
>  18 files changed, 824 insertions(+), 123 deletions(-)
>  create