linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dt-bindings: Add bindings for aliases node
@ 2018-09-25 21:02 Matthias Kaehlcke
  2018-10-09  1:07 ` Brian Norris
  2018-10-09  5:57 ` Stephen Boyd
  0 siblings, 2 replies; 9+ messages in thread
From: Matthias Kaehlcke @ 2018-09-25 21:02 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland
  Cc: devicetree, linux-kernel, Brian Norris, Matthias Kaehlcke

Add a global binding for the 'aliases' node. This includes an initial list
of standardized alias names for some hardware components that are commonly
found in 'aliases'.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
---
 Documentation/devicetree/bindings/aliases.txt | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/aliases.txt

diff --git a/Documentation/devicetree/bindings/aliases.txt b/Documentation/devicetree/bindings/aliases.txt
new file mode 100644
index 000000000000..d64ed1c7eb34
--- /dev/null
+++ b/Documentation/devicetree/bindings/aliases.txt
@@ -0,0 +1,47 @@
+The aliases node
+----------------
+
+The aliases node contains properties that represent aliases to device tree
+nodes. The name of the property is the alias name, the value is the path of
+a the device tree node that corresponds to the alias. The path may be
+specified as a string or a phandle.
+
+Alias names are often suffixed with a numeric ID, especially when there may
+be multiple instances of the same type. The ID typically corresponds to the
+hardware layout, it may also be used by drivers for a stable mapping of
+device names and hardware entities.
+
+Alias names
+-----------
+
+The devicetree specification doesn't require the use of specific alias
+names to refer to hardware entities of a given type, however the Linux
+kernel aims for a certain level of consistency.
+
+The following standardized alias names shall be used for their
+corresponding hardware components:
+
+  bluetoothN		Bluetooth controller
+  ethernetN		Ethernet interface
+  gpioN			GPIO controller
+  i2cN			i2c bus
+  mmcN			MMC bus
+  rtcN			Real time clock
+  serialN		UART port
+  spiN			SPI bus
+  wifiN			Wireless network interface
+
+The above list is not exhaustive and will be extended over time. Please
+send patches to devicetree@vger.kernel.org if you think a hardware
+component and its alias name should be on the list.
+
+Example
+-------
+
+aliases {
+	bluetooth0 = "/soc/serial@fdf01000/bluetooth";
+	rtc0 = &rtc0;
+	wifi0 = &wlcore;
+};
+
+(based on arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts)
-- 
2.19.0.605.g01d371f741-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] dt-bindings: Add bindings for aliases node
  2018-09-25 21:02 [PATCH] dt-bindings: Add bindings for aliases node Matthias Kaehlcke
@ 2018-10-09  1:07 ` Brian Norris
  2018-10-09  7:22   ` Geert Uytterhoeven
  2018-10-09  5:57 ` Stephen Boyd
  1 sibling, 1 reply; 9+ messages in thread
From: Brian Norris @ 2018-10-09  1:07 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
	linux-wireless, linux-spi, netdev, Stephen Boyd,
	Florian Fainelli

+ linux-spi, linux-wireless, netdev
+ others from previous conversations

Hi,

On Tue, Sep 25, 2018 at 02:02:55PM -0700, Matthias Kaehlcke wrote:
> Add a global binding for the 'aliases' node. This includes an initial list
> of standardized alias names for some hardware components that are commonly
> found in 'aliases'.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  Documentation/devicetree/bindings/aliases.txt | 47 +++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/aliases.txt
> 
> diff --git a/Documentation/devicetree/bindings/aliases.txt b/Documentation/devicetree/bindings/aliases.txt
> new file mode 100644
> index 000000000000..d64ed1c7eb34
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/aliases.txt
> @@ -0,0 +1,47 @@
> +The aliases node
> +----------------

I like the idea in general, and it might be good to note (e.g., commit
message) that this was inspired by this thread:

https://lore.kernel.org/lkml/20180815221601.GB24830@rob-hp-laptop/

where we were interested in firmware-to-device-tree path stability --
and the answer was basically: don't memorize paths, just use aliases
instead. But then, it was clear that aliases were not documented very
formally at all.

So here we are!

> +
> +The aliases node contains properties that represent aliases to device tree
> +nodes. The name of the property is the alias name, the value is the path of
> +a the device tree node that corresponds to the alias. The path may be
> +specified as a string or a phandle.
> +
> +Alias names are often suffixed with a numeric ID, especially when there may
> +be multiple instances of the same type. The ID typically corresponds to the
> +hardware layout, it may also be used by drivers for a stable mapping of
> +device names and hardware entities.
> +
> +Alias names
> +-----------
> +
> +The devicetree specification doesn't require the use of specific alias
> +names to refer to hardware entities of a given type, however the Linux
> +kernel aims for a certain level of consistency.
> +
> +The following standardized alias names shall be used for their
> +corresponding hardware components:
> +
> +  bluetoothN		Bluetooth controller
> +  ethernetN		Ethernet interface
> +  gpioN			GPIO controller
> +  i2cN			i2c bus
> +  mmcN			MMC bus
> +  rtcN			Real time clock
> +  serialN		UART port
> +  spiN			SPI bus
> +  wifiN			Wireless network interface

For the network-device-related names (bluetooth, ethernet, and wifi), I
think there's a clear documented reason for this (supporting MAC address
plumbing from a DT-aware bootloader). I'm not quite as sure about all
the others, and unfortunately, I'm aware of at least one subsystem owner
that explicitly does NOT like the aliases usage that is currently
supported (spi), and shot down a patch where I tried to use it in a DTS
file (despite its regular usage in many other DTS files).

So I guess I'm saying: perhaps we should get buy-in from various
subsystems before we include them? So maybe it's wiser to start
small(er) and only add once we're sure they are useful? Or perhaps Rob
has other thoughts.

> +
> +The above list is not exhaustive and will be extended over time. Please
> +send patches to devicetree@vger.kernel.org if you think a hardware
> +component and its alias name should be on the list.
> +
> +Example
> +-------
> +
> +aliases {
> +	bluetooth0 = "/soc/serial@fdf01000/bluetooth";
> +	rtc0 = &rtc0;
> +	wifi0 = &wlcore;
> +};
> +
> +(based on arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts)

What is the relevance of this line? This doesn't look anything like that
hikey DTS. Maybe the "based on" line should just be removed? The example
seems fine though.

Anyway, perhaps with a trimmed list of supported alias names:

Reviewed-by: Brian Norris <briannorris@chromium.org>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dt-bindings: Add bindings for aliases node
  2018-09-25 21:02 [PATCH] dt-bindings: Add bindings for aliases node Matthias Kaehlcke
  2018-10-09  1:07 ` Brian Norris
@ 2018-10-09  5:57 ` Stephen Boyd
  2018-10-11 23:50   ` Matthias Kaehlcke
  1 sibling, 1 reply; 9+ messages in thread
From: Stephen Boyd @ 2018-10-09  5:57 UTC (permalink / raw)
  To: Mark Rutland, Matthias Kaehlcke, Rob Herring
  Cc: devicetree, linux-kernel, Brian Norris, Matthias Kaehlcke

Quoting Matthias Kaehlcke (2018-09-25 14:02:55)
> Add a global binding for the 'aliases' node. This includes an initial list
> of standardized alias names for some hardware components that are commonly
> found in 'aliases'.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  Documentation/devicetree/bindings/aliases.txt | 47 +++++++++++++++++++

Any chance we can get a documentation update for the devicetree spec as
well?

>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/aliases.txt
> 
> diff --git a/Documentation/devicetree/bindings/aliases.txt b/Documentation/devicetree/bindings/aliases.txt
> new file mode 100644
> index 000000000000..d64ed1c7eb34
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/aliases.txt
> @@ -0,0 +1,47 @@
> +The aliases node
> +----------------
> +
> +The aliases node contains properties that represent aliases to device tree
> +nodes. The name of the property is the alias name, the value is the path of
> +a the device tree node that corresponds to the alias. The path may be

s/a //

> +specified as a string or a phandle.
> +
> +Alias names are often suffixed with a numeric ID, especially when there may
> +be multiple instances of the same type. The ID typically corresponds to the
> +hardware layout, it may also be used by drivers for a stable mapping of
> +device names and hardware entities.

Indicate the numeric ID is a u32 or something like that?

