* [PATCH] odroid: add CROS_EC to odroid_defconfig
@ 2023-01-20 23:16 Joost van Zwieten
2023-01-23 8:33 ` Peter Robinson
0 siblings, 1 reply; 6+ messages in thread
From: Joost van Zwieten @ 2023-01-20 23:16 UTC (permalink / raw)
To: u-boot; +Cc: Simon Glass, Minkyu Kang
As of commit e44d7e73fe ("dm: core: Switch uclass_*_device_err to use
uclass_*_device_check") U-Boot built with `odroid_defconfig` fails to
initialize on an Odroid U2: `board_late_init()`
(board/samsung/common/board.c)
returns with an error due to the absence of the CROS_EC uclass, and the
board
resets. This patch adds `CROS_EC` to the defconfig, leading to a
succcessful
initialization of U-Boot.
Signed-off-by: Joost van Zwieten <joost@diskos.nl>
---
configs/odroid_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/configs/odroid_defconfig b/configs/odroid_defconfig
index 6e1c29b355..b1ccabd0d2 100644
--- a/configs/odroid_defconfig
+++ b/configs/odroid_defconfig
@@ -55,6 +55,7 @@ CONFIG_DFU_MMC=y
CONFIG_SET_DFU_ALT_INFO=y
CONFIG_SYS_DFU_DATA_BUF_SIZE=0x2000000
CONFIG_SYS_I2C_S3C24X0=y
+CONFIG_CROS_EC=y
CONFIG_MMC_DW=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_SDMA=y
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] odroid: add CROS_EC to odroid_defconfig
2023-01-20 23:16 [PATCH] odroid: add CROS_EC to odroid_defconfig Joost van Zwieten
@ 2023-01-23 8:33 ` Peter Robinson
2023-01-23 9:39 ` Joost van Zwieten
0 siblings, 1 reply; 6+ messages in thread
From: Peter Robinson @ 2023-01-23 8:33 UTC (permalink / raw)
To: Joost van Zwieten; +Cc: u-boot, Simon Glass, Minkyu Kang
On Fri, Jan 20, 2023 at 11:17 PM Joost van Zwieten <joost@diskos.nl> wrote:
>
> As of commit e44d7e73fe ("dm: core: Switch uclass_*_device_err to use
> uclass_*_device_check") U-Boot built with `odroid_defconfig` fails to
> initialize on an Odroid U2: `board_late_init()`
> (board/samsung/common/board.c)
> returns with an error due to the absence of the CROS_EC uclass, and the
> board
> resets. This patch adds `CROS_EC` to the defconfig, leading to a
> succcessful
> initialization of U-Boot.
This doesn't seem to be the right fix to the problem, why would an
odroid device have a need for a driver for the ChromeOS Embedded
Controller?
> Signed-off-by: Joost van Zwieten <joost@diskos.nl>
> ---
> configs/odroid_defconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/configs/odroid_defconfig b/configs/odroid_defconfig
> index 6e1c29b355..b1ccabd0d2 100644
> --- a/configs/odroid_defconfig
> +++ b/configs/odroid_defconfig
> @@ -55,6 +55,7 @@ CONFIG_DFU_MMC=y
> CONFIG_SET_DFU_ALT_INFO=y
> CONFIG_SYS_DFU_DATA_BUF_SIZE=0x2000000
> CONFIG_SYS_I2C_S3C24X0=y
> +CONFIG_CROS_EC=y
> CONFIG_MMC_DW=y
> CONFIG_MMC_SDHCI=y
> CONFIG_MMC_SDHCI_SDMA=y
> --
> 2.30.2
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] odroid: add CROS_EC to odroid_defconfig
2023-01-23 8:33 ` Peter Robinson
@ 2023-01-23 9:39 ` Joost van Zwieten
2023-01-23 18:42 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Joost van Zwieten @ 2023-01-23 9:39 UTC (permalink / raw)
To: Peter Robinson; +Cc: u-boot, Simon Glass, Minkyu Kang
On Mon, Jan 23, 2023 at 08:33, Peter Robinson <pbrobinson@gmail.com>
wrote:
> On Fri, Jan 20, 2023 at 11:17 PM Joost van Zwieten <joost@diskos.nl>
> wrote:
>>
>> As of commit e44d7e73fe ("dm: core: Switch uclass_*_device_err to
>> use
>> uclass_*_device_check") U-Boot built with `odroid_defconfig` fails
>> to
>> initialize on an Odroid U2: `board_late_init()`
>> (board/samsung/common/board.c)
>> returns with an error due to the absence of the CROS_EC uclass, and
>> the
>> board
>> resets. This patch adds `CROS_EC` to the defconfig, leading to a
>> succcessful
>> initialization of U-Boot.
>
> This doesn't seem to be the right fix to the problem, why would an
> odroid device have a need for a driver for the ChromeOS Embedded
> Controller?
For reference, here's the thread that led to this patch: [1].
My board returns `ENODEV` when calling
`uclass_first_device_err(UCLASS_CROS_EC, &dev)` with `CROS_EC` enabled,
so I don't seem to have this device.
At the end of [1] I have suggested an alternative fix: enclosing a
couple of statements that assume there's a CROS_EC driver inside an
`#ifdef CONFIG_CROS_EC`. Do you think that is a viable approach? I'm not
at all familiar with the U-Boot codebase, so forgive me if this doesn't
make any sense.
[1]: https://lists.denx.de/pipermail/u-boot/2023-January/505627.html
>
>> Signed-off-by: Joost van Zwieten <joost@diskos.nl>
>> ---
>> configs/odroid_defconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/configs/odroid_defconfig b/configs/odroid_defconfig
>> index 6e1c29b355..b1ccabd0d2 100644
>> --- a/configs/odroid_defconfig
>> +++ b/configs/odroid_defconfig
>> @@ -55,6 +55,7 @@ CONFIG_DFU_MMC=y
>> CONFIG_SET_DFU_ALT_INFO=y
>> CONFIG_SYS_DFU_DATA_BUF_SIZE=0x2000000
>> CONFIG_SYS_I2C_S3C24X0=y
>> +CONFIG_CROS_EC=y
>> CONFIG_MMC_DW=y
>> CONFIG_MMC_SDHCI=y
>> CONFIG_MMC_SDHCI_SDMA=y
>> --
>> 2.30.2
>>
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] odroid: add CROS_EC to odroid_defconfig
2023-01-23 9:39 ` Joost van Zwieten
@ 2023-01-23 18:42 ` Simon Glass
2023-01-24 13:25 ` Joost van Zwieten
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2023-01-23 18:42 UTC (permalink / raw)
To: Joost van Zwieten; +Cc: Peter Robinson, u-boot, Minkyu Kang
Hi,
On Mon, 23 Jan 2023 at 02:39, Joost van Zwieten <joost@diskos.nl> wrote:
>
>
> On Mon, Jan 23, 2023 at 08:33, Peter Robinson <pbrobinson@gmail.com>
> wrote:
> > On Fri, Jan 20, 2023 at 11:17 PM Joost van Zwieten <joost@diskos.nl>
> > wrote:
> >>
> >> As of commit e44d7e73fe ("dm: core: Switch uclass_*_device_err to
> >> use
> >> uclass_*_device_check") U-Boot built with `odroid_defconfig` fails
> >> to
> >> initialize on an Odroid U2: `board_late_init()`
> >> (board/samsung/common/board.c)
> >> returns with an error due to the absence of the CROS_EC uclass, and
> >> the
> >> board
> >> resets. This patch adds `CROS_EC` to the defconfig, leading to a
> >> succcessful
> >> initialization of U-Boot.
> >
> > This doesn't seem to be the right fix to the problem, why would an
> > odroid device have a need for a driver for the ChromeOS Embedded
> > Controller?
>
> For reference, here's the thread that led to this patch: [1].
>
> My board returns `ENODEV` when calling
> `uclass_first_device_err(UCLASS_CROS_EC, &dev)` with `CROS_EC` enabled,
> so I don't seem to have this device.
This is really adding the driver rather than the device. The device
still does not exist. It is because the exynos boards share common
code like board_init().
Reviewed-by: Simon Glass <sjg@chromium.org>
>
> At the end of [1] I have suggested an alternative fix: enclosing a
> couple of statements that assume there's a CROS_EC driver inside an
> `#ifdef CONFIG_CROS_EC`. Do you think that is a viable approach? I'm not
> at all familiar with the U-Boot codebase, so forgive me if this doesn't
> make any sense.
That would be OK too, but if you do that, please use IS_ENABLED()
rather than #ifdef
>
> [1]: https://lists.denx.de/pipermail/u-boot/2023-January/505627.html
>
> >
> >> Signed-off-by: Joost van Zwieten <joost@diskos.nl>
> >> ---
> >> configs/odroid_defconfig | 1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> diff --git a/configs/odroid_defconfig b/configs/odroid_defconfig
> >> index 6e1c29b355..b1ccabd0d2 100644
> >> --- a/configs/odroid_defconfig
> >> +++ b/configs/odroid_defconfig
> >> @@ -55,6 +55,7 @@ CONFIG_DFU_MMC=y
> >> CONFIG_SET_DFU_ALT_INFO=y
> >> CONFIG_SYS_DFU_DATA_BUF_SIZE=0x2000000
> >> CONFIG_SYS_I2C_S3C24X0=y
> >> +CONFIG_CROS_EC=y
> >> CONFIG_MMC_DW=y
> >> CONFIG_MMC_SDHCI=y
> >> CONFIG_MMC_SDHCI_SDMA=y
> >> --
> >> 2.30.2
> >>
> >>
> >>
> >>
> >>
>
>
Regards,
SImon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] odroid: add CROS_EC to odroid_defconfig
2023-01-23 18:42 ` Simon Glass
@ 2023-01-24 13:25 ` Joost van Zwieten
2023-01-24 18:38 ` Henrik Grimler
0 siblings, 1 reply; 6+ messages in thread
From: Joost van Zwieten @ 2023-01-24 13:25 UTC (permalink / raw)
To: Simon Glass; +Cc: Peter Robinson, u-boot, Minkyu Kang
On Mon, Jan 23, 2023 at 11:42, Simon Glass <sjg@chromium.org> wrote:
> Hi,
>
> On Mon, 23 Jan 2023 at 02:39, Joost van Zwieten <joost@diskos.nl>
> wrote:
>>
>>
>> On Mon, Jan 23, 2023 at 08:33, Peter Robinson <pbrobinson@gmail.com>
>> wrote:
>> > On Fri, Jan 20, 2023 at 11:17 PM Joost van Zwieten
>> <joost@diskos.nl>
>> > wrote:
>> >>
>> >> As of commit e44d7e73fe ("dm: core: Switch uclass_*_device_err
>> to
>> >> use
>> >> uclass_*_device_check") U-Boot built with `odroid_defconfig`
>> fails
>> >> to
>> >> initialize on an Odroid U2: `board_late_init()`
>> >> (board/samsung/common/board.c)
>> >> returns with an error due to the absence of the CROS_EC uclass,
>> and
>> >> the
>> >> board
>> >> resets. This patch adds `CROS_EC` to the defconfig, leading to a
>> >> succcessful
>> >> initialization of U-Boot.
>> >
>> > This doesn't seem to be the right fix to the problem, why would an
>> > odroid device have a need for a driver for the ChromeOS Embedded
>> > Controller?
>>
>> For reference, here's the thread that led to this patch: [1].
>>
>> My board returns `ENODEV` when calling
>> `uclass_first_device_err(UCLASS_CROS_EC, &dev)` with `CROS_EC`
>> enabled,
>> so I don't seem to have this device.
>
> This is really adding the driver rather than the device. The device
> still does not exist. It is because the exynos boards share common
> code like board_init().
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
>>
>> At the end of [1] I have suggested an alternative fix: enclosing a
>> couple of statements that assume there's a CROS_EC driver inside an
>> `#ifdef CONFIG_CROS_EC`. Do you think that is a viable approach?
>> I'm not
>> at all familiar with the U-Boot codebase, so forgive me if this
>> doesn't
>> make any sense.
>
> That would be OK too, but if you do that, please use IS_ENABLED()
> rather than #ifdef
I've tested the alternative, and that works for me as well. I am
slightly in favour of the alternative, because it seems illogical to
require building a driver that is never going to be used. On the other
hand this patch only affects the Odroid U2, and the alternative every
board using an Exnyos chip.
I'm still wondering why other boards, e.g. the Odroid XU3, are seemingly
(I don't have any) unaffected by this.
Best regards,
Joost
>>
>> [1]: https://lists.denx.de/pipermail/u-boot/2023-January/505627.html
>>
>> >
>> >> Signed-off-by: Joost van Zwieten <joost@diskos.nl>
>> >> ---
>> >> configs/odroid_defconfig | 1 +
>> >> 1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/configs/odroid_defconfig b/configs/odroid_defconfig
>> >> index 6e1c29b355..b1ccabd0d2 100644
>> >> --- a/configs/odroid_defconfig
>> >> +++ b/configs/odroid_defconfig
>> >> @@ -55,6 +55,7 @@ CONFIG_DFU_MMC=y
>> >> CONFIG_SET_DFU_ALT_INFO=y
>> >> CONFIG_SYS_DFU_DATA_BUF_SIZE=0x2000000
>> >> CONFIG_SYS_I2C_S3C24X0=y
>> >> +CONFIG_CROS_EC=y
>> >> CONFIG_MMC_DW=y
>> >> CONFIG_MMC_SDHCI=y
>> >> CONFIG_MMC_SDHCI_SDMA=y
>> >> --
>> >> 2.30.2
>> >>
>> >>
>> >>
>> >>
>> >>
>>
>>
>
> Regards,
> SImon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] odroid: add CROS_EC to odroid_defconfig
2023-01-24 13:25 ` Joost van Zwieten
@ 2023-01-24 18:38 ` Henrik Grimler
0 siblings, 0 replies; 6+ messages in thread
From: Henrik Grimler @ 2023-01-24 18:38 UTC (permalink / raw)
To: u-boot
Hi Joost,
[ ... ]
> > That would be OK too, but if you do that, please use IS_ENABLED()
> > rather than #ifdef
>
> I've tested the alternative, and that works for me as well. I am
> slightly in favour of the alternative, because it seems illogical to
> require building a driver that is never going to be used. On the other
> hand this patch only affects the Odroid U2, and the alternative every
> board using an Exnyos chip.
>
> I'm still wondering why other boards, e.g. the Odroid XU3, are seemingly
> (I don't have any) unaffected by this.
Odroid-xu4 has the same issue, and the proposed solutions fixes the
issue there as well. I also think the alternative `#if
IS_ENABLED(CONFIG_CROS_EC)`-solution would be better.
Thanks for reporting the issue!
Best regards,
Henrik Grimler
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-24 19:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-20 23:16 [PATCH] odroid: add CROS_EC to odroid_defconfig Joost van Zwieten
2023-01-23 8:33 ` Peter Robinson
2023-01-23 9:39 ` Joost van Zwieten
2023-01-23 18:42 ` Simon Glass
2023-01-24 13:25 ` Joost van Zwieten
2023-01-24 18:38 ` Henrik Grimler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox