Re: [PATCH v6 4/8] hwmon: (amc6821) add support for tsd,mule

[Krzysztof Kozlowski]

On 12/08/2024 13:58, Quentin Schulz wrote:
> Hi Krzysztof,
> 
> On 8/12/24 1:38 PM, Krzysztof Kozlowski wrote:
>> [Some people who received this message don't often get email from krzk@xxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> On 31/07/2024 17:12, Guenter Roeck wrote:
>>> On Thu, Jul 25, 2024 at 03:27:50PM +0200, Farouk Bouabid wrote:
>>>> Theobroma Systems Mule is an MCU that emulates a set of I2C devices,
>>>> among which is an amc6821 and other devices that are reachable through
>>>> an I2C-mux.
>>>>
>>>> The devices on the mux can be selected by writing the appropriate device
>>>> number to an I2C config register (amc6821: reg 0xff)
>>>>
>>>> Implement "tsd,mule" compatible to instantiate the I2C-mux platform device
>>>> when probing the amc6821.
>>>>
>>>> Signed-off-by: Farouk Bouabid <farouk.bouabid@xxxxxxxxx>
>>>> Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>
>>>
>>> Applied.
>>
>> Eh, there is undocumented dependency on I2C here. Next has warning
>> because of this.
>>
> 
> I think you meant to comment this on 
> https://lore.kernel.org/linux-i2c/20240725-dev-mule-i2c-mux-v6-0-f9f6d7b60fb2@xxxxxxxxx/T/#mdb7976f1dc16fce0b7db9abee6fd0b1fd0a2e2ba 
> (patch 3 and not 4 of the series). This patch (4) is fine on its own I 
> believe, no dependency on anything else. (well, except if we expect 
> bindings to be absolutely merged before the drivers? I think what 
> matters is the Device Tree changes making use of the new binding be 
> merged after dt-binding changes?).

Yeah, this was about DT binding.

> 
> I agree that there's a somewhat non-obvious dependency between patch 1 
> and 3 (the dt-bindings) and 5-8 with everything before, we could have 
> made this more explicit.
> 
>> Farouk, please *always mention* the dependencies between patches.
>>
> 
> I wasn't aware of that rule, my apologies for not catching this before 
> upstream submission.
> 
> For anyone wondering the rule is made explicit here:
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes
> 
> "If one patch depends on another patch in order for a change to be 
> complete, that is OK. Simply note “this patch depends on patch X” in 
> your patch description."
> 
> Question about b4 workflow though. I encourage using b4 to avoid as many 
> mistakes as possible and make the workflow as painless as possible. I 
> believe b4 doesn't allow you to have per-patch notes, only in the 
> cover-letter.

"Patch description" or "per patch notes" is whatever you write in
changelog, so under ---.


> a) is this dependency list in cover-letter acceptable, or
> b) need to add it to the patch note (below the ---), or

One of above should be enough, both are more welcomed because many
maintainers ignore completely cover letters.

> c) can add it to the patch commit log

No, if patches go through separate trees then it would be just confusing
and not helping at all.

Best regards,
Krzysztof