Re: [RFCv4 3/7] dt-bindings: devfreq: imx: Describe interconnect properties

[Leonard Crestez <leonard.crestez@nxp.com>]

On 24.09.2019 00:04, Rob Herring wrote:
> On Mon, Sep 23, 2019 at 12:42 PM Leonard Crestez
> <leonard.crestez@nxp.com> wrote:
>>
>> On 17.09.2019 23:20, Rob Herring wrote:
>>> On Fri, Aug 23, 2019 at 05:36:56PM +0300, Leonard Crestez wrote:
>>>> The interconnect-node-id property is parsed by the imx interconnect
>>>> driver to find nodes on which frequencies can be adjusted.
>>>>
>>>> Add #interconnect-cells so that device drivers can request paths from
>>>> bus nodes instead of requiring a separate "virtual" node to represent
>>>> the interconnect itself.
>>>>
>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/devfreq/imx-ddrc.yaml | 5 +++++
>>>>    Documentation/devicetree/bindings/devfreq/imx.yaml      | 5 +++++
>>>>    2 files changed, 10 insertions(+)
>>>
>>> Please combine this with the other series for devfreq support.
>>
>> I understand that having two series which add to the same bindings file
>> is odd but the devfreq and interconnect parts are independent to a very
>> large degree and devfreq can be useful on it's own.
> 
> To start with, I'm suspicious of any 'devfreq' binding because that's
> a Linux thing. I somewhat expect that the interconnect binding should
> replace the devfreq binding, but I haven't been able to investigate.
> 
> Design something that matches the structure of the h/w not how Linux
> drivers happen to be structured. I can't tell what that is without any
> context around adding a couple of properties. Nor do I have the time
> to dig into each SoC vendor's bus structure if it's even documented
> publicly.

Device tree binding files are organized based on linux subsystems but 
otherwise there's little particularly specific to "devfreq" in this new 
binding.

An imx NIC or NOC is a physical component of a SOC which can run at a 
variable clock speed. The device binding uses standard clk and opp 
tables in meaningful ways so that first part is reasonable.

I also want to implement an interconnect provider but an "interconnect" 
is not a single device but rather a graph of discrete buses. Some options:
* Add a custom virtual device, easy but not upstreamable.
* Have a single icc provider device use the individual buses as 
"proxies" for OF parting. Implemented in this series but not very pretty.
* Pick the "main noc" as the single interconnect provider?

Alternatively the approach of defining the graph in the driver could be 
dropped and everything could be described in DT (quite verbose). This 
seems to be what Samsung is doing:

https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=176291

It might even possible to use of_graph; this is complicated but the NICs 
and NOCs do in fact have discrete ports with assignable properties.

 > I also don't follow why you need 'interconnect-node-id' and if you do,
 > it should be a common property.

The "interconnect-node-id" property in devicetree identifies nodes from 
the interconnect graph; for example the DDRC node identifies itself as 
IMX8MM_ICS_DRAM.

--
Regards,
Leonard