Re: [PATCH] rockchip: rk3308: Drop unused rk_board_init()

Jonas Karlman
2024-11-06T21:15:10Z

Hi Quentin,

On 2024-11-06 14:49, Quentin Schulz wrote:
> Hi Jonas,
>
> On 11/2/24 9:45 PM, Jonas Karlman wrote:
>> Nothing is calling the function rk_board_init() and the io-domain driver
>> can handle the functions intended purpose based on information from DT.
>>
>> Cleanup by removing the unused rk_board_init() function and re-sort
>> included headers.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>
> git log -p -S rk_board_init
>
> only returns one match and it's the introducing commit in which this
> function seemingly is never called.
>
> I'm really wondering why this wasn't shown as a warning by GCC?

Building with W=1 at least show one warning (and a lot of others):

warning: no previous prototype for ‘rk_board_init’ [-Wmissing-prototypes]

>
> Since this is the IO domain for storage medium on RK3308, shouldn't we
> make sure the IO domain is enabled in SPL for fallback mechanism (i.e.
> proper isn't found on the same storage medium as the one used for
> loading SPL)?

I took a new look at schematics of supported boards and all seem to
signal correct VCCIO3 voltage using GPIO0_A4 (0: 3v3, 1: 1v8).

Also re-tested emmc/sd-nand and sd-card cross-read in SPL and there was
no issues on my Rock Pi S and ROCK S0 boards, so current reset values
to use GPIO0_A4 should work until io-domain driver later decide change
to use io_vsel3.

>
> The IODOMAIN isn't even enabled in proper for the EVB and the ROC-CC
> board. And there isn't an IODOMAIN symbol for SPL in Kconfig AFAICT?

Originally I probably only wanted to enable the io-domain driver on
boards that I could runtime test. Also the primary use for Rock Pi S and
ROCK S0 was to configure correct io voltage for Wi-Fi.

For rk3308 U-Boot IODOMAIN=y may not be fully necessary, at least now
that Linux io-domain driver also has support for rk3308.

For now, I do not think any board need to include io-domain driver in
SPL, could be changed in a future series if/when that is needed :-)

Regards,
Jonas

>
> Not that this commit would change anything, so:
>
> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de>
>
> Thanks!
> Quentin