Re: [PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

Re: [PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

[Rob Herring]

On Thu, Apr 23, 2026 at 11:23:11AM +0000, sashiko-bot@kernel.org wrote:
> Sashiko AI review found 3 potential issue(s):

Does anyone read replies to the bot? If so, why was everyone hwmon list 
dropped?

> - [Medium] The `mipi-i3c-static-method` schema incorrectly enforces `minimum: 0x1`, preventing devices from declaring no static address assignment support.
> - [Low] The `assigned-address` property description is now inaccurate and contradicts the new capabilities.
> - [Low] Missing macro definition for Bit 2 ('Other CCC') in `include/dt-bindings/i3c/i3c.h`.
> --

This should go below the quoted commit msg.

> 
> commit dbfc3abafb12042fa0c42239f7ff49c55d30eb3d
> Author: Akhil R <akhilrajeev@nvidia.com>
> 
> dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
> 
> This commit adds the 'mipi-i3c-static-method' property to specify which
> discovery method an I3C device supports during bus initialization. The
> property serves as a bitmap where each bit indicates support for methods
> like SETDASA or SETAASA.
> 
> [1] https://www.mipi.org/mipi-disco-for-i3c-download
> 
> > diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
> > index e25fa72fd7857..225844abdd1fd 100644
> > --- a/Documentation/devicetree/bindings/i3c/i3c.yaml
> > +++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
> [ ... ]
> > @@ -147,6 +149,27 @@ patternProperties:
> >            through SETDASA. If static address is not present, this address is assigned
> >            through SETNEWDA after assigning a temporary address via ENTDAA.
> 
> Does the description for assigned-address need an update here? It currently
> states that if a static address is present, the address is assigned through
> SETDASA. This seems to contradict the new capabilities allowing SETAASA.

Yes.

> 
> >  
> > +      mipi-i3c-static-method:
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        minimum: 0x1
> 
> The description below states that "0 indicates lack of support", but this
> minimum validation prevents setting the bitmap to 0. If an I3C target has a
> static address but relies purely on ENTDAA, how can a developer override the
> default value of 1 to specify that no static methods are supported?

0 is referring to the bit values, not the overall value. However, 
whether 0 should be allowed is a valid point.

> > +        maximum: 0x7
> > +        default: 1
> > +        description: |
> > +          Bitmap describing which methods of Dynamic Address Assignment from a
> > +          static address are supported by this I3C Target. A bit value of 1
> > +          indicates support for that method, and 0 indicates lack of support.
> > +
> > +            Bit 0: SETDASA CCC (Direct)
> > +            Bit 1: SETAASA CCC (Broadcast)
> > +            Bit 2: Other CCC (vendor / standards extension)
> > +            All other bits are reserved.
> [ ... ]
> > diff --git a/include/dt-bindings/i3c/i3c.h b/include/dt-bindings/i3c/i3c.h
> > index 373439218bba6..8409addff0115 100644
> > --- a/include/dt-bindings/i3c/i3c.h
> > +++ b/include/dt-bindings/i3c/i3c.h
> > @@ -13,4 +13,7 @@
> >  #define I2C_NO_FILTER_HIGH_FREQUENCY    (1 << 5)
> >  #define I2C_NO_FILTER_LOW_FREQUENCY     (2 << 5)
> >  
> > +#define I3C_ADDR_METHOD_SETDASA     (1 << 0)
> > +#define I3C_ADDR_METHOD_SETAASA     (1 << 1)
> 
> Since Bit 2 is documented as "Other CCC" in the binding and the maximum value
> allows combinations up to 0x7, should there be a macro defined for Bit 2?
> Adding one could prevent device tree authors from needing to use magic
> numbers to represent this capability.

Yes.

Rob

Re: [PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

[Guenter Roeck]

On 5/5/26 14:32, Rob Herring wrote:
> On Thu, Apr 23, 2026 at 11:23:11AM +0000, sashiko-bot@kernel.org wrote:
>> Sashiko AI review found 3 potential issue(s):
> 
> Does anyone read replies to the bot? If so, why was everyone hwmon list
> dropped?
> 

It appears that people either complain about getting AI feedback, or
they complain that they don't. That is pretty much a lose-lose situation.
For patches sent to linux-hwmon, Sashiko is configured to respond only
to the mailing list and to the submitter in the hope to minimize the
"I do not want to see this" feedback. That matches the configuration
for several subsystems.

As far as I can see it is not currently possible to limit AI review to
patches actually associated with the given subsystem if sent to that
subsystem's mailing list. I'll discuss with Roman to see if patches
like this one can be filtered out when reviewing patches sent to
hwmon mailing list.

>> - [Medium] The `mipi-i3c-static-method` schema incorrectly enforces `minimum: 0x1`, preventing devices from declaring no static address assignment support.
>> - [Low] The `assigned-address` property description is now inaccurate and contradicts the new capabilities.
>> - [Low] Missing macro definition for Bit 2 ('Other CCC') in `include/dt-bindings/i3c/i3c.h`.
>> --
> 
> This should go below the quoted commit msg.
> 

Others (like me) want to see it at the very top to see a quick summary.
Another lose-lose situation.

Again, I'll discuss with Roman if responses can be limited to patches
actually affecting the hwmon subsystem. This should hopefully reduce
unwanted feedback.

Thanks,
Guenter

>>
>> commit dbfc3abafb12042fa0c42239f7ff49c55d30eb3d
>> Author: Akhil R <akhilrajeev@nvidia.com>
>>
>> dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA
>>
>> This commit adds the 'mipi-i3c-static-method' property to specify which
>> discovery method an I3C device supports during bus initialization. The
>> property serves as a bitmap where each bit indicates support for methods
>> like SETDASA or SETAASA.
>>
>> [1] https://www.mipi.org/mipi-disco-for-i3c-download
>>
>>> diff --git a/Documentation/devicetree/bindings/i3c/i3c.yaml b/Documentation/devicetree/bindings/i3c/i3c.yaml
>>> index e25fa72fd7857..225844abdd1fd 100644
>>> --- a/Documentation/devicetree/bindings/i3c/i3c.yaml
>>> +++ b/Documentation/devicetree/bindings/i3c/i3c.yaml
>> [ ... ]
>>> @@ -147,6 +149,27 @@ patternProperties:
>>>             through SETDASA. If static address is not present, this address is assigned
>>>             through SETNEWDA after assigning a temporary address via ENTDAA.
>>
>> Does the description for assigned-address need an update here? It currently
>> states that if a static address is present, the address is assigned through
>> SETDASA. This seems to contradict the new capabilities allowing SETAASA.
> 
> Yes.
> 
>>
>>>   
>>> +      mipi-i3c-static-method:
>>> +        $ref: /schemas/types.yaml#/definitions/uint32
>>> +        minimum: 0x1
>>
>> The description below states that "0 indicates lack of support", but this
>> minimum validation prevents setting the bitmap to 0. If an I3C target has a
>> static address but relies purely on ENTDAA, how can a developer override the
>> default value of 1 to specify that no static methods are supported?
> 
> 0 is referring to the bit values, not the overall value. However,
> whether 0 should be allowed is a valid point.
> 
>>> +        maximum: 0x7
>>> +        default: 1
>>> +        description: |
>>> +          Bitmap describing which methods of Dynamic Address Assignment from a
>>> +          static address are supported by this I3C Target. A bit value of 1
>>> +          indicates support for that method, and 0 indicates lack of support.
>>> +
>>> +            Bit 0: SETDASA CCC (Direct)
>>> +            Bit 1: SETAASA CCC (Broadcast)
>>> +            Bit 2: Other CCC (vendor / standards extension)
>>> +            All other bits are reserved.
>> [ ... ]
>>> diff --git a/include/dt-bindings/i3c/i3c.h b/include/dt-bindings/i3c/i3c.h
>>> index 373439218bba6..8409addff0115 100644
>>> --- a/include/dt-bindings/i3c/i3c.h
>>> +++ b/include/dt-bindings/i3c/i3c.h
>>> @@ -13,4 +13,7 @@
>>>   #define I2C_NO_FILTER_HIGH_FREQUENCY    (1 << 5)
>>>   #define I2C_NO_FILTER_LOW_FREQUENCY     (2 << 5)
>>>   
>>> +#define I3C_ADDR_METHOD_SETDASA     (1 << 0)
>>> +#define I3C_ADDR_METHOD_SETAASA     (1 << 1)
>>
>> Since Bit 2 is documented as "Other CCC" in the binding and the maximum value
>> allows combinations up to 0x7, should there be a macro defined for Bit 2?
>> Adding one could prevent device tree authors from needing to use magic
>> numbers to represent this capability.
> 
> Yes.
> 
> Rob
> 

Re: [PATCH v3 01/13] dt-bindings: i3c: Add mipi-i3c-static-method to support SETAASA

[Rob Herring]

On Tue, May 5, 2026 at 5:17 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 5/5/26 14:32, Rob Herring wrote:
> > On Thu, Apr 23, 2026 at 11:23:11AM +0000, sashiko-bot@kernel.org wrote:
> >> Sashiko AI review found 3 potential issue(s):
> >
> > Does anyone read replies to the bot? If so, why was everyone hwmon list

everyone *but* the hwmon list...

> > dropped?
> >
>
> It appears that people either complain about getting AI feedback, or
> they complain that they don't. That is pretty much a lose-lose situation.
> For patches sent to linux-hwmon, Sashiko is configured to respond only
> to the mailing list and to the submitter in the hope to minimize the
> "I do not want to see this" feedback. That matches the configuration
> for several subsystems.
>
> As far as I can see it is not currently possible to limit AI review to
> patches actually associated with the given subsystem if sent to that
> subsystem's mailing list. I'll discuss with Roman to see if patches
> like this one can be filtered out when reviewing patches sent to
> hwmon mailing list.

No, I want the replies. I was just wondering why it is not reply-all
which is standard. I was also wondering why I was only sometimes
seeing the reviews. I have since figured that out and submitted a PR
to add DT. I set it to reply-all, so I guess there will be some
comment about that.

I don't think there's any solution if a patch hits 2 conflicting
configurations. Surely the folks that don't want to see it can figure
out how to delete or ignore emails... I have thousands of ignored
emails. It's not hard. :)

> >> - [Medium] The `mipi-i3c-static-method` schema incorrectly enforces `minimum: 0x1`, preventing devices from declaring no static address assignment support.
> >> - [Low] The `assigned-address` property description is now inaccurate and contradicts the new capabilities.
> >> - [Low] Missing macro definition for Bit 2 ('Other CCC') in `include/dt-bindings/i3c/i3c.h`.
> >> --
> >
> > This should go below the quoted commit msg.
> >
>
> Others (like me) want to see it at the very top to see a quick summary.
> Another lose-lose situation.

If there's anything kernel developers agree on, it is not top posting.
I don't see how sashiko is special here. Isn't below the commit
message close enough to the top? I don't mind so much about the list
of issues being at the top, but I really don't see why the commit
message is not quoted (I would guess that's just an artifact of
creating the reply out of the git tree it was applied to?). The commit
hash is also completely useless to anyone without access to the git
tree sashiko uses.

One thing I would love to see is reviews of the commit message itself.
I would love to never have to say "don't use 'This commit/patch'" or
"the commit msg should answer why, not what" again.

Rob