* [U-Boot] [PATCH 1/2] rockchip: chromebook_minnie: Enable sound @ 2018-12-29 13:16 Simon Glass 2018-12-29 13:16 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Simon Glass ` (2 more replies) 0 siblings, 3 replies; 27+ messages in thread From: Simon Glass @ 2018-12-29 13:16 UTC (permalink / raw) To: u-boot Enable sound for this board, which has the same codec as jerry. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/arm/dts/rk3288-veyron-minnie.dts | 12 ++++++++++++ configs/chromebook_minnie_defconfig | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/arch/arm/dts/rk3288-veyron-minnie.dts b/arch/arm/dts/rk3288-veyron-minnie.dts index 0a2915fba09..646f6ae7424 100644 --- a/arch/arm/dts/rk3288-veyron-minnie.dts +++ b/arch/arm/dts/rk3288-veyron-minnie.dts @@ -85,6 +85,18 @@ regulator-boot-on; vin-supply = <&vcc18_wl>; }; + + sound { + compatible = "rockchip,audio-max98090-jerry"; + + cpu { + sound-dai = <&i2s 0>; + }; + + codec { + sound-dai = <&max98090 0>; + }; + }; }; &backlight { diff --git a/configs/chromebook_minnie_defconfig b/configs/chromebook_minnie_defconfig index b0428740736..93e40c02101 100644 --- a/configs/chromebook_minnie_defconfig +++ b/configs/chromebook_minnie_defconfig @@ -31,6 +31,7 @@ CONFIG_CMD_USB=y # CONFIG_CMD_SETEXPR is not set CONFIG_CMD_CACHE=y CONFIG_CMD_TIME=y +CONFIG_CMD_SOUND=y CONFIG_CMD_PMIC=y CONFIG_CMD_REGULATOR=y # CONFIG_SPL_DOS_PARTITION is not set @@ -75,6 +76,10 @@ CONFIG_RAM=y CONFIG_SPL_RAM=y CONFIG_DEBUG_UART_SHIFT=2 CONFIG_ROCKCHIP_SERIAL=y +CONFIG_SOUND=y +CONFIG_I2S=y +CONFIG_I2S_ROCKCHIP=y +CONFIG_SOUND_MAX98090=y CONFIG_ROCKCHIP_SPI=y CONFIG_SYSRESET=y CONFIG_USB=y -- 2.20.1.415.g653613c723-goog ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2018-12-29 13:16 [U-Boot] [PATCH 1/2] rockchip: chromebook_minnie: Enable sound Simon Glass @ 2018-12-29 13:16 ` Simon Glass 2018-12-29 17:57 ` Philipp Tomsich ` (2 more replies) 2019-01-27 17:31 ` [U-Boot] [U-Boot, 1/2] rockchip: chromebook_minnie: Enable sound Philipp Tomsich 2019-01-28 9:12 ` Philipp Tomsich 2 siblings, 3 replies; 27+ messages in thread From: Simon Glass @ 2018-12-29 13:16 UTC (permalink / raw) To: u-boot This function causes a 5-second delay and stops the display working on minnie. This code should be in a driver and should only be enabled by a device-tree property, so that it does not affect devices which do not have this feature. Signed-off-by: Simon Glass <sjg@chromium.org> --- arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c index f32b3c4ce56..d7997d71e30 100644 --- a/arch/arm/mach-rockchip/boot_mode.c +++ b/arch/arm/mach-rockchip/boot_mode.c @@ -61,7 +61,13 @@ int setup_boot_mode(void) void *reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG; int boot_mode = readl(reg); - rockchip_dnl_mode_check(); + /* + * This should be handled using a driver-tree property and a suitable + * driver which can read the appropriate settings. As it is, this + * breaks chromebook_minnie.\ + * + * rockchip_dnl_mode_check(); + */ boot_mode = readl(reg); debug("%s: boot mode 0x%08x\n", __func__, boot_mode); -- 2.20.1.415.g653613c723-goog ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2018-12-29 13:16 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Simon Glass @ 2018-12-29 17:57 ` Philipp Tomsich 2018-12-29 18:00 ` Philipp Tomsich 2019-01-03 7:32 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz.jvt@chromium-sendgmr.bounces.google.com] Kever Yang 2019-01-31 21:12 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Philipp Tomsich 2 siblings, 1 reply; 27+ messages in thread From: Philipp Tomsich @ 2018-12-29 17:57 UTC (permalink / raw) To: u-boot > On 29.12.2018, at 14:16, Simon Glass <sjg@chromium.org> wrote: > > This function causes a 5-second delay and stops the display working on > minnie. This code should be in a driver and should only be enabled by > a device-tree property, so that it does not affect devices which do not > have this feature. > > Signed-off-by: Simon Glass <sjg@chromium.org> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2018-12-29 17:57 ` Philipp Tomsich @ 2018-12-29 18:00 ` Philipp Tomsich 2019-01-10 12:56 ` Simon Glass 0 siblings, 1 reply; 27+ messages in thread From: Philipp Tomsich @ 2018-12-29 18:00 UTC (permalink / raw) To: u-boot Simon, As an alternative, you can turn this off by setting CONFIG_BOOT_MODE_REG=0 in your defconfig. We’ve done the same for our RK3399-Q7 module: configs/puma-rk3399_defconfig:CONFIG_ROCKCHIP_BOOT_MODE_REG=0x0 If this works for you, we can avoid removing this feature this late in the cycle and defeature/remove it in the next release, unless moved into a DTS-controlled driver. Thanks, Philipp. > On 29.12.2018, at 18:57, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > > > >> On 29.12.2018, at 14:16, Simon Glass <sjg@chromium.org> wrote: >> >> This function causes a 5-second delay and stops the display working on >> minnie. This code should be in a driver and should only be enabled by >> a device-tree property, so that it does not affect devices which do not >> have this feature. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> > > Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2018-12-29 18:00 ` Philipp Tomsich @ 2019-01-10 12:56 ` Simon Glass 0 siblings, 0 replies; 27+ messages in thread From: Simon Glass @ 2019-01-10 12:56 UTC (permalink / raw) To: u-boot Hi Philipp, On Sat, 29 Dec 2018 at 11:01, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > > Simon, > > As an alternative, you can turn this off by setting CONFIG_BOOT_MODE_REG=0 in > your defconfig. > We’ve done the same for our RK3399-Q7 module: > configs/puma-rk3399_defconfig:CONFIG_ROCKCHIP_BOOT_MODE_REG=0x0 > > If this works for you, we can avoid removing this feature this late in the cycle and > defeature/remove it in the next release, unless moved into a DTS-controlled driver. That's OK with me. Since the display was broken in the last release too, I'll just wait for the fix. Regards, Simon ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz.jvt@chromium-sendgmr.bounces.google.com] 2018-12-29 13:16 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Simon Glass 2018-12-29 17:57 ` Philipp Tomsich @ 2019-01-03 7:32 ` Kever Yang 2019-01-03 21:29 ` Simon Glass 2019-01-31 21:12 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Philipp Tomsich 2 siblings, 1 reply; 27+ messages in thread From: Kever Yang @ 2019-01-03 7:32 UTC (permalink / raw) To: u-boot Hi Simon, On 12/29/2018 09:16 PM, Simon Glass wrote: > This function causes a 5-second delay and stops the display working on > minnie. I don't understand why need 5-second, the logic is to check the adc key press to decide the mode, maybe because the different usage of the adc in minnie? Most of boards(which do not have ec like chromebook) need this check to get into download mode. I would prefer chrombook to implement a function to replace the week rockchip_dnl_mode_check(); Thanks, - Kever > This code should be in a driver and should only be enabled by > a device-tree property, so that it does not affect devices which do not > have this feature. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c > index f32b3c4ce56..d7997d71e30 100644 > --- a/arch/arm/mach-rockchip/boot_mode.c > +++ b/arch/arm/mach-rockchip/boot_mode.c > @@ -61,7 +61,13 @@ int setup_boot_mode(void) > void *reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG; > int boot_mode = readl(reg); > > - rockchip_dnl_mode_check(); > + /* > + * This should be handled using a driver-tree property and a suitable > + * driver which can read the appropriate settings. As it is, this > + * breaks chromebook_minnie.\ > + * > + * rockchip_dnl_mode_check(); > + */ > > boot_mode = readl(reg); > debug("%s: boot mode 0x%08x\n", __func__, boot_mode); ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz.jvt@chromium-sendgmr.bounces.google.com] 2019-01-03 7:32 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz.jvt@chromium-sendgmr.bounces.google.com] Kever Yang @ 2019-01-03 21:29 ` Simon Glass 2019-01-03 21:36 ` Philipp Tomsich 0 siblings, 1 reply; 27+ messages in thread From: Simon Glass @ 2019-01-03 21:29 UTC (permalink / raw) To: u-boot Hi, On Thu, 3 Jan 2019 at 00:32, Kever Yang <kever.yang@rock-chips.com> wrote: > > Hi Simon, > > > On 12/29/2018 09:16 PM, Simon Glass wrote: > > This function causes a 5-second delay and stops the display working on > > minnie. > > I don't understand why need 5-second, the logic is to check the adc key > press to decide the mode, maybe because the different usage of the adc > in minnie? > > Most of boards(which do not have ec like chromebook) need this check to > get into download mode. > I would prefer chrombook to implement a function to replace the week > rockchip_dnl_mode_check(); But where is it specified in the DT that it uses the ADC for this feature? It really should have a driver... Regards, Simon > > Thanks, > - Kever > > This code should be in a driver and should only be enabled by > > a device-tree property, so that it does not affect devices which do not > > have this feature. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c > > index f32b3c4ce56..d7997d71e30 100644 > > --- a/arch/arm/mach-rockchip/boot_mode.c > > +++ b/arch/arm/mach-rockchip/boot_mode.c > > @@ -61,7 +61,13 @@ int setup_boot_mode(void) > > void *reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG; > > int boot_mode = readl(reg); > > > > - rockchip_dnl_mode_check(); > > + /* > > + * This should be handled using a driver-tree property and a suitable > > + * driver which can read the appropriate settings. As it is, this > > + * breaks chromebook_minnie.\ > > + * > > + * rockchip_dnl_mode_check(); > > + */ > > > > boot_mode = readl(reg); > > debug("%s: boot mode 0x%08x\n", __func__, boot_mode); > > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz.jvt@chromium-sendgmr.bounces.google.com] 2019-01-03 21:29 ` Simon Glass @ 2019-01-03 21:36 ` Philipp Tomsich 0 siblings, 0 replies; 27+ messages in thread From: Philipp Tomsich @ 2019-01-03 21:36 UTC (permalink / raw) To: u-boot > On 03.01.2019, at 22:29, Simon Glass <sjg@chromium.org> wrote: > > Hi, > > On Thu, 3 Jan 2019 at 00:32, Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>> wrote: >> >> Hi Simon, >> >> >> On 12/29/2018 09:16 PM, Simon Glass wrote: >>> This function causes a 5-second delay and stops the display working on >>> minnie. >> >> I don't understand why need 5-second, the logic is to check the adc key >> press to decide the mode, maybe because the different usage of the adc >> in minnie? >> >> Most of boards(which do not have ec like chromebook) need this check to >> get into download mode. >> I would prefer chrombook to implement a function to replace the week >> rockchip_dnl_mode_check(); > > But where is it specified in the DT that it uses the ADC for this > feature? It really should have a driver… 3 things: a. It really should have a driver. b. One does not have to replace rockchip_dnl_mode_check(), as there already is logic to remove it if CONFIG_ROCKCHIP_BOOT_MODE_REG is set to 0 in the config. c. We really need to get away from configuring magic addresses via Kconfig. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2018-12-29 13:16 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Simon Glass 2018-12-29 17:57 ` Philipp Tomsich 2019-01-03 7:32 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz.jvt@chromium-sendgmr.bounces.google.com] Kever Yang @ 2019-01-31 21:12 ` Philipp Tomsich 2019-02-01 1:32 ` andy.yan at rock-chips.com 2019-02-01 9:32 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 Kever Yang 2 siblings, 2 replies; 27+ messages in thread From: Philipp Tomsich @ 2019-01-31 21:12 UTC (permalink / raw) To: u-boot > This function causes a 5-second delay and stops the display working on > minnie. This code should be in a driver and should only be enabled by > a device-tree property, so that it does not affect devices which do not > have this feature. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > > arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > Applied to u-boot-rockchip, thanks! ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2019-01-31 21:12 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Philipp Tomsich @ 2019-02-01 1:32 ` andy.yan at rock-chips.com 2019-02-01 8:38 ` Philipp Tomsich 2019-02-01 9:32 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 Kever Yang 1 sibling, 1 reply; 27+ messages in thread From: andy.yan at rock-chips.com @ 2019-02-01 1:32 UTC (permalink / raw) To: u-boot Hi: > This function causes a 5-second delay and stops the display working on > minnie. This code should be in a driver and should only be enabled by > a device-tree property, so that it does not affect devices which do not > have this feature. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > > arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > 》Applied to u-boot-rockchip, thanks! According to the previous discussing, shouldn‘t this be fixed by by setting CONFIG_BOOT_MODE_REG=0 ? All rockchip products need this function,we really don't want it be turned off. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2019-02-01 1:32 ` andy.yan at rock-chips.com @ 2019-02-01 8:38 ` Philipp Tomsich 2019-02-01 8:39 ` Philipp Tomsich 0 siblings, 1 reply; 27+ messages in thread From: Philipp Tomsich @ 2019-02-01 8:38 UTC (permalink / raw) To: u-boot Andy, > On 01.02.2019, at 02:32, andy.yan at rock-chips.com wrote: > > Hi: > > > This function causes a 5-second delay and stops the display working on > > minnie. This code should be in a driver and should only be enabled by > > a device-tree property, so that it does not affect devices which do not > > have this feature. > > > > Signed-off-by: Simon Glass <sjg@chromium.org> > > Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > > --- > > > > arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > 》Applied to u-boot-rockchip, thanks! > > > According to the previous discussing, shouldn‘t this be fixed by by setting CONFIG_BOOT_MODE_REG=0 ? > All rockchip products need this function,we really don't want it be turned off. The actual decision was that this needed to be converted to the driver-model. Setting CONFIG_BOOT_MODE_REG=0 was a workaround that we used only for 2019.1, so I could delay applying Simon’s patch until this cycle. Please convert this into a DM driver, so we can add the driver for 2019.4. Thanks, Philipp. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2019-02-01 8:38 ` Philipp Tomsich @ 2019-02-01 8:39 ` Philipp Tomsich 0 siblings, 0 replies; 27+ messages in thread From: Philipp Tomsich @ 2019-02-01 8:39 UTC (permalink / raw) To: u-boot > On 01.02.2019, at 09:38, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > > Andy, > >> On 01.02.2019, at 02:32, andy.yan at rock-chips.com wrote: >> >> Hi: >> >>> This function causes a 5-second delay and stops the display working on >>> minnie. This code should be in a driver and should only be enabled by >>> a device-tree property, so that it does not affect devices which do not >>> have this feature. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> >>> --- >>> >>> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >> >> 》Applied to u-boot-rockchip, thanks! >> >> >> According to the previous discussing, shouldn‘t this be fixed by by setting CONFIG_BOOT_MODE_REG=0 ? >> All rockchip products need this function,we really don't want it be turned off. > > The actual decision was that this needed to be converted to the driver-model. > Setting CONFIG_BOOT_MODE_REG=0 was a workaround that we used only > for 2019.1, so I could delay applying Simon’s patch until this cycle. Just for the record, this is a verbatim quote from the original review: > If this works for you, we can avoid removing this feature this late in the cycle and > defeature/remove it in the next release, unless moved into a DTS-controlled driver. > > Please convert this into a DM driver, so we can add the driver for 2019.4. > > Thanks, > Philipp. > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 2019-01-31 21:12 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Philipp Tomsich 2019-02-01 1:32 ` andy.yan at rock-chips.com @ 2019-02-01 9:32 ` Kever Yang 2019-02-01 9:34 ` Philipp Tomsich 1 sibling, 1 reply; 27+ messages in thread From: Kever Yang @ 2019-02-01 9:32 UTC (permalink / raw) To: u-boot Hi Philipp, This is not right, this patch should not merged like this!!! I have give my review comment in previous mail, and this will break many boards. My another patch do not break anything, but you insist NAK it without acceptable reason; This patch definitely break other board and I have comment it, but you just ignore other people's review and merge it, good job! Thanks, - Kever On 02/01/2019 05:12 AM, Philipp Tomsich wrote: >> This function causes a 5-second delay and stops the display working on >> minnie. This code should be in a driver and should only be enabled by >> a device-tree property, so that it does not affect devices which do not >> have this feature. >> >> Signed-off-by: Simon Glass <sjg@chromium.org> >> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> >> --- >> >> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> > Applied to u-boot-rockchip, thanks! > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 2019-02-01 9:32 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 Kever Yang @ 2019-02-01 9:34 ` Philipp Tomsich 2019-02-01 10:26 ` Philipp Tomsich 0 siblings, 1 reply; 27+ messages in thread From: Philipp Tomsich @ 2019-02-01 9:34 UTC (permalink / raw) To: u-boot > On 01.02.2019, at 10:32, Kever Yang <kever.yang@rock-chips.com> wrote: > > Hi Philipp, > > This is not right, this patch should not merged like this!!! > > I have give my review comment in previous mail, and this will break > many boards. > > My another patch do not break anything, but you insist NAK it > without acceptable reason; What other patch? I don’t remember seeing that one... > This patch definitely break other board and I have comment it, but > you just ignore other people's review and merge it, good job! > > Thanks, > - Kever > On 02/01/2019 05:12 AM, Philipp Tomsich wrote: >>> This function causes a 5-second delay and stops the display working on >>> minnie. This code should be in a driver and should only be enabled by >>> a device-tree property, so that it does not affect devices which do not >>> have this feature. >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> >>> --- >>> >>> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >> Applied to u-boot-rockchip, thanks! >> _______________________________________________ >> U-Boot mailing list >> U-Boot at lists.denx.de >> https://lists.denx.de/listinfo/u-boot > > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 2019-02-01 9:34 ` Philipp Tomsich @ 2019-02-01 10:26 ` Philipp Tomsich 2019-02-01 14:39 ` Simon Glass 2019-02-12 3:03 ` Andy Yan 0 siblings, 2 replies; 27+ messages in thread From: Philipp Tomsich @ 2019-02-01 10:26 UTC (permalink / raw) To: u-boot Kever, Independent of whether we revert this for the current cycle (and also independent of if I ever find the other patch you had been referring to — I couldn’t find it in my local mailing list archive) and then deprecate it for the next release (unless converted to DM), we still have a number of architectural issues that need to be addressed: 1. This really should be a driver under DTS control. 2. We need to not get away from configuring SOM-specific addresses via Kconfig Both these issues are technical debt that we’ve accumulated over the last 18 months and need to address for the sake of future maintainability. E.g. ‘setting an address to 0x0 via Kconfig to disable a driver/feature’ really isn’t in line with the architectural direction of U-Boot. I don’t have my own house completely in order (I’ve been talking for a year now about finally wrapping the RGMII/GMII selection into an ioctl-call to a driver) yet, but that doesn’t mean that we we should delay this clean-up more than absolutely necessary. Thanks, Philipp. > On 01.02.2019, at 10:34, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > > > >> On 01.02.2019, at 10:32, Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>> wrote: >> >> Hi Philipp, >> >> This is not right, this patch should not merged like this!!! >> >> I have give my review comment in previous mail, and this will break >> many boards. >> >> My another patch do not break anything, but you insist NAK it >> without acceptable reason; > > What other patch? > I don’t remember seeing that one... > >> This patch definitely break other board and I have comment it, but >> you just ignore other people's review and merge it, good job! >> >> Thanks, >> - Kever >> On 02/01/2019 05:12 AM, Philipp Tomsich wrote: >>>> This function causes a 5-second delay and stops the display working on >>>> minnie. This code should be in a driver and should only be enabled by >>>> a device-tree property, so that it does not affect devices which do not >>>> have this feature. >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> >>>> --- >>>> >>>> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- >>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>> >>> Applied to u-boot-rockchip, thanks! >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot at lists.denx.de >>> https://lists.denx.de/listinfo/u-boot >> >> >> > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de> > https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 2019-02-01 10:26 ` Philipp Tomsich @ 2019-02-01 14:39 ` Simon Glass 2019-02-12 3:03 ` Andy Yan 1 sibling, 0 replies; 27+ messages in thread From: Simon Glass @ 2019-02-01 14:39 UTC (permalink / raw) To: u-boot Hi Kever, On Fri, 1 Feb 2019 at 03:26, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > > Kever, > > Independent of whether we revert this for the current cycle (and also independent of > if I ever find the other patch you had been referring to — I couldn’t find it in my local > mailing list archive) and then deprecate it for the next release (unless converted to > DM), we still have a number of architectural issues that need to be addressed: > 1. This really should be a driver under DTS control. > 2. We need to not get away from configuring SOM-specific addresses via Kconfig Let me know if I can help with this. Philipp I am find with any short-term solution that comes up, but agree that it should be tidied up. - Simon > > Both these issues are technical debt that we’ve accumulated over the last 18 months > and need to address for the sake of future maintainability. > E.g. ‘setting an address to 0x0 via Kconfig to disable a driver/feature’ really isn’t in line > with the architectural direction of U-Boot. > > I don’t have my own house completely in order (I’ve been talking for a year now about > finally wrapping the RGMII/GMII selection into an ioctl-call to a driver) yet, but that doesn’t > mean that we we should delay this clean-up more than absolutely necessary. > > Thanks, > Philipp. > > > On 01.02.2019, at 10:34, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote: > > > > > > > >> On 01.02.2019, at 10:32, Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>> wrote: > >> > >> Hi Philipp, > >> > >> This is not right, this patch should not merged like this!!! > >> > >> I have give my review comment in previous mail, and this will break > >> many boards. > >> > >> My another patch do not break anything, but you insist NAK it > >> without acceptable reason; > > > > What other patch? > > I don’t remember seeing that one... > > > >> This patch definitely break other board and I have comment it, but > >> you just ignore other people's review and merge it, good job! > >> > >> Thanks, > >> - Kever > >> On 02/01/2019 05:12 AM, Philipp Tomsich wrote: > >>>> This function causes a 5-second delay and stops the display working on > >>>> minnie. This code should be in a driver and should only be enabled by > >>>> a device-tree property, so that it does not affect devices which do not > >>>> have this feature. > >>>> > >>>> Signed-off-by: Simon Glass <sjg@chromium.org> > >>>> Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > >>>> --- > >>>> > >>>> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- > >>>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>>> > >>> Applied to u-boot-rockchip, thanks! > >>> _______________________________________________ > >>> U-Boot mailing list > >>> U-Boot at lists.denx.de > >>> https://lists.denx.de/listinfo/u-boot > >> > >> > >> > > > > _______________________________________________ > > U-Boot mailing list > > U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de> > > https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot> > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 2019-02-01 10:26 ` Philipp Tomsich 2019-02-01 14:39 ` Simon Glass @ 2019-02-12 3:03 ` Andy Yan 2019-02-12 15:31 ` Simon Glass 1 sibling, 1 reply; 27+ messages in thread From: Andy Yan @ 2019-02-12 3:03 UTC (permalink / raw) To: u-boot Hi Philipp: Sorry for the late reply, we just come back from the Chinese Spring Festival. On 2019/2/1 下午6:26, Philipp Tomsich wrote: > Kever, > > Independent of whether we revert this for the current cycle (and also > independent of > if I ever find the other patch you had been referring to — I couldn’t > find it in my local > mailing list archive) and then deprecate it for the next release > (unless converted to > DM), we still have a number of architectural issues that need to be > addressed: I still doubt is this a right work-flow for patch apply. Before we apply a patch which will break many other boards , should we make sure there is a solution patch applied for these boars first? > 1.This really should be a driver under DTS control. > 2.We need to not get away from configuring SOM-specific addresses via > Kconfig > > Both these issues are technical debt that we’ve accumulated over the > last 18 months > and need to address for the sake of future maintainability. > E.g. ‘setting an address to 0x0 via Kconfig to disable a > driver/feature’ really isn’t in line > with the architectural direction of U-Boot. > For technical side, I think CONFIG_ROCKCHIP_BOOT_MODE_REG is necessary here, we will read this register from save_boot_params when we get out from bootrom, the dtb is not available at this point. On the other hand, almost rockchip based products use a recovery key to enter download(upgrade)mode, this is a muti-funtion key, most of them reuse with vol+- key, we would like the u-boot share dtb with linux kernel. To keep the linux kernel dts as clean as possible ,we don't want to add another dts property to describe this key either. This is why I implement function rockchip_dnl_key_pressed as __weak. > I don’t have my own house completely in order (I’ve been talking for a > year now about > finally wrapping the RGMII/GMII selection into an ioctl-call to a > driver) yet, but that doesn’t > mean that we we should delay this clean-up more than absolutely necessary. > > Thanks, > Philipp. > >> On 01.02.2019, at 10:34, Philipp Tomsich >> <philipp.tomsich@theobroma-systems.com >> <mailto:philipp.tomsich@theobroma-systems.com>> wrote: >> >> >> >>> On 01.02.2019, at 10:32, Kever Yang <kever.yang@rock-chips.com >>> <mailto:kever.yang@rock-chips.com>> wrote: >>> >>> Hi Philipp, >>> >>> This is not right, this patch should not merged like this!!! >>> >>> I have give my review comment in previous mail, and this will break >>> many boards. >>> >>> My another patch do not break anything, but you insist NAK it >>> without acceptable reason; >> >> What other patch? >> I don’t remember seeing that one... >> >>> This patch definitely break other board and I have comment it, but >>> you just ignore other people's review and merge it, good job! >>> >>> Thanks, >>> - Kever >>> On 02/01/2019 05:12 AM, Philipp Tomsich wrote: >>>>> This function causes a 5-second delay and stops the display working on >>>>> minnie. This code should be in a driver and should only be enabled by >>>>> a device-tree property, so that it does not affect devices which >>>>> do not >>>>> have this feature. >>>>> >>>>> Signed-off-by: Simon Glass <sjg@chromium.org >>>>> <mailto:sjg@chromium.org>> >>>>> Reviewed-by: Philipp Tomsich >>>>> <philipp.tomsich@theobroma-systems.com >>>>> <mailto:philipp.tomsich@theobroma-systems.com>> >>>>> --- >>>>> >>>>> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- >>>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>>> >>>> Applied to u-boot-rockchip, thanks! >>>> _______________________________________________ >>>> U-Boot mailing list >>>> U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de> >>>> https://lists.denx.de/listinfo/u-boot >>> >>> >>> >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de> >> https://lists.denx.de/listinfo/u-boot > ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 2019-02-12 3:03 ` Andy Yan @ 2019-02-12 15:31 ` Simon Glass 2019-03-06 10:52 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 Andy Yan 0 siblings, 1 reply; 27+ messages in thread From: Simon Glass @ 2019-02-12 15:31 UTC (permalink / raw) To: u-boot HI Andy, On Tue, 12 Feb 2019 at 04:05, Andy Yan <andy.yan@rock-chips.com> wrote: > > Hi Philipp: > > Sorry for the late reply, we just come back from the Chinese Spring > Festival. > > On 2019/2/1 下午6:26, Philipp Tomsich wrote: > > Kever, > > > > Independent of whether we revert this for the current cycle (and also > > independent of > > if I ever find the other patch you had been referring to — I couldn’t > > find it in my local > > mailing list archive) and then deprecate it for the next release > > (unless converted to > > DM), we still have a number of architectural issues that need to be > > addressed: > > I still doubt is this a right work-flow for patch apply. Before we > apply a patch which will break many other boards , should we make > sure there is a solution patch applied for these boars first? > > > > 1.This really should be a driver under DTS control. > > 2.We need to not get away from configuring SOM-specific addresses via > > Kconfig > > > > Both these issues are technical debt that we’ve accumulated over the > > last 18 months > > and need to address for the sake of future maintainability. > > E.g. ‘setting an address to 0x0 via Kconfig to disable a > > driver/feature’ really isn’t in line > > with the architectural direction of U-Boot. > > > For technical side, I think CONFIG_ROCKCHIP_BOOT_MODE_REG is necessary > here, we will read this register from save_boot_params when we get out > from bootrom, the dtb is not available at this point. Yes this is happening very early, but I wonder why this is necessary? Can it be moved to later? The call to check_back_to_brom_dnl_flag() happens much later so there should be no problem to convert that. > > On the other hand, almost rockchip based products use a recovery key to > enter download(upgrade)mode, this is a muti-funtion key, most of them > reuse with vol+- key, we would like the u-boot share > > dtb with linux kernel. To keep the linux kernel dts as clean as possible > ,we don't want to add another dts property to describe this key either. > This is why I implement function rockchip_dnl_key_pressed as __weak. > OK, but given that it is called later, it seems to me that it could use a driver. Regards, Simon > > > I don’t have my own house completely in order (I’ve been talking for a > > year now about > > finally wrapping the RGMII/GMII selection into an ioctl-call to a > > driver) yet, but that doesn’t > > mean that we we should delay this clean-up more than absolutely necessary. > > > > Thanks, > > Philipp. > > > >> On 01.02.2019, at 10:34, Philipp Tomsich > >> <philipp.tomsich@theobroma-systems.com > >> <mailto:philipp.tomsich@theobroma-systems.com>> wrote: > >> > >> > >> > >>> On 01.02.2019, at 10:32, Kever Yang <kever.yang@rock-chips.com > >>> <mailto:kever.yang@rock-chips.com>> wrote: > >>> > >>> Hi Philipp, > >>> > >>> This is not right, this patch should not merged like this!!! > >>> > >>> I have give my review comment in previous mail, and this will break > >>> many boards. > >>> > >>> My another patch do not break anything, but you insist NAK it > >>> without acceptable reason; > >> > >> What other patch? > >> I don’t remember seeing that one... > >> > >>> This patch definitely break other board and I have comment it, but > >>> you just ignore other people's review and merge it, good job! > >>> > >>> Thanks, > >>> - Kever > >>> On 02/01/2019 05:12 AM, Philipp Tomsich wrote: > >>>>> This function causes a 5-second delay and stops the display working on > >>>>> minnie. This code should be in a driver and should only be enabled by > >>>>> a device-tree property, so that it does not affect devices which > >>>>> do not > >>>>> have this feature. > >>>>> > >>>>> Signed-off-by: Simon Glass <sjg@chromium.org > >>>>> <mailto:sjg@chromium.org>> > >>>>> Reviewed-by: Philipp Tomsich > >>>>> <philipp.tomsich@theobroma-systems.com > >>>>> <mailto:philipp.tomsich@theobroma-systems.com>> > >>>>> --- > >>>>> > >>>>> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- > >>>>> 1 file changed, 7 insertions(+), 1 deletion(-) > >>>>> > >>>> Applied to u-boot-rockchip, thanks! > >>>> _______________________________________________ > >>>> U-Boot mailing list > >>>> U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de> > >>>> https://lists.denx.de/listinfo/u-boot > >>> > >>> > >>> > >> > >> _______________________________________________ > >> U-Boot mailing list > >> U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de> > >> https://lists.denx.de/listinfo/u-boot > > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > https://lists.denx.de/listinfo/u-boot ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 2019-02-12 15:31 ` Simon Glass @ 2019-03-06 10:52 ` Andy Yan 2019-03-11 2:30 ` Simon Glass 0 siblings, 1 reply; 27+ messages in thread From: Andy Yan @ 2019-03-06 10:52 UTC (permalink / raw) To: u-boot Hi Simon: On 2019/2/12 下午11:31, Simon Glass wrote: > HI Andy, > > On Tue, 12 Feb 2019 at 04:05, Andy Yan <andy.yan@rock-chips.com> wrote: >> Hi Philipp: >> >> Sorry for the late reply, we just come back from the Chinese Spring >> Festival. >> >> On 2019/2/1 下午6:26, Philipp Tomsich wrote: >>> Kever, >>> >>> Independent of whether we revert this for the current cycle (and also >>> independent of >>> if I ever find the other patch you had been referring to — I couldn’t >>> find it in my local >>> mailing list archive) and then deprecate it for the next release >>> (unless converted to >>> DM), we still have a number of architectural issues that need to be >>> addressed: >> I still doubt is this a right work-flow for patch apply. Before we >> apply a patch which will break many other boards , should we make >> sure there is a solution patch applied for these boars first? >> >> >>> 1.This really should be a driver under DTS control. >>> 2.We need to not get away from configuring SOM-specific addresses via >>> Kconfig >>> >>> Both these issues are technical debt that we’ve accumulated over the >>> last 18 months >>> and need to address for the sake of future maintainability. >>> E.g. ‘setting an address to 0x0 via Kconfig to disable a >>> driver/feature’ really isn’t in line >>> with the architectural direction of U-Boot. >>> >> For technical side, I think CONFIG_ROCKCHIP_BOOT_MODE_REG is necessary >> here, we will read this register from save_boot_params when we get out >> from bootrom, the dtb is not available at this point. > Yes this is happening very early, but I wonder why this is necessary? > Can it be moved to later? > > The call to check_back_to_brom_dnl_flag() happens much later so there > should be no problem to convert that. > >> On the other hand, almost rockchip based products use a recovery key to >> enter download(upgrade)mode, this is a muti-funtion key, most of them >> reuse with vol+- key, we would like the u-boot share >> >> dtb with linux kernel. To keep the linux kernel dts as clean as possible >> ,we don't want to add another dts property to describe this key either. >> This is why I implement function rockchip_dnl_key_pressed as __weak. >> > OK, but given that it is called later, it seems to me that it could > use a driver. We can't let it called later. See the discuss here : http://patchwork.ozlabs.org/patch/812228/ > Regards, > Simon > > >>> I don’t have my own house completely in order (I’ve been talking for a >>> year now about >>> finally wrapping the RGMII/GMII selection into an ioctl-call to a >>> driver) yet, but that doesn’t >>> mean that we we should delay this clean-up more than absolutely necessary. >>> >>> Thanks, >>> Philipp. >>> >>>> On 01.02.2019, at 10:34, Philipp Tomsich >>>> <philipp.tomsich@theobroma-systems.com >>>> <mailto:philipp.tomsich@theobroma-systems.com>> wrote: >>>> >>>> >>>> >>>>> On 01.02.2019, at 10:32, Kever Yang <kever.yang@rock-chips.com >>>>> <mailto:kever.yang@rock-chips.com>> wrote: >>>>> >>>>> Hi Philipp, >>>>> >>>>> This is not right, this patch should not merged like this!!! >>>>> >>>>> I have give my review comment in previous mail, and this will break >>>>> many boards. >>>>> >>>>> My another patch do not break anything, but you insist NAK it >>>>> without acceptable reason; >>>> What other patch? >>>> I don’t remember seeing that one... >>>> >>>>> This patch definitely break other board and I have comment it, but >>>>> you just ignore other people's review and merge it, good job! >>>>> >>>>> Thanks, >>>>> - Kever >>>>> On 02/01/2019 05:12 AM, Philipp Tomsich wrote: >>>>>>> This function causes a 5-second delay and stops the display working on >>>>>>> minnie. This code should be in a driver and should only be enabled by >>>>>>> a device-tree property, so that it does not affect devices which >>>>>>> do not >>>>>>> have this feature. >>>>>>> >>>>>>> Signed-off-by: Simon Glass <sjg@chromium.org >>>>>>> <mailto:sjg@chromium.org>> >>>>>>> Reviewed-by: Philipp Tomsich >>>>>>> <philipp.tomsich@theobroma-systems.com >>>>>>> <mailto:philipp.tomsich@theobroma-systems.com>> >>>>>>> --- >>>>>>> >>>>>>> arch/arm/mach-rockchip/boot_mode.c | 8 +++++++- >>>>>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>>>>> >>>>>> Applied to u-boot-rockchip, thanks! >>>>>> _______________________________________________ >>>>>> U-Boot mailing list >>>>>> U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de> >>>>>> https://lists.denx.de/listinfo/u-boot >>>>> >>>>> >>>> _______________________________________________ >>>> U-Boot mailing list >>>> U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de> >>>> https://lists.denx.de/listinfo/u-boot >> _______________________________________________ >> U-Boot mailing list >> U-Boot at lists.denx.de >> https://lists.denx.de/listinfo/u-boot > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 2019-03-06 10:52 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 Andy Yan @ 2019-03-11 2:30 ` Simon Glass 2019-03-11 13:04 ` Philipp Tomsich 2019-03-12 6:28 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 Andy Yan 0 siblings, 2 replies; 27+ messages in thread From: Simon Glass @ 2019-03-11 2:30 UTC (permalink / raw) To: u-boot Hi Andy, On Wed, 6 Mar 2019 at 03:52, Andy Yan <andy.yan@rock-chips.com> wrote: > > Hi Simon: > > On 2019/2/12 下午11:31, Simon Glass wrote: > > HI Andy, > > > > On Tue, 12 Feb 2019 at 04:05, Andy Yan <andy.yan@rock-chips.com> wrote: > >> Hi Philipp: > >> > >> Sorry for the late reply, we just come back from the Chinese Spring > >> Festival. > >> > >> On 2019/2/1 下午6:26, Philipp Tomsich wrote: > >>> Kever, > >>> > >>> Independent of whether we revert this for the current cycle (and also > >>> independent of > >>> if I ever find the other patch you had been referring to — I couldn’t > >>> find it in my local > >>> mailing list archive) and then deprecate it for the next release > >>> (unless converted to > >>> DM), we still have a number of architectural issues that need to be > >>> addressed: > >> I still doubt is this a right work-flow for patch apply. Before we > >> apply a patch which will break many other boards , should we make > >> sure there is a solution patch applied for these boars first? > >> > >> > >>> 1.This really should be a driver under DTS control. > >>> 2.We need to not get away from configuring SOM-specific addresses via > >>> Kconfig > >>> > >>> Both these issues are technical debt that we’ve accumulated over the > >>> last 18 months > >>> and need to address for the sake of future maintainability. > >>> E.g. ‘setting an address to 0x0 via Kconfig to disable a > >>> driver/feature’ really isn’t in line > >>> with the architectural direction of U-Boot. > >>> > >> For technical side, I think CONFIG_ROCKCHIP_BOOT_MODE_REG is necessary > >> here, we will read this register from save_boot_params when we get out > >> from bootrom, the dtb is not available at this point. > > Yes this is happening very early, but I wonder why this is necessary? > > Can it be moved to later? > > > > The call to check_back_to_brom_dnl_flag() happens much later so there > > should be no problem to convert that. > > > >> On the other hand, almost rockchip based products use a recovery key to > >> enter download(upgrade)mode, this is a muti-funtion key, most of them > >> reuse with vol+- key, we would like the u-boot share > >> > >> dtb with linux kernel. To keep the linux kernel dts as clean as possible > >> ,we don't want to add another dts property to describe this key either. > >> This is why I implement function rockchip_dnl_key_pressed as __weak. > >> > > OK, but given that it is called later, it seems to me that it could > > use a driver. > > > > We can't let it called later. See the discuss here : > http://patchwork.ozlabs.org/patch/812228/ Yes I read that, and took a look myself, and certainly it does not look very easy and I take your point about potential bugs being introduced. It seems like you have tried quite hard, so I'm not going to object if you can't find a way. My main objection was that it broke other boards. Is there any way the check could be delayed enough to actually be able to read the device tree? It could call spl_early_init() quite early and then do that. Regards, Simon ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 2019-03-11 2:30 ` Simon Glass @ 2019-03-11 13:04 ` Philipp Tomsich 2019-03-12 1:23 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Kever Yang 2019-03-12 6:28 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 Andy Yan 1 sibling, 1 reply; 27+ messages in thread From: Philipp Tomsich @ 2019-03-11 13:04 UTC (permalink / raw) To: u-boot > On 11.03.2019, at 03:30, Simon Glass <sjg@chromium.org> wrote: > > Hi Andy, > > On Wed, 6 Mar 2019 at 03:52, Andy Yan <andy.yan@rock-chips.com> wrote: >> >> Hi Simon: >> >> On 2019/2/12 下午11:31, Simon Glass wrote: >>> HI Andy, >>> >>> On Tue, 12 Feb 2019 at 04:05, Andy Yan <andy.yan@rock-chips.com> wrote: >>>> Hi Philipp: >>>> >>>> Sorry for the late reply, we just come back from the Chinese Spring >>>> Festival. >>>> >>>> On 2019/2/1 下午6:26, Philipp Tomsich wrote: >>>>> Kever, >>>>> >>>>> Independent of whether we revert this for the current cycle (and also >>>>> independent of >>>>> if I ever find the other patch you had been referring to — I couldn’t >>>>> find it in my local >>>>> mailing list archive) and then deprecate it for the next release >>>>> (unless converted to >>>>> DM), we still have a number of architectural issues that need to be >>>>> addressed: >>>> I still doubt is this a right work-flow for patch apply. Before we >>>> apply a patch which will break many other boards , should we make >>>> sure there is a solution patch applied for these boars first? >>>> >>>> >>>>> 1.This really should be a driver under DTS control. >>>>> 2.We need to not get away from configuring SOM-specific addresses via >>>>> Kconfig >>>>> >>>>> Both these issues are technical debt that we’ve accumulated over the >>>>> last 18 months >>>>> and need to address for the sake of future maintainability. >>>>> E.g. ‘setting an address to 0x0 via Kconfig to disable a >>>>> driver/feature’ really isn’t in line >>>>> with the architectural direction of U-Boot. >>>>> >>>> For technical side, I think CONFIG_ROCKCHIP_BOOT_MODE_REG is necessary >>>> here, we will read this register from save_boot_params when we get out >>>> from bootrom, the dtb is not available at this point. >>> Yes this is happening very early, but I wonder why this is necessary? >>> Can it be moved to later? >>> >>> The call to check_back_to_brom_dnl_flag() happens much later so there >>> should be no problem to convert that. >>> >>>> On the other hand, almost rockchip based products use a recovery key to >>>> enter download(upgrade)mode, this is a muti-funtion key, most of them >>>> reuse with vol+- key, we would like the u-boot share >>>> >>>> dtb with linux kernel. To keep the linux kernel dts as clean as possible >>>> ,we don't want to add another dts property to describe this key either. >>>> This is why I implement function rockchip_dnl_key_pressed as __weak. >>>> >>> OK, but given that it is called later, it seems to me that it could >>> use a driver. >> >> >> >> We can't let it called later. See the discuss here : >> http://patchwork.ozlabs.org/patch/812228/ > > Yes I read that, and took a look myself, and certainly it does not > look very easy and I take your point about potential bugs being > introduced. > > It seems like you have tried quite hard, so I'm not going to object if > you can't find a way. My main objection was that it broke other > boards. > > Is there any way the check could be delayed enough to actually be able > to read the device tree? It could call spl_early_init() quite early > and then do that. Note that the minimum improvement that I’d expect to get this fully enabled again would be a clean-up of the Kconfig options, so it is easy and (from the documentation) predictable for boards to turn this off. I still want this to be turned back on in the current cycle. Thanks, Philipp. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2019-03-11 13:04 ` Philipp Tomsich @ 2019-03-12 1:23 ` Kever Yang 2019-03-18 11:58 ` Simon Glass 0 siblings, 1 reply; 27+ messages in thread From: Kever Yang @ 2019-03-12 1:23 UTC (permalink / raw) To: u-boot On 03/11/2019 09:04 PM, Philipp Tomsich wrote: > Note that the minimum improvement that I’d expect to get this fully > enabled again would be a clean-up of the Kconfig options, so it is > easy and (from the documentation) predictable for boards to turn > this off. > > I still want this to be turned back on in the current cycle. Great maintainer!!!! You objection for other patch is that it break other boards(which is non exist actually), and you merge this patch(break more than 10 boards very clearly) with objection comment. And then you require for all the Kconfig options update before the feature back. WoW!!! Thanks, - Kever ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2019-03-12 1:23 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Kever Yang @ 2019-03-18 11:58 ` Simon Glass 2019-03-18 12:07 ` Philipp Tomsich 0 siblings, 1 reply; 27+ messages in thread From: Simon Glass @ 2019-03-18 11:58 UTC (permalink / raw) To: u-boot Hi, On Tue, 12 Mar 2019 at 09:26, Kever Yang <kever.yang@rock-chips.com> wrote: > > > > On 03/11/2019 09:04 PM, Philipp Tomsich wrote: > > Note that the minimum improvement that I’d expect to get this fully > > enabled again would be a clean-up of the Kconfig options, so it is > > easy and (from the documentation) predictable for boards to turn > > this off. > > > > I still want this to be turned back on in the current cycle. > > Great maintainer!!!! > > You objection for other patch is that it break other boards(which is non > exist actually), > and you merge this patch(break more than 10 boards very clearly) with > objection comment. > And then you require for all the Kconfig options update before the > feature back. > > WoW!!! How do we resolve this? I have not had time to dig into this myself, and there seem to be big problems with doing this 'properly'. Should we bring back the Kconfig for this release? I may be able to look at it for the next release, although time is not my friend at present. Regards, Simon ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now 2019-03-18 11:58 ` Simon Glass @ 2019-03-18 12:07 ` Philipp Tomsich 0 siblings, 0 replies; 27+ messages in thread From: Philipp Tomsich @ 2019-03-18 12:07 UTC (permalink / raw) To: u-boot Simon, > On 18.03.2019, at 12:58, Simon Glass <sjg@chromium.org> wrote: > > Hi, > > On Tue, 12 Mar 2019 at 09:26, Kever Yang <kever.yang at rock-chips.com <mailto:kever.yang@rock-chips.com>> wrote: >> >> >> >> On 03/11/2019 09:04 PM, Philipp Tomsich wrote: >>> Note that the minimum improvement that I’d expect to get this fully >>> enabled again would be a clean-up of the Kconfig options, so it is >>> easy and (from the documentation) predictable for boards to turn >>> this off. >>> >>> I still want this to be turned back on in the current cycle. >> >> Great maintainer!!!! >> >> You objection for other patch is that it break other boards(which is non >> exist actually), >> and you merge this patch(break more than 10 boards very clearly) with >> objection comment. >> And then you require for all the Kconfig options update before the >> feature back. >> >> WoW!!! > > How do we resolve this? I have not had time to dig into this myself, > and there seem to be big problems with doing this 'properly'. > > Should we bring back the Kconfig for this release? I may be able to > look at it for the next release, although time is not my friend at > present. I may have the time to update the Kconfig this week. If not, I’ll revert the deprecation-patch for this release. Philipp. ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 2019-03-11 2:30 ` Simon Glass 2019-03-11 13:04 ` Philipp Tomsich @ 2019-03-12 6:28 ` Andy Yan 1 sibling, 0 replies; 27+ messages in thread From: Andy Yan @ 2019-03-12 6:28 UTC (permalink / raw) To: u-boot Hi Simon: On 2019/3/11 上午10:30, Simon Glass wrote: > Hi Andy, > > On Wed, 6 Mar 2019 at 03:52, Andy Yan <andy.yan@rock-chips.com> wrote: >> Hi Simon: >> >> On 2019/2/12 下午11:31, Simon Glass wrote: >>> HI Andy, >>> >>> On Tue, 12 Feb 2019 at 04:05, Andy Yan <andy.yan@rock-chips.com> wrote: >>>> Hi Philipp: >>>> >>>> Sorry for the late reply, we just come back from the Chinese Spring >>>> Festival. >>>> >>>> On 2019/2/1 下午6:26, Philipp Tomsich wrote: >>>>> Kever, >>>>> >>>>> Independent of whether we revert this for the current cycle (and also >>>>> independent of >>>>> if I ever find the other patch you had been referring to — I couldn’t >>>>> find it in my local >>>>> mailing list archive) and then deprecate it for the next release >>>>> (unless converted to >>>>> DM), we still have a number of architectural issues that need to be >>>>> addressed: >>>> I still doubt is this a right work-flow for patch apply. Before we >>>> apply a patch which will break many other boards , should we make >>>> sure there is a solution patch applied for these boars first? >>>> >>>> >>>>> 1.This really should be a driver under DTS control. >>>>> 2.We need to not get away from configuring SOM-specific addresses via >>>>> Kconfig >>>>> >>>>> Both these issues are technical debt that we’ve accumulated over the >>>>> last 18 months >>>>> and need to address for the sake of future maintainability. >>>>> E.g. ‘setting an address to 0x0 via Kconfig to disable a >>>>> driver/feature’ really isn’t in line >>>>> with the architectural direction of U-Boot. >>>>> >>>> For technical side, I think CONFIG_ROCKCHIP_BOOT_MODE_REG is necessary >>>> here, we will read this register from save_boot_params when we get out >>>> from bootrom, the dtb is not available at this point. >>> Yes this is happening very early, but I wonder why this is necessary? >>> Can it be moved to later? >>> >>> The call to check_back_to_brom_dnl_flag() happens much later so there >>> should be no problem to convert that. >>> >>>> On the other hand, almost rockchip based products use a recovery key to >>>> enter download(upgrade)mode, this is a muti-funtion key, most of them >>>> reuse with vol+- key, we would like the u-boot share >>>> >>>> dtb with linux kernel. To keep the linux kernel dts as clean as possible >>>> ,we don't want to add another dts property to describe this key either. >>>> This is why I implement function rockchip_dnl_key_pressed as __weak. >>>> >>> OK, but given that it is called later, it seems to me that it could >>> use a driver. >> >> >> We can't let it called later. See the discuss here : >> http://patchwork.ozlabs.org/patch/812228/ > Yes I read that, and took a look myself, and certainly it does not > look very easy and I take your point about potential bugs being > introduced. > > It seems like you have tried quite hard, so I'm not going to object if > you can't find a way. My main objection was that it broke other > boards. I took the code many times, what this code done is : call adc_channel_single_shot to check if a download key is pressed, so it is very strange it broke the display on minnie. Would you please show the boot log when the display broke. > > Is there any way the check could be delayed enough to actually be able > to read the device tree? It could call spl_early_init() quite early > and then do that. Actually, the interrupt overwrite happens in start.s, so this don't do help. > > Regards, > Simon > > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 1/2] rockchip: chromebook_minnie: Enable sound 2018-12-29 13:16 [U-Boot] [PATCH 1/2] rockchip: chromebook_minnie: Enable sound Simon Glass 2018-12-29 13:16 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Simon Glass @ 2019-01-27 17:31 ` Philipp Tomsich 2019-01-28 9:12 ` Philipp Tomsich 2 siblings, 0 replies; 27+ messages in thread From: Philipp Tomsich @ 2019-01-27 17:31 UTC (permalink / raw) To: u-boot > Enable sound for this board, which has the same codec as jerry. > > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > arch/arm/dts/rk3288-veyron-minnie.dts | 12 ++++++++++++ > configs/chromebook_minnie_defconfig | 5 +++++ > 2 files changed, 17 insertions(+) > Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> ^ permalink raw reply [flat|nested] 27+ messages in thread
* [U-Boot] [U-Boot, 1/2] rockchip: chromebook_minnie: Enable sound 2018-12-29 13:16 [U-Boot] [PATCH 1/2] rockchip: chromebook_minnie: Enable sound Simon Glass 2018-12-29 13:16 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Simon Glass 2019-01-27 17:31 ` [U-Boot] [U-Boot, 1/2] rockchip: chromebook_minnie: Enable sound Philipp Tomsich @ 2019-01-28 9:12 ` Philipp Tomsich 2 siblings, 0 replies; 27+ messages in thread From: Philipp Tomsich @ 2019-01-28 9:12 UTC (permalink / raw) To: u-boot > Enable sound for this board, which has the same codec as jerry. > > Signed-off-by: Simon Glass <sjg@chromium.org> > Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com> > --- > > arch/arm/dts/rk3288-veyron-minnie.dts | 12 ++++++++++++ > configs/chromebook_minnie_defconfig | 5 +++++ > 2 files changed, 17 insertions(+) > Applied to u-boot-rockchip, thanks! ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2019-03-18 12:07 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-29 13:16 [U-Boot] [PATCH 1/2] rockchip: chromebook_minnie: Enable sound Simon Glass 2018-12-29 13:16 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Simon Glass 2018-12-29 17:57 ` Philipp Tomsich 2018-12-29 18:00 ` Philipp Tomsich 2019-01-10 12:56 ` Simon Glass 2019-01-03 7:32 ` [U-Boot] [PATCH 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now[Please note, mail behalf by 3vnmnxammciszqnjoyvtpbt.vynrlcly.fhunyvjr-jopwz.jvt@chromium-sendgmr.bounces.google.com] Kever Yang 2019-01-03 21:29 ` Simon Glass 2019-01-03 21:36 ` Philipp Tomsich 2019-01-31 21:12 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Philipp Tomsich 2019-02-01 1:32 ` andy.yan at rock-chips.com 2019-02-01 8:38 ` Philipp Tomsich 2019-02-01 8:39 ` Philipp Tomsich 2019-02-01 9:32 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】 Kever Yang 2019-02-01 9:34 ` Philipp Tomsich 2019-02-01 10:26 ` Philipp Tomsich 2019-02-01 14:39 ` Simon Glass 2019-02-12 3:03 ` Andy Yan 2019-02-12 15:31 ` Simon Glass 2019-03-06 10:52 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 Andy Yan 2019-03-11 2:30 ` Simon Glass 2019-03-11 13:04 ` Philipp Tomsich 2019-03-12 1:23 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now Kever Yang 2019-03-18 11:58 ` Simon Glass 2019-03-18 12:07 ` Philipp Tomsich 2019-03-12 6:28 ` [U-Boot] [U-Boot, 2/2] rockchip: Drop call to rockchip_dnl_mode_check() for now【请注意,邮件由u-boot-bounces@lists.denx.de代发】【请注意,邮件由sjg@google.com代发】 Andy Yan 2019-01-27 17:31 ` [U-Boot] [U-Boot, 1/2] rockchip: chromebook_minnie: Enable sound Philipp Tomsich 2019-01-28 9:12 ` Philipp Tomsich
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.