| Message ID | 20221108232228.1177199-1-nfraprado@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64: defconfig: Enable missing kconfigs for mt8183-kukui-jacuzzi-juniper | expand |
On Wed, Nov 9, 2022, at 00:22, Nícolas F. R. A. Prado wrote: > mt8183-kukui-jacuzzi-juniper is one of the devices set up to run tests > on KernelCI, but several of its drivers are currently disabled in the > defconfig. This series enables all the missing kconfigs on the defconfig > to get everything probing on that machine so that it can be fully tested > by KernelCI. The changes all look fine, but I would recommend not separating it out into 13 patches when you are doing just one thing here. As a general rule, if you keep saying the same things in each patch description, it is usually an indication that they should be combined. Similarly, if you find describing unrelated changes ("also, ..."), that would be an indication that patches should be split up. > Given that all kconfigs added in the series are to enable support for a > MediaTek platform, it seems reasonable for it to be applied through the > MediaTek tree, but the commits themselves are independent (apart from > MTK_CMDQ and MTK_SVS) and could be applied separately. Agreed, merging this through the Mediatek tree is the preferred way. Arnd
On 09/11/2022 08:28, Arnd Bergmann wrote: > On Wed, Nov 9, 2022, at 00:22, Nícolas F. R. A. Prado wrote: >> mt8183-kukui-jacuzzi-juniper is one of the devices set up to run tests >> on KernelCI, but several of its drivers are currently disabled in the >> defconfig. This series enables all the missing kconfigs on the defconfig >> to get everything probing on that machine so that it can be fully tested >> by KernelCI. > > The changes all look fine, but I would recommend not separating it > out into 13 patches when you are doing just one thing here. > > As a general rule, if you keep saying the same things in each > patch description, it is usually an indication that they should > be combined. Similarly, if you find describing unrelated changes > ("also, ..."), that would be an indication that patches should > be split up. I agree. Descriptions you wrote are useful - they explain why you are doing it - but it got all really too detailed, just for defconfigs. One commit per one symbol is a bit too much... Best regards, Krzysztof
On Wed, Nov 09, 2022 at 10:08:27AM +0100, Krzysztof Kozlowski wrote: > On 09/11/2022 08:28, Arnd Bergmann wrote: > > On Wed, Nov 9, 2022, at 00:22, Nícolas F. R. A. Prado wrote: > >> mt8183-kukui-jacuzzi-juniper is one of the devices set up to run tests > >> on KernelCI, but several of its drivers are currently disabled in the > >> defconfig. This series enables all the missing kconfigs on the defconfig > >> to get everything probing on that machine so that it can be fully tested > >> by KernelCI. > > > > The changes all look fine, but I would recommend not separating it > > out into 13 patches when you are doing just one thing here. > > > > As a general rule, if you keep saying the same things in each > > patch description, it is usually an indication that they should > > be combined. Similarly, if you find describing unrelated changes > > ("also, ..."), that would be an indication that patches should > > be split up. > > I agree. Descriptions you wrote are useful - they explain why you are > doing it - but it got all really too detailed, just for defconfigs. One > commit per one symbol is a bit too much... Okay, thank you both for the feedback. Given that this is a contentious file, splitting independent symbols throughout the commits seemed to be more friendly to possible merge conflicts. But you're right that there's a single overarching goal for all the changes, so I'll join them all into a single commit as suggested. Thanks, Nícolas