[PATCH next v3 0/6] rockchip: display PMIC variant properly + misc fixes for Theobroma boards
This fixes how the Rockchip PMIC variant is shown for all but RK808 by stripping the LSB.
Also fix the size of the environment on Jaguar to match the default (smaller) size.
Fix SPL_PAD_TO on Ringneck.
Remove duplicated default value of ENV_OFFSET in puma defconfig.
Remove unnecessary override of the fit offset in u-boot dtsi for Puma and Ringneck.
Fix TPL_MAX_SIZE for Ringneck to match the (valid) default value for PX30 boards.
Signed-off-by: Quentin Schulz quentin.schulz@cherry.de --- Changes in v3: - add missing bitfield include in PMIC patch - Link to v2: https://lore.kernel.org/r/20240605-misc-tsd-v2-0-12479432c90f@cherry.de
Changes in v2: - add patch for fixing TPL_MAX_SIZE on PX30 Ringneck - print show_variant when PMIC variant not found - use bitfield_extract_by_mask instead of using hardcoded shift operator - Link to v1: https://lore.kernel.org/r/20240524-misc-tsd-v1-0-b7bde1344b9e@cherry.de
--- Quentin Schulz (6): rockchip: jaguar-rk3588: use default env size for Rockchip on MMC rockchip: rk3399-puma: remove default value from defconfig rockchip: rk3399-puma: remove unnecessary simple-bin:fit:offset override rockchip: px30-ringneck: Update SPL_PAD_TO Kconfig option power: rk8xx: properly print all supported PMICs name rockchip: ringneck-px30: fix TPL_MAX_SIZE
arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi | 8 -------- arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi | 6 ------ configs/jaguar-rk3588_defconfig | 1 - configs/puma-rk3399_defconfig | 1 - configs/ringneck-px30_defconfig | 3 +-- drivers/power/pmic/rk8xx.c | 6 +++--- 6 files changed, 4 insertions(+), 21 deletions(-) --- base-commit: 8887ffb625f65475fd6619f8b05fb7a5a12b016b change-id: 20240524-misc-tsd-e08551c01c8a
Best regards,
From: Quentin Schulz quentin.schulz@cherry.de
The default env size is 0x8000 when building for Rockchip SoCs with support for environment stored in MMC.
Jaguar hasn't entered mass production just yet, so it's a breaking change we can afford in the name of consistency.
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de --- configs/jaguar-rk3588_defconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/configs/jaguar-rk3588_defconfig b/configs/jaguar-rk3588_defconfig index b69cf4cd057..36bf34d97c8 100644 --- a/configs/jaguar-rk3588_defconfig +++ b/configs/jaguar-rk3588_defconfig @@ -5,7 +5,6 @@ CONFIG_ARCH_ROCKCHIP=y CONFIG_SPL_GPIO=y CONFIG_SF_DEFAULT_SPEED=24000000 CONFIG_SF_DEFAULT_MODE=0x2000 -CONFIG_ENV_SIZE=0x1f000 CONFIG_DEFAULT_DEVICE_TREE="rockchip/rk3588-jaguar" CONFIG_ROCKCHIP_RK3588=y CONFIG_ROCKCHIP_BOOT_MODE_REG=0x0
From: Quentin Schulz quentin.schulz@cherry.de
CONFIG_ENV_OFFSET already defaults to 0x3F8000, however it is stored in lowercase hexdigits instead of uppercase like in the defconfig.
No change in behavior intended.
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de --- configs/puma-rk3399_defconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/configs/puma-rk3399_defconfig b/configs/puma-rk3399_defconfig index 34a0b575991..42819102d70 100644 --- a/configs/puma-rk3399_defconfig +++ b/configs/puma-rk3399_defconfig @@ -6,7 +6,6 @@ CONFIG_SPL_GPIO=y CONFIG_NR_DRAM_BANKS=1 CONFIG_SF_DEFAULT_SPEED=20000000 CONFIG_ENV_SIZE=0x3000 -CONFIG_ENV_OFFSET=0x3F8000 CONFIG_DEFAULT_DEVICE_TREE="rockchip/rk3399-puma-haikou" CONFIG_DM_RESET=y CONFIG_ROCKCHIP_RK3399=y
From: Quentin Schulz quentin.schulz@cherry.de
Since commit 6007b69d544e ("rockchip: rk3399-puma: Update SPL_PAD_TO Kconfig option"), SPL_PAD_TO matches (CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR - 64) * 512 and the default value for simple-bin:fit:offset in rockchip-u-boot.dtsi is SPL_PAD_TO, so let's remove this override.
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de --- arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi b/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi index 5a9bd320ec4..55895d0dd19 100644 --- a/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi +++ b/arch/arm/dts/rk3399-puma-haikou-u-boot.dtsi @@ -33,12 +33,6 @@ };
&binman { - simple-bin { - fit { - offset = <((CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR - 64) * 512)>; - }; - }; - #ifdef CONFIG_ROCKCHIP_SPI_IMAGE simple-bin-spi { fit {
From: Quentin Schulz quentin.schulz@cherry.de
On px30-ringneck the FIT payload is located at sector 0x200 compared to the more Rockchip common sector 0x4000 offset: SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x200
Because FIT payload is located at sector 0x200 and the TPL+SPL is located at sector 64, the combined size of TPL+SPL cannot take up more than 224KiB: (0x200 - 64) x 512 = 0x38000 (224 KiB)
Adjust SPL_PAD_TO to match the used 0x200 sector offset.
While at it, update the px30-ringneck-u-boot.dtsi to remove the now unnecessary override of simple-bin:fit:offset since SPL_PAD_TO matches with the current formula.
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de --- arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi | 8 -------- configs/ringneck-px30_defconfig | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi b/arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi index e04766ad09c..29ea2763636 100644 --- a/arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi +++ b/arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi @@ -15,14 +15,6 @@ }; };
-&binman { - simple-bin { - fit { - offset = <((CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR - 64) * 512)>; - }; - }; -}; - &emmc_clk { bootph-all; }; diff --git a/configs/ringneck-px30_defconfig b/configs/ringneck-px30_defconfig index dedf35d4347..a22d25e0089 100644 --- a/configs/ringneck-px30_defconfig +++ b/configs/ringneck-px30_defconfig @@ -25,7 +25,7 @@ CONFIG_DEFAULT_FDT_FILE="rockchip/px30-ringneck-haikou.dtb" CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_MISC_INIT_R=y CONFIG_SPL_MAX_SIZE=0x20000 -CONFIG_SPL_PAD_TO=0x0 +CONFIG_SPL_PAD_TO=0x38000 CONFIG_SPL_BOARD_INIT=y CONFIG_SPL_BOOTROM_SUPPORT=y # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
From: Quentin Schulz quentin.schulz@cherry.de
The ID of the PMIC is stored in the 2 16b registers but the only part that matters right now is the 3 MSB, which make the 3 digits (in hex) of the part number.
Right now, only RK808 was properly displayed, with this all currently supported PMICs should display the proper part number.
Additionally, when the PMIC variant is not found, print that value instead of the masked unshifted value as all PMICs we support for now have their LSB ignored to represent the actual part number.
Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30 Ringneck).
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de --- drivers/power/pmic/rk8xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 12ff26a0855..617bb511e4e 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -6,6 +6,7 @@
#include <dm.h> #include <dm/lists.h> +#include <bitfield.h> #include <errno.h> #include <log.h> #include <linux/bitfield.h> @@ -277,10 +278,9 @@ static int rk8xx_probe(struct udevice *dev) return ret;
priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK; - show_variant = priv->variant; + show_variant = bitfield_extract_by_mask(priv->variant, RK8XX_ID_MSK); switch (priv->variant) { case RK808_ID: - show_variant = 0x808; /* RK808 hardware ID is 0 */ break; case RK805_ID: case RK816_ID: @@ -311,7 +311,7 @@ static int rk8xx_probe(struct udevice *dev) init_data_num = ARRAY_SIZE(rk806_init_reg); break; default: - printf("Unknown PMIC: RK%x!!\n", priv->variant); + printf("Unknown PMIC: RK%x!!\n", show_variant); return -EINVAL; }
On 2024-06-06 10:45, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@cherry.de
The ID of the PMIC is stored in the 2 16b registers but the only part that matters right now is the 3 MSB, which make the 3 digits (in hex) of the part number.
Right now, only RK808 was properly displayed, with this all currently supported PMICs should display the proper part number.
Additionally, when the PMIC variant is not found, print that value instead of the masked unshifted value as all PMICs we support for now have their LSB ignored to represent the actual part number.
Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30 Ringneck).
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de
Obviously, my Reviewed-by tag for the v2 of this patch still applies.
drivers/power/pmic/rk8xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 12ff26a0855..617bb511e4e 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -6,6 +6,7 @@
#include <dm.h> #include <dm/lists.h> +#include <bitfield.h> #include <errno.h> #include <log.h> #include <linux/bitfield.h> @@ -277,10 +278,9 @@ static int rk8xx_probe(struct udevice *dev) return ret;
priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK;
- show_variant = priv->variant;
- show_variant = bitfield_extract_by_mask(priv->variant, RK8XX_ID_MSK); switch (priv->variant) { case RK808_ID:
show_variant = 0x808; /* RK808 hardware ID is 0 */@@ -311,7 +311,7 @@ static int rk8xx_probe(struct udevice *dev) init_data_num = ARRAY_SIZE(rk806_init_reg); break; default:
printf("Unknown PMIC: RK%x!!\n", priv->variant);
printf("Unknown PMIC: RK%x!!\n", show_variant);
Hi all,
On 6/6/24 10:45 AM, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@cherry.de
The ID of the PMIC is stored in the 2 16b registers but the only part that matters right now is the 3 MSB, which make the 3 digits (in hex) of the part number.
Right now, only RK808 was properly displayed, with this all currently supported PMICs should display the proper part number.
Additionally, when the PMIC variant is not found, print that value instead of the masked unshifted value as all PMICs we support for now have their LSB ignored to represent the actual part number.
Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30 Ringneck).
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de
drivers/power/pmic/rk8xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 12ff26a0855..617bb511e4e 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -6,6 +6,7 @@
#include <dm.h> #include <dm/lists.h> +#include <bitfield.h> #include <errno.h> #include <log.h> #include <linux/bitfield.h> @@ -277,10 +278,9 @@ static int rk8xx_probe(struct udevice *dev) return ret;
priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK;
- show_variant = priv->variant;
- show_variant = bitfield_extract_by_mask(priv->variant, RK8XX_ID_MSK); switch (priv->variant) { case RK808_ID:
show_variant = 0x808; /* RK808 hardware ID is 0 */
This line removal is actually incorrect, I should have left this in as we cannot use the same logic as other PMICs for RK808 as it returns 0, so 0 masked/shifted is still zero.
I saw that Kever has already sent a merge request for next with this patch, so we have two options: reject the merge and I send another patch for next branch, or Kever cancels the merge request, I send a v4 for this patch and Kever sends a new merge request? How do we want to proceed with this. I have a feeling the additional patch is going to be easier for everyone as 1) it's for next branch, 2) isn't breaking anything except some info message, which was already wrong (but for all non-RK808 PMICs :) ).
Cheers, Quentin
Hello Quentin,
On 2024-06-17 16:10, Quentin Schulz wrote:
On 6/6/24 10:45 AM, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@cherry.de
The ID of the PMIC is stored in the 2 16b registers but the only part that matters right now is the 3 MSB, which make the 3 digits (in hex) of the part number.
Right now, only RK808 was properly displayed, with this all currently supported PMICs should display the proper part number.
Additionally, when the PMIC variant is not found, print that value instead of the masked unshifted value as all PMICs we support for now have their LSB ignored to represent the actual part number.
Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30 Ringneck).
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de
drivers/power/pmic/rk8xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 12ff26a0855..617bb511e4e 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -6,6 +6,7 @@ #include <dm.h> #include <dm/lists.h> +#include <bitfield.h> #include <errno.h> #include <log.h> #include <linux/bitfield.h> @@ -277,10 +278,9 @@ static int rk8xx_probe(struct udevice *dev) return ret; priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK;
- show_variant = priv->variant;
- show_variant = bitfield_extract_by_mask(priv->variant,
RK8XX_ID_MSK); switch (priv->variant) { case RK808_ID:
show_variant = 0x808; /* RK808 hardware ID is 0 */This line removal is actually incorrect, I should have left this in as we cannot use the same logic as other PMICs for RK808 as it returns 0, so 0 masked/shifted is still zero.
Thanks for catching this! Moreover, I think we should skip reading the msb and lsb values entirely for the RK808, because its datasheet lists the default ID_MSB (0x17) and ID_LSB (0x18) registers as reserved, and provides no information about gathering the chip variant.
I saw that Kever has already sent a merge request for next with this patch, so we have two options: reject the merge and I send another patch for next branch, or Kever cancels the merge request, I send a v4 for this patch and Kever sends a new merge request? How do we want to proceed with this. I have a feeling the additional patch is going to be easier for everyone as 1) it's for next branch, 2) isn't breaking anything except some info message, which was already wrong (but for all non-RK808 PMICs :) ).
I think the latter is the way to go, because the pull request has been already merged.
Hi Dragan,
On 6/17/24 4:58 PM, Dragan Simic wrote:
Hello Quentin,
On 2024-06-17 16:10, Quentin Schulz wrote:
On 6/6/24 10:45 AM, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@cherry.de
The ID of the PMIC is stored in the 2 16b registers but the only part that matters right now is the 3 MSB, which make the 3 digits (in hex) of the part number.
Right now, only RK808 was properly displayed, with this all currently supported PMICs should display the proper part number.
Additionally, when the PMIC variant is not found, print that value instead of the masked unshifted value as all PMICs we support for now have their LSB ignored to represent the actual part number.
Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30 Ringneck).
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de
drivers/power/pmic/rk8xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 12ff26a0855..617bb511e4e 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -6,6 +6,7 @@ #include <dm.h> #include <dm/lists.h> +#include <bitfield.h> #include <errno.h> #include <log.h> #include <linux/bitfield.h> @@ -277,10 +278,9 @@ static int rk8xx_probe(struct udevice *dev) return ret; priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK; - show_variant = priv->variant; + show_variant = bitfield_extract_by_mask(priv->variant, RK8XX_ID_MSK); switch (priv->variant) { case RK808_ID: - show_variant = 0x808; /* RK808 hardware ID is 0 */
This line removal is actually incorrect, I should have left this in as we cannot use the same logic as other PMICs for RK808 as it returns 0, so 0 masked/shifted is still zero.
Thanks for catching this! Moreover, I think we should skip reading the msb and lsb values entirely for the RK808, because its datasheet lists the default ID_MSB (0x17) and ID_LSB (0x18) registers as reserved, and provides no information about gathering the chip variant.
We've been reading those registers on RK808 in our production lines for probably what's half a decade now, I think it's probably safe to use :)
Cheers, Quentin
On 2024-06-17 18:54, Quentin Schulz wrote:
On 6/17/24 4:58 PM, Dragan Simic wrote:
On 2024-06-17 16:10, Quentin Schulz wrote:
On 6/6/24 10:45 AM, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@cherry.de
The ID of the PMIC is stored in the 2 16b registers but the only part that matters right now is the 3 MSB, which make the 3 digits (in hex) of the part number.
Right now, only RK808 was properly displayed, with this all currently supported PMICs should display the proper part number.
Additionally, when the PMIC variant is not found, print that value instead of the masked unshifted value as all PMICs we support for now have their LSB ignored to represent the actual part number.
Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30 Ringneck).
Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de
drivers/power/pmic/rk8xx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c index 12ff26a0855..617bb511e4e 100644 --- a/drivers/power/pmic/rk8xx.c +++ b/drivers/power/pmic/rk8xx.c @@ -6,6 +6,7 @@ #include <dm.h> #include <dm/lists.h> +#include <bitfield.h> #include <errno.h> #include <log.h> #include <linux/bitfield.h> @@ -277,10 +278,9 @@ static int rk8xx_probe(struct udevice *dev) return ret; priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK; - show_variant = priv->variant; + show_variant = bitfield_extract_by_mask(priv->variant, RK8XX_ID_MSK); switch (priv->variant) { case RK808_ID: - show_variant = 0x808; /* RK808 hardware ID is 0 */
This line removal is actually incorrect, I should have left this in as we cannot use the same logic as other PMICs for RK808 as it returns 0, so 0 masked/shifted is still zero.
Thanks for catching this! Moreover, I think we should skip reading the msb and lsb values entirely for the RK808, because its datasheet lists the default ID_MSB (0x17) and ID_LSB (0x18) registers as reserved, and provides no information about gathering the chip variant.
We've been reading those registers on RK808 in our production lines for probably what's half a decade now, I think it's probably safe to use :)
True, it's obviously safe, but not reading those reserved RK808 registers would be more about accuracy and following the datasheets as precisely as possible. :)
From: Quentin Schulz quentin.schulz@cherry.de
Ringneck was mistakenly set to allow up to 128KiB for the TPL code size while PX30 SoC only has 16KiB of SRAM.
Therefore, let's use the default value of TPL_MAX_SIZE from the SoC (which is 10KiB) so that the max code size is actually checked and useful.
Fixes: c925be73a0a8 ("rockchip: add support for PX30 Ringneck SoM on Haikou Devkit") Reviewed-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Quentin Schulz quentin.schulz@cherry.de --- configs/ringneck-px30_defconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/configs/ringneck-px30_defconfig b/configs/ringneck-px30_defconfig index a22d25e0089..9965e55d611 100644 --- a/configs/ringneck-px30_defconfig +++ b/configs/ringneck-px30_defconfig @@ -14,7 +14,6 @@ CONFIG_SPL_DRIVERS_MISC=y CONFIG_DEBUG_UART_BASE=0xFF030000 CONFIG_DEBUG_UART_CLOCK=24000000 CONFIG_SYS_LOAD_ADDR=0x800800 -CONFIG_TPL_MAX_SIZE=0x20000 CONFIG_DEBUG_UART=y # CONFIG_ANDROID_BOOT_IMAGE is not set CONFIG_FIT=y
participants (3)
-
Dragan Simic -
Quentin Schulz -
Quentin Schulz