Re: picotcp tftp support [was Adding IPv4 multicast support]
On Sun, Sep 28, 2014 at 4:22 PM, Antony Pavlov wrote:
>
> 1. Do we really need this '#ifdef PICO_SUPPORT_UDP' BEFORE (!)
> guard macro in pico_dhcp_client.h?
>
I moved these in latest PicoTCP commit. Thanks.
> 2. New pico_tftp_start_{r,t}x programming interface can be used for
> realization legacy standalone 'tftp' command.
> But I have no idea how to use this interface for filesystem-over-tftp
> (it's the modern barebox' tftp realization)!
>
> Barebox filesystem interface needs something like this programming
> interface:
>
> struct pico_tftp_session *s;
>
> s = pico_tftp_new_session_rx(addr, port, family, filename);
> ...
> filesize = pico_tftp_get_file_size(s);
> ...
> pico_tftp_receive(s, buf, len);
> ...
> pico_tftp_receive(s, buf, len);
> ... ... ...
> pico_tftp_receive(s, buf, len);
> ...
> pico_tftp_close(s);
>
>
> I mean that receiving process has to be controlled by barebox,
> not by picotcp. Because a new portion of data is received
> by filesystem user initiative, not by network stack initiative.
Thanks for the comments. A few from my side:
- pico_tftp_receive is non-blocking, and will return immediately if no
data is available. You always have to be bound to some RX event to be
sure that your next receive() call is successful, so in some way you
still want it network-bound.
- We can implement the get_file_size, but not all servers will support
it. Michele has a strategy for developing more advanced TFTP features,
he will post on it later
regards,
/d
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: picotcp tftp support [was Adding IPv4 multicast support]
On Mon, Sep 29, 2014 at 11:45 AM, Daniele Lacamera
wrote:
> On Sun, Sep 28, 2014 at 4:22 PM, Antony Pavlov
> wrote:
>>
>> 1. Do we really need this '#ifdef PICO_SUPPORT_UDP' BEFORE (!)
>> guard macro in pico_dhcp_client.h?
>>
>
> I moved these in latest PicoTCP commit. Thanks.
>
>
>> 2. New pico_tftp_start_{r,t}x programming interface can be used for
>> realization legacy standalone 'tftp' command.
>> But I have no idea how to use this interface for filesystem-over-tftp
>> (it's the modern barebox' tftp realization)!
>>
>> Barebox filesystem interface needs something like this programming
>> interface:
>>
>> struct pico_tftp_session *s;
>>
>> s = pico_tftp_new_session_rx(addr, port, family, filename);
>> ...
>> filesize = pico_tftp_get_file_size(s);
>> ...
>> pico_tftp_receive(s, buf, len);
>> ...
>> pico_tftp_receive(s, buf, len);
>> ... ... ...
>> pico_tftp_receive(s, buf, len);
>> ...
>> pico_tftp_close(s);
>>
>>
>> I mean that receiving process has to be controlled by barebox,
>> not by picotcp. Because a new portion of data is received
>> by filesystem user initiative, not by network stack initiative.
>
> Thanks for the comments. A few from my side:
>
> - pico_tftp_receive is non-blocking, and will return immediately if no
> data is available. You always have to be bound to some RX event to be
> sure that your next receive() call is successful, so in some way you
> still want it network-bound.
I'll modify the API in order to add this capability, the application
will be responsible to sending ACK message as soon as possible (it
will be wrapped into the new applications API we will provide.
> - We can implement the get_file_size, but not all servers will support
> it. Michele has a strategy for developing more advanced TFTP features,
> he will post on it later
>
TFTP version 2 (RFC 1350) handles only simple GET and PUT requests.
Query for file size is not there, it's stated in RFC 2359 as an
OPTIONAL feature; this means that his support is not guaranteed by all
servers.
We currently support only RFC 1350 but we plan to add support for RFC
2359 soon. I've just added two new feature request on our issue
tracker. In meanwhile please try if you can use the current API as
suggested in the example file test/examples/tftp.c
> regards,
Best regards
Michele
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: picotcp tftp support [was Adding IPv4 multicast support]
On Mon, 29 Sep 2014 11:45:08 +0200
Daniele Lacamera wrote:
> On Sun, Sep 28, 2014 at 4:22 PM, Antony Pavlov
> wrote:
> >
> > 1. Do we really need this '#ifdef PICO_SUPPORT_UDP' BEFORE (!)
> > guard macro in pico_dhcp_client.h?
> >
>
> I moved these in latest PicoTCP commit. Thanks.
>
>
> > 2. New pico_tftp_start_{r,t}x programming interface can be used for
> > realization legacy standalone 'tftp' command.
> > But I have no idea how to use this interface for filesystem-over-tftp
> > (it's the modern barebox' tftp realization)!
> >
> > Barebox filesystem interface needs something like this programming
> > interface:
> >
> > struct pico_tftp_session *s;
> >
> > s = pico_tftp_new_session_rx(addr, port, family, filename);
> > ...
> > filesize = pico_tftp_get_file_size(s);
> > ...
> > pico_tftp_receive(s, buf, len);
> > ...
> > pico_tftp_receive(s, buf, len);
> > ... ... ...
> > pico_tftp_receive(s, buf, len);
> > ...
> > pico_tftp_close(s);
> >
> >
> > I mean that receiving process has to be controlled by barebox,
> > not by picotcp. Because a new portion of data is received
> > by filesystem user initiative, not by network stack initiative.
>
> Thanks for the comments. A few from my side:
>
> - pico_tftp_receive is non-blocking, and will return immediately if no
> data is available. You always have to be bound to some RX event to be
> sure that your next receive() call is successful, so in some way you
> still want it network-bound.
>
> - We can implement the get_file_size, but not all servers will support
> it. Michele has a strategy for developing more advanced TFTP features,
> he will post on it later
I want just to say that barebox tries to get file size if it's possible.
Please see tftp_parse_oack() function in fs/tftp.c:
if (!strcmp(opt, "tsize"))
priv->filesize = simple_strtoul(val, NULL, 10);
This priv->filesize is used in tftp filesystem stat() call realization.
So it's reasonable to give barebox a means to set priv->filesize
after switching to picotcp.
--
Best regards,
Antony Pavlov
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCHv2] commands: add of_display_timings
Hello Sascha,
Am Montag, den 29.09.2014, 08:05 +0200 schrieb Sascha Hauer:
> On Fri, Sep 26, 2014 at 01:22:31PM +0200, Teresa Gámez wrote:
> > A lot of boards use display-timings nodes to define the timings of one or
> > more
> > displays. This command makes it possible to list and select displays which
> > are
> > defined in a device tree.
> >
> > Signed-off-by: Teresa Gámez
> > ---
> > Changes v2:
> > - removed cast
> > - added free(ftd)
> > - freed root
> >
...
> > +
> > +static int do_of_display_timings(int argc, char *argv[])
> > +{
> > + int opt;
> > + int list = 0;
> > + int selected = 0;
> > + int ret = 0;
> > + struct device_node *root = NULL;
> > + struct device_node *internal_root = NULL;
> > + struct device_node *display = NULL;
> > + struct device_node *timings = NULL;
> > + char *timingpath = NULL;
> > + char *dtbfile = NULL;
> > +
> > + while ((opt = getopt(argc, argv, "sS:lf:")) > 0) {
> > + switch (opt) {
> > + case 'l':
> > + list = 1;
> > + break;
> > + case 'f':
> > + dtbfile = optarg;
> > + break;
> > + case 's':
> > + selected = 1;
> > + break;
> > + case 'S':
> > + timingpath = xzalloc(strlen(optarg) + 1);
> > + strcpy(timingpath, optarg);
> > + break;
>
> You're right, there's no place to free this string.
>
> BTW we have xstrdup for duplicating strings.
>
>
> > + /*
> > +* We set the external node to our internal node as
> > +* long as the command runs. This makes life easier to
> > +* use of tree functions. We backup the internal root node
> > +* and set it back at the end.
> > +*/
> > + internal_root = of_get_root_node();
> > + of_set_root_node(NULL);
> > + of_set_root_node(root);
>
> Since the only device tree function you are using is
> for_each_node_by_name(), could you add a:
>
> #define for_each_node_by_name_from(dn, root, name) \
> for (dn = of_find_node_by_name(root, name); dn; \
>dn = of_find_node_by_name(dn, name))
>
> to include/of.h and remove the temporary overwriting of the
> internal device tree?
I'm also using of_parse_phandle() which calls of_find_node_by_phandle().
And this is using the root_node again.
I can create *_from() functions for both of them as overwriting the root
node is really not pretty..
Teresa
>
> Sascha
>
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCHv2] commands: add of_display_timings
On Mon, Sep 29, 2014 at 02:50:11PM +0200, Teresa Gamez wrote:
> Hello Sascha,
>
> Am Montag, den 29.09.2014, 08:05 +0200 schrieb Sascha Hauer:
> > On Fri, Sep 26, 2014 at 01:22:31PM +0200, Teresa Gámez wrote:
> > > A lot of boards use display-timings nodes to define the timings of one or
> > > more
> > > displays. This command makes it possible to list and select displays
> > > which are
> > > defined in a device tree.
> > >
> > > Signed-off-by: Teresa Gámez
> > > ---
> > > Changes v2:
> > > - removed cast
> > > - added free(ftd)
> > > - freed root
> > >
>
> ...
> > > +
> > > +static int do_of_display_timings(int argc, char *argv[])
> > > +{
> > > + int opt;
> > > + int list = 0;
> > > + int selected = 0;
> > > + int ret = 0;
> > > + struct device_node *root = NULL;
> > > + struct device_node *internal_root = NULL;
> > > + struct device_node *display = NULL;
> > > + struct device_node *timings = NULL;
> > > + char *timingpath = NULL;
> > > + char *dtbfile = NULL;
> > > +
> > > + while ((opt = getopt(argc, argv, "sS:lf:")) > 0) {
> > > + switch (opt) {
> > > + case 'l':
> > > + list = 1;
> > > + break;
> > > + case 'f':
> > > + dtbfile = optarg;
> > > + break;
> > > + case 's':
> > > + selected = 1;
> > > + break;
> > > + case 'S':
> > > + timingpath = xzalloc(strlen(optarg) + 1);
> > > + strcpy(timingpath, optarg);
> > > + break;
> >
> > You're right, there's no place to free this string.
> >
> > BTW we have xstrdup for duplicating strings.
> >
> >
> > > + /*
> > > + * We set the external node to our internal node as
> > > + * long as the command runs. This makes life easier to
> > > + * use of tree functions. We backup the internal root node
> > > + * and set it back at the end.
> > > + */
> > > + internal_root = of_get_root_node();
> > > + of_set_root_node(NULL);
> > > + of_set_root_node(root);
> >
> > Since the only device tree function you are using is
> > for_each_node_by_name(), could you add a:
> >
> > #define for_each_node_by_name_from(dn, root, name) \
> > for (dn = of_find_node_by_name(root, name); dn; \
> > dn = of_find_node_by_name(dn, name))
> >
> > to include/of.h and remove the temporary overwriting of the
> > internal device tree?
>
> I'm also using of_parse_phandle() which calls of_find_node_by_phandle().
> And this is using the root_node again.
>
> I can create *_from() functions for both of them as overwriting the root
> node is really not pretty..
Yes, I think it's better to create functions that can be passed a root
node. It's probably not the last time we need to modify an external
device tree. Overwriting the root node maybe has funny side effects in
the future.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] commands: usb: drop help for force rescan option
From: "Wjatscheslaw Stoljarski"
Signed-off-by: Wjatscheslaw Stoljarski
---
commands/usb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/commands/usb.c b/commands/usb.c
index a37d503..48c6619 100644
--- a/commands/usb.c
+++ b/commands/usb.c
@@ -138,7 +138,6 @@ BAREBOX_CMD_HELP_START(usb)
BAREBOX_CMD_HELP_TEXT("Scan for USB devices.")
BAREBOX_CMD_HELP_TEXT("")
BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT("-f", "force rescan")
BAREBOX_CMD_HELP_OPT("-s", "show devices")
BAREBOX_CMD_HELP_OPT("-t", "show USB tree")
BAREBOX_CMD_HELP_END
@@ -146,7 +145,7 @@ BAREBOX_CMD_HELP_END
BAREBOX_CMD_START(usb)
.cmd= do_usb,
BAREBOX_CMD_DESC("(re-)detect USB devices")
- BAREBOX_CMD_OPTS("[-fts]")
+ BAREBOX_CMD_OPTS("[-ts]")
BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
BAREBOX_CMD_HELP(cmd_usb_help)
BAREBOX_CMD_COMPLETE(empty_complete)
--
1.9.1
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
