2016-03-14 04:29:28

by Daeseok Youn

[permalink] [raw]
Subject: [PATCH 3/3] staging: dgnc: use tty_alloc_driver instead of kcalloc

the tty_alloc_driver() can allocate memory for ttys and termios.
And also it can release allocated memory easly with using
put_tty_driver().

Signed-off-by: Daeseok Youn <[email protected]>
---
drivers/staging/dgnc/dgnc_tty.c | 86 +++++++++++++++--------------------------
1 file changed, 31 insertions(+), 55 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_tty.c b/drivers/staging/dgnc/dgnc_tty.c
index 4e4efa2..118936b 100644
--- a/drivers/staging/dgnc/dgnc_tty.c
+++ b/drivers/staging/dgnc/dgnc_tty.c
@@ -176,9 +176,15 @@ int dgnc_tty_preinit(void)
*/
int dgnc_tty_register(struct dgnc_board *brd)
{
- int rc = 0;
+ int rc;
+
+ brd->serial_driver = tty_alloc_driver(brd->maxports,
+ TTY_DRIVER_REAL_RAW |
+ TTY_DRIVER_DYNAMIC_DEV |
+ TTY_DRIVER_HARDWARE_BREAK);

- brd->serial_driver->magic = TTY_DRIVER_MAGIC;
+ if (IS_ERR(brd->serial_driver))
+ return PTR_ERR(brd->serial_driver);

snprintf(brd->SerialName, MAXTTYNAMELEN, "tty_dgnc_%d_", brd->boardnum);

@@ -186,31 +192,10 @@ int dgnc_tty_register(struct dgnc_board *brd)
brd->serial_driver->name_base = 0;
brd->serial_driver->major = 0;
brd->serial_driver->minor_start = 0;
- brd->serial_driver->num = brd->maxports;
brd->serial_driver->type = TTY_DRIVER_TYPE_SERIAL;
brd->serial_driver->subtype = SERIAL_TYPE_NORMAL;
brd->serial_driver->init_termios = DgncDefaultTermios;
brd->serial_driver->driver_name = DRVSTR;
- brd->serial_driver->flags = (TTY_DRIVER_REAL_RAW |
- TTY_DRIVER_DYNAMIC_DEV |
- TTY_DRIVER_HARDWARE_BREAK);
-
- /*
- * The kernel wants space to store pointers to
- * tty_struct's and termios's.
- */
- brd->serial_driver->ttys = kcalloc(brd->maxports,
- sizeof(*brd->serial_driver->ttys),
- GFP_KERNEL);
- if (!brd->serial_driver->ttys)
- return -ENOMEM;
-
- kref_init(&brd->serial_driver->kref);
- brd->serial_driver->termios = kcalloc(brd->maxports,
- sizeof(*brd->serial_driver->termios),
- GFP_KERNEL);
- if (!brd->serial_driver->termios)
- return -ENOMEM;

/*
* Entry points for driver. Called by the kernel from
@@ -224,7 +209,7 @@ int dgnc_tty_register(struct dgnc_board *brd)
if (rc < 0) {
dev_dbg(&brd->pdev->dev,
"Can't register tty device (%d)\n", rc);
- return rc;
+ goto free_serial_driver;
}
brd->dgnc_Major_Serial_Registered = true;
}
@@ -234,38 +219,26 @@ int dgnc_tty_register(struct dgnc_board *brd)
* again, separately so we don't get the LD confused about what major
* we are when we get into the dgnc_tty_open() routine.
*/
- brd->print_driver->magic = TTY_DRIVER_MAGIC;
+ brd->print_driver = tty_alloc_driver(brd->maxports,
+ TTY_DRIVER_REAL_RAW |
+ TTY_DRIVER_DYNAMIC_DEV |
+ TTY_DRIVER_HARDWARE_BREAK);
+
+ if (IS_ERR(brd->print_driver)) {
+ rc = PTR_ERR(brd->print_driver);
+ goto unregister_serial_driver;
+ }
+
snprintf(brd->PrintName, MAXTTYNAMELEN, "pr_dgnc_%d_", brd->boardnum);

brd->print_driver->name = brd->PrintName;
brd->print_driver->name_base = 0;
brd->print_driver->major = brd->serial_driver->major;
brd->print_driver->minor_start = 0x80;
- brd->print_driver->num = brd->maxports;
brd->print_driver->type = TTY_DRIVER_TYPE_SERIAL;
brd->print_driver->subtype = SERIAL_TYPE_NORMAL;
brd->print_driver->init_termios = DgncDefaultTermios;
brd->print_driver->driver_name = DRVSTR;
- brd->print_driver->flags = (TTY_DRIVER_REAL_RAW |
- TTY_DRIVER_DYNAMIC_DEV |
- TTY_DRIVER_HARDWARE_BREAK);
-
- /*
- * The kernel wants space to store pointers to
- * tty_struct's and termios's. Must be separated from
- * the Serial Driver so we don't get confused
- */
- brd->print_driver->ttys = kcalloc(brd->maxports,
- sizeof(*brd->print_driver->ttys),
- GFP_KERNEL);
- if (!brd->print_driver->ttys)
- return -ENOMEM;
- kref_init(&brd->print_driver->kref);
- brd->print_driver->termios = kcalloc(brd->maxports,
- sizeof(*brd->print_driver->termios),
- GFP_KERNEL);
- if (!brd->print_driver->termios)
- return -ENOMEM;

/*
* Entry points for driver. Called by the kernel from
@@ -280,7 +253,7 @@ int dgnc_tty_register(struct dgnc_board *brd)
dev_dbg(&brd->pdev->dev,
"Can't register Transparent Print device(%d)\n",
rc);
- return rc;
+ goto free_print_driver;
}
brd->dgnc_Major_TransparentPrint_Registered = true;
}
@@ -289,6 +262,15 @@ int dgnc_tty_register(struct dgnc_board *brd)
brd->dgnc_Serial_Major = brd->serial_driver->major;
brd->dgnc_TransparentPrint_Major = brd->print_driver->major;

+ return 0;
+
+free_print_driver:
+ put_tty_driver(brd->print_driver);
+unregister_serial_driver:
+ tty_unregister_driver(brd->serial_driver);
+free_serial_driver:
+ put_tty_driver(brd->serial_driver);
+
return rc;
}

@@ -433,14 +415,8 @@ void dgnc_tty_uninit(struct dgnc_board *brd)
brd->dgnc_Major_TransparentPrint_Registered = false;
}

- kfree(brd->serial_driver->ttys);
- brd->serial_driver->ttys = NULL;
- kfree(brd->serial_driver->termios);
- brd->serial_driver->termios = NULL;
- kfree(brd->print_driver->ttys);
- brd->print_driver->ttys = NULL;
- kfree(brd->print_driver->termios);
- brd->print_driver->termios = NULL;
+ put_tty_driver(brd->serial_driver);
+ put_tty_driver(brd->print_driver);
}

/*
--
1.9.1


2016-03-21 21:05:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: dgnc: use tty_alloc_driver instead of kcalloc

On Mon, Mar 14, 2016 at 01:29:00PM +0900, Daeseok Youn wrote:
> the tty_alloc_driver() can allocate memory for ttys and termios.
> And also it can release allocated memory easly with using
> put_tty_driver().
>
> Signed-off-by: Daeseok Youn <[email protected]>

But you broke the driver in the previous patch, you can't do that, each
patch has to be 'stand-alone'.

thanks,

greg k-h

2016-03-22 07:40:28

by Daeseok Youn

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: dgnc: use tty_alloc_driver instead of kcalloc

2016-03-22 6:05 GMT+09:00 Greg KH <[email protected]>:
> On Mon, Mar 14, 2016 at 01:29:00PM +0900, Daeseok Youn wrote:
>> the tty_alloc_driver() can allocate memory for ttys and termios.
>> And also it can release allocated memory easly with using
>> put_tty_driver().
>>
>> Signed-off-by: Daeseok Youn <[email protected]>
>
> But you broke the driver in the previous patch, you can't do that, each
> patch has to be 'stand-alone'.
I am not sure about 'stand-alone'. this patch has to have dependency on
previous patch..

I am not sure how to make a series of patches that are related with others.

In this series of patches, 1/3 was failed to merge because changelog entry was
missing. So other patches(2/3, 3/3) was breaking working codes.
I will resend this patch with previous and check more before sending
these patches.
Sorry for the noise..

And I have a question not related this thread,
I had been waiting my patches merged and also have other patches on
same file(in case of dgnc).
I couldn't send next patches until previous patches are taken.
How can I manage my patches for this case?

thanks.

regards,
Daeseok.

>
> thanks,
>
> greg k-h

2016-03-22 13:02:20

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: dgnc: use tty_alloc_driver instead of kcalloc

On Tue, Mar 22, 2016 at 04:40:24PM +0900, DaeSeok Youn wrote:
> 2016-03-22 6:05 GMT+09:00 Greg KH <[email protected]>:
> > On Mon, Mar 14, 2016 at 01:29:00PM +0900, Daeseok Youn wrote:
> >> the tty_alloc_driver() can allocate memory for ttys and termios.
> >> And also it can release allocated memory easly with using
> >> put_tty_driver().
> >>
> >> Signed-off-by: Daeseok Youn <[email protected]>
> >
> > But you broke the driver in the previous patch, you can't do that, each
> > patch has to be 'stand-alone'.
> I am not sure about 'stand-alone'. this patch has to have dependency on
> previous patch..

Yes, that is ok, but your first patch said it was just function
renaming, and then the second patch broke functionality, and the third
fixed it up. You should have a working kernel at each step in your
patch series, which you did not.

> And I have a question not related this thread,
> I had been waiting my patches merged and also have other patches on
> same file(in case of dgnc).
> I couldn't send next patches until previous patches are taken.
> How can I manage my patches for this case?

Just send your new patches and say they depend on the previous ones. My
staging patch queue is almost empty at the moment so odds are I have
already applied, or rejected them.

thanks,

greg k-h

2016-03-23 01:38:21

by Daeseok Youn

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: dgnc: use tty_alloc_driver instead of kcalloc

2016-03-22 22:02 GMT+09:00 Greg KH <[email protected]>:
> On Tue, Mar 22, 2016 at 04:40:24PM +0900, DaeSeok Youn wrote:
>> 2016-03-22 6:05 GMT+09:00 Greg KH <[email protected]>:
>> > On Mon, Mar 14, 2016 at 01:29:00PM +0900, Daeseok Youn wrote:
>> >> the tty_alloc_driver() can allocate memory for ttys and termios.
>> >> And also it can release allocated memory easly with using
>> >> put_tty_driver().
>> >>
>> >> Signed-off-by: Daeseok Youn <[email protected]>
>> >
>> > But you broke the driver in the previous patch, you can't do that, each
>> > patch has to be 'stand-alone'.
>> I am not sure about 'stand-alone'. this patch has to have dependency on
>> previous patch..
>
> Yes, that is ok, but your first patch said it was just function
> renaming, and then the second patch broke functionality, and the third
> fixed it up. You should have a working kernel at each step in your
> patch series, which you did not.
I got it. So I start over from fixing coding style of dgnc module like
I was doing
dgap module.

>
>> And I have a question not related this thread,
>> I had been waiting my patches merged and also have other patches on
>> same file(in case of dgnc).
>> I couldn't send next patches until previous patches are taken.
>> How can I manage my patches for this case?
>
> Just send your new patches and say they depend on the previous ones. My
> staging patch queue is almost empty at the moment so odds are I have
> already applied, or rejected them.
thanks for your explanation. :-)

I will try to make patches more carefully.

regards,
Daeseok.
>
> thanks,
>
> greg k-h