[PATCH V2 2/4] board: rockchip: Add vdd_cpu reg fixup for RGXX3 Series
Chris Morgan
macromorgan at hotmail.com
Mon Sep 23 19:36:33 CEST 2024
On Mon, Sep 23, 2024 at 01:21:01PM +0200, Quentin Schulz wrote:
> Hi Chris,
>
> On 9/19/24 4:00 PM, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan at hotmail.com>
> >
> > Some of the Powkiddy devices switched to using a different vendor for
> > the vdd_cpu regulator. Unfortunately the device does not have a new
> > revision to denote this, so users have no way of knowing in advance.
> >
> > Add code to detect if a device is present at addresses 0x1c or 0x40 on
> > the i2c0 bus and update the devicetree if needed.
> >
> > Signed-off-by: Chris Morgan <macromorgan at hotmail.com>
> > ---
> > board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c | 147 +++++++++++++++++++++
> > 1 file changed, 147 insertions(+)
> >
> > diff --git a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
> > index 224019f9ba3..c1d1826fd14 100644
> > --- a/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
> > +++ b/board/anbernic/rgxx3_rk3566/rgxx3-rk3566.c
> > @@ -43,6 +43,7 @@ struct rg3xx_model {
> > const char *board_name;
> > const char *fdtfile;
> > const bool detect_panel;
> > + const bool detect_regulator;
> > const bool uart_con;
> > };
> > @@ -69,6 +70,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > /* Device is identical to RG353P. */
> > .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb",
> > .detect_panel = 1,
> > + .detect_regulator = 0,
> > .uart_con = 1,
> > },
> > [RG353P] = {
> > @@ -77,6 +79,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Anbernic RG353P",
> > .fdtfile = DTB_DIR "rk3566-anbernic-rg353p.dtb",
> > .detect_panel = 1,
> > + .detect_regulator = 0,
> > .uart_con = 1,
> > },
> > [RG353V] = {
> > @@ -85,6 +88,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Anbernic RG353V",
> > .fdtfile = DTB_DIR "rk3566-anbernic-rg353v.dtb",
> > .detect_panel = 1,
> > + .detect_regulator = 0,
> > .uart_con = 1,
> > },
> > [RG503] = {
> > @@ -93,6 +97,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Anbernic RG503",
> > .fdtfile = DTB_DIR "rk3566-anbernic-rg503.dtb",
> > .detect_panel = 0,
> > + .detect_regulator = 0,
> > .uart_con = 1,
> > },
> > [RGB30] = {
> > @@ -101,6 +106,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Powkiddy RGB30",
> > .fdtfile = DTB_DIR "rk3566-powkiddy-rgb30.dtb",
> > .detect_panel = 0,
> > + .detect_regulator = 1,
> > .uart_con = 0,
> > },
> > [RK2023] = {
> > @@ -109,6 +115,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Powkiddy RK2023",
> > .fdtfile = DTB_DIR "rk3566-powkiddy-rk2023.dtb",
> > .detect_panel = 0,
> > + .detect_regulator = 1,
> > .uart_con = 0,
> > },
> > [RGARCD] = {
> > @@ -117,6 +124,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Anbernic RG ARC-D",
> > .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-d.dtb",
> > .detect_panel = 0,
> > + .detect_regulator = 0,
> > .uart_con = 1,
> > },
> > [RGB10MAX3] = {
> > @@ -125,6 +133,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Powkiddy RGB10MAX3",
> > .fdtfile = DTB_DIR "rk3566-powkiddy-rgb10max3.dtb",
> > .detect_panel = 0,
> > + .detect_regulator = 1,
> > .uart_con = 0,
> > },
> > /* Devices with duplicate ADC value */
> > @@ -134,6 +143,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Anbernic RG353PS",
> > .fdtfile = DTB_DIR "rk3566-anbernic-rg353ps.dtb",
> > .detect_panel = 1,
> > + .detect_regulator = 0,
> > .uart_con = 1,
> > },
> > [RG353VS] = {
> > @@ -142,6 +152,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Anbernic RG353VS",
> > .fdtfile = DTB_DIR "rk3566-anbernic-rg353vs.dtb",
> > .detect_panel = 1,
> > + .detect_regulator = 0,
> > .uart_con = 1,
> > },
> > [RGARCS] = {
> > @@ -150,6 +161,7 @@ static const struct rg3xx_model rg3xx_model_details[] = {
> > .board_name = "Anbernic RG ARC-S",
> > .fdtfile = DTB_DIR "rk3566-anbernic-rg-arc-s.dtb",
> > .detect_panel = 0,
> > + .detect_regulator = 0,
> > .uart_con = 1,
> > },
> > };
> > @@ -172,6 +184,22 @@ static const struct rg353_panel rg353_panel_details[] = {
> > },
> > };
> > +struct powkiddy_regulators {
> > + const u8 addr;
> > + const char *regulator_compat;
> > +};
> > +
> > +static const struct powkiddy_regulators regulator_details[] = {
> > + {
> > + .addr = 0x1c,
> > + .regulator_compat = "tcs,tcs4525",
> > + },
> > + {
> > + .addr = 0x40,
> > + .regulator_compat = "fcs,fan53555",
> > + },
> > +};
> > +
> > /*
> > * Start LED very early so user knows device is on. Set color
> > * to red.
> > @@ -361,6 +389,44 @@ int rgxx3_detect_display(void)
> > return 0;
> > }
> > +/*
> > + * Some of the Powkiddy devices switched the CPU regulator, but users
> > + * are not able to determine this by looking at their hardware.
> > + * Attempt to auto-detect this situation and fixup the device-tree.
> > + */
> > +int rgxx3_detect_regulator(void)
> > +{
> > + struct udevice *bus;
> > + struct udevice *chip;
> > + u8 val;
> > + int ret;
> > +
> > + /* Get the correct i2c bus (i2c0). */
> > + ret = uclass_get_device_by_name(UCLASS_I2C,
> > + "i2c at fdd40000", &bus);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Check for all vdd_cpu regulators and read an arbitrary
> > + * register to confirm it's present.
> > + */
> > + for (int i = 0; i < ARRAY_SIZE(regulator_details); i++) {
> > + ret = i2c_get_chip(bus, regulator_details[i].addr,
> > + 1, &chip);
> > + if (ret)
> > + return ret;
> > +
> > + ret = dm_i2c_read(chip, 0, &val, 1);
> > + if (!ret) {
> > + env_set("vdd_cpu", regulator_details[i].regulator_compat);
> > + break;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int rgxx3_read_board_id(void)
> > {
> > u32 adc_info;
> > @@ -485,6 +551,16 @@ int rk_board_late_init(void)
> > printf("Failed to detect panel type\n");
> > }
> > + /*
> > + * Skip vdd_cpu regulator detection if not needed. Warn but
> > + * don't fail for errors in auto-detection of regulator.
> > + */
> > + if (rg3xx_model_details[gd->board_type].detect_regulator) {
> > + ret = rgxx3_detect_regulator();
> > + if (ret)
> > + printf("Unable to detect vdd_cpu regulator\n");
> > + }
> > +
> > end:
> > /* Turn off red LED and turn on orange LED. */
> > writel(GPIO_WRITEMASK(GPIO_C7 | GPIO_C6 | GPIO_C5) | GPIO_C6,
> > @@ -547,6 +623,71 @@ int rgxx3_panel_fixup(void *blob)
> > return 0;
> > }
> > +int rgxx3_regulator_fixup(void *blob)
> > +{
> > + const struct powkiddy_regulators *vdd_cpu = NULL;
> > + int node, ret, i;
> > + char path[] = "/i2c at fdd40000/regulator at 00";
> > + char name[] = "regulator at 00";
> > + char *env;
> > +
> > + env = env_get("vdd_cpu");
> > + if (!env) {
> > + printf("Can't get vdd_cpu env\n");
> > + return -EINVAL;
> > + }
> > +
> > + /*
> > + * Find the device we have in our tree, which may or may not
> > + * be present.
> > + */
> > + for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
> > + sprintf(path, "/i2c at fdd40000/regulator@%02x",
> > + regulator_details[i].addr);
> > + node = fdt_path_offset(blob, path);
> > + if (node > 0)
> > + break;
> > +
> > + printf("Unable to find vdd_cpu\n");
> > + return -ENODEV;
> > + }
> > +
> > + node = fdt_path_offset(blob, path);
> > + if (!(node > 0)) {
> > + printf("Can't find the vdd_cpu node\n");
> > + return -ENODEV;
> > + }
> > +
> > + ret = fdt_node_check_compatible(blob, node, env);
> > + if (ret < 0)
> > + return -ENODEV;
> > +
> > + /* vdd_cpu regulators match, return 0. */
> > + if (!ret)
> > + return 0;
> > +
> > + /* Regulators don't match, search by first compatible value. */
> > + for (i = 0; i < ARRAY_SIZE(regulator_details); i++) {
> > + if (!strcmp(env, regulator_details[i].regulator_compat)) {
> > + vdd_cpu = ®ulator_details[i];
> > + break;
> > + }
> > + }
> > +
> > + if (!vdd_cpu) {
> > + printf("Unable to identify vdd_cpu by compat string\n");
> > + return -ENODEV;
> > + }
> > +
> > + /* Set the compatible and reg with the auto-detected values */
> > + fdt_setprop_string(blob, node, "compatible", vdd_cpu->regulator_compat);
> > + fdt_setprop_u32(blob, node, "reg", vdd_cpu->addr);
> > + sprintf(name, "regulator@%02x", vdd_cpu->addr);
> > + fdt_set_name(blob, node, name);
> > +
>
> I'm not sure this is viable in the long run... This relies on the properties
> for the two devices to be identical. Are we sure it is absolutely the case?
Yes, for the moment at least the devices are entirely identical except
for the CPU regulator. Unfortunately the identical ADC IDs is why I
cannot simply have an additional entry for a new hardware revision.
>
> An option could be to have a DTSO for the "new" regulator that removes the
> old regulator entirely and we would simply be applying that DTSO at runtime
> before the kernel starts. I assume you probably want this for SPL or U-Boot
> proper as well so applying the DTSO there would be required as well, not
> sure how to do this though.
In truth I really don't need the regulator this way for U-Boot at all.
Near as I can tell it defaults to being on and the right voltage. I can
do a DTSO, I just figured a simple fixup would be easier (plus we're
already doing a fixup for a similar situation with panel revisions).
>
> Can you please tell the HW vendor to have some way of detecting some
> variants of the HW instead of letting us just guess stuff? How do they even
> support this in their downstream kernel/bootloader????
> Have they already ran out of channels on SARADC?
>
I have told both Powkiddy and Anbernic this. Whether they will listen
is a different matter. They haven't run out of channels on the SARADC
(except for the Powkiddy X55, which is its own board and has no
detection logic). In theory if we allow for a ~10% variance in the
resistor values for a resistor on ADC channel 1, and the range is
between 0 and 1024 (20 and 1004), that gives us about 49 total devices
we could "detect" at one time, assuming no one collides.
I have no idea how Powkiddy fixes up the CPU regulator, but I know
Anbernic does panel fixup with some hacky code in U-Boot and Linux
proper that detects for the panel like I do. In their case it's built
into the probe function of Rockchip's own "display-simple-dsi" driver.
In our case we detect the panel ID in U-Boot and then fixup the device
tree for Linux with the correct compatible.
> Cheers,
> Quentin
Thank you,
Chris
More information about the U-Boot
mailing list