linux-kernel - Re: [PATCH RFC v6 1/6] dt-bindings: exynos-bus: Add documentation for interconnect properties

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]

Date:   Wed, 9 Sep 2020 12:07:46 +0300
From:   Georgi Djakov <georgi.djakov@...aro.org>
To:     Sylwester Nawrocki <s.nawrocki@...sung.com>,
        Rob Herring <robh@...nel.org>
Cc:     cw00.choi@...sung.com, krzk@...nel.org, devicetree@...r.kernel.org,
        a.swigon@...sung.com, myungjoo.ham@...sung.com,
        inki.dae@...sung.com, sw0312.kim@...sung.com,
        b.zolnierkie@...sung.com, m.szyprowski@...sung.com,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-samsung-soc@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH RFC v6 1/6] dt-bindings: exynos-bus: Add documentation for
 interconnect properties

Hi Sylwester,

On 8/28/20 17:49, Sylwester Nawrocki wrote:
> On 30.07.2020 14:28, Sylwester Nawrocki wrote:
>> On 09.07.2020 23:04, Rob Herring wrote:
>>> On Thu, Jul 02, 2020 at 06:37:19PM +0200, Sylwester Nawrocki wrote:
>>>> Add documentation for new optional properties in the exynos bus nodes:
>>>> samsung,interconnect-parent, #interconnect-cells, bus-width.
>>>> These properties allow to specify the SoC interconnect structure which
>>>> then allows the interconnect consumer devices to request specific
>>>> bandwidth requirements.
>>>>
>>>> Signed-off-by: Artur Świgoń <a.swigon@...sung.com>
>>>> Signed-off-by: Sylwester Nawrocki <s.nawrocki@...sung.com>
> 
>>>> --- a/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>>>> +++ b/Documentation/devicetree/bindings/devfreq/exynos-bus.txt
>>>> @@ -51,6 +51,13 @@ Optional properties only for parent bus device:
>>>>  - exynos,saturation-ratio: the percentage value which is used to calibrate
>>>>  			the performance count against total cycle count.
>>>>  
>>>> +Optional properties for interconnect functionality (QoS frequency constraints):
>>>> +- samsung,interconnect-parent: phandle to the parent interconnect node; for
>>>> +  passive devices should point to same node as the exynos,parent-bus property.
> 
>>> Adding vendor specific properties for a common binding defeats the 
>>> point.
> 
> Actually we could do without any new property if we used existing interconnect
> consumers binding to specify linking between the provider nodes. I think those
> exynos-bus nodes could well be considered both the interconnect providers 
> and consumers. The example would then be something along the lines 
> (yes, I know the bus node naming needs to be fixed):
> 
> 	soc {
> 		bus_dmc: bus_dmc {
> 			compatible = "samsung,exynos-bus";
> 			/* ... */
> 			samsung,data-clock-ratio = <4>;
> 			#interconnect-cells = <0>;
> 		};
> 
> 		bus_leftbus: bus_leftbus {
> 			compatible = "samsung,exynos-bus";
> 			/* ... */
> 			interconnects = <&bus_leftbus &bus_dmc>;
> 			#interconnect-cells = <0>;
> 		};
> 
> 		bus_display: bus_display {
> 			compatible = "samsung,exynos-bus";
> 			/* ... */
> 			interconnects = <&bus_display &bus_leftbus>;

Hmm, bus_display being a consumer of itself is a bit odd? Did you mean:
 			interconnects = <&bus_dmc &bus_leftbus>;

> 			#interconnect-cells = <0>;
> 		};
> 
> 
> 		&mixer {
> 			compatible = "samsung,exynos4212-mixer";
> 			interconnects = <&bus_display &bus_dmc>;
> 			/* ... */
> 		};
> 	};
> 
> What do you think, Georgi, Rob?

I can't understand the above example with bus_display being it's own consumer.
This seems strange to me. Could you please clarify it?

Otherwise the interconnect consumer DT bindings are already well established
and i don't see anything preventing a node to be both consumer and provider.
So this should be okay in general.

Thanks,
Georgi