Checkpatch.pl raises 2 warnings in gdm_tty.c. This patchset contains
two patches that cleanup the code as per the Linux coding style & fix
the warnings. Patches can be applied in any order.
Soumya Negi (2):
tty: gdm724x: Match alignment with open parenthesis
tty: gdm724x: Add blank line after declaration
drivers/staging/gdm724x/gdm_tty.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
--
2.42.0
Fix CHECK: Alignment should match open parenthesis
Issue found by checkpatch.pl
Signed-off-by: Soumya Negi <[email protected]>
---
drivers/staging/gdm724x/gdm_tty.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
index 32b2e817ff04..4e5cac76db58 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -271,8 +271,8 @@ int register_lte_tty_driver(void)
int ret;
for (i = 0; i < TTY_MAX_COUNT; i++) {
- tty_driver = tty_alloc_driver(GDM_TTY_MINOR,
- TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
+ tty_driver = tty_alloc_driver(GDM_TTY_MINOR, TTY_DRIVER_REAL_RAW |
+ TTY_DRIVER_DYNAMIC_DEV);
if (IS_ERR(tty_driver))
return PTR_ERR(tty_driver);
--
2.42.0
Fix WARNING: Missing a blank line after declarations
Issue found by checkpatch.pl
Signed-off-by: Soumya Negi <[email protected]>
---
drivers/staging/gdm724x/gdm_tty.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
index 4e5cac76db58..0c61eaff6122 100644
--- a/drivers/staging/gdm724x/gdm_tty.c
+++ b/drivers/staging/gdm724x/gdm_tty.c
@@ -160,6 +160,7 @@ static ssize_t gdm_tty_write(struct tty_struct *tty, const u8 *buf, size_t len)
while (remain) {
size_t sending_len = min_t(size_t, MUX_TX_MAX_SIZE, remain);
+
gdm->tty_dev->send_func(gdm->tty_dev->priv_dev,
(void *)(buf + sent_len),
sending_len,
--
2.42.0
On Fri, Oct 13, 2023 at 01:26:34AM -0700, Soumya Negi wrote:
> Fix CHECK: Alignment should match open parenthesis
> Issue found by checkpatch.pl
>
> Signed-off-by: Soumya Negi <[email protected]>
> ---
> drivers/staging/gdm724x/gdm_tty.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
> index 32b2e817ff04..4e5cac76db58 100644
> --- a/drivers/staging/gdm724x/gdm_tty.c
> +++ b/drivers/staging/gdm724x/gdm_tty.c
> @@ -271,8 +271,8 @@ int register_lte_tty_driver(void)
> int ret;
>
> for (i = 0; i < TTY_MAX_COUNT; i++) {
> - tty_driver = tty_alloc_driver(GDM_TTY_MINOR,
> - TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
> + tty_driver = tty_alloc_driver(GDM_TTY_MINOR, TTY_DRIVER_REAL_RAW |
> + TTY_DRIVER_DYNAMIC_DEV);
I prefered the original code. It was more readable.
regards,
dan carpenter
On Fri, Oct 13, 2023 at 11:57:40AM +0300, Dan Carpenter wrote:
> On Fri, Oct 13, 2023 at 01:26:34AM -0700, Soumya Negi wrote:
> > Fix CHECK: Alignment should match open parenthesis
> > Issue found by checkpatch.pl
> >
> > Signed-off-by: Soumya Negi <[email protected]>
> > ---
> > drivers/staging/gdm724x/gdm_tty.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
> > index 32b2e817ff04..4e5cac76db58 100644
> > --- a/drivers/staging/gdm724x/gdm_tty.c
> > +++ b/drivers/staging/gdm724x/gdm_tty.c
> > @@ -271,8 +271,8 @@ int register_lte_tty_driver(void)
> > int ret;
> >
> > for (i = 0; i < TTY_MAX_COUNT; i++) {
> > - tty_driver = tty_alloc_driver(GDM_TTY_MINOR,
> > - TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
> > + tty_driver = tty_alloc_driver(GDM_TTY_MINOR, TTY_DRIVER_REAL_RAW |
> > + TTY_DRIVER_DYNAMIC_DEV);
>
> I prefered the original code. It was more readable.
>
> regards,
> dan carpenter
Hi Dan,
Noted. I'm curious what happens when some of the patches in a patchset
are acceptable and some are not. Is everything disregarded by
maintainers or are the good patches cherry-picked from the set?
Thanks,
Soumya
On Fri, Oct 13, 2023 at 02:13:26PM -0700, Soumya Negi wrote:
> On Fri, Oct 13, 2023 at 11:57:40AM +0300, Dan Carpenter wrote:
> > On Fri, Oct 13, 2023 at 01:26:34AM -0700, Soumya Negi wrote:
> > > Fix CHECK: Alignment should match open parenthesis
> > > Issue found by checkpatch.pl
> > >
> > > Signed-off-by: Soumya Negi <[email protected]>
> > > ---
> > > drivers/staging/gdm724x/gdm_tty.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
> > > index 32b2e817ff04..4e5cac76db58 100644
> > > --- a/drivers/staging/gdm724x/gdm_tty.c
> > > +++ b/drivers/staging/gdm724x/gdm_tty.c
> > > @@ -271,8 +271,8 @@ int register_lte_tty_driver(void)
> > > int ret;
> > >
> > > for (i = 0; i < TTY_MAX_COUNT; i++) {
> > > - tty_driver = tty_alloc_driver(GDM_TTY_MINOR,
> > > - TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
> > > + tty_driver = tty_alloc_driver(GDM_TTY_MINOR, TTY_DRIVER_REAL_RAW |
> > > + TTY_DRIVER_DYNAMIC_DEV);
> >
> > I prefered the original code. It was more readable.
> >
> > regards,
> > dan carpenter
> Hi Dan,
>
> Noted. I'm curious what happens when some of the patches in a patchset
> are acceptable and some are not. Is everything disregarded by
> maintainers or are the good patches cherry-picked from the set?
Most of the time, you should just resend the series. Sometimes a
maintainer will take the first few patches and then when they hit one
that can't be merged they'll stop. So you should try to organize your
patchsets from fixes first, then cleanups and then least controversial
to most controversial. Except people might be annoyed if it looks like
you're hiding a really controversial one at the end of a long series.
regards,
dan carpenter
On Fri, Oct 13, 2023 at 01:26:35AM -0700, Soumya Negi wrote:
> Fix WARNING: Missing a blank line after declarations
> Issue found by checkpatch.pl
>
> Signed-off-by: Soumya Negi <[email protected]>
> ---
> drivers/staging/gdm724x/gdm_tty.c | 1 +
> 1 file changed, 1 insertion(+)
Why do you have "tty:" as the prefix for a staging driver? Shouldn't it
be "staging: gdm724x: ...."?
thanks,
greg k-h
On Sun, Oct 15, 2023 at 07:37:30AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 13, 2023 at 01:26:35AM -0700, Soumya Negi wrote:
> > Fix WARNING: Missing a blank line after declarations
> > Issue found by checkpatch.pl
> >
> > Signed-off-by: Soumya Negi <[email protected]>
> > ---
> > drivers/staging/gdm724x/gdm_tty.c | 1 +
> > 1 file changed, 1 insertion(+)
>
> Why do you have "tty:" as the prefix for a staging driver? Shouldn't it
> be "staging: gdm724x: ...."?
Hi Greg,
Thats what I thought too. But when I looked at the git history for
gdm_tty.c the last few commits had "tty:". So I went with that.
Should I change it to "staging:"?
Regards,
Soumya
> thanks,
>
> greg k-h
On Sat, Oct 14, 2023 at 11:07:16PM -0700, Soumya Negi wrote:
> On Sun, Oct 15, 2023 at 07:37:30AM +0200, Greg Kroah-Hartman wrote:
> > On Fri, Oct 13, 2023 at 01:26:35AM -0700, Soumya Negi wrote:
> > > Fix WARNING: Missing a blank line after declarations
> > > Issue found by checkpatch.pl
> > >
> > > Signed-off-by: Soumya Negi <[email protected]>
> > > ---
> > > drivers/staging/gdm724x/gdm_tty.c | 1 +
> > > 1 file changed, 1 insertion(+)
> >
> > Why do you have "tty:" as the prefix for a staging driver? Shouldn't it
> > be "staging: gdm724x: ...."?
>
> Hi Greg,
>
> Thats what I thought too. But when I looked at the git history for
> gdm_tty.c the last few commits had "tty:". So I went with that.
That is because those commits were tty-wide, and they changed things in
multiple drivers all at once, not just for the one file.
> Should I change it to "staging:"?
Please do.
thanks,
greg k-h
Hi Greg,
On Sun, Oct 15, 2023 at 02:50:44PM +0200, Greg Kroah-Hartman wrote:
> On Sat, Oct 14, 2023 at 11:07:16PM -0700, Soumya Negi wrote:
> > On Sun, Oct 15, 2023 at 07:37:30AM +0200, Greg Kroah-Hartman wrote:
> > > On Fri, Oct 13, 2023 at 01:26:35AM -0700, Soumya Negi wrote:
> > > > Fix WARNING: Missing a blank line after declarations
> > > > Issue found by checkpatch.pl
> > > >
> > > > Signed-off-by: Soumya Negi <[email protected]>
> > > > ---
> > > > drivers/staging/gdm724x/gdm_tty.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > >
> > > Why do you have "tty:" as the prefix for a staging driver? Shouldn't it
> > > be "staging: gdm724x: ...."?
> >
> > Hi Greg,
> >
> > Thats what I thought too. But when I looked at the git history for
> > gdm_tty.c the last few commits had "tty:". So I went with that.
>
> That is because those commits were tty-wide, and they changed things in
> multiple drivers all at once, not just for the one file.
>
> > Should I change it to "staging:"?
>
> Please do.
I'm sending this patch in a new thread since the other patch in the
patchset(had 2 patches) was reviewed as unneeded.
Regards,
Soumya
On Sat, Oct 14, 2023 at 10:38:22AM +0300, Dan Carpenter wrote:
> On Fri, Oct 13, 2023 at 02:13:26PM -0700, Soumya Negi wrote:
> > On Fri, Oct 13, 2023 at 11:57:40AM +0300, Dan Carpenter wrote:
> > > On Fri, Oct 13, 2023 at 01:26:34AM -0700, Soumya Negi wrote:
> > > > Fix CHECK: Alignment should match open parenthesis
> > > > Issue found by checkpatch.pl
> > > >
> > > > Signed-off-by: Soumya Negi <[email protected]>
> > > > ---
> > > > drivers/staging/gdm724x/gdm_tty.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c
> > > > index 32b2e817ff04..4e5cac76db58 100644
> > > > --- a/drivers/staging/gdm724x/gdm_tty.c
> > > > +++ b/drivers/staging/gdm724x/gdm_tty.c
> > > > @@ -271,8 +271,8 @@ int register_lte_tty_driver(void)
> > > > int ret;
> > > >
> > > > for (i = 0; i < TTY_MAX_COUNT; i++) {
> > > > - tty_driver = tty_alloc_driver(GDM_TTY_MINOR,
> > > > - TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
> > > > + tty_driver = tty_alloc_driver(GDM_TTY_MINOR, TTY_DRIVER_REAL_RAW |
> > > > + TTY_DRIVER_DYNAMIC_DEV);
> > >
> > > I prefered the original code. It was more readable.
> > >
> > > regards,
> > > dan carpenter
> > Hi Dan,
> >
> > Noted. I'm curious what happens when some of the patches in a patchset
> > are acceptable and some are not. Is everything disregarded by
> > maintainers or are the good patches cherry-picked from the set?
>
> Most of the time, you should just resend the series. Sometimes a
> maintainer will take the first few patches and then when they hit one
> that can't be merged they'll stop. So you should try to organize your
> patchsets from fixes first, then cleanups and then least controversial
> to most controversial. Except people might be annoyed if it looks like
> you're hiding a really controversial one at the end of a long series.
>
> regards,
> dan carpenter
Thank you!
- Soumya