Hi Marek,
> Dear Lukasz Majewski,
>
> [...]
>
> > > > +static void handle_getstatus(struct usb_request *req)
> > > > +{
> > > > + struct dfu_status *dstat = (struct dfu_status
> > > > *)req->buf;
> > > > + struct f_dfu *f_dfu = req->context;
> > > > +
> > > > + switch (f_dfu->dfu_state) {
> > >
> > > > + case DFU_STATE_dfuDNLOAD_SYNC:
> > > What's this crazy cammel case in here ? :-)
> >
> > I know, that camel case descriptions are not welcome :-),
> > but those are written in this way for purpose.
> >
> > dfuDNLOAD_SYNC is the exact state name mentioned at DFU
> > specification. Other states - like dfuDNBUSY are exactly the same.
> >
> > From mine experience it is more readable. Please consider for
> > example the Fig. A1 - Interface state transition diagram from page
> > 28 at DFU_1.1.pdf spec (link below).
> >
> > http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf
>
> OK, good.
>
> > > > + case DFU_STATE_dfuDNBUSY:
> > > > + f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_IDLE;
> > > > + break;
> > > > + case DFU_STATE_dfuMANIFEST_SYNC:
> > > > + break;
> > > > + default:
> > > > + break;
> > > > + }
> > > > +
> > > > + /* send status response */
> > > > + dstat->bStatus = f_dfu->dfu_status;
> > > > + /* FIXME: set dstat->bwPollTimeout */
> > >
> > > FIXME ... so fix it ;-)
> >
> > This is not u-boot related - will be removed
> >
> > > > + dstat->bState = f_dfu->dfu_state;
> > > > + dstat->iString = 0;
> > > > +}
> > > > +
> > > > +static void handle_getstate(struct usb_request *req)
> > > > +{
> > > > + struct f_dfu *f_dfu = req->context;
> > > > +
> > > > + ((u8 *)req->buf)[0] = f_dfu->dfu_state & 0xff;
> > >
> > > Now ... this is ubercrazy ... can't this be made without this
> > > typecasting voodoo?
> >
> > The problem is that req->buf is a void pointer. And the goal is to
> > store dfu state at 8 bits.
>
> Sure, but why not make the buffer u8 ?
The req->buf is a member of usb_request defined at gadget.h.
It represents the request from USB. I cannot guarantee, that we will
always regard data pointed by buf as u8. For flexibility of gadget
usage it is safer to leave it as void pointer.
>
> > > > + req->actual = sizeof(u8);
> > > > +}
> > > > +
>
> [...]
>
> > > > +static int
> > > > +dfu_prepare_strings(struct f_dfu *f_dfu, int n)
> > > > +{
> > > > + struct dfu_entity *de = NULL;
> > > > + int i = 0;
> > > > +
> > > > + f_dfu->strings = calloc(((n + 1) * sizeof(struct
> > > > usb_string)), 1);
> > >
> > > calloc(n, 1) ... that's the same as malloc(), isn't it ?
> >
> > I now wonder how mine mind produced this :-)
> >
> > Correct version:
> >
> > f_dfu->strings = calloc(sizeof(struct usb_string), n + 1);
> >
> > I prefer calloc, since it delivers zeroed memory.
>
> ok, that's better :)
>
> [...]
--
Best regards,
Lukasz Majewski
Samsung Poland R&D Center | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
