Re: [PATCH 4/6] stdio: Update to use compiler for Kconfig checks
Hi Tom,
I can't see anything other than my patches at that link. But it looks
like I didn't send it, sadly. I'll resend that patch.
Regards,
SImon
On Tue, 4 Aug 2020 at 07:46, Tom Rini wrote:
>
> On Tue, Aug 04, 2020 at 07:37:02AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 3 Aug 2020 at 20:18, Tom Rini wrote:
> > >
> > > On Mon, Aug 03, 2020 at 06:57:05PM -0400, Tom Rini wrote:
> > > > On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
> > > >
> > > > > Drop use of the preprocessor where possible.
> > > > >
> > > > > Signed-off-by: Simon Glass
> > > > [snip]
> > > > > + if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> > > > > + /*
> > > > > +* If the console setting is not in environment variables
> > > > > then
> > > > > +* console_init_r() will not be calling iomux_doenv()
> > > > > (which
> > > > > +* calls search_device()). So we will not dynamically add
> > > > > +* devices by calling stdio_probe_device().
> > > > > +*
> > > > > +* So just probe all video devices now so that whichever
> > > > > one is
> > > > > +* required will be available.
> > > > > +*/
> > > > > + struct udevice *vdev;
> > > > > + int ret;
> > > > > +
> > > > > + if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> > > > > + for (ret = uclass_first_device(UCLASS_VIDEO,
> > > > > );
> > > > > +vdev;
> > > > > +ret = uclass_next_device())
> > > > > + ;
> > > > > + if (ret)
> > > > > + printf("%s: Video device failed
> > > > > (ret=%d)\n",
> > > > > + __func__, ret);
> > > > > + }
> > > > > + if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> > > > > + IS_ENABLED(CONFIG_CMD_BMP))
> > > > > + splash_display();
> > > >
> > > > We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
> > > > test fails and we drop the bmp logo. I got this run-time tested and
> > > > confirmed on colibri imx6 by my colleague Matt Porter. So I'm holding
> > > > off on _this_ patch (and then the style clean up patch) until I can push
> > > > SPLASH_SCREEN migration through.
> > >
> > > This is also an issue for "stdio: Tidy up use of
> > > CONFIG_SYS_DEVICE_NULLDEV" as well, so I'm going to put aside the whole
> > > series until I can convert SPLASH_SCREEN.
> >
> > OK, let me know if you'd like me to do something here. My patch 2 was
> > supposed to migrate CONFIG_SPLASH_SCREEN, etc. Did that not work?
>
> I don't see that patch:
> http://patchwork.ozlabs.org/user/todo/uboot/?series=190587
>
> --
> Tom
Re: [PATCH 4/6] stdio: Update to use compiler for Kconfig checks
On Tue, Aug 04, 2020 at 07:37:02AM -0600, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 3 Aug 2020 at 20:18, Tom Rini wrote:
> >
> > On Mon, Aug 03, 2020 at 06:57:05PM -0400, Tom Rini wrote:
> > > On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
> > >
> > > > Drop use of the preprocessor where possible.
> > > >
> > > > Signed-off-by: Simon Glass
> > > [snip]
> > > > + if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> > > > + /*
> > > > +* If the console setting is not in environment variables
> > > > then
> > > > +* console_init_r() will not be calling iomux_doenv() (which
> > > > +* calls search_device()). So we will not dynamically add
> > > > +* devices by calling stdio_probe_device().
> > > > +*
> > > > +* So just probe all video devices now so that whichever
> > > > one is
> > > > +* required will be available.
> > > > +*/
> > > > + struct udevice *vdev;
> > > > + int ret;
> > > > +
> > > > + if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> > > > + for (ret = uclass_first_device(UCLASS_VIDEO, );
> > > > +vdev;
> > > > +ret = uclass_next_device())
> > > > + ;
> > > > + if (ret)
> > > > + printf("%s: Video device failed (ret=%d)\n",
> > > > + __func__, ret);
> > > > + }
> > > > + if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> > > > + IS_ENABLED(CONFIG_CMD_BMP))
> > > > + splash_display();
> > >
> > > We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
> > > test fails and we drop the bmp logo. I got this run-time tested and
> > > confirmed on colibri imx6 by my colleague Matt Porter. So I'm holding
> > > off on _this_ patch (and then the style clean up patch) until I can push
> > > SPLASH_SCREEN migration through.
> >
> > This is also an issue for "stdio: Tidy up use of
> > CONFIG_SYS_DEVICE_NULLDEV" as well, so I'm going to put aside the whole
> > series until I can convert SPLASH_SCREEN.
>
> OK, let me know if you'd like me to do something here. My patch 2 was
> supposed to migrate CONFIG_SPLASH_SCREEN, etc. Did that not work?
I don't see that patch:
http://patchwork.ozlabs.org/user/todo/uboot/?series=190587
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 4/6] stdio: Update to use compiler for Kconfig checks
Hi Tom,
On Mon, 3 Aug 2020 at 20:18, Tom Rini wrote:
>
> On Mon, Aug 03, 2020 at 06:57:05PM -0400, Tom Rini wrote:
> > On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
> >
> > > Drop use of the preprocessor where possible.
> > >
> > > Signed-off-by: Simon Glass
> > [snip]
> > > + if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> > > + /*
> > > +* If the console setting is not in environment variables then
> > > +* console_init_r() will not be calling iomux_doenv() (which
> > > +* calls search_device()). So we will not dynamically add
> > > +* devices by calling stdio_probe_device().
> > > +*
> > > +* So just probe all video devices now so that whichever one
> > > is
> > > +* required will be available.
> > > +*/
> > > + struct udevice *vdev;
> > > + int ret;
> > > +
> > > + if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> > > + for (ret = uclass_first_device(UCLASS_VIDEO, );
> > > +vdev;
> > > +ret = uclass_next_device())
> > > + ;
> > > + if (ret)
> > > + printf("%s: Video device failed (ret=%d)\n",
> > > + __func__, ret);
> > > + }
> > > + if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> > > + IS_ENABLED(CONFIG_CMD_BMP))
> > > + splash_display();
> >
> > We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
> > test fails and we drop the bmp logo. I got this run-time tested and
> > confirmed on colibri imx6 by my colleague Matt Porter. So I'm holding
> > off on _this_ patch (and then the style clean up patch) until I can push
> > SPLASH_SCREEN migration through.
>
> This is also an issue for "stdio: Tidy up use of
> CONFIG_SYS_DEVICE_NULLDEV" as well, so I'm going to put aside the whole
> series until I can convert SPLASH_SCREEN.
OK, let me know if you'd like me to do something here. My patch 2 was
supposed to migrate CONFIG_SPLASH_SCREEN, etc. Did that not work?
Regards,
SImon
Re: [PATCH 4/6] stdio: Update to use compiler for Kconfig checks
On Mon, Aug 03, 2020 at 06:57:05PM -0400, Tom Rini wrote:
> On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
>
> > Drop use of the preprocessor where possible.
> >
> > Signed-off-by: Simon Glass
> [snip]
> > + if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> > + /*
> > +* If the console setting is not in environment variables then
> > +* console_init_r() will not be calling iomux_doenv() (which
> > +* calls search_device()). So we will not dynamically add
> > +* devices by calling stdio_probe_device().
> > +*
> > +* So just probe all video devices now so that whichever one is
> > +* required will be available.
> > +*/
> > + struct udevice *vdev;
> > + int ret;
> > +
> > + if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> > + for (ret = uclass_first_device(UCLASS_VIDEO, );
> > +vdev;
> > +ret = uclass_next_device())
> > + ;
> > + if (ret)
> > + printf("%s: Video device failed (ret=%d)\n",
> > + __func__, ret);
> > + }
> > + if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> > + IS_ENABLED(CONFIG_CMD_BMP))
> > + splash_display();
>
> We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
> test fails and we drop the bmp logo. I got this run-time tested and
> confirmed on colibri imx6 by my colleague Matt Porter. So I'm holding
> off on _this_ patch (and then the style clean up patch) until I can push
> SPLASH_SCREEN migration through.
This is also an issue for "stdio: Tidy up use of
CONFIG_SYS_DEVICE_NULLDEV" as well, so I'm going to put aside the whole
series until I can convert SPLASH_SCREEN.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 4/6] stdio: Update to use compiler for Kconfig checks
On Fri, Jul 17, 2020 at 09:03:17PM -0600, Simon Glass wrote:
> Drop use of the preprocessor where possible.
>
> Signed-off-by: Simon Glass
[snip]
> + if (IS_ENABLED(CONFIG_DM_VIDEO)) {
> + /*
> + * If the console setting is not in environment variables then
> + * console_init_r() will not be calling iomux_doenv() (which
> + * calls search_device()). So we will not dynamically add
> + * devices by calling stdio_probe_device().
> + *
> + * So just probe all video devices now so that whichever one is
> + * required will be available.
> + */
> + struct udevice *vdev;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_SYS_CONSOLE_IS_IN_ENV)) {
> + for (ret = uclass_first_device(UCLASS_VIDEO, );
> + vdev;
> + ret = uclass_next_device())
> + ;
> + if (ret)
> + printf("%s: Video device failed (ret=%d)\n",
> +__func__, ret);
> + }
> + if (IS_ENABLED(CONFIG_SPLASH_SCREEN) &&
> + IS_ENABLED(CONFIG_CMD_BMP))
> + splash_display();
We can't do this yet because CONFIG_SPLASH_SCREEN isn't migrated so the
test fails and we drop the bmp logo. I got this run-time tested and
confirmed on colibri imx6 by my colleague Matt Porter. So I'm holding
off on _this_ patch (and then the style clean up patch) until I can push
SPLASH_SCREEN migration through.
--
Tom
signature.asc
Description: PGP signature
[PATCH 4/6] stdio: Update to use compiler for Kconfig checks
Drop use of the preprocessor where possible.
Signed-off-by: Simon Glass
---
common/stdio.c | 172 -
1 file changed, 83 insertions(+), 89 deletions(-)
diff --git a/common/stdio.c b/common/stdio.c
index 33a795e7bb..1921dc6719 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -18,10 +18,7 @@
#include
#include
#include
-
-#if defined(CONFIG_SYS_I2C)
#include
-#endif
#include
@@ -109,7 +106,6 @@ struct list_head* stdio_get_list(void)
return &(devs.list);
}
-#ifdef CONFIG_DM_VIDEO
/**
* stdio_probe_device() - Find a device which provides the given stdio device
*
@@ -160,7 +156,6 @@ static int stdio_probe_device(const char *name, enum
uclass_id id,
return 0;
}
-#endif
struct stdio_dev *stdio_get_by_name(const char *name)
{
@@ -175,22 +170,23 @@ struct stdio_dev *stdio_get_by_name(const char *name)
if (strcmp(sdev->name, name) == 0)
return sdev;
}
-#ifdef CONFIG_DM_VIDEO
- /*
-* We did not find a suitable stdio device. If there is a video
-* driver with a name starting with 'vidconsole', we can try probing
-* that in the hope that it will produce the required stdio device.
-*
-* This function is sometimes called with the entire value of
-* 'stdout', which may include a list of devices separate by commas.
-* Obviously this is not going to work, so we ignore that case. The
-* call path in that case is console_init_r() -> search_device() ->
-* stdio_get_by_name().
-*/
- if (!strncmp(name, "vidconsole", 10) && !strchr(name, ',') &&
- !stdio_probe_device(name, UCLASS_VIDEO, ))
- return sdev;
-#endif
+ if (IS_ENABLED(CONFIG_DM_VIDEO)) {
+ /*
+* We did not find a suitable stdio device. If there is a video
+* driver with a name starting with 'vidconsole', we can try
+* probing that in the hope that it will produce the required
+* stdio device.
+*
+* This function is sometimes called with the entire value of
+* 'stdout', which may include a list of devices separate by
+* commas. Obviously this is not going to work, so we ignore
+* that case. The call path in that case is
+* console_init_r() -> search_device() -> stdio_get_by_name()
+*/
+ if (!strncmp(name, "vidconsole", 10) && !strchr(name, ',') &&
+ !stdio_probe_device(name, UCLASS_VIDEO, ))
+ return sdev;
+ }
return NULL;
}
@@ -234,7 +230,6 @@ int stdio_register(struct stdio_dev *dev)
/* deregister the device "devname".
* returns 0 if success, -1 if device is assigned and 1 if devname not found
*/
-#if CONFIG_IS_ENABLED(SYS_STDIO_DEREGISTER)
int stdio_deregister_dev(struct stdio_dev *dev, int force)
{
int l;
@@ -281,7 +276,6 @@ int stdio_deregister(const char *devname, int force)
return stdio_deregister_dev(dev, force);
}
-#endif /* CONFIG_IS_ENABLED(SYS_STDIO_DEREGISTER) */
int stdio_init_tables(void)
{
@@ -305,87 +299,87 @@ int stdio_init_tables(void)
int stdio_add_devices(void)
{
-#ifdef CONFIG_DM_KEYBOARD
struct udevice *dev;
struct uclass *uc;
int ret;
- /*
-* For now we probe all the devices here. At some point this should be
-* done only when the devices are required - e.g. we have a list of
-* input devices to start up in the stdin environment variable. That
-* work probably makes more sense when stdio itself is converted to
-* driver model.
-*
-* TODO(s...@chromium.org): Convert changing uclass_first_device() etc.
-* to return the device even on error. Then we could use that here.
-*/
- ret = uclass_get(UCLASS_KEYBOARD, );
- if (ret)
- return ret;
-
- /* Don't report errors to the caller - assume that they are non-fatal */
- uclass_foreach_dev(dev, uc) {
- ret = device_probe(dev);
+ if (IS_ENABLED(CONFIG_DM_KEYBOARD)) {
+ /*
+* For now we probe all the devices here. At some point this
+* should be done only when the devices are required - e.g. we
+* have a list of input devices to start up in the stdin
+* environment variable. That work probably makes more sense
+* when stdio itself is converted to driver model.
+*
+* TODO(s...@chromium.org): Convert changing
+* uclass_first_device() etc. to return the device even on
+* error. Then we could use that here.
+*/
+ ret = uclass_get(UCLASS_KEYBOARD, );
if (ret)
- printf("Failed to
