Re: [PATCH v8 1/5] cxl, ACPI, APEI, EINJ: Add CXL_EINJ Kconfig option

[Ben Cheatham <benjamin.cheatham@amd.com>]

Hi Dan, sorry for the wait, but I finally am getting some time to take a look at this. I haven't gotten around
to taking a look at updating the module init to use a platform driver, but I should be able to get to it in the next
couple of days here. Thanks for the review!

On 12/18/23 5:48 PM, Dan Williams wrote:
> Ben Cheatham wrote:
>> Add CONFIG_CXL_EINJ to cxl/Kconfig. This option will allow for the CXL
>> core module to access helpers inside the EINJ module, while also giving
>> users the option of disabling CXL EINJ error types at build time.
>>
>> Also update CONFIG_ACPI_APEI_EINJ to set CONFIG_CXL_EINJ by default.
>>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com>
>> ---
>>  drivers/acpi/apei/Kconfig |  1 +
>>  drivers/cxl/Kconfig       | 12 ++++++++++++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
>> index 6b18f8bc7be3..4c3f0ec5731e 100644
>> --- a/drivers/acpi/apei/Kconfig
>> +++ b/drivers/acpi/apei/Kconfig
>> @@ -55,6 +55,7 @@ config ACPI_APEI_MEMORY_FAILURE
>>  config ACPI_APEI_EINJ
>>  	tristate "APEI Error INJection (EINJ)"
>>  	depends on ACPI_APEI && DEBUG_FS
>> +	imply CXL_BUS
> 
> This can safely be deleted, it is sufficient for CXL_BUS to depend on
> ACPI_APEI_EINJ.
> 

Ok, will do.

>>  	help
>>  	  EINJ provides a hardware error injection mechanism, it is
>>  	  mainly used for debugging and testing the other parts of
>> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
>> index 8ea1d340e438..6f4adcd733e5 100644
>> --- a/drivers/cxl/Kconfig
>> +++ b/drivers/cxl/Kconfig
>> @@ -154,4 +154,16 @@ config CXL_PMU
>>  	  monitoring units and provide standard perf based interfaces.
>>  
>>  	  If unsure say 'm'.
>> +
>> +config CXL_EINJ
>> +	bool "CXL Error INJection Support"
>> +	default ACPI_APEI_EINJ
>> +	depends on ACPI_APEI_EINJ && ACPI_APEI_EINJ=CXL_BUS
> 
> It is ok for CXL_BUS to be a module while ACPI_APEI_EINJ is built-in, so
> the && can be dropped since CXL_BUS is guaranteed to be > 0 here, i.e.:
> 
>     default ACPI_APEI_EINJ
>     depends on ACPI_APEI_EINJ >= CXL_BUS

Sounds good. I don't have much experience with Kconfig and wasn't aware that >= was a thing, good to know!