[Regtool/topgen] Add vendor_specific_attributes env variable

[sha-ron]

Add an option to define environment variable that will specify the vendor_specific_fields.hjson file.
To use it you can add to the sim_cfg hjson file something like:
exports: [{VENDOR_SPECIFIC_FIELDS: "<path_to_vendor_specific_fields.hjson"}]

[andreaskurth]

Thx @sha-ron, I think this should work but would appreciate if @rswarbrick and @engdoreis also review. Just one suggestion

[andreaskurth]

Since this is an env var that is "globally visible" in the execution environment, should its name be more precise regarding its namespace? E.g., OT_REGTOOL_VENDOR_SPECIFIC_FIELDS?

[rswarbrick]

Also, do you still have a use for the --vendor-specific-fields argument?

If so, we probably need to define what happens if it and the environment variable are both supplied. At the moment, the argument wins: reasonable, but we probably need at least a warning message.

My preference is to delete one of the options, or to spit out an error if both have been supplied :-)

Also, this code is getting duplicated in topgen.py. If we decide to keep both paths, I suggest moving the code to vendor_specific.py. Maybe the extend_optional_fields function could be something like this:

def extend_optional_fields(arg_vendor_file: str | None) -> None:
    """Extend optional fields with vendor-specific fields if supplied.

    If the OT_REGTOOL_VENDOR_SPECIFIC_FIELDS environment variable has been set,
    it is used as a fall-back when arg_vendor_file is also supplied.
    """

    from_environ = os.environ.get("OT_REGTOOL_VENDOR_SPECIFIC_FIELDS")

    if arg_vendor_file is not None and from_environ is not None:
        raise RuntimeError("Conflict between command line and environment.")

    vendor_file = arg_vendor_file or from_environ
    if not vendor_file:
        # No vendor file supplied: nothing to do.
        return

    vendor_specific = import_fields(vendor_file)
    ...

[sha-ron]

--vendor-specific-fields argument is used by @meisnere for the design so I guess we need both paths. But indeed it will be good to issue an error if both are supplied. I can try to move the code to vendor_specific.py or if you have a better idea how to control --vendor-specific-fields argument from hjson/core file you can issue a new PR and I will close this one (you have a better understanding of those scripts so it might be better).

[sha-ron]

Since this is an env var that is "globally visible" in the execution environment, should its name be more precise regarding its namespace? E.g., OT_REGTOOL_VENDOR_SPECIFIC_FIELDS?

I updated the name. Thank you @andreaskurth for noticing and noting.

[sha-ron]

Thanks for the review @rswarbrick. Hope it is ok now.

[rswarbrick]

This looks sensible to me. I think it should be good to go once the lint errors are fixed.

[sha-ron]

This looks sensible to me. I think it should be good to go once the lint errors are fixed.

Fixed lint issue, not sure if/how FPGA failure is related.

[andreaskurth]

Thanks for implementing the feedback, @sha-ron. I have two suggestions; other than that LGTM

[andreaskurth]

This environment, if set, also is a path to a file, right? So for clarity, I suggest:

Suggested change

	from_environ = os.environ.get("OT_REGTOOL_VENDOR_SPECIFIC_FIELDS")
	env_vendor_file = os.environ.get("OT_REGTOOL_VENDOR_SPECIFIC_FIELDS_FILE")

[andreaskurth]

I recommend expanding and clarifying this docstring:

Suggested change

	If the OT_REGTOOL_VENDOR_SPECIFIC_FIELDS environment variable has been set,
	it is used as a fall-back when arg_vendor_file is also supplied.
	There are two ways to supply a path to a file listing vendor-specific fields (either ... or):
	a) through the arg_vendor_file argument of this function
	b) through the OT_REGTOOL_VENDOR_SPECIFIC_FIELDS_FILE environment variable
	If the function argument is not None and the environment variable is set, a runtime error is raised.