Message-ID: <YxcSaYuFlFoIphJ5@EPUAKYIW015D>
Date:   Tue, 6 Sep 2022 09:27:05 +0000
From:   Oleksii Moisieiev <Oleksii_Moisieiev@...m.com>
To:     Ahmad Fatoum <a.fatoum@...gutronix.de>
CC:     "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mcoquelin.stm32@...il.com" <mcoquelin.stm32@...il.com>,
        "alexandre.torgue@...com" <alexandre.torgue@...com>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "tomase@...inx.com" <tomase@...inx.com>,
        Benjamin Gaignard <benjamin.gaignard@...labora.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "arnd@...db.de" <arnd@...db.de>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "fabio.estevam@....com" <fabio.estevam@....com>,
        "loic.pallardy@...com" <loic.pallardy@...com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        Sudeep Holla <sudeep.holla@....com>,
        Cristian Marussi <cristian.marussi@....com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        "Peng Fan (OSS)" <peng.fan@....nxp.com>
Subject: Re: [PATCH v4 0/2] dt-bindings: Intorduce domain-controller

On Tue, Aug 30, 2022 at 09:34:16AM +0200, Ahmad Fatoum wrote:
> Hello Oleksii,
> 
> On 18.08.22 11:05, Oleksii Moisieiev wrote:
> > On Mon, Aug 15, 2022 at 06:37:23PM +0200, Ahmad Fatoum wrote:
> >> Hello Oleksii,
> >>
> >> On 07.07.22 12:25, Oleksii Moisieiev wrote:
> >>> Introducing the domain controller provider/consumenr bindngs which allow to
> >>> divided system on chip into multiple domains that can be used to select
> >>> by who hardware blocks could be accessed.
> >>> A domain could be a cluster of CPUs, a group of hardware blocks or the
> >>> set of devices, passed-through to the Guest in the virtualized systems.
> >>>
> >>> Device controllers are typically used to set the permissions of the hardware
> >>> block. The contents of the domain configuration properties are defined by the
> >>> binding for the individual domain controller device.
> >>>
> >>> The device controller conception in the virtualized systems is to set
> >>> the device configuration for SCMI (System Control and Management
> >>> Interface) which controls clocks/power-domains/resets etc from the
> >>> Firmware. This configuratio sets the device_id to set the device permissions
> >>> for the Fimware using BASE_SET_DEVICE_PERMISSIONS message (see 4.2.2.10 of [0]).
> >>> There is no BASE_GET_DEVICE_PERMISSIONS call in SCMI and the way to
> >>> determine device_id is not covered by the specification.
> >>> Device permissions management described in DEN 0056, Section 4.2.2.10 [0].
> >>> Given parameter should set the device_id, needed to set device
> >>> permissions in the Firmware.
> >>> This property is used by trusted Agent (which is hypervisor in our case)
> >>> to set permissions for the devices, passed-through to the non-trusted
> >>> Agents. Trusted Agent will use device-perms to set the Device
> >>> permissions for the Firmware (See Section 4.2.2.10 [0] for details).
> >>> Agents concept is described in Section 4.2.1 [0].
> >>>
> >>> Domains in Device-tree node example:
> >>> usb@...90000
> >>> {
> >>>     domain-0 = <&scmi 19>; //Set domain id 19 to usb node
> >>>     clocks = <&scmi_clock 3>, <&scmi_clock 2>;
> >>>     resets = <&scmi_reset 10>, <&scmi_reset 9>;
> >>>     power-domains = <&scmi_power 0>;
> >>> };
> >>>
> >>> &scmi {
> >>>     #domain-cells = <1>;
> >>> }
> >>>
> >>> All mentioned bindings are going to be processed by XEN SCMI mediator
> >>> feature, which is responsible to redirect SCMI calls from guests to the
> >>> firmware, and not going be passed to the guests.
> >>>
> >>> Domain-controller provider/consumenr concept was taken from the bus
> >>> controller framework patch series, provided in the following thread:
> >>> [1].
> >>
> >> I also was inspired by Benjamin's series to draft up a binding, but for a slightly
> >> different problem: Some SoCs like the i.MX8MP have a great deal of variation
> >> in which IPs are actually available. After factory testing, fuses are burnt
> >> to describe which IPs are available and as the upstream DT only describes
> >> the full featured SoCs, either board DT or bootloader is expected to turn
> >> off the device that are unavailable.
> >>
> >> What I came up with as a binding for the bootloader to guide its fixup
> >> looks very similar to what you have:
> >>
> >> feat: &ocotp { /* This is the efuse (On-Chip OTP) device */
> >>     feature-controller;
> >>     feature-cells = <1>;
> >> };
> >>
> >> &vpu_g1 {
> >>     features-gates = <&feat IMX8MP_VPU>;
> >> };
> >>
> >> The OCOTP driver would see that it has a feature-controller property and register
> >> a callback with a feature controller framework that checks whether a device
> >> is available. barebox, that I implemented this binding for, would walk
> >> the kernel device tree on boot looking for the feature-gates property and then
> >> disable/delete nodes as indicated without having to write any SoC specific code
> >> and especially without hardcoding node names and hierarchies, which is quite brittle.
> >>
> >> There was a previous attempt at defining a binding for this, but Rob's NAK
> >> mentioned that a solution should cover both cases:
> >>
> >>  https://urldefense.com/v3/__https://lore.kernel.org/all/20220324042024.26813-1-peng.fan@oss.nxp.com/__;!!GF_29dbcQIUBPA!2j_vN6Jc1k2XI3EegAC2yzTLgJ1Rw1DhDrjGF03a5tDtOGpm_qp9B0zHJeAJzw-fWOeJp5HtnzYmOJZ0XPJxHzHFDBt_$  [lore[.]kernel[.]org]
> >>
> >> Having implemented nearly the same binding as what you describe, I obviously like your
> >> patch. Only thing I think that should be changed is the naming. A domain doesn't
> >> really describe this gated-by-fuses scenario I have. Calling it feature-gates
> >> instead OTOH makes sense for both your and my use case. Same goes for the documentation
> >> that could be worded more generically. I am open to other suggestions of course. :-)
> >>
> >> Also a general gpio-controller like property would be nice. It would allow drivers
> >> to easily check whether they are supposed to register a domain/feature controller.
> >> For devices like yours where a dedicated device node represents the domain controller,
> >> it's redundant, but for a fuse bank, it's useful. #feature-cells could be used for
> >> that, but I think a dedicated property may be better.
> >>
> >> Let me know what you think and thanks for working on this!
> >>
> >> Cheers,
> >> Ahmad
> >>
> > 
> > Hello Ahmad,
> > 
> > I'm very happy that you are interested in my proposal. It will be great
> > if we produce common binding to suite both our requirements.
> > I agree that binding should be renamed, but I don't think feature-gates
> > name would fit my case.
> > IIUC both our cases requires different devices across the system to
> > provide some information to the controller device. This information
> > could be used to identify the devices later or to make some
> > controller-specific configuration. In this case I would prefer name
> > "device-feature" or "bus-domain", suggested by Linus Walleij.
> > Also I like your idea to add dedicated property. This will make bindings
> > more clear.
> > Summarizing all above, I would suggest the following names:
> > 
> >  feat: &ocotp { /* This is the efuse (On-Chip OTP) device */
> >      device-feature-controller;
> >      device-feature-cells = <1>;
> >  };
> > 
> >  &vpu_g1 {
> >      device-features = <&feat IMX8MP_VPU>;
> >  };
> > 
> > What do you think about this?
> 
> Sorry for the late answer. Full plate before vacation :)
> 
> A device- prefix for device properties is kind of redundant IMO.
> And [device-]features is somewhat ambiguous (it's not
> a list of features of the device, but a list of features that
> control the device). I see that gates might sounds a bit odd, how about
> feature-domains, feature-domain-controller, #feature-domain-cells?
> 
> Cheers,
> Ahmad
> 

