12:48 <mwalle> I'm just porting u-boot to a new board and I've stumbled across the following:

12:48 <mwalle> https://elixir.bootlin.com/u-boot/latest/source/drivers/pinctrl/pinctrl-uclass.c#L320

12:48 <mwalle> I'm not sure why there is the fallback to the pinctrl_select_state_simple()

12:49 <mwalle> what happens on my board is the following: the pinctrl is a child of a syscon,simple-mfd and it uses its regmap

12:50 <mwalle> the fallback to the simple one, will lead to a bind of the pinctrl, which doesn't work because its parent isn't probed yet

15:02 <marex> mwalle: if CONFIG_IS_ENABLED(PINCTRL_FULL) -> pinctrl_select_state_full() returns -ENODEV , it's a measure to save space in SPL

15:03 <marex> mwalle: does pinctrl_select_state_full() return -ve somewhere from within itself ?

15:03 <marex> is PINCTRL_FULL enabled ?

15:34 <mwalle> marex: when PINCTRL_FULL is enabled then pinctrl_select_state_full returns enodev?

15:34 <mwalle> you mean if it's not enabled?

15:37 <mwalle> locally, i've guarded the simple call with !CONFIG_IS_ENABLED(PINCTRL_FULL) as I don't see the usecase that both are used. which was my initial question, why is there an if (!full()) simple()

15:38 <mwalle> marex: and yes I have PINCTRL_FULL and no SPL

15:51 <marex> mwalle: see the macro protection at the beginning ... #if CONFIG_IS_ENABLED(PINCTRL_FULL)

15:51 <marex> 164 #else

15:51 <marex> 165 static int pinctrl_select_state_full(struct udevice *dev, const char *statename)

15:51 <marex> 166 {

15:51 <marex> 167 return -ENODEV;

15:51 <marex> 168 }

15:52 <marex> so, in case ^ always returns -ENODEV for !PINCTRL_FULL

15:52 <marex> 320 if (pinctrl_select_state_full(dev, statename))

15:52 <marex> 321 return pinctrl_select_state_simple(dev);

15:52 <marex> this becomes really plain return pinctrl_select_state_simple(dev);

15:52 <mwalle> marex: I know, it makes partially sense if PINCTRL_FULL is not enabled. But what if its enabled

15:52 <mwalle> why is there a fallback to simple at all

15:55 <marex> mwalle: there probably shouldn't be one

15:55 <marex> at least by looking at the code, the fallback makes little sense

15:57 <mwalle> marex: that's also my impression

15:57 <marex> mwalle: send a bugfix then

15:58 <mwalle> marex: sure, let me dig a bit more into it. I'll need to have it fixed anyway, when i upstream the board ;)

15:58 <marex> thanks

15:58 <Tartarus> qschulz: Around?

15:59 <Tartarus> Or anyone else with a rockchip px30

16:03 <qschulz> Tartarus: yes

16:03 <Tartarus> qschulz: Can you test yet another ns16550 patch today?

16:03 <qschulz> Tartarus: sure

16:03 <Tartarus> What fixed your case didn't fix sunxi too, which also broke :(

16:03 <qschulz> Tartarus: :/

16:08 <Tartarus> qschulz: OK, hopefully something in 10-15min, sunxi does work again here, but now to test everything else still

16:24 <qschulz> Tartarus: got a link for a patch somewhere? Kinda confused by your messages on the ML :/

16:25 <Tartarus> qschulz: Almost have a link as, yes, now my lab passed

16:25 <qschulz> Tartarus: :)

16:25 <Tartarus> https://github.com/trini/u-boot/pull/new/WIP/2022-01-17-further-cleanup-CONFIG-migration

16:25 <Tartarus> if that branch passes, good

16:25 <Tartarus> If not, arrrg

16:25 <qschulz> Tartarus: requires github login?

16:26 <Tartarus> Sigh, moment

16:26 <Tartarus> https://source.denx.de/u-boot/u-boot/-/tree/WIP/2022-01-17-further-cleanup-CONFIG-migration

16:33 <qschulz> Tartarus: something else's fucked, I cannot get the board to boot with master

16:34 <Tartarus> qschulz: Ugh

17:11 <qschulz> Tartarus: ok found the issue, a KConfig symbol rename

17:12 <qschulz> Tartarus: tested 69d5f497d3b6 ("CI: Make check for new defined CONFIG symbols even more robust") on Ringneck PX30, I get serial properly

17:13 <Tartarus> qschulz: OK, thanks, I'll cc you on this serial patch too when I post it

20:30 <mwalle> marex: so, pinctrl_select_state() is called for every device (except the root and the pinctrl itself) regardless if there is a pinctrl property or not. and if there is none, the pinctrl_select_state_full() will fail with EINVAL

20:31 <mwalle> marex: which means, that right now, the _simple() will be called for every device which doesn't have a pinctrl property

20:32 <mwalle> marex: I think the real bug here is that the _full() never returns -ENOSYS

21:16 <marex> mwalle: ENOSYS in case property not present ?

21:20 <mwalle> marex: yes, right now, this is returened via the _simple()

21:23 <mwalle> marex: what actually happens on my board is the following (before relocation). pinctrl_select_state() is called for each device (or just some? doesn't matter), the full will fail, because most devices don't have a pinctrl-names property. then _simple() is called, that does a device_probe of the pinctrl, which fails because the parent isn't probed yet (and i need the regmap of the parent).

21:24 <mwalle> the device_probe() will allocate memory.. which is never freed because we are before relocation, and somewhere along the path, I'm running out of pre-reloc heap memory

21:46 <marex> mwalle: what would be the expected behavior ?

21:56 <mwalle> marex: regarding the pinctrl or regarding my board ;)

22:18 <marex> mwalle: regarding the early pinctrl config

22:18 <marex> the pinctrl should likely be configured anyway, right ?

22:21 <mwalle> marex: in what case? i need it for the uart, but i have a pinctrl node there

22:22 <mwalle> marex: and it's working as long as there isn't the _simple() fallback which eats up my memory as i mentioned

22:24 <marex> mwalle: so who does configure the pinctrl for the UART then ? the full ?

22:28 <mwalle> marex: yes the ful

22:28 <mwalle> *full

22:41 <marex> mwalle: OK, I see, in that case yes, skip the simple and be done with it