[PATCH 1/4] power: regulator: Trigger probe of regulators which are always-on or boot-on

[Svyatoslav Ryhel]

пн, 9 вер. 2024 р. о 19:13 Marek Vasut <marex at denx.de> пише:
>
> On 8/20/24 9:08 AM, Svyatoslav Ryhel wrote:
> > пн, 19 серп. 2024 р. о 20:27 Marek Vasut <marex at denx.de> пише:
> >>
> >> On 8/1/24 2:28 AM, Marek Vasut wrote:
> >>> On 7/29/24 1:55 PM, Svyatoslav Ryhel wrote:
> >>>
> >>> [...]
> >>>
> >>>>>>> What is the problem you observe on tegra 3 ?
> >>>>>> i2c line fails since it probes in spl with your patch, but it does not
> >>>>>> relocate and then probes once more after relocation. Probe fails along
> >>>>>> with all devices on same line.
> >>>>>
> >>>>> Could it be that you either have to:
> >>>>> - Add DM_I2C to tegra 3 SPL
> >>>>> - Remove bootph-* from DT to remove the regulator node from SPL
> >>>>> - /delete-property/ regulator-always-on; and /delete-property/
> >>>>> regulator-boot-on; in -u-boot.dtsi to prevent the regulator from being
> >>>>> enabled in SPL ?
> >>>>>
> >>>> Obviously NO, you propose nonsense. Same dts is used for both stages.
> >>>
> >>> DT source yes, DT blob likely no.
> >>>
> >>>> And I have to add hack-ish stuff just because you wanna introduce code
> >>>> which causes known regressions.
> >>>
> >>> I am trying to understand what problem there is on tegra 3, but it is
> >>> still not clear to me.
> >>>
> >>> Is the problem somehow related to PMICs (?) being probed in SPL (?)
> >>> because they have regulators (?) which are marked as regulator-always-on
> >>> ? If so, then this is correct behavior, and if this is not desired in
> >>> SPL, then you can remove this property from SPL DT in -u-boot.dtsi using
> >>> /delete-property/ .
> >>>
> >>> [...]
> >>>
> >>>> "We must not probe things as we go. There might be other
> >>>> dependencies not yet bound. It may also take some time. This is not
> >>>> following driver model design, sorry.
> >>>>
> >>>> So please think of a way to do this properly."
> >>>
> >>> What is this quote about ? Where is this from ?
> >>
> >> What is the problem with Tegra 3 and this patchset ?
> >>
> >> Can you please explain that so this patchset can move forward ?
> >>
> >
> > with your changes
> >
> > U-Boot 2024.07-00696-ge217e2769db9-dirty (Aug 20 2024 - 09:55:29 +0300)
> >
> > SoC: tegra114
> > Model: NVIDIA Tegra Note 7
> > Board: NVIDIA TegraTab
> > DRAM:  1 GiB
> > tegra_i2c_probe: called
> > i2c: controller bus 0 at 7000d000, speed 0: tegra_i2c_probe: exit
> > i2c_init_controller: speed=400000
> > i2c_init_controller: CLK_DIV_STD_FAST_MODE setting = 25
> > i2c_xfer: 2 messages
> > i2c_xfer: chip=0x58, len=0x1
> > i2c_write_data: chip=0x58, len=0x1
> > write_data:  0x37
> > pkt header 1 sent (0x10010)
> > pkt header 2 sent (0x0)
> > pkt header 3 sent (0x100b0)
> > pkt data sent (0x37)
> > tegra_i2c_write_data: Error (-1) !!!
> > i2c_write_data(): rc=-1
> > i2c_write: error sending
> > read error from device: bd26f8e0 register: 0x37!
>
> This seems like the PMIC driver (palmas?) is trying to read register
> 0x37 PGOOD and the I2C transfer fails . Why does the I2C transfer fail ?

You  are asking me? Because your patches break some important setup sequence.
PMIC model does not matter, all behave same way.

> You did mention something regarding I2C/PMIC driver probe timing, but it
> seems the I2C driver and PMIC drivers probe roughly around the same time
> in both pass and fail cases ?

Yes, here I agree that they both probe and probe passes, but I assume
timing of i2c call is critical and there may be some dependency which
is not ready.

> It seems the tegra3 DTs have most of the PMIC regulators marked as
> boot-on and always-on , so enabling the regulators early is the right
> thing to do.

Only essentials are added, thus they are marked this way.

>
> [...]
>
> > SoC: tegra114
> > Model: NVIDIA Tegra Note 7
> > Board: NVIDIA TegraTab
> > DRAM:  1 GiB
> > tegra_i2c_probe: called
> > i2c: controller bus 0 at 7000d000, speed 0: tegra_i2c_probe: exit
> > i2c_init_controller: speed=400000
> > i2c_init_controller: CLK_DIV_STD_FAST_MODE setting = 25
> > pkt header 1 sent (0x10010)
> > pkt header 2 sent (0x0)
> > pkt header 3 sent (0xb0)
> > pkt data sent (0x0)
> > i2c_xfer: 2 messages
> > i2c_xfer: chip=0x58, len=0x1
> > i2c_write_data: chip=0x58, len=0x1
> > write_data:  0xfb
> This seems to be access to register 0xfb , i.e. something else ?