Hello Ahmad,

feature-domains works for me as well. I will prepare patches if nobody
have any additional comments.

--
Oleksii

> > 
> > Best regards,
> > Oleksii.
> > 
> >>
> >>>
> >>> I think we can cooperate with the bus controller framework developers
> >>> and produce the common binding, which will fit the requirements of both
> >>> features
> >>>
> >>> Also, I think that binding can also be used for STM32 ETZPC bus
> >>> controller feature, proposed in the following thread: [2].
> >>>
> >>> Looking forward for your thoughts and ideas.
> >>>
> >>> [0] https://urldefense.com/v3/__https://developer.arm.com/documentation/den0056/latest__;!!GF_29dbcQIUBPA!2j_vN6Jc1k2XI3EegAC2yzTLgJ1Rw1DhDrjGF03a5tDtOGpm_qp9B0zHJeAJzw-fWOeJp5HtnzYmOJZ0XPJxH59KKjhc$  [developer[.]arm[.]com]
> >>> [1] https://urldefense.com/v3/__https://lore.kernel.org/all/20190318100605.29120-1-benjamin.gaignard@st.com/__;!!GF_29dbcQIUBPA!2j_vN6Jc1k2XI3EegAC2yzTLgJ1Rw1DhDrjGF03a5tDtOGpm_qp9B0zHJeAJzw-fWOeJp5HtnzYmOJZ0XPJxHy1kyyWZ$  [lore[.]kernel[.]org]
> >>> [2] https://urldefense.com/v3/__https://lore.kernel.org/all/20200701132523.32533-1-benjamin.gaignard@st.com/__;!!GF_29dbcQIUBPA!2j_vN6Jc1k2XI3EegAC2yzTLgJ1Rw1DhDrjGF03a5tDtOGpm_qp9B0zHJeAJzw-fWOeJp5HtnzYmOJZ0XPJxHzVdVT4B$  [lore[.]kernel[.]org]
> >>>
> >>> ---
> >>> Changes v1 -> V2:
> >>>    - update parameter name, made it xen-specific
> >>>    - add xen vendor bindings
> >>>
> >>> Changes V2 -> V3:
> >>>    - update parameter name, make it generic
> >>>    - update parameter format, add link to controller
> >>>    - do not include xen vendor bindings as already upstreamed
> >>>
> >>> Changes V3 -> V4:
> >>>    - introduce domain controller provider/consumer device tree bindings
> >>>    - making scmi node to act as domain controller provider when the
> >>>      device permissions should be configured
> >>> ---
> >>>
> >>> Oleksii Moisieiev (2):
> >>>   dt-bindings: Document common device controller bindings
> >>>   dt-bindings: Update scmi node description
> >>>
> >>>  .../bindings/domains/domain-controller.yaml   | 80 +++++++++++++++++++
> >>>  .../bindings/firmware/arm,scmi.yaml           | 25 ++++++
> >>>  2 files changed, 105 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/domains/domain-controller.yaml
> >>>
> >>
> >>
> >> -- 
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | https://urldefense.com/v3/__http://www.pengutronix.de/__;!!GF_29dbcQIUBPA!2j_vN6Jc1k2XI3EegAC2yzTLgJ1Rw1DhDrjGF03a5tDtOGpm_qp9B0zHJeAJzw-fWOeJp5HtnzYmOJZ0XPJxH_HqFmwM$  [pengutronix[.]de]  |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://urldefense.com/v3/__http://www.pengutronix.de/__;!!GF_29dbcQIUBPA!3ZZDuNOsR-mCQt8F9mbsVQvjDi0X_yfmxS65xA-VBjaknyBBRUGdS2y5z6lnRcdi0AfVhp0n_2LXh1V4GabWEUYzqKJ3$  [pengutronix[.]de]  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Powered by blists - more mailing lists