* [RFC 00/42] devm improvement series, part #1
@ 2013-05-10 8:16 Wolfram Sang
2013-05-10 8:17 ` [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource Wolfram Sang
0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2013-05-10 8:16 UTC (permalink / raw)
To: linux-kernel
Cc: Wan ZongShun, linux-mips, alsa-devel, linux-usb, Kevin Hilman,
Wolfram Sang, Alexander Shishkin, Viresh Kumar, Linus Walleij,
Matt Mackall, Sekhar Nori, linux-fbdev, dri-devel,
Jaroslav Kysela, linux-ide, Wim Van Sebroeck, netdev, linux-mtd,
linux-i2c, Evgeniy Polyakov, Jiri Slaby, ac100, devel,
Kukjin Kim, Russell King, Herbert Xu, Florian
Lately, I have been experimenting how to improve the devm interface to make
writing device drivers easier and less error prone while also getting rid of
its subtle issues. I think it has more potential but still needs work and
definately conistency, especiall in its usage.
The first thing I come up with is a low hanging fruit regarding
devm_ioremap_resouce(). This function already checks if the passed resource is
valid and gives an error message if not. So, we can remove similar checks from
the drivers and get rid of a bit of code and a number of inconsistent error
strings.
If generally accepted, I'd suggest I rerun my scripts again when rc1 is out and
ask Linus to pull this branch [1] directly? This series is merely to show what
I am up to.
Thanks,
Wolfram
[1] git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git devm_no_resource_check
Wolfram Sang (42):
drivers/ata: don't check resource with devm_ioremap_resource
drivers/char/hw_random: don't check resource with
devm_ioremap_resource
drivers/cpufreq: don't check resource with devm_ioremap_resource
drivers/dma: don't check resource with devm_ioremap_resource
drivers/gpio: don't check resource with devm_ioremap_resource
drivers/gpu/drm/exynos: don't check resource with
devm_ioremap_resource
drivers/gpu/host1x: don't check resource with devm_ioremap_resource
drivers/gpu/host1x/drm: don't check resource with
devm_ioremap_resource
drivers/i2c/busses: don't check resource with devm_ioremap_resource
drivers/input/keyboard: don't check resource with
devm_ioremap_resource
drivers/input/serio: don't check resource with devm_ioremap_resource
drivers/memory: don't check resource with devm_ioremap_resource
drivers/mfd: don't check resource with devm_ioremap_resource
drivers/misc: don't check resource with devm_ioremap_resource
drivers/mtd/maps: don't check resource with devm_ioremap_resource
drivers/mtd/nand: don't check resource with devm_ioremap_resource
drivers/net/ethernet/renesas: don't check resource with
devm_ioremap_resource
drivers/pinctrl: don't check resource with devm_ioremap_resource
drivers/pinctrl/spear: don't check resource with
devm_ioremap_resource
drivers/pwm: don't check resource with devm_ioremap_resource
drivers/remoteproc: don't check resource with devm_ioremap_resource
drivers/rtc: don't check resource with devm_ioremap_resource
drivers/spi: don't check resource with devm_ioremap_resource
drivers/staging/dwc2: don't check resource with devm_ioremap_resource
drivers/staging/nvec: don't check resource with devm_ioremap_resource
drivers/thermal: don't check resource with devm_ioremap_resource
drivers/tty/serial: don't check resource with devm_ioremap_resource
drivers/usb/chipidea: don't check resource with devm_ioremap_resource
drivers/usb/gadget: don't check resource with devm_ioremap_resource
drivers/usb/host: don't check resource with devm_ioremap_resource
drivers/usb/phy: don't check resource with devm_ioremap_resource
drivers/video: don't check resource with devm_ioremap_resource
drivers/video/omap2: don't check resource with devm_ioremap_resource
drivers/video/omap2/dss: don't check resource with
devm_ioremap_resource
drivers/w1/masters: don't check resource with devm_ioremap_resource
drivers/watchdog: don't check resource with devm_ioremap_resource
arch/arm/mach-tegra: don't check resource with devm_ioremap_resource
arch/arm/plat-omap: don't check resource with devm_ioremap_resource
arch/arm/plat-samsung: don't check resource with
devm_ioremap_resource
arch/mips/lantiq/xway: don't check resource with
devm_ioremap_resource
sound/soc/fsl: don't check resource with devm_ioremap_resource
sound/soc/kirkwood: don't check resource with devm_ioremap_resource
arch/arm/mach-tegra/tegra2_emc.c | 5 -----
arch/arm/plat-omap/dmtimer.c | 7 +------
arch/arm/plat-samsung/adc.c | 5 -----
arch/mips/lantiq/xway/gptu.c | 7 +------
drivers/ata/pata_ep93xx.c | 5 -----
drivers/char/hw_random/mxc-rnga.c | 6 ------
drivers/char/hw_random/omap-rng.c | 5 -----
drivers/cpufreq/kirkwood-cpufreq.c | 4 ----
drivers/dma/tegra20-apb-dma.c | 5 -----
drivers/gpio/gpio-mvebu.c | 12 +-----------
drivers/gpio/gpio-spear-spics.c | 7 +------
drivers/gpio/gpio-tegra.c | 5 -----
drivers/gpu/drm/exynos/exynos_hdmi.c | 5 -----
drivers/gpu/host1x/dev.c | 7 +------
drivers/gpu/host1x/drm/dc.c | 5 -----
drivers/i2c/busses/i2c-davinci.c | 6 +-----
drivers/i2c/busses/i2c-designware-platdrv.c | 6 +-----
drivers/i2c/busses/i2c-imx.c | 6 +-----
drivers/i2c/busses/i2c-omap.c | 6 +-----
drivers/i2c/busses/i2c-rcar.c | 7 +------
drivers/i2c/busses/i2c-s3c2410.c | 5 -----
drivers/i2c/busses/i2c-sirf.c | 6 ------
drivers/i2c/busses/i2c-tegra.c | 5 -----
drivers/input/keyboard/imx_keypad.c | 7 +------
drivers/input/keyboard/spear-keyboard.c | 7 +------
drivers/input/keyboard/tegra-kbc.c | 7 +------
drivers/input/serio/arc_ps2.c | 7 +------
drivers/memory/emif.c | 6 ------
drivers/mfd/intel_msic.c | 6 +-----
drivers/misc/atmel-ssc.c | 5 -----
drivers/mtd/maps/autcpu12-nvram.c | 7 +------
drivers/mtd/maps/lantiq-flash.c | 8 +-------
drivers/mtd/nand/lpc32xx_mlc.c | 5 -----
drivers/mtd/nand/lpc32xx_slc.c | 7 +------
drivers/net/ethernet/renesas/sh_eth.c | 14 +-------------
drivers/pinctrl/pinctrl-at91.c | 7 +------
drivers/pinctrl/pinctrl-coh901.c | 5 -----
drivers/pinctrl/pinctrl-exynos5440.c | 5 -----
drivers/pinctrl/pinctrl-samsung.c | 5 -----
drivers/pinctrl/pinctrl-xway.c | 4 ----
drivers/pinctrl/spear/pinctrl-plgpio.c | 7 +------
drivers/pwm/pwm-imx.c | 5 -----
drivers/pwm/pwm-puv3.c | 5 -----
drivers/pwm/pwm-pxa.c | 5 -----
drivers/pwm/pwm-spear.c | 7 +------
drivers/pwm/pwm-tegra.c | 5 -----
drivers/pwm/pwm-tiecap.c | 5 -----
drivers/pwm/pwm-tiehrpwm.c | 5 -----
drivers/pwm/pwm-tipwmss.c | 5 -----
drivers/pwm/pwm-vt8500.c | 5 -----
drivers/remoteproc/da8xx_remoteproc.c | 14 +-------------
drivers/rtc/rtc-nuc900.c | 6 +-----
drivers/rtc/rtc-omap.c | 5 -----
drivers/rtc/rtc-s3c.c | 5 -----
drivers/rtc/rtc-spear.c | 7 +------
drivers/rtc/rtc-tegra.c | 6 ------
drivers/spi/spi-bcm63xx.c | 8 +-------
drivers/spi/spi-ep93xx.c | 8 +-------
drivers/spi/spi-omap2-mcspi.c | 7 +------
drivers/spi/spi-s3c64xx.c | 7 +------
drivers/spi/spi-sirf.c | 7 +------
drivers/spi/spi-tegra114.c | 7 +------
drivers/spi/spi-tegra20-sflash.c | 5 -----
drivers/spi/spi-tegra20-slink.c | 7 +------
drivers/staging/dwc2/platform.c | 5 -----
drivers/staging/nvec/nvec.c | 5 -----
drivers/thermal/dove_thermal.c | 11 +----------
drivers/thermal/exynos_thermal.c | 5 -----
drivers/thermal/kirkwood_thermal.c | 7 +------
drivers/thermal/rcar_thermal.c | 6 +-----
drivers/tty/serial/serial-tegra.c | 6 +-----
drivers/usb/chipidea/core.c | 5 -----
drivers/usb/gadget/bcm63xx_udc.c | 10 ----------
drivers/usb/host/ehci-atmel.c | 9 +--------
drivers/usb/host/ehci-mxc.c | 8 +-------
drivers/usb/host/ehci-platform.c | 6 +-----
drivers/usb/host/ehci-sh.c | 10 +---------
drivers/usb/host/ohci-nxp.c | 6 ------
drivers/usb/host/ohci-platform.c | 7 +------
drivers/usb/phy/phy-mv-u3d-usb.c | 5 -----
drivers/usb/phy/phy-mxs-usb.c | 5 -----
drivers/usb/phy/phy-samsung-usb2.c | 5 -----
drivers/usb/phy/phy-samsung-usb3.c | 5 -----
drivers/video/mxsfb.c | 7 +------
drivers/video/omap2/dss/hdmi.c | 7 +------
drivers/video/omap2/vrfb.c | 5 -----
drivers/w1/masters/omap_hdq.c | 5 -----
drivers/watchdog/imx2_wdt.c | 5 -----
sound/soc/fsl/imx-ssi.c | 6 ------
sound/soc/kirkwood/kirkwood-i2s.c | 5 -----
90 files changed, 43 insertions(+), 525 deletions(-)
--
1.7.10.4
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource
2013-05-10 8:16 [RFC 00/42] devm improvement series, part #1 Wolfram Sang
@ 2013-05-10 8:17 ` Wolfram Sang
2013-05-10 11:07 ` Sergei Shtylyov
2013-05-11 20:35 ` Sergei Shtylyov
0 siblings, 2 replies; 9+ messages in thread
From: Wolfram Sang @ 2013-05-10 8:17 UTC (permalink / raw)
To: linux-kernel; +Cc: Wolfram Sang, netdev
devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/net/ethernet/renesas/sh_eth.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 33dc6f2..6175839 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2661,14 +2661,6 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
struct sh_eth_private *mdp = NULL;
struct sh_eth_plat_data *pd = pdev->dev.platform_data;
- /* get base addr */
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (unlikely(res == NULL)) {
- dev_err(&pdev->dev, "invalid resource\n");
- ret = -EINVAL;
- goto out;
- }
-
ndev = alloc_etherdev(sizeof(struct sh_eth_private));
if (!ndev) {
ret = -ENOMEM;
@@ -2697,6 +2689,7 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
mdp = netdev_priv(ndev);
mdp->num_tx_ring = TX_RING_SIZE;
mdp->num_rx_ring = RX_RING_SIZE;
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
mdp->addr = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(mdp->addr)) {
ret = PTR_ERR(mdp->addr);
@@ -2745,11 +2738,6 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
if (mdp->cd->tsu) {
struct resource *rtsu;
rtsu = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (!rtsu) {
- dev_err(&pdev->dev, "Not found TSU resource\n");
- ret = -ENODEV;
- goto out_release;
- }
mdp->tsu_addr = devm_ioremap_resource(&pdev->dev, rtsu);
if (IS_ERR(mdp->tsu_addr)) {
ret = PTR_ERR(mdp->tsu_addr);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource
2013-05-10 8:17 ` [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource Wolfram Sang
@ 2013-05-10 11:07 ` Sergei Shtylyov
2013-05-11 20:31 ` Sergei Shtylyov
2013-05-11 20:35 ` Sergei Shtylyov
1 sibling, 1 reply; 9+ messages in thread
From: Sergei Shtylyov @ 2013-05-10 11:07 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, netdev
Hello.
On 10-05-2013 12:17, Wolfram Sang wrote:
> devm_ioremap_resource does sanity checks on the given resource. No need to
> duplicate this in the driver.
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
WBR, Sergei
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource
2013-05-10 11:07 ` Sergei Shtylyov
@ 2013-05-11 20:31 ` Sergei Shtylyov
0 siblings, 0 replies; 9+ messages in thread
From: Sergei Shtylyov @ 2013-05-11 20:31 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, netdev
Hello.
On 05/10/2013 03:07 PM, Sergei Shtylyov wrote:
>
>> devm_ioremap_resource does sanity checks on the given resource. No
>> need to
>> duplicate this in the driver.
>
>> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
>
> Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
I was too fast, I'm now withdawing my ACK.
> WBR, Sergei
>
WBR, Sergei
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource
2013-05-10 8:17 ` [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource Wolfram Sang
2013-05-10 11:07 ` Sergei Shtylyov
@ 2013-05-11 20:35 ` Sergei Shtylyov
2013-05-12 9:42 ` Wolfram Sang
1 sibling, 1 reply; 9+ messages in thread
From: Sergei Shtylyov @ 2013-05-11 20:35 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, netdev
Hello.
On 05/10/2013 12:17 PM, Wolfram Sang wrote:
> devm_ioremap_resource does sanity checks on the given resource. No need to
> duplicate this in the driver.
>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
> ---
> drivers/net/ethernet/renesas/sh_eth.c | 14 +-------------
> 1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 33dc6f2..6175839 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -2661,14 +2661,6 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> struct sh_eth_private *mdp = NULL;
> struct sh_eth_plat_data *pd = pdev->dev.platform_data;
>
> - /* get base addr */
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (unlikely(res == NULL)) {
> - dev_err(&pdev->dev, "invalid resource\n");
> - ret = -EINVAL;
> - goto out;
> - }
> -
> ndev = alloc_etherdev(sizeof(struct sh_eth_private));
> if (!ndev) {
> ret = -ENOMEM;
There's ndev->base_addr = res->start; below this which you broke. NAK.
You should really compile-test your patches.
WBR, Sergei
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource
2013-05-11 20:35 ` Sergei Shtylyov
@ 2013-05-12 9:42 ` Wolfram Sang
2013-05-12 15:26 ` Sergei Shtylyov
0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2013-05-12 9:42 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-kernel, netdev
> You should really compile-test your patches.
Easier said than done with a cleanup series touching various platforms
and architectures. If there are already solutions how to find a .config
compiling the source file modified, I am all ears.
That being said, the actual mistake I did was embarassing, I agree. I am
sorry for that and will improve. And, doing things means doing mistakes,
too.
Regards,
Wolfram
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource
2013-05-12 9:42 ` Wolfram Sang
@ 2013-05-12 15:26 ` Sergei Shtylyov
2013-05-12 16:32 ` Wolfram Sang
0 siblings, 1 reply; 9+ messages in thread
From: Sergei Shtylyov @ 2013-05-12 15:26 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, netdev
Hello.
On 12-05-2013 13:42, Wolfram Sang wrote:
>> You should really compile-test your patches.
> Easier said than done with a cleanup series touching various platforms
> and architectures. If there are already solutions how to find a .config
> compiling the source file modified, I am all ears.
You can always try to do:
$ make drivers/net/ethernet/renesas/sh_eth.o
with a .config file you have. It will compile a file regardless of
whether it's enabled in your .config or not...
> That being said, the actual mistake I did was embarassing, I agree. I am
> sorry for that and will improve. And, doing things means doing mistakes,
> too.
Be more attentive in the future please. :-)
> Regards,
> Wolfram
WBR, Sergei
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource
2013-05-12 15:26 ` Sergei Shtylyov
@ 2013-05-12 16:32 ` Wolfram Sang
2013-05-12 18:33 ` Sergei Shtylyov
0 siblings, 1 reply; 9+ messages in thread
From: Wolfram Sang @ 2013-05-12 16:32 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: linux-kernel, netdev
Hi,
> You can always try to do:
>
> $ make drivers/net/ethernet/renesas/sh_eth.o
>
> with a .config file you have. It will compile a file regardless of
> whether it's enabled in your .config or not...
Been there. This mainly works for platform independent drivers. In this case:
CC drivers/net/ethernet/renesas/sh_eth.o
drivers/net/ethernet/renesas/sh_eth.c: In function 'sh_eth_dev_init':
drivers/net/ethernet/renesas/sh_eth.c:1274:2: error: implicit declaration of function 'sh_eth_reset' [-Werror=implicit-function-declaration]
drivers/net/ethernet/renesas/sh_eth.c: In function 'sh_eth_drv_probe':
drivers/net/ethernet/renesas/sh_eth.c:2724:13: error: 'sh_eth_my_cpu_data' undeclared (first use in this function)
drivers/net/ethernet/renesas/sh_eth.c:2724:13: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/renesas/sh_eth.c: At top level:
drivers/net/ethernet/renesas/sh_eth.c:889:12: warning: 'sh_eth_check_reset' defined but not used [-Wunused-function]
> >That being said, the actual mistake I did was embarassing, I agree. I am
> >sorry for that and will improve. And, doing things means doing mistakes,
> >too.
>
> Be more attentive in the future please. :-)
Sure thing.
Regards,
Wolfram
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource
2013-05-12 16:32 ` Wolfram Sang
@ 2013-05-12 18:33 ` Sergei Shtylyov
0 siblings, 0 replies; 9+ messages in thread
From: Sergei Shtylyov @ 2013-05-12 18:33 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-kernel, netdev
Hello.
On 05/12/2013 08:32 PM, Wolfram Sang wrote:
>
>> You can always try to do:
>>
>> $ make drivers/net/ethernet/renesas/sh_eth.o
>>
>> with a .config file you have. It will compile a file regardless of
>> whether it's enabled in your .config or not...
> Been there. This mainly works for platform independent drivers. In this case:
>
> CC drivers/net/ethernet/renesas/sh_eth.o
> drivers/net/ethernet/renesas/sh_eth.c: In function 'sh_eth_dev_init':
> drivers/net/ethernet/renesas/sh_eth.c:1274:2: error: implicit declaration of function 'sh_eth_reset' [-Werror=implicit-function-declaration]
> drivers/net/ethernet/renesas/sh_eth.c: In function 'sh_eth_drv_probe':
> drivers/net/ethernet/renesas/sh_eth.c:2724:13: error: 'sh_eth_my_cpu_data' undeclared (first use in this function)
> drivers/net/ethernet/renesas/sh_eth.c:2724:13: note: each undeclared identifier is reported only once for each function it appears in
> drivers/net/ethernet/renesas/sh_eth.c: At top level:
> drivers/net/ethernet/renesas/sh_eth.c:889:12: warning: 'sh_eth_check_reset' defined but not used [-Wunused-function]
At least you can try to filter out errors/warning caused by your
patch from
those that are caused by platform dependency. Concerning this driver,
hopefully
the platform dependencies will go away in 3.11.
> Regards,
>
> Wolfram
>
WBR, Sergei
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-05-12 18:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-10 8:16 [RFC 00/42] devm improvement series, part #1 Wolfram Sang
2013-05-10 8:17 ` [RFC 17/42] drivers/net/ethernet/renesas: don't check resource with devm_ioremap_resource Wolfram Sang
2013-05-10 11:07 ` Sergei Shtylyov
2013-05-11 20:31 ` Sergei Shtylyov
2013-05-11 20:35 ` Sergei Shtylyov
2013-05-12 9:42 ` Wolfram Sang
2013-05-12 15:26 ` Sergei Shtylyov
2013-05-12 16:32 ` Wolfram Sang
2013-05-12 18:33 ` Sergei Shtylyov
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).