[PATCH v2 3/3] doc/develop/codingstyle.rst: Expand to include CONFIG_IS_ENABLED and PHASE_

[Quentin Schulz]

Hi Tom,

On 3/13/25 5:03 PM, Tom Rini wrote:
> On Thu, Mar 13, 2025 at 11:20:09AM +0100, Quentin Schulz wrote:
>> Hi Tom,
>>
>> On 3/12/25 2:00 AM, Tom Rini wrote:
>>> Expand the conditional compilation section to explain when to use
>>> CONFIG_IS_ENABLED rather than IS_ENABLED and provide an example. Next,
>>> note what the PHASE_ macro is supposed to be used for as well.
>>>
>>
>> Ah! Then ignore my comment on Patch 2/3 about the missing CONFIG_IS_ENABLED
>> :)
>>
>>> Signed-off-by: Tom Rini <trini at konsulko.com>
>>> ---
>>> Changes in v2:
>>> - New patch.
>>> ---
>>>    doc/develop/codingstyle.rst | 17 +++++++++++++++++
>>>    1 file changed, 17 insertions(+)
>>>
>>> diff --git a/doc/develop/codingstyle.rst b/doc/develop/codingstyle.rst
>>> index 7211e4e4eed1..3303fff165de 100644
>>> --- a/doc/develop/codingstyle.rst
>>> +++ b/doc/develop/codingstyle.rst
>>> @@ -192,6 +192,23 @@ inside the block, and check it for correctness (syntax, types, symbol
>>>    references, etc).  Thus, you still have to use an #ifdef if the code inside the
>>>    block references symbols that will not exist if the condition is not met.
>>> +In the case where a symbol may be referenced with an xPL-specific Kconfig
>>
>> Please provide some information about what xPL means (maybe a link to
>> somewhere it's already explained? doc/develop/init.rst:Board Initialisation
>> Flow could be a start? Or maybe rather doc/develop/spl.rst which seems to
>> better match what this is referring to?
> 
> A problem is that, ah yes, doc/develop/spl.rst exists but needs to be
> renamed to xpl.rst and expanded on too. How about for this patch I
> change this to something like:
> 
> When working with xPL (see :doc:`spl` for more information) we need to
> take further care to use the right macro. In the case where a symbol may
> be referenced with an xPL-specific Kconfig...
> 

Ack.

> And in a follow-up patch start addressing spl.rst. And that may get
> slightly blocked on splitting out the patch that allows the HTML code to
> have redirects automatically. I figured that out for the pytest docs
> RFC, so it's just a matter of making that part standalone and posting
> it, I know how to do it.
> 

Not sure what's the issue with the HTML code you're talking about but I 
have a hunch I'm going to figure this out with the soon-to-be-posted 
patch :)

>>> +symbol, use the CONFIG_IS_ENABLED macro instead, in a similar manner:
>>> +
>>> +.. code-block:: c
>>> +
>>> +	if (CONIG_IS_ENABLED(SOMETHING)) {
>>> +		...
>>> +	}
>>> +
>>> +When dealing with a Kconfig symbol that has both a normal name and one or more
>>> +xPL-prefixed names, the Makefile needs special consideration as well. The
>>> +PHASE\_ macro helps us in this situation thusly:
>>> +
>>> +.. code-block:: make
>>> +
>>> +        obj-$(CONFIG_$(PHASE_)SOMETHING) += something.o
>>> +
>>
>> Please highlight non-English words with double tick quotes, that also remove
>> the need to escape special characters like _, e.g.
>>
>> ``PHASE_``
>>
>> Looks good to me otherwise,
> 
> OK, will do.
> 

Depends if you want to keep the import from the kernel verbatim without 
adding tick quotes and all, then I guess consistency will be better and 
just not have tick quotes at all?

Up to you.

Cheers,
Quentin