On Tue, Jun 29, 2021 at 08:34:55PM +0200, H. Nikolaus Schaller wrote: > > Am 29.06.2021 um 17:59 schrieb Mark Brown <broonie@xxxxxxxxxx>: > > What is that rule and how is this patch intended to ensure that Palmas > > meets it? > > As covered in submitting-patches.rst your changelog should > > explain this so that in review we can verify that this is a good fix. > I am very sorry, but I simply believed that it is not necessary to copy&paste or > describe this because it appears not to be difficult to retrieve. So, I did actually look at the commit but I couldn't figure out what the change was supposed to do about it. > This rule (rdev->supply_name && !rdev->supply) did not exist before 98e48cd9283d > and it seems to return early with EPROBE_DEFER if there is a desc->supply_name defined, > but no supply resolved. > The Palmas driver is setting desc->supply_name to some string constant (i.e. not NULL) > and is then calling devm_regulator_register(). Right, this is how a regualtor driver should specify the name of its supply. > So it was working fine without having the supplying regulator resolved. AFAIK they > just serve as fixed regulators in the device tree and have no physical equivalent. No, not at all - it's representing whatever provides input power to the regulator. There may be no physical control of it at runtime on your system but that may not be true on other systems. It's quite common for there to be a chain of regulators (eg, DCDCs supplying LDOs) and then they all need to get get power managed appropriately and you don't end up thinking a regulator is enabled when the input regulator is disabled. > My proposal just moves setting the supply_name behind devm_regulator_register() and > by that restores the old behaviour. This means that we won't actually map the supply and any system that relies on software handling the supply regulator will be broken. > Well, unless... > ... devm_regulator_register() does something differently if desc->supply_name > is not set before and changed afterwards. It may miss that change. We resolve supplies during regulator registration, this would effectively just skip mapping of the supply. > So I hope for guidance if my approach is good or needs a different solution. What I would expect to happen here would be that once vsys_cobra is registered the regulators supplied by it can register and then all their consumers would in turn be able to register. You should look into why that supply regulator isn't appearing and resolve that, or if a consumer isn't handling deferral then that would need to be addressed.
Attachment:
signature.asc
Description: PGP signature