On Thu, Mar 27, 2025 at 06:02:27AM +0000, Wei Fang wrote: > > > > > > >The current patch logic is that if the pcie device node is found to > > > > > > >have the "xxx-supply" property, the scan will be skipped, and then > > > > > > >the pwrctrl driver will rescan and enable the regulators. However, > > > > > > >after merging this patch, there is a problem on our platform. The > > > > > > >.probe() of our device driver will not be called. The reason is > > > > > > >that CONFIG_PCI_PWRCTL_SLOT is not enabled at all in our > > > > > > >configuration file, and the compatible string of the device is also not > > > > added to the pwrctrl driver. > > > > > > > > > > > > Hmm. So I guess the controller driver itself is enabling the > > > > > > supplies I believe (which I failed to spot). May I know what platforms are > > > > affected? > > > > > > > > > > Yes, the affected device is an Ethernet controller on our i.MX95 > > > > > platform, it has a "phy-supply" property to control the power of the > > > > > external Ethernet PHY chip in the device driver. > > > > > > > > Ah, I was not aware of any devices using 'phy-supply' in the pcie device node. > > > > > > It is not a standard property defined in ethernet-controller.yaml. Maybe > > > for other vendors, it’s called "vdd-supply" or something else. > > > > > > > Ah, then why is it used at all in the first place (if not defined in the > > binding)? This makes me wonder if I really have to fix anything since everything > > we are talking about are out of tree. > > "phy-supply" is a vendor defined property, we have added it to fsl,fec.yaml, > but fec is not a PCIe device. And this property is also added to other Ethernet > devices such as allwinner,sun4i-a10-mdio.yaml and rockchip,emac.yaml, etc. > But they are all not a PCIe device. So there is no need to fix it in upstream. > > > > > > > > > > > > This part has not been > > > > > pushed upstream yet. So for upstream tree, there is no need to fix our > > > > > platform, but I am not sure whether other platforms are affected by > > > > > this on the upstream tree. > > > > > > > > > > > > > Ok, this makes sense and proves that my grep skills are not bad :) I don't > > think > > > > there is any platform in upstream that has the 'phy-supply' in the pcie node. > > > > But I do not want to ignore this property since it is pretty valid for existing > > > > ethernet drivers to control the ethernet device attached via PCIe. > > > > > > > > > > > > > > > > > I think other > > > > > > >platforms should also have similar problems, which undoubtedly make > > > > > > >these platforms be unstable. This patch has been applied, and I am > > > > > > >not familiar with this. Can you fix this problem? I mean that those > > > > > > >platforms that do not use pwrctrl can avoid skipping the scan. > > > > > > > > > > > > Sure. It makes sense to add a check to see if the pwrctrl driver is enabled > > or > > > > not. > > > > > > If it is not enabled, then the pwrctrl device creation could be > > > > > > skipped. I'll send a patch once I'm infront of my computer. > > > > > > > > > > > > > > > > I don't know whether check the pwrctrl driver is enabled is a good > > > > > idea, for some devices it is more convenient to manage these > > > > > regulators in their drivers, for some devices, we may want pwrctrl > > > > > driver to manage the regulators. If both types of devices appear on > > > > > the same platform, it is not enough to just check whether the pinctrl driver > > is > > > > enabled. > > > > > > > > > > > > > Hmm. Now that I got the problem clearly, I think more elegant fix would be > > to > > > > ignore the device nodes that has the 'phy-supply' property. I do not envision > > > > device nodes to mix 'phy-supply' and other '-supply' properties though. > > > > > > > > > > I think the below solution is not generic, "phy-supply" is just an example, > > > the following modification is only for this case. In fact, there is also a > > > "serdes-supply" on our platform, of course, this is not included in the > > > upstream, because we haven't had time to complete these. So for the > > > "serdes-supply" case, the below solution won't take effect. > > > > > > > Does your platform have a serdes connected to the PCIe port? I doubt so. Again, > > these are all non-standard properties, not available in upstream. So I'm not > > going to worry about them. > > No, the serdes is inside the Ethernet MAC. I was wondering how to bypass > pwrctrl in the future if we add such a "xxx-supply" to a PCIe device node, > so that our drivers can be smoothly accepted by upstream. > In that case, the logic should be changed to only look for bindings defined supplies for the endpoint devices. Like, "v*pcie*-supply" properties instead of all supplies. - Mani -- மணிவண்ணன் சதாசிவம்