> +
> +Alias names
> +-----------
> +
> +The devicetree specification doesn't require the use of specific alias
> +names to refer to hardware entities of a given type, however the Linux
> +kernel aims for a certain level of consistency.
> +
> +The following standardized alias names shall be used for their
> +corresponding hardware components:
> +
> +  bluetoothN           Bluetooth controller

Can we use syntax like:

	bluetooth<N>

to indicate that <N> is an unsigned integer?

> +  ethernetN            Ethernet interface
> +  gpioN                        GPIO controller
> +  i2cN                 i2c bus
> +  mmcN                 MMC bus
> +  rtcN                 Real time clock
> +  serialN              UART port
> +  spiN                 SPI bus
> +  wifiN                        Wireless network interface
> +
> +The above list is not exhaustive and will be extended over time. Please
> +send patches to devicetree@vger.kernel.org if you think a hardware
> +component and its alias name should be on the list.
> +

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dt-bindings: Add bindings for aliases node
  2018-10-09  1:07 ` Brian Norris
@ 2018-10-09  7:22   ` Geert Uytterhoeven
  2018-10-09 18:31     ` Brian Norris
  0 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2018-10-09  7:22 UTC (permalink / raw)
  To: Brian Norris
  Cc: Matthias Kaehlcke, Rob Herring, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List, linux-wireless, linux-spi, netdev,
	swboyd, Florian Fainelli

Hi Brian,

On Tue, Oct 9, 2018 at 3:07 AM Brian Norris <briannorris@chromium.org> wrote:
> On Tue, Sep 25, 2018 at 02:02:55PM -0700, Matthias Kaehlcke wrote:
> > Add a global binding for the 'aliases' node. This includes an initial list
> > of standardized alias names for some hardware components that are commonly
> > found in 'aliases'.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/aliases.txt
> > @@ -0,0 +1,47 @@
> > +The aliases node
> > +----------------
>
> I like the idea in general, and it might be good to note (e.g., commit
> message) that this was inspired by this thread:
>
> https://lore.kernel.org/lkml/20180815221601.GB24830@rob-hp-laptop/
>
> where we were interested in firmware-to-device-tree path stability --
> and the answer was basically: don't memorize paths, just use aliases
> instead. But then, it was clear that aliases were not documented very
> formally at all.
>
> So here we are!
>
> > +
> > +The aliases node contains properties that represent aliases to device tree
> > +nodes. The name of the property is the alias name, the value is the path of
> > +a the device tree node that corresponds to the alias. The path may be
> > +specified as a string or a phandle.
> > +
> > +Alias names are often suffixed with a numeric ID, especially when there may
> > +be multiple instances of the same type. The ID typically corresponds to the
> > +hardware layout, it may also be used by drivers for a stable mapping of
> > +device names and hardware entities.
> > +
> > +Alias names
> > +-----------
> > +
> > +The devicetree specification doesn't require the use of specific alias
> > +names to refer to hardware entities of a given type, however the Linux
> > +kernel aims for a certain level of consistency.
> > +
> > +The following standardized alias names shall be used for their

s/shall/may/

> > +corresponding hardware components:
> > +
> > +  bluetoothN         Bluetooth controller
> > +  ethernetN          Ethernet interface
> > +  gpioN                      GPIO controller
> > +  i2cN                       i2c bus
> > +  mmcN                       MMC bus
> > +  rtcN                       Real time clock
> > +  serialN            UART port
> > +  spiN                       SPI bus
> > +  wifiN                      Wireless network interface
>
> For the network-device-related names (bluetooth, ethernet, and wifi), I
> think there's a clear documented reason for this (supporting MAC address
> plumbing from a DT-aware bootloader). I'm not quite as sure about all
> the others, and unfortunately, I'm aware of at least one subsystem owner
> that explicitly does NOT like the aliases usage that is currently
> supported (spi), and shot down a patch where I tried to use it in a DTS
> file (despite its regular usage in many other DTS files).
>
> So I guess I'm saying: perhaps we should get buy-in from various
> subsystems before we include them? So maybe it's wiser to start
> small(er) and only add once we're sure they are useful? Or perhaps Rob
> has other thoughts.

Please note these aliases become cumbersome once you start considering
(dynamic) DT overlays.  That's why I made them optional in the sh-sci
serial driver, cfr. commit 7678f4c20fa7670f ("serial: sh-sci: Add support
for dynamic instances").
Relevant parts of the commit description are:

    On DT platforms, the sh-sci driver requires the presence of "serialN"
    aliases in DT, from which instance IDs are derived.  If a DT alias is
    missing, the drivers fails to probe the corresponding serial port.

    This becomes cumbersome when considering DT overlays, as currently
    there is no upstream support for dynamically updating the /aliases node
    in DT.  Furthermore, even in the presence of such support, hardcoded
    instance IDs in independent overlays are prone to conflicts.

    Hence add support for dynamic instance IDs, to be used in the absence of
    a DT alias.  This makes serial ports behave similar to I2C and SPI
    buses, which already support dynamic instances.

To clarify my point: R-Car M2-W has 4 different types of serial ports, for a
total of 18 ports, and the two ports on a board labeled 0 and 1 may not
correspond to the physical first two ports (what's "first" in a collection of
4 different types?).

Aliases may be fine for referring to the main serial console (labeled
port 0 on the device, too), and the primary Ethernet interface (so U-Boot
knows where to add the "local-mac-address" property), but beyond that,
I think they should be avoided.

Just my two^H^H^Hfive €c.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dt-bindings: Add bindings for aliases node
  2018-10-09  7:22   ` Geert Uytterhoeven
