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, 17 May 2023 12:14:08 -0500
From:   Andrew Davis <afd@...com>
To:     Vignesh Raghavendra <vigneshr@...com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
CC:     <linux-clk@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] dt-bindings: clock: ehrpwm: Remove unneeded syscon
 compatible

On 5/16/23 11:36 PM, Vignesh Raghavendra wrote:
> 
> 
> On 17/05/23 00:16, Andrew Davis wrote:
>> This node's register space is not accessed by any other node, which
>> is the traditional use for the "syscon" hint.
> 
> Unfortunately that's not the case across SoCs. Eg AM65x See  TRM section
> Table 5-582. CTRLMMR_EPWM0_CTRL Register Field Descriptions
> 

Not sure what version of the TRM you have, latest (Rev. E) has this
register as Table 5-636.. but I found it and see your point here.

> TB_CLKEN is clubbed with SYNCIN_SEL and ePWM tripzone configuration
> signals which may require register to be shared with other drivers in future
> 

This looks to only be a problem in AM65x, all later devices we have fixed
the issue and now group the clock enable bits all together.

Do we actually expect this to be an issue and have a user of these
other bits? If so then we modeled this region wrong in AM65x DT, these
registers are not "tbclk gate registers" any more then they are to the
other functions they provide. These registers should be a syscon node
and then each function within should be a child node.

syscon@...0 {
	compatible = "ti,am654-epwm-crtl", "syscon";
	reg = <0x4140 0x18>;

	ehrpwm_tbclk: clock {
		compatible = "ti,am654-ehrpwm-tbclk";
		#clock-cells = <1>;
	};

	pwm_mux: mux-controller {
		compatible = "mmio-mux";
		#mux-control-cells = <1>;
	};
};

Something like that. That way we do not give preference to one device
and have to have it give out shared registers.

Either that or split the binding compatible, one for AM65x with syscon
and one for all later device compatibles that do not share the register:

compatible:
     oneOf:
       - items:
           - const: ti,am654-ehrpwm-tbclk
           - const: syscon
       - items:
         - enum:
           - ti,am64-epwm-tbclk
           - ti,am62-epwm-tbclk

Would rather the first option.

Andrew

> 
>> It looks to have been
>> added here to make use of a Linux kernel helper syscon_node_to_regmap().
>> The Linux driver now uses a more appropriate helper that does not
>> require the hint, so let's remove it from the binding.
>>
>> Signed-off-by: Andrew Davis <afd@...com>
>> ---
>>   .../devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml     | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml b/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml
>> index 66765116aff5..64b8bce5962c 100644
>> --- a/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml
>> +++ b/Documentation/devicetree/bindings/clock/ti,am654-ehrpwm-tbclk.yaml
>> @@ -16,7 +16,6 @@ properties:
>>             - ti,am654-ehrpwm-tbclk
>>             - ti,am64-epwm-tbclk
>>             - ti,am62-epwm-tbclk
>> -      - const: syscon
>>   
>>     "#clock-cells":
>>       const: 1
>> @@ -33,8 +32,8 @@ additionalProperties: false
>>   
>>   examples:
>>     - |
>> -    ehrpwm_tbclk: syscon@...0 {
>> -        compatible = "ti,am654-ehrpwm-tbclk", "syscon";
>> +    ehrpwm_tbclk: clock@...0 {
>> +        compatible = "ti,am654-ehrpwm-tbclk";
>>           reg = <0x4140 0x18>;
>>           #clock-cells = <1>;
>>       };
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