On Wed, Jan 8, 2020 at 4:52 PM Nicolas Boichat <drinkcat@xxxxxxxxxxxx> wrote: > > On Wed, Jan 8, 2020 at 9:23 PM Mark Brown <broonie@xxxxxxxxxx> wrote: > > > > On Wed, Jan 08, 2020 at 01:23:34PM +0800, Nicolas Boichat wrote: > > > > > Some GPUs, namely, the bifrost/g72 part on MT8183, have a second > > > regulator for their SRAM, let's add support for that. > > > > > + pfdev->regulator_sram = devm_regulator_get_optional(pfdev->dev, "sram"); > > > + if (IS_ERR(pfdev->regulator_sram)) { > > > > This supply is required for the devices that need it so I'd therefore > > expect the driver to request the supply non-optionally based on the > > compatible string rather than just hoping that a missing regulator isn't > > important. > > That'd be a bit awkward to match, though... Currently all bifrost > share the same compatible "arm,mali-bifrost", and it'd seem > weird/wrong to match "mediatek,mt8183-mali" in this driver? I have no > idea if any other Mali implementation will require a second regulator, > but with the MT8183 we do need it, see below. The current number of supported bifrost platforms is 0. It's only a matter of time until SoC specific compatibles need to be used in the driver. This is why we require them. It could very well be that all bifrost implementations need 2 supplies. On chip RAMs are very frequently a separate thing which are synthesized differently from logic. At least within a specific IP model, I somewhat doubt there's a variable number of supplies. It could be possible to connect both to the same supply, but the correct way to handle that is both -supply properties point to the same regulator. Rob