On Fri, Apr 22, 2022 at 09:43:39PM +0700, Quan Nguyen wrote:
> On 22/04/2022 13:20, Greg Kroah-Hartman wrote:
> > On Fri, Apr 22, 2022 at 09:46:47AM +0700, Quan Nguyen wrote:
> > > This commit adds Ampere's SMpro error monitor driver for monitoring
> > > and reporting RAS-related errors as reported by SMpro co-processor
> > > found on Ampere's Altra processor family.
> > >
> > > Signed-off-by: Quan Nguyen <[email protected]>
> > > ---
> > > Changes in v8:
> > > + Update wording for SMPRO_ERRMON on Kconfig file [Quan]
> > > + Avoid uninitialized variable use [kernel test robot]
> > > + Switch to use sysfs_emit() [Greg]
> > > + Make sysfs to return single value [Greg]
> > > + Change errors_* sysfs to error_* [Quan]
> > > + Add overflow_[core|mem|pcie|other]_[ce|ue] sysfs to report
> > > overflow status of each type of HW errors [Quan]
> > > + Add some minor refactor [Quan]
> > >
> > > Changes in v7:
> > > + Remove regmap_acquire/release_lock(), read_i2c_block_data() [Quan]
> > > + Use regmap_noinc_read() instead of errmon_read_block() [Quan]
> > > + Validate number of errors before read [Quan]
> > > + Fix wrong return type of *_show() function [kernel test robot]
> > > + Adjust patch order to avoid dependence with smpro-mfd [Lee Jones]
> > > + Use pointer instead of stack memory [Quan]
> > >
> > > Changes in v6:
> > > + First introduced in v6 [Quan]
> > >
> > > drivers/misc/Kconfig | 12 +
> > > drivers/misc/Makefile | 1 +
> > > drivers/misc/smpro-errmon.c | 477 ++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 490 insertions(+)
> > > create mode 100644 drivers/misc/smpro-errmon.c
> > >
> > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > > index 41d2bb0ae23a..9fbe6797c440 100644
> > > --- a/drivers/misc/Kconfig
> > > +++ b/drivers/misc/Kconfig
> > > @@ -176,6 +176,18 @@ config SGI_XP
> > > this feature will allow for direct communication between SSIs
> > > based on a network adapter and DMA messaging.
> > > +config SMPRO_ERRMON
> > > + tristate "Ampere Computing SMPro error monitor driver"
> > > + depends on MFD_SMPRO || COMPILE_TEST
> > > + help
> > > + Say Y here to get support for the SMpro error monitor function
> > > + provided by Ampere Computing's Altra and Altra Max SoCs. Upon
> > > + loading, the driver creates sysfs files which can be use to gather
> > > + multiple HW error data reported via read and write system calls.
> > > +
> > > + To compile this driver as a module, say M here. The driver will be
> > > + called smpro-errmon.
> > > +
> > > config CS5535_MFGPT
> > > tristate "CS5535/CS5536 Geode Multi-Function General Purpose Timer (MFGPT) support"
> > > depends on MFD_CS5535
> > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > > index 70e800e9127f..483308a6e113 100644
> > > --- a/drivers/misc/Makefile
> > > +++ b/drivers/misc/Makefile
> > > @@ -23,6 +23,7 @@ obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
> > > obj-$(CONFIG_KGDB_TESTS) += kgdbts.o
> > > obj-$(CONFIG_SGI_XP) += sgi-xp/
> > > obj-$(CONFIG_SGI_GRU) += sgi-gru/
> > > +obj-$(CONFIG_SMPRO_ERRMON) += smpro-errmon.o
> > > obj-$(CONFIG_CS5535_MFGPT) += cs5535-mfgpt.o
> > > obj-$(CONFIG_GEHC_ACHC) += gehc-achc.o
> > > obj-$(CONFIG_HP_ILO) += hpilo.o
> > > diff --git a/drivers/misc/smpro-errmon.c b/drivers/misc/smpro-errmon.c
> > > new file mode 100644
> > > index 000000000000..df7d8fc4ff3f
> > > --- /dev/null
> > > +++ b/drivers/misc/smpro-errmon.c
> > > @@ -0,0 +1,477 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> >
> > Are you sure you mean "or any later version"? I have to ask.
> >
>
> Thank Greg for the review.
>
> Will change all to SPDX-License-Identifier: GPL-2.0-or-later in next
> version.
That is not what I am asking (the SPDX tag format). I mean, do you
really mean "or later" for your license as that is not the license of
the kernel overall? If so, wonderful, but I have to ask that as your
legal group needs to be aware of it, sorry.
thanks,
greg k-h
On Fri, Apr 22, 2022 at 04:57:52PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Apr 22, 2022 at 09:43:39PM +0700, Quan Nguyen wrote:
> > On 22/04/2022 13:20, Greg Kroah-Hartman wrote:
> > > On Fri, Apr 22, 2022 at 09:46:47AM +0700, Quan Nguyen wrote:
> > > > This commit adds Ampere's SMpro error monitor driver for monitoring
> > > > and reporting RAS-related errors as reported by SMpro co-processor
> > > > found on Ampere's Altra processor family.
> > > >
> > > > Signed-off-by: Quan Nguyen <[email protected]>
> > > > ---
> > > > Changes in v8:
> > > > + Update wording for SMPRO_ERRMON on Kconfig file [Quan]
> > > > + Avoid uninitialized variable use [kernel test robot]
> > > > + Switch to use sysfs_emit() [Greg]
> > > > + Make sysfs to return single value [Greg]
> > > > + Change errors_* sysfs to error_* [Quan]
> > > > + Add overflow_[core|mem|pcie|other]_[ce|ue] sysfs to report
> > > > overflow status of each type of HW errors [Quan]
> > > > + Add some minor refactor [Quan]
> > > >
> > > > Changes in v7:
> > > > + Remove regmap_acquire/release_lock(), read_i2c_block_data() [Quan]
> > > > + Use regmap_noinc_read() instead of errmon_read_block() [Quan]
> > > > + Validate number of errors before read [Quan]
> > > > + Fix wrong return type of *_show() function [kernel test robot]
> > > > + Adjust patch order to avoid dependence with smpro-mfd [Lee Jones]
> > > > + Use pointer instead of stack memory [Quan]
> > > >
> > > > Changes in v6:
> > > > + First introduced in v6 [Quan]
> > > >
> > > > drivers/misc/Kconfig | 12 +
> > > > drivers/misc/Makefile | 1 +
> > > > drivers/misc/smpro-errmon.c | 477 ++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 490 insertions(+)
> > > > create mode 100644 drivers/misc/smpro-errmon.c
> > > >
> > > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > > > index 41d2bb0ae23a..9fbe6797c440 100644
> > > > --- a/drivers/misc/Kconfig
> > > > +++ b/drivers/misc/Kconfig
> > > > @@ -176,6 +176,18 @@ config SGI_XP
> > > > this feature will allow for direct communication between SSIs
> > > > based on a network adapter and DMA messaging.
> > > > +config SMPRO_ERRMON
> > > > + tristate "Ampere Computing SMPro error monitor driver"
> > > > + depends on MFD_SMPRO || COMPILE_TEST
> > > > + help
> > > > + Say Y here to get support for the SMpro error monitor function
> > > > + provided by Ampere Computing's Altra and Altra Max SoCs. Upon
> > > > + loading, the driver creates sysfs files which can be use to gather
> > > > + multiple HW error data reported via read and write system calls.
> > > > +
> > > > + To compile this driver as a module, say M here. The driver will be
> > > > + called smpro-errmon.
> > > > +
> > > > config CS5535_MFGPT
> > > > tristate "CS5535/CS5536 Geode Multi-Function General Purpose Timer (MFGPT) support"
> > > > depends on MFD_CS5535
> > > > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> > > > index 70e800e9127f..483308a6e113 100644
> > > > --- a/drivers/misc/Makefile
> > > > +++ b/drivers/misc/Makefile
> > > > @@ -23,6 +23,7 @@ obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
> > > > obj-$(CONFIG_KGDB_TESTS) += kgdbts.o
> > > > obj-$(CONFIG_SGI_XP) += sgi-xp/
> > > > obj-$(CONFIG_SGI_GRU) += sgi-gru/
> > > > +obj-$(CONFIG_SMPRO_ERRMON) += smpro-errmon.o
> > > > obj-$(CONFIG_CS5535_MFGPT) += cs5535-mfgpt.o
> > > > obj-$(CONFIG_GEHC_ACHC) += gehc-achc.o
> > > > obj-$(CONFIG_HP_ILO) += hpilo.o
> > > > diff --git a/drivers/misc/smpro-errmon.c b/drivers/misc/smpro-errmon.c
> > > > new file mode 100644
> > > > index 000000000000..df7d8fc4ff3f
> > > > --- /dev/null
> > > > +++ b/drivers/misc/smpro-errmon.c
> > > > @@ -0,0 +1,477 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > >
> > > Are you sure you mean "or any later version"? I have to ask.
> > >
> >
> > Thank Greg for the review.
> >
> > Will change all to SPDX-License-Identifier: GPL-2.0-or-later in next
> > version.
>
> That is not what I am asking (the SPDX tag format). I mean, do you
> really mean "or later" for your license as that is not the license of
> the kernel overall? If so, wonderful, but I have to ask that as your
> legal group needs to be aware of it, sorry.
Generally speaking we should be sticking with the standard license for projects
we engage with, in this case "GPL-2.0".
Quan, unless you have heard differently or have a specific reason to include the
"+", please drop from the next version. If you do feel the "+" is required,
let's have an internal conversation about that.
Apologies for missing that in earlier reviews.
Thanks,
--
Darren Hart
Ampere Computing / OS and Kernel
On 22/04/2022 21:57, Greg Kroah-Hartman wrote:
> On Fri, Apr 22, 2022 at 09:43:39PM +0700, Quan Nguyen wrote:
>> On 22/04/2022 13:20, Greg Kroah-Hartman wrote:
>>> On Fri, Apr 22, 2022 at 09:46:47AM +0700, Quan Nguyen wrote:
>>>> This commit adds Ampere's SMpro error monitor driver for monitoring
>>>> and reporting RAS-related errors as reported by SMpro co-processor
>>>> found on Ampere's Altra processor family.
>>>>
>>>> Signed-off-by: Quan Nguyen <[email protected]>
>>>> ---
>>>> Changes in v8:
>>>> + Update wording for SMPRO_ERRMON on Kconfig file [Quan]
>>>> + Avoid uninitialized variable use [kernel test robot]
>>>> + Switch to use sysfs_emit() [Greg]
>>>> + Make sysfs to return single value [Greg]
>>>> + Change errors_* sysfs to error_* [Quan]
>>>> + Add overflow_[core|mem|pcie|other]_[ce|ue] sysfs to report
>>>> overflow status of each type of HW errors [Quan]
>>>> + Add some minor refactor [Quan]
>>>>
>>>> Changes in v7:
>>>> + Remove regmap_acquire/release_lock(), read_i2c_block_data() [Quan]
>>>> + Use regmap_noinc_read() instead of errmon_read_block() [Quan]
>>>> + Validate number of errors before read [Quan]
>>>> + Fix wrong return type of *_show() function [kernel test robot]
>>>> + Adjust patch order to avoid dependence with smpro-mfd [Lee Jones]
>>>> + Use pointer instead of stack memory [Quan]
>>>>
>>>> Changes in v6:
>>>> + First introduced in v6 [Quan]
>>>>
>>>> drivers/misc/Kconfig | 12 +
>>>> drivers/misc/Makefile | 1 +
>>>> drivers/misc/smpro-errmon.c | 477 ++++++++++++++++++++++++++++++++++++
>>>> 3 files changed, 490 insertions(+)
>>>> create mode 100644 drivers/misc/smpro-errmon.c
>>>>
>>>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>>>> index 41d2bb0ae23a..9fbe6797c440 100644
>>>> --- a/drivers/misc/Kconfig
>>>> +++ b/drivers/misc/Kconfig
>>>> @@ -176,6 +176,18 @@ config SGI_XP
>>>> this feature will allow for direct communication between SSIs
>>>> based on a network adapter and DMA messaging.
>>>> +config SMPRO_ERRMON
>>>> + tristate "Ampere Computing SMPro error monitor driver"
>>>> + depends on MFD_SMPRO || COMPILE_TEST
>>>> + help
>>>> + Say Y here to get support for the SMpro error monitor function
>>>> + provided by Ampere Computing's Altra and Altra Max SoCs. Upon
>>>> + loading, the driver creates sysfs files which can be use to gather
>>>> + multiple HW error data reported via read and write system calls.
>>>> +
>>>> + To compile this driver as a module, say M here. The driver will be
>>>> + called smpro-errmon.
>>>> +
>>>> config CS5535_MFGPT
>>>> tristate "CS5535/CS5536 Geode Multi-Function General Purpose Timer (MFGPT) support"
>>>> depends on MFD_CS5535
>>>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>>>> index 70e800e9127f..483308a6e113 100644
>>>> --- a/drivers/misc/Makefile
>>>> +++ b/drivers/misc/Makefile
>>>> @@ -23,6 +23,7 @@ obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
>>>> obj-$(CONFIG_KGDB_TESTS) += kgdbts.o
>>>> obj-$(CONFIG_SGI_XP) += sgi-xp/
>>>> obj-$(CONFIG_SGI_GRU) += sgi-gru/
>>>> +obj-$(CONFIG_SMPRO_ERRMON) += smpro-errmon.o
>>>> obj-$(CONFIG_CS5535_MFGPT) += cs5535-mfgpt.o
>>>> obj-$(CONFIG_GEHC_ACHC) += gehc-achc.o
>>>> obj-$(CONFIG_HP_ILO) += hpilo.o
>>>> diff --git a/drivers/misc/smpro-errmon.c b/drivers/misc/smpro-errmon.c
>>>> new file mode 100644
>>>> index 000000000000..df7d8fc4ff3f
>>>> --- /dev/null
>>>> +++ b/drivers/misc/smpro-errmon.c
>>>> @@ -0,0 +1,477 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>
>>> Are you sure you mean "or any later version"? I have to ask.
>>>
>>
>> Thank Greg for the review.
>>
>> Will change all to SPDX-License-Identifier: GPL-2.0-or-later in next
>> version.
>
> That is not what I am asking (the SPDX tag format). I mean, do you
> really mean "or later" for your license as that is not the license of
> the kernel overall? If so, wonderful, but I have to ask that as your
> legal group needs to be aware of it, sorry.
>
Dear Greg,
My sincere thanks for bringing this up. This would need help from our
legal team to recheck/review and will make appropriate change in my next
version.
- Quan