linux-kernel - Re: [PATCH v2 1/3] regulator: mt6360: Add OF match table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b3b57c9e393a4330f7430db490c8ef758b89bec6.camel@fi.rohmeurope.com>
Date:   Mon, 11 Jan 2021 12:53:33 +0000
From:   "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
To:     "matthias.bgg@...nel.org" <matthias.bgg@...nel.org>,
        "mbrugger@...e.com" <mbrugger@...e.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "gene_chen@...htek.com" <gene_chen@...htek.com>
CC:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "chen.zhong@...iatek.com" <chen.zhong@...iatek.com>,
        "axel.lin@...ics.com" <axel.lin@...ics.com>,
        "hsin-hsiung.wang@...iatek.com" <hsin-hsiung.wang@...iatek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH v2 1/3] regulator: mt6360: Add OF match table


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@...nel.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