Re: [ndctl PATCH v3 2/4] test: Don't skip tests if nfit modules are missing

"Verma, Vishal L" <[email protected]> writes:

> On Fri, 2021-03-19 at 11:20 +0530, Santosh Sivaraj wrote:
>> "Verma, Vishal L" <[email protected]> writes:
>
> [..]
>
>> > 
>> > fix multi line comment to the right formatting:
>> > /*
>> >  * line 1, etc
>> >  */
>> > 
>> 
>> Will fix that.
>> 
>> > > +	if (access("/sys/bus/acpi", F_OK) == -1) {
>> > > +		if (errno == ENOENT)
>> > > +			family = NVDIMM_FAMILY_PAPR;
>> > > +	}
>> > 
>> > Instead of a blind default, can we perform a similar check for presence of
>> > PAPR too?
>> > 
>> 
>> Yes, I wanted to do that, but there is no reliable way of check that; there is
>> no ofnode before module load, and there won't be any PAPR specific DT entries if
>> the platform is not Power.
>> 
>> I also test the 'ndtest' module on x86 with NDCTL_TEST_FAMILY environment
>> variable. I can let the default be nfit_test (NVDIMM_FAMILY_INTEL) and only load
>> PAPR module when the environment variable is set. Thoughts?
>> 
> The env variable seems reasonable to me. If there is ever a third
> 'family' adding tests, having an arbitrary default might be awkward.
> I may suggest - if acpi is detected, use NFIT. If env has something that
> is known, e.g. PAPR, use that. If env is unset or doesn't match anything
> we know about, then bail with an error message. Does that sound
> reasonable?

Yes, that sounds too to me. I will send in the next version soon.
Thanks for the review!

Thanks,
Santosh
_______________________________________________
Linux-nvdimm mailing list -- [email protected]
To unsubscribe send an email to [email protected]