From: "Enrico Weigelt, metux IT consult" <lkml@metux.net>
To: Greg KH <gregkh@linuxfoundation.org>,
"Enrico Weigelt, metux IT consult" <info@metux.net>
Cc: linux-kernel@vger.kernel.org, eric@anholt.net,
stefan.wahren@i2se.com, f.fainelli@gmail.com, rjui@broadcom.com,
sbranden@broadcom.com, bcm-kernel-feedback-list@broadcom.com,
andriy.shevchenko@linux.intel.com, vz@mleia.com,
matthias.bgg@gmail.com, yamada.masahiro@socionext.com,
tklauser@distanz.ch, richard.genoud@gmail.com,
macro@linux-mips.org, u.kleine-koenig@pengutronix.de,
kernel@pengutronix.de, slemieux.tyco@gmail.com,
andy.gross@linaro.org, david.brown@linaro.org,
shawnguo@kernel.org, s.hauer@pengutronix.de, festevam@gmail.com,
linux-imx@nxp.com, baohua@kernel.org, jacmet@sunsite.dk,
linux-serial@vger.kernel.org, linux-arm-msm@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
Date: Fri, 15 Mar 2019 10:06:16 +0100 [thread overview]
Message-ID: <3734d588-6b9c-29e2-45b6-82e778f47602@metux.net> (raw)
In-Reply-To: <20190314225204.GB1795@kroah.com>
On 14.03.19 23:52, Greg KH wrote:
> On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
>> Use the safer devm versions of memory mapping functions.
>
> What is "safer" about them?
Garbage collection :)
Several drivers didn't seem to clean up properly (maybe these're just
used compiled-in, so nobody noticed yet).
In general, I think devm_* should be the standard case, unless there's
a really good reason to do otherwise.
<snip>
> Isn't the whole goal of the devm* functions such that you are not
> required to call "release" on them?
Looks that one slipped through, when I was doing that big bulk change
in the middle of the night and not enough coffe ;-)
One problem here is that many drivers do this stuff in request/release
port, instead of probe/remove. I'm not sure yet, whether we should
rewrite that. There're also cases which do request/release, but no
ioremap (doing hardcoded register accesses), others do ioremap w/o
request/release.
IMHO, we should have a closer look at those and check whether that's
really okay (just adding request/release blindly could cause trouble)
> And also, why make the change, you aren't changing any functionality for
> these old drivers at all from what I can tell (for the devm calls).
> What am I missing here?
Okay, there's a bigger story behind, you can't know yet. Finally, I'd
like to move everything to using struct resource and corresponding
helpers consistently, so most of the drivers would be pretty simple
at that point. (there're of course special cases, like devices w/
multiple register spaces, etc)
Here's my wip branch:
https://github.com/metux/linux/commits/wip/serial-res
In this consolidation process, I'm trying to move everything to
devm_*, to have it more generic (for now, I still need two versions
of the request/release/ioremap/iounmap helpers - one w/ and one
w/o devm).
My idea was moving to devm first, so it can be reviewed/tested
independently, before moving forward. Smaller, easily digestable
pieces should minimize the risk of breaking anything. But if you
prefer having this things squashed together, just let me know.
In the queue are also other minor cleanups like using dev_err()
instead of printk(), etc. Should I send these separately ?
By the way: do you have some public branch where you're collecting
accepted patches, which I could base mine on ? (tty.git/tty-next ?)
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
WARNING: multiple messages have this Message-ID (diff)
From: "Enrico Weigelt, metux IT consult" <lkml@metux.net>
To: Greg KH <gregkh@linuxfoundation.org>,
"Enrico Weigelt, metux IT consult" <info@metux.net>
Cc: linux-arm-msm@vger.kernel.org, yamada.masahiro@socionext.com,
macro@linux-mips.org, jacmet@sunsite.dk, festevam@gmail.com,
stefan.wahren@i2se.com, f.fainelli@gmail.com,
bcm-kernel-feedback-list@broadcom.com, linux-imx@nxp.com,
linux-serial@vger.kernel.org, slemieux.tyco@gmail.com,
andy.gross@linaro.org, tklauser@distanz.ch,
david.brown@linaro.org, rjui@broadcom.com,
s.hauer@pengutronix.de, u.kleine-koenig@pengutronix.de,
vz@mleia.com, matthias.bgg@gmail.com,
andriy.shevchenko@linux.intel.com, baohua@kernel.org,
sbranden@broadcom.com, eric@anholt.net, richard.genoud@gmail.com,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
kernel@pengutronix.de, shawnguo@kernel.org
Subject: Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
Date: Fri, 15 Mar 2019 10:06:16 +0100 [thread overview]
Message-ID: <3734d588-6b9c-29e2-45b6-82e778f47602@metux.net> (raw)
In-Reply-To: <20190314225204.GB1795@kroah.com>
On 14.03.19 23:52, Greg KH wrote:
> On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
>> Use the safer devm versions of memory mapping functions.
>
> What is "safer" about them?
Garbage collection :)
Several drivers didn't seem to clean up properly (maybe these're just
used compiled-in, so nobody noticed yet).
In general, I think devm_* should be the standard case, unless there's
a really good reason to do otherwise.
<snip>
> Isn't the whole goal of the devm* functions such that you are not
> required to call "release" on them?
Looks that one slipped through, when I was doing that big bulk change
in the middle of the night and not enough coffe ;-)
One problem here is that many drivers do this stuff in request/release
port, instead of probe/remove. I'm not sure yet, whether we should
rewrite that. There're also cases which do request/release, but no
ioremap (doing hardcoded register accesses), others do ioremap w/o
request/release.
IMHO, we should have a closer look at those and check whether that's
really okay (just adding request/release blindly could cause trouble)
> And also, why make the change, you aren't changing any functionality for
> these old drivers at all from what I can tell (for the devm calls).
> What am I missing here?
Okay, there's a bigger story behind, you can't know yet. Finally, I'd
like to move everything to using struct resource and corresponding
helpers consistently, so most of the drivers would be pretty simple
at that point. (there're of course special cases, like devices w/
multiple register spaces, etc)
Here's my wip branch:
https://github.com/metux/linux/commits/wip/serial-res
In this consolidation process, I'm trying to move everything to
devm_*, to have it more generic (for now, I still need two versions
of the request/release/ioremap/iounmap helpers - one w/ and one
w/o devm).
My idea was moving to devm first, so it can be reviewed/tested
independently, before moving forward. Smaller, easily digestable
pieces should minimize the risk of breaking anything. But if you
prefer having this things squashed together, just let me know.
In the queue are also other minor cleanups like using dev_err()
instead of printk(), etc. Should I send these separately ?
By the way: do you have some public branch where you're collecting
accepted patches, which I could base mine on ? (tty.git/tty-next ?)
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
next prev parent reply other threads:[~2019-03-15 9:06 UTC|newest]
Thread overview: 142+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-14 22:33 serial driver cleanups v2 Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 01/45] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource() Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource() Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-15 9:04 ` Andy Shevchenko
2019-03-15 9:04 ` Andy Shevchenko
2019-03-15 9:04 ` Andy Shevchenko
2019-03-15 9:37 ` Enrico Weigelt, metux IT consult
2019-03-15 9:37 ` Enrico Weigelt, metux IT consult
2019-03-15 9:37 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 03/45] drivers: tty: serial: 8250_ingenic: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 04/45] drivers: tty: serial: amba-pl010: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 05/45] drivers: tty: serial: sirfsoc_uart: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 06/45] drivers: tty: serial: samsung: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 07/45] drivers: tty: serial: 8250_uniphier: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-18 7:44 ` Masahiro Yamada
2019-03-18 7:44 ` Masahiro Yamada
2019-03-18 7:44 ` Masahiro Yamada
2019-03-14 22:33 ` [PATCH v2 08/45] drivers: tty: serial: 8250_lpc18xx: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 09/45] drivers: tty: serial: 8250_mtk: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:52 ` Greg KH
2019-03-14 22:52 ` Greg KH
2019-03-15 9:06 ` Enrico Weigelt, metux IT consult [this message]
2019-03-15 9:06 ` Enrico Weigelt, metux IT consult
2019-03-15 14:26 ` Greg KH
2019-03-15 14:26 ` Greg KH
2019-03-15 19:12 ` Enrico Weigelt, metux IT consult
2019-03-15 19:12 ` Enrico Weigelt, metux IT consult
2019-03-16 3:26 ` Greg KH
2019-03-16 3:26 ` Greg KH
2019-03-16 9:17 ` Enrico Weigelt, metux IT consult
2019-03-16 9:17 ` Enrico Weigelt, metux IT consult
2019-03-17 9:54 ` Greg KH
2019-03-17 9:54 ` Greg KH
2019-03-18 8:03 ` Maciej W. Rozycki
2019-03-18 8:03 ` Maciej W. Rozycki
2019-03-14 22:33 ` [PATCH v2 11/45] drivers: tty: serial: zs: use dev_err() instead of printk() Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 12/45] drivers: tty: serial: xilinx_uartps: use devm_* functions Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 13/45] drivers: tty: serial: 21285: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 14/45] drivers: tty: serial: vr41xx_siu: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 15/45] drivers: tty: serial: uartlite: use dev_err() instead of printk() Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 16/45] drivers: tty: serial: uartlite: use devm_* functions Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 17/45] drivers: tty: serial: timuart: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 18/45] drivers: tty: serial: sirfsoc_uart: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 19/45] drivers: tty: serial: sh-sci: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 20/45] drivers: tty: serial: msm_serial: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-05-27 5:57 ` Bjorn Andersson
2019-05-27 5:57 ` Bjorn Andersson
2019-03-14 22:33 ` [PATCH v2 21/45] drivers: tty: serial: altera_jtaguart: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 22/45] drivers: tty: serial: altera_uart: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 23/45] drivers: tty: serial: amba-pl010: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 24/45] drivers: tty: serial: mxs-auart: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 25/45] drivers: tty: serial: pxa: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 26/45] drivers: tty: serial: dz: use dev_err() instead of printk() Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 27/45] drivers: tty: serial: dz: use devm_* functions Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 28/45] drivers: tty: serial: netx-serial: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` [PATCH v2 29/45] drivers: tty: serial: serial_txx9: " Enrico Weigelt, metux IT consult
2019-03-14 22:33 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 30/45] drivers: tty: serial: serial_ks8695: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 31/45] drivers: tty: serial: amba-pl011: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 32/45] drivers: tty: serial: atmel_serial: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 33/45] drivers: tty: serial: sb1250-duart: use dev_err() instead of printk() Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 34/45] drivers: tty: serial: sb1250-duart: use devm_* functions Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 35/45] drivers: tty: serial: lpc32xx_hs: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 36/45] drivers: tty: serial: pic32_uart: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 37/45] drivers: tty: serial: apbuart: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 38/45] drivers: tty: serial: samsung: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 39/45] drivers: tty: serial: efm32-uart: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-15 7:46 ` Uwe Kleine-König
2019-03-15 7:46 ` Uwe Kleine-König
2019-03-14 22:34 ` [PATCH v2 40/45] drivers: tty: serial: mpc52xx_uart: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 41/45] drivers: tty: serial: timuart: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 42/45] drivers: tty: serial: sa1100: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 43/45] drivers: tty: serial: pmac_zilog: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 44/45] drivers: tty: serial: pnx8xxx_uart: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` [PATCH v2 45/45] drivers: tty: serial: mux: " Enrico Weigelt, metux IT consult
2019-03-14 22:34 ` Enrico Weigelt, metux IT consult
2019-03-15 9:08 ` Andy Shevchenko
2019-03-15 9:08 ` Andy Shevchenko
2019-03-15 9:08 ` Andy Shevchenko
2019-03-15 9:12 ` serial driver cleanups v2 Andy Shevchenko
2019-03-15 9:12 ` Andy Shevchenko
2019-03-15 9:12 ` Andy Shevchenko
2019-03-15 9:20 ` Andy Shevchenko
2019-03-15 9:20 ` Andy Shevchenko
2019-03-15 9:20 ` Andy Shevchenko
2019-03-15 10:36 ` Enrico Weigelt, metux IT consult
2019-03-15 10:36 ` Enrico Weigelt, metux IT consult
2019-03-15 10:36 ` Enrico Weigelt, metux IT consult
2019-03-15 18:11 ` Andy Shevchenko
2019-03-15 18:11 ` Andy Shevchenko
2019-03-15 18:11 ` Andy Shevchenko
2019-03-15 18:41 ` Enrico Weigelt, metux IT consult
2019-03-15 18:41 ` Enrico Weigelt, metux IT consult
2019-03-15 18:41 ` Enrico Weigelt, metux IT consult
2019-03-15 18:45 ` Andy Shevchenko
2019-03-15 18:45 ` Andy Shevchenko
2019-03-15 18:45 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3734d588-6b9c-29e2-45b6-82e778f47602@metux.net \
--to=lkml@metux.net \
--cc=andriy.shevchenko@linux.intel.com \
--cc=andy.gross@linaro.org \
--cc=baohua@kernel.org \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=david.brown@linaro.org \
--cc=eric@anholt.net \
--cc=f.fainelli@gmail.com \
--cc=festevam@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=info@metux.net \
--cc=jacmet@sunsite.dk \
--cc=kernel@pengutronix.de \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=macro@linux-mips.org \
--cc=matthias.bgg@gmail.com \
--cc=richard.genoud@gmail.com \
--cc=rjui@broadcom.com \
--cc=s.hauer@pengutronix.de \
--cc=sbranden@broadcom.com \
--cc=shawnguo@kernel.org \
--cc=slemieux.tyco@gmail.com \
--cc=stefan.wahren@i2se.com \
--cc=tklauser@distanz.ch \
--cc=u.kleine-koenig@pengutronix.de \
--cc=vz@mleia.com \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.