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: <Z0Q4vGXbvU3j9H65@google.com>
Date: Mon, 25 Nov 2024 16:43:40 +0800
From: "Sung-Chi, Li" <lschyi@...omium.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org>,
	Sebastian Reichel <sre@...nel.org>, Lee Jones <lee@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, chrome-platform@...ts.linux.dev,
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH 2/2] dt-bindings: mfd: cros-ec: add properties for
 thermal cooling cells

On Mon, Nov 25, 2024 at 08:32:19AM +0100, Krzysztof Kozlowski wrote:
> On 25/11/2024 03:50, Sung-Chi, Li wrote:
> > On Fri, Nov 22, 2024 at 08:49:14AM +0100, Krzysztof Kozlowski wrote:
> >> On Fri, Nov 22, 2024 at 11:47:22AM +0800, Sung-Chi Li wrote:
> >>> The cros_ec supports limiting the input current to act as a passive
> >>> thermal cooling device. Add the property '#cooling-cells' bindings, such
> >>> that thermal framework can recognize cros_ec as a valid thermal cooling
> >>> device.
> >>>
> >>> Signed-off-by: Sung-Chi Li <lschyi@...omium.org>
> >>> ---
> >>>  Documentation/devicetree/bindings/mfd/google,cros-ec.yaml | 3 +++
> >>>  1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> >>> index aac8819bd00b..2b6f098057af 100644
> >>> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> >>> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> >>> @@ -96,6 +96,9 @@ properties:
> >>>    '#gpio-cells':
> >>>      const: 2
> >>>  
> >>> +  '#cooling-cells':
> >>> +    const: 2
> >>
> >> This is not a cooling device. BTW, your commit msg is somehow circular.
> >> "Add cooling to make it a cooling device because it will be then cooling
> >> device."
> >>
> >> Power supply already provides necessary framework for managing charging
> >> current and temperatures. If this is to stay, you need to explain why
> >> this is suitable to be considered a thermal zone or system cooling
> >> device (not power supply or input power cooling).
> >>
> >> Best regards,
> >> Krzysztof
> >>
> > 
> > Thank you, I will rephrase the commit message. The reason to not to use the
> > managing charging current and temperatures in the power supply framework is
> > that:
> > 
> > - The EC may not have the thermal sensor value for the charger, and there is no
> >   protocol for getting the thermal sensor value for the charger (there is
> >   command for reading thermal sensor values, but there is no specification for
> >   what sensor index is for the charger, if the charger provides thermal value).
> > - The managing mechanism only take the charger thermal value into account, and
> >   I would like to control the current based on the thermal condition of the
> >   whole device.
> > 
> > I will include these explanation in the following changes.
> 
> 
> This does not explain me why this is supposed to be thermal zone. I
> already said it, but let's repeat: This is not a thermal zone. This
> isn't thermal zone sensor, either.

Hi, I added the explanation in the commit message in v2, in short, I need to use
different thermal sensors, and need finer thermal controls, so I have to use
thermal zone. This is included in the v2 commit message.

> 
> 
> Best regards,
> Krzysztof

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