* [PATCH] rpmsg: char: Remove useless includes @ 2021-04-29 8:06 Arnaud Pouliquen 2021-05-03 17:42 ` Mathieu Poirier 0 siblings, 1 reply; 6+ messages in thread From: Arnaud Pouliquen @ 2021-04-29 8:06 UTC (permalink / raw) To: Bjorn Andersson, Ohad Ben-Cohen, Mathieu Poirier Cc: linux-remoteproc, linux-kernel, linux-stm32, arnaud.pouliquen Remove includes that are not requested to build the module. Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> --- applied without issue on Bjorn next branch (dc0e14fa833b) --- drivers/rpmsg/rpmsg_char.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c index 2bebc9b2d163..e4e54f515af6 100644 --- a/drivers/rpmsg/rpmsg_char.c +++ b/drivers/rpmsg/rpmsg_char.c @@ -10,19 +10,10 @@ * was based on TI & Google OMX rpmsg driver. */ #include <linux/cdev.h> -#include <linux/device.h> -#include <linux/fs.h> -#include <linux/idr.h> #include <linux/kernel.h> #include <linux/module.h> -#include <linux/poll.h> #include <linux/rpmsg.h> #include <linux/skbuff.h> -#include <linux/slab.h> -#include <linux/uaccess.h> -#include <uapi/linux/rpmsg.h> - -#include "rpmsg_internal.h" #define RPMSG_DEV_MAX (MINORMASK + 1) -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] rpmsg: char: Remove useless includes 2021-04-29 8:06 [PATCH] rpmsg: char: Remove useless includes Arnaud Pouliquen @ 2021-05-03 17:42 ` Mathieu Poirier 2021-05-04 7:16 ` Arnaud POULIQUEN 0 siblings, 1 reply; 6+ messages in thread From: Mathieu Poirier @ 2021-05-03 17:42 UTC (permalink / raw) To: Arnaud Pouliquen Cc: Bjorn Andersson, Ohad Ben-Cohen, linux-remoteproc, linux-kernel, linux-stm32 On Thu, Apr 29, 2021 at 10:06:39AM +0200, Arnaud Pouliquen wrote: > Remove includes that are not requested to build the module. > > Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> > --- > applied without issue on Bjorn next branch (dc0e14fa833b) > --- > drivers/rpmsg/rpmsg_char.c | 9 --------- > 1 file changed, 9 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c > index 2bebc9b2d163..e4e54f515af6 100644 > --- a/drivers/rpmsg/rpmsg_char.c > +++ b/drivers/rpmsg/rpmsg_char.c > @@ -10,19 +10,10 @@ > * was based on TI & Google OMX rpmsg driver. > */ > #include <linux/cdev.h> > -#include <linux/device.h> This is where the declaration for struct device is along with other goodies like get/put_device(). > -#include <linux/fs.h> That is where struct file is declared. > -#include <linux/idr.h> This is where you get ida_simple_get() and ida_simple_remove() from. > #include <linux/kernel.h> > #include <linux/module.h> > -#include <linux/poll.h> This is where struct poll_table and poll_wait() comes from. > #include <linux/rpmsg.h> > #include <linux/skbuff.h> > -#include <linux/slab.h> This gives you kzalloc() and kfree(). > -#include <linux/uaccess.h> This gives you copy_from_user(). > -#include <uapi/linux/rpmsg.h> This gives you RPMSG_CREATE_EPT_IOCTL and RPMSG_DESTROY_EPT_IOCTL. > - > -#include "rpmsg_internal.h" That one I agree with. Thanks, Mathieu > > #define RPMSG_DEV_MAX (MINORMASK + 1) > > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rpmsg: char: Remove useless includes 2021-05-03 17:42 ` Mathieu Poirier @ 2021-05-04 7:16 ` Arnaud POULIQUEN 2021-05-04 17:05 ` Mathieu Poirier 0 siblings, 1 reply; 6+ messages in thread From: Arnaud POULIQUEN @ 2021-05-04 7:16 UTC (permalink / raw) To: Mathieu Poirier Cc: Bjorn Andersson, Ohad Ben-Cohen, linux-remoteproc, linux-kernel, linux-stm32 Hello Mathieu, On 5/3/21 7:42 PM, Mathieu Poirier wrote: > On Thu, Apr 29, 2021 at 10:06:39AM +0200, Arnaud Pouliquen wrote: >> Remove includes that are not requested to build the module. >> >> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com> >> --- >> applied without issue on Bjorn next branch (dc0e14fa833b) >> --- >> drivers/rpmsg/rpmsg_char.c | 9 --------- >> 1 file changed, 9 deletions(-) >> >> diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c >> index 2bebc9b2d163..e4e54f515af6 100644 >> --- a/drivers/rpmsg/rpmsg_char.c >> +++ b/drivers/rpmsg/rpmsg_char.c >> @@ -10,19 +10,10 @@ >> * was based on TI & Google OMX rpmsg driver. >> */ >> #include <linux/cdev.h> >> -#include <linux/device.h> > > This is where the declaration for struct device is along with other goodies like > get/put_device(). > >> -#include <linux/fs.h> > > That is where struct file is declared. > >> -#include <linux/idr.h> > > This is where you get ida_simple_get() and ida_simple_remove() from. > >> #include <linux/kernel.h> >> #include <linux/module.h> >> -#include <linux/poll.h> > > This is where struct poll_table and poll_wait() comes from. > >> #include <linux/rpmsg.h> >> #include <linux/skbuff.h> >> -#include <linux/slab.h> > > This gives you kzalloc() and kfree(). > >> -#include <linux/uaccess.h> > > This gives you copy_from_user(). > >> -#include <uapi/linux/rpmsg.h> > > This gives you RPMSG_CREATE_EPT_IOCTL and RPMSG_DESTROY_EPT_IOCTL. > >> - >> -#include "rpmsg_internal.h" > > That one I agree with. I started by this one and then I got carried away tested the other include... You are right, I just don't follow her the first rule of the "submit checklist" "If you use a facility then #include the file that defines/declares that facility. Don’t depend on other header files pulling in ones that you use." That said I just have a doubt for uapi/linux/rpmsg.h that will be include by rpmsg.h[2], as these includes are part of the rpmsg framework API, should we keep both, considering the rule as strict? [1] https://www.kernel.org/doc/html/latest/process/submit-checklist.html [2] https://patchwork.kernel.org/project/linux-remoteproc/patch/20210311140413.31725-3-arnaud.pouliquen@foss.st.com/ Thanks, Arnaud > > Thanks, > Mathieu > >> >> #define RPMSG_DEV_MAX (MINORMASK + 1) >> >> -- >> 2.17.1 >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rpmsg: char: Remove useless includes 2021-05-04 7:16 ` Arnaud POULIQUEN @ 2021-05-04 17:05 ` Mathieu Poirier 2021-05-04 18:20 ` Arnaud POULIQUEN 0 siblings, 1 reply; 6+ messages in thread From: Mathieu Poirier @ 2021-05-04 17:05 UTC (permalink / raw) To: Arnaud POULIQUEN Cc: Bjorn Andersson, Ohad Ben-Cohen, linux-remoteproc, linux-kernel, linux-stm32 Hi Arnaud, [...] > > I started by this one and then I got carried away tested the other include... > You are right, I just don't follow her the first rule of the "submit checklist" > > "If you use a facility then #include the file that defines/declares that > facility. Don’t depend on other header files pulling in ones that you use." > > That said I just have a doubt for uapi/linux/rpmsg.h that will be include > by rpmsg.h[2], as these includes are part of the rpmsg framework API, should we > keep both, considering the rule as strict? I red the last paragraph several times I can't understand what you are trying to convey. Please rephrase, provide more context or detail exactly where you think we have a problem. Thanks, Mathieu > > [1] https://www.kernel.org/doc/html/latest/process/submit-checklist.html > [2] > https://patchwork.kernel.org/project/linux-remoteproc/patch/20210311140413.31725-3-arnaud.pouliquen@foss.st.com/ > > Thanks, > Arnaud > > > > > Thanks, > > Mathieu > > > >> > >> #define RPMSG_DEV_MAX (MINORMASK + 1) > >> > >> -- > >> 2.17.1 > >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rpmsg: char: Remove useless includes 2021-05-04 17:05 ` Mathieu Poirier @ 2021-05-04 18:20 ` Arnaud POULIQUEN 2021-05-05 17:08 ` Mathieu Poirier 0 siblings, 1 reply; 6+ messages in thread From: Arnaud POULIQUEN @ 2021-05-04 18:20 UTC (permalink / raw) To: Mathieu Poirier Cc: Bjorn Andersson, Ohad Ben-Cohen, linux-remoteproc, linux-kernel, linux-stm32 On 5/4/21 7:05 PM, Mathieu Poirier wrote: > Hi Arnaud, > > [...] > >> >> I started by this one and then I got carried away tested the other include... >> You are right, I just don't follow her the first rule of the "submit checklist" >> >> "If you use a facility then #include the file that defines/declares that >> facility. Don’t depend on other header files pulling in ones that you use." >> >> That said I just have a doubt for uapi/linux/rpmsg.h that will be include >> by rpmsg.h[2], as these includes are part of the rpmsg framework API, should we >> keep both, considering the rule as strict? > > I red the last paragraph several times I can't understand what you are > trying to convey. Please rephrase, provide more context or detail exactly where > you think we have a problem. There is no problem, just a question before sending an update. As you mention the #include "rpmsg_internal.h" line can be removed, I plan to send a patch V2 for this. That's said before sending a new version I would like to propose to also remove the #include <uapi/linux/rpmsg.h> line. The rational to remove it is that include/rpmsg.h would already include <uapi/linux/rpmsg.h> in 5.13 [2]. And looking at some frameworks (e.g I2C, TTY) the drivers seem to include only the include/xxx.h and not the uapi/linux/xxx.h in such case. So my question is should I remove #include <uapi/linux/rpmsg.h> line? Or do you prefer that i keep it? Hope it is more clear... else please just forget my proposal, I wouldn't want you to waste too much time for a point of detail. Thanks, Arnaud > > Thanks, > Mathieu > > >> >> [1] https://www.kernel.org/doc/html/latest/process/submit-checklist.html >> [2] >> https://patchwork.kernel.org/project/linux-remoteproc/patch/20210311140413.31725-3-arnaud.pouliquen@foss.st.com/ >> >> Thanks, >> Arnaud >> >>> >>> Thanks, >>> Mathieu >>> >>>> >>>> #define RPMSG_DEV_MAX (MINORMASK + 1) >>>> >>>> -- >>>> 2.17.1 >>>> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rpmsg: char: Remove useless includes 2021-05-04 18:20 ` Arnaud POULIQUEN @ 2021-05-05 17:08 ` Mathieu Poirier 0 siblings, 0 replies; 6+ messages in thread From: Mathieu Poirier @ 2021-05-05 17:08 UTC (permalink / raw) To: Arnaud POULIQUEN Cc: Bjorn Andersson, Ohad Ben-Cohen, linux-remoteproc, linux-kernel, linux-stm32 On Tue, May 04, 2021 at 08:20:25PM +0200, Arnaud POULIQUEN wrote: > > > On 5/4/21 7:05 PM, Mathieu Poirier wrote: > > Hi Arnaud, > > > > [...] > > > >> > >> I started by this one and then I got carried away tested the other include... > >> You are right, I just don't follow her the first rule of the "submit checklist" > >> > >> "If you use a facility then #include the file that defines/declares that > >> facility. Don’t depend on other header files pulling in ones that you use." > >> > >> That said I just have a doubt for uapi/linux/rpmsg.h that will be include > >> by rpmsg.h[2], as these includes are part of the rpmsg framework API, should we > >> keep both, considering the rule as strict? > > > > I red the last paragraph several times I can't understand what you are > > trying to convey. Please rephrase, provide more context or detail exactly where > > you think we have a problem. > > There is no problem, just a question before sending an update. > > As you mention the #include "rpmsg_internal.h" line can be removed, I plan to > send a patch V2 for this. > > That's said before sending a new version I would like to propose to also remove > the #include <uapi/linux/rpmsg.h> line. > > The rational to remove it is that include/rpmsg.h would already include > <uapi/linux/rpmsg.h> in 5.13 [2]. And looking at some frameworks (e.g I2C, TTY) > the drivers seem to include only the include/xxx.h and not the uapi/linux/xxx.h > in such case. > > So my question is should I remove #include <uapi/linux/rpmsg.h> line? Or do > you prefer that i keep it? Thanks for the clarifications, this is much much better. Less changes is always preferred, so unless there is a clear guideline or a good reason to make a change I would prefer to keep things the way they are. > > Hope it is more clear... else please just forget my proposal, I wouldn't want > you to waste too much time for a point of detail. > > Thanks, > Arnaud > > > > > Thanks, > > Mathieu > > > > > >> > >> [1] https://www.kernel.org/doc/html/latest/process/submit-checklist.html > >> [2] > >> https://patchwork.kernel.org/project/linux-remoteproc/patch/20210311140413.31725-3-arnaud.pouliquen@foss.st.com/ > >> > >> Thanks, > >> Arnaud > >> > >>> > >>> Thanks, > >>> Mathieu > >>> > >>>> > >>>> #define RPMSG_DEV_MAX (MINORMASK + 1) > >>>> > >>>> -- > >>>> 2.17.1 > >>>> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-05 17:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-29 8:06 [PATCH] rpmsg: char: Remove useless includes Arnaud Pouliquen 2021-05-03 17:42 ` Mathieu Poirier 2021-05-04 7:16 ` Arnaud POULIQUEN 2021-05-04 17:05 ` Mathieu Poirier 2021-05-04 18:20 ` Arnaud POULIQUEN 2021-05-05 17:08 ` Mathieu Poirier
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).