Re: [PATCH V2 3/6] board: rockchip: Add support for RG353PS to RGxx3

Kever Yang
2023-07-26T17:08:11Z

On 2023/5/16 00:00, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add support for the RG353PS to the Anbernic RGxx3. This device is a
> slightly pared down version of the RG353P with no eMMC, no touchscreen,
> and only 1GB of RAM.
>
> Refactor board logic so that all supported devices are defined with
> ADC values and that future boards can be added by just defining the
> board values in the device array.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
> board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 126 ++++++++++++++-------
> 1 file changed, 86 insertions(+), 40 deletions(-)
>
> diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
> index decc46db78..4d3c724b9c 100644
> --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
> +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
> @@ -7,24 +7,30 @@
> #include <adc.h>
> #include <asm/io.h>
> #include <dm.h>
> +#include <dm/lists.h>
> +#include <env.h>
> +#include <fdt_support.h>
> #include <linux/delay.h>
> +#include <mmc.h>
> #include <pwm.h>
> #include <rng.h>
> #include <stdlib.h>
> -#include <mmc.h>
> -#include <env.h>
>
> #define GPIO0_BASE 0xfdd60000
> +#define GPIO_SWPORT_DR_L 0x0000
> #define GPIO_SWPORT_DR_H 0x0004
> +#define GPIO_SWPORT_DDR_L 0x0008
> #define GPIO_SWPORT_DDR_H 0x000c
> -#define GPIO_A5 BIT(5)
> -#define GPIO_A6 BIT(6)
> +#define GPIO_C5 BIT(5)
> +#define GPIO_C6 BIT(6)
> +#define GPIO_C7 BIT(7)
>
> #define GPIO_WRITEMASK(bits) ((bits) << 16)
>
> #define DTB_DIR "rockchip/"
>
> struct rg3xx_model {
> + const u16 adc_value;
> const char *board;
> const char *board_name;
> const char *fdtfile;
> @@ -34,49 +40,64 @@ enum rgxx3_device_id {
> RG353M,
> RG353P,
> RG353V,
> - RG353VS,
> RG503,
> + /* Devices with duplicate ADC value */
> + RG353PS,
> + RG353VS,
> };
>
> static const struct rg3xx_model rg3xx_model_details[] = {
> [RG353M] = {
> + 517, /* Observed average from device */
> "rk3566-anbernic-rg353m",
> "RG353M",
> - DTB_DIR "rk3566-anbernic-rg353m.dtb",
> + DTB_DIR "rk3566-anbernic-rg353p.dtb", /* Identical devices */
> },
> [RG353P] = {
> + 860, /* Documented value of 860 */
> "rk3566-anbernic-rg353p",
> "RG353P",
> DTB_DIR "rk3566-anbernic-rg353p.dtb",
> },
> [RG353V] = {
> + 695, /* Observed average from device */
> "rk3566-anbernic-rg353v",
> "RG353V",
> DTB_DIR "rk3566-anbernic-rg353v.dtb",
> },
> - [RG353VS] = {
> - "rk3566-anbernic-rg353vs",
> - "RG353VS",
> - DTB_DIR "rk3566-anbernic-rg353vs.dtb",
> - },
> [RG503] = {
> + 1023, /* Observed average from device */
> "rk3566-anbernic-rg503",
> "RG503",
> DTB_DIR "rk3566-anbernic-rg503.dtb",
> },
> + /* Devices with duplicate ADC value */
> + [RG353PS] = {
> + 860, /* Observed average from device */
> + "rk3566-anbernic-rg353ps",
> + "RG353PS",
> + DTB_DIR "rk3566-anbernic-rg353ps.dtb",
> + },
> + [RG353VS] = {
> + 695, /* Gathered from second hand information */
> + "rk3566-anbernic-rg353vs",
> + "RG353VS",
> + DTB_DIR "rk3566-anbernic-rg353vs.dtb",
> + },
> };
>
> /*
> * Start LED very early so user knows device is on. Set color
> - * to amber.
> + * to red.
> */
> void spl_board_init(void)
> {
> - /* Set GPIO0_A5 and GPIO0_A6 to output. */
> - writel(GPIO_WRITEMASK(GPIO_A6 | GPIO_A5) | (GPIO_A6 | GPIO_A5),
> + /* Set GPIO0_C5, GPIO0_C6, and GPIO0_C7 to output. */
> + writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | \
> + (GPIO_C7 | GPIO_C6 | GPIO_C5),
> (GPIO0_BASE + GPIO_SWPORT_DDR_H));
> - /* Set GPIO0_A5 to 0 and GPIO0_A6 to 1. */
> - writel(GPIO_WRITEMASK(GPIO_A6 | GPIO_A5) | GPIO_A6,
> + /* Set GPIO0_C5 and GPIO_C6 to 0 and GPIO0_C7 to 1. */
> + writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C7,
> (GPIO0_BASE + GPIO_SWPORT_DR_H));
> }
>
> @@ -129,12 +150,12 @@ void __maybe_unused startup_buzz(void)
> /* Detect which Anbernic RGXX3 device we are using so as to load the
> * correct devicetree for Linux. Set an environment variable once
> * found. The detection depends on the value of ADC channel 1, the
> - * presence of an eMMC on mmc0, and querying the DSI panel (TODO).
> + * presence of an eMMC on mmc0, and querying the DSI panel.
> */
> int rgxx3_detect_device(void)
> {
> u32 adc_info;
> - int ret;
> + int ret, i;
> int board_id = -ENXIO;
> struct mmc *mmc;
>
> @@ -144,30 +165,37 @@ int rgxx3_detect_device(void)
> return ret;
> }
>
> - /* Observed value 517. */
> - if (adc_info > 505 && adc_info < 530)
> - board_id = RG353M;
> - /* Observed value 695. */
> - if (adc_info > 680 && adc_info < 710)
> - board_id = RG353V;
> - /* Documented value 860. */
> - if (adc_info > 850 && adc_info < 870)
> - board_id = RG353P;
> - /* Observed value 1023. */
> - if (adc_info > 1010)
> - board_id = RG503;
> + /*
> + * Get the correct device from the table. The ADC value is
> + * determined by a resistor on ADC channel 0. The hardware
> + * design calls for no more than a 1% variance on the
> + * resistor, so assume a +- value of 15 should be enough.
> + */
> + for (i = 0; i < ARRAY_SIZE(rg3xx_model_details); i++) {
> + u32 adc_min = rg3xx_model_details[i].adc_value - 15;
> + u32 adc_max = rg3xx_model_details[i].adc_value + 15;
> +
> + if (adc_min < adc_info && adc_max > adc_info) {
> + board_id = i;
> + break;
> + }
> + }
>
> /*
> - * Try to access the eMMC on an RG353V. If it's missing, it's
> - * an RG353VS. Note we could also check for a touchscreen at
> - * 0x1a on i2c2.
> + * Try to access the eMMC on an RG353V or RG353P. If it's
> + * missing, it's an RG353VS or RG353PS. Note we could also
> + * check for a touchscreen at 0x1a on i2c2.
> */
> - if (board_id == RG353V) {
> + if (board_id == RG353V || board_id == RG353P) {
> mmc = find_mmc_device(0);
> if (mmc) {
> ret = mmc_init(mmc);
> - if (ret)
> - board_id = RG353VS;
> + if (ret) {
> + if (board_id == RG353V)
> + board_id = RG353VS;
> + else
> + board_id = RG353PS;
> + }
> }
> }
>
> @@ -186,18 +214,36 @@ int rk_board_late_init(void)
> {
> int ret;
>
> - /* Turn off orange LED and turn on green LED. */
> - writel(GPIO_WRITEMASK(GPIO_A6 | GPIO_A5) | GPIO_A5,
> - (GPIO0_BASE + GPIO_SWPORT_DR_H));
> -
> ret = rgxx3_detect_device();
> if (ret) {
> printf("Unable to detect device type: %d\n", ret);
> return ret;
> }
>
> + /* Turn off red LED and turn on orange LED. */
> + writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6,
> + (GPIO0_BASE + GPIO_SWPORT_DR_H));
> +
> if (IS_ENABLED(CONFIG_DM_PWM))
> startup_buzz();
>
> return 0;
> }
> +
> +int ft_board_setup(void *blob, struct bd_info *bd)
> +{
> + char *env;
> +
> + /* No fixups necessary for the RG503 */
> + env = env_get("board_name");
> + if (env && (!strcmp(env, rg3xx_model_details[RG503].board_name)))
> + return 0;
> +
> + /* Change the model name of the RG353M */
> + if (env && (!strcmp(env, rg3xx_model_details[RG353M].board_name)))
> + fdt_setprop(blob, 0, "model",
> + rg3xx_model_details[RG353M].board_name,
> + sizeof(rg3xx_model_details[RG353M].board_name));
> +
> + return 0;
> +}