On Sat, Sep 02, 2023 at 08:49:58PM -0700, Davidlohr Bueso wrote: > On Mon, 07 Aug 2023, Jehoon Park wrote: > > > Add a new command: 'set-alert-config', which configures device's warning > > alert. > > The example in the cover-letter should be here and also mention explicitly > that > the get counterpart is via cxl-list -A, like you have in the manpage. >
Thank you for comments. I will reorganize and update cover letter and commit message for clear explanation about the patchset. > > > > Signed-off-by: Jehoon Park <[email protected]> > > --- > > Documentation/cxl/cxl-set-alert-config.txt | 96 +++++++++ > > Documentation/cxl/meson.build | 1 + > > cxl/builtin.h | 1 + > > cxl/cxl.c | 1 + > > cxl/memdev.c | 220 ++++++++++++++++++++- > > 5 files changed, 318 insertions(+), 1 deletion(-) > > create mode 100644 Documentation/cxl/cxl-set-alert-config.txt > > > > diff --git a/Documentation/cxl/cxl-set-alert-config.txt > > b/Documentation/cxl/cxl-set-alert-config.txt > > new file mode 100644 > > index 0000000..c905f7c > > --- /dev/null > > +++ b/Documentation/cxl/cxl-set-alert-config.txt > > @@ -0,0 +1,96 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +cxl-set-alert-config(1) > > +======================= > > + > > +NAME > > +---- > > +cxl-set-alert-config - set the warning alert threshold on a CXL memdev > > + > > +SYNOPSIS > > +-------- > > +[verse] > > +'cxl set-alert-config <mem0> [<mem1>..<memN>] [<options>]' > > + > > +DESCRIPTION > > +----------- > > +CXL device raises an alert when its health status is changed. Critical > > alert > > +shall automatically be configured by the device after a device reset. > > +If supported, programmable warning thresholds also be initialized to vendor > > +recommended defaults, then could be configured by the user. > > + > > +Use this command to configure warning alert thresholds of a device. > > +Having issued this command, the newly requested warning thresholds would > > +override the previously programmed warning thresholds. > > + > > +To enable warning alert, set both 'threshold=value' and 'alert=on'. To > > disable > > +warning alert, set only 'alert=off'. Other cases would cause errors. > > So what's the point of having to use double parameter to enable the warning? > Just do alert=threshold if you've established that threshold=N and alert=off > is > not valid. > IMHO, using two separate parameters would be better because it would be safer by protecting a misuse out of an user. I think it is important to clearly interpret user's intention. Also, CXL spec (8.2.9.8.3.3) defines two payload fields, threshold and enablement, separetely. Please also remind that the 'ndctl-inject-smart' command already uses two parameters, threshold and alarm, to set smart threshold. > > + > > +Use "cxl list -m <memdev> -A" to examine the programming warning threshold > > +capabilities of a device. > > + > > +EXAMPLES > > +-------- > > +Set warning threshold to 30 and enable alert for life used. > > +[verse] > > +cxl set-alert-config mem0 -L 30 --life-used-alert=on > > + > > +Disable warning alert for device over temperature. > > +[verse] > > +cxl set-alert-config mem0 --over-temperature-alert=off > > + > > +OPTIONS > > +------- > > +<memory device(s)>:: > > +include::memdev-option.txt[] > > + > > +-v:: > > +--verbose=:: > > + Turn on verbose debug messages in the library (if libcxl was built > > with > > + logging and debug enabled). > > Should be at the end. > Agree > > Thanks, > Davidlohr Sincerely, Jehoon
