drivers: serial: rpi migration to pl011 broke driver

[Explorer001]

Describe the bug
Prior to #71074 it was possible to use the uart_pl011 driver with the rpi_pico.
After this pull request the build fails.

To Reproduce

1. Add the pl011 driver to a rpi_pico based board:

&uart0 {
    compatible = "arm,pl011";
    /delete-property/ resets;
    pinctrl-0 = <&uart0_default>;
    pinctrl-names = "default";
    current-speed = <115200>;
    status = "okay";
    clocks = <&clk_peri>;
};

2. Try to build with west build

3. Switching to a commit prior to drivers: serial: rpi_pico: Migrate to pl011 driver #71074 works (e.g 5f6a68a)

Expected behavior
A clean build.

Impact
Driver is not usable.

Logs and console output

Build log:

/data/projects/u2c/firmware_dwt9946/build/app/zephyr/include/generated/devicetree_generated.h:6857:32: error: 'DT_N_S_soc_S_uart_40038000_P_clocks_IDX_0_VAL_ARM_PL011_CLOCK_CTLR_SUBSYS_CELL' undeclared here (not in a function)
 6857 | #define DT_N_INST_1_arm_pl011  DT_N_S_soc_S_uart_40038000
      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:72:26: note: in definition of macro '__DEBRACKET'
   72 | #define __DEBRACKET(...) __VA_ARGS__
      |                          ^~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:64:9: note: in expansion of macro '__GET_ARG2_DEBRACKET'
   64 |         __GET_ARG2_DEBRACKET(one_or_two_args _if_code, _else_code)
      |         ^~~~~~~~~~~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:59:9: note: in expansion of macro '__COND_CODE'
   59 |         __COND_CODE(_XXXX##_flag, _if_1_code, _else_code)
      |         ^~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_macro.h:180:9: note: in expansion of macro 'Z_COND_CODE_1'
  180 |         Z_COND_CODE_1(_flag, _if_1_code, _else_code)
      |         ^~~~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/devicetree.h:4263:9: note: in expansion of macro 'COND_CODE_1'
 4263 |         COND_CODE_1(DT_HAS_COMPAT_STATUS_OKAY(DT_DRV_COMPAT),   \
      |         ^~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:69:53: note: in expansion of macro '__DEBRACKET'
   69 | #define __GET_ARG2_DEBRACKET(ignore_this, val, ...) __DEBRACKET val
      |                                                     ^~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:64:9: note: in expansion of macro '__GET_ARG2_DEBRACKET'
   64 |         __GET_ARG2_DEBRACKET(one_or_two_args _if_code, _else_code)
      |         ^~~~~~~~~~~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:59:9: note: in expansion of macro '__COND_CODE'
   59 |         __COND_CODE(_XXXX##_flag, _if_1_code, _else_code)
      |         ^~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_macro.h:180:9: note: in expansion of macro 'Z_COND_CODE_1'
  180 |         Z_COND_CODE_1(_flag, _if_1_code, _else_code)
      |         ^~~~~~~~~~~~~
/data/projects/u2c/zephyr/drivers/serial/uart_pl011.c:571:9: note: in expansion of macro 'COND_CODE_1'
  571 |         COND_CODE_1(DT_NODE_HAS_COMPAT(DT_INST_CLOCKS_CTLR(n), fixed_clock), (),                   \
      |         ^~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/devicetree.h:1362:9: note: in expansion of macro 'DT_CAT7'
 1362 |         DT_CAT7(node_id, _P_, pha, _IDX_, idx, _VAL_, cell)
      |         ^~~~~~~
/data/projects/u2c/zephyr/include/zephyr/devicetree/clocks.h:208:9: note: in expansion of macro 'DT_PHA_BY_IDX'
  208 |         DT_PHA_BY_IDX(node_id, clocks, idx, cell)
      |         ^~~~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/devicetree/clocks.h:327:9: note: in expansion of macro 'DT_CLOCKS_CELL_BY_IDX'
  327 |         DT_CLOCKS_CELL_BY_IDX(DT_DRV_INST(inst), idx, cell)
      |         ^~~~~~~~~~~~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:105:36: note: in expansion of macro 'DT_N_INST_1_arm_pl011'
  105 | #define UTIL_PRIMITIVE_CAT(a, ...) a##__VA_ARGS__
      |                                    ^
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:104:26: note: in expansion of macro 'UTIL_PRIMITIVE_CAT'
  104 | #define UTIL_CAT(a, ...) UTIL_PRIMITIVE_CAT(a, __VA_ARGS__)
      |                          ^~~~~~~~~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/devicetree.h:336:31: note: in expansion of macro 'UTIL_CAT'
  336 | #define DT_INST(inst, compat) UTIL_CAT(DT_N_INST, DT_DASH(inst, compat))
      |                               ^~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/devicetree.h:3413:27: note: in expansion of macro 'DT_INST'
 3413 | #define DT_DRV_INST(inst) DT_INST(inst, DT_DRV_COMPAT)
      |                           ^~~~~~~
/data/projects/u2c/zephyr/include/zephyr/devicetree/clocks.h:327:31: note: in expansion of macro 'DT_DRV_INST'
  327 |         DT_CLOCKS_CELL_BY_IDX(DT_DRV_INST(inst), idx, cell)
      |                               ^~~~~~~~~~~
/data/projects/u2c/zephyr/include/zephyr/devicetree/clocks.h:348:9: note: in expansion of macro 'DT_INST_CLOCKS_CELL_BY_IDX'
  348 |         DT_INST_CLOCKS_CELL_BY_IDX(inst, 0, cell)
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
/data/projects/u2c/zephyr/drivers/serial/uart_pl011.c:573:58: note: in expansion of macro 'DT_INST_CLOCKS_CELL'
  573 |                      .clock_id = (clock_control_subsys_t)DT_INST_CLOCKS_CELL(n,                    \
      |                                                          ^~~~~~~~~~~~~~~~~~~
/data/projects/u2c/zephyr/drivers/serial/uart_pl011.c:618:17: note: in expansion of macro 'CLOCK_INIT'
  618 |                 CLOCK_INIT(n)                                                           \
      |                 ^~~~~~~~~~
/data/projects/u2c/zephyr/drivers/serial/uart_pl011.c:636:9: note: in expansion of macro 'PL011_CONFIG_PORT'
  636 |         PL011_CONFIG_PORT(n)                                    \
      |         ^~~~~~~~~~~~~~~~~
/data/projects/u2c/firmware_dwt9946/build/app/zephyr/include/generated/devicetree_generated.h:8316:50: note: in expansion of macro 'PL011_INIT'
 8316 | #define DT_FOREACH_OKAY_INST_arm_pl011(fn) fn(0) fn(1)
      |                                                  ^~
/data/projects/u2c/zephyr/include/zephyr/sys/util_internal.h:105:36: note: in expansion of macro 'DT_FOREACH_OKAY_INST_arm_pl011'
  105 | #define UTIL_PRIMITIVE_CAT(a, ...) a##__VA_ARGS__
      |                                    ^
/data/projects/u2c/zephyr/drivers/serial/uart_pl011.c:659:1: note: in expansion of macro 'DT_INST_FOREACH_STATUS_OKAY'
  659 | DT_INST_FOREACH_STATUS_OKAY(PL011_INIT)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~

Environment (please complete the following information):

• Toolchain: zephyr sdk 0.16.3
• Zephyr Commit: 950382c

[soburi]

@Explorer001

One question.

Please let me know if there is a reason why you changed this setting from DTS.

    compatible = "arm,pl011";
    /delete-property/ resets;

If you are using rpi_pico's PL011, the default should be equivalent to the behavior of the previous pico driver.

compatible = "arm,pl011"; to explicitly use the pl011 driver. The /delete-property/ resets; is necessary because the pl011 device tree binding has no reset property. With this dts description it was possible to use the pl011 driver prior to the pull request

[Explorer001]

compatible = "arm,pl011"; to explicitly use the pl011 driver. The /delete-property/ resets; is necessary because the pl011 device tree binding has no reset property. With this dts description it was possible to use the pl011 driver prior to the pull request

[soburi]

If you use RPI Pico's pl011, I think it is better to use the original configuration.

compatible = "raspberrypi,pico-uart", "arm,pl011";
resets = <&reset RPI_PICO_RESETS_RESET_UART0>;

With this configuration, the pl011 driver will be used, and in the case of rpi_pico, reset at initialization will be handled correctly.

("raspberrypi,pico-uart" is used only as an identifier.)

I don't think so, but If there are additional PL011s other than those implemented in rp2040,

compatible = "arm,pl011";

may need to be done, but the answers above believe otherwise.

[soburi]

@Explorer001

    clocks = <&clk_peri>;

As I can see from this line, I think the example you provided is not working after #62186.
(might not after #71074)

As the rpi_pico platform, the pl011 driver cannot be used as is (because reset and clock are not set correctly), so we created rpi_pico specific driver. So, at that point, I was not expecting to use it in combination with the pl011 driver.

After #71074, we added a mechanism to handle reset and clock to the pl011 driver, so rpi_pico also uses the pl011 driver with the default settings.

In other words, I think you can do what you want with the default settings.

[Explorer001]

Building with default settings seems to be possible. Ty