@ 2018-10-09 18:31     ` Brian Norris
  2018-10-12  0:08       ` Matthias Kaehlcke
  2018-10-15 18:00       ` Rob Herring
  0 siblings, 2 replies; 9+ messages in thread
From: Brian Norris @ 2018-10-09 18:31 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Matthias Kaehlcke, Rob Herring, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List, linux-wireless, linux-spi, netdev,
	swboyd, Florian Fainelli

On Tue, Oct 09, 2018 at 09:22:07AM +0200, Geert Uytterhoeven wrote:
> Please note these aliases become cumbersome once you start considering
> (dynamic) DT overlays.  That's why I made them optional in the sh-sci
> serial driver, cfr. commit 7678f4c20fa7670f ("serial: sh-sci: Add support
> for dynamic instances").

Note that as I understand it, the entire point of documenting this sort
of thing is to help solidify the interface between a DT aware boot
program (e.g., bootloader) and a device tree which is provided
separately, to avoid memorizing node/path hierarchy. It doesn't need to
(and doesn't, as I read it) enforce an OS's device naming policy.

> Relevant parts of the commit description are:
> 
>     On DT platforms, the sh-sci driver requires the presence of "serialN"
>     aliases in DT, from which instance IDs are derived.  If a DT alias is
>     missing, the drivers fails to probe the corresponding serial port.
> 
>     This becomes cumbersome when considering DT overlays, as currently
>     there is no upstream support for dynamically updating the /aliases node
>     in DT.

That part is not a DT spec problem :)

>     Furthermore, even in the presence of such support, hardcoded
>     instance IDs in independent overlays are prone to conflicts.
> 
>     Hence add support for dynamic instance IDs, to be used in the absence of
>     a DT alias.  This makes serial ports behave similar to I2C and SPI
>     buses, which already support dynamic instances.

This seems to be a much different sort of problem. People always love
having predictable IDs given by the OS (myself included), but that's
just plain hard to do and impossible in some cases. I don't think that's
what this document is about though.

IOW, this document seems pretty consistent with the above: it doesn't
require the usage of aliases (and it seems silly to have a driver
*require* an alias) -- it just documents how one should name such an
alias if you expect multiple independent software components to
understand it.

> To clarify my point: R-Car M2-W has 4 different types of serial ports, for a
> total of 18 ports, and the two ports on a board labeled 0 and 1 may not
> correspond to the physical first two ports (what's "first" in a collection of
> 4 different types?).
> 
> Aliases may be fine for referring to the main serial console (labeled
> port 0 on the device, too), and the primary Ethernet interface (so U-Boot
> knows where to add the "local-mac-address" property), but beyond that,
> I think they should be avoided.

That's fair enough. Just because the solution isn't an all-purpose tool
doesn't mean it shouldn't be documented. The general concept is already
in ePAPR, but it's just not very specific about property names.

> Just my two^H^H^Hfive €c.

Thanks,
Brian

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dt-bindings: Add bindings for aliases node
  2018-10-09  5:57 ` Stephen Boyd
@ 2018-10-11 23:50   ` Matthias Kaehlcke
  0 siblings, 0 replies; 9+ messages in thread
From: Matthias Kaehlcke @ 2018-10-11 23:50 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Mark Rutland, Rob Herring, devicetree, linux-kernel, Brian Norris

On Mon, Oct 08, 2018 at 10:57:24PM -0700, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2018-09-25 14:02:55)
> > Add a global binding for the 'aliases' node. This includes an initial list
> > of standardized alias names for some hardware components that are commonly
> > found in 'aliases'.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> >  Documentation/devicetree/bindings/aliases.txt | 47 +++++++++++++++++++
> 
> Any chance we can get a documentation update for the devicetree spec as
> well?

I'm not involved in the DT spec process, but I'd certainly welcome an
update ;-)

> >  1 file changed, 47 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/aliases.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/aliases.txt b/Documentation/devicetree/bindings/aliases.txt
> > new file mode 100644
> > index 000000000000..d64ed1c7eb34
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/aliases.txt
> > @@ -0,0 +1,47 @@
> > +The aliases node
> > +----------------
> > +
> > +The aliases node contains properties that represent aliases to device tree
> > +nodes. The name of the property is the alias name, the value is the path of
> > +a the device tree node that corresponds to the alias. The path may be
> 
> s/a //

ack

> > +specified as a string or a phandle.
> > +
> > +Alias names are often suffixed with a numeric ID, especially when there may
> > +be multiple instances of the same type. The ID typically corresponds to the
> > +hardware layout, it may also be used by drivers for a stable mapping of
> > +device names and hardware entities.
> 
> Indicate the numeric ID is a u32 or something like that?

ok

> > +
> > +Alias names
> > +-----------
> > +
> > +The devicetree specification doesn't require the use of specific alias
> > +names to refer to hardware entities of a given type, however the Linux
> > +kernel aims for a certain level of consistency.
> > +
> > +The following standardized alias names shall be used for their
> > +corresponding hardware components:
> > +
> > +  bluetoothN           Bluetooth controller
> 
> Can we use syntax like:
> 
> 	bluetooth<N>
> 
> to indicate that <N> is an unsigned integer?

sounds good

Thanks for the review!

Matthias

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dt-bindings: Add bindings for aliases node
  2018-10-09 18:31     ` Brian Norris
@ 2018-10-12  0:08       ` Matthias Kaehlcke
  2018-10-12 16:31         ` Christian Lamparter
  2018-10-15 18:00       ` Rob Herring
  1 sibling, 1 reply; 9+ messages in thread
From: Matthias Kaehlcke @ 2018-10-12  0:08 UTC (permalink / raw)
  To: Brian Norris
  Cc: Geert Uytterhoeven, Rob Herring, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List, linux-wireless, linux-spi, netdev,
	swboyd, Florian Fainelli

On Tue, Oct 09, 2018 at 11:31:42AM -0700, Brian Norris wrote:
> On Tue, Oct 09, 2018 at 09:22:07AM +0200, Geert Uytterhoeven wrote:
> > Please note these aliases become cumbersome once you start considering
> > (dynamic) DT overlays.  That's why I made them optional in the sh-sci
> > serial driver, cfr. commit 7678f4c20fa7670f ("serial: sh-sci: Add support
> > for dynamic instances").
> 
> Note that as I understand it, the entire point of documenting this sort
> of thing is to help solidify the interface between a DT aware boot
> program (e.g., bootloader) and a device tree which is provided
> separately, to avoid memorizing node/path hierarchy. It doesn't need to
> (and doesn't, as I read it) enforce an OS's device naming policy.
> 
> > Relevant parts of the commit description are:
> > 
> >     On DT platforms, the sh-sci driver requires the presence of "serialN"
> >     aliases in DT, from which instance IDs are derived.  If a DT alias is
> >     missing, the drivers fails to probe the corresponding serial port.
> > 
> >     This becomes cumbersome when considering DT overlays, as currently
> >     there is no upstream support for dynamically updating the /aliases node
> >     in DT.
> 
> That part is not a DT spec problem :)
> 
> >     Furthermore, even in the presence of such support, hardcoded
> >     instance IDs in independent overlays are prone to conflicts.
> > 
> >     Hence add support for dynamic instance IDs, to be used in the absence of
> >     a DT alias.  This makes serial ports behave similar to I2C and SPI
> >     buses, which already support dynamic instances.
> 
> This seems to be a much different sort of problem. People always love
> having predictable IDs given by the OS (myself included), but that's
> just plain hard to do and impossible in some cases. I don't think that's
> what this document is about though.
> 
> IOW, this document seems pretty consistent with the above: it doesn't
> require the usage of aliases (and it seems silly to have a driver
> *require* an alias) -- it just documents how one should name such an
> alias if you expect multiple independent software components to
> understand it.
> 
> > To clarify my point: R-Car M2-W has 4 different types of serial ports, for a
> > total of 18 ports, and the two ports on a board labeled 0 and 1 may not
> > correspond to the physical first two ports (what's "first" in a collection of
> > 4 different types?).
> > 
> > Aliases may be fine for referring to the main serial console (labeled
> > port 0 on the device, too), and the primary Ethernet interface (so U-Boot
> > knows where to add the "local-mac-address" property), but beyond that,
> > I think they should be avoided.
> 
> That's fair enough. Just because the solution isn't an all-purpose tool
> doesn't mean it shouldn't be documented. The general concept is already
> in ePAPR, but it's just not very specific about property names.

Basically what Brian said, this doc doesn't encourage the use of
aliases, it just intends to establish a consistent naming for cases
where aliases are needed/more useful than harmful. The misuse of
aliases needs to be addressed in the reviews of the patches that
introduce them.

Maybe the doc should include a recommendation to use aliases
sparingly? I'm open to input on that from folks who have a better
understanding of the potential pitfalls ;-)

Cheers

Matthias

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dt-bindings: Add bindings for aliases node
  2018-10-12  0:08       ` Matthias Kaehlcke
@ 2018-10-12 16:31         ` Christian Lamparter
  0 siblings, 0 replies; 9+ messages in thread
From: Christian Lamparter @ 2018-10-12 16:31 UTC (permalink / raw)
  To: Matthias Kaehlcke
  Cc: Brian Norris, Geert Uytterhoeven, Rob Herring, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List, linux-wireless, linux-spi, netdev,
	swboyd, Florian Fainelli

On Friday, October 12, 2018 2:08:37 AM CEST Matthias Kaehlcke wrote:
> Maybe the doc should include a recommendation to use aliases
> sparingly? I'm open to input on that from folks who have a better
> understanding of the potential pitfalls 

I had a similar discussion with the OpenWrt devs over the
use of "led-$function" aliases in a DTS. I did a bit of digging and
found this wonderful emails from Mark Rutland regarding the general
use and abuse of aliases in a reply to a patch by Christer Weinigel 
"devicetree - document using aliases to set spi bus number."

<https://patchwork.kernel.org/patch/9133903/#19207021>

|"If those ports are physically organised and labelled the same, then
|using aliases could make sense, to describe the well-defined physical
|labels. If you've assigned the numbers artificially, or if the physical
|organisation differs across boards, then aliases are not the right tool
|for the job.
|
|In the latter cases we're altering the hardware description to suit an
|application, rather than providing the necessary abstraction, which is
|the kind of (ab)use of aliases which we want to avoid."

And he followed it up with a summary:
<https://patchwork.kernel.org/comment/19207071/>

|Typically, serial ports are much more user-accessible (physically), and
|much more directly useful to a user in a generic fashion. They're often
|labelled (physically or in a manual) with a number, and we use aliases
|to describe those labels to the kernel. The fact that the kernel may use
|that to drive its own internal numbering is immaterial to the binding.

So the gist of this is that aliases are meant for user-accessible /
physically devices/ports/etc... that are labeled as such. And this of
course works perfectly for power/status LEDs and such because they
usually have little "power" symbols/pictograms/lables near them. 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] dt-bindings: Add bindings for aliases node
  2018-10-09 18:31     ` Brian Norris
  2018-10-12  0:08       ` Matthias Kaehlcke
@ 2018-10-15 18:00       ` Rob Herring
  1 sibling, 0 replies; 9+ messages in thread
From: Rob Herring @ 2018-10-15 18:00 UTC (permalink / raw)
  To: Brian Norris
  Cc: Geert Uytterhoeven, Matthias Kaehlcke, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Kernel Mailing List, linux-wireless, linux-spi, netdev,
	swboyd, Florian Fainelli

On Tue, Oct 09, 2018 at 11:31:42AM -0700, Brian Norris wrote:
> On Tue, Oct 09, 2018 at 09:22:07AM +0200, Geert Uytterhoeven wrote:
> > Please note these aliases become cumbersome once you start considering
> > (dynamic) DT overlays.  That's why I made them optional in the sh-sci
> > serial driver, cfr. commit 7678f4c20fa7670f ("serial: sh-sci: Add support
> > for dynamic instances").
> 
> Note that as I understand it, the entire point of documenting this sort
> of thing is to help solidify the interface between a DT aware boot
> program (e.g., bootloader) and a device tree which is provided
> separately, to avoid memorizing node/path hierarchy. It doesn't need to
> (and doesn't, as I read it) enforce an OS's device naming policy.

I'm all for documenting this primarily to prevent folks from just adding 
whatever they wish in /aliases. Some platforms seem to want to have 
aliases for everything.

> > Relevant parts of the commit description are:
> > 
> >     On DT platforms, the sh-sci driver requires the presence of "serialN"
> >     aliases in DT, from which instance IDs are derived.  If a DT alias is
> >     missing, the drivers fails to probe the corresponding serial port.
> > 
> >     This becomes cumbersome when considering DT overlays, as currently
> >     there is no upstream support for dynamically updating the /aliases node
> >     in DT.
> 
> That part is not a DT spec problem :)
> 
> >     Furthermore, even in the presence of such support, hardcoded
> >     instance IDs in independent overlays are prone to conflicts.
> > 
> >     Hence add support for dynamic instance IDs, to be used in the absence of
> >     a DT alias.  This makes serial ports behave similar to I2C and SPI
> >     buses, which already support dynamic instances.
> 
> This seems to be a much different sort of problem. People always love
> having predictable IDs given by the OS (myself included), but that's
> just plain hard to do and impossible in some cases. I don't think that's
> what this document is about though.
> 
> IOW, this document seems pretty consistent with the above: it doesn't
> require the usage of aliases (and it seems silly to have a driver
> *require* an alias) -- it just documents how one should name such an
> alias if you expect multiple independent software components to
> understand it.
> 
> > To clarify my point: R-Car M2-W has 4 different types of serial ports, for a
> > total of 18 ports, and the two ports on a board labeled 0 and 1 may not
> > correspond to the physical first two ports (what's "first" in a collection of
> > 4 different types?).
> > 
> > Aliases may be fine for referring to the main serial console (labeled
> > port 0 on the device, too), and the primary Ethernet interface (so U-Boot
> > knows where to add the "local-mac-address" property), but beyond that,
> > I think they should be avoided.

This basically matches my opinion on aliases.
 
I'd decouple it from board labels a bit. Sometimes the numbering may 
match, but others not. What if a board serial port is labeled "DBG" for 
example? I think 'label' is the right way to handle human identifible 
ports (and then we should have something like /dev/serial/by-label/...).

> That's fair enough. Just because the solution isn't an all-purpose tool
> doesn't mean it shouldn't be documented. The general concept is already
> in ePAPR, but it's just not very specific about property names.

Agreed. I guess the question is what to do on used, but not recommended 
aliases. I would put SPI and I2C into that category BTW.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-10-15 18:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-25 21:02 [PATCH] dt-bindings: Add bindings for aliases node Matthias Kaehlcke
2018-10-09  1:07 ` Brian Norris
2018-10-09  7:22   ` Geert Uytterhoeven
2018-10-09 18:31     ` Brian Norris
2018-10-12  0:08       ` Matthias Kaehlcke
2018-10-12 16:31         ` Christian Lamparter
2018-10-15 18:00       ` Rob Herring
2018-10-09  5:57 ` Stephen Boyd
2018-10-11 23:50   ` Matthias Kaehlcke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).