mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Andrey Smirnov <andrew.smirnov@gmail.com>
To: Trent Piepho <tpiepho@kymetacorp.com>
Cc: "barebox@lists.infradead.org" <barebox@lists.infradead.org>
Subject: Re: [PATCH 15/18] [RFC] net: eth: Always use DEVICE_ID_DYNAMIC
Date: Fri, 19 Feb 2016 09:17:00 -0800	[thread overview]
Message-ID: <CAHQ1cqGttPdYNYLBWDx-Jd+zvoYV_U+FKvM__4xNdMPsS0XJFw@mail.gmail.com> (raw)
In-Reply-To: <1455823875.996.47.camel@rtred1test09.kymeta.local>

On Thu, Feb 18, 2016 at 11:30 AM, Trent Piepho <tpiepho@kymetacorp.com> wrote:
> On Tue, 2016-02-16 at 20:16 -0800, Andrey Smirnov wrote:
>> >> This patch solves the problem by forcing all Ethernet adapters to
>> >> use dynamic ID allocation.
>> >
>> > A lot of systems depend on aliases/ethernet0 pointing to the proper
>> > ethernet adapter.  How will they work with this?
>> >
>> > How will a boot script know which ethernet device to use ethact on?
>> >
>> > Suppose someone who doesn't know much about device trees and hasn't
>> > looked at the SoC datasheet, which in my experience is about 80% of the
>> > barebox userbase, comes up to you and says, "This port on the board is
>> > labeled eth1, how do tell barebox to use it?"  Right now the answer is
>> > "ethact eth1" but after this change, which basically eliminates the OF
>> > alias system, the answer is????  I think it's going to be something that
>> > said developer doesn't like very much.
>>
>> I agree with your points and this commit isn't really a proper fix, I
>> put an RFC tag there to indicate that this patch was more of "Hey
>> guys, there's this problem, let's discuss how we solve it" rather than
>> "There's your problem, here's the fix".
>>
>> >
>> > Maybe a better solution would be to have dynamically allocated devices
>> > not use IDs that have been "reserved" by the existence of an OF alias?
>> > There wouldn't be some call to reserve an id, just the alias's existence
>> > would be sufficient. Common code that allocates IDs should have all the
>> > info it needs to check for an alias and then either use it or allocate
>> > an id not already aliased.
>>
>> It is a fix, but only partial. To play devil's advocate, imagine a
>> board that has SoC with two PCIe Ethernet chips (think i210) attached
>> to it. Even if you query for aliases you get back to the same problem
>> where your silkscreen labels are not providing any good info to inform
>> general user what to give to "ethact" (granted it is possible to
>> assigned a DT node to PCI device and thus probably possible to create
>> an aliases for such interfaces, but it is very clunky in practice not
>> many .dts files has any provisions like that)
>
> Adding an alias for such a device seems reasonable to me.  My though
> process goes something like this:
>
> If the ports are a fixed part of the device, even if on a probe-able bus
> like PCIe or USB, then we should be able to assign them a name.  They
> are always there, always in the same place, always connected the same
> way, so it's not impossible.
>
> So we use what's constant, how the port is connected.  It will be a
> certain device-function in a certain slot on a certain bus on a certain
> pci controller.  And we make that the name because it's always the same.
>
> But that name is huge and impossible to remember or type in.  So we
> create a link, or alias, for it.  An easier to use name that points to
> full name.

I don't necessarily agree with this point:

- IMHO, the amount if information needed to be encoded in the name to
cover 99% of the use-cases is not that impossible to type or remember,
compare e.g. "eth-pci-00-01", "eth-usb-00-01", "eth-spi-00-01" to
"eth0", "eth1"

- Even if we ignore the first point. IMHO, one's chances to remember
that "that one port to the left edge of the board" is "eth0" and "the
one to the right edge of the board" is "eth1" are the same as
remembering that former is "eth-pci-xx-yy" and latter is
"eth-spi-zz-zz", so the only way to make the process painless is to
have appropriate silkscreen and maybe labels on the case of the
device. At this point it doesn't make that much difference if it is
the "old" style name or a "new" one that's captured in those places.

>
> And thus we arrive at what is basically the OF alias system.
>

