[PATCH 0/2] tty: gdm724x: Fix coding style in gdm_tty.c

[PATCH 0/2] tty: gdm724x: Fix coding style in gdm_tty.c

[Soumya Negi]

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

[PATCH 1/2] tty: gdm724x: Match alignment with open parenthesis

[Soumya Negi]

Fix CHECK: Alignment should match open parenthesis
Issue found by checkpatch.pl

Signed-off-by: Soumya Negi <soumya.negi97@gmail.com>
---
 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

[PATCH 2/2] tty: gdm724x: Add blank line after declaration

[Soumya Negi]

Fix WARNING: Missing a blank line after declarations
Issue found by checkpatch.pl

Signed-off-by: Soumya Negi <soumya.negi97@gmail.com>
---
 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

Re: [PATCH 1/2] tty: gdm724x: Match alignment with open parenthesis

[Dan Carpenter]

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 <soumya.negi97@gmail.com>
> ---
>  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

Re: [PATCH 1/2] tty: gdm724x: Match alignment with open parenthesis

[Soumya Negi]

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 <soumya.negi97@gmail.com>
> > ---
> >  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

Re: [PATCH 1/2] tty: gdm724x: Match alignment with open parenthesis

[Dan Carpenter]

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 <soumya.negi97@gmail.com>
> > > ---
> > >  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

Re: [PATCH 2/2] tty: gdm724x: Add blank line after declaration

[Greg Kroah-Hartman]

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 <soumya.negi97@gmail.com>
> ---
>  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

Re: [PATCH 2/2] tty: gdm724x: Add blank line after declaration

[Soumya Negi]

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 <soumya.negi97@gmail.com>
> > ---
> >  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

Re: [PATCH 2/2] tty: gdm724x: Add blank line after declaration

[Greg Kroah-Hartman]

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 <soumya.negi97@gmail.com>
> > > ---
> > >  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

Re: [PATCH 2/2] tty: gdm724x: Add blank line after declaration

[Soumya Negi]

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 <soumya.negi97@gmail.com>
> > > > ---
> > > >  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

Re: [PATCH 1/2] tty: gdm724x: Match alignment with open parenthesis

[Soumya Negi]

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 <soumya.negi97@gmail.com>
> > > > ---
> > > >  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