[PATCH v2 1/3] regulator: mt6360: Add OF match table

[Vaittinen, Matti]

On Mon, 2021-01-11 at 13:38 +0100, Matthias Brugger wrote:
> Hi Matti,
> 
> On 11/01/2021 11:32, Vaittinen, Matti wrote:
> > Hello Matthias & All,
> > 
> > On Mon, 2021-01-11 at 11:08 +0100, Matthias Brugger wrote:
> > > On 11/01/2021 03:18, gene_chen(陳俊宇) wrote:
> > > > [ Internal Use - External ]
> > > > 
> > > 
> > > Please don't top-post in the future.
> > > 
> > > > Hi Matthias,
> > > > 
> > > > I discussed OF match table with Mark in previous mail in our
> > > > PATCH
> > > > v3,
> > > > MFD should just instantiate the platform device.
> > > 
> > > Did you ever test that? Which MFD driver should instantiate the
> > > device?
> > > I had a look at mt6360-core.c (the obvious one) but I don't see
> > > any
> > > reference to
> > > the regulator [1]. What am I missing?
> > > 
> > > > > Mark Brown <broonie at kernel.org> 於 2020年8月20日 週四 下午7:45寫道：
> > > > > > This device only exists in the context of a single parent
> > > > > > device, there
> > > > > > should be no need for a compatible string here - this is
> > > > > > just a
> > > > > > detail
> > > > > > of how Linux does things.  The MFD should just instntiate
> > > > > > the
> > > > > > platform
> > > > > > device.
> > > > > Trying to autoload module without of_id_table will cause run-
> > > > > time 
> > > > > error:
> > > > > ueventd: LoadWithAliases was unable to load
> > > > > of:NregulatorT(null)Cmediatek,mt6360-regulator
> > > > 
> > > > You shouldn't have this described in the device tree at all,
> > > > like I
> > > > say
> > > > the MFD should just instantiate the platform device.
> > > > 
> > > 
> > > Well from my understanding the regulator has a device-tree entry
> > > [2],
> > > so it
> > > needs to match against a device-tree node. My understanding is,
> > > that
> > > you need a
> > > the devicetree node to describe the regulators provided by the
> > > device. TBH I'm a
> > > bit puzzled about the comment from Mark here. How does another DT
> > > node be able
> > > to reference a regulator if this is not described in DT? I think
> > > we
> > > need a DT
> > > node here and the matching in the regulator and MFD driver to get
> > > the
> > > regulator
> > > loaded via udev.
> > 
> > Others are better to answer - but as I spotted this from my inbox
> > I'll
> > give my 2 cents :) 
> > 
> > This can be done W/O regulators having own compatible. Please see
> > following:
> > 
> > drivers/mfd/rohm-bd718x7.c
> > drivers/regulator/bd718x7-regulator.c
> > Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml
> > Documentation/devicetree/bindings/regulator/rohm,bd71847-
> > regulator.yaml
> > 
> > as example.
> 
> Thanks for the links. Yes that's' a way to go, but...
> 
> > The device matching can be done via platform_device_id table.
> > I think the MODULE_ALIAS() is needed for module matching - but I
> > don't
> > remember without further code browsing.
> > 
> > Anyways, the BD71837/47/50 DT entry without compatible for
> > clk/regulators do probe and load the sub-devices.
> > 
> > As a "tradeoff" subdevices must retrieve the DT node from the
> > parent
> > device.
> > 
> > For my uneducated eyes the DT binding for regulators should be
> > changed.
> > Compatible should not be required and the example node should be
> > moved
> > to MFD binding document in the MFD node. But that's just my view on
> > this - not willing to push this to any direction!
> > 
> 
> Generally speaking, DT bindings are stable and can't be changed in a
> none-compatible way. We could argue if there is any user of the
> compatible out
> there (probably there isn't any, as the code seems to not even be
> working).

I think this depends on the device where driver(s) are used. Quite a
few devices allow updating the device-tree when SW is updated. But as I
said, I don't want to try pushing this one direction or other - I leave
it for those who have broader shoulders for pushing XD.

I was merely trying to answer your question :)

Best Regards
	Matti