It's not that this can't "solve" this problem, but in my opinion (and
I could be completely wrong) the purpose of aliases in DT is to allow
flexibility in DT code and the fact it can be used to alter the
behavior of BB is somewhat of a serendipitous side-effect. What I
don't like about this solution is that it creates a dependency between
DT and BB's board's scripts that doesn't have to be there.

>
>> I think the most sound fix for this problem would be to embed the
>> NIC's topology information into its name (not unlike Consistent
>
> Isn't that basically the same thing as the OF path of a device?

Yes and no, it would be a rather abridged version of OF path that
omits a number of irrelevant details

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2016-02-19 17:17 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17  1:29 [PATCH 00/17] RFC for additional 'nvmem' plumbing Andrey Smirnov
2016-02-17  1:29 ` [PATCH 01/18] common: Add EPROBE_DEFER to strerror Andrey Smirnov
2016-02-17  1:29 ` [PATCH 02/18] base: driver.c: Coalesce error checking code Andrey Smirnov
2016-02-17  1:29 ` [PATCH 03/18] [RFC] at91: Make IS_ERR work for I/O pointers Andrey Smirnov
2016-02-17  8:34   ` Sascha Hauer
2016-02-17 20:39     ` Andrey Smirnov
2016-02-18 10:26       ` Sascha Hauer
2016-02-17  1:29 ` [PATCH 04/18] [RFC] base/driver.c: Remove dev_request_mem_region_err_null Andrey Smirnov
2016-02-17  1:29 ` [PATCH 05/18] i2c-at91: Use IS_ERR instead of checking for NULL Andrey Smirnov
2016-02-17  1:29 ` [PATCH 06/18] clk-imx6: Call clk_enable on mmdc_ch0_axi_podf Andrey Smirnov
2016-02-17  1:29 ` [PATCH 07/18] fec_imx: Deallocate clocks when probe fails Andrey Smirnov
2016-02-17  1:29 ` [PATCH 08/18] [RFC] base: Introduce dev_request_mem_resource Andrey Smirnov
2016-02-17  1:29 ` [PATCH 09/18] fec_imx: Deallocate I/O resources if probe fails Andrey Smirnov
2016-02-17  1:29 ` [PATCH 10/18] fec_imx: Free phy_reset GPIO if when " Andrey Smirnov
2016-02-17  1:29 ` [PATCH 11/18] fec_imx: Use FEC_ECNTRL_RESET instead of a magic number Andrey Smirnov
2016-02-17  1:29 ` [PATCH 12/18] fec_imx: Impelemnt reset timeout Andrey Smirnov
2016-02-17  8:43   ` Sascha Hauer
2016-02-17  1:29 ` [PATCH 13/18] fec_imx: Deallocate DMA buffers when probe fails Andrey Smirnov
2016-02-17  1:29 ` [PATCH 14/18] fec_imx: Unregister MDIO " Andrey Smirnov
2016-02-17  1:29 ` [PATCH 15/18] [RFC] net: eth: Always use DEVICE_ID_DYNAMIC Andrey Smirnov
2016-02-17  2:40   ` Trent Piepho
2016-02-17  4:16     ` Andrey Smirnov
2016-02-18 19:30       ` Trent Piepho
2016-02-19 17:17         ` Andrey Smirnov [this message]
2016-02-17  1:29 ` [PATCH 16/18] [RFC] nvmem: Add of_nvmem_cell_from_cell_np Andrey Smirnov
2016-02-17  1:29 ` [PATCH 17/18] [RFC] nvmem: Add nvmem-ro-of-blob driver Andrey Smirnov
2016-02-17  8:52   ` Sascha Hauer
2016-02-17 20:40     ` Andrey Smirnov
2016-02-17  1:29 ` [PATCH 18/18] [RFC] nvmem: Add nvmem-sg driver Andrey Smirnov
2016-02-17  3:09 ` [PATCH 00/17] RFC for additional 'nvmem' plumbing Trent Piepho
2016-02-17  4:20   ` Andrey Smirnov

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=CAHQ1cqGttPdYNYLBWDx-Jd+zvoYV_U+FKvM__4xNdMPsS0XJFw@mail.gmail.com \
    --to=andrew.smirnov@gmail.com \
    --cc=barebox@lists.infradead.org \
    --cc=tpiepho@kymetacorp.com \
    /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