Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> writes: > On 12/10/2022 04:39, jerome Neanne wrote: >>> You explained what you did, which is easily visible. You did not explain >>> why you are doing it. >>> >>> Best regards, >>> Krzysztof >>> >> Thanks for pointing me to the detailed guidelines >> I'm new to upstream and not well aware of all good practices. >> >> Would below commit message be more suitable: >> >> Add support for the TPS65219 PMIC by enabling MFD, regulator and >> power-button drivers. All drivers enabled as modules. > > This still says only what you did. I still does not explain why. Jerome, maybe adding a bit of preamble like: "Development boards from TI include the TPS65219 PMIC. Add support..." Krzysztof, I'm the first to argue for descriptive/verbose changelogs, but IMO, this is getting a little bit nit-picky. The series adds a new driver, DTS and defconfig patches to enable support the new driver. The "why" for changes to defconfig changes like this are kind of implied/obvious, and there is lots of precedent for changelogs of defconfig changes for simple drivers to simply say "enable X and Y". If my above suggesion is not enough, please make a suggestion for what you think would qualify as an appropritate changelong that answers "why" for a simple driver change. Kevin
