On 4/15/25 3:37 PM, Matthias Schiffer wrote:
> On Tue, 2025-04-15 at 15:36 +0200, Matthias Schiffer wrote:
>> On Tue, 2025-04-15 at 15:20 +0200, Andrew Lunn wrote:
>>>
>>>> + **UNCOMMENTED_RGMII_MODE**
>>>> + Historially, the RGMII PHY modes specified in Device Trees have been
>>>> + used inconsistently, often referring to the usage of delays on the PHY
>>>> + side rather than describing the board.
>>>> +
>>>> + PHY modes "rgmii", "rgmii-rxid" and "rgmii-txid" modes require the clock
>>>> + signal to be delayed on the PCB; this unusual configuration should be
>>>> + described in a comment. If they are not (meaning that the delay is realized
>>>> + internally in the MAC or PHY), "rgmii-id" is the correct PHY mode.
>>>
>>> It is unclear to me how much ctx_has_comment() will return. Maybe
>>> include an example here of how it should look. I'm assuming:
>>>
>>> /* RGMII delays added via PCB traces */
>>> &enet2 {
>>> phy-mode = "rgmii";
>>> status = "okay";
>>>
>>> fails, but
>>>
>>> &enet2 {
>>> /* RGMII delays added via PCB traces */
>>> phy-mode = "rgmii";
>>> status = "okay";
>>>
>>> passes?
>>
>> Yes, it works like that. I can't claim to fully understand the checkpatch code
>> handling comments, but I copied it from other similar checks and tested it on a
>> few test patches.
>>
>> One thing to note is that I implemented it as a CHK() and not a WARN() because
>> that's what is used for other comment checks like DATA_RACE - meaning it will
>> only trigger with --strict.
>
> Oops, DATA_RACE is actually a WARN(). I must have copied it from some other
> comment check that uses CHK(). Let me know which you want me to use.
I think it's better if this will trigger on plain invocation, so that
there are more chances people are going to actually notice/correct the
thing before the actual submission.
Thanks,
Paolo
