public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Stefan Roese <sr@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] Setting ipaddr via setenv() - H_INTERACTIVE vs H_PROGRAMMATIC
Date: Wed, 2 Mar 2016 08:01:53 +0100	[thread overview]
Message-ID: <56D68FE1.4040006@denx.de> (raw)
In-Reply-To: <CANr=Z=ZwZLweBV0x3z99L7RPnoCQzym=icqdzAXFuNq1Kj+O6A@mail.gmail.com>

Hi Joe,

On 02.03.2016 07:46, Joe Hershberger wrote:
> On Wed, Mar 2, 2016 at 12:21 AM, Stefan Roese <sr@denx.de> wrote:
>> Hi Joe,
>>
>> (adding Tom to Cc as this seems to be a fundamental issue)
>>
>> On 01.03.2016 20:40, Joe Hershberger wrote:
>>> On Tue, Mar 1, 2016 at 4:35 AM, Stefan Roese <sr@denx.de> wrote:
>>>> Hi Joe,
>>>>
>>>> I'm currently stumbling over a problem in some board specific code
>>>> (not in mainline yet), that sets the "ipaddr" env variable via
>>>> setenv(). But as I now noticed, the callback on_ipaddr() doesn't
>>>> set net_ip to this new value. As flags has H_PROGRAMMATIC set and
>>>> op_ipaddr() skips setting this value in this case.
>>>>
>>>> I fail to see why we need this check in on_ipaddr(). Could you
>>>> please explain why this is needed? Why would someone use setenv()
>>>> for the "ipaddr" and not want to also update the net_ip value.
>>>> This results in the "ipaddr" value being updated but net_ip
>>>> still being configured to the old value.
>>>
>>> The purpose is that programmatic accesses may write directly to the
>>> net_ip variable, but the user on the console cannot. This is because
>>> the "programmatic" accesses to these variables is expected to be
>>> things like the dhcp command and the linklocal command after
>>> successful netloop.
>>
>> Please correct me, but it seems to be implemented the other way around.
>> If H_PROGRAMMATIC is set (called via setenv()) the net variables
>> (net_ip...) will *not* be set:
>>
>> static int on_ipaddr(const char *name, const char *value, enum env_op op,
>>          int flags)
>> {
>>          if (flags & H_PROGRAMMATIC)
>>                  return 0;
>>
>>          net_ip = string_to_ip(value);
>>
>>          return 0;
>> }
>>
>>> You can see this call in the netboot_update_env()
>>> function in cmd/net.c.
>>
>> netboot_update_env() does not update the internal variables. At least
>> not in my tests. op_ipaddr() returns directly after the "flags" check
>> (see above).
>
> That's exactly the point. It doesn't set the internal variables
> because the source *is* the internal variables.

Now I understand what you mean. The PROGRAMMATIC approach should set
the variables directly. I'll set the net_ip variable directly from
the board specific code then.

But again, why exactly do we need to distinguish between this
interactive and programmatic access? Why not set the internal variables
every time the special env variables are set (interactively and
programmatically)?

Thanks,
Stefan

  reply	other threads:[~2016-03-02  7:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-01 10:35 [U-Boot] Setting ipaddr via setenv() - H_INTERACTIVE vs H_PROGRAMMATIC Stefan Roese
2016-03-01 19:40 ` Joe Hershberger
2016-03-02  6:21   ` Stefan Roese
2016-03-02  6:46     ` Joe Hershberger
2016-03-02  7:01       ` Stefan Roese [this message]
2016-03-03  1:58         ` Joe Hershberger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56D68FE1.4040006@denx.de \
    --to=sr@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox