linux-kernel - Re: [PATCH v1 0/2] Enable WDT reload feature

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:
 <OSQPR06MB72529E67E67D0D07E59AF1C08B4F2@OSQPR06MB7252.apcprd06.prod.outlook.com>
Date: Fri, 25 Oct 2024 02:00:39 +0000
From: Billy Tsai <billy_tsai@...eedtech.com>
To: Guenter Roeck <linux@...ck-us.net>, Uwe Kleine-König
	<u.kleine-koenig@...libre.com>
CC: "jdelvare@...e.com" <jdelvare@...e.com>, "robh@...nel.org"
	<robh@...nel.org>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>, "joel@....id.au"
	<joel@....id.au>, "andrew@...econstruct.com.au"
	<andrew@...econstruct.com.au>, "linux-hwmon@...r.kernel.org"
	<linux-hwmon@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-aspeed@...ts.ozlabs.org"
	<linux-aspeed@...ts.ozlabs.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-pwm@...r.kernel.org"
	<linux-pwm@...r.kernel.org>, BMC-SW <BMC-SW@...eedtech.com>
Subject: Re: [PATCH v1 0/2] Enable WDT reload feature

> On 10/24/24 08:40, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Thu, Oct 24, 2024 at 03:15:46PM +0800, Billy Tsai wrote:
> >> Aspeed PWM controller has the WDT reload feature, which changes the duty
> >> cycle to a preprogrammed value after a WDT/EXTRST#.
> >>
> >> Billy Tsai (2):
> >>    hwmon: (aspeed-g6-pwm-tacho): Extend the #pwm-cells to 4
> >>    hwmon: (aspeed-g6-pwm-tacho): Support the WDT reload
> >
> > Huh, I'm not convinced that extending #pwm-cells for that feature is a
> > good idea. Unless I'm missing something none of the other supported PWM
> > chips can do that, so I hesitate to change a standard for it. I suggest
> > to make this a separate property instead.
> >

> Agreed.
> Guenter

Hi Uwe and Guenter,

Using a separate property to enable this feature is a straightforward method, but I don’t understand why extending #pwm-cells isn’t a good idea in my situation. The feature ‘WDT reload’ can be set for individual PWM channels, and the PWM subsystem has the of_xlate callback hook, which allows each driver to define its arguments for the PWM consumer. I’m unsure if I misunderstood this callback usage, as I couldn’t find examples. If my understanding is correct, this method is better for adding our specific feature, rather than using child nodes or separate properties to indicate which PWM channel should enable this feature with the corresponding duty cycle values. I think using separate properties to achieve this feature would be quite cumbersome.
As I know the arguments for this usage are as follows:
First: PWM channel index
Second: PWM period in ns
Third: PWM polarity
Therefore, I extended our feature to a fourth argument to avoid any confusion regarding usage and added the description in our yaml file.

If my thinking is incorrect or doesn’t make sense, please let me know.

Thanks