| 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 10/10/22 22: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/pinctrl-st.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c > index cf7f9cbe6044..985dfceb127d 100644 > --- a/drivers/pinctrl/pinctrl-st.c > +++ b/drivers/pinctrl/pinctrl-st.c > @@ -5,21 +5,26 @@ > * Srinivas Kandagatla <srinivas.kandagatla@st.com> > */ > > -#include <linux/init.h> > -#include <linux/module.h> > -#include <linux/slab.h> > #include <linux/err.h> > +#include <linux/gpio/driver.h> > +#include <linux/init.h> > #include <linux/io.h> > +#include <linux/mfd/syscon.h> > +#include <linux/module.h> > #include <linux/of.h> > -#include <linux/of_irq.h> > #include <linux/of_address.h> > -#include <linux/gpio/driver.h> > +#include <linux/of_irq.h> > +#include <linux/platform_device.h> > #include <linux/regmap.h> > -#include <linux/mfd/syscon.h> > +#include <linux/seq_file.h> > +#include <linux/slab.h> > +#include <linux/string_helpers.h> > + > +#include <linux/pinctrl/consumer.h> > +#include <linux/pinctrl/pinconf.h> > #include <linux/pinctrl/pinctrl.h> > #include <linux/pinctrl/pinmux.h> > -#include <linux/pinctrl/pinconf.h> > -#include <linux/platform_device.h> > + > #include "core.h" > > /* PIO Block registers */ Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com> Thanks Patrice
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 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. In any case Acked-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > index 5b544fb7f3d8..6a8a9cfe8965 100644 > --- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c > @@ -15,8 +15,10 @@ > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/reset.h> > +#include <linux/seq_file.h> > #include <linux/spinlock.h> > > +#include <linux/pinctrl/consumer.h> > #include <linux/pinctrl/pinctrl.h> > #include <linux/pinctrl/pinmux.h> > > -- > 2.35.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
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 Mon, Oct 10, 2022 at 11:14:29PM +0300, 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> > --- Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com> Thanks, Charles
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!
The 10/10/2022 23: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> > --- Acked-by: Horatiu Vultur <horatiu.vultur@microchip.com> > drivers/pinctrl/pinctrl-ocelot.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c > index 647e91490bac..a9343c242cd5 100644 > --- a/drivers/pinctrl/pinctrl-ocelot.c > +++ b/drivers/pinctrl/pinctrl-ocelot.c > @@ -13,15 +13,17 @@ > #include <linux/of_device.h> > #include <linux/of_irq.h> > #include <linux/of_platform.h> > -#include <linux/pinctrl/pinctrl.h> > -#include <linux/pinctrl/pinmux.h> > -#include <linux/pinctrl/pinconf.h> > -#include <linux/pinctrl/pinconf-generic.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > #include <linux/reset.h> > #include <linux/slab.h> > > +#include <linux/pinctrl/consumer.h> > +#include <linux/pinctrl/pinconf-generic.h> > +#include <linux/pinctrl/pinconf.h> > +#include <linux/pinctrl/pinctrl.h> > +#include <linux/pinctrl/pinmux.h> > + > #include "core.h" > #include "pinconf.h" > #include "pinmux.h" > -- > 2.35.1 >
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?