| Message ID | 20221010201453.77401-1-andriy.shevchenko@linux.intel.com |
|---|---|
| Headers | show |
| Series | pinctrl: Clean up and add missed headers | expand |
On 2022/10/11 5:15, Andy Shevchenko wrote: > Do not imply that some of the generic headers may be always included. > Instead, include explicitly what we are direct user of. > > While at it, sort headers alphabetically. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Looks OK to me, but the patch title should be: pinctrl: k210: Add missing header(s) Same remark for the entire series. You need s/missed/missing in all patch titles. With that fixed, Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > --- > drivers/pinctrl/pinctrl-k210.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-k210.c b/drivers/pinctrl/pinctrl-k210.c > index ecab6bf63dc6..288e44457fec 100644 > --- a/drivers/pinctrl/pinctrl-k210.c > +++ b/drivers/pinctrl/pinctrl-k210.c > @@ -3,18 +3,20 @@ > * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com> > * Copyright (c) 2020 Western Digital Corporation or its affiliates. > */ > -#include <linux/io.h> > -#include <linux/of_device.h> > +#include <linux/bitfield.h> > #include <linux/clk.h> > +#include <linux/io.h> > #include <linux/mfd/syscon.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > -#include <linux/bitfield.h> > #include <linux/regmap.h> > +#include <linux/seq_file.h> > #include <linux/slab.h> > + > +#include <linux/pinctrl/pinconf-generic.h> > +#include <linux/pinctrl/pinconf.h> > #include <linux/pinctrl/pinctrl.h> > #include <linux/pinctrl/pinmux.h> > -#include <linux/pinctrl/pinconf.h> > -#include <linux/pinctrl/pinconf-generic.h> > > #include <dt-bindings/pinctrl/k210-fpioa.h> >
On Mon, Oct 10, 2022 at 10:15 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > Currently the header inclusion inside the pinctrl headers seems more arbitrary > than logical. This series is basically out of two parts: > - add missed headers to the pin control drivers / users > - clean up the headers of pin control subsystem > > The idea is to have this series to be pulled after -rc1 by the GPIO and > pin control subsystems, so all new drivers will utilize cleaned up headers > of the pin control. > > Please, review and comment. > > Changelog v2: > - added preparatory patches: all, but last (LKP) > - added missed forward declaration to the last patch (LKP) > Thanks for doing this. Did you use any kind of automation for detecting for which symbols the headers are missing? Bart
On Tue, Oct 11, 2022 at 1:33 AM Damien Le Moal <Damien.LeMoal@wdc.com> wrote: > On 2022/10/11 5:15, Andy Shevchenko wrote: > > Do not imply that some of the generic headers may be always included. > > Instead, include explicitly what we are direct user of. > > > > While at it, sort headers alphabetically. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Looks OK to me, but the patch title should be: > > pinctrl: k210: Add missing header(s) > > Same remark for the entire series. You need s/missed/missing in all patch titles. Oh, the missing word 'missing' :-) I will replace it locally (I won't resend it because of that). > With that fixed, > > Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Thanks!
On Tue, Oct 11, 2022 at 09:10:07AM +0200, Bartosz Golaszewski wrote: > On Mon, Oct 10, 2022 at 10:15 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > Currently the header inclusion inside the pinctrl headers seems more arbitrary > > than logical. This series is basically out of two parts: > > - add missed headers to the pin control drivers / users > > - clean up the headers of pin control subsystem > > > > The idea is to have this series to be pulled after -rc1 by the GPIO and > > pin control subsystems, so all new drivers will utilize cleaned up headers > > of the pin control. > > > > Please, review and comment. > > > > Changelog v2: > > - added preparatory patches: all, but last (LKP) > > - added missed forward declaration to the last patch (LKP) > > Thanks for doing this. You're welcome! > Did you use any kind of automation for > detecting for which symbols the headers are missing? No, it's manual + what CI(s) reported back to me, that's why even in this series I have got a few compile breakages. I have very limited compile-testing cycle.
On Tue, Oct 11, 2022 at 10:31:33AM +0200, Emil Renner Berthing wrote: > On Mon, 10 Oct 2022 at 22:26, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > > Do not imply that some of the generic headers may be always included. > > Instead, include explicitly what we are direct user of. > > > > While at it, sort headers alphabetically. > > The patch is fine, but I don't see any sorting other than just adding > the headers at the appropriate place. I will amend commit message here. > In any case > > Acked-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> Thank you!
Il 10/10/22 22:14, Andy Shevchenko ha scritto: > Do not imply that some of the generic headers may be always included. > Instead, include explicitly what we are direct user of. > > While at it, sort headers alphabetically. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/mediatek/pinctrl-moore.c | 3 +++ > drivers/pinctrl/mediatek/pinctrl-paris.c | 5 +++++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/pinctrl/mediatek/pinctrl-moore.c b/drivers/pinctrl/mediatek/pinctrl-moore.c > index 526faaebaf77..9474ada5addb 100644 > --- a/drivers/pinctrl/mediatek/pinctrl-moore.c > +++ b/drivers/pinctrl/mediatek/pinctrl-moore.c > @@ -9,6 +9,9 @@ > */ > > #include <linux/gpio/driver.h> > + Apart from this blank line that I deem unnecessary.... Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
On 10/10/2022 16:14, Andy Shevchenko wrote: > Do not imply that some of the generic headers may be always included. > Instead, include explicitly what we are direct user of. > > While at it, sort headers alphabetically. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/samsung/pinctrl-samsung.c | 11 ++++++----- Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 10/10/2022 1:14 PM, Andy Shevchenko wrote: > Currently the header inclusion inside the pinctrl headers seems more arbitrary > than logical. This series is basically out of two parts: > - add missed headers to the pin control drivers / users > - clean up the headers of pin control subsystem > > The idea is to have this series to be pulled after -rc1 by the GPIO and > pin control subsystems, so all new drivers will utilize cleaned up headers > of the pin control. > > Please, review and comment. Did you really need to split this on a per-driver basis as opposed to just a treewide drivers/pinctrl, drivers/media and drivers/gpiolib patch set? 36 patches seems needlessly high when 4 patches could have achieve the same outcome.
On Tue, Oct 11, 2022 at 11:56 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > On 10/10/2022 1:14 PM, Andy Shevchenko wrote: > > Currently the header inclusion inside the pinctrl headers seems more arbitrary > > than logical. This series is basically out of two parts: > > - add missed headers to the pin control drivers / users > > - clean up the headers of pin control subsystem > > > > The idea is to have this series to be pulled after -rc1 by the GPIO and > > pin control subsystems, so all new drivers will utilize cleaned up headers > > of the pin control. > > > > Please, review and comment. > > Did you really need to split this on a per-driver basis as opposed to > just a treewide drivers/pinctrl, drivers/media and drivers/gpiolib patch > set? > > 36 patches seems needlessly high when 4 patches could have achieve the > same outcome. I can combine them if maintainers ask for that, nevertheless for Intel pin control and GPIO drivers, which I care more about, I would like to leave as separate changes (easy to see in history what was done).
On Wed, Oct 12, 2022 at 01:04:10PM +0300, Andy Shevchenko wrote: > On Tue, Oct 11, 2022 at 11:56 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > On 10/10/2022 1:14 PM, Andy Shevchenko wrote: > > > Currently the header inclusion inside the pinctrl headers seems more arbitrary > > > than logical. This series is basically out of two parts: > > > - add missed headers to the pin control drivers / users > > > - clean up the headers of pin control subsystem > > > > > > The idea is to have this series to be pulled after -rc1 by the GPIO and > > > pin control subsystems, so all new drivers will utilize cleaned up headers > > > of the pin control. > > > > > > Please, review and comment. > > > > Did you really need to split this on a per-driver basis as opposed to > > just a treewide drivers/pinctrl, drivers/media and drivers/gpiolib patch > > set? > > > > 36 patches seems needlessly high when 4 patches could have achieve the > > same outcome. > > I can combine them if maintainers ask for that, nevertheless for Intel > pin control and GPIO drivers, which I care more about, I would like to > leave as separate changes (easy to see in history what was done). I can now tell why I don't like to combine. While doing a revert (it's not related to GPIO nor to pin control), it appears that I reverted extra bits as merge conflict resolution. This is per se is not an issue, but when I tried to find and reapply that missed piece I can't, because the patch is combined and Git simply ignores to have `git cherry-pick _something in the past_` done. But again, up to maintainers.
On Mon, Oct 10, 2022 at 10:15 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Currently the header inclusion inside the pinctrl headers seems more arbitrary > than logical. This series is basically out of two parts: > - add missed headers to the pin control drivers / users > - clean up the headers of pin control subsystem > > The idea is to have this series to be pulled after -rc1 by the GPIO and > pin control subsystems, so all new drivers will utilize cleaned up headers > of the pin control. Aha I see you want to send a pull request so I backed out the applied patches from the series for now. Yours, Linus Walleij
On Mon, Oct 17, 2022 at 11:02:09AM +0200, Linus Walleij wrote: > On Mon, Oct 10, 2022 at 10:15 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > Currently the header inclusion inside the pinctrl headers seems more arbitrary > > than logical. This series is basically out of two parts: > > - add missed headers to the pin control drivers / users > > - clean up the headers of pin control subsystem > > > > The idea is to have this series to be pulled after -rc1 by the GPIO and > > pin control subsystems, so all new drivers will utilize cleaned up headers > > of the pin control. > > Aha I see you want to send a pull request so I backed out the applied patches > from the series for now. Can I consider all that you answered to as Rb tag?
On Mon, Oct 17, 2022 at 11:27 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Mon, Oct 17, 2022 at 11:02:09AM +0200, Linus Walleij wrote: > > On Mon, Oct 10, 2022 at 10:15 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > Currently the header inclusion inside the pinctrl headers seems more arbitrary > > > than logical. This series is basically out of two parts: > > > - add missed headers to the pin control drivers / users > > > - clean up the headers of pin control subsystem > > > > > > The idea is to have this series to be pulled after -rc1 by the GPIO and > > > pin control subsystems, so all new drivers will utilize cleaned up headers > > > of the pin control. > > > > Aha I see you want to send a pull request so I backed out the applied patches > > from the series for now. > > Can I consider all that you answered to as Rb tag? Acked-by: Linus Walleij <linus.walleij@linaro.org> I haven't reviewed in detail but I fully trust you to do the right thing and fix any fallout so will happily pull this. Yours, Linus Walleij
On Mon, Oct 17, 2022 at 11:58:03AM +0200, Linus Walleij wrote: > On Mon, Oct 17, 2022 at 11:27 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Oct 17, 2022 at 11:02:09AM +0200, Linus Walleij wrote: > > > On Mon, Oct 10, 2022 at 10:15 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > Currently the header inclusion inside the pinctrl headers seems more arbitrary > > > > than logical. This series is basically out of two parts: > > > > - add missed headers to the pin control drivers / users > > > > - clean up the headers of pin control subsystem > > > > > > > > The idea is to have this series to be pulled after -rc1 by the GPIO and > > > > pin control subsystems, so all new drivers will utilize cleaned up headers > > > > of the pin control. > > > > > > Aha I see you want to send a pull request so I backed out the applied patches > > > from the series for now. > > > > Can I consider all that you answered to as Rb tag? > > Acked-by: Linus Walleij <linus.walleij@linaro.org> Thank you! > I haven't reviewed in detail but I fully trust you to do the right thing > and fix any fallout so will happily pull this. The plan is to push this to Linux Next for a couple of days and then I'll send PR to you and Bart.