* [PATCH v2] drivers: serial: probe all uart devices
@ 2020-09-29 17:26 Vabhav Sharma
2020-09-30 5:15 ` Stefan Roese
2020-09-30 16:44 ` Simon Glass
0 siblings, 2 replies; 8+ messages in thread
From: Vabhav Sharma @ 2020-09-29 17:26 UTC (permalink / raw)
To: u-boot
From: Vabhav Sharma <vabhav.sharma@nxp.com>
U-Boot DM model probe only single device at a time
which is enabled and configured using device tree
or platform data method.
PL011 UART IP is SBSA compliant and firmware does the
serial port set-up, initialization and let the kernel use
UART port for sending and receiving characters.
Normally software talk to one serial port time but some
LayerScape platform require all the UART devices enabled
in Linux for various use case.
Adding support to probe all enabled serial devices like SBSA
compliant PL011 UART ports probe and initialization by firmware.
Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
---
v2:
Incorporated Stefan review comment, Update #ifdef with macro
if (IS_ENABLED)..
---
---
drivers/serial/Kconfig | 17 +++++++++++++++++
drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index e344677..b2e30f1 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
If unsure, say N.
+config SERIAL_PROBE_ALL
+ bool "Probe all available serial devices"
+ depends on DM_SERIAL
+ default n
+ help
+ The serial subsystem only probe for single serial device,
+ but does not probe for other remaining serial devices.
+ With this option set,we make probing and searching for
+ all available devices optional.
+ Normally, U-Boot talk to one serial port at a time but SBSA
+ compliant UART devices like PL011 require initialization
+ by firmware and let the kernel use serial port for sending
+ and receiving the characters.
+
+ If probing is not required for all remaining available
+ devices other than default current console device, say N.
+
config SPL_DM_SERIAL
bool "Enable Driver Model for serial drivers in SPL"
depends on DM_SERIAL && SPL_DM
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 0027625..d303a66 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
uclass_first_device(UCLASS_SERIAL, &dev);
if (dev) {
gd->cur_serial_dev = dev;
+ if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
+ /* Scanning uclass to probe all devices */
+ for (; dev; uclass_next_device(&dev))
+ ;
+ }
return;
}
} else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
@@ -96,11 +101,21 @@ static void serial_find_console_or_panic(void)
if (np && !uclass_get_device_by_ofnode(UCLASS_SERIAL,
np_to_ofnode(np), &dev)) {
gd->cur_serial_dev = dev;
+ if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
+ /* Scanning uclass to probe devices */
+ for (; dev; uclass_next_device(&dev))
+ ;
+ }
return;
}
} else {
if (!serial_check_stdout(blob, &dev)) {
gd->cur_serial_dev = dev;
+ if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
+ /* Scanning uclass to probe devices */
+ for (; dev; uclass_next_device(&dev))
+ ;
+ }
return;
}
}
@@ -125,6 +140,11 @@ static void serial_find_console_or_panic(void)
!uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
if (dev->flags & DM_FLAG_ACTIVATED) {
gd->cur_serial_dev = dev;
+ if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
+ /* Scanning uclass to probe devices */
+ for (; dev; uclass_next_device(&dev))
+ ;
+ }
return;
}
}
@@ -136,6 +156,11 @@ static void serial_find_console_or_panic(void)
if (!ret) {
/* Device did succeed probing */
gd->cur_serial_dev = dev;
+ if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
+ /* Scanning uclass to probe devices */
+ for (; dev; uclass_next_device(&dev))
+ ;
+ }
return;
}
}
@@ -144,6 +169,11 @@ static void serial_find_console_or_panic(void)
!uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
(!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
gd->cur_serial_dev = dev;
+ if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
+ /* Scanning uclass to probe all devices */
+ for (; dev; uclass_next_device(&dev))
+ ;
+ }
return;
}
#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2] drivers: serial: probe all uart devices
2020-09-29 17:26 [PATCH v2] drivers: serial: probe all uart devices Vabhav Sharma
@ 2020-09-30 5:15 ` Stefan Roese
2020-10-14 11:15 ` Vabhav Sharma
2020-09-30 16:44 ` Simon Glass
1 sibling, 1 reply; 8+ messages in thread
From: Stefan Roese @ 2020-09-30 5:15 UTC (permalink / raw)
To: u-boot
On 29.09.20 19:26, Vabhav Sharma wrote:
> From: Vabhav Sharma <vabhav.sharma@nxp.com>
>
> U-Boot DM model probe only single device at a time
> which is enabled and configured using device tree
> or platform data method.
>
> PL011 UART IP is SBSA compliant and firmware does the
> serial port set-up, initialization and let the kernel use
> UART port for sending and receiving characters.
>
> Normally software talk to one serial port time but some
> LayerScape platform require all the UART devices enabled
> in Linux for various use case.
>
> Adding support to probe all enabled serial devices like SBSA
> compliant PL011 UART ports probe and initialization by firmware.
>
> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> ---
> v2:
> Incorporated Stefan review comment, Update #ifdef with macro
> if (IS_ENABLED)..
Better, thanks. But...
> ---
> ---
> drivers/serial/Kconfig | 17 +++++++++++++++++
> drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index e344677..b2e30f1 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
>
> If unsure, say N.
>
> +config SERIAL_PROBE_ALL
> + bool "Probe all available serial devices"
> + depends on DM_SERIAL
> + default n
> + help
> + The serial subsystem only probe for single serial device,
> + but does not probe for other remaining serial devices.
> + With this option set,we make probing and searching for
> + all available devices optional.
> + Normally, U-Boot talk to one serial port at a time but SBSA
> + compliant UART devices like PL011 require initialization
> + by firmware and let the kernel use serial port for sending
> + and receiving the characters.
> +
> + If probing is not required for all remaining available
> + devices other than default current console device, say N.
> +
> config SPL_DM_SERIAL
> bool "Enable Driver Model for serial drivers in SPL"
> depends on DM_SERIAL && SPL_DM
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 0027625..d303a66 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
> uclass_first_device(UCLASS_SERIAL, &dev);
> if (dev) {
> gd->cur_serial_dev = dev;
> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> + /* Scanning uclass to probe all devices */
> + for (; dev; uclass_next_device(&dev))
> + ;
> + }
> return;
> }
> } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
> @@ -96,11 +101,21 @@ static void serial_find_console_or_panic(void)
> if (np && !uclass_get_device_by_ofnode(UCLASS_SERIAL,
> np_to_ofnode(np), &dev)) {
> gd->cur_serial_dev = dev;
> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> + /* Scanning uclass to probe devices */
> + for (; dev; uclass_next_device(&dev))
> + ;
> + }
This code sequence (incl. gd->cur_serial_dev = dev;) is repeated
multiple times (below as well). Could you instead move this into
a function and call this function to reduce code and binary size?
Thanks,
Stefan
> return;
> }
> } else {
> if (!serial_check_stdout(blob, &dev)) {
> gd->cur_serial_dev = dev;
> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> + /* Scanning uclass to probe devices */
> + for (; dev; uclass_next_device(&dev))
> + ;
> + }
> return;
> }
> }
> @@ -125,6 +140,11 @@ static void serial_find_console_or_panic(void)
> !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
> if (dev->flags & DM_FLAG_ACTIVATED) {
> gd->cur_serial_dev = dev;
> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> + /* Scanning uclass to probe devices */
> + for (; dev; uclass_next_device(&dev))
> + ;
> + }
> return;
> }
> }
> @@ -136,6 +156,11 @@ static void serial_find_console_or_panic(void)
> if (!ret) {
> /* Device did succeed probing */
> gd->cur_serial_dev = dev;
> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> + /* Scanning uclass to probe devices */
> + for (; dev; uclass_next_device(&dev))
> + ;
> + }
> return;
> }
> }
> @@ -144,6 +169,11 @@ static void serial_find_console_or_panic(void)
> !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
> (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
> gd->cur_serial_dev = dev;
> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> + /* Scanning uclass to probe all devices */
> + for (; dev; uclass_next_device(&dev))
> + ;
> + }
> return;
> }
> #endif
>
Viele Gr??e,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] drivers: serial: probe all uart devices
2020-09-29 17:26 [PATCH v2] drivers: serial: probe all uart devices Vabhav Sharma
2020-09-30 5:15 ` Stefan Roese
@ 2020-09-30 16:44 ` Simon Glass
2020-10-14 11:46 ` Vabhav Sharma
1 sibling, 1 reply; 8+ messages in thread
From: Simon Glass @ 2020-09-30 16:44 UTC (permalink / raw)
To: u-boot
Hi Vabhav,
On Tue, 29 Sep 2020 at 11:33, Vabhav Sharma <vabhav.sharma@oss.nxp.com> wrote:
>
> From: Vabhav Sharma <vabhav.sharma@nxp.com>
>
> U-Boot DM model probe only single device at a time
> which is enabled and configured using device tree
> or platform data method.
>
> PL011 UART IP is SBSA compliant and firmware does the
> serial port set-up, initialization and let the kernel use
> UART port for sending and receiving characters.
>
> Normally software talk to one serial port time but some
> LayerScape platform require all the UART devices enabled
> in Linux for various use case.
>
> Adding support to probe all enabled serial devices like SBSA
> compliant PL011 UART ports probe and initialization by firmware.
>
> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> ---
> v2:
> Incorporated Stefan review comment, Update #ifdef with macro
> if (IS_ENABLED)..
> ---
> ---
> drivers/serial/Kconfig | 17 +++++++++++++++++
> drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 47 insertions(+)
>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index e344677..b2e30f1 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
>
> If unsure, say N.
>
> +config SERIAL_PROBE_ALL
> + bool "Probe all available serial devices"
> + depends on DM_SERIAL
> + default n
not needed
> + help
> + The serial subsystem only probe for single serial device,
> + but does not probe for other remaining serial devices.
> + With this option set,we make probing and searching for
> + all available devices optional.
> + Normally, U-Boot talk to one serial port at a time but SBSA
> + compliant UART devices like PL011 require initialization
> + by firmware and let the kernel use serial port for sending
> + and receiving the characters.
> +
> + If probing is not required for all remaining available
> + devices other than default current console device, say N.
> +
> config SPL_DM_SERIAL
> bool "Enable Driver Model for serial drivers in SPL"
> depends on DM_SERIAL && SPL_DM
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 0027625..d303a66 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
> uclass_first_device(UCLASS_SERIAL, &dev);
> if (dev) {
> gd->cur_serial_dev = dev;
> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> + /* Scanning uclass to probe all devices */
> + for (; dev; uclass_next_device(&dev))
> + ;
Please put this into a function in drivers/core/uclass.c since this is
a useful function. E.g. uclass_probe_all(enum uclass_id)
Also you could use device_foreach_child_probe().
Can you put this in the only caller of serial_find_console_or_panic() instead?
Do you ever have a situation where there is no serial console in
U-Boot but you still want to probe everything? If so, a better place
might be at the end of dm_init_and_scan().
Finally, do you want to do this in SPL as well, or is it enough to
just do it in U-Boot proper? If the latter you could use
if (CONFIG_IS_ENABLED(SERIAL_PROBE_ALL)) {
// do it
}
Regards,
Simon,
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] drivers: serial: probe all uart devices
2020-09-30 5:15 ` Stefan Roese
@ 2020-10-14 11:15 ` Vabhav Sharma
2020-10-14 12:46 ` Stefan Roese
0 siblings, 1 reply; 8+ messages in thread
From: Vabhav Sharma @ 2020-10-14 11:15 UTC (permalink / raw)
To: u-boot
Hi Stefan,
Sorry for delayed reply, Occupied with high priority task
> -----Original Message-----
> From: Stefan Roese <sr@denx.de>
> Sent: Wednesday, September 30, 2020 10:46 AM
> To: Vabhav Sharma (OSS) <vabhav.sharma@oss.nxp.com>;
> andre.przywara at arm.com; u-boot at lists.denx.de; sjg at chromium.org
> Cc: Vabhav Sharma <vabhav.sharma@nxp.com>
> Subject: Re: [PATCH v2] drivers: serial: probe all uart devices
>
> On 29.09.20 19:26, Vabhav Sharma wrote:
> > From: Vabhav Sharma <vabhav.sharma@nxp.com>
> >
> > U-Boot DM model probe only single device at a time which is enabled
> > and configured using device tree or platform data method.
> >
> > PL011 UART IP is SBSA compliant and firmware does the serial port
> > set-up, initialization and let the kernel use UART port for sending
> > and receiving characters.
> >
> > Normally software talk to one serial port time but some LayerScape
> > platform require all the UART devices enabled in Linux for various use
> > case.
> >
> > Adding support to probe all enabled serial devices like SBSA compliant
> > PL011 UART ports probe and initialization by firmware.
> >
> > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> > ---
> > v2:
> > Incorporated Stefan review comment, Update #ifdef with macro
> > if (IS_ENABLED)..
>
> Better, thanks. But...
>
> > ---
> > ---
> > drivers/serial/Kconfig | 17 +++++++++++++++++
> > drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
> > 2 files changed, 47 insertions(+)
> >
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index
> > e344677..b2e30f1 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
> >
> > If unsure, say N.
> >
> > +config SERIAL_PROBE_ALL
> > + bool "Probe all available serial devices"
> > + depends on DM_SERIAL
> > + default n
> > + help
> > + The serial subsystem only probe for single serial device,
> > + but does not probe for other remaining serial devices.
> > + With this option set,we make probing and searching for
> > + all available devices optional.
> > + Normally, U-Boot talk to one serial port at a time but SBSA
> > + compliant UART devices like PL011 require initialization
> > + by firmware and let the kernel use serial port for sending
> > + and receiving the characters.
> > +
> > + If probing is not required for all remaining available
> > + devices other than default current console device, say N.
> > +
> > config SPL_DM_SERIAL
> > bool "Enable Driver Model for serial drivers in SPL"
> > depends on DM_SERIAL && SPL_DM
> > diff --git a/drivers/serial/serial-uclass.c
> > b/drivers/serial/serial-uclass.c index 0027625..d303a66 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
> > uclass_first_device(UCLASS_SERIAL, &dev);
> > if (dev) {
> > gd->cur_serial_dev = dev;
> > + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> > + /* Scanning uclass to probe all devices */
> > + for (; dev; uclass_next_device(&dev))
> > + ;
> > + }
> > return;
> > }
> > } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -96,11
> > +101,21 @@ static void serial_find_console_or_panic(void)
> > if (np
> && !uclass_get_device_by_ofnode(UCLASS_SERIAL,
> > np_to_ofnode(np), &dev)) {
> > gd->cur_serial_dev = dev;
> > + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> {
> > + /* Scanning uclass to probe devices
> */
> > + for (; dev; uclass_next_device(&dev))
> > + ;
> > + }
>
> This code sequence (incl. gd->cur_serial_dev = dev;) is repeated multiple
> times (below as well). Could you instead move this into a function and call
> this function to reduce code and binary size?
I understand.
Checked and found that 56 bytes of code is added (including enabling the macro on LS platform)
Intended to define it earlier but defining one line of code in a function cause more overhead (function call, Stack operations, Pointer arithmetic) in execution time.
Tradeoff is to choose between Function overhead Vs Binary Size, What is your suggestion
>
> Thanks,
> Stefan
>
> > return;
> > }
> > } else {
> > if (!serial_check_stdout(blob, &dev)) {
> > gd->cur_serial_dev = dev;
> > + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> {
> > + /* Scanning uclass to probe devices
> */
> > + for (; dev; uclass_next_device(&dev))
> > + ;
> > + }
> > return;
> > }
> > }
> > @@ -125,6 +140,11 @@ static void serial_find_console_or_panic(void)
> > !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
> > if (dev->flags & DM_FLAG_ACTIVATED) {
> > gd->cur_serial_dev = dev;
> > + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> {
> > + /* Scanning uclass to probe devices
> */
> > + for (; dev; uclass_next_device(&dev))
> > + ;
> > + }
> > return;
> > }
> > }
> > @@ -136,6 +156,11 @@ static void serial_find_console_or_panic(void)
> > if (!ret) {
> > /* Device did succeed probing */
> > gd->cur_serial_dev = dev;
> > + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> {
> > + /* Scanning uclass to probe devices
> */
> > + for (; dev; uclass_next_device(&dev))
> > + ;
> > + }
> > return;
> > }
> > }
> > @@ -144,6 +169,11 @@ static void serial_find_console_or_panic(void)
> > !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
> > (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
> > gd->cur_serial_dev = dev;
> > + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> > + /* Scanning uclass to probe all devices */
> > + for (; dev; uclass_next_device(&dev))
> > + ;
> > + }
> > return;
> > }
> > #endif
> >
>
>
> Viele Gr??e,
> Stefan
>
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] drivers: serial: probe all uart devices
2020-09-30 16:44 ` Simon Glass
@ 2020-10-14 11:46 ` Vabhav Sharma
0 siblings, 0 replies; 8+ messages in thread
From: Vabhav Sharma @ 2020-10-14 11:46 UTC (permalink / raw)
To: u-boot
Hi Simon,
Apology for delayed reply, Got occupied due to other business deliverables
> -----Original Message-----
> From: Simon Glass <sjg@chromium.org>
> Sent: Wednesday, September 30, 2020 10:15 PM
> To: Vabhav Sharma (OSS) <vabhav.sharma@oss.nxp.com>
> Cc: Andre Przywara <andre.przywara@arm.com>; U-Boot Mailing List <u-
> boot at lists.denx.de>; Stefan Roese <sr@denx.de>; Vabhav Sharma
> <vabhav.sharma@nxp.com>
> Subject: Re: [PATCH v2] drivers: serial: probe all uart devices
>
> Hi Vabhav,
>
> On Tue, 29 Sep 2020 at 11:33, Vabhav Sharma <vabhav.sharma@oss.nxp.com>
> wrote:
> >
> > From: Vabhav Sharma <vabhav.sharma@nxp.com>
> >
> > U-Boot DM model probe only single device at a time which is enabled
> > and configured using device tree or platform data method.
> >
> > PL011 UART IP is SBSA compliant and firmware does the serial port
> > set-up, initialization and let the kernel use UART port for sending
> > and receiving characters.
> >
> > Normally software talk to one serial port time but some LayerScape
> > platform require all the UART devices enabled in Linux for various use
> > case.
> >
> > Adding support to probe all enabled serial devices like SBSA compliant
> > PL011 UART ports probe and initialization by firmware.
> >
> > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> > ---
> > v2:
> > Incorporated Stefan review comment, Update #ifdef with macro
> > if (IS_ENABLED)..
> > ---
> > ---
> > drivers/serial/Kconfig | 17 +++++++++++++++++
> > drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
> > 2 files changed, 47 insertions(+)
> >
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index
> > e344677..b2e30f1 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
> >
> > If unsure, say N.
> >
> > +config SERIAL_PROBE_ALL
> > + bool "Probe all available serial devices"
> > + depends on DM_SERIAL
> > + default n
>
> not needed
>
> > + help
> > + The serial subsystem only probe for single serial device,
> > + but does not probe for other remaining serial devices.
> > + With this option set,we make probing and searching for
> > + all available devices optional.
> > + Normally, U-Boot talk to one serial port at a time but SBSA
> > + compliant UART devices like PL011 require initialization
> > + by firmware and let the kernel use serial port for sending
> > + and receiving the characters.
> > +
> > + If probing is not required for all remaining available
> > + devices other than default current console device, say N.
> > +
> > config SPL_DM_SERIAL
> > bool "Enable Driver Model for serial drivers in SPL"
> > depends on DM_SERIAL && SPL_DM diff --git
> > a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> > index 0027625..d303a66 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
> > uclass_first_device(UCLASS_SERIAL, &dev);
> > if (dev) {
> > gd->cur_serial_dev = dev;
> > + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> > + /* Scanning uclass to probe all devices */
> > + for (; dev; uclass_next_device(&dev))
> > + ;
>
> Please put this into a function in drivers/core/uclass.c since this is a useful
> function. E.g. uclass_probe_all(enum uclass_id)
>
> Also you could use device_foreach_child_probe().
>
> Can you put this in the only caller of serial_find_console_or_panic() instead?
>
Ok, Let me check the above the above functions feasibility to probe serial ports
> Do you ever have a situation where there is no serial console in U-Boot but
> you still want to probe everything? If so, a better place might be at the end
> of dm_init_and_scan().
I see, Situation is rare but possible as SBSA compliance required initialization by firmware.
>
> Finally, do you want to do this in SPL as well, or is it enough to just do it in U-
> Boot proper? If the latter you could use
>
> if (CONFIG_IS_ENABLED(SERIAL_PROBE_ALL)) { // do it } Regards, Simon,
Ok, Both are possible as per the requirement.
Current changes target PL011 UART IP which is SBSA compliant and accordingly used Serial uclass functions to do the needful.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] drivers: serial: probe all uart devices
2020-10-14 11:15 ` Vabhav Sharma
@ 2020-10-14 12:46 ` Stefan Roese
2020-10-15 15:05 ` Simon Glass
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Roese @ 2020-10-14 12:46 UTC (permalink / raw)
To: u-boot
Hi Vabhav,
On 14.10.20 13:15, Vabhav Sharma (OSS) wrote:
> Hi Stefan,
> Sorry for delayed reply, Occupied with high priority task
>
>> -----Original Message-----
>> From: Stefan Roese <sr@denx.de>
>> Sent: Wednesday, September 30, 2020 10:46 AM
>> To: Vabhav Sharma (OSS) <vabhav.sharma@oss.nxp.com>;
>> andre.przywara at arm.com; u-boot at lists.denx.de; sjg at chromium.org
>> Cc: Vabhav Sharma <vabhav.sharma@nxp.com>
>> Subject: Re: [PATCH v2] drivers: serial: probe all uart devices
>>
>> On 29.09.20 19:26, Vabhav Sharma wrote:
>>> From: Vabhav Sharma <vabhav.sharma@nxp.com>
>>>
>>> U-Boot DM model probe only single device at a time which is enabled
>>> and configured using device tree or platform data method.
>>>
>>> PL011 UART IP is SBSA compliant and firmware does the serial port
>>> set-up, initialization and let the kernel use UART port for sending
>>> and receiving characters.
>>>
>>> Normally software talk to one serial port time but some LayerScape
>>> platform require all the UART devices enabled in Linux for various use
>>> case.
>>>
>>> Adding support to probe all enabled serial devices like SBSA compliant
>>> PL011 UART ports probe and initialization by firmware.
>>>
>>> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
>>> ---
>>> v2:
>>> Incorporated Stefan review comment, Update #ifdef with macro
>>> if (IS_ENABLED)..
>>
>> Better, thanks. But...
>>
>>> ---
>>> ---
>>> drivers/serial/Kconfig | 17 +++++++++++++++++
>>> drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
>>> 2 files changed, 47 insertions(+)
>>>
>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index
>>> e344677..b2e30f1 100644
>>> --- a/drivers/serial/Kconfig
>>> +++ b/drivers/serial/Kconfig
>>> @@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
>>>
>>> If unsure, say N.
>>>
>>> +config SERIAL_PROBE_ALL
>>> + bool "Probe all available serial devices"
>>> + depends on DM_SERIAL
>>> + default n
>>> + help
>>> + The serial subsystem only probe for single serial device,
>>> + but does not probe for other remaining serial devices.
>>> + With this option set,we make probing and searching for
>>> + all available devices optional.
>>> + Normally, U-Boot talk to one serial port at a time but SBSA
>>> + compliant UART devices like PL011 require initialization
>>> + by firmware and let the kernel use serial port for sending
>>> + and receiving the characters.
>>> +
>>> + If probing is not required for all remaining available
>>> + devices other than default current console device, say N.
>>> +
>>> config SPL_DM_SERIAL
>>> bool "Enable Driver Model for serial drivers in SPL"
>>> depends on DM_SERIAL && SPL_DM
>>> diff --git a/drivers/serial/serial-uclass.c
>>> b/drivers/serial/serial-uclass.c index 0027625..d303a66 100644
>>> --- a/drivers/serial/serial-uclass.c
>>> +++ b/drivers/serial/serial-uclass.c
>>> @@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
>>> uclass_first_device(UCLASS_SERIAL, &dev);
>>> if (dev) {
>>> gd->cur_serial_dev = dev;
>>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
>>> + /* Scanning uclass to probe all devices */
>>> + for (; dev; uclass_next_device(&dev))
>>> + ;
>>> + }
>>> return;
>>> }
>>> } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -96,11
>>> +101,21 @@ static void serial_find_console_or_panic(void)
>>> if (np
>> && !uclass_get_device_by_ofnode(UCLASS_SERIAL,
>>> np_to_ofnode(np), &dev)) {
>>> gd->cur_serial_dev = dev;
>>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
>> {
>>> + /* Scanning uclass to probe devices
>> */
>>> + for (; dev; uclass_next_device(&dev))
>>> + ;
>>> + }
>>
>> This code sequence (incl. gd->cur_serial_dev = dev;) is repeated multiple
>> times (below as well). Could you instead move this into a function and call
>> this function to reduce code and binary size?
> I understand.
> Checked and found that 56 bytes of code is added (including enabling
> the macro on LS platform)
>
> Intended to define it earlier but defining one line of code in a
> function cause more overhead (function call, Stack operations,
> Pointer arithmetic) in execution time.
So you did measure a penalty in bootup time with this code moved
into a function? I would have thought that this is neglectable.
> Tradeoff is to choose between Function overhead Vs Binary Size, What
> is your suggestion
My vote is for the function. Mainly not because of the smaller image
size, but because of the smaller source code base, being easier to
maintain (IMHO).
Thanks,
Stefan
>>
>> Thanks,
>> Stefan
>>
>>> return;
>>> }
>>> } else {
>>> if (!serial_check_stdout(blob, &dev)) {
>>> gd->cur_serial_dev = dev;
>>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
>> {
>>> + /* Scanning uclass to probe devices
>> */
>>> + for (; dev; uclass_next_device(&dev))
>>> + ;
>>> + }
>>> return;
>>> }
>>> }
>>> @@ -125,6 +140,11 @@ static void serial_find_console_or_panic(void)
>>> !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
>>> if (dev->flags & DM_FLAG_ACTIVATED) {
>>> gd->cur_serial_dev = dev;
>>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
>> {
>>> + /* Scanning uclass to probe devices
>> */
>>> + for (; dev; uclass_next_device(&dev))
>>> + ;
>>> + }
>>> return;
>>> }
>>> }
>>> @@ -136,6 +156,11 @@ static void serial_find_console_or_panic(void)
>>> if (!ret) {
>>> /* Device did succeed probing */
>>> gd->cur_serial_dev = dev;
>>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
>> {
>>> + /* Scanning uclass to probe devices
>> */
>>> + for (; dev; uclass_next_device(&dev))
>>> + ;
>>> + }
>>> return;
>>> }
>>> }
>>> @@ -144,6 +169,11 @@ static void serial_find_console_or_panic(void)
>>> !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
>>> (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
>>> gd->cur_serial_dev = dev;
>>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
>>> + /* Scanning uclass to probe all devices */
>>> + for (; dev; uclass_next_device(&dev))
>>> + ;
>>> + }
>>> return;
>>> }
>>> #endif
>>>
>>
>>
>> Viele Gr??e,
>> Stefan
>>
>> --
>> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
Viele Gr??e,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] drivers: serial: probe all uart devices
2020-10-14 12:46 ` Stefan Roese
@ 2020-10-15 15:05 ` Simon Glass
2020-10-19 8:19 ` Vabhav Sharma
0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2020-10-15 15:05 UTC (permalink / raw)
To: u-boot
Hi,
On Wed, 14 Oct 2020 at 06:47, Stefan Roese <sr@denx.de> wrote:
>
> Hi Vabhav,
>
> On 14.10.20 13:15, Vabhav Sharma (OSS) wrote:
> > Hi Stefan,
> > Sorry for delayed reply, Occupied with high priority task
> >
> >> -----Original Message-----
> >> From: Stefan Roese <sr@denx.de>
> >> Sent: Wednesday, September 30, 2020 10:46 AM
> >> To: Vabhav Sharma (OSS) <vabhav.sharma@oss.nxp.com>;
> >> andre.przywara at arm.com; u-boot at lists.denx.de; sjg at chromium.org
> >> Cc: Vabhav Sharma <vabhav.sharma@nxp.com>
> >> Subject: Re: [PATCH v2] drivers: serial: probe all uart devices
> >>
> >> On 29.09.20 19:26, Vabhav Sharma wrote:
> >>> From: Vabhav Sharma <vabhav.sharma@nxp.com>
> >>>
> >>> U-Boot DM model probe only single device at a time which is enabled
> >>> and configured using device tree or platform data method.
> >>>
> >>> PL011 UART IP is SBSA compliant and firmware does the serial port
> >>> set-up, initialization and let the kernel use UART port for sending
> >>> and receiving characters.
> >>>
> >>> Normally software talk to one serial port time but some LayerScape
> >>> platform require all the UART devices enabled in Linux for various use
> >>> case.
> >>>
> >>> Adding support to probe all enabled serial devices like SBSA compliant
> >>> PL011 UART ports probe and initialization by firmware.
> >>>
> >>> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> >>> ---
> >>> v2:
> >>> Incorporated Stefan review comment, Update #ifdef with macro
> >>> if (IS_ENABLED)..
> >>
> >> Better, thanks. But...
> >>
> >>> ---
> >>> ---
> >>> drivers/serial/Kconfig | 17 +++++++++++++++++
> >>> drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
> >>> 2 files changed, 47 insertions(+)
> >>>
> >>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index
> >>> e344677..b2e30f1 100644
> >>> --- a/drivers/serial/Kconfig
> >>> +++ b/drivers/serial/Kconfig
> >>> @@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
> >>>
> >>> If unsure, say N.
> >>>
> >>> +config SERIAL_PROBE_ALL
> >>> + bool "Probe all available serial devices"
> >>> + depends on DM_SERIAL
> >>> + default n
> >>> + help
> >>> + The serial subsystem only probe for single serial device,
> >>> + but does not probe for other remaining serial devices.
> >>> + With this option set,we make probing and searching for
> >>> + all available devices optional.
> >>> + Normally, U-Boot talk to one serial port at a time but SBSA
> >>> + compliant UART devices like PL011 require initialization
> >>> + by firmware and let the kernel use serial port for sending
> >>> + and receiving the characters.
> >>> +
> >>> + If probing is not required for all remaining available
> >>> + devices other than default current console device, say N.
> >>> +
> >>> config SPL_DM_SERIAL
> >>> bool "Enable Driver Model for serial drivers in SPL"
> >>> depends on DM_SERIAL && SPL_DM
> >>> diff --git a/drivers/serial/serial-uclass.c
> >>> b/drivers/serial/serial-uclass.c index 0027625..d303a66 100644
> >>> --- a/drivers/serial/serial-uclass.c
> >>> +++ b/drivers/serial/serial-uclass.c
> >>> @@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
> >>> uclass_first_device(UCLASS_SERIAL, &dev);
> >>> if (dev) {
> >>> gd->cur_serial_dev = dev;
> >>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> >>> + /* Scanning uclass to probe all devices */
> >>> + for (; dev; uclass_next_device(&dev))
> >>> + ;
> >>> + }
> >>> return;
> >>> }
> >>> } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -96,11
> >>> +101,21 @@ static void serial_find_console_or_panic(void)
> >>> if (np
> >> && !uclass_get_device_by_ofnode(UCLASS_SERIAL,
> >>> np_to_ofnode(np), &dev)) {
> >>> gd->cur_serial_dev = dev;
> >>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> >> {
> >>> + /* Scanning uclass to probe devices
> >> */
> >>> + for (; dev; uclass_next_device(&dev))
> >>> + ;
> >>> + }
> >>
> >> This code sequence (incl. gd->cur_serial_dev = dev;) is repeated multiple
> >> times (below as well). Could you instead move this into a function and call
> >> this function to reduce code and binary size?
> > I understand.
> > Checked and found that 56 bytes of code is added (including enabling
> > the macro on LS platform)
> >
> > Intended to define it earlier but defining one line of code in a
> > function cause more overhead (function call, Stack operations,
> > Pointer arithmetic) in execution time.
>
> So you did measure a penalty in bootup time with this code moved
> into a function? I would have thought that this is neglectable.
>
> > Tradeoff is to choose between Function overhead Vs Binary Size, What
> > is your suggestion
>
> My vote is for the function. Mainly not because of the smaller image
> size, but because of the smaller source code base, being easier to
> maintain (IMHO).
Same for me.
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] drivers: serial: probe all uart devices
2020-10-15 15:05 ` Simon Glass
@ 2020-10-19 8:19 ` Vabhav Sharma
0 siblings, 0 replies; 8+ messages in thread
From: Vabhav Sharma @ 2020-10-19 8:19 UTC (permalink / raw)
To: u-boot
Hi Simon, Stefan,
Thank you for feedback and time
> -----Original Message-----
> From: Simon Glass <sjg@chromium.org>
> Sent: Thursday, October 15, 2020 8:35 PM
> To: Stefan Roese <sr@denx.de>
> Cc: Vabhav Sharma (OSS) <vabhav.sharma@oss.nxp.com>;
> andre.przywara at arm.com; u-boot at lists.denx.de
> Subject: Re: [PATCH v2] drivers: serial: probe all uart devices
>
> Hi,
>
> On Wed, 14 Oct 2020 at 06:47, Stefan Roese <sr@denx.de> wrote:
> >
> > Hi Vabhav,
> >
> > On 14.10.20 13:15, Vabhav Sharma (OSS) wrote:
> > > Hi Stefan,
> > > Sorry for delayed reply, Occupied with high priority task
> > >
> > >> -----Original Message-----
> > >> From: Stefan Roese <sr@denx.de>
> > >> Sent: Wednesday, September 30, 2020 10:46 AM
> > >> To: Vabhav Sharma (OSS) <vabhav.sharma@oss.nxp.com>;
> > >> andre.przywara at arm.com; u-boot at lists.denx.de; sjg at chromium.org
> > >> Cc: Vabhav Sharma <vabhav.sharma@nxp.com>
> > >> Subject: Re: [PATCH v2] drivers: serial: probe all uart devices
> > >>
> > >> On 29.09.20 19:26, Vabhav Sharma wrote:
> > >>> From: Vabhav Sharma <vabhav.sharma@nxp.com>
> > >>>
> > >>> U-Boot DM model probe only single device at a time which is
> > >>> enabled and configured using device tree or platform data method.
> > >>>
> > >>> PL011 UART IP is SBSA compliant and firmware does the serial port
> > >>> set-up, initialization and let the kernel use UART port for
> > >>> sending and receiving characters.
> > >>>
> > >>> Normally software talk to one serial port time but some LayerScape
> > >>> platform require all the UART devices enabled in Linux for various
> > >>> use case.
> > >>>
> > >>> Adding support to probe all enabled serial devices like SBSA
> > >>> compliant
> > >>> PL011 UART ports probe and initialization by firmware.
> > >>>
> > >>> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com>
> > >>> ---
> > >>> v2:
> > >>> Incorporated Stefan review comment, Update #ifdef with macro
> > >>> if (IS_ENABLED)..
> > >>
> > >> Better, thanks. But...
> > >>
> > >>> ---
> > >>> ---
> > >>> drivers/serial/Kconfig | 17 +++++++++++++++++
> > >>> drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
> > >>> 2 files changed, 47 insertions(+)
> > >>>
> > >>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index
> > >>> e344677..b2e30f1 100644
> > >>> --- a/drivers/serial/Kconfig
> > >>> +++ b/drivers/serial/Kconfig
> > >>> @@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
> > >>>
> > >>> If unsure, say N.
> > >>>
> > >>> +config SERIAL_PROBE_ALL
> > >>> + bool "Probe all available serial devices"
> > >>> + depends on DM_SERIAL
> > >>> + default n
> > >>> + help
> > >>> + The serial subsystem only probe for single serial device,
> > >>> + but does not probe for other remaining serial devices.
> > >>> + With this option set,we make probing and searching for
> > >>> + all available devices optional.
> > >>> + Normally, U-Boot talk to one serial port at a time but SBSA
> > >>> + compliant UART devices like PL011 require initialization
> > >>> + by firmware and let the kernel use serial port for sending
> > >>> + and receiving the characters.
> > >>> +
> > >>> + If probing is not required for all remaining available
> > >>> + devices other than default current console device, say N.
> > >>> +
> > >>> config SPL_DM_SERIAL
> > >>> bool "Enable Driver Model for serial drivers in SPL"
> > >>> depends on DM_SERIAL && SPL_DM diff --git
> > >>> a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> > >>> index 0027625..d303a66 100644
> > >>> --- a/drivers/serial/serial-uclass.c
> > >>> +++ b/drivers/serial/serial-uclass.c
> > >>> @@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
> > >>> uclass_first_device(UCLASS_SERIAL, &dev);
> > >>> if (dev) {
> > >>> gd->cur_serial_dev = dev;
> > >>> + if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> > >>> + /* Scanning uclass to probe all devices */
> > >>> + for (; dev; uclass_next_device(&dev))
> > >>> + ;
> > >>> + }
> > >>> return;
> > >>> }
> > >>> } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -96,11
> > >>> +101,21 @@ static void serial_find_console_or_panic(void)
> > >>> if (np
> > >> && !uclass_get_device_by_ofnode(UCLASS_SERIAL,
> > >>> np_to_ofnode(np), &dev)) {
> > >>> gd->cur_serial_dev = dev;
> > >>> + if
> > >>> + (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> > >> {
> > >>> + /* Scanning uclass to probe
> > >>> + devices
> > >> */
> > >>> + for (; dev; uclass_next_device(&dev))
> > >>> + ;
> > >>> + }
> > >>
> > >> This code sequence (incl. gd->cur_serial_dev = dev;) is repeated
> > >> multiple times (below as well). Could you instead move this into a
> > >> function and call this function to reduce code and binary size?
> > > I understand.
> > > Checked and found that 56 bytes of code is added (including enabling
> > > the macro on LS platform)
> > >
> > > Intended to define it earlier but defining one line of code in a
> > > function cause more overhead (function call, Stack operations,
> > > Pointer arithmetic) in execution time.
> >
> > So you did measure a penalty in bootup time with this code moved into
> > a function? I would have thought that this is neglectable.
> >
> > > Tradeoff is to choose between Function overhead Vs Binary Size, What
> > > is your suggestion
> >
> > My vote is for the function. Mainly not because of the smaller image
> > size, but because of the smaller source code base, being easier to
> > maintain (IMHO).
>
> Same for me.
Sure, I understand.
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-10-19 8:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-29 17:26 [PATCH v2] drivers: serial: probe all uart devices Vabhav Sharma
2020-09-30 5:15 ` Stefan Roese
2020-10-14 11:15 ` Vabhav Sharma
2020-10-14 12:46 ` Stefan Roese
2020-10-15 15:05 ` Simon Glass
2020-10-19 8:19 ` Vabhav Sharma
2020-09-30 16:44 ` Simon Glass
2020-10-14 11:46 ` Vabhav Sharma
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